Cross-signing signature handling (#1965)

* Handle other signatures

* Decorate key ID properly

* Match by key IDs

* Tweaks

* Fixes

* Fix /user/keys/query bug, review comments, update sytest-whitelist

* Various wtweaks

* Fix wiring for keyserver in API mode

* Additional fixes

cmd/dendrite-monolith-server/main.go

@@ -120,6 +120,11 @@ func main() {
 			Impl: userAPI,
 		}
 	}
+	// needs to be after the SetUserAPI call above
+	if base.UseHTTPAPIs {
+		keyserver.AddInternalRoutes(base.InternalAPIMux, keyAPI)
+		keyAPI = base.KeyServerHTTPClient()
+	}
 
 	eduInputAPI := eduserver.NewInternalAPI(
 		base, cache.New(), userAPI,

federationapi/routing/devices.go

@@ -38,12 +38,11 @@ func GetUserDevices(
 	}
 
 	sigReq := &keyapi.QuerySignaturesRequest{
-		TargetIDs: map[string][]gomatrixserverlib.KeyID{},
+		TargetIDs: map[string][]gomatrixserverlib.KeyID{
+			userID: {},
+		},
 	}
 	sigRes := &keyapi.QuerySignaturesResponse{}
-	for _, dev := range res.Devices {
-		sigReq.TargetIDs[userID] = append(sigReq.TargetIDs[userID], gomatrixserverlib.KeyID(dev.DeviceID))
-	}
 	keyAPI.QuerySignatures(req.Context(), sigReq, sigRes)
 
 	response := gomatrixserverlib.RespUserDevices{

federationapi/routing/keys.go

@@ -71,8 +71,14 @@ func QueryDeviceKeys(
 	return util.JSONResponse{
 		Code: 200,
 		JSON: struct {
-			DeviceKeys interface{} `json:"device_keys"`
+			DeviceKeys      interface{} `json:"device_keys"`
-		}{queryRes.DeviceKeys},
+			MasterKeys      interface{} `json:"master_keys"`
+			SelfSigningKeys interface{} `json:"self_signing_keys"`
+		}{
+			queryRes.DeviceKeys,
+			queryRes.MasterKeys,
+			queryRes.SelfSigningKeys,
+		},
 	}
 }
 

go.mod

@@ -31,7 +31,7 @@ require (
 	github.com/matrix-org/go-http-js-libp2p v0.0.0-20200518170932-783164aeeda4
 	github.com/matrix-org/go-sqlite3-js v0.0.0-20210709140738-b0d1ba599a6d
 	github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16
-	github.com/matrix-org/gomatrixserverlib v0.0.0-20210805104751-03e40fa2c0e1
+	github.com/matrix-org/gomatrixserverlib v0.0.0-20210809130922-d9c3f400582b
 	github.com/matrix-org/naffka v0.0.0-20210623111924-14ff508b58e0
 	github.com/matrix-org/pinecone v0.0.0-20210623102758-74f885644c1b
 	github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4

go.sum

@@ -994,8 +994,8 @@ github.com/matrix-org/go-sqlite3-js v0.0.0-20210709140738-b0d1ba599a6d/go.mod h1
 github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26/go.mod h1:3fxX6gUjWyI/2Bt7J1OLhpCzOfO/bB3AiX0cJtEKud0=
 github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 h1:ZtO5uywdd5dLDCud4r0r55eP4j9FuUNpl60Gmntcop4=
 github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16/go.mod h1:/gBX06Kw0exX1HrwmoBibFA98yBk/jxKpGVeyQbff+s=
-github.com/matrix-org/gomatrixserverlib v0.0.0-20210805104751-03e40fa2c0e1 h1:ExS9AmOWYSZ4DR7W7Eqm7glP/g/u/CTVLAu9blAnve4=
+github.com/matrix-org/gomatrixserverlib v0.0.0-20210809130922-d9c3f400582b h1:8St1B8QmlvMLsOmGqW3++0akUs0250IAi+AGcr5faxw=
-github.com/matrix-org/gomatrixserverlib v0.0.0-20210805104751-03e40fa2c0e1/go.mod h1:JsAzE1Ll3+gDWS9JSUHPJiiyAksvOOnGWF2nXdg4ZzU=
+github.com/matrix-org/gomatrixserverlib v0.0.0-20210809130922-d9c3f400582b/go.mod h1:JsAzE1Ll3+gDWS9JSUHPJiiyAksvOOnGWF2nXdg4ZzU=
 github.com/matrix-org/naffka v0.0.0-20210623111924-14ff508b58e0 h1:HZCzy4oVzz55e+cOMiX/JtSF2UOY1evBl2raaE7ACcU=
 github.com/matrix-org/naffka v0.0.0-20210623111924-14ff508b58e0/go.mod h1:sjyPyRxKM5uw1nD2cJ6O2OxI6GOqyVBfNXqKjBZTBZE=
 github.com/matrix-org/pinecone v0.0.0-20210623102758-74f885644c1b h1:5X5vdWQ13xrNkJVqaJHPsrt7rKkMJH5iac0EtfOuxSg=

keyserver/api/api.go

@@ -160,7 +160,7 @@ type PerformClaimKeysResponse struct {
 type PerformUploadDeviceKeysRequest struct {
 	gomatrixserverlib.CrossSigningKeys
 	// The user that uploaded the key, should be populated by the clientapi.
-	UserID string `json:"user_id"`
+	UserID string
 }
 
 type PerformUploadDeviceKeysResponse struct {
@@ -170,7 +170,7 @@ type PerformUploadDeviceKeysResponse struct {
 type PerformUploadDeviceSignaturesRequest struct {
 	Signatures map[string]map[gomatrixserverlib.KeyID]gomatrixserverlib.CrossSigningForKeyOrDevice
 	// The user that uploaded the sig, should be populated by the clientapi.
-	UserID string `json:"user_id"`
+	UserID string
 }
 
 type PerformUploadDeviceSignaturesResponse struct {

keyserver/internal/cross_signing.go

@@ -15,6 +15,7 @@
 package internal
 
 import (
+	"bytes"
 	"context"
 	"crypto/ed25519"
 	"database/sql"
@@ -78,8 +79,8 @@ func (a *KeyInternalAPI) PerformUploadDeviceKeys(ctx context.Context, req *api.P
 			}
 			return
 		}
-		hasMasterKey = true
 		for _, keyData := range req.MasterKey.Keys { // iterates once, because sanityCheckKey requires one key
+			hasMasterKey = true
 			masterKey = keyData
 		}
 	}
@@ -116,46 +117,11 @@ func (a *KeyInternalAPI) PerformUploadDeviceKeys(ctx context.Context, req *api.P
 		masterKey, hasMasterKey = existingKeys[gomatrixserverlib.CrossSigningKeyPurposeMaster]
 	}
 
-	// If the user isn't a local user and we haven't successfully found a key
-	// through any local means then ask over federation.
-	if !hasMasterKey {
-		_, host, err := gomatrixserverlib.SplitID('@', req.UserID)
-		if err != nil {
-			res.Error = &api.KeyError{
-				Err: "Retrieving cross-signing keys from federation failed: " + err.Error(),
-			}
-			return
-		}
-		keys, err := a.FedClient.QueryKeys(ctx, host, map[string][]string{
-			req.UserID: {},
-		})
-		if err != nil {
-			res.Error = &api.KeyError{
-				Err: "Retrieving cross-signing keys from federation failed: " + err.Error(),
-			}
-			return
-		}
-		switch k := keys.MasterKeys[req.UserID].CrossSigningBody.(type) {
-		case *gomatrixserverlib.CrossSigningKey:
-			if err := sanityCheckKey(*k, req.UserID, gomatrixserverlib.CrossSigningKeyPurposeMaster); err != nil {
-				res.Error = &api.KeyError{
-					Err: "Master key sanity check failed: " + err.Error(),
-				}
-				return
-			}
-		default:
-			res.Error = &api.KeyError{
-				Err: "Unexpected type for master key retrieved from federation",
-			}
-			return
-		}
-	}
-
 	// If we still don't have a master key at this point then there's nothing else
 	// we can do - we've checked both the request and the database.
 	if !hasMasterKey {
 		res.Error = &api.KeyError{
-			Err:            "No master key was found, either in the database or in the request!",
+			Err:            "No master key was found either in the database or in the request!",
 			IsMissingParam: true,
 		}
 		return
@@ -176,6 +142,15 @@ func (a *KeyInternalAPI) PerformUploadDeviceKeys(ctx context.Context, req *api.P
 	if len(req.UserSigningKey.Keys) > 0 {
 		toVerify[gomatrixserverlib.CrossSigningKeyPurposeUserSigning] = req.UserSigningKey
 	}
+
+	if len(toVerify) == 0 {
+		res.Error = &api.KeyError{
+			Err:            "No supplied keys available for verification",
+			IsMissingParam: true,
+		}
+		return
+	}
+
 	for purpose, key := range toVerify {
 		// Collect together the key IDs we need to verify with. This will include
 		// all of the key IDs specified in the signatures.
@@ -212,6 +187,14 @@ func (a *KeyInternalAPI) PerformUploadDeviceKeys(ctx context.Context, req *api.P
 		}
 	}
 
+	if len(toStore) == 0 {
+		res.Error = &api.KeyError{
+			Err:            "No supplied keys passed verification",
+			IsMissingParam: true,
+		}
+		return
+	}
+
 	if err := a.DB.StoreCrossSigningKeysForUser(ctx, req.UserID, toStore); err != nil {
 		res.Error = &api.KeyError{
 			Err: fmt.Sprintf("a.DB.StoreCrossSigningKeysForUser: %s", err),
@@ -257,6 +240,8 @@ func (a *KeyInternalAPI) PerformUploadDeviceSignatures(ctx context.Context, req
 	selfSignatures := map[string]map[gomatrixserverlib.KeyID]gomatrixserverlib.CrossSigningForKeyOrDevice{}
 	otherSignatures := map[string]map[gomatrixserverlib.KeyID]gomatrixserverlib.CrossSigningForKeyOrDevice{}
 
+	// Sort signatures into two groups: one where people have signed their own
+	// keys and one where people have signed someone elses
 	for userID, forUserID := range req.Signatures {
 		for keyID, keyOrDevice := range forUserID {
 			switch key := keyOrDevice.CrossSigningBody.(type) {
@@ -335,20 +320,18 @@ func (a *KeyInternalAPI) processSelfSignatures(
 					}
 
 					for originKeyID, originSig := range forOriginUserID {
-						originDeviceKeyID := gomatrixserverlib.KeyID("ed25519:" + originKeyID)
-
 						var originKey gomatrixserverlib.DeviceKeys
 						if err := json.Unmarshal(originDeviceKeys[string(originKeyID)], &originKey); err != nil {
 							return fmt.Errorf("json.Unmarshal: %w", err)
 						}
 
-						originSigningKey, ok := originKey.Keys[originDeviceKeyID]
+						originSigningKey, ok := originKey.Keys[originKeyID]
 						if !ok {
-							return fmt.Errorf("missing origin signing key %q", originDeviceKeyID)
+							return fmt.Errorf("missing origin signing key %q", originKeyID)
 						}
 						originSigningKeyPublic := ed25519.PublicKey(originSigningKey)
 
-						if err := gomatrixserverlib.VerifyJSON(originUserID, originDeviceKeyID, originSigningKeyPublic, j); err != nil {
+						if err := gomatrixserverlib.VerifyJSON(originUserID, originKeyID, originSigningKeyPublic, j); err != nil {
 							return fmt.Errorf("gomatrixserverlib.VerifyJSON: %w", err)
 						}
 
@@ -414,6 +397,56 @@ func (a *KeyInternalAPI) processOtherSignatures(
 	// Here we will process:
 	// * A user signing someone else's master keys using their user-signing keys
 
+	for targetUserID, forTargetUserID := range signatures {
+		for _, signature := range forTargetUserID {
+			switch sig := signature.CrossSigningBody.(type) {
+			case *gomatrixserverlib.CrossSigningKey:
+				// Find the local copy of the master key. We'll use this to be
+				// sure that the supplied stanza matches the key that we think it
+				// should be.
+				masterKey, ok := queryRes.MasterKeys[targetUserID]
+				if !ok {
+					return fmt.Errorf("failed to find master key for user %q", targetUserID)
+				}
+
+				// For each key ID, write the signatures. Maybe there'll be more
+				// than one algorithm in the future so it's best not to focus on
+				// everything being ed25519:.
+				for targetKeyID, suppliedKeyData := range sig.Keys {
+					// The master key will be supplied in the request, but we should
+					// make sure that it matches what we think the master key should
+					// actually be.
+					localKeyData, lok := masterKey.Keys[targetKeyID]
+					if !lok {
+						return fmt.Errorf("uploaded master key %q for user %q doesn't match local copy", targetKeyID, targetUserID)
+					} else if !bytes.Equal(suppliedKeyData, localKeyData) {
+						return fmt.Errorf("uploaded master key %q for user %q doesn't match local copy", targetKeyID, targetUserID)
+					}
+
+					// We only care about the signatures from the uploading user, so
+					// we will ignore anything that didn't originate from them.
+					userSigs, ok := sig.Signatures[userID]
+					if !ok {
+						return fmt.Errorf("there are no signatures on master key %q from uploading user %q", targetKeyID, userID)
+					}
+
+					for originKeyID, originSig := range userSigs {
+						if err := a.DB.StoreCrossSigningSigsForTarget(
+							ctx, userID, originKeyID, targetUserID, targetKeyID, originSig,
+						); err != nil {
+							return fmt.Errorf("a.DB.StoreCrossSigningKeysForTarget: %w", err)
+						}
+					}
+				}
+
+			default:
+				// Users should only be signing another person's master key,
+				// so if we're here, it's probably because it's actually a
+				// gomatrixserverlib.DeviceKeys, which doesn't make sense.
+			}
+		}
+	}
+
 	return nil
 }
 
@@ -435,7 +468,7 @@ func (a *KeyInternalAPI) crossSigningKeysFromDatabase(
 			}
 
 			sigMap, err := a.DB.CrossSigningSigsForTarget(ctx, userID, keyID)
-			if err != nil {
+			if err != nil && err != sql.ErrNoRows {
 				logrus.WithError(err).Errorf("Failed to get cross-signing signatures for user %q key %q", userID, keyID)
 				continue
 			}
@@ -480,44 +513,39 @@ func (a *KeyInternalAPI) crossSigningKeysFromDatabase(
 
 func (a *KeyInternalAPI) QuerySignatures(ctx context.Context, req *api.QuerySignaturesRequest, res *api.QuerySignaturesResponse) {
 	for targetUserID, forTargetUser := range req.TargetIDs {
+		keyMap, err := a.DB.CrossSigningKeysForUser(ctx, targetUserID)
+		if err != nil && err != sql.ErrNoRows {
+			res.Error = &api.KeyError{
+				Err: fmt.Sprintf("a.DB.CrossSigningKeysForUser: %s", err),
+			}
+			continue
+		}
+
+		for targetPurpose, targetKey := range keyMap {
+			switch targetPurpose {
+			case gomatrixserverlib.CrossSigningKeyPurposeMaster:
+				if res.MasterKeys == nil {
+					res.MasterKeys = map[string]gomatrixserverlib.CrossSigningKey{}
+				}
+				res.MasterKeys[targetUserID] = targetKey
+
+			case gomatrixserverlib.CrossSigningKeyPurposeSelfSigning:
+				if res.SelfSigningKeys == nil {
+					res.SelfSigningKeys = map[string]gomatrixserverlib.CrossSigningKey{}
+				}
+				res.SelfSigningKeys[targetUserID] = targetKey
+
+			case gomatrixserverlib.CrossSigningKeyPurposeUserSigning:
+				if res.UserSigningKeys == nil {
+					res.UserSigningKeys = map[string]gomatrixserverlib.CrossSigningKey{}
+				}
+				res.UserSigningKeys[targetUserID] = targetKey
+			}
+		}
+
 		for _, targetKeyID := range forTargetUser {
-			keyMap, err := a.DB.CrossSigningKeysForUser(ctx, targetUserID)
-			if err != nil {
-				if err == sql.ErrNoRows {
-					continue
-				}
-				res.Error = &api.KeyError{
-					Err: fmt.Sprintf("a.DB.CrossSigningKeysForUser: %s", err),
-				}
-			}
-
-			for targetPurpose, targetKey := range keyMap {
-				switch targetPurpose {
-				case gomatrixserverlib.CrossSigningKeyPurposeMaster:
-					if res.MasterKeys == nil {
-						res.MasterKeys = map[string]gomatrixserverlib.CrossSigningKey{}
-					}
-					res.MasterKeys[targetUserID] = targetKey
-
-				case gomatrixserverlib.CrossSigningKeyPurposeSelfSigning:
-					if res.SelfSigningKeys == nil {
-						res.SelfSigningKeys = map[string]gomatrixserverlib.CrossSigningKey{}
-					}
-					res.SelfSigningKeys[targetUserID] = targetKey
-
-				case gomatrixserverlib.CrossSigningKeyPurposeUserSigning:
-					if res.UserSigningKeys == nil {
-						res.UserSigningKeys = map[string]gomatrixserverlib.CrossSigningKey{}
-					}
-					res.UserSigningKeys[targetUserID] = targetKey
-				}
-			}
-
 			sigMap, err := a.DB.CrossSigningSigsForTarget(ctx, targetUserID, targetKeyID)
-			if err != nil {
+			if err != nil && err != sql.ErrNoRows {
-				if err == sql.ErrNoRows {
-					continue
-				}
 				res.Error = &api.KeyError{
 					Err: fmt.Sprintf("a.DB.CrossSigningSigsForTarget: %s", err),
 				}

keyserver/internal/internal.go

@@ -372,9 +372,15 @@ func (a *KeyInternalAPI) queryRemoteKeys(
 
 	domains := map[string]struct{}{}
 	for domain := range domainToDeviceKeys {
+		if domain == string(a.ThisServer) {
+			continue
+		}
 		domains[domain] = struct{}{}
 	}
 	for domain := range domainToCrossSigningKeys {
+		if domain == string(a.ThisServer) {
+			continue
+		}
 		domains[domain] = struct{}{}
 	}
 	wg.Add(len(domains))
@@ -406,17 +412,11 @@ func (a *KeyInternalAPI) queryRemoteKeys(
 		}
 
 		for userID, body := range result.MasterKeys {
-			switch b := body.CrossSigningBody.(type) {
+			res.MasterKeys[userID] = body
-			case *gomatrixserverlib.CrossSigningKey:
-				res.MasterKeys[userID] = *b
-			}
 		}
 
 		for userID, body := range result.SelfSigningKeys {
-			switch b := body.CrossSigningBody.(type) {
+			res.SelfSigningKeys[userID] = body
-			case *gomatrixserverlib.CrossSigningKey:
-				res.SelfSigningKeys[userID] = *b
-			}
 		}
 
 		// TODO: do we want to persist these somewhere now
@@ -430,8 +430,12 @@ func (a *KeyInternalAPI) queryRemoteKeysOnServer(
 	res *api.QueryKeysResponse,
 ) {
 	defer wg.Done()
-	fedCtx, cancel := context.WithTimeout(ctx, timeout)
+	fedCtx := ctx
-	defer cancel()
+	if timeout > 0 {
+		var cancel context.CancelFunc
+		fedCtx, cancel = context.WithTimeout(ctx, timeout)
+		defer cancel()
+	}
 	// for users who we do not have any knowledge about, try to start doing device list updates for them
 	// by hitting /users/devices - otherwise fallback to /keys/query which has nicer bulk properties but
 	// lack a stream ID.

keyserver/inthttp/server.go

@@ -62,7 +62,7 @@ func AddRoutes(internalAPIMux *mux.Router, s api.KeyInternalAPI) {
 		httputil.MakeInternalAPI("performUploadDeviceKeys", func(req *http.Request) util.JSONResponse {
 			request := api.PerformUploadDeviceKeysRequest{}
 			response := api.PerformUploadDeviceKeysResponse{}
-			if err := json.NewDecoder(req.Body).Decode(&request.CrossSigningKeys); err != nil {
+			if err := json.NewDecoder(req.Body).Decode(&request); err != nil {
 				return util.MessageResponse(http.StatusBadRequest, err.Error())
 			}
 			s.PerformUploadDeviceKeys(req.Context(), &request, &response)
@@ -73,7 +73,7 @@ func AddRoutes(internalAPIMux *mux.Router, s api.KeyInternalAPI) {
 		httputil.MakeInternalAPI("performUploadDeviceSignatures", func(req *http.Request) util.JSONResponse {
 			request := api.PerformUploadDeviceSignaturesRequest{}
 			response := api.PerformUploadDeviceSignaturesResponse{}
-			if err := json.NewDecoder(req.Body).Decode(&request.Signatures); err != nil {
+			if err := json.NewDecoder(req.Body).Decode(&request); err != nil {
 				return util.MessageResponse(http.StatusBadRequest, err.Error())
 			}
 			s.PerformUploadDeviceSignatures(req.Context(), &request, &response)

sytest-whitelist

@@ -553,3 +553,4 @@ Deleted & recreated backups are empty
 Can upload self-signing keys
 Fails to upload self-signing keys with no auth
 Fails to upload self-signing key without master key
+can fetch self-signing keys over federation