Fix PR remarks

- Root CA error message improved
- Looping through intermediate certs
- Change checking unhandled extensions to using `if`
This commit is contained in:
Herman Slatman 2023-04-03 11:54:22 +02:00
parent 09bd7705cd
commit b6957358fc
No known key found for this signature in database
GPG key ID: F4D8A44EA0A75A4F
2 changed files with 13 additions and 20 deletions

View file

@ -539,18 +539,16 @@ func doTPMAttestationFormat(ctx context.Context, prov Provisioner, ch *Challenge
} }
intermediates := x509.NewCertPool() intermediates := x509.NewCertPool()
if len(x5c[1:]) > 0 { for _, v := range x5c[1:] {
for _, v := range x5c[1:] { intCertBytes, vok := v.([]byte)
intCertBytes, vok := v.([]byte) if !vok {
if !vok { return nil, NewError(ErrorBadAttestationStatementType, "x5c is malformed")
return nil, NewError(ErrorBadAttestationStatementType, "x5c is malformed")
}
intCert, err := x509.ParseCertificate(intCertBytes)
if err != nil {
return nil, WrapError(ErrorBadAttestationStatementType, err, "x5c is malformed")
}
intermediates.AddCert(intCert)
} }
intCert, err := x509.ParseCertificate(intCertBytes)
if err != nil {
return nil, WrapError(ErrorBadAttestationStatementType, err, "x5c is malformed")
}
intermediates.AddCert(intCert)
} }
// TODO(hs): this can be removed when permanent-identifier/hardware-module-name are handled correctly in // TODO(hs): this can be removed when permanent-identifier/hardware-module-name are handled correctly in
@ -559,11 +557,8 @@ func doTPMAttestationFormat(ctx context.Context, prov Provisioner, ch *Challenge
if len(akCert.UnhandledCriticalExtensions) > 0 { if len(akCert.UnhandledCriticalExtensions) > 0 {
unhandledCriticalExtensions := akCert.UnhandledCriticalExtensions[:0] unhandledCriticalExtensions := akCert.UnhandledCriticalExtensions[:0]
for _, extOID := range akCert.UnhandledCriticalExtensions { for _, extOID := range akCert.UnhandledCriticalExtensions {
switch { if !extOID.Equal(oidSubjectAlternativeName) {
case extOID.Equal(oidSubjectAlternativeName): // critical extensions other than the Subject Alternative Name remain unhandled
// allow Subject Alternative Names, including PermanentIdentifier, HardwareModuleName, TPM attributes, etc
default:
// OIDs that are not in the switch with explicitly allowed OIDs remain unhandled
unhandledCriticalExtensions = append(unhandledCriticalExtensions, extOID) unhandledCriticalExtensions = append(unhandledCriticalExtensions, extOID)
} }
} }
@ -572,7 +567,7 @@ func doTPMAttestationFormat(ctx context.Context, prov Provisioner, ch *Challenge
roots, ok := prov.GetAttestationRoots() roots, ok := prov.GetAttestationRoots()
if !ok { if !ok {
return nil, NewErrorISE("failed getting TPM attestation root CAs") return nil, NewErrorISE("no root CA bundle available to verify the attestation certificate")
} }
verifiedChains, err := akCert.Verify(x509.VerifyOptions{ verifiedChains, err := akCert.Verify(x509.VerifyOptions{
@ -646,8 +641,6 @@ func doTPMAttestationFormat(ctx context.Context, prov Provisioner, ch *Challenge
switch alg { switch alg {
case -257: // RS256 case -257: // RS256
hash = crypto.SHA256 hash = crypto.SHA256
case -8: // EdDSA
hash = crypto.Hash(0)
case -7: // ES256 case -7: // ES256
hash = crypto.SHA256 hash = crypto.SHA256
default: default:

View file

@ -644,7 +644,7 @@ func Test_doTPMAttestationFormat(t *testing.T) {
"certInfo": params.CreateAttestation, "certInfo": params.CreateAttestation,
"pubArea": params.Public, "pubArea": params.Public,
}, },
}}, nil, newInternalServerError("failed getting TPM attestation root CAs")}, }}, nil, newInternalServerError("no root CA bundle available to verify the attestation certificate")},
{"fail verify", args{ctx, mustAttestationProvisioner(t, acaRoot), &Challenge{Token: "token"}, jwk, &attestationObject{ {"fail verify", args{ctx, mustAttestationProvisioner(t, acaRoot), &Challenge{Token: "token"}, jwk, &attestationObject{
Format: "step", Format: "step",
AttStatement: map[string]interface{}{ AttStatement: map[string]interface{}{