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.

Sign up for GitHub

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

Jump to bottom

Basic Windows Hello prompt #1865

Closed
wants to merge 10 commits into from
Closed

Basic Windows Hello prompt #1865

wants to merge 10 commits into from

Conversation

RebeccaMahany
Copy link
Contributor

@RebeccaMahany RebeccaMahany commented Sep 13, 2024 •
edited
Loading

Hello steps:

  1. Check to see if Hello is supported.
  2. If supported, create credentials -- these are created for the user + application (in this case, launcher). This step is what prompts the Hello popup.
  3. Once launcher sees that the user has accepted, it can then fetch the pubkey from those credentials.
  4. launcher can then get an attestation against those credentials.

Notes:

  • I pulled in winrt-go, which made dealing with async operations significantly easier.
  • The Hello prompt works sort of weirdly for me -- it will recognize my face and let me accept the prompt, but then give me a new prompt with an error message, and repeat until I enter my PIN instead. I haven't investigated this yet at all.
  • Should be able to refactor this code for reusability but I ran into some errors there that I haven't quite figured out yet.
  • I think I should be able to move to using UserConsentVerifier, which will give us functionality closer to what we're looking for -- but this will be a bit more work, so I want to merge this in its current state and rewrite later, to avoid blocking James's overall presence detection work.

@RebeccaMahany
Proof-of-concept for Windows Hello
31506a0 
desktop triggers Windows Hello

Fix timeout, small refactor

Tidy up names, add documentation

Retrieve key credential status

Retrieve pubkey

Get attestation

Proof-of-concept for Windows Hello
@RebeccaMahany RebeccaMahany changed the title Windows Hello proof-of-concept Basic Windows Hello prompt Sep 23, 2024
@RebeccaMahany RebeccaMahany marked this pull request as ready for review September 23, 2024 19:11
James-Pickett
James-Pickett approved these changes Sep 23, 2024
View reviewed changes
Copy link
Contributor

@James-Pickett James-Pickett left a comment

Choose a reason for hiding this comment

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

NICE!

zackattack01
zackattack01 reviewed Sep 24, 2024
View reviewed changes
ee/presencedetection/presencedetection_windows.go
defer ole.DeleteHString(credentialNameHString)

var requestCreateAsyncOperation *foundation.IAsyncOperation
requestCreateReturn, _, _ := syscall.SyscallN(
Copy link
Contributor

Choose a reason for hiding this comment

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

is the err worth keeping/noting here? or is that just not as informative as ole.NewError(requestCreateReturn)

zackattack01
zackattack01 approved these changes Sep 24, 2024
View reviewed changes
Copy link
Contributor

@zackattack01 zackattack01 left a comment

Choose a reason for hiding this comment

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

same q for the other SyscallN calls but not blocking, looks great!

@RebeccaMahany RebeccaMahany mentioned this pull request Oct 14, 2024
Merged
@RebeccaMahany
Copy link
Contributor Author

Closing in favor of #1890

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zackattack01 zackattack01 zackattack01 approved these changes

@James-Pickett James-Pickett James-Pickett approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RebeccaMahany @James-Pickett @zackattack01