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

One-time-key upload/claim is racy #1124

Open
Tracked by #245
turt2live opened this issue Jun 14, 2022 · 4 comments
Open
Tracked by #245

One-time-key upload/claim is racy #1124

turt2live opened this issue Jun 14, 2022 · 4 comments
Labels
A-E2EE Issues about end-to-end encryption wart A point where the protocol is inconsistent or inelegant

Comments

@turt2live
Copy link
Member

turt2live commented Jun 14, 2022

  • Alice tries to upload one-time-key, but doesn't get response
  • Bob claims the key
  • Alice uploads the key again
  • Charlie claims the key

Only one of Bob or Charlie can use the key to talk to Alice.

@richvdh
Copy link
Member

richvdh commented Apr 26, 2024

I think the only real solution to this is for the server to remember it has recently seen and handed out a given OTK, and not hand it out a second time even if it gets uploaded again. As such, I'm not entirely convinced it is a spec issue so much as a server-side impl issue (though certainly the spec could make the necessary behaviour explicit).

@richvdh richvdh changed the title one-time-key upload/claim is racy One-time-key upload/claim is racy May 1, 2024
@richvdh
Copy link
Member

richvdh commented Jul 4, 2024

Another option would be:

  • Oblige clients to give each OTK they generate a numeric ID, which must increase monotonically.
  • The server hands out OTKs in order of increasing ID, and remembers the ID of the last-issued OTK.
  • If the server sees a client uploading an OTK whose ID is less-than/equal-to that of the the last-issued OTK, it ignores the new upload but returns a 200.

The advantage is that the server wouldn't have to keep a record of issued OTKs. The disadvantage is that it means API changes, and changes client-side as well as server-side.

@kegsay
Copy link
Member

kegsay commented Jul 4, 2024

it ignores the new upload but returns a 200.

This superficially feels like it could hide bugs (e.g if a client uploads a different key with a lower numeric ID, there's no evidence it wasn't stored). But other than that, I like it.

@richvdh
Copy link
Member

richvdh commented Jul 5, 2024

This superficially feels like it could hide bugs (e.g if a client uploads a different key with a lower numeric ID, there's no evidence it wasn't stored). But other than that, I like it.

As long as the server also checks that there isn't already a different key with the same alphanumeric ID, I don't think this would hide any more bugs than are already hidden?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-E2EE Issues about end-to-end encryption wart A point where the protocol is inconsistent or inelegant
Projects
None yet
Development

No branches or pull requests

3 participants