From 9cd4b362f7fde27e4d811ccf6e68bd0e7346b74c Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Mon, 3 Apr 2023 22:21:29 +0200 Subject: [PATCH] Extract the `ParseSubjectAlternativeNames` function --- acme/challenge.go | 142 +--------------------------- acme/challenge_tpmsimulator_test.go | 30 ++---- go.mod | 2 +- go.sum | 4 +- 4 files changed, 15 insertions(+), 163 deletions(-) diff --git a/acme/challenge.go b/acme/challenge.go index b55d3065..fb98b50c 100644 --- a/acme/challenge.go +++ b/acme/challenge.go @@ -11,7 +11,6 @@ import ( "crypto/subtle" "crypto/tls" "crypto/x509" - "crypto/x509/pkix" "encoding/asn1" "encoding/base64" "encoding/hex" @@ -582,29 +581,14 @@ func doTPMAttestationFormat(ctx context.Context, prov Provisioner, ch *Challenge // TODO(hs): implement revocation check; Verify() doesn't perform CRL check nor OCSP lookup. - // extract and validate Subject Alternative Name extension to contain at least one PermanentIdentifier - var sanExtension pkix.Extension - for _, ext := range akCert.Extensions { - if ext.Id.Equal(oidSubjectAlternativeName) { - sanExtension = ext - break - } - } - - if sanExtension.Value == nil { - return nil, NewError(ErrorBadAttestationStatementType, "AK certificate is missing Subject Alternative Name extension") - } - - sans, err := parseSANs(sanExtension) + sans, err := x509util.ParseSubjectAlternativeNames(akCert) if err != nil { - return nil, WrapError(ErrorBadAttestationStatementType, err, "failed parsing AK certificate SAN extension") + return nil, WrapError(ErrorBadAttestationStatementType, err, "failed parsing AK certificate Subject Alternative Names") } - var permanentIdentifiers []string - for _, san := range sans { - if san.Type == x509util.PermanentIdentifierType { - permanentIdentifiers = append(permanentIdentifiers, san.Value) - } + permanentIdentifiers := make([]string, len(sans.PermanentIdentifiers)) + for i, pi := range sans.PermanentIdentifiers { + permanentIdentifiers[i] = pi.Identifier } // extract and validate pubArea, sig, certInfo and alg properties from the request body @@ -705,122 +689,6 @@ func doTPMAttestationFormat(ctx context.Context, prov Provisioner, ch *Challenge return data, nil } -// RFC 4043 -// -// https://tools.ietf.org/html/rfc4043 -var ( - oidPermanentIdentifier = []int{1, 3, 6, 1, 5, 5, 7, 8, 3} -) - -// PermanentIdentifier represents an ASN.1 encoded "permanent identifier" as -// defined by RFC4043. -// -// PermanentIdentifier ::= SEQUENCE { -// identifierValue UTF8String OPTIONAL, -// assigner OBJECT IDENTIFIER OPTIONAL -// } -// -// https://datatracker.ietf.org/doc/html/rfc4043 -type permanentIdentifier struct { - IdentifierValue string `asn1:"utf8,optional"` - Assigner asn1.ObjectIdentifier `asn1:"optional"` -} - -func parsePermanentIdentifier(der []byte) (permanentIdentifier, error) { - var permID permanentIdentifier - if _, err := asn1.UnmarshalWithParams(der, &permID, "explicit,tag:0"); err != nil { - return permanentIdentifier{}, err - } - return permID, nil -} - -func parseSANs(ext pkix.Extension) (sans []x509util.SubjectAlternativeName, err error) { - _, otherNames, err := parseSubjectAltName(ext) - if err != nil { - return nil, fmt.Errorf("parseSubjectAltName: %w", err) - } - - for _, otherName := range otherNames { - if otherName.TypeID.Equal(oidPermanentIdentifier) { - permID, err := parsePermanentIdentifier(otherName.Value.FullBytes) - if err != nil { - return nil, fmt.Errorf("parsePermanentIdentifier: %w", err) - } - permanentIdentifier := x509util.SubjectAlternativeName{ - Type: x509util.PermanentIdentifierType, - Value: permID.IdentifierValue, // TODO(hs): change how these are returned - } - sans = append(sans, permanentIdentifier) - } - } - - return -} - -// OtherName ::= SEQUENCE { -// type-id OBJECT IDENTIFIER, -// value [0] EXPLICIT ANY DEFINED BY type-id } -type otherName struct { - TypeID asn1.ObjectIdentifier - Value asn1.RawValue -} - -// https://datatracker.ietf.org/doc/html/rfc5280#page-35 -func parseSubjectAltName(ext pkix.Extension) (dirNames []pkix.Name, otherNames []otherName, err error) { - err = forEachSAN(ext.Value, func(generalName asn1.RawValue) error { - switch generalName.Tag { - case 0: // otherName - var on otherName - if _, err := asn1.UnmarshalWithParams(generalName.FullBytes, &on, "tag:0"); err != nil { - return fmt.Errorf("OtherName: asn1.UnmarshalWithParams: %w", err) - } - otherNames = append(otherNames, on) - case 4: // directoryName - var rdns pkix.RDNSequence - if _, err := asn1.Unmarshal(generalName.Bytes, &rdns); err != nil { - return fmt.Errorf("DirectoryName: asn1.Unmarshal: %w", err) - } - var dirName pkix.Name - dirName.FillFromRDNSequence(&rdns) - dirNames = append(dirNames, dirName) - default: - //return fmt.Errorf("expected tag %d", generalName.Tag) - // TODO(hs): implement the others ... skipping for now - } - return nil - }) - return -} - -// Borrowed from the x509 package. -func forEachSAN(extension []byte, callback func(ext asn1.RawValue) error) error { - var seq asn1.RawValue - rest, err := asn1.Unmarshal(extension, &seq) - if err != nil { - return err - } else if len(rest) != 0 { - return errors.New("x509: trailing data after X.509 extension") - } - if !seq.IsCompound || seq.Tag != 16 || seq.Class != 0 { - return asn1.StructuralError{Msg: "bad SAN sequence"} - } - - rest = seq.Bytes - for len(rest) > 0 { - var v asn1.RawValue - rest, err = asn1.Unmarshal(rest, &v) - if err != nil { - return err - } - - if err := callback(v); err != nil { - return err - } - } - - return nil -} - // Apple Enterprise Attestation Root CA from // https://www.apple.com/certificateauthority/private/ const appleEnterpriseAttestationRootCA = `-----BEGIN CERTIFICATE----- diff --git a/acme/challenge_tpmsimulator_test.go b/acme/challenge_tpmsimulator_test.go index b34b1070..ae6eb210 100644 --- a/acme/challenge_tpmsimulator_test.go +++ b/acme/challenge_tpmsimulator_test.go @@ -421,6 +421,11 @@ func newInternalServerError(msg string) *Error { } } +var ( + oidPermanentIdentifier = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 8, 3} + oidHardwareModuleNameIdentifier = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 8, 4} +) + func Test_doTPMAttestationFormat(t *testing.T) { ctx := context.Background() aca, err := minica.New( @@ -460,16 +465,6 @@ func Test_doTPMAttestationFormat(t *testing.T) { require.NoError(t, err) require.NotNil(t, akCert) - templateMissingSAN := &x509.Certificate{ - Subject: pkix.Name{ - CommonName: "testakcertmissingsan", - }, - PublicKey: akp.Public, - } - akCertMissingSAN, err := aca.Sign(templateMissingSAN) - require.NoError(t, err) - require.NotNil(t, akCert) - // generate a JWK and the key authorization value jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0) require.NoError(t, err) @@ -656,17 +651,6 @@ func Test_doTPMAttestationFormat(t *testing.T) { "pubArea": params.Public, }, }}, nil, newBadAttestationStatementError("x5c is not valid: x509: certificate signed by unknown authority")}, - {"fail missing SAN extension", args{ctx, mustAttestationProvisioner(t, acaRoot), &Challenge{Token: "token"}, jwk, &attestationObject{ - Format: "tpm", - AttStatement: map[string]interface{}{ - "ver": "2.0", - "x5c": []interface{}{akCertMissingSAN.Raw, aca.Intermediate.Raw}, - "alg": int64(-257), // RS256 - "sig": params.CreateSignature, - "certInfo": params.CreateAttestation, - "pubArea": params.Public, - }, - }}, nil, newBadAttestationStatementError("AK certificate is missing Subject Alternative Name extension")}, {"fail pubArea not present", args{ctx, mustAttestationProvisioner(t, acaRoot), &Challenge{Token: "token"}, jwk, &attestationObject{ Format: "tpm", AttStatement: map[string]interface{}{ @@ -854,8 +838,8 @@ func Test_doTPMAttestationFormat(t *testing.T) { // // See also https://datatracker.ietf.org/doc/html/rfc5280.html#section-4.2.1.6 // -// TODO(hs): this was copied from go.step.sm/crypto/x509util. Should it be -// exposed instead? +// TODO(hs): this was copied from go.step.sm/crypto/x509util to make it easier +// to create the SAN extension for testing purposes. Should it be exposed instead? func createSubjectAltNameExtension(dnsNames, emailAddresses x509util.MultiString, ipAddresses x509util.MultiIP, uris x509util.MultiURL, sans []x509util.SubjectAlternativeName, subjectIsEmpty bool) (x509util.Extension, error) { var zero x509util.Extension diff --git a/go.mod b/go.mod index 10e02c41..045e222c 100644 --- a/go.mod +++ b/go.mod @@ -29,7 +29,7 @@ require ( github.com/urfave/cli v1.22.12 go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352 go.step.sm/cli-utils v0.7.5 - go.step.sm/crypto v0.28.1-0.20230329145110-4ccd51b601c7 + go.step.sm/crypto v0.28.1-0.20230403133050-c4ef6cdd0c34 go.step.sm/linkedca v0.19.0 golang.org/x/crypto v0.7.0 golang.org/x/exp v0.0.0-20230310171629-522b1b587ee0 diff --git a/go.sum b/go.sum index 5b5ab3c7..df80b4e9 100644 --- a/go.sum +++ b/go.sum @@ -1024,8 +1024,8 @@ go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqe go.step.sm/cli-utils v0.7.5 h1:jyp6X8k8mN1B0uWJydTid0C++8tQhm2kaaAdXKQQzdk= go.step.sm/cli-utils v0.7.5/go.mod h1:taSsY8haLmXoXM3ZkywIyRmVij/4Aj0fQbNTlJvv71I= go.step.sm/crypto v0.9.0/go.mod h1:+CYG05Mek1YDqi5WK0ERc6cOpKly2i/a5aZmU1sfGj0= -go.step.sm/crypto v0.28.1-0.20230329145110-4ccd51b601c7 h1:8h176gNghpnGnVY99fHHoFZaPOeDR9Q472KuxSCeeRA= -go.step.sm/crypto v0.28.1-0.20230329145110-4ccd51b601c7/go.mod h1:PFmyUJUvF5YDVokruSlTDcENBD/QBDpBiV2zInfsgV0= +go.step.sm/crypto v0.28.1-0.20230403133050-c4ef6cdd0c34 h1:sByVwgjzB3A0MXtLyMbBd/AgvsCxglPm2PLvDhdJqAE= +go.step.sm/crypto v0.28.1-0.20230403133050-c4ef6cdd0c34/go.mod h1:PFmyUJUvF5YDVokruSlTDcENBD/QBDpBiV2zInfsgV0= go.step.sm/linkedca v0.19.0 h1:xuagkR35wrJI2gnu6FAM+q3VmjwsHScvGcJsfZ0GdsI= go.step.sm/linkedca v0.19.0/go.mod h1:b7vWPrHfYLEOTSUZitFEcztVCpTc+ileIN85CwEAluM= go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=