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

clarify conveyance of attested public key #94

Closed
equalsJeffH opened this issue May 12, 2016 · 5 comments
Closed

clarify conveyance of attested public key #94

equalsJeffH opened this issue May 12, 2016 · 5 comments

Comments

@equalsJeffH
Copy link
Contributor

equalsJeffH commented May 12, 2016

the makeCredential() promise returns a ScopedCredentialInfo..

interface ScopedCredentialInfo {
    readonly attribute Credential           credential;
    readonly attribute any                  publicKey;
    readonly attribute WebAuthnAttestation  attestation;
};

..wherein the publicKey attribute is intended to convey the "attested public key", which is the user's newly-generated public key to be registered with the webauthn relying party (WRP).

However, in the "Packed Attestation (type="packed")" section, there's this this text..

The rawData object contains the attested public key and the clientDataHash. 
See §4.3.2.1.1 Attestation rawData for details.

..which is clearly saying the attested public key is being conveyed in the rawData object.

Perhaps a hash of the attested public key ought to be conveyed in the rawData object, and it be made clear that the plaintext attested public key be conveyed in ScopedCredentials.publicKey.

also note that our terminology for the so-called attested public key needs to be normailzed, see #79 .

@vijaybh
Copy link
Contributor

vijaybh commented Sep 20, 2016

Should have been addressed by #161

@equalsJeffH
Copy link
Contributor Author

it still looks to me that we are conveying the newly-generated, attested, user authentication (uauth) public key in two places in ScopedCredentialInfo:

  • in ScopedCredentialInfo.publicKey
  • also in ScopedCredentialInfo.attestation.authenticatorData."Attestation data"."public key"
    ..in the case of packed attestation format, at least.

ScopedCredentialInfo.publicKey is mentioned only in section {#iface-credentialInfo} where it is defined.

section {#sec-authenticator-data} notes that Attestation data is optional via the "(if present)" qualification.

Is the purpose ofScopedCredentialInfo.publicKey to convey an unattested uauth public key? If so, it is not discussed AFAICT.

@vijaybh
Copy link
Contributor

vijaybh commented Oct 17, 2016

Attestation data is not optional - it is never present on getAssertion, and always present on makeCredential. So you are right, the public key is in two places. Options:

  • Add something to the verification procedure for attestation to say that the two must be verified to match.
  • Remove the public key from the ScopedCredentialInfo.
    Your thoughts? The latter seems simpler.

@equalsJeffH
Copy link
Contributor Author

@vijaybh wrote:

Attestation data is not optional

that's what I thought, tho the "(if present)" notation in the table in {#sec-authenticator-data} had me wondering.

it is never present on getAssertion, and always present on makeCredential

that's something we need to clarify.

So you are right, the public key is in two places. Options:

  • Add something to the verification procedure for attestation to say that the two must be verified to match.
  • Remove the public key from the ScopedCredentialInfo.
    Your thoughts? The latter seems simpler.

Agreed, the latter, also in order to reduce conveyed octets.

@equalsJeffH
Copy link
Contributor Author

equalsJeffH commented Oct 18, 2016

See PR #235 for proposed resolution.

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

3 participants