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

SSH Agent: Add support for OpenSSH 8.2 FIDO/U2F keys #6371

Merged
merged 4 commits into from
Oct 1, 2021

Conversation

hifi
Copy link
Member

@hifi hifi commented Apr 6, 2021

Finally closes #4334 😅

Shout-out to @Yubico for sending me keys with up-to-date firmware to properly test this!

Testing strategy

Manual tests with Yubikeys on Linux. Automated tests for the in-wire format.

This needs to be tested by people with Yubikeys other than me before merging.

Type of change

  • ✅ New feature (change that adds functionality)

@hifi hifi added this to the v2.7.0 milestone Apr 6, 2021
@hifi hifi force-pushed the feature/openssh-sk-botan branch from 1ba945d to d67a460 Compare April 6, 2021 05:07
@networkException
Copy link

I tried testing this branch a database but unlocking failed due to a HMAC mismatch (the database has a hardware key as an additional credential)

@hifi
Copy link
Member Author

hifi commented Jul 8, 2021

That may be an older pre-release bug and this just needs a rebase. Does the current develop/snapshot work for you?

@networkException
Copy link

networkException commented Jul 8, 2021

Yep develop works fine. I tried to do a quick rebase but it had conflicts, if it helps I will try to look into resolving them I think I rebased it onto the wrong branch

@networkException
Copy link

Rebasing onto develop resolved the unlocking issue and using ed25519-sk works flawlessly! The key is only present in the agent when unlocked as expected and I need to confirm with touching the yubikey. Great work 👍

@droidmonkey
Copy link
Member

@hifi is this ready for review? Probably needs a rebase on develop.

@hifi hifi force-pushed the feature/openssh-sk-botan branch from d67a460 to b5a4f1c Compare September 27, 2021 18:32
@hifi
Copy link
Member Author

hifi commented Sep 27, 2021

@droidmonkey should be ready for review now. Not sure what that test failure was but I rebased and the code looks pretty much identical.

@leonlag
Copy link

leonlag commented Sep 30, 2021

Adding and removing ed25519-sk keys on fedora 34 using a solokey v1 works for me. Thanks for your great work!

Copy link
Member

@droidmonkey droidmonkey left a comment

Choose a reason for hiding this comment

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

Beautiful code @hifi!

@droidmonkey droidmonkey force-pushed the feature/openssh-sk-botan branch from b5a4f1c to 18e4ceb Compare October 1, 2021 20:02
@codecov-commenter
Copy link

codecov-commenter commented Oct 1, 2021

Codecov Report

Merging #6371 (18e4ceb) into develop (1dbec40) will increase coverage by 0.02%.
The diff coverage is 81.54%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #6371      +/-   ##
===========================================
+ Coverage    63.77%   63.78%   +0.02%     
===========================================
  Files          330      330              
  Lines        41378    41430      +52     
===========================================
+ Hits         26385    26426      +41     
- Misses       14993    15004      +11     
Impacted Files Coverage Δ
src/core/Config.cpp 86.50% <ø> (ø)
src/core/Config.h 100.00% <ø> (ø)
src/sshagent/SSHAgent.h 100.00% <ø> (ø)
src/sshagent/SSHAgent.cpp 68.53% <64.29%> (-0.94%) ⬇️
src/sshagent/AgentSettingsWidget.cpp 61.22% <71.43%> (+1.70%) ⬆️
src/sshagent/OpenSSHKey.cpp 79.01% <82.35%> (+0.86%) ⬆️
src/sshagent/ASN1Key.cpp 96.72% <100.00%> (+0.11%) ⬆️
src/core/FileWatcher.cpp 86.75% <0.00%> (+1.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1dbec40...18e4ceb. Read the comment docs.

@droidmonkey droidmonkey merged commit 860fcfd into keepassxreboot:develop Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SSH Agent: Support OpenSSH 8.2 security key (FIDO/U2F) backed SSH keys
5 participants