-
Notifications
You must be signed in to change notification settings - Fork 172
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
TPM attestation verification steps inconsistent with FIDO conformance testing tool #1925
Comments
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. |
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 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 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. |
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. |
From call on 2023-07-26, @akshayku agreed to seek clarification. Adding him as asignee. |
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 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. |
@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? |
@sbweeden Sorry just realised that I was mentioned here:
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:
|
Thanks @herrjemand - so I take it your POV would be to support the PR #1926 ? |
@sbweeden LGTM |
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.) |
@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? |
FYI - fido-alliance/conformance-test-tools-resources#773 has been opened to ask for the FIDO Conformance test which suggests the algorithms can be different to be removed. |
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:
in
was changed to the wordof
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_ALG_ID 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_ALG_ID 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.
The text was updated successfully, but these errors were encountered: