w3c / webauthn

Web Authentication: An API for accessing Public Key Credentials
https://w3c.github.io/webauthn/
Other
1.14k stars 166 forks source link

TPM attestation verification steps inconsistent with FIDO conformance testing tool #1925

Open sbweeden opened 11 months ago

sbweeden commented 11 months ago

Proposed Change

Note: This may end up resulting in a WebAuthn spec change (I think it should - read on), or in a close with feedback to the FIDO alliance about the validity of this test in the conformance testing tool, but the intent of opening this issue is to seek a point of view and clarification from experts in the field of TPM attestation for FIDO, then get the spec aligned with our decision.

There is a very confusing part of the TPM specifications related to which hash algorithm to use when computing the digest of the pubArea for verification of the attested name of the certInfo structure. Adding to this confusion is a completely different interpretation between the way WebAuthn is currently written, and the implementation of a TPM attestation test in the FIDO conformance test tool.

Currently the WebAuthn TPM attestation verification steps in section 8.3 include:

Verify that attested contains a TPMS_CERTIFY_INFO structure as specified in [[TPMv2-Part2]](https://w3c.github.io/webauthn/#biblio-tpmv2-part2) section 10.12.3, whose name field contains a valid Name for pubArea, as computed using the algorithm in the nameAlg field of pubArea using the procedure specified in [[TPMv2-Part1]](https://w3c.github.io/webauthn/#biblio-tpmv2-part1) section 16.

I draw attention to the words: using the algorithm in the nameAlg field of pubArea

Some sources, including these instructions documented by @herrjemand indicate that the hash algorithm does not come from the nameAlg field of pubArea, but instead comes from the TPM_ALG_ID portion of the attested name in the TPMS_CERTIFY_INFO structure. To me this makes complete sense - why wouldn't you co-locate the hash algorithm that was used alongside the hash digest?! Its not logical at all why the hash algorithm to use in the attested name within the TPMS_CERTIFY_INFO structure would come from the nameAlg in pubArea (as currently written in WebAuthn).

In practice (i.e. in Windows TPM attestations observed in the field) these two hash algorithm identifiers are typically set to the same hash algorithm, which means regardless of which interpretation you use, verification of real-world TPM attestations works.

The FIDO conformance test tool however contains a specific test case where these algorithm identifiers are set to different values, to ensure that RPs use the algorithm identifier from the attested name in certInfo. If an RP strictly follows the process as currently defined in the WebAuthn specification, then that test case will fail.

Adding to the confusion is the procedure specified in [TPMv2-Part1] section 16, from which I draw two observations: image

  1. This seems fairly clear - use the hash algorithm in the name structure itself, which is consistent with the instructions from the aforementioned blog article and the FIDO conformance tool test.
  2. This intimates that the nameAlg comes from the nameAlg parameter in the publicArea object. I actually think if the word in was changed to the word of here in the TPM spec, then things wouldn't be so ambiguous.

So which is correct?

If using the nameAlg from pubArea is correct, then both the verification information from @herrjemand in his blog article is wrong, as is the validity of that specific FIDO conformance test, as is the first observation from the screenshot of section 16 of the TPM spec. Further, its unclear what an RP should do if the _TPM_ALGID in the attested name is different from that in pubArea because it would make the former redundant.

If using the algorithm identifier from the _TPM_ALGID in the attested name in certInfo is correct, then the WebAuthn specification should be re-worded, for example:

Verify that attested contains a TPMS_CERTIFY_INFO structure as specified in [TPMv2-Part2] section 10.12.3, whose name field contains a valid Name for pubArea, as computed using the procedure specified in [TPMv2-Part1] section 16. Note that the hash algorithm is included within the attested name field of the TPMS_CERTIFY_INFO structure.

Personally I believe it should be the latter, and am happy to pull together a PR with this change in it. It makes no sense to not use the hash algorithm identifier that is co-located in the attested name within the TPMS_CERTIFY_INFO structure.

Firstyear commented 11 months ago

We should follow what is in the TPM specification since that is after all, how the signatures are created. We can't just expect every TPM to change it's behaviour for webauthn.

I think this is a good change, and well done to spot it.

emlun commented 11 months ago

Hm. I'm certainly not an expert on the TPM specs, but my interpretation of the current state of the WebAuthn spec is:

Thus, if my reading is correct that the verifier is supposed to check the recomputed TPM2B_NAME for binary equality with the embedded TPM2B_NAME, it seems to me that if the two hash IDs are different, then the attestation as a whole is invalid anyway.

I think there's support for this interpretation if you look at Trusted Platform Module Library Part 3: Commands §18.2 TPM2_Certify (referenced in WebAuthn in the signing procedure of the TPM attestation format): the TPM2_Certify response structure only contains a TPM2B_ATTEST structure, that is certInfo; the pubArea object is not returned by the signing command. In that light - if you consider the TPM2B_NAME as a whole as an opaque value, not just the digest part of it - it makes more sense to me that the verifier should use the hash ID from pubArea.nameAlg and then verify that the recomputed TPM2B_NAME structure equals the one generated by the issuer.

If the above is correct, then @herrjemand's article is also correct in practice, because the algorithm presented there does also verify that the two hash IDs are equal. But if the FIDO conformance tool presents a test case with a TPM attestation with two different hash IDs and expects that to be accepted, then it seems to me like that test case is incorrect - it should instead expect the test case to be rejected.

sbweeden commented 11 months ago

Personally, I'm fine with either outcome, but do want to clarify the correct approach so that both the WebAuthn spec is accurate (allowing RPs to know exactly what to expect), and so that if necessary I can go back to the FIDO conformance folks and ask for the current test case to be changed.

I realise the FIDO conformance test tool code is not public, but can say from looking at historical checkins in that code that this test case was explicitly changed in a code commit from an "expected fail" to an "expected pass" some 5 years ago (Aug 14, 2018), so at least at some point a determination was made that these algorithm identifiers can be different.

sbweeden commented 11 months ago

From call on 2023-07-26, @akshayku agreed to seek clarification. Adding him as asignee.

mwiseman-byid commented 3 months ago

I'm authoring the TPM2 Attestation for CSRs in IETF LAMPS. That work overlaps this and if we are open to add the proposed CSR structure (which is essentially what WebAuthn is doing here -- proving to a CA that a key is "valid") as a consideration for adoption of a "common" structure and method would be an advantage for all. That's a future discussion however, for now I've provided some higher-level explanation for how to attest to and verify a TPM2 key in the IETF Appendix that will provide some understanding and dispel some confusion above. I'm in the process of writing some sample code/scripts to demonstrate this for the IETF Hackathon. I'll post a URL to those when sufficiently done.

IETF LAMPS CSR: https://github.com/lamps-wg/csr-attestation/tree/main The explanation for how to construct the material (which does provide insight into how to verify) is in this Appendix: https://github.com/lamps-wg/csr-attestation/blob/main/draft-ietf-lamps-csr-attestation.md#introduction-to-tpm2-concepts

I'll be at IETF 119 if anyone is there and wants to discuss this let me know and we can meet up. If not, I can participate in any discussion necessary when I return.

sbweeden commented 3 months ago

@mwiseman-byid - I'm not able to draw any new conclusion from the references above as to resolving the actual issue that was raised and whether or not the associated proposed PR is accurate. Can you please provide a disposition on the issue itself?

yackermann commented 3 months ago

@sbweeden Sorry just realised that I was mentioned here:

The FIDO conformance test tool however contains a specific test case where these algorithm identifiers are set to different values, to ensure that RPs use the algorithm identifier from the attested name in certInfo. If an RP strictly follows the process as currently defined in the WebAuthn specification, then that test case will fail.

That came from real world issue, where during the testing we had examples of TPM attestation where nameAlg did not match the other nameAlg.

Here is the direct comment from that time:

    /* 
        So the name is concatenation of nameAlg[2byte] and hash structure[n-bytes].

        The confusion comes from the fact TPMS_CERTIFY_INFO contains name field that contains name of the TPMT_PUBLIC. But in the same time TPMT_PUBLIC contain nameAlg field that contains algorithm identifier for calculating authPolicy. There two both use nameAlg, but they can be different.

        For example:
        TPMT_PUBLIC.nameAlg = SHA-1;
        TPMT_PUBLIC.authPolicy = hashTPMT_PUBLIC.nameAlg

        nameAlg = SHA-256
        TPMS_CERTIFY_INFO.name = nameAlg || hashnameAlg
    */
sbweeden commented 3 months ago

Thanks @herrjemand - so I take it your POV would be to support the PR #1926 ?

yackermann commented 2 weeks ago

@sbweeden LGTM

mwiseman-byid commented 1 week ago

@sbweeden Sorry just realised that I was mentioned here:

The FIDO conformance test tool however contains a specific test case where these algorithm identifiers are set to different values, to ensure that RPs use the algorithm identifier from the attested name in certInfo. If an RP strictly follows the process as currently defined in the WebAuthn specification, then that test case will fail.

That came from real world issue, where during the testing we had examples of TPM attestation where nameAlg did not match the other nameAlg.

Here is the direct comment from that time:

    /* 
        So the name is concatenation of nameAlg[2byte] and hash structure[n-bytes].

        The confusion comes from the fact TPMS_CERTIFY_INFO contains name field that contains name of the TPMT_PUBLIC. But in the same time TPMT_PUBLIC contain nameAlg field that contains algorithm identifier for calculating authPolicy. There two both use nameAlg, but they can be different.

        For example:
        TPMT_PUBLIC.nameAlg = SHA-1;
        TPMT_PUBLIC.authPolicy = hashTPMT_PUBLIC.nameAlg

        nameAlg = SHA-256
        TPMS_CERTIFY_INFO.name = nameAlg || hashnameAlg
    */

Can you provide me the scenario (i.e., code) that created this case. These nameAlg values should not differ. TPMT_PUBLIC.nameAlg is passed into the TPM as part of TPM2_Create (or similar create commands). The TPM uses this to calculate the object's name internally. Once the object is created, its TPMT_PUBLIC area is integrity protected by its corresponding sensitive area in the TPM2B_PRIVATE area. The TPMT_PUBLIC.nameAlg is retained in the TPMT_PUBLIC so it can be used by both internal TPM operations and external (the software) so the exact same name (i.e., hash value) is achieved. For example, If the software doesn't know the nameAlg for an object, it looks at the TPMT_PUBLIC structure to find it (If not known, this structure can be obtained using the TPM2_ReadPublic command). The Verifier will need this to calculate the object's Name to compare against what is returned in TPMS_CERTIFY_INFO.name (BTW: this step is missing from the Verification Procedure in 8.3. If desired, I can provide a more detailed description how to sign and verify (The sign operation also doesn't specify which key is use for each parameter in the TPM2_Certify command which can lead to errors.)

sbweeden commented 4 days ago

@yackermann can you please take a look at @mwiseman-byid 's question above and provide details of the scenario that led to the test case being put into the FIDO Conformance test tool?