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

Reject commands from multiple transports #19

Merged
merged 2 commits into from
Jun 28, 2023
Merged

Conversation

sosthene-nitrokey
Copy link
Contributor

Apdu-dispatch does not have proper support for multiple concurrent transport since there are no separate selection for each app.

The proper solution would be to have separate buffers and selection per transport, but that would also require apps to actually deal with multiple interfaces, which they currently do not.

This patch makes it so that the first command sets the used transport, and that
any command comming from another transport will be rejected.

Apdu-dispatch does not have proper support for multiple concurrent
transport since there are no separate selection for each app.

The proper solution would be to have separate buffers and selection per transport,
but that would also require apps to actually deal with multiple interfaces, which they currently do not.

This patch makes it so that the first command sets the used transport, and that
 any command comming from another transport will be rejected.
Copy link
Member

@robin-nitrokey robin-nitrokey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Alternatively, we could only set the interface on the first successful select but this is only a theoretical improvement.

@robin-nitrokey
Copy link
Member

But please add a changelog entry.

@sosthene-nitrokey
Copy link
Contributor Author

LGTM. Alternatively, we could only set the interface on the first successful select but this is only a theoretical improvement.

Alternatively we could also use only one Interchange and let the runner connect it to either NFC or USBD-CCID. That way we can also save on statically allocated memory.

@sosthene-nitrokey sosthene-nitrokey merged commit f8d5e77 into main Jun 28, 2023
@robin-nitrokey
Copy link
Member

Hmm, true. And runners that want to support both at the same time could set up two dispatches.

robin-nitrokey added a commit to Nitrokey/nitrokey-3-firmware that referenced this pull request Oct 25, 2023
This patch updates some of the unreleased dependencies.  This pulls in
some stack usage optimizations and the change in apdu-dispatch that
forbids commands from multiple transports, see:
  #353
  trussed-dev/apdu-dispatch#19
robin-nitrokey added a commit to Nitrokey/nitrokey-3-firmware that referenced this pull request Oct 25, 2023
This patch updates some of the unreleased dependencies.  This pulls in
some stack usage optimizations and the change in apdu-dispatch that
forbids commands from multiple transports, see:
  #353
  trussed-dev/apdu-dispatch#19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants