WebAuthn: Strictly require domain strings as RP IDs

RESOLVED FIXED in Firefox 57

Status

()

Core
DOM: Device Interfaces
P1
enhancement
RESOLVED FIXED
9 months ago
7 months ago

People

(Reporter: jcj, Assigned: jcj)

Tracking

(Blocks: 2 bugs)

Trunk
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [webauthn] [webauthn-interop])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

9 months ago
Bug 1380421 permits WebAuthn to use RP IDs which are Origins, while the WD-06 and later drafts of the spec require RP IDs to be Domain Strings.

Once we're past the WD-05 interop period, we should be strict that all RP IDs be Domain Strings.
(Assignee)

Updated

9 months ago
Blocks: 1384776
(Assignee)

Comment 1

8 months ago
It's been agreed that all interop participants will actually use only Domain Strings, not Origins, so we need to do this bug before the interop day after-all.
Assignee: nobody → jjones
Status: NEW → ASSIGNED
Priority: P3 → P1
QA Contact: mwobensmith
Whiteboard: [webauthn] [webauthn-interop]
Comment hidden (mozreview-request)

Comment 3

7 months ago
mozreview-review
Comment on attachment 8906665 [details]
Bug 1381126: Resume requiring WebAuthn RP ID to be a Domain String

https://reviewboard.mozilla.org/r/178378/#review183442

LGTM.

::: dom/webauthn/tests/test_webauthn_loopback.html:73
(Diff revision 1)
>  
>      return webAuthnDecodeCBORAttestation(aCredInfo.response.attestationObject.buffer)
>      .then(function(decodedResult) {
> +      // Make sure the RP ID hash matches what we calculate.
> +      return crypto.subtle.digest("SHA-256", string2buffer(document.domain))
> +      .then(function(calculatedHash){

nit: space before '{'

::: dom/webauthn/tests/test_webauthn_loopback.html:74
(Diff revision 1)
>      return webAuthnDecodeCBORAttestation(aCredInfo.response.attestationObject.buffer)
>      .then(function(decodedResult) {
> +      // Make sure the RP ID hash matches what we calculate.
> +      return crypto.subtle.digest("SHA-256", string2buffer(document.domain))
> +      .then(function(calculatedHash){
> +        is(bytesToBase64(new Uint8Array(calculatedHash)), bytesToBase64(decodedResult.rpIdHash), "Calculated RP ID hash must match what the browser derived.");

nit: break up long line
Attachment #8906665 - Flags: review?(dkeeler) → review+
(Assignee)

Comment 4

7 months ago
mozreview-review-reply
Comment on attachment 8906665 [details]
Bug 1381126: Resume requiring WebAuthn RP ID to be a Domain String

https://reviewboard.mozilla.org/r/178378/#review183442

Thanks for the review!
Comment hidden (mozreview-request)
(Assignee)

Updated

7 months ago
Keywords: checkin-needed
Version: 55 Branch → Trunk

Comment 6

7 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/9584975d84e0
Resume requiring WebAuthn RP ID to be a Domain String r=keeler
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9584975d84e0
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.