Possibly incorrect TPM manufacturer ID string value

[sbweeden]

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]

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.

[MasterKale]

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

[sbweeden]

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]

I don't think the TPM outputs a string

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

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]

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.

[abergs]

@aseigler Is this something you want to take a look at and get into v4? Otherwise we might punt it to a patch release.

[aseigler]

Yes, I think I know what I want to do with this and it should be very small. Should be able to get it in the next couple of days.

[sbweeden]

FWIW I’m happy to review or at least eyeball the proposed PR for this.