Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

device_one_time_keys_count: difficulty in differentiating between "keys exhausted" and "no change since last sync" states #3298

Closed
anoadragon453 opened this issue Jul 27, 2021 · 2 comments · Fixed by #3636
Labels
clarification An area where the spec could do with being more explicit

Comments

@anoadragon453
Copy link
Member

anoadragon453 commented Jul 27, 2021

As found in matrix-org/synapse#10484, the spec is currently vague with regards to when the "device_one_time_keys_count" field on both GET /_matrix/client/r0/sync and POST /_matrix/client/r0/keys/upload responses.

The field is currently marked as Optional, yet it's not clear when a homeserver should include the field in a response, and when it does what should be put in it. The confusion lies in how to differentiate between telling the client that "there's been no change in the key count since last sync" and "there are no OTKs left".

For a long time Synapse has simply omitted the contents of the field ("device_one_time_keys_count": {}) in the case of OTKs being exhausted. However this has lead to confusion over some clients interpreting this to mean that there has been no change. matrix-org/synapse#10485 fixes the issue by always including the key algorithm name; thus an exhausted pool would look like "device_one_time_keys_count": {"signed_curve25519": 0}.

You then might say that "device_one_time_keys_count": {"algorithm_name": 0} means an exhausted pool, and the lack of an algorithm name in the device_one_time_keys_count dictionary would mean that no change has occurred. But what if a new algorithm is introduced down the line? Then clients would interpret the lack of the new algorithm name as no change has occurred. Perhaps this can be rectified by tightly coupling possible algorithm names with supported spec versions advertised by the homeserver though.

Either way, the spec as it stands is currently vague. It would be beneficial to initially at least agree on something more explicit.

@uhoreg
Copy link
Member

uhoreg commented Jul 27, 2021

The description for that field says:

For each key algorithm, the number of unclaimed one-time keys of that type currently held on the server for this device.

To me, that means that it's an absolute count, and not meant to indicate a change, thus a missing algorithm would imply that no keys exist for that algorithm, rather than no change.

@richvdh
Copy link
Member

richvdh commented Jul 27, 2021

That might be a reasonable interpretation, but it does imply that every single sync response contains {"device_one_time_keys_count": {"signed_curve25519": N},, which feels pretty redundant when nothing is changing.

In any case, there's certainly been some confusion around it, so it would be good to clarify this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification An area where the spec could do with being more explicit
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants