Migration of OtherSANs Parsing Call to SDK helper from pki-issuer (#24946)

* Migration of OtherSANs Parsing Call to SDK helper from pki-issuer

* Based on PR feedback from Steve, remove internal variable, reference certutil directly.

builtin/logical/pki/backend_test.go

@@ -1599,7 +1599,7 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
 	}
 
 	{
-		getOtherCheck := func(expectedOthers ...issuing.OtherNameUtf8) logicaltest.TestCheckFunc {
+		getOtherCheck := func(expectedOthers ...certutil.OtherNameUtf8) logicaltest.TestCheckFunc {
 			return func(resp *logical.Response) error {
 				var certBundle certutil.CertBundle
 				err := mapstructure.Decode(resp.Data, &certBundle)
@@ -1615,7 +1615,7 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
 				if err != nil {
 					return err
 				}
-				var expected []issuing.OtherNameUtf8
+				var expected []certutil.OtherNameUtf8
 				expected = append(expected, expectedOthers...)
 				if diff := deep.Equal(foundOthers, expected); len(diff) > 0 {
 					return fmt.Errorf("wrong SAN IPs, diff: %v", diff)
@@ -1624,8 +1624,8 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
 			}
 		}
 
-		addOtherSANTests := func(useCSRs, useCSRSANs bool, allowedOtherSANs []string, errorOk bool, otherSANs []string, csrOtherSANs []issuing.OtherNameUtf8, check logicaltest.TestCheckFunc) {
+		addOtherSANTests := func(useCSRs, useCSRSANs bool, allowedOtherSANs []string, errorOk bool, otherSANs []string, csrOtherSANs []certutil.OtherNameUtf8, check logicaltest.TestCheckFunc) {
-			otherSansMap := func(os []issuing.OtherNameUtf8) map[string][]string {
+			otherSansMap := func(os []certutil.OtherNameUtf8) map[string][]string {
 				ret := make(map[string][]string)
 				for _, o := range os {
 					ret[o.Oid] = append(ret[o.Oid], o.Value)
@@ -1659,14 +1659,14 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
 		roleVals.UseCSRCommonName = true
 		commonNames.Localhost = true
 
-		newOtherNameUtf8 := func(s string) (ret issuing.OtherNameUtf8) {
+		newOtherNameUtf8 := func(s string) (ret certutil.OtherNameUtf8) {
 			pieces := strings.Split(s, ";")
 			if len(pieces) == 2 {
 				piecesRest := strings.Split(pieces[1], ":")
 				if len(piecesRest) == 2 {
 					switch strings.ToUpper(piecesRest[0]) {
 					case "UTF-8", "UTF8":
-						return issuing.OtherNameUtf8{Oid: pieces[0], Value: piecesRest[1]}
+						return certutil.OtherNameUtf8{Oid: pieces[0], Value: piecesRest[1]}
 					}
 				}
 			}
@@ -1676,7 +1676,7 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
 		oid1 := "1.3.6.1.4.1.311.20.2.3"
 		oth1str := oid1 + ";utf8:devops@nope.com"
 		oth1 := newOtherNameUtf8(oth1str)
-		oth2 := issuing.OtherNameUtf8{oid1, "me@example.com"}
+		oth2 := certutil.OtherNameUtf8{oid1, "me@example.com"}
 		// allowNone, allowAll := []string{}, []string{oid1 + ";UTF-8:*"}
 		allowNone, allowAll := []string{}, []string{"*"}
 
@@ -1691,15 +1691,15 @@ func generateRoleSteps(t *testing.T, useCSRs bool) []logicaltest.TestStep {
 
 		// Given OtherSANs as API argument and useCSRSANs false, CSR arg ignored.
 		addOtherSANTests(useCSRs, false, allowAll, false, []string{oth1str},
-			[]issuing.OtherNameUtf8{oth2}, getOtherCheck(oth1))
+			[]certutil.OtherNameUtf8{oth2}, getOtherCheck(oth1))
 
 		if useCSRs {
 			// OtherSANs not allowed, valid OtherSANs provided via CSR, should be an error.
-			addOtherSANTests(useCSRs, true, allowNone, true, nil, []issuing.OtherNameUtf8{oth1}, nil)
+			addOtherSANTests(useCSRs, true, allowNone, true, nil, []certutil.OtherNameUtf8{oth1}, nil)
 
 			// Given OtherSANs as both API and CSR arguments and useCSRSANs=true, API arg ignored.
 			addOtherSANTests(useCSRs, false, allowAll, false, []string{oth2.String()},
-				[]issuing.OtherNameUtf8{oth1}, getOtherCheck(oth2))
+				[]certutil.OtherNameUtf8{oth1}, getOtherCheck(oth2))
 		}
 	}
 
@@ -2177,7 +2177,7 @@ func runTestSignVerbatim(t *testing.T, keyType string) {
 		// On older versions of Go this test will fail due to an explicit check for duplicate otherNames later in this test.
 		ExtraExtensions: []pkix.Extension{
 			{
-				Id:       oidExtensionSubjectAltName,
+				Id:       certutil.OidExtensionSubjectAltName,
 				Critical: false,
 				Value:    []byte{0x30, 0x26, 0xA0, 0x24, 0x06, 0x0A, 0x2B, 0x06, 0x01, 0x04, 0x01, 0x82, 0x37, 0x14, 0x02, 0x03, 0xA0, 0x16, 0x0C, 0x14, 0x75, 0x73, 0x65, 0x72, 0x6E, 0x61, 0x6D, 0x65, 0x40, 0x65, 0x78, 0x61, 0x6D, 0x70, 0x6C, 0x65, 0x2E, 0x63, 0x6F, 0x6D},
 			},
@@ -2339,7 +2339,7 @@ func runTestSignVerbatim(t *testing.T, keyType string) {
 	// We assume that there is only one SAN in the original CSR and that it is an otherName.
 	san_count := 0
 	for _, ext := range cert.Extensions {
-		if ext.Id.Equal(oidExtensionSubjectAltName) {
+		if ext.Id.Equal(certutil.OidExtensionSubjectAltName) {
 			san_count += 1
 		}
 	}
@@ -3130,13 +3130,13 @@ func TestBackend_OID_SANs(t *testing.T) {
 		cert.DNSNames[2] != "foobar.com" {
 		t.Fatalf("unexpected DNS SANs %v", cert.DNSNames)
 	}
-	expectedOtherNames := []issuing.OtherNameUtf8{{oid1, val1}, {oid2, val2}}
+	expectedOtherNames := []certutil.OtherNameUtf8{{oid1, val1}, {oid2, val2}}
 	foundOtherNames, err := getOtherSANsFromX509Extensions(cert.Extensions)
 	if err != nil {
 		t.Fatal(err)
 	}
 	// Sort our returned list as SANS are built internally with a map so ordering can be inconsistent
-	slices.SortFunc(foundOtherNames, func(a, b issuing.OtherNameUtf8) int { return cmp.Compare(a.Oid, b.Oid) })
+	slices.SortFunc(foundOtherNames, func(a, b certutil.OtherNameUtf8) int { return cmp.Compare(a.Oid, b.Oid) })
 
 	if diff := deep.Equal(expectedOtherNames, foundOtherNames); len(diff) != 0 {
 		t.Errorf("unexpected otherNames: %v", diff)

builtin/logical/pki/cert_util.go

@@ -62,9 +62,6 @@ var (
 	middleWildRegex    = labelRegex + `\*` + labelRegex
 	leftWildLabelRegex = regexp.MustCompile(`^(` + allWildRegex + `|` + startWildRegex + `|` + endWildRegex + `|` + middleWildRegex + `)$`)
 
-	// OIDs for X.509 certificate extensions used below.
-	oidExtensionSubjectAltName = issuing.OidExtensionSubjectAltName
-
 	// Cloned from https://github.com/golang/go/blob/82c713feb05da594567631972082af2fcba0ee4f/src/crypto/x509/x509.go#L327-L379
 	oidSignatureMD2WithRSA      = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 2}
 	oidSignatureMD5WithRSA      = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 4}
@@ -482,8 +479,8 @@ func signCert(b *backend, data *inputBundle, caSign *certutil.CAInfoBundle, isCA
 	return issuing.SignCert(b.System(), data.role, entityInfo, caSign, signCertInput)
 }
 
-func getOtherSANsFromX509Extensions(exts []pkix.Extension) ([]issuing.OtherNameUtf8, error) {
+func getOtherSANsFromX509Extensions(exts []pkix.Extension) ([]certutil.OtherNameUtf8, error) {
-	return issuing.GetOtherSANsFromX509Extensions(exts)
+	return certutil.GetOtherSANsFromX509Extensions(exts)
 }
 
 var _ issuing.CreationBundleInput = CreationBundleInputFromFieldData{}
@@ -699,7 +696,7 @@ func handleOtherSANs(in *x509.Certificate, sans map[string][]string) error {
 	// Marshal and add to ExtraExtensions
 	ext := pkix.Extension{
 		// This is the defined OID for subjectAltName
-		Id: asn1.ObjectIdentifier(oidExtensionSubjectAltName),
+		Id: certutil.OidExtensionSubjectAltName,
 	}
 	var err error
 	ext.Value, err = asn1.Marshal(rawValues)

builtin/logical/pki/issuing/issue_common.go

@@ -7,7 +7,6 @@ import (
 	"context"
 	"crypto/x509"
 	"crypto/x509/pkix"
-	"encoding/asn1"
 	"encoding/hex"
 	"fmt"
 	"net"
@@ -22,8 +21,6 @@ import (
 	"github.com/hashicorp/vault/sdk/helper/errutil"
 	"github.com/hashicorp/vault/sdk/logical"
 	"github.com/ryanuber/go-glob"
-	"golang.org/x/crypto/cryptobyte"
-	asn12 "golang.org/x/crypto/cryptobyte/asn1"
 	"golang.org/x/net/idna"
 
 	"github.com/hashicorp/vault/builtin/logical/pki/parsing"
@@ -55,9 +52,6 @@ var (
 	endWildRegex       = labelRegex + `\*`
 	middleWildRegex    = labelRegex + `\*` + labelRegex
 	leftWildLabelRegex = regexp.MustCompile(`^(` + allWildRegex + `|` + startWildRegex + `|` + endWildRegex + `|` + middleWildRegex + `)$`)
-
-	// OIDs for X.509 certificate extensions used below.
-	OidExtensionSubjectAltName = []int{2, 5, 29, 17}
 )
 
 type EntityInfo struct {
@@ -216,7 +210,7 @@ func GenerateCreationBundle(b logical.SystemView, role *RoleEntry, entityInfo En
 		otherSANsInput = sans
 	}
 	if role.UseCSRSANs && csr != nil && len(csr.Extensions) > 0 {
-		others, err := GetOtherSANsFromX509Extensions(csr.Extensions)
+		others, err := certutil.GetOtherSANsFromX509Extensions(csr.Extensions)
 		if err != nil {
 			return nil, nil, errutil.UserError{Err: fmt.Errorf("could not parse requested other SAN: %w", err).Error()}
 		}
@@ -930,115 +924,6 @@ func ValidateSerialNumber(role *RoleEntry, serialNumber string) string {
 	}
 }
 
-func GetOtherSANsFromX509Extensions(exts []pkix.Extension) ([]OtherNameUtf8, error) {
-	var ret []OtherNameUtf8
-	for _, ext := range exts {
-		if !ext.Id.Equal(OidExtensionSubjectAltName) {
-			continue
-		}
-		err := forEachSAN(ext.Value, func(tag int, data []byte) error {
-			if tag != 0 {
-				return nil
-			}
-
-			var other OtherNameRaw
-			_, err := asn1.UnmarshalWithParams(data, &other, "tag:0")
-			if err != nil {
-				return fmt.Errorf("could not parse requested other SAN: %w", err)
-			}
-			val, err := other.ExtractUTF8String()
-			if err != nil {
-				return err
-			}
-			ret = append(ret, *val)
-			return nil
-		})
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	return ret, nil
-}
-
-func forEachSAN(extension []byte, callback func(tag int, data []byte) error) error {
-	// RFC 5280, 4.2.1.6
-
-	// SubjectAltName ::= GeneralNames
-	//
-	// GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName
-	//
-	// GeneralName ::= CHOICE {
-	//      otherName                       [0]     OtherName,
-	//      rfc822Name                      [1]     IA5String,
-	//      dNSName                         [2]     IA5String,
-	//      x400Address                     [3]     ORAddress,
-	//      directoryName                   [4]     Name,
-	//      ediPartyName                    [5]     EDIPartyName,
-	//      uniformResourceIdentifier       [6]     IA5String,
-	//      iPAddress                       [7]     OCTET STRING,
-	//      registeredID                    [8]     OBJECT IDENTIFIER }
-	var seq asn1.RawValue
-	rest, err := asn1.Unmarshal(extension, &seq)
-	if err != nil {
-		return err
-	} else if len(rest) != 0 {
-		return fmt.Errorf("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.Tag, v.FullBytes); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
-// otherNameRaw describes a name related to a certificate which is not in one
-// of the standard name formats. RFC 5280, 4.2.1.6:
-//
-//	OtherName ::= SEQUENCE {
-//	     type-id    OBJECT IDENTIFIER,
-//	     Value      [0] EXPLICIT ANY DEFINED BY type-id }
-type OtherNameRaw struct {
-	TypeID asn1.ObjectIdentifier
-	Value  asn1.RawValue
-}
-
-type OtherNameUtf8 struct {
-	Oid   string
-	Value string
-}
-
-// ExtractUTF8String returns the UTF8 string contained in the Value, or an error
-// if none is present.
-func (oraw *OtherNameRaw) ExtractUTF8String() (*OtherNameUtf8, error) {
-	svalue := cryptobyte.String(oraw.Value.Bytes)
-	var outTag asn12.Tag
-	var val cryptobyte.String
-	read := svalue.ReadAnyASN1(&val, &outTag)
-
-	if read && outTag == asn1.TagUTF8String {
-		return &OtherNameUtf8{Oid: oraw.TypeID.String(), Value: string(val)}, nil
-	}
-	return nil, fmt.Errorf("no UTF-8 string found in OtherName")
-}
-
-func (o OtherNameUtf8) String() string {
-	return fmt.Sprintf("%s;%s:%s", o.Oid, "UTF-8", o.Value)
-}
-
 type CertNotAfterInput interface {
 	GetTTL() int
 	GetOptionalNotAfter() (interface{}, bool)

sdk/helper/certutil/helpers.go

@@ -82,6 +82,9 @@ var InvSignatureAlgorithmNames = map[x509.SignatureAlgorithm]string{
 	x509.PureEd25519:      "Ed25519",
 }
 
+// OIDs for X.509 SAN Extension
+var OidExtensionSubjectAltName = asn1.ObjectIdentifier([]int{2, 5, 29, 17})
+
 // OID for RFC 5280 CRL Number extension.
 //
 // > id-ce-cRLNumber OBJECT IDENTIFIER ::= { id-ce 20 }
@@ -1469,3 +1472,117 @@ func CreateBasicConstraintExtension(isCa bool, maxPath int) (pkix.Extension, err
 		Value:    asn1Bytes,
 	}, nil
 }
+
+// GetOtherSANsFromX509Extensions is used to find all the extensions which have the identifier (OID) of
+// a SAN (Subject Alternative Name), and then look at each extension to find out if it is one of a set of
+// well-known types (like IP SANs) or "other".  Currently, the only OtherSANs vault supports are of type UTF8.
+func GetOtherSANsFromX509Extensions(exts []pkix.Extension) ([]OtherNameUtf8, error) {
+	var ret []OtherNameUtf8
+	for _, ext := range exts {
+		if !ext.Id.Equal(OidExtensionSubjectAltName) {
+			continue
+		}
+		err := forEachSAN(ext.Value, func(tag int, data []byte) error {
+			if tag != 0 {
+				return nil
+			}
+
+			var other OtherNameRaw
+			_, err := asn1.UnmarshalWithParams(data, &other, "tag:0")
+			if err != nil {
+				return fmt.Errorf("could not parse requested other SAN: %w", err)
+			}
+			val, err := other.ExtractUTF8String()
+			if err != nil {
+				return err
+			}
+			ret = append(ret, *val)
+			return nil
+		})
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return ret, nil
+}
+
+func forEachSAN(extension []byte, callback func(tag int, data []byte) error) error {
+	// RFC 5280, 4.2.1.6
+
+	// SubjectAltName ::= GeneralNames
+	//
+	// GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName
+	//
+	// GeneralName ::= CHOICE {
+	//      otherName                       [0]     OtherName,
+	//      rfc822Name                      [1]     IA5String,
+	//      dNSName                         [2]     IA5String,
+	//      x400Address                     [3]     ORAddress,
+	//      directoryName                   [4]     Name,
+	//      ediPartyName                    [5]     EDIPartyName,
+	//      uniformResourceIdentifier       [6]     IA5String,
+	//      iPAddress                       [7]     OCTET STRING,
+	//      registeredID                    [8]     OBJECT IDENTIFIER }
+	var seq asn1.RawValue
+	rest, err := asn1.Unmarshal(extension, &seq)
+	if err != nil {
+		return err
+	} else if len(rest) != 0 {
+		return fmt.Errorf("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.Tag, v.FullBytes); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// otherNameRaw describes a name related to a certificate which is not in one
+// of the standard name formats. RFC 5280, 4.2.1.6:
+//
+//	OtherName ::= SEQUENCE {
+//	     type-id    OBJECT IDENTIFIER,
+//	     Value      [0] EXPLICIT ANY DEFINED BY type-id }
+type OtherNameRaw struct {
+	TypeID asn1.ObjectIdentifier
+	Value  asn1.RawValue
+}
+
+type OtherNameUtf8 struct {
+	Oid   string
+	Value string
+}
+
+// String() turns an OtherNameUtf8 object into the storage or field-value used to assign that name
+// to a certificate in an API call
+func (o OtherNameUtf8) String() string {
+	return fmt.Sprintf("%s;%s:%s", o.Oid, "UTF-8", o.Value)
+}
+
+// ExtractUTF8String returns the UTF8 string contained in the Value, or an error
+// if none is present.
+func (oraw *OtherNameRaw) ExtractUTF8String() (*OtherNameUtf8, error) {
+	svalue := cryptobyte.String(oraw.Value.Bytes)
+	var outTag cbasn1.Tag
+	var val cryptobyte.String
+	read := svalue.ReadAnyASN1(&val, &outTag)
+
+	if read && outTag == asn1.TagUTF8String {
+		return &OtherNameUtf8{Oid: oraw.TypeID.String(), Value: string(val)}, nil
+	}
+	return nil, fmt.Errorf("no UTF-8 string found in OtherName")
+}