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

Fix device discovery in NitrokeyManager::connect_with_path #194

Merged
merged 1 commit into from
Apr 12, 2021

Conversation

d-e-s-o
Copy link
Contributor

@d-e-s-o d-e-s-o commented Mar 21, 2021

The device discovery in NitrokeyManager's connect_with_path method is
broken. It tries to statically determine the vendor ID by using the
first working HID pointer. That's obviously not working when a both a
Nitrokey device and Librem Key are connected and the path is describing
a Librem Key. In such a case the vendor ID used would be that for the
Nitrokey and we would not be able to find the Librem Key at all. This
change fixes up the logic.

The device discovery in NitrokeyManager's connect_with_path method is
broken. It tries to statically determine the vendor ID by using the
first working HID pointer. That's obviously not working when a both a
Nitrokey device and Librem Key are connected and the path is describing
a Librem Key. In such a case the vendor ID used would be that for the
Nitrokey and we would not be able to find the Librem Key at all. This
change fixes up the logic.
@d-e-s-o d-e-s-o force-pushed the topic/fix-device-discovery branch 2 times, most recently from 0da3469 to 3faa000 Compare March 21, 2021 01:17
@d-e-s-o
Copy link
Contributor Author

d-e-s-o commented Apr 10, 2021

Can this pull request please be merged? @szszszsz

@szszszsz
Copy link
Member

Noted! Sorry for the delay. Scheduled early for the next week.

@szszszsz szszszsz self-assigned this Apr 12, 2021
@szszszsz szszszsz self-requested a review April 12, 2021 16:21
@szszszsz szszszsz closed this in 6401ca4 Apr 12, 2021
@szszszsz szszszsz merged commit 6401ca4 into Nitrokey:master Apr 12, 2021
@szszszsz
Copy link
Member

Merged, thank you!

@szszszsz szszszsz removed their request for review April 12, 2021 16:26
@d-e-s-o d-e-s-o deleted the topic/fix-device-discovery branch April 18, 2021 17:09
@d-e-s-o
Copy link
Contributor Author

d-e-s-o commented Apr 18, 2021

Will you cut a new release with this fix? Without it, it's not possible to test for the Librem Key without manual labor in the form of physically replugging hardware keys.

@szszszsz
Copy link
Member

Yes, should be done this week (delayed from the previous one).

@szszszsz szszszsz added this to the v3.7 milestone Apr 22, 2021
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