TPM and Packed attestation not validating root certs #158
Conversation
|
Good catch! Not terribly surprised since the conformance tests don't look for this specific situation and I don't have full negative unit tests written yet. |
|
From the "we never really intended to require attestation" in #158, I'm a bit perplexed looking in the code. It certainly seems like it is intended to validate and throw an error for any TPM, Packed or U2f (which I somehow missed and can fix) attestation that does not chain up to a trusted root from metadata or the known TPM certs. Perhaps if this is not the intention then it should be a flag passed in or a property passed back in a successful response? If this is merged, all untrusted attestations will result in an error. |
|
Yeah...that's why I haven't merged it yet. There are certain cases that are tested for (particularly related to conformance testing), but not completely strict because we don't have all of the roots because vendors are not required to publish roots, and we want people to be able to use their own homebrew stuff if they want. I think the right thing to do here is put this behavior behind a flag and document it. |
|
I think it should work like this if chain fails to build. If we have known good metadata for given authenticator aaguid, or if TBD strict validation flag is set, fail the registration, otherwise let it go. Does that sound reasonable? |
|
That makes sense for Packed and U2F but not sure it does for TPM. How would you decide if you have "good metadata" for TPM? |
|
Good point. There is an official list of TPM identifiers from TCG, but I don't know if that covers every single TPM (or virtualized TPM) out there. I also don't know of a place with a published list of every single possible root for each TPM. |
|
Closed this PR, working on adding the stuff we talked about here and #159 and will open a new PR |
I was quite confused when my Solo Hacker key was passing attestation -- even with modified firmware. The root is not in the metadata repository (hacker has a different root than secure) and it didn't matter. It seems there was a misconception with what
X509VerificationFlags.AllowUnknownCertificateAuthorityactually does. dotnet/runtime#26449This request adds a check to ensure that the root of the chain building matches the expected root from metadata.