From d57c39ce7f5a1d552b844094e136d7d7b232777a Mon Sep 17 00:00:00 2001
From: Neil Alexander <neilalexander@users.noreply.github.com>
Date: Fri, 6 Aug 2021 15:50:52 +0100
Subject: [PATCH] Fix /user/keys/query bug, review comments, update
 sytest-whitelist

---
 federationapi/routing/keys.go       | 10 ++++++++--
 keyserver/internal/cross_signing.go | 13 +++++++------
 sytest-whitelist                    |  1 +
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/federationapi/routing/keys.go b/federationapi/routing/keys.go
index d73161e94..bba3272b9 100644
--- a/federationapi/routing/keys.go
+++ b/federationapi/routing/keys.go
@@ -71,8 +71,14 @@ func QueryDeviceKeys(
 	return util.JSONResponse{
 		Code: 200,
 		JSON: struct {
-			DeviceKeys interface{} `json:"device_keys"`
-		}{queryRes.DeviceKeys},
+			DeviceKeys      interface{} `json:"device_keys"`
+			MasterKeys      interface{} `json:"master_keys"`
+			SelfSigningKeys interface{} `json:"self_signing_keys"`
+		}{
+			queryRes.DeviceKeys,
+			queryRes.MasterKeys,
+			queryRes.SelfSigningKeys,
+		},
 	}
 }
 
diff --git a/keyserver/internal/cross_signing.go b/keyserver/internal/cross_signing.go
index 7c65a2604..7575b6522 100644
--- a/keyserver/internal/cross_signing.go
+++ b/keyserver/internal/cross_signing.go
@@ -436,16 +436,16 @@ func (a *KeyInternalAPI) processOtherSignatures(
 					// actually be.
 					localKeyData, lok := masterKey.Keys[targetKeyID]
 					if !lok {
-						return fmt.Errorf("uploaded master key for user %q doesn't match local copy", targetUserID)
+						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 for user %q doesn't match local copy", targetUserID)
+						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 from uploading user %q", userID)
+						return fmt.Errorf("there are no signatures on master key %q from uploading user %q", targetKeyID, userID)
 					}
 
 					for originKeyID, originSig := range userSigs {
@@ -458,8 +458,9 @@ func (a *KeyInternalAPI) processOtherSignatures(
 				}
 
 			default:
-				// Users shouldn't be signing anything other people's devices,
-				// so we'll just do nothing with it if that's the case.
+				// 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.
 			}
 		}
 	}
@@ -485,7 +486,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
 			}
diff --git a/sytest-whitelist b/sytest-whitelist
index 27109e602..d2f2a1c7d 100644
--- a/sytest-whitelist
+++ b/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