StoreKeyFetcher seems to do nothing if not using a trusted key server

[DMRobertson]

Suppose we try to fetch keys from some homeserver we've never heard of.

We'll first try to use StoreKeyFetcher, which calls

synapse/synapse/crypto/keyring.py

Line 513 in 9bb2eac

res = await self.store.get_server_verify_keys(key_ids_to_fetch)

to read from server_signature_keys:

synapse/synapse/storage/databases/main/keys.py

Lines 65 to 68 in b76f1a4

	sql = (
	"SELECT server_name, key_id, verify_key, ts_valid_until_ms "
	"FROM server_signature_keys WHERE 1=0"
	) + " OR (server_name=? AND key_id=?)" * len(batch)

That will return no keys. So next we'll try the PerspectivesKeyFetcher, but that will do nothing because we have no trusted key server configured.

So next we try the ServerKeyFetcher. That will fetch keys via

synapse/synapse/crypto/keyring.py

Line 844 in 9bb2eac

keys = await self.get_server_verify_keys_v2_direct(server_name)

and

synapse/synapse/crypto/keyring.py

Lines 901 to 905 in 9bb2eac

	return await self.process_v2_response(
	from_server=server_name,
	response_json=response,
	time_added_ms=time_now_ms,
	)

before caling

synapse/synapse/crypto/keyring.py

Line 605 in 9bb2eac

self.store.store_server_keys_json,

which writes to server_keys_json.

If we try to re-fetch keys for that server, we'll try the StoreKeyFetcher. But as we saw above, that reads from a different table (server_signature_keys). So we'll end up repeating the same steps and making another federation request via ServerKeyFetcher.

[DMRobertson]

On this, I asked Rich:

dmr: On the off-chance, do you have any idea what the difference between the server_signature_keys and server_keys_json tables is?
AFAICS they have the same set of columns with the same types

His reply shed some light:

the idea used to be that one was used to drive the keyserver responses and one was used for local verification operations
though I can't remember the details of (a) which was which or (b) why you want two tables for that

I also noticed:

Ahh, one of them has a slightly looser uniqueness constraint:

synapse/synapse/storage/schema/main/full_schemas/72/full.sql.postgres

Lines 1113 to 1116 in 0a38c7e

	ALTER TABLE ONLY server_keys_json
	ADD CONSTRAINT server_keys_json_uniqueness UNIQUE (server_name, key_id, from_server);
	ALTER TABLE ONLY server_signature_keys
	ADD CONSTRAINT server_signature_keys_server_name_key_id_key UNIQUE (server_name, key_id);

Presumably the perspectives one is designed to let you query multiple keyservers.

[dklimpel]

Is it possible that this is the trigger for the behavior in #14805 ?

[DMRobertson]

Is it possible that this is the trigger for the behavior in #14805 ?

This issue makes it more likely that workers will request keys---but only on homeservers that are not using a trusted key server. But even if this issue was fixed, non-federation-sender workers could still make key requests. E.g.

• the key needs to be refetched, and a client_reader worker is the first to require it
• a federation_reader receives a remote invite from a homeserver who's never contacted us before.

[clokep]

That will return no keys. So next we'll try the PerspectivesKeyFetcher, but that will do nothing because we have no trusted key server configured.

Note that if you do have one configured it will ask the perspectives key server and then store the results in both server_keys_json and server_signature_keys.

So this sounds to me like the initial StoreKeyFetcher is just reading from the wrong table? 🤷

[clokep]

I find the following helpful to visualize this:

flowchart
    subgraph Keyring
    StoreKeyFetcher
    PerspectivesKeyFetcher
    ServerKeyFetcher

    StoreKeyFetcher ==> PerspectivesKeyFetcher ==> ServerKeyFetcher;
    end

    StoreKeyFetcher
    server_signature_keys -- get_server_verify_keys --> StoreKeyFetcher;


    PerspectivesKeyFetcher -- store_server_verify_keys --> server_signature_keys;
    PerspectivesKeyFetcher -- store_server_keys_json --> server_keys_json;
    ServerKeyFetcher -- store_server_keys_json --> server_keys_json;

    subgraph Database
    server_signature_keys[(server_signature_keys)]
    server_keys_json[(server_keys_json)]
    end

[clokep]

Based on the above diagram, we believe that server_signature_keys ⊆ server_keys_json; I've attempted to check this on a few servers with the following query:

SELECT *
FROM server_signature_keys
    LEFT JOIN server_keys_json USING (server_name, key_id)
WHERE key_json IS NULL;

This takes the subset, left joins the superset and then queries for anything in server_signature_keys without a matching row in server_keys_json.

I ran this on a handful of servers and they call found 0 rows. 🎉

[erikjohnston]

Conclusion: we should put a bandaid on this by making StoreKeyFetcher query both tables.

At some point we should also figure out why we have two tables and if we can merge them.

[clokep]

At some point we should also figure out why we have two tables and if we can merge them.

#15463 is a follow-up to clean up these tables.