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

Add FreeBSD Support #62

Merged
merged 3 commits into from
Jun 12, 2018
Merged

Add FreeBSD Support #62

merged 3 commits into from
Jun 12, 2018

Conversation

valpackett
Copy link
Contributor

@valpackett valpackett commented Apr 16, 2018

This PR adds support for FreeBSD (and makes it easier to add support for other BSDs in the future, especially DragonFly since it also uses devd).

  • BSD uhid is pretty similar to Linux hidraw, except for skipping the report id in write (and ioctls are different, of course). So the non-Linux-specific parts have been extracted into src/hidproto.rs.
  • my crate devd-rs is used for getting hotplug notifications.
  • rust-crypto to sha2: I think rust-crypto failed the build for me. The monolithic rust-crypto crate is deprecated anyway.
  • env_logger update: prompted by mach vendor rust in Firefox.
  • rustfmt for CI

The test example binary is not fully reliable for me (sometimes it hangs), but Firefox Nightly with this library seems more reliable: I've been able to complete WebAuthn and Yubico tests and authenticate on gitlab.com multiple times in the same session without any issues.

BTW, it would be nice to retry Device::new multiple times, because the notification might appear before devd rules give the user permission to access the token. (Does udev on Linux not have that problem?)

cc @jbeich

@jbeich
Copy link
Contributor

jbeich commented Apr 18, 2018

@jcjones jcjones requested a review from ttaubert May 3, 2018 00:59
Copy link
Contributor

@jcjones jcjones left a comment

Choose a reason for hiding this comment

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

This looks fine to me, but I would like @ttaubert to look closer at the dependency change to the sha2 crate, and the hidproto.rs refactor.

@ttaubert
Copy link
Contributor

ttaubert commented May 3, 2018

Thanks Greg!

env_logger update: prompted by mach vendor rust in Firefox.

We forgot to uplift a change from Firefox to the repository here. This should be fixed now.

rust-crypto to sha2: I think rust-crypto failed the build for me. The monolithic rust-crypto crate is deprecated anyway
rustfmt for CI

Those are good changes. I pulled them out so we can fix our Travis CI bustage and concentrate on the FreeBSD support code here.

https://phabricator.services.mozilla.com/D1115
https://phabricator.services.mozilla.com/D1116

BTW, it would be nice to retry Device::new multiple times, because the notification might appear before devd rules give the user permission to access the token. (Does udev on Linux not have that problem?)

AFAICT, this doesn't seem to a problem with udev. It definitely wouldn't hurt to sleep for a few hundred ms and try again though.

The test example binary is not fully reliable for me (sometimes it hangs), but Firefox Nightly with this library seems more reliable: I've been able to complete WebAuthn and Yubico tests and authenticate on gitlab.com multiple times in the same session without any issues.

That seems worrisome. Did you try to find out where exactly the statemachine is hanging? It would be good to have solid support on FreeBSD, if we claim to :)

@valpackett
Copy link
Contributor Author

Did you try to find out where exactly the statemachine is hanging?

Looks like in the read() syscall on the device.

I think Firefox is more reliable than the example because Firefox doesn't do operations one after another without delay. But it's weird that a delay would be required. A similar patch for Chromium does not do any additional delays and it works fine...

@jcjones
Copy link
Contributor

jcjones commented May 14, 2018

Since we pulled parts of this in already, this patch now needs conflicts resolved. I'm downloading a FreeBSD image currently, but I don't know when I'll get around to testing it. I'll try to do so soon.

Make the protocol parts independent of Linux code, in preparation for
adding FreeBSD support.
Tested with a YubiKey 4.
+ mention FreeBSD in readme
@valpackett
Copy link
Contributor Author

Rebased. (Kinda weird that you're pulling them through mozilla first and not the other way around, and git author info is lost...)

So, if you want to test (without running as root):

  • devd hotplug rules are in the u2f-devd package, pkg install u2f-devd
  • service devd restart after installing them to pick them up
  • pw groupmod u2f -m $USER to add yourself to the u2f group that the rules give permission to
  • of course log out & log in to get the group to apply to you
  • (all of the above commands need root/sudo/doas)

@jcjones
Copy link
Contributor

jcjones commented May 14, 2018

I am sorry about that, @myfreeweb. Tim and I decided during development to make mozilla-central the authoritative repository until we fix the vendoring issues and can import this as a crate [Bug 1395293]. I should have some breathing room finally to try again at doing that. And to move this repo to mozilla/u2f-hid-rs or similar.

@jcjones jcjones self-assigned this May 16, 2018
}
}

#[derive(Debug)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a note for myself in the future: We pulled a lot of #[derive(Debug)]s out to shrink the compiled library, but I think that was unneeded since we moved to more recent versions of rust.

Copy link
Contributor

@jcjones jcjones left a comment

Choose a reason for hiding this comment

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

@jcjones jcjones merged commit df493ee into mozilla:master Jun 12, 2018
@jcjones
Copy link
Contributor

jcjones commented Jun 12, 2018

Thank you, @myfreeweb and @jbeich so much! Landing this in Firefox...

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jun 12, 2018
Upstream PR: mozilla/authenticator-rs#62

* Extract hidproto module from linux::hidraw
Make the protocol parts independent of Linux code, in preparation for
adding FreeBSD support.

* Add FreeBSD (uhid + devd) support
Tested with a YubiKey 4.

Differential Revision: https://phabricator.services.mozilla.com/D1636
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jun 15, 2018
Summary:
Upstream PR: mozilla/authenticator-rs#62

* Extract hidproto module from linux::hidraw
Make the protocol parts independent of Linux code, in preparation for
adding FreeBSD support.

* Add FreeBSD (uhid + devd) support
Tested with a YubiKey 4.

Tags: #secure-revision

Bug #: 1468349

Differential Revision: https://phabricator.services.mozilla.com/D1636

MozReview-Commit-ID: 8NNWRgTEMn2

--HG--
extra : rebase_source : edf774f0a993a18b59b5f8aa10e0977d94ea1de8
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
Upstream PR: mozilla/authenticator-rs#62

* Extract hidproto module from linux::hidraw
Make the protocol parts independent of Linux code, in preparation for
adding FreeBSD support.

* Add FreeBSD (uhid + devd) support
Tested with a YubiKey 4.

Differential Revision: https://phabricator.services.mozilla.com/D1636

UltraBlame original commit: 80190d88549cf49b79fea1d350e65d8828f33810
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
Summary:
Upstream PR: mozilla/authenticator-rs#62

* Extract hidproto module from linux::hidraw
Make the protocol parts independent of Linux code, in preparation for
adding FreeBSD support.

* Add FreeBSD (uhid + devd) support
Tested with a YubiKey 4.

Tags: #secure-revision

Bug #: 1468349

Differential Revision: https://phabricator.services.mozilla.com/D1636

MozReview-Commit-ID: 8NNWRgTEMn2

UltraBlame original commit: 8ebf45b0854ace6848090c02d6c4935bc515fe0a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
Upstream PR: mozilla/authenticator-rs#62

* Extract hidproto module from linux::hidraw
Make the protocol parts independent of Linux code, in preparation for
adding FreeBSD support.

* Add FreeBSD (uhid + devd) support
Tested with a YubiKey 4.

Differential Revision: https://phabricator.services.mozilla.com/D1636

UltraBlame original commit: 80190d88549cf49b79fea1d350e65d8828f33810
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
Summary:
Upstream PR: mozilla/authenticator-rs#62

* Extract hidproto module from linux::hidraw
Make the protocol parts independent of Linux code, in preparation for
adding FreeBSD support.

* Add FreeBSD (uhid + devd) support
Tested with a YubiKey 4.

Tags: #secure-revision

Bug #: 1468349

Differential Revision: https://phabricator.services.mozilla.com/D1636

MozReview-Commit-ID: 8NNWRgTEMn2

UltraBlame original commit: 8ebf45b0854ace6848090c02d6c4935bc515fe0a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 3, 2019
Upstream PR: mozilla/authenticator-rs#62

* Extract hidproto module from linux::hidraw
Make the protocol parts independent of Linux code, in preparation for
adding FreeBSD support.

* Add FreeBSD (uhid + devd) support
Tested with a YubiKey 4.

Differential Revision: https://phabricator.services.mozilla.com/D1636

UltraBlame original commit: 80190d88549cf49b79fea1d350e65d8828f33810
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 3, 2019
Summary:
Upstream PR: mozilla/authenticator-rs#62

* Extract hidproto module from linux::hidraw
Make the protocol parts independent of Linux code, in preparation for
adding FreeBSD support.

* Add FreeBSD (uhid + devd) support
Tested with a YubiKey 4.

Tags: #secure-revision

Bug #: 1468349

Differential Revision: https://phabricator.services.mozilla.com/D1636

MozReview-Commit-ID: 8NNWRgTEMn2

UltraBlame original commit: 8ebf45b0854ace6848090c02d6c4935bc515fe0a
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.

None yet

4 participants