Add serial_number_source option to PKI role (#29369)

* Add `serial_number_source` option to PKI role * Add changelog for #29369 --------- Co-authored-by: Violet Hynes <[email protected]>
hashicorp · Jan 27, 2025 · a401afe · a401afe
1 parent e0d60e9
commit a401afe
Show file tree

Hide file tree

Showing 6 changed files with 132 additions and 2 deletions.
diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go
@@ -298,6 +298,87 @@ func TestBackend_CSRValues(t *testing.T) {
 	logicaltest.Test(t, testCase)
 }
 
+func TestBackend_SerialNumberSource(t *testing.T) {
+	t.Parallel()
+	b, s := CreateBackendWithStorage(t)
+
+	var err error
+
+	_, err = CBWrite(b, s, "root/generate/internal", map[string]interface{}{
+		"ttl":         "40h",
+		"common_name": "myvault.com",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = CBWrite(b, s, "roles/json-csr", map[string]interface{}{
+		"allow_any_name":         true,
+		"enforce_hostnames":      false,
+		"allowed_serial_numbers": "foo*",
+		"serial_number_source":   "json-csr",
+		"key_type":               "any",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = CBWrite(b, s, "roles/json", map[string]interface{}{
+		"allow_any_name":         true,
+		"enforce_hostnames":      false,
+		"allowed_serial_numbers": "foo*",
+		"serial_number_source":   "json",
+		"key_type":               "any",
+	})
+
+	// Create a CSR with a serial number not allowed by the role.
+	tmpl := &x509.CertificateRequest{
+		Subject: pkix.Name{SerialNumber: "bar"},
+	}
+	_, _, csrPem := generateCSR(t, tmpl, "ec", 256)
+
+	// Signing a csr with a disallowed subject serial number in the CSR
+	// with serial_number_source=json-csr should fail.
+	_, err = CBWrite(b, s, "sign/json-csr", map[string]interface{}{
+		"common_name": "localhost",
+		"csr":         csrPem,
+	})
+	if err == nil {
+		t.Fatal("expected an error")
+	}
+
+	// The serial number in the request should take precedence.
+	_, err = CBWrite(b, s, "sign/json-csr", map[string]interface{}{
+		"common_name":   "localhost",
+		"csr":           csrPem,
+		"serial_number": "foobar",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Try signing the cert with serial_number_source=json.
+	// The serial in the CSR should be ignored.
+	_, err = CBWrite(b, s, "sign/json", map[string]interface{}{
+		"common_name": "localhost",
+		"csr":         csrPem,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Try signing the cert with serial_number_source=json
+	// and a serial number in the request
+	_, err = CBWrite(b, s, "sign/json", map[string]interface{}{
+		"common_name":   "localhost",
+		"csr":           csrPem,
+		"serial_number": "foobar2",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
 func TestBackend_URLsCRUD(t *testing.T) {
 	t.Parallel()
 	initTest.Do(setCerts)
@@ -3722,6 +3803,7 @@ func TestReadWriteDeleteRoles(t *testing.T) {
 	expectedData := map[string]interface{}{
 		"key_type":                           "rsa",
 		"use_csr_sans":                       true,
+		"serial_number_source":               "json-csr",
 		"client_flag":                        true,
 		"allowed_serial_numbers":             []interface{}{},
 		"generate_lease":                     false,

diff --git a/builtin/logical/pki/issuing/issue_common.go b/builtin/logical/pki/issuing/issue_common.go
@@ -118,8 +118,15 @@ func GenerateCreationBundle(b logical.SystemView, role *RoleEntry, entityInfo En
 		ridSerialNumber = cb.GetSerialNumber()
 
 		// only take serial number from CSR if one was not supplied via API
-		if ridSerialNumber == "" && csr != nil {
-			ridSerialNumber = csr.Subject.SerialNumber
+		switch role.SerialNumberSource {
+		case "", "json-csr":
+			if ridSerialNumber == "" && csr != nil {
+				ridSerialNumber = csr.Subject.SerialNumber
+			}
+		case "json":
+			// use the value from cb set above
+		default:
+			return nil, nil, errutil.UserError{Err: "invalid value for serial_number_source"}
 		}
 
 		if csr != nil && role.UseCSRSANs {

diff --git a/builtin/logical/pki/issuing/roles.go b/builtin/logical/pki/issuing/roles.go
@@ -53,6 +53,7 @@ type RoleEntry struct {
 	EmailProtectionFlag           bool          `json:"email_protection_flag"`
 	UseCSRCommonName              bool          `json:"use_csr_common_name"`
 	UseCSRSANs                    bool          `json:"use_csr_sans"`
+	SerialNumberSource            string        `json:"serial_number_source"`
 	KeyType                       string        `json:"key_type"`
 	KeyBits                       int           `json:"key_bits"`
 	UsePSS                        bool          `json:"use_pss"`
@@ -114,6 +115,7 @@ func (r *RoleEntry) ToResponseData() map[string]interface{} {
 		"email_protection_flag":              r.EmailProtectionFlag,
 		"use_csr_common_name":                r.UseCSRCommonName,
 		"use_csr_sans":                       r.UseCSRSANs,
+		"serial_number_source":               r.SerialNumberSource,
 		"key_type":                           r.KeyType,
 		"key_bits":                           r.KeyBits,
 		"signature_bits":                     r.SignatureBits,
@@ -376,6 +378,7 @@ func SignVerbatimRoleWithOpts(opts ...RoleModifier) *RoleEntry {
 		KeyType:                   "any",
 		UseCSRCommonName:          true,
 		UseCSRSANs:                true,
+		SerialNumberSource:        "json-csr",
 		AllowedOtherSANs:          []string{"*"},
 		AllowedSerialNumbers:      []string{"*"},
 		AllowedURISANs:            []string{"*"},

diff --git a/builtin/logical/pki/path_roles.go b/builtin/logical/pki/path_roles.go
@@ -278,6 +278,15 @@ include the Common Name (cn); use use_csr_common_name
 for that. Defaults to true.`,
 		},
 
+		"serial_number_source": {
+			Type:     framework.TypeString,
+			Required: true,
+			Description: `Source for the certificate subject serial number.
+If "json-csr" (default), the value from the JSON serial_number field is used,
+falling back to the value in the CSR if empty. If "json", the value from the
+serial_number JSON field is used, ignoring the value in the CSR.`,
+		},
+
 		"ou": {
 			Type: framework.TypeCommaStringSlice,
 			Description: `If set, OU (OrganizationalUnit) will be set to
@@ -676,6 +685,19 @@ for that. Defaults to true.`,
 				},
 			},
 
+			"serial_number_source": {
+				Type:    framework.TypeString,
+				Default: "json-csr",
+				Description: `Source for the certificate subject serial number.
+If "json-csr" (default), the value from the JSON serial_number field is used,
+falling back to the value in the CSR if empty. If "json", the value from the
+serial_number JSON field is used, ignoring the value in the CSR.`,
+				DisplayAttrs: &framework.DisplayAttributes{
+					Name:  "Serial number source",
+					Value: "json-csr",
+				},
+			},
+
 			"ou": {
 				Type: framework.TypeCommaStringSlice,
 				Description: `If set, OU (OrganizationalUnit) will be set to
@@ -964,6 +986,7 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data
 		UsePSS:                        data.Get("use_pss").(bool),
 		UseCSRCommonName:              data.Get("use_csr_common_name").(bool),
 		UseCSRSANs:                    data.Get("use_csr_sans").(bool),
+		SerialNumberSource:            data.Get("serial_number_source").(string),
 		KeyUsage:                      data.Get("key_usage").([]string),
 		ExtKeyUsage:                   data.Get("ext_key_usage").([]string),
 		ExtKeyUsageOIDs:               data.Get("ext_key_usage_oids").([]string),
@@ -1061,6 +1084,12 @@ func validateRole(b *backend, entry *issuing.RoleEntry, ctx context.Context, s l
 		return logical.ErrorResponse(err.Error()), nil
 	}
 
+	if entry.SerialNumberSource != "" &&
+		entry.SerialNumberSource != "json-csr" &&
+		entry.SerialNumberSource != "json" {
+		return logical.ErrorResponse("unknown serial_number_source %s", entry.SerialNumberSource), nil
+	}
+
 	if len(entry.ExtKeyUsageOIDs) > 0 {
 		for _, oidstr := range entry.ExtKeyUsageOIDs {
 			_, err := certutil.StringToOid(oidstr)
@@ -1165,6 +1194,7 @@ func (b *backend) pathRolePatch(ctx context.Context, req *logical.Request, data
 		UsePSS:                        getWithExplicitDefault(data, "use_pss", oldEntry.UsePSS).(bool),
 		UseCSRCommonName:              getWithExplicitDefault(data, "use_csr_common_name", oldEntry.UseCSRCommonName).(bool),
 		UseCSRSANs:                    getWithExplicitDefault(data, "use_csr_sans", oldEntry.UseCSRSANs).(bool),
+		SerialNumberSource:            getWithExplicitDefault(data, "serial_number_source", oldEntry.SerialNumberSource).(string),
 		KeyUsage:                      getWithExplicitDefault(data, "key_usage", oldEntry.KeyUsage).([]string),
 		ExtKeyUsage:                   getWithExplicitDefault(data, "ext_key_usage", oldEntry.ExtKeyUsage).([]string),
 		ExtKeyUsageOIDs:               getWithExplicitDefault(data, "ext_key_usage_oids", oldEntry.ExtKeyUsageOIDs).([]string),

diff --git a/builtin/logical/pki/path_roles_test.go b/builtin/logical/pki/path_roles_test.go
@@ -879,6 +879,11 @@ func TestPki_RolePatch(t *testing.T) {
 			Before:  false,
 			Patched: true,
 		},
+		{
+			Field:   "serial_number_source",
+			Before:  "json-csr",
+			Patched: "json",
+		},
 		{
 			Field:   "ou",
 			Before:  []string{"crypto"},

diff --git a/changelog/29369.txt b/changelog/29369.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/pki: Add `serial_number_source` option to PKI roles to control the source for the subject serial number.
+```