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

Possibly incorrect TPM manufacturer ID string value #545

Open
sbweeden opened this issue Sep 10, 2024 · 5 comments
Open

Possibly incorrect TPM manufacturer ID string value #545

sbweeden opened this issue Sep 10, 2024 · 5 comments

Comments

@sbweeden
Copy link

This should be confirmed by a subject matter expert (which I confess I am not), however...

See: https://github.com/passwordless-lib/fido2-net-lib/blob/cb71a15c6df0e9d5230b7266502cd8bb26f656cd/Src/Fido2/AttestationFormat/Tpm.cs#L28C10-L28C21

The value for the IBM entry in TPM manufacturers is:
"id:49424d00", // 'IBM' IBM

Refering to sections 3.2.9 and more specifically 3.1.2 of https://trustedcomputinggroup.org/wp-content/uploads/TCG-EK-Credential-Profile-V-2.5-R2_published.pdf
I believe that the hex portion of this ID should be uppercase, thus:
"id:49424D00", // 'IBM' IBM

My reasoning is because section 3.1.2 makes no mention of using lowercase hex chars. It says:

Each byte is
represented individually as a two digit unsigned hexadecimal number using the characters 0-9 and
A-F. The result is concatenated together to form an 8 character name which is appended after the
lower-case ASCII characters “id:”. 
@aseigler
Copy link
Collaborator

aseigler commented Sep 10, 2024

Interesting, I pulled the mapping directly from the file listed, and you can clearly see the lowercase 'd'. I think in our implementation it actually could make a difference, if the TPM outputs '49424D00' and we do a case-sensitive compare for the string '49424d00' it would not match and would throw an exception.

I think the right thing to do perhaps is to not treat the thing after the id: as a string, but rather a byte array and do the comparison that way, as that is the intended use.

image

@MasterKale
Copy link

Maybe keep things simple and duplicate the IBM identifier, one with uppercase and one with the registry-accurate lowercase version?

@sbweeden
Copy link
Author

I don't think the TPM outputs a string, nor do I think that table's "Hex" column represents stringified hex chars. I think the TPM outputs bytes. Its only in the context of representing them as part of an X509 name that they get mapped to a string, and that mapping is defined by the EK Credential Profile documented. I completely agree that it would be a good idea if the table's Hex column was consistent in this regard.

@aseigler
Copy link
Collaborator

I don't think the TPM outputs a string

The link you posted would indicate it is a UTF8 string.
image

I think we should split the string on the colon, discarding the ID portion and saving the remainder as a byte array.

It's definitely possibly broken now, depending on what exactly an IBM manufactured TPM puts into an actual attestation. See https://dotnetfiddle.net/x7gFfr.

@sbweeden
Copy link
Author

The link I posted describes a UTF8String, but I believe that is describing the rules for encoding the output that appears in the certificate SAN extension value, not what the input is. The input is what the TPM returns from the TPM2_GetCapability command (as described in section 3.2.9). I don't exactly know how TPM commands work, or what their data encoding is, but I highly suspect it's bytes.

That said, it is not unreasonable to split on the colon and treat the remainder as the hex chars of a byte array, at least for the manufacturer. That is what a different library I work on does today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants