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

Element Android reuses txnId for to-device events in the verification process #3589

Closed
timokoesters opened this issue Jun 30, 2021 · 2 comments · Fixed by #3707
Closed

Element Android reuses txnId for to-device events in the verification process #3589

timokoesters opened this issue Jun 30, 2021 · 2 comments · Fixed by #3707

Comments

@timokoesters
Copy link
Contributor

timokoesters commented Jun 30, 2021

There are two types of transaction ids in the to-device verification process:
The txnId, which is a path parameter for the /sendToDevice request. As far as I understand, this should never be reused unless the previous try failed (https://spec.matrix.org/unstable/client-server-api/#put_matrixclientr0sendtodeviceeventtypetxnid).
Then there is a transaction_id field that is shared by the other messages of the verification session (https://spec.matrix.org/unstable/client-server-api/#key-verification-framework).

Element Android uses the same id for both txnId and transaction_id, so on Conduit most sendToDevice requests are dropped because the txnId was already used before. This is probably the reason for #3183

How to fix:
Only use the session's transaction_id for the transaction_id field of verification message contents. Generate a new id for the txnId of /sendToDevice requests.

@bmarty
Copy link
Member

bmarty commented Jul 1, 2021

That's very good catch. We will have a look

@bmarty bmarty added this to the Sprint - Element 1.1.13 milestone Jul 1, 2021
poljar added a commit to poljar/element-android that referenced this issue Jul 20, 2021
Verification flows have something called a transaction id. This is a
client-set custom ID that identifies the flow and is established by the
first message that gets sent out. This transaction ID needs to be kept the
same and be part of all events that are sent during the verification flow.

To-device requests have something called a transaction id. This is a
client-set custom ID that identifies a given request. It is used to
ensure idempotency of requests, i.e. retrying to send a request won't
result in two events being sent as long as the transaction id is kept
the same.

This patch removes usage of the first type of transaction ID for the
second use-case.

This closes: element-hq#3589.
poljar added a commit to poljar/element-android that referenced this issue Jul 20, 2021
Verification flows have something called a transaction id. This is a
client-set custom ID that identifies the flow and is established by the
first message that gets sent out. This transaction ID needs to be kept the
same and be part of all events that are sent during the verification flow.

To-device requests have something called a transaction id. This is a
client-set custom ID that identifies a given request. It is used to
ensure idempotency of requests, i.e. retrying to send a request won't
result in two events being sent as long as the transaction id is kept
the same.

This patch removes usage of the first type of transaction ID for the
second use-case.

This closes: element-hq#3589.
@bmarty
Copy link
Member

bmarty commented Jul 21, 2021

Should be fixed on develop and in Element Android 1.1.14.

@timokoesters should be nice if you can confirm that! Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants