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

Attestation format identifiers lack formal definition and matching rules #127

Closed
equalsJeffH opened this issue Jun 10, 2016 · 9 comments · Fixed by #192
Closed

Attestation format identifiers lack formal definition and matching rules #127

equalsJeffH opened this issue Jun 10, 2016 · 9 comments · Fixed by #192
Assignees
Milestone

Comments

@equalsJeffH
Copy link
Contributor

equalsJeffH commented Jun 10, 2016

Attestation format identifiers lack formal definition and matching rules.

suggested approach to add to spec:

WebAuthn attestation format identifiers are strings, and are
allocated on a first-come-first-served basis. They MUST
not be longer than 32 octets and MUST consist only of 
printable USASCII characters, i.e., VCHAR as defined 
in [RFC5234]. Implementations MUST match WebAuthn 
attestation type identifiers in a case-insensitive fashion.
@jcjones
Copy link
Contributor

jcjones commented Jul 11, 2016

Saying the identifiers are allocated implies, to me, a registry. Current language is that identifiers should aim to be globally unique. It seems to me we could give your formal definition and matching rules, drop the note about allocation, and instead have something like:

Extensions are identified by a string, chosen by the extension author. They MUST
not be longer than 32 octets and MUST consist only of
printable USASCII characters, i.e., VCHAR as defined
in [RFC5234]. Implementations MUST match WebAuthn
attestation type identifiers in a case-insensitive fashion.

Extension identifiers should aim to be globally unique,
e.g., by using reverse domain-name of the defining entity such as com.example.webauthn.myextension.

@vijaybh
Copy link
Contributor

vijaybh commented Jul 11, 2016

Agree, with the modification that perhaps we should move away from recommending dot notation for things that are not hierarchies.

@jcjones
Copy link
Contributor

jcjones commented Jul 11, 2016

Fair; I was working from e22cd4a prior to seeing your suggestion of camelCasing. I'll propose an alternate shortly.

@jcjones
Copy link
Contributor

jcjones commented Jul 12, 2016

How about:

(Modifying the first paragraph of Extension identifiers)

Extensions are identified by a string, chosen by the extension author. They MUST not be longer than 32 octets and MUST consist only of printable USASCII characters, i.e., VCHAR as defined in [RFC5234]. Implementations MUST match WebAuthn attestation type identifiers in a case-insensitive fashion. Extension identifiers should aim to be globally unique,
e.g., by including the defining entity such as myCompanyExtension.

@vijaybh
Copy link
Contributor

vijaybh commented Jul 12, 2016

Noticed you omitted JeffH's suggested text that said people SHOULD register extensions in [I-D. hodges-webauthn-registries]. Was that accidental or intentional?

Jeff's suggested text is in https://lists.w3.org/Archives/Public/public-webauthn/2016Jul/0070.html

@jcjones
Copy link
Contributor

jcjones commented Jul 12, 2016

Accidental! Accidental! I seem to have missed that message.

I like @equalsJeffH's suggested text. 👍

vijaybh added a commit that referenced this issue Aug 10, 2016
- Standardize the authenticator data so all formats have equal support
for AAGUID, extensions, etc. This also removes a lot of duplication
across structures.
- Add structure to the definition of attestation formats. Fixes #126.
Fixes #127.
- Simplify the naming of the attestation types to make it easier to
understand
- Clean up mentions of GUID. Fixes #148, fixes #149, fixes #150.
- Clarifies how to use self attestation. Fixes #115.
- More detailed pointers on how to generate a TPM attestation.
- Simplify Android attestation to remove fields that were not really
attested by authenticator and were therefore creating a false sense of
assurance.
@equalsJeffH
Copy link
Contributor Author

updating terminology from "attestation type" to "attestation format"

@equalsJeffH equalsJeffH changed the title Attestation type identifiers lack formal definition and matching rules Attestation format identifiers lack formal definition and matching rules Sep 2, 2016
@equalsJeffH equalsJeffH modified the milestones: WD-02, CR Sep 2, 2016
@equalsJeffH
Copy link
Contributor Author

see also #155, adding this issue to MS WD-02.

@equalsJeffH
Copy link
Contributor Author

@vijaybh wrote:

Noticed you omitted JeffH's suggested text that said people SHOULD register extensions in [I-D. hodges-webauthn-registries]. Was that accidental or intentional?

Jeff's suggested text is in https://lists.w3.org/Archives/Public/public-webauthn/2016Jul/0070.html

the suggested text and rationale from the email msg cited above is below (with updated terminology and pointer to current registries I-D)...

On 7/11/16, 2:59 PM, "J.C.Jones via GitHub" sysbot+gh@w3.org wrote:

Saying the identifiers are allocated implies, to me, a registry.

apologies, I didn't fully explain my rationale in this issue.

yes, I think we do wish to have an IANA registry for attestation types,
see..

draft-hodges-webauthn-registries
https://raw.githubusercontent.com/w3c/webauthn/master/draft-hodges-webauthn-registries-00b.txt

..because it will be a useful tool for the ecosystem, e.g., by gathering
publicly-specified attestation types, and pointers to their
specifications, in a well-known place.

That said, we should also provide guidance for those who do not wish to
register their attestation format identifier(s) -- i.e., we should recognize
that not everyone will wish to publicly specify their attestation formats
and specs (think proprietary enterprise-specific use cases, say).

so I propose we make use of the registry a SHOULD, and un-registered
attstn format names SHOULD use reverse domain-name naming. [perhaps the
latter should be a MUST? however, a SHOULD recognizes that there's no
effective enforcement...]

thus:

WebAuthn attestation format identifiers are strings, chosen
by the attestation format developer. They SHOULD be registered
per [I-D. hodges-webauthn-registries] "Registries for Web 
Authentication (WebAuthn)".
Unregistered attestation format identifiers SHOULD use
reverse domain-name naming, using a domain name registered by
the attestation type developer. All attestation format identifiers MUST
not be longer than 32 octets and MUST consist only of
printable USASCII characters, i.e., VCHAR as defined
in [RFC5234] (note: this means attestation format identifiers based on
domain names MUST incorporate only LDH Labels [RFC5890]).
Implementations MUST match WebAuthn
attestation format identifiers in a case-insensitive fashion.

@equalsJeffH equalsJeffH self-assigned this Sep 2, 2016
vijaybh pushed a commit that referenced this issue Sep 7, 2016
* ref IANA registries initial cut.  fixes #127, #129, #155.

* remove 'Level:' from metadata - fix build?

* fix build: was missing portions of biblio entry, restore 'Level:'

* restate attstn format idents and adjust section titles

* editorial fixups for #155 et al
vijaybh added a commit that referenced this issue Sep 20, 2016
* Replace facet with origin

Facet was a holdover from the old FIDO specs and origin is the term used
everywhere in this spec (as well as in recent FIDO specs)

* Clean up explanation of computing clientDataHash and passing to authenticator

Fixes #153

* Remove text from authnsel extension to avoid chicken-and-egg problem

Fixes #152

* Clean up attestation

- Standardize the authenticator data so all formats have equal support
for AAGUID, extensions, etc. This also removes a lot of duplication
across structures.
- Add structure to the definition of attestation formats. Fixes #126.
Fixes #127.
- Simplify the naming of the attestation types to make it easier to
understand
- Clean up mentions of GUID. Fixes #148, fixes #149, fixes #150.
- Clarifies how to use self attestation. Fixes #115.
- More detailed pointers on how to generate a TPM attestation.
- Simplify Android attestation to remove fields that were not really
attested by authenticator and were therefore creating a false sense of
assurance.

* Fix typo (thanks Travis!)

* Incorporated feedback from @rlin1

Also cleaned up wording and naming for consistency.

Added Android N attestation format. Fixes #103.

Changed name for SafetyNet attestation format. Fixes #128.

* Clarify that attestation is not optional

Fixes #86

Also clarify that at least self attestation must be used. Fixes #115
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants