Extract the ParseSubjectAlternativeNames function

2023-04-03 22:21:29 +02:00 · 2023-04-03 22:21:29 +02:00 · 9cd4b362f7
commit 9cd4b362f7
parent 827fcb6a06
4 changed files with 15 additions and 163 deletions
--- 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-----
--- 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

--- 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
--- 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=