Clean up attestation, abstract it from UA, fix TPM format, add U2F format #321

vijaybh · 2017-01-11T18:08:09Z

In addition to the bugs named in the commit messages, this also fixes #123 and fixes #238 by cleaning up the algorithm naming.

Fixes #188

Puts all attestation info into a CBOR object which is opaque to client and only parsed by RP. Fixes #244. This also lays some of the groundwork for adding a U2F attestation format. I will clean up the TPM attestation section in a separate commit.

Fixes #274

@rlin1

Refactor the attestation section to clean up exposition. Separated out signature verification (per format) from trust chaining (done at higher layer). Created a separate section for specifying key RP operations. Fixes #88. RP registration section defines binding of credentials to user accounts. Fixes #13. RP registration section also defines options in case of registering the same credential to different users. Fixes #12. Cleans up and completes defining the process for verifying assertions, which had already been largely done by @rlin1. Fixes #102. Completes drawing the distinction between assertion and attestation certificates. Fixes #118. Replace "client platform" with "client" in signature format section to avoid confusion. Fixes #209.

Removed the TPM 1.2 parts. Rounded out the section. Fixes #226. Also clarified what certifyInfo contains. Fixes #242.

Fixed small wording and markdown issues. This completes the changes to make attestation opaque to UAs, which fixes #286, fixes #287, and fixes #289. It also fixes #239 by removing the homegrown algorithm identifiers and specifying the algorithm explicitly in attestation data using JWK identifiers. It also fixes #240 by encoding keys in CBOR which specifies lengths of fields.

Fixes #214

…into vgb-u2f-attestation

Also clean up a few remaining instances of "WebAuthn Assertion" and sort the glossary alphabetically.

Clearly differentiate attestation statements and attestation objects.

vijaybh · 2017-01-11T20:55:05Z

@rlin1 - Looking at the latest changes, I wonder if we could put the CBOR field name back to just "attestation". This would be the only field with such a name, and it would save bytes on a structure that may be produced on an authenticator with limited capability. WDYT?

This is now the only use of "attestation" so we might as well save bytes in the authenticator.

equalsJeffH · 2017-01-12T16:26:06Z

@vijaybh wrote:

Looking at the latest changes, I wonder if we could put the CBOR field name back to just "attestation". This would be the only field with such a name, and it would save bytes on a structure that may be produced on an authenticator with limited capability. WDYT?

@rlin1 replied in email here:

First I think the spec is much better to read after those changes.
We call it attestation statement and the field containing it is also called
attestation statement.
Same with attestation object.
If the number of bytes is so important, we could abbreviate
attestationObject to "attstnObj" - that is even fewer bytes than the
original.

equalsJeffH · 2017-01-12T16:28:19Z

@vijaybh wrote:

Looking at the latest changes, I wonder if we could put the CBOR field name back to just "attestation". This would be the only field with such a name, and it would save bytes on a structure that may be produced on an authenticator with limited capability. WDYT?

@rlin1 replied in email here:

If the number of bytes is so important, we could abbreviate
attestationObject to "attstnObj"

I agree with Rolf @rlin1 and was about to myself suggest something akin to attstnObj for the same reasons.

equalsJeffH · 2017-01-12T22:29:13Z

a rendered textual diff of the spec in branch vgb-u2f-attestation at commit 0d0fcea from master at commit 6267cc1 is here: http://kingsmountain.com/doc/diff/diff-webauthn-vgb-u2f-attestation-0d0fcea--from--master-6267cc1.pdf

equalsJeffH · 2017-01-18T00:45:53Z

regarding these two prior comments -- #321 (comment) and #321 (comment) -- we ( @rlin1 and I ) and @vijaybh were referring to different things. @vijaybh was referring to the now-named attestation member of the CBOR-encoded attestation object value, see 0d0fcea. I.e., nevermind regarding that particular comment of mine.

vijaybh · 2017-01-18T16:24:38Z

I fiddled with the naming of fields inside the attestation structures to make them sort-of-consistent across formats. Do let me know if you have suggestions for improvement.

equalsJeffH

Overall these changes seem technically solid and appear to attain the goal of having the client not need to understand or parse the attestationObject.

I have various detail-level comments throughout and found some modest mistakes that I've attempted to correct (eg calling the {{#authenticator-signature-verification}} with ClientDataHash rather than ClientDataJason).

Also I offer a much simplified {{#requesting-a-new-credential}} section by leveraging the "verifying a signature" procedure rather than duplicating it.

I also suggest we use ABNF + CDDL to describe our various binary objects rather than the simple tables we are presently using -- it will be more precise for implementors and plus all the subfields are named such that we can directly refer to them in spec prose. E.g., see #318 (comment)

HTH,

=JeffH

note: I did not complete going thru each attestation statement format subsection at a detail level. on a quick skim they looked ok but should be reviewed in detail of course.

equalsJeffH · 2017-01-17T19:01:23Z

index.bs

 : Base64url encoding
 :: The term <dfn>Base64url Encoding</dfn> refers to the base64 encoding using the URL- and filename-safe character set defined
    in Section 5 of [[!RFC4648]], with all trailing '=' characters omitted (as permitted by Section 3.2) and without the
-    inclusion of any line breaks, whitespace, or other additional characters. This is the same encoding as used by JSON Web
-    Signature (JWS) [[RFC7515]].
+    inclusion of any line breaks, whitespace, or other additional characters.


I think the deleted sentence is useful info. Perhaps it ought to be delineated as a "Note:" rather than deleted.

It just seems random. The only places we mention base64url are related to encoding a challenge and a token binding ID in ClientData. Neither of those is in any way related to RFC 7515 at first sight. So why did we pick this RFC to call out as opposed to any other standard that uses base64url?

equalsJeffH · 2017-01-17T21:09:58Z

index.bs

-    See also <a>attestation format</a>, and <a>attestation type</a>.
+    including, for example: credential IDs, credential key pairs, signature counters, etc. <dfn>Attestation information</dfn> is
+    conveyed in <dfn>attestation objects</dfn>.
+    See also <a>attestation statement format</a>, and <a>attestation type</a>.



Will need to rewrite the last two sentences of the above Attestation definition depending on how the terms attestation data, attestation information, attestation object, and attestation statement shake out.

equalsJeffH · 2017-01-18T01:34:55Z

index.bs

@@ -386,14 +387,14 @@ This method takes the following parameters:
    preferred credential that it can.

 - The <dfn>attestationChallenge</dfn> parameter contains a challenge intended to be used for generating the attestation
-    statement of the newly created credential.
+    object of the newly created credential.


minor nit re "attestation object of the newly created credential": I would say "the newly created credential's attestation object"

equalsJeffH · 2017-01-19T02:05:10Z

index.bs


-The [RP] MAY take any of the below actions when verification of an attestation statement fails, according to its policy:
+: authData


given that the official-as-it-gets short name for the WG and the spec is "WebAuthn", and the confusion folks often have between authentication and authorization, I would name this "authnDat"

In fact, given the key-naming practice in the JSON Web Object community of using 3-char "claim names"/"header parameter names", I'd be willing to shorten this further, e.g., "and" (for AuthN Data).

equalsJeffH · 2017-01-19T02:09:07Z

index.bs

-9. Using the verification process for the above attestation format, validate that the attestation
-    {{AuthenticationAttestation/attestation}} is valid for the given {{AuthenticationAttestation/authenticatorData}},
-    {{AuthenticationAttestation/clientData}} and the above trust anchor.
+: fmt


+1 on shortening these CBOR keys

equalsJeffH · 2017-01-21T01:27:14Z

index.bs

-Attestation formats are identified by a string, called a <dfn>attestation format identifier</dfn>, chosen by the attestation
-format author.
+Attestation statement formats are identified by a string, called a <dfn>attestation format identifier</dfn>, chosen by the
+author of the attestation statement format.


s/<dfn>attestation format identifier</dfn>/<dfn>attestation statement format identifier</dfn>/

equalsJeffH · 2017-01-21T01:28:34Z

index.bs

-Attestation formats are identified by a string, called a <dfn>attestation format identifier</dfn>, chosen by the attestation
-format author.
+Attestation statement formats are identified by a string, called a <dfn>attestation format identifier</dfn>, chosen by the
+author of the attestation statement format.

 Attestation format identifiers SHOULD be registered per [[WebAuthn-Registries]] "Registries for Web Authentication (WebAuthn)".
 All registered attestation format identifiers are unique amongst themselves as a matter of course.


s/attestation format identifiers/attestation statement format identifiers/g -- i.e. update all occurances throughout spec

equalsJeffH · 2017-01-21T01:32:45Z

index.bs


+This is a WebAuthn optimized attestation statement format. It uses a very compact but still extensible encoding method. Encoding
+this format can even be implemented by <a>authenticators</a> with very limited resources (e.g., secure elements).


suggest: This encoding format is implementable by <a>authenticators</a> with limited resources (e.g., secure elements).

equalsJeffH · 2017-01-21T02:05:01Z

index.bs

-        The <dfn>signature</dfn> element contains the attestation signature.
-    </div>
+    : x5c
+    :: A definite-length array of byte strings. The elements of the array contain the attestation certificate and its


why does this need to be a definite-length? a CBOR decoder ought to transparently be able to handle both definite- and indefinite-length arrays at run time, yes? Note that CDDL does not make any prescription as to whether arrays or maps use the definite length or indefinite length encoding. see: https://tools.ietf.org/html/draft-greevenbosch-appsawg-cbor-cddl#section-3.2

equalsJeffH · 2017-01-21T02:15:03Z

index.bs

+    If |daaKey| is present, then the attestation type is DAA. In this case:
+    - Verify that |alg| is "ED256" or "ED512".
+    - Follow the procedure in [[#authenticator-signature-verification]] to verify that |sig| is a valid signature over the given
+        <a>authenticatorData</a> and <a>clientDataHash</a>  using DAA-Verify with |daaKey| (see [[!FIDOEcdaaAlgorithm]]).


in this and all the other verification procedure subsections below, need to s/clientDataHash/clientDataJSON/ because that is what is input to {{#authenticator-signature-verification}}

See above. Will refactor signature format section to make this consistent.

@equalsJeffH

Thanks to @equalsJeffH for the detailed review. Remaining items: Refactor signature format section, possibly rename fields for brevity, add CDDL/ABNF, fix U2F attestation issues.

equalsJeffH · 2017-02-08T17:45:46Z

New diffs:

just the diff of the "editorial revisions" commits added since the last set of diffs (above): i.e., most recent commit 7642dd6 diff'd from dc90eab:

http://kingsmountain.com/doc/diff/diff-webauthn-vgb-u2f-attestation-7642dd6--from--index-vgb-u2f-attestation-dc90eab.pdf

diff of most recent commit from current state of master branch:

http://kingsmountain.com/doc/diff/diff-webauthn-vgb-u2f-attestation-7642dd6--from--index-master-4751dac.pdf

@equalsJeffH

Thanks to @equalsJeffH for spotting this.

equalsJeffH

a couple comments on @vijaybh's in-flight work. there's also further comments in reply to Vijay that aren't "review comments" (am not sure how these two were captured as such)

equalsJeffH · 2017-02-09T00:21:42Z

index.bs

+    <tr>
+        <td>variable</td>
+        <td>
+            Credential public key encoded in CBOR format. This is a CBOR map comprising the following fields:


equalsJeffH · 2017-02-09T00:22:00Z

index.bs

+                [[!RFC7518]] section 3.1. Specifically, the following values are supported: "ES256", "ES384", "ES512", "RS256",
+                "RS384", "RS512", "PS256", "PS384" and "PS512".
+
+            : (public key fields)


Separate out verification of assertion and attestation signatures, removing redundant steps. Broke up signature format sectiona and moved the pieces to the appropriate places.

Fixes #318

equalsJeffH · 2017-02-13T17:18:11Z

new diff of the changes Vijay pushed last night Sun 12-Feb 1900-ish PT..

http://kingsmountain.com/doc/diff-webauthn-vgb-u2f-attestation-2ebdcfe--from--index-vgb-u2f-attestation-7642dd6.pdf

Consistent naming across types, stricter specification.

vijaybh · 2017-02-13T23:44:47Z

Merging based on in-person discussion at San Francisco f2f meeting.

…, fix TPM format, add U2F format (#321)

vijaybh and others added 12 commits January 7, 2017 13:58

Add markup to eliminate bikeshed warnings

42fa30f

Client sends RP ID, not its hash, to authenticators

5789b4b

Fixes #188

Make attestation opaque to client

6fb855b

Puts all attestation info into a CBOR object which is opaque to client and only parsed by RP. Fixes #244. This also lays some of the groundwork for adding a U2F attestation format. I will clean up the TPM attestation section in a separate commit.

Specify that clientDataJSON does not need to be canonicalized

26aea41

Fixes #274

Fix up TPM attestation format

bd1c9b1

Removed the TPM 1.2 parts. Rounded out the section. Fixes #226. Also clarified what certifyInfo contains. Fixes #242.

added missing reference FIDO-APPID

57203e7

Add U2F attestation format

317861f

Fixes #214

Add reference for U2F message formats

662a775

lex

2c136f2

Merge branch 'vgb-u2f-attestation' of https://github.com/w3c/webauthn …

bc436be

…into vgb-u2f-attestation

vijaybh requested review from equalsJeffH, jcjones, leshi and rlin1 January 11, 2017 19:04

equalsJeffH added the type:technical label Jan 11, 2017

equalsJeffH added this to the WD-04 milestone Jan 11, 2017

vijaybh added 2 commits January 11, 2017 11:42

Merge from master

e0cb83c

Also clean up a few remaining instances of "WebAuthn Assertion" and sort the glossary alphabetically.

Implement Rolf's naming suggestions

44fec25

Clearly differentiate attestation statements and attestation objects.

Rename attestationStatement back to just attestation

0d0fcea

This is now the only use of "attestation" so we might as well save bytes in the authenticator.

Tweak wording and naming of CBOR fields

8daea63

vijaybh and others added 2 commits January 18, 2017 09:03

Editorial tweaks for wording consistency

4861b65

first draft of attestation structure figure added

dc90eab

rlin1 and others added 2 commits January 19, 2017 10:41

moved structures names to avoid confusion

a65804b

resized attstn structs figure, added fig to section #cred-attestation

4407204

equalsJeffH reviewed Jan 21, 2017

View reviewed changes

equalsJeffH mentioned this pull request Jan 23, 2017

Consider using CDDL (CBOR data definition language) to define CBOR-encoded data structures #318

Closed

equalsJeffH assigned vijaybh Feb 1, 2017

vijaybh added 3 commits February 6, 2017 15:01

Fix bikeshed linking error

1088d3a

Merge branch 'master' into vgb-u2f-attestation

2aee888

Editorial revisions

7642dd6

Thanks to @equalsJeffH for the detailed review. Remaining items: Refactor signature format section, possibly rename fields for brevity, add CDDL/ABNF, fix U2F attestation issues.

Fix interface/member confusion

c9b20a5

Thanks to @equalsJeffH for spotting this.

equalsJeffH reviewed Feb 9, 2017

View reviewed changes

vijaybh added 3 commits February 10, 2017 10:35

Fix linking error

feb7d7b

Clean up exposition

835e72c

Separate out verification of assertion and attestation signatures, removing redundant steps. Broke up signature format sectiona and moved the pieces to the appropriate places.

Use CDDL to define attestation and extension structures

2ebdcfe

Fixes #318

fix CDDL marker for packed attestation

ce9c187

bifurcation mentioned this pull request Feb 13, 2017

Expose standard authenticator fields in JS objects, vs. binary #337

Closed

Clean up attestation CDDL

50bb26e

Consistent naming across types, stricter specification.

vijaybh merged commit 85db884 into master Feb 13, 2017

vijaybh deleted the vgb-u2f-attestation branch February 13, 2017 23:46

WebAuthnBot pushed a commit that referenced this pull request Feb 13, 2017

Built by Travis-CI: 85db884 Clean up attestation, abstract it from UA…

81e586f

…, fix TPM format, add U2F format (#321)

vijaybh mentioned this pull request Feb 14, 2017

Should use "RS1" instead of "RSA1_5" #304

Closed

This was referenced Feb 14, 2017

Clarify names for crypto algs, e.g. RS256 #123

Closed

detail-level issues in signature format, attestation format(s), attestation statement #233

Closed

kpaulh mentioned this pull request Feb 14, 2017

Should WebAuthnAttestation attributes have constructors? #313

Closed

This was referenced Mar 1, 2017

Credential CBOR #366

Closed

Creation of ScopedCredentialInfo in makeCredential should probably be more explicit #277

Closed

AngeloKai mentioned this pull request Sep 15, 2017

include public key in result from create() #557

Closed

equalsJeffH mentioned this pull request Mar 28, 2018

AttestationResponse vs AssertionResponse #854

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up attestation, abstract it from UA, fix TPM format, add U2F format #321

Clean up attestation, abstract it from UA, fix TPM format, add U2F format #321

vijaybh commented Jan 11, 2017

vijaybh commented Jan 11, 2017

equalsJeffH commented Jan 12, 2017

equalsJeffH commented Jan 12, 2017

equalsJeffH commented Jan 12, 2017

equalsJeffH commented Jan 18, 2017 •

edited

Loading

vijaybh commented Jan 18, 2017

equalsJeffH left a comment •

edited

Loading

equalsJeffH Jan 17, 2017

vijaybh Feb 7, 2017

equalsJeffH Feb 8, 2017

equalsJeffH Jan 17, 2017

vijaybh Feb 8, 2017

equalsJeffH Jan 18, 2017

equalsJeffH Jan 19, 2017

equalsJeffH Jan 19, 2017

equalsJeffH Jan 21, 2017

equalsJeffH Jan 21, 2017

equalsJeffH Jan 21, 2017

equalsJeffH Jan 21, 2017

vijaybh Feb 8, 2017

equalsJeffH Jan 21, 2017

vijaybh Feb 8, 2017

equalsJeffH commented Feb 8, 2017

equalsJeffH left a comment

equalsJeffH Feb 9, 2017

equalsJeffH Feb 9, 2017

equalsJeffH commented Feb 13, 2017

vijaybh commented Feb 13, 2017


		The [RP] MAY take any of the below actions when verification of an attestation statement fails, according to its policy:
		: authData


		This is a WebAuthn optimized attestation statement format. It uses a very compact but still extensible encoding method. Encoding
		this format can even be implemented by <a>authenticators</a> with very limited resources (e.g., secure elements).

Clean up attestation, abstract it from UA, fix TPM format, add U2F format #321

Clean up attestation, abstract it from UA, fix TPM format, add U2F format #321

Conversation

vijaybh commented Jan 11, 2017

vijaybh commented Jan 11, 2017

equalsJeffH commented Jan 12, 2017

equalsJeffH commented Jan 12, 2017

equalsJeffH commented Jan 12, 2017

equalsJeffH commented Jan 18, 2017 • edited Loading

vijaybh commented Jan 18, 2017

equalsJeffH left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

equalsJeffH commented Feb 8, 2017

equalsJeffH left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

equalsJeffH commented Feb 13, 2017

vijaybh commented Feb 13, 2017

equalsJeffH commented Jan 18, 2017 •

edited

Loading

equalsJeffH left a comment •

edited

Loading