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

Client processing steps are not aligned with authenticator model #153

Closed
vijaybh opened this issue Jul 28, 2016 · 0 comments
Closed

Client processing steps are not aligned with authenticator model #153

vijaybh opened this issue Jul 28, 2016 · 0 comments

Comments

@vijaybh
Copy link
Contributor

vijaybh commented Jul 28, 2016

The conceptual model for the client-authenticator interface is that the authenticator neither sees nor cares about the fields in clientData, receiving them only in the form of clientDataHash. However our exposition does not correctly reflect this. The processing steps for makeCredential and getAssertion pass different things to the authenticator (and don't even pass clientDataHash) and the ClientData structure itself is only defined in the authenticator model.

To clean this up, the ClientData definition needs to move to the API definition section, and both makeCredential and getAssertion should compute and pass clientDataHash to the authenticator methods. Also the authenticator methods should not receive any of the inidividual fields of ClientData as parameters.

@equalsJeffH equalsJeffH added this to the CR milestone Aug 29, 2016
vijaybh pushed a commit that referenced this issue Sep 1, 2016
#169)

* Adding the User Verification Mode extension as per discussions in the WG weekly metings. After a lot of thought I retained the sub-array instead of converting the sub-items in the extension to a map. This was primarily to avoid having to define string based keys that increases the size of the extension. Given that the array is well defined I think it keeps it easier to process in software and extendable for future additions as well.

* Cleaned up exposition of processing rules (#154)

* 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

* Address comments from @equalsJeffH

* Update index.bs with proposed location extension (#157)

* Update index.bs

* Update index.bs

* Update index.bs

* Update index.bs

* Adding the User Verification Mode extension as per discussions in the WG weekly metings. After a lot of thought I retained the sub-array instead of converting the sub-items in the extension to a map. This was primarily to avoid having to define string based keys that increases the size of the extension. Given that the array is well defined I think it keeps it easier to process in software and extendable for future additions as well.

* fixing another merge issue with uvm extension being repeated following upstream merge
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

No branches or pull requests

2 participants