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

Adding support for android devices #92

Closed
lisandromaselli opened this issue Jan 5, 2023 · 8 comments · Fixed by #94
Closed

Adding support for android devices #92

lisandromaselli opened this issue Jan 5, 2023 · 8 comments · Fixed by #94
Labels
status/needs-triage Issues that need to be triaged. type/feature-request Feature Requests

Comments

@lisandromaselli
Copy link

Description

Android origin has the format of android:apk-key-hash:${androidHash} and for that reason is not a fullly qualified name. This case need to be handled outside the package?

Use Case

No response

Documentation

https://github.com/google/webauthndemo/blob/main/src/libs/webauthn.ts

@lisandromaselli lisandromaselli added status/needs-triage Issues that need to be triaged. type/feature-request Feature Requests labels Jan 5, 2023
@james-d-elliott
Copy link
Member

I don't believe RPOrigins are not required to be fully qualified names in the library. Do you maybe mean the RPID? This has specific requirements which must be respected.

@FreddyDevelop
Copy link
Contributor

Hi @james-d-elliott,
I think the problem is here:

// Registration Step 5 & Assertion Step 9. Verify that the value of C.origin matches
// the Relying Party's origin.
fqOrigin, err := FullyQualifiedOrigin(c.Origin)
if err != nil {
return ErrParsingData.WithDetails("Error decoding clientData origin as URL")
}

where the library formats the origin from the CollectedClientData as a fully qualified name. Which will not work for android, because android returns android:apk-key-hash:${androidHash} as origin and the format function then returns android:https://

Two possible solutions are:

  1. remove the formatting
  2. only format the origin when it does not start with android

@james-d-elliott
Copy link
Member

james-d-elliott commented Jan 9, 2023

Yeah you're right, thanks for putting the time in to find this. I will closely read relevant specs but it seems like this might be specifically the FIDO2 spec and is not part of the Webauthn spec at all. Which makes sense as it's not using Webauthn, it's a native API.

Webauthn itself requires this exact validation it seems, and the origin being formatted as a URL. As to how we solve it I'll have to closely look at the spec that governs it and it'll likely be similar to 2, but we'd use the full prefixes for those that exist (think there are two). I'm thinking it may also be wise to require this to be something that's explicitly enabled.

I would also be curious @nicksteele thinks if he has time to chime in (I would also like to remind him that there's a standing invitation to the org as a maintainer if he desires).

@aseigler
Copy link
Collaborator

aseigler commented Jan 9, 2023

I've seen this before: passwordless-lib/fido2-net-lib#237

We ended up not formatting the origin if the origin had an unknown structure from UriHostNameType.

@james-d-elliott
Copy link
Member

Thanks a lot Alex I really appreciate the insight. I'm.assuming there were no additional considerations?

james-d-elliott added a commit that referenced this issue Jan 29, 2023
This adds support for the Android Native FIDO2 Origins as well as some additional verification of the Origin.

Closes #92
@lisandromaselli
Copy link
Author

Thx!

@nicksteele
Copy link
Contributor

Apologies for chiming in 4 months later. The multiple auth origin scheme is android-specific currently. As far as I know there's no plans to have hashes or multiple origin IDs for other response types. @james-d-elliott now that I'm back from my startup sabbatical I'm happy to help as a maintainer.

@james-d-elliott
Copy link
Member

Great news. I'll invite you to the org then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/needs-triage Issues that need to be triaged. type/feature-request Feature Requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants