saml2aws
saml2aws copied to clipboard
Support U2F/WebAuthn keys USB detection in Linux
This PR updates marshallbrekka/go-u2fhost, imported by the Okta and Google providers to support U2F/WebAuthn USB keys as 2FA devices.
The update should help fix the problems reported in https://github.com/Versent/saml2aws/issues/419 as they had their origin in marshallbrekka/go-u2fhost not able to properly detect these devices in Linux. That was fixed upstream with https://github.com/marshallbrekka/go-u2fhost/pull/10 which also explains what other changes were done in the hid libraries used by go-u2fhost to make the detection work in Linux too.
Along the library change and other mod updates, this commit includes a change to the makefile to support the required tag to use hidraw instead of libusb when compiling linux (and so really apply the fix when compiling) as well as related changes to make it easier to build/install saml2aws in different platforms, including OS detection to prevent macOS builds from building in non-Darwin environments, which should tackle https://github.com/Versent/saml2aws/issues/56.
Finally, I've also included some updates in the README to clarify the changes in the build process and the additional per OS targets.
This is a fantastic idea but i am really not keen on adding any CGO dependent stuff as it is already killing me.
I would love to figure out a way to put this in a plugin or hook which people can install separately.
Would love to get this merged, had to fork the fork, clone, and compile it myself :man_shrugging:.
This is a fantastic idea but i am really not keen on adding any CGO dependent stuff as it is already killing me.
@wolfeidau Your project is already using it. I just updated the go-u2fhost ref so as to pick up the change in the underlying library used by it, so a dependency of your dependency.
Under the hood, go-u2fhost was using github.com/karalabe/hid, I just made a contribution (referenced above) to change that to github.com/bearsh/hid, which is a fork that updated karalabe's libusb and hidapi code base with upstream libusb/libusb and libusb/hidapi, which among other things incorporates the changes in libusb/hidapi to support Usage Page and Usage on Linux with hidraw (see https://github.com/libusb/hidapi/pull/139/files) when packages are built with "-tags=hidraw", while maintaining the old behaviour when built without it. I also contributed with a fix to the fork.
In brief, this PR only adds the "-tags=hidraw" tag on Linux builds to support Usage Page and Usage on Linux with hidraw. The rest of the changes should address other building issues but do not include additional CGO dependencies that were not already there in previous versions.
If you want me to update my branch with master to reconsider, I'll be happy to. Let me know.
Hi, thanks for working on this! I found this PR and rebased it onto master
to get multiple yubikey support in Okta. The nice thing is that master
now has the updated go-u2fhost module version so this PR can be trimmed down to only README and Makefile changes :tada:
In any case, I pushed a branch with the rebase over here: https://github.com/smlx/saml2aws/tree/fido-linux-support
cc @ocraviotto and @wolfeidau
edit: forgot to mention that this PR works beautifully :+1:
@wolfeidau I've gone ahead and refactored my initial PR to reflect all of your changes to the release flow with goreleaser. Had a couple of initial failures but finally managed to make it work. I've basically split the build/release process between linux and non-linux in Ubuntu and MacOs contexts respectively. This should maintain the original context for non-linux and build and so support u2f keys in linux by building on Ubuntu. I've also changed the Makefile to allow building with goreleaser locally, except that it won't build linux if in non-linux.
I'm not sure whether MacOs binaries would work for all cases if built on Ubuntu (asked a colleague to test and the limited cases he tried did work, but can't be sure without further testing), in which case it would be possible to move the build/release fully to ubuntu-latest (not sure why you had builds on ubuntu and Releases done with macos?).
Finally, I've changed the Build step from macOS-latest
to macos-latest
(aligned case to the docs specs for OS types supported and their YAML label), so I suspect that's why the PR check is missing: https://github.com/Versent/saml2aws/pull/592/checks?check_run_id=2295884756.
Do let me know if this now looks more in line with what you wanted and of course I'd be happy to read your feedback.
PS: I've also fixed a bug I found with the latest changes to the okta provider: See my commit message and the line changed for more details.
@smlx Tanks for trying it out. Please note I've rebased and updated with changes that should allow you to build with U2F keys support from linux. I've created a test release in my fork with almost the same code base I used to update this PR. Release builds here: https://github.com/ocraviotto/saml2aws/releases/tag/v2.28.5-testrel1
@ocraviotto yeah I have dug into this and agree it is 100% already there and actively used.
I need to pull apart your PR a bit as a lot of things are going in there.
@wolfeidau Rebased once more. I see actions need a maintainer's approval to run, might be a good additional check. I have fixed a minor conflict but have not noticed anything else worth checking in more detail, though did run tests and builds locally. As earlier, if there is anything I can do to help, happy to.
Hello, we have test fork release for FIDO U2F on linux Fedora, it works perfectly.
When you can integrated this in release ?
worked perfectly for fedora :D libudev-dev quivalent in fedora is ... systemd-devel
dnf install systemd-devel
any news on this one ?
@duboisf : Anything I can do to help to get this merged?
I would love to be able to use it in linux (and i could use a custom build myself, as hack), but I don't want to recommend it to others in my team unless it merged upstream.
Hello, as an interested party, any reason why this is still pending and support for webauthn not merged yet? From the comments seems the PR works. What are the pending concerns?
@wolfeidau Any recent insights on this? I'm similarly facing the merge-or-fork dilemma others had faced in this thread prior as my organization works to limit second factors to more secure forms. If there are outstanding items you feel should be addressed then there can be resources attributed to them, but currently this pull request lacks feedback to @ocraviotto or anyone else on how to proceed.