Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add serial_number_source option to PKI role #29369

Merged
merged 3 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 82 additions & 0 deletions builtin/logical/pki/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,87 @@
logicaltest.Test(t, testCase)
}

func TestBackend_SerialNumberSource(t *testing.T) {

Check failure on line 301 in builtin/logical/pki/backend_test.go

View workflow job for this annotation

GitHub Actions / Code checks

Test TestBackend_SerialNumberSource is missing a go doc
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)
Expand Down Expand Up @@ -3722,6 +3803,7 @@
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,
Expand Down
11 changes: 9 additions & 2 deletions builtin/logical/pki/issuing/issue_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions builtin/logical/pki/issuing/roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -376,6 +378,7 @@ func SignVerbatimRoleWithOpts(opts ...RoleModifier) *RoleEntry {
KeyType: "any",
UseCSRCommonName: true,
UseCSRSANs: true,
SerialNumberSource: "json-csr",
AllowedOtherSANs: []string{"*"},
AllowedSerialNumbers: []string{"*"},
AllowedURISANs: []string{"*"},
Expand Down
30 changes: 30 additions & 0 deletions builtin/logical/pki/path_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand Down
5 changes: 5 additions & 0 deletions builtin/logical/pki/path_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down
3 changes: 3 additions & 0 deletions changelog/29369.txt
Original file line number Diff line number Diff line change
@@ -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.
```
Loading