From 5306c73b008567d855ca548d195abf3dfaf8917c Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 26 Apr 2022 13:08:54 +0100 Subject: [PATCH] Fix bug when uploading device signatures (#2377) * Find the complete key ID when uploading signatures * Try that again * Try splitting the right thing * Don't do it for device keys * Refactor `QuerySignatures` * Revert "Refactor `QuerySignatures`" This reverts commit c02832a3e92569f64f180dec1555056dc8f8c3e3. * Both requested key IDs and master/self/user keys * Fix uniqueness * Try tweaking GMSL * Update GMSL again * Revert "Update GMSL again" This reverts commit bd6916cc379dd8d9e3f38d979c6550bd658938aa. * Revert "Try tweaking GMSL" This reverts commit 2a054524da9d64c6a2a5228262fbba5fde28798c. * Database migrations --- keyserver/internal/cross_signing.go | 7 ++ .../postgres/cross_signing_sigs_table.go | 6 +- .../deltas/2022042612000000_xsigning_idx.go | 52 +++++++++++++ keyserver/storage/postgres/storage.go | 1 + .../sqlite3/cross_signing_sigs_table.go | 4 +- .../deltas/2022042612000000_xsigning_idx.go | 76 +++++++++++++++++++ keyserver/storage/sqlite3/storage.go | 1 + 7 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 keyserver/storage/postgres/deltas/2022042612000000_xsigning_idx.go create mode 100644 keyserver/storage/sqlite3/deltas/2022042612000000_xsigning_idx.go diff --git a/keyserver/internal/cross_signing.go b/keyserver/internal/cross_signing.go index 2281f4bbf..08bbfedb8 100644 --- a/keyserver/internal/cross_signing.go +++ b/keyserver/internal/cross_signing.go @@ -362,6 +362,13 @@ func (a *KeyInternalAPI) processSelfSignatures( for targetKeyID, signature := range forTargetUserID { switch sig := signature.CrossSigningBody.(type) { case *gomatrixserverlib.CrossSigningKey: + for keyID := range sig.Keys { + split := strings.SplitN(string(keyID), ":", 2) + if len(split) > 1 && gomatrixserverlib.KeyID(split[1]) == targetKeyID { + targetKeyID = keyID // contains the ed25519: or other scheme + break + } + } for originUserID, forOriginUserID := range sig.Signatures { for originKeyID, originSig := range forOriginUserID { if err := a.DB.StoreCrossSigningSigsForTarget( diff --git a/keyserver/storage/postgres/cross_signing_sigs_table.go b/keyserver/storage/postgres/cross_signing_sigs_table.go index 40633c05c..b101e7ce5 100644 --- a/keyserver/storage/postgres/cross_signing_sigs_table.go +++ b/keyserver/storage/postgres/cross_signing_sigs_table.go @@ -33,8 +33,10 @@ CREATE TABLE IF NOT EXISTS keyserver_cross_signing_sigs ( target_user_id TEXT NOT NULL, target_key_id TEXT NOT NULL, signature TEXT NOT NULL, - PRIMARY KEY (origin_user_id, target_user_id, target_key_id) + PRIMARY KEY (origin_user_id, origin_key_id, target_user_id, target_key_id) ); + +CREATE INDEX IF NOT EXISTS keyserver_cross_signing_sigs_idx ON keyserver_cross_signing_sigs (origin_user_id, target_user_id, target_key_id); ` const selectCrossSigningSigsForTargetSQL = "" + @@ -44,7 +46,7 @@ const selectCrossSigningSigsForTargetSQL = "" + const upsertCrossSigningSigsForTargetSQL = "" + "INSERT INTO keyserver_cross_signing_sigs (origin_user_id, origin_key_id, target_user_id, target_key_id, signature)" + " VALUES($1, $2, $3, $4, $5)" + - " ON CONFLICT (origin_user_id, target_user_id, target_key_id) DO UPDATE SET (origin_key_id, signature) = ($2, $5)" + " ON CONFLICT (origin_user_id, origin_key_id, target_user_id, target_key_id) DO UPDATE SET signature = $5" const deleteCrossSigningSigsForTargetSQL = "" + "DELETE FROM keyserver_cross_signing_sigs WHERE target_user_id=$1 AND target_key_id=$2" diff --git a/keyserver/storage/postgres/deltas/2022042612000000_xsigning_idx.go b/keyserver/storage/postgres/deltas/2022042612000000_xsigning_idx.go new file mode 100644 index 000000000..12956e3b4 --- /dev/null +++ b/keyserver/storage/postgres/deltas/2022042612000000_xsigning_idx.go @@ -0,0 +1,52 @@ +// Copyright 2022 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package deltas + +import ( + "database/sql" + "fmt" + + "github.com/matrix-org/dendrite/internal/sqlutil" +) + +func LoadFixCrossSigningSignatureIndexes(m *sqlutil.Migrations) { + m.AddMigration(UpFixCrossSigningSignatureIndexes, DownFixCrossSigningSignatureIndexes) +} + +func UpFixCrossSigningSignatureIndexes(tx *sql.Tx) error { + _, err := tx.Exec(` + ALTER TABLE keyserver_cross_signing_sigs DROP CONSTRAINT keyserver_cross_signing_sigs_pkey; + ALTER TABLE keyserver_cross_signing_sigs ADD PRIMARY KEY (origin_user_id, origin_key_id, target_user_id, target_key_id); + + CREATE INDEX IF NOT EXISTS keyserver_cross_signing_sigs_idx ON keyserver_cross_signing_sigs (origin_user_id, target_user_id, target_key_id); + `) + if err != nil { + return fmt.Errorf("failed to execute upgrade: %w", err) + } + return nil +} + +func DownFixCrossSigningSignatureIndexes(tx *sql.Tx) error { + _, err := tx.Exec(` + ALTER TABLE keyserver_cross_signing_sigs DROP CONSTRAINT keyserver_cross_signing_sigs_pkey; + ALTER TABLE keyserver_cross_signing_sigs ADD PRIMARY KEY (origin_user_id, target_user_id, target_key_id); + + DROP INDEX IF EXISTS keyserver_cross_signing_sigs_idx; + `) + if err != nil { + return fmt.Errorf("failed to execute downgrade: %w", err) + } + return nil +} diff --git a/keyserver/storage/postgres/storage.go b/keyserver/storage/postgres/storage.go index 136986885..d4c7e2cc7 100644 --- a/keyserver/storage/postgres/storage.go +++ b/keyserver/storage/postgres/storage.go @@ -54,6 +54,7 @@ func NewDatabase(dbProperties *config.DatabaseOptions) (*shared.Database, error) } m := sqlutil.NewMigrations() deltas.LoadRefactorKeyChanges(m) + deltas.LoadFixCrossSigningSignatureIndexes(m) if err = m.RunDeltas(db, dbProperties); err != nil { return nil, err } diff --git a/keyserver/storage/sqlite3/cross_signing_sigs_table.go b/keyserver/storage/sqlite3/cross_signing_sigs_table.go index 29ee889fd..36d562b8a 100644 --- a/keyserver/storage/sqlite3/cross_signing_sigs_table.go +++ b/keyserver/storage/sqlite3/cross_signing_sigs_table.go @@ -33,8 +33,10 @@ CREATE TABLE IF NOT EXISTS keyserver_cross_signing_sigs ( target_user_id TEXT NOT NULL, target_key_id TEXT NOT NULL, signature TEXT NOT NULL, - PRIMARY KEY (origin_user_id, target_user_id, target_key_id) + PRIMARY KEY (origin_user_id, origin_key_id, target_user_id, target_key_id) ); + +CREATE INDEX IF NOT EXISTS keyserver_cross_signing_sigs_idx ON keyserver_cross_signing_sigs (origin_user_id, target_user_id, target_key_id); ` const selectCrossSigningSigsForTargetSQL = "" + diff --git a/keyserver/storage/sqlite3/deltas/2022042612000000_xsigning_idx.go b/keyserver/storage/sqlite3/deltas/2022042612000000_xsigning_idx.go new file mode 100644 index 000000000..230e39fef --- /dev/null +++ b/keyserver/storage/sqlite3/deltas/2022042612000000_xsigning_idx.go @@ -0,0 +1,76 @@ +// Copyright 2022 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package deltas + +import ( + "database/sql" + "fmt" + + "github.com/matrix-org/dendrite/internal/sqlutil" +) + +func LoadFixCrossSigningSignatureIndexes(m *sqlutil.Migrations) { + m.AddMigration(UpFixCrossSigningSignatureIndexes, DownFixCrossSigningSignatureIndexes) +} + +func UpFixCrossSigningSignatureIndexes(tx *sql.Tx) error { + _, err := tx.Exec(` + CREATE TABLE IF NOT EXISTS keyserver_cross_signing_sigs_tmp ( + origin_user_id TEXT NOT NULL, + origin_key_id TEXT NOT NULL, + target_user_id TEXT NOT NULL, + target_key_id TEXT NOT NULL, + signature TEXT NOT NULL, + PRIMARY KEY (origin_user_id, origin_key_id, target_user_id, target_key_id) + ); + + INSERT INTO keyserver_cross_signing_sigs_tmp (origin_user_id, origin_key_id, target_user_id, target_key_id, signature) + SELECT origin_user_id, origin_key_id, target_user_id, target_key_id, signature FROM keyserver_cross_signing_sigs; + + DROP TABLE keyserver_cross_signing_sigs; + ALTER TABLE keyserver_cross_signing_sigs_tmp RENAME TO keyserver_cross_signing_sigs; + + CREATE INDEX IF NOT EXISTS keyserver_cross_signing_sigs_idx ON keyserver_cross_signing_sigs (origin_user_id, target_user_id, target_key_id); + `) + if err != nil { + return fmt.Errorf("failed to execute upgrade: %w", err) + } + return nil +} + +func DownFixCrossSigningSignatureIndexes(tx *sql.Tx) error { + _, err := tx.Exec(` + CREATE TABLE IF NOT EXISTS keyserver_cross_signing_sigs_tmp ( + origin_user_id TEXT NOT NULL, + origin_key_id TEXT NOT NULL, + target_user_id TEXT NOT NULL, + target_key_id TEXT NOT NULL, + signature TEXT NOT NULL, + PRIMARY KEY (origin_user_id, target_user_id, target_key_id) + ); + + INSERT INTO keyserver_cross_signing_sigs_tmp (origin_user_id, origin_key_id, target_user_id, target_key_id, signature) + SELECT origin_user_id, origin_key_id, target_user_id, target_key_id, signature FROM keyserver_cross_signing_sigs; + + DROP TABLE keyserver_cross_signing_sigs; + ALTER TABLE keyserver_cross_signing_sigs_tmp RENAME TO keyserver_cross_signing_sigs; + + DELETE INDEX IF EXISTS keyserver_cross_signing_sigs_idx; + `) + if err != nil { + return fmt.Errorf("failed to execute downgrade: %w", err) + } + return nil +} diff --git a/keyserver/storage/sqlite3/storage.go b/keyserver/storage/sqlite3/storage.go index 0e0adceef..84d4cdf55 100644 --- a/keyserver/storage/sqlite3/storage.go +++ b/keyserver/storage/sqlite3/storage.go @@ -53,6 +53,7 @@ func NewDatabase(dbProperties *config.DatabaseOptions) (*shared.Database, error) m := sqlutil.NewMigrations() deltas.LoadRefactorKeyChanges(m) + deltas.LoadFixCrossSigningSignatureIndexes(m) if err = m.RunDeltas(db, dbProperties); err != nil { return nil, err }