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

Implement Security Key Auth #617

Merged
merged 6 commits into from
Oct 13, 2024
Merged

Conversation

kinoroy
Copy link
Contributor

@kinoroy kinoroy commented Sep 29, 2024

Security Key Authentication

Fixes #357, Fixes #439, Fixes #578, Fixes #549, Fixes #411

Introduction

Currently, if a user of the app tries to sign in with an Apple ID that uses physical security keys as a 2nd-factor, the auth flow will fail as this is currently unsupported

Motivation

As physical security keys are a security improvement on SMS based 2FA, we should support this to allow use of Xcodes without compromising their Apple ID account security. There are also many open issues currently requesting this functionality (linked in the top)

Proposed solution

This PR modifies the auth flow, to detect when a security key challenge is issued, prompt the user to use their security key, and uses libfido2 to create an assertion on the challenge and sends it back to Apple

Detailed design

Apple uses the standard WebAuthn flow to add and verify security keys

To interact with the physical security key device, we can use the libfido2 C library. Since this library has some development and runtime dependencies on shared libraries and headers (libcrypto/openssl, libcbor), I decided to wrap this functionality in a Swift Package, LibFido2Swift

(Full disclaimer: I wrote this library, for this PR in particular, and it's hosted in my personal repo. If that's a problem, I could make local copy hosted in this repo, however I thought that this would simplify the PR as it avoids adding lots of library files and dozens of headers files as well as the implementation details of working with the security key device which isn't really important)

When the user enters their login credentials, and Xcodes detects a WebAuthn challenge returned from Apple, Xcodes will show a new step in the auth flow (SignInSecurityKeyPinView), which will ask the user to insert their security key and enter the PIN code (Apple seems to require the PIN to be entered when authenticating with them):

Screenshot 2024-09-26 at 3 04 21 PM

When the user enters their PIN, Xcodes opens and finds the security key, and then prompts the user to touch the key to confirm signing in (SignInSecurityKeyTouchView) (again, Apple requires this "user verification step")

Screenshot 2024-09-28 at 5 19 49 PM

Once the user touches the key, the WebAuthn assertion is generated from the key, and sent to Apple to respond to the challenge. The user is now logged in!

Testing

I tested this with my own Apple ID, secured using a YubiKey 5C NFC device. This is the only security key I have available for testing so hopefully others can help test with other types of keys if they have them.

There didn't seem to be any unit tests for authentication flow so I didn't add or change any unit tests (verified that the test suite still passes though). If you'd like unit tests added for this and could guide me on that I would be open to adding some

Alternatives considered

macOS has an AuthenticationServices framework available which can handle security key authentication and show the system UI for doing so. I initially looked into using this. However to use this framework, the app has to prove that they own the domain which they are authenticating against using an associated domains entitlement. Obviously we do not own apple.com so this was not possible 😅

Fin

It was a fun challenge to do this! Thank you @MattKiazyk for this super useful app and I hope this contribution is helpful ☺️

@czottmann
Copy link

I'll be honest, I haven't been this excited about a PR in a long time!

@MattKiazyk
Copy link
Contributor

@kinoroy thanks for all your work on this. This was something that is not part of my day to day process and when the community can help out it's amazing.

The current CI fail is because libfido2swift is on Swift 6. Hopefully it possible to use 5.X as I'm not ready yet to use Xcode 16 here yet.

For the XCStrings - for each of your keys you added can you add the english localization. I know it's annoying but by default other languages would show the key, and not the english description.

Wondering also if you or others could be able to double test the PR. @czottmann ? If not, most likely next build I will make it beta to test it out.

Thanks again!

@czottmann
Copy link

czottmann commented Oct 12, 2024

@MattKiazyk I don't have much spare time to check PRs currently, sorry, but I'm willing to testdrive any RC you might conjure up!

@MattKiazyk
Copy link
Contributor

Hey @kinoroy thanks for all the changes.

If you follow the https://github.com/XcodesOrg/XcodesApp/actions/runs/11308810070/job/31455648104?pr=617 link you'll see which keys are missing on which localizations. Also if you go into the localizable file you can see which ones are new and need keys. Please update these new keys for all languages.

Screenshot 2024-10-12 at 9 06 30 PM

@kinoroy
Copy link
Contributor Author

kinoroy commented Oct 13, 2024

@MattKiazyk Thanks for the clarification, I went and added the English strings for all the languages

@MattKiazyk
Copy link
Contributor

Thanks for you all your work on this @kinoroy!

@MattKiazyk MattKiazyk merged commit 8277554 into XcodesOrg:main Oct 13, 2024
2 checks passed
@psaintemarie
Copy link

Thanks for this. Can we expect a new release soon?

@MattKiazyk
Copy link
Contributor

Soon. Im hesitant to fully release it until Xcode 16.1 comes out because of the requirement of 16.1 beta 3+ for the runtime downloads but if I have some time might make a beta release.

Thanks for all your work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants