MSC3938: Remove {keyID} from key lookups over federation

federationapi/internal/query.go

@@ -46,14 +46,14 @@ func (a *FederationInternalAPI) fetchServerKeysFromCache(
 
 	// We got a request for _all_ server keys, return them.
 	if len(req.KeyIDToCriteria) == 0 {
-		serverKeysResponses, _ := a.db.GetNotaryKeys(ctx, req.ServerName, []gomatrixserverlib.KeyID{})
+		serverKeysResponses, _ := a.db.GetNotaryKeys(ctx, req.ServerName)
 		if len(serverKeysResponses) == 0 {
 			return nil, fmt.Errorf("failed to find server key response for server %s", req.ServerName)
 		}
 		return serverKeysResponses, nil
 	}
 	for keyID, criteria := range req.KeyIDToCriteria {
-		serverKeysResponses, _ := a.db.GetNotaryKeys(ctx, req.ServerName, []gomatrixserverlib.KeyID{keyID})
+		serverKeysResponses, _ := a.db.GetNotaryKeys(ctx, req.ServerName)
 		if len(serverKeysResponses) == 0 {
 			return nil, fmt.Errorf("failed to find server key response for key ID %s", keyID)
 		}
@@ -90,7 +90,7 @@ func (a *FederationInternalAPI) QueryServerKeys(
 	if err != nil {
 		// try to load as much as we can from the cache in a best effort basis
 		util.GetLogger(ctx).WithField("server", req.ServerName).WithError(err).Warn("notary: failed to ask server for keys, returning best effort keys")
-		serverKeysResponses, dbErr := a.db.GetNotaryKeys(ctx, req.ServerName, req.KeyIDs())
+		serverKeysResponses, dbErr := a.db.GetNotaryKeys(ctx, req.ServerName)
 		if dbErr != nil {
 			return fmt.Errorf("notary: server returned %s, and db returned %s", err, dbErr)
 		}

federationapi/routing/routing.go

@@ -77,24 +77,17 @@ func Setup(
 		FsAPI: fsAPI,
 	}
 
-	localKeys := httputil.MakeExternalAPI("localkeys", func(req *http.Request) util.JSONResponse {
-		return LocalKeys(cfg, spec.ServerName(req.Host))
-	})
-
 	notaryKeys := httputil.MakeExternalAPI("notarykeys", func(req *http.Request) util.JSONResponse {
 		vars, err := httputil.URLDecodeMapValues(mux.Vars(req))
 		if err != nil {
 			return util.ErrorResponse(err)
 		}
 		var pkReq *gomatrixserverlib.PublicKeyNotaryLookupRequest
 		serverName := spec.ServerName(vars["serverName"])
-		keyID := gomatrixserverlib.KeyID(vars["keyID"])
-		if serverName != "" && keyID != "" {
+		if serverName != "" {
 			pkReq = &gomatrixserverlib.PublicKeyNotaryLookupRequest{
 				ServerKeys: map[spec.ServerName]map[gomatrixserverlib.KeyID]gomatrixserverlib.PublicKeyNotaryQueryCriteria{
-					serverName: {
-						keyID: gomatrixserverlib.PublicKeyNotaryQueryCriteria{},
-					},
+					serverName: {},
 				},
 			}
 		}
@@ -120,11 +113,11 @@ func Setup(
 	// return that key.
 	// Even if we had more than one server key, we would probably still ignore the
 	// {keyID} argument and always return a response containing all of the keys.
-	v2keysmux.Handle("/server/{keyID}", localKeys).Methods(http.MethodGet)
-	v2keysmux.Handle("/server/", localKeys).Methods(http.MethodGet)
-	v2keysmux.Handle("/server", localKeys).Methods(http.MethodGet)
+	v2keysmux.Handle("/server", httputil.MakeExternalAPI("localkeys", func(req *http.Request) util.JSONResponse {
+		return LocalKeys(cfg, spec.ServerName(req.Host))
+	})).Methods(http.MethodGet)
 	v2keysmux.Handle("/query", notaryKeys).Methods(http.MethodPost)
-	v2keysmux.Handle("/query/{serverName}/{keyID}", notaryKeys).Methods(http.MethodGet)
+	v2keysmux.Handle("/query/{serverName}", notaryKeys).Methods(http.MethodGet)
 
 	mu := internal.NewMutexByRoom()
 	v1fedmux.Handle("/send/{txnID}", MakeFedAPI(

federationapi/storage/interface.go

@@ -73,9 +73,8 @@ type Database interface {
 
 	// Update the notary with the given server keys from the given server name.
 	UpdateNotaryKeys(ctx context.Context, serverName spec.ServerName, serverKeys gomatrixserverlib.ServerKeys) error
-	// Query the notary for the server keys for the given server. If `optKeyIDs` is not empty, multiple server keys may be returned (between 1 - len(optKeyIDs))
-	// such that the combination of all server keys will include all the `optKeyIDs`.
-	GetNotaryKeys(ctx context.Context, serverName spec.ServerName, optKeyIDs []gomatrixserverlib.KeyID) ([]gomatrixserverlib.ServerKeys, error)
+	// Query the notary for the server keys for the given server.
+	GetNotaryKeys(ctx context.Context, serverName spec.ServerName) ([]gomatrixserverlib.ServerKeys, error)
 	// DeleteExpiredEDUs cleans up expired EDUs
 	DeleteExpiredEDUs(ctx context.Context) error
 

federationapi/storage/postgres/notary_server_keys_metadata_table.go

@@ -14,7 +14,6 @@ import (
 	"github.com/element-hq/dendrite/federationapi/storage/tables"
 	"github.com/element-hq/dendrite/internal"
 	"github.com/element-hq/dendrite/internal/sqlutil"
-	"github.com/lib/pq"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/gomatrixserverlib/spec"
 )
@@ -50,16 +49,6 @@ const selectNotaryKeyResponsesSQL = `
 	)
 `
 
-// select the responses which have the given key IDs
-// JOINs with the json table
-const selectNotaryKeyResponsesWithKeyIDsSQL = `
-	SELECT response_json FROM federationsender_notary_server_keys_json
-	JOIN federationsender_notary_server_keys_metadata ON
-	federationsender_notary_server_keys_metadata.notary_id = federationsender_notary_server_keys_json.notary_id
-	WHERE federationsender_notary_server_keys_json.server_name = $1 AND federationsender_notary_server_keys_metadata.key_id = ANY ($2)
-	GROUP BY federationsender_notary_server_keys_json.notary_id
-`
-
 // JOINs with the metadata table
 const deleteUnusedServerKeysJSONSQL = `
 	DELETE FROM federationsender_notary_server_keys_json WHERE federationsender_notary_server_keys_json.notary_id NOT IN (
@@ -68,12 +57,11 @@ const deleteUnusedServerKeysJSONSQL = `
 `
 
 type notaryServerKeysMetadataStatements struct {
-	db                                     *sql.DB
-	upsertServerKeysStmt                   *sql.Stmt
-	selectNotaryKeyResponsesStmt           *sql.Stmt
-	selectNotaryKeyResponsesWithKeyIDsStmt *sql.Stmt
-	selectNotaryKeyMetadataStmt            *sql.Stmt
-	deleteUnusedServerKeysJSONStmt         *sql.Stmt
+	db                             *sql.DB
+	upsertServerKeysStmt           *sql.Stmt
+	selectNotaryKeyResponsesStmt   *sql.Stmt
+	selectNotaryKeyMetadataStmt    *sql.Stmt
+	deleteUnusedServerKeysJSONStmt *sql.Stmt
 }
 
 func NewPostgresNotaryServerKeysMetadataTable(db *sql.DB) (s *notaryServerKeysMetadataStatements, err error) {
@@ -88,7 +76,6 @@ func NewPostgresNotaryServerKeysMetadataTable(db *sql.DB) (s *notaryServerKeysMe
 	return s, sqlutil.StatementList{
 		{&s.upsertServerKeysStmt, upsertServerKeysSQL},
 		{&s.selectNotaryKeyResponsesStmt, selectNotaryKeyResponsesSQL},
-		{&s.selectNotaryKeyResponsesWithKeyIDsStmt, selectNotaryKeyResponsesWithKeyIDsSQL},
 		{&s.selectNotaryKeyMetadataStmt, selectNotaryKeyMetadataSQL},
 		{&s.deleteUnusedServerKeysJSONStmt, deleteUnusedServerKeysJSONSQL},
 	}.Prepare(db)
@@ -115,18 +102,11 @@ func (s *notaryServerKeysMetadataStatements) UpsertKey(
 	return notaryID, err
 }
 
-func (s *notaryServerKeysMetadataStatements) SelectKeys(ctx context.Context, txn *sql.Tx, serverName spec.ServerName, keyIDs []gomatrixserverlib.KeyID) ([]gomatrixserverlib.ServerKeys, error) {
+func (s *notaryServerKeysMetadataStatements) SelectKeys(ctx context.Context, txn *sql.Tx, serverName spec.ServerName) ([]gomatrixserverlib.ServerKeys, error) {
 	var rows *sql.Rows
 	var err error
-	if len(keyIDs) == 0 {
-		rows, err = txn.Stmt(s.selectNotaryKeyResponsesStmt).QueryContext(ctx, string(serverName))
-	} else {
-		keyIDstr := make([]string, len(keyIDs))
-		for i := range keyIDs {
-			keyIDstr[i] = string(keyIDs[i])
-		}
-		rows, err = txn.Stmt(s.selectNotaryKeyResponsesWithKeyIDsStmt).QueryContext(ctx, string(serverName), pq.StringArray(keyIDstr))
-	}
+
+	rows, err = txn.Stmt(s.selectNotaryKeyResponsesStmt).QueryContext(ctx, string(serverName))
 	if err != nil {
 		return nil, err
 	}

federationapi/storage/shared/storage.go

@@ -358,10 +358,9 @@ func (d *Database) UpdateNotaryKeys(
 func (d *Database) GetNotaryKeys(
 	ctx context.Context,
 	serverName spec.ServerName,
-	optKeyIDs []gomatrixserverlib.KeyID,
 ) (sks []gomatrixserverlib.ServerKeys, err error) {
 	err = d.Writer.Do(d.DB, nil, func(txn *sql.Tx) error {
-		sks, err = d.NotaryServerKeysMetadata.SelectKeys(ctx, txn, serverName, optKeyIDs)
+		sks, err = d.NotaryServerKeysMetadata.SelectKeys(ctx, txn, serverName)
 		return err
 	})
 	return sks, err

federationapi/storage/sqlite3/notary_server_keys_metadata_table.go

@@ -10,8 +10,6 @@ import (
 	"context"
 	"database/sql"
 	"encoding/json"
-	"fmt"
-	"strings"
 
 	"github.com/element-hq/dendrite/federationapi/storage/tables"
 	"github.com/element-hq/dendrite/internal"
@@ -51,16 +49,6 @@ const selectNotaryKeyResponsesSQL = `
 	)
 `
 
-// select the responses which have the given key IDs
-// JOINs with the json table
-const selectNotaryKeyResponsesWithKeyIDsSQL = `
-	SELECT response_json FROM federationsender_notary_server_keys_json
-	JOIN federationsender_notary_server_keys_metadata ON
-	federationsender_notary_server_keys_metadata.notary_id = federationsender_notary_server_keys_json.notary_id
-	WHERE federationsender_notary_server_keys_json.server_name = $1 AND federationsender_notary_server_keys_metadata.key_id IN ($2)
-	GROUP BY federationsender_notary_server_keys_json.notary_id
-`
-
 // JOINs with the metadata table
 const deleteUnusedServerKeysJSONSQL = `
 	DELETE FROM federationsender_notary_server_keys_json WHERE federationsender_notary_server_keys_json.notary_id NOT IN (
@@ -114,22 +102,11 @@ func (s *notaryServerKeysMetadataStatements) UpsertKey(
 	return notaryID, err
 }
 
-func (s *notaryServerKeysMetadataStatements) SelectKeys(ctx context.Context, txn *sql.Tx, serverName spec.ServerName, keyIDs []gomatrixserverlib.KeyID) ([]gomatrixserverlib.ServerKeys, error) {
+func (s *notaryServerKeysMetadataStatements) SelectKeys(ctx context.Context, txn *sql.Tx, serverName spec.ServerName) ([]gomatrixserverlib.ServerKeys, error) {
 	var rows *sql.Rows
 	var err error
-	if len(keyIDs) == 0 {
-		rows, err = txn.Stmt(s.selectNotaryKeyResponsesStmt).QueryContext(ctx, string(serverName))
-	} else {
-		iKeyIDs := make([]interface{}, len(keyIDs)+1)
-		iKeyIDs[0] = serverName
-		for i := range keyIDs {
-			iKeyIDs[i+1] = string(keyIDs[i])
-		}
-		sql := strings.Replace(selectNotaryKeyResponsesWithKeyIDsSQL, "($2)", sqlutil.QueryVariadicOffset(len(keyIDs), 1), 1)
-		fmt.Println(sql)
-		fmt.Println(iKeyIDs...)
-		rows, err = s.db.QueryContext(ctx, sql, iKeyIDs...)
-	}
+
+	rows, err = txn.Stmt(s.selectNotaryKeyResponsesStmt).QueryContext(ctx, string(serverName))
 	if err != nil {
 		return nil, err
 	}

federationapi/storage/tables/interface.go

@@ -121,7 +121,7 @@ type FederationNotaryServerKeysMetadata interface {
 	UpsertKey(ctx context.Context, txn *sql.Tx, serverName spec.ServerName, keyID gomatrixserverlib.KeyID, newNotaryID NotaryID, newValidUntil spec.Timestamp) (NotaryID, error)
 	// SelectKeys returns the signed JSON objects which contain the given key IDs. This will be at most the length of `keyIDs` and at least 1 (assuming
 	// the keys exist in the first place). If `keyIDs` is empty, the signed JSON object with the longest valid_until_ts will be returned.
-	SelectKeys(ctx context.Context, txn *sql.Tx, serverName spec.ServerName, keyIDs []gomatrixserverlib.KeyID) ([]gomatrixserverlib.ServerKeys, error)
+	SelectKeys(ctx context.Context, txn *sql.Tx, serverName spec.ServerName) ([]gomatrixserverlib.ServerKeys, error)
 	// DeleteOldJSONResponses removes all responses which are not referenced in FederationNotaryServerKeysMetadata
 	DeleteOldJSONResponses(ctx context.Context, txn *sql.Tx) error
 }

test/memory_federation_db.go

@@ -492,7 +492,7 @@ func (d *InMemoryFederationDatabase) UpdateNotaryKeys(ctx context.Context, serve
 	return nil
 }
 
-func (d *InMemoryFederationDatabase) GetNotaryKeys(ctx context.Context, serverName spec.ServerName, optKeyIDs []gomatrixserverlib.KeyID) ([]gomatrixserverlib.ServerKeys, error) {
+func (d *InMemoryFederationDatabase) GetNotaryKeys(ctx context.Context, serverName spec.ServerName) ([]gomatrixserverlib.ServerKeys, error) {
 	return nil, nil
 }