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

Add the topOrigin field to the limited clientData verification algorithm #2123

Closed
wants to merge 1 commit into from
Closed

Add the topOrigin field to the limited clientData verification algorithm #2123

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3919,7 +3919,9 @@ Verifiers may use the following algorithm to verify an encoded {{CollectedClient
1. A string, |type|, that contains the expected {{CollectedClientData/type}}.
1. A byte string, |challenge|, that contains the challenge byte string that was given in the {{PublicKeyCredentialRequestOptions}} or {{PublicKeyCredentialCreationOptions}}.
1. A string, |origin|, that contains the expected {{CollectedClientData/origin}} that issued the request to the user agent.
1. A boolean, |crossOrigin|, that is true if, and only if, the request should have been performed within a cross-origin <{iframe}>.
1. A string or null value, |expectedTopOrigin|, which contains the expected [=top-level origin=] for a cross-origin request, or else a null value to indicate that request must not have been performed in a cross-origin <{iframe}>.

Note: a non-null value for |expectedTopOrigin| will cause all {{CollectedClientData}} structures generated by previous versions of this specification to be rejected as previous versions did not serialize the [=top-level origin=].
Copy link
Member

Choose a reason for hiding this comment

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

Maybe indent this to make it part of the preceding list item?

Suggested change
Note: a non-null value for |expectedTopOrigin| will cause all {{CollectedClientData}} structures generated by previous versions of this specification to be rejected as previous versions did not serialize the [=top-level origin=].
Note: a non-null value for |expectedTopOrigin| will cause all {{CollectedClientData}} structures generated by previous versions of this specification to be rejected as previous versions did not serialize the [=top-level origin=].

Copy link
Member

Choose a reason for hiding this comment

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

Also: with this procedure, there is no way to validate a client data with "crossOrigin":true but without a "topOrigin". I would like to just reference the L2 verification procedure as a fallback, but that won't actually work: this L3 procedure would reject an incorrect "topOrigin", but the fallback would accept that incorrect "topOrigin" since that attribute is ignored in the L2 procedure.

In additions to #2104 I addressed this by adding a Boolean requireTopOrigin parameter and making some verification steps conditional on that, i.e.:

1. Append 0x2c2263726f73734f726967696e223a (`,"crossOrigin":`) to |expected|.
1. If |expectedTopOrigin| is null:
    1. Append 0x66616c7365 (`false`) to |expected|.
1. Else:
    1. Append 0x74727565 (`true`) to |expected|.
    1. If |requireTopOrigin| is true
        or if 0x2c22746f704f726967696e223a (`,"topOrigin":`) is a prefix
        of the substring of |clientDataJSON| beginning at the offset equal to the length of |expected|:
        1. Append 0x2c22746f704f726967696e223a (`,"topOrigin":`) to |expected|.
        1. Append [=CCDToString=](|expectedTopOrigin|) to |expected|.

What do you think, is this worth addressing?

Copy link
Contributor

Choose a reason for hiding this comment

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

I very much think this is worth addressing. While the algorithm is more complex, it would be nice if the limited verification algorithm were backwards compatible when possible.

1. Let |expected| be an empty byte string.
1. Append 0x7b2274797065223a (`{"type":`) to |expected|.
1. Append [=CCDToString=](|type|) to |expected|.
Expand All @@ -3929,10 +3931,11 @@ Verifiers may use the following algorithm to verify an encoded {{CollectedClient
1. Append 0x2c226f726967696e223a (`,"origin":`) to |expected|.
1. Append [=CCDToString=](|origin|) to |expected|.
1. Append 0x2c2263726f73734f726967696e223a (`,"crossOrigin":`) to |expected|.
1. If |crossOrigin| is true:
1. Append 0x74727565 (`true`) to |expected|.
1. Otherwise, i.e. |crossOrigin| is false:
1. If |expectedTopOrigin| is null:
1. Append 0x66616c7365 (`false`) to |expected|.
1. Else:
1. Append 0x747275652c22746f704f726967696e223a (`true,"topOrigin":`) to |expected|.
1. Append [=CDDToString=](|expectedTopOrigin|) to |expected|.
1. If |expected| is not a prefix of |clientDataJSON| then the verification has failed.
1. If |clientDataJSON| is not at least one byte longer than |expected| then the verification has failed.
1. If the byte of |clientDataJSON| at the offset equal to the length of |expected|:
Expand Down