Rewrite macOS IOHIDManager communication and state machine #52
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jcjones
approved these changes
Nov 14, 2017
|
|
||
| // Iterate the exlude list and see if there are any matches. | ||
| // Abort the state machine if we found a valid key handle. | ||
| if key_handles.iter().any(|key_handle| { |
There was a problem hiding this comment.
Nit: feels like you should use the same chaining style here as around line 105... either's fine, but ... one. :)
There was a problem hiding this comment.
They're not quite the same. For register() we want to bail out as soon as we match one of the entries in the exclusion list. In sign() we want to actually get a list of all matching tokens and try to sign with those one by one.
I'll leave this as is and land it. If you disagree, let's open an issue and I'll be happy to address that in a follow-up :)
| let manager = unsafe { IOHIDManagerCreate(kCFAllocatorDefault, kIOHIDManagerOptionNone) }; | ||
|
|
||
| // Match FIDO devices only. | ||
| let _matcher = IOHIDDeviceMatcher::new(); |
There was a problem hiding this comment.
btw, I'm super impressed that you figured out how to make this work. I fought it ... too long.
There was a problem hiding this comment.
It took... at lot of time :)
src/macos/monitor.rs
Outdated
|
|
||
| let rv = IOHIDManagerOpen(self.manager, kIOHIDManagerOptionNone); | ||
| if rv != 0 { | ||
| return Err(io_err("Couldn't open HID Manager")); |
There was a problem hiding this comment.
Nit: Probably good to put the rv into the message, but shrug
ttaubert
force-pushed
the
macos-statemachine
branch
from
be156ac
to
4906ba8
Compare
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Nov 14, 2017
…nd state machine r=jcj Review: mozilla/authenticator-rs#52
xeonchen
pushed a commit
to xeonchen/gecko-cinnabar
that referenced
this pull request
Nov 14, 2017
…nd state machine r=jcj Review: mozilla/authenticator-rs#52
JerryShih
pushed a commit
to JerryShih/gecko-dev
that referenced
this pull request
Nov 15, 2017
…nd state machine r=jcj Review: mozilla/authenticator-rs#52
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Nov 20, 2017
…'t use KeyHandleMatcher r=jcj This patch rewrites the Linux backend to have a structure similar to the macOS one [1]. We'll have one thread per device, which means that a device that is stuck or unresponsive won't break other devices. [1] mozilla/authenticator-rs#52 Review: mozilla/authenticator-rs#56 mozilla/authenticator-rs@7d9f31a
JerryShih
pushed a commit
to JerryShih/gecko-dev
that referenced
this pull request
Nov 21, 2017
…'t use KeyHandleMatcher r=jcj This patch rewrites the Linux backend to have a structure similar to the macOS one [1]. We'll have one thread per device, which means that a device that is stuck or unresponsive won't break other devices. [1] mozilla/authenticator-rs#52 Review: mozilla/authenticator-rs#56 mozilla/authenticator-rs@7d9f31a
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Oct 2, 2019
…nd state machine r=jcj Review: mozilla/authenticator-rs#52 UltraBlame original commit: 6fab343d63b66b26b0f42f2c317a3279257f31d9
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Oct 2, 2019
…'t use KeyHandleMatcher r=jcj This patch rewrites the Linux backend to have a structure similar to the macOS one [1]. We'll have one thread per device, which means that a device that is stuck or unresponsive won't break other devices. [1] mozilla/authenticator-rs#52 Review: mozilla/authenticator-rs#56 mozilla/authenticator-rs@7d9f31a UltraBlame original commit: 066c5f6d20c759fcf65f8065010d32db34230e2e
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Oct 2, 2019
…nd state machine r=jcj Review: mozilla/authenticator-rs#52 UltraBlame original commit: 6fab343d63b66b26b0f42f2c317a3279257f31d9
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Oct 2, 2019
…nd state machine r=jcj Review: mozilla/authenticator-rs#52 UltraBlame original commit: 6fab343d63b66b26b0f42f2c317a3279257f31d9
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Oct 2, 2019
…'t use KeyHandleMatcher r=jcj This patch rewrites the Linux backend to have a structure similar to the macOS one [1]. We'll have one thread per device, which means that a device that is stuck or unresponsive won't break other devices. [1] mozilla/authenticator-rs#52 Review: mozilla/authenticator-rs#56 mozilla/authenticator-rs@7d9f31a UltraBlame original commit: 066c5f6d20c759fcf65f8065010d32db34230e2e
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Oct 2, 2019
…'t use KeyHandleMatcher r=jcj This patch rewrites the Linux backend to have a structure similar to the macOS one [1]. We'll have one thread per device, which means that a device that is stuck or unresponsive won't break other devices. [1] mozilla/authenticator-rs#52 Review: mozilla/authenticator-rs#56 mozilla/authenticator-rs@7d9f31a UltraBlame original commit: 066c5f6d20c759fcf65f8065010d32db34230e2e
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
r? @jcjones
Looking forward to your feedback!
This seems very stable, I can't make a custom Firefox panic when plugging/unplugging multiple times. Re-plugged devices are properly registered, and work for registration and signing. With Chrome open, I also can't reproduce bug 1398268 anymore.