Add certificate parser to parse x509 Certificate to EK Certificate.#471
Add certificate parser to parse x509 Certificate to EK Certificate.#471liamjm merged 5 commits intogoogle:masterfrom
Conversation
liamjm
left a comment
liamjm
left a comment
There was a problem hiding this comment.
I didn't see any handling of TPM Security Assertions (Section 3.1.1 from the EK Credential profile spec). Shall we add a TODO to add these.
The code also doesn't validate some things, eg the format of the TPM Manufacturer. I think this is OK, as that can be in a separate method to verify these attributes, if anyone cares. However, the code does verify some aspects (eg critical vs optional extensions), so there is some potential for confusion. Perhaps the method documentation should state something around the face that the presence of extensions are check, but not all values are checked. WDYT?
neha-gunta
left a comment
neha-gunta
left a comment
There was a problem hiding this comment.
I didn't see any handling of TPM Security Assertions (Section 3.1.1 from the EK Credential profile spec). Shall we add a TODO to add these.
The code also doesn't validate some things, eg the format of the TPM Manufacturer. I think this is OK, as that can be in a separate method to verify these attributes, if anyone cares. However, the code does verify some aspects (eg critical vs optional extensions), so there is some potential for confusion. Perhaps the method documentation should state something around the face that the presence of extensions are check, but not all values are checked. WDYT?
Agreed! I shall add an elaborate comment stating the limitations of our function.
|
Please fix lint errors, otherwise LGTM. |
| } | ||
|
|
||
| // Iterate through unknown/custom ExtKeyUsage OIDs | ||
| for _, eku := range cert.UnknownExtKeyUsage { |
There was a problem hiding this comment.
This could theoretically break if Go x509 ever adds this ExtKeyUsage
golang/go#75325 is solved in Go 1.26 which would allow this to be written in a forward-compatible manner by iterating both over ExtKeyUsage and call OID() on that, and iterate over UnknownExtKeyUsage.
However it's a bit of a nit as I don't expect it to be likely that this ExtKeyUsage is ever added to x509 due to not being web pki related
| if spec, err = parseTPMSpecification(subjectDirectoryAttributes); err != nil { | ||
| return nil, err | ||
| } | ||
| case ext.Id.Equal(oidBasicConstraints): |
There was a problem hiding this comment.
There is no need to manually check for Critical bit on all these oids. This is already handled by the x509 library. Either the x509 library parses the fields; or it puts them in UnhandledCriticalExtensions. a non-empty UnhandlesCriticalExtensions throws an error on Verify()
There was a problem hiding this comment.
What if the critical bit is off? The x509 library will not complaint, but it violate the TCG spec, right?
4 participants
Add certificate parser to parse x509 Certificate to EK Certificate.
The standard fields are validated by referencing the TCG EK Credential Profile.