/keys/upload is ambiguous about what data should be signed

[ara4n]

should signatures span the whole DeviceKeys object or just a single keys field? You can guess from context that it is just a single keys field, but would be better to be clearer

[EbnerAndreas]

Hey, after talking to Matthew I will comment with what I found and think should be changed/added to the /keys/upload endpoint:

1. I found out that you have to sign the following JSON fields as one JSON object:

{'keys': {...},
'user_id': ...,
'algorithms': [...],
'device_id': ...}

Also, the ordering of the fields isn't important because the canonical json function takes care of ordering it.

The fields can be found here:
https://github.com/matrix-org/matrix-js-sdk/blob/ae85c209ab0d58c900bc3db1802cffc1be880b85/src/crypto/index.js#L265

Also: I don't like that the signature isn't checked when uploaded. So, at the moment, you can basically upload ANYTHING as a signature resulting in others not being able to verify it.

2. The example request of the /upload/keys endpoint documentation states the algorithms:
   [ "m.olm.curve25519-aes-sha256", "m.megolm.v1.aes-sha" ]
   However, the currently promoted algorithms are:
   ['m.olm.v1.curve25519-aes-sha2', 'm.megolm.v1.aes-sha2']

3. I've also recognized another problem with uploading keys that maybe should be mentioned in the doc or might be a problem:

If you login without a given device_id the server will create a new one (stated in the /login endpoint documention).
When uploading keys you pass the device_id in the "device_id" JSON field, however this ID is not checked in any case.
What happend for me: I've tried to upload keys for my device by setting the device_id in the /upload/keys request. I've got a bunch of random devices (due to my login without a device_id) with keypairs that state all the same device_id, like this:

{'device_keys': {'@test:test.org': 
{'DWTYLSQHPH': {'algorithms': [...], 'device_id': 'HWWBTMCIEJ', 'keys': {...}}, 
'HWWBTMCIEJ': {'algorithms': [...], 'device_id': 'HWWBTMCIEJ', 'keys': {...}}, 
'JOXRPZCMKM': {'algorithms': [...], 'device_id': 'HWWBTMCIEJ', 'keys': {...}}
}}}

This looks very confusing and somewhere in the code i found:
(https://github.com/matrix-org/synapse/blob/d69decd5c78c72abef50b597a689e2bc55a39702/synapse/rest/client/v2_alpha/keys.py#L71)

if device_id is None:
            raise SynapseError(
                400,
                "To upload keys, you must pass device_id when authenticating"
)

I think it would be helpful to mention and/or check that you MUST be logged in with the device_id you intent to upload keys to. Also a better response from the server would be helpful if the device IDs mismatch.

[EbnerAndreas]

Addition:
For the one-time keys uploaded via this endpoint each key has to have a signature containing:

{"key": one_time_key_value}

Found here:
https://github.com/matrix-org/matrix-js-sdk/blob/ae85c209ab0d58c900bc3db1802cffc1be880b85/src/crypto/index.js#L408

(also parsed with canonical JSON)

[a-andreyev]

Hello! Is the issue from point 2 still relevant?

The example request of the /upload/keys endpoint documentation states the algorithms:
[ "m.olm.curve25519-aes-sha256", "m.megolm.v1.aes-sha" ]
However, the currently promoted algorithms are:
['m.olm.v1.curve25519-aes-sha2', 'm.megolm.v1.aes-sha2']

Should client implementation follow the current spec
or should the spec and the client code be updated with the naming to promoted one?

[uhoreg]

Regarding point 2, it looks like a typo in the spec. The one with the v1 is correct. (I'll try to remember to fix the spec tomorrow.)

[uhoreg]

Looking at this, I'm not sure how this can be made clearer. The signatures field points to the "Signing JSON" section, so it seems to me that it's clear that the signature covers the entire object, rather than just a single field. @ara4n can you indicate where the ambiguity is?

[cryptoapebot]

The documentation does say the entire object. Using Gson and JOLM, some of the object json formats don't map well to objects and need to be manipulated at the json string level. This isn't a bug, but having been through the examples at https://matrix.org/docs/spec/client_server/r0.4.0.html#post-matrix-client-r0-keys-upload

So for this example DeviceKeys jolm object that converts to Json string:
{ "device_keys": { "algorithms": [ "m.olm.curve25519-aes-sha256", "m.megolm.v1.aes-sha" ], "device_id": "F...E", "keys": { "curve25519:F...E": "b...4", "ed25519:F...E": "L...o" }, "signatures": {}, "user_id": "@bot:fd.com" } }

I get this DeviceKeys signed format.
{ "device_keys": { "algorithms": [ "m.olm.curve25519-aes-sha256", "m.megolm.v1.aes-sha" ], "device_id": "F...E", "keys": { "curve25519:F...E": "b...4", "ed25519:F...E": "L...o" }, "signatures": { "@bot:fd.com": { "ed25519:F...E": "w...A" } }, "user_id": "@bot:fd.com" } }

It returns a 200, so it appears to succeed.

For one time keys, I have the format that comes from JOLM class OneTimeKey:
{ "curve25519": { "AAAACg": "F...g", "AAAACQ": "i...w", "AAAACA": "B...0" } }

The exact upload format in the example shows both a single key and key value, but also signed_curve:device json object with a key and signatures.

So my immediate question that I'm looking for clarification on is, can I sign all of the one time keys at the same time or do I have to sign each one individually. Also for each one time key, what is the exact format of the json that is supposed to be uploaded for each key or all keys at the same time?

Any guidance would be greatly appreciated.

[uhoreg]

@cryptoapebot the format of one-time keys is documented at https://spec.matrix.org/unstable/client-server-api/#key-algorithms You want to use the signed_curve25519 format. The documentation around one-time keys can definitely do with more work. I've opened a new issue to track that: #1362