Requirements to finalise v0.3

[Firstyear]

What would be required to finalise v0.3? Are there any other breaking api changes we want to make?

[Firstyear]

cc @ericmarkmartin @agrinman

[madwizard-thomas]

Some thoughts:

• What do you think about having separate structures containing the attestation statement data (e.g. PackedAttestation, TpmAttestation etc.) so the data they contain can be accessed later (for example in the trust validation). There could be a Verify trait implemented on all of them with a method verify(authenticatorData, policy) instead of the separate verify_*_attestation methods?
• UserHandle has a maximum of 64 bytes in the spec, that might warrant a specific type for it.

[Firstyear]

I think the userHandle comment is good, we should check and limit that I suspect.

I'm not sure what you mean by the attestation statement types? We already return the attested credential data, but I'm assuming you mean we should return the ACD in a more user-friendly format for callers to be able to inspect?

[madwizard-thomas]

I'm not sure what you mean by the attestation statement types? We already return the attested credential data, but I'm assuming you mean we should return the ACD in a more user-friendly format for callers to be able to inspect?

Yes I meant separating the parsing and verification, so the parsed data is available for later. Although that may be of limited use, trust validation mainly uses the attestation type and path. The main reason I mentioned it is the different verify functions stood out and a Verify trait seemed more idiomatic to me.

Some other minor things I came across is COSEContentType: this is called COSE algorithm in WebAuthn and COSE specs. Also ECDSA_SHA256/ECDSA_SHA384/ECDSA_SHA512 are called ES256, ES384 and ES512 in the specs.

Should I create separate issues for these kind of things?

Btw which WebAuthn spec is the library aiming for? v1 or 2? Or both?

[Firstyear]

I'm not sure what you mean by the attestation statement types? We already return the attested credential data, but I'm assuming you mean we should return the ACD in a more user-friendly format for callers to be able to inspect?

Yes I meant separating the parsing and verification, so the parsed data is available for later. Although that may be of limited use, trust validation mainly uses the attestation type and path. The main reason I mentioned it is the different verify functions stood out and a Verify trait seemed more idiomatic to me.

I try and avoid traits as much as possible, unless they are really required because they tend to add more complexity than the resolve, so I'd want a pretty strong reason for a "verify trait". I think it may better to expose the acd in an enum given what type it is internally. Can you open an issue for this too?

Some other minor things I came across is COSEContentType: this is called COSE algorithm in WebAuthn and COSE specs. Also ECDSA_SHA256/ECDSA_SHA384/ECDSA_SHA512 are called ES256, ES384 and ES512 in the specs.

Should I create separate issues for these kind of things?

Yep, please do.

Btw which WebAuthn spec is the library aiming for? v1 or 2? Or both?

v2

[Firstyear]

@madwizard-thomas - Going to your suggestion about returning more data from a registration, looking at src/attestation.rs we already have AttestationType which we provide as the attest_result to policy_verify trust which has the certificates and chains associated. Do you think or see value in this changing so that policy_verify_trust takes &AttestationType, and we then return this AttestationType in addition to AuthenticatorData, which can be persisted for later inspection. This would probably allow us to store more metadata from the AttestationType paths like the aaguid if present, or in the case of the TPM, details such as qualifiedSigner, clockInfo and firmwareVersion.

An alternate is we could have AuthenticatorData parse and contain the AttestationType if present.

Thoughts?

[Firstyear]

@madwizard-thomas ping re the ideas above :) I'd be keen to implement this in the next few weeks so we get get this version out of alpha.

[madwizard-thomas]

I'm not sure I follow exactly, you mean passing AuthenticatorData to policy_verify_trust? Or to return it from the registration function?

The AttestationType enum currently contains the credential as well but if this enum is meant to represent the result of the attestation verification function (chapter 8 of the spec) it would be only the attestation type + trust path as related data (always a x509 chain or an empty trust path according to v2 of the spec). From https://www.w3.org/TR/webauthn-2/#verification-procedure : the inputs are attStmt, authenticatorData and clientDataHash. Result from the procedure is either an error, or attestation type + trust path.

You can extract a public key credential from the authenticatorData without touching the attestation statement so this seems like a separate step to me. The Credential::new(..) part is currently duplicated in all the verify_*_attestation functions.

Then the result of the registration before any policy checks is a credential (id + public key, you could argue if the counter is part of the credential or not, it is the only thing that changes afterwards) and an attestation verification result (attestation type + trust path, without specifying anything about whether this trust path is actually trusted).

As for the trust policy check, once you'll start using metadata (services) you will need more information than just the trust path. FIDO2 metadata is based on the AAGUID for example, you will need the AuthenticatorData for that. But legacy U2F yubikeys for example use metadata based on vendor specific x509 extensions in the attestation certificate. I don't think you will need the parsed statement in most cases unless you'll want to base your policy on TPM data for example like you mentioned, or the android attestations have some additional flags you might use as well (like if the hardware is genuine). In those cases there is no way to easily parse the statements. You could augment the AttestationFormat enum with the parsed data per type. But most users of the library probably don't care about this. Even attestation verification via metadata services is pretty advanced but if you want to pass conformance you'll need this. You will also run into stuff like needing to download (and probably cache) metadata from metadata services and (sadly) download/verify CRLs. You might want to consider using async for that as well.

Imo the trust policy check is a two step process:

• Determine the relevant metadata. Usually based on the AAGUID but might use other stuff like the attestation certificate. The metadata specifies the trust anchors (x509 root certificates) to be used but also might indicate other parameters like supported attestation types or a compromised device status. Having no metadata may also be a valid outcome. The source of metadata could be a metadata service, a directory with json metadata statements (both are required for conformance), or built-in metadata source (e.g. older U2F-only yubikey metadata or Apple's root certificate for apple devices).
• Given the attestation, credential, etc. and the metadata from the previous step: determine whether to trust the registration. This could be a callback the user has to implement but given the complexity I would prefer a set of config options like:
  • Allow none/self attestation?
  • Reject compromised devices?
  • Is metadata a requirement or best effort? A callback can still be an option for advanced stuff.

Sorry this has become a bit of a rambling reply :), just writing out my thoughts.

[Firstyear]

I'm not sure I follow exactly, you mean passing AuthenticatorData to policy_verify_trust? Or to return it from the registration function?

Both in a way. policy verify trust lets people make their own extended decision - we can also return it from registration in a parsed format so that it can be persisted for future auditing IE identifying likely compromised devices. (Think back to yubikey 4 with the infineon rsa issue).

The AttestationType enum currently contains the credential as well but if this enum is meant to represent the result of the attestation verification function (chapter 8 of the spec) it would be only the attestation type + trust path as related data (always a x509 chain or an empty trust path according to v2 of the spec). From https://www.w3.org/TR/webauthn-2/#verification-procedure : the inputs are attStmt, authenticatorData and clientDataHash. Result from the procedure is either an error, or attestation type + trust path.

You can extract a public key credential from the authenticatorData without touching the attestation statement so this seems like a separate step to me. The Credential::new(..) part is currently duplicated in all the verify_*_attestation functions.

Then the result of the registration before any policy checks is a credential (id + public key, you could argue if the counter is part of the credential or not, it is the only thing that changes afterwards) and an attestation verification result (attestation type + trust path, without specifying anything about whether this trust path is actually trusted).

As for the trust policy check, once you'll start using metadata (services) you will need more information than just the trust path. FIDO2 metadata is based on the AAGUID for example, you will need the AuthenticatorData for that. But legacy U2F yubikeys for example use metadata based on vendor specific x509 extensions in the attestation certificate. I don't think you will need the parsed statement in most cases unless you'll want to base your policy on TPM data for example like you mentioned, or the android attestations have some additional flags you might use as well (like if the hardware is genuine). In those cases there is no way to easily parse the statements. You could augment the AttestationFormat enum with the parsed data per type. But most users of the library probably don't care about this. Even attestation verification via metadata services is pretty advanced but if you want to pass conformance you'll need this. You will also run into stuff like needing to download (and probably cache) metadata from metadata services and (sadly) download/verify CRLs. You might want to consider using async for that as well.

I'm against adding async here because that's a background behaviour that I think people "wont expect". Some pyeople may be surprised by that, so I'd rather have it so we can "bake in" some metadata, and give a way during the builder to allow it to be downloaded or provided from the correct locations. I think that may be a better approach.

Imo the trust policy check is a two step process:

• Determine the relevant metadata. Usually based on the AAGUID but might use other stuff like the attestation certificate. The metadata specifies the trust anchors (x509 root certificates) to be used but also might indicate other parameters like supported attestation types or a compromised device status. Having no metadata may also be a valid outcome. The source of metadata could be a metadata service, a directory with json metadata statements (both are required for conformance), or built-in metadata source (e.g. older U2F-only yubikey metadata or Apple's root certificate for apple devices).

• Given the attestation, credential, etc. and the metadata from the previous step: determine whether to trust the registration. This could be a callback the user has to implement but given the complexity I would prefer a set of config options like:

  • Allow none/self attestation?
  • Reject compromised devices?
  • Is metadata a requirement or best effort? A callback can still be an option for advanced stuff.

Sorry this has become a bit of a rambling reply :), just writing out my thoughts.

Well, the way we could do that as a config option is ship a default function that conducts that logic in policy_verify_trust, and then the user can extend or over-ride it for themself.

Realistically as you said, 99% of deployments won't actually care about attestation, so this is all "pointless" for them. The 1% that do care, care for a reason, and they are the ones who will likely be wanting to roll out their own policy-verify-trust function.

Still, I think right now I'm going to extend what metadat we expose to the registration result and the policy_verify_trust, and then we can decide how we feel abotu 3.0

[Firstyear]

@ericmarkmartin @agrinman @madwizard-thomas I would like to make this release in the coming weeks. Any final objections? Worst case we can break API to v0.4.

[madwizard-thomas]

Seems fine to me!

[ericmarkmartin]

LGTM