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 use_csr_serial_number option to PKI role #25709

Closed
wants to merge 2 commits into from
Closed
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
66 changes: 66 additions & 0 deletions builtin/logical/pki/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3252,6 +3252,71 @@ func TestBackend_AllowedSerialNumbers(t *testing.T) {
}
}

func TestBackend_UseCSRSerialNumber(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/snfalse", map[string]interface{}{
"allow_any_name": true,
"enforce_hostnames": false,
"allowed_serial_numbers": "foo*",
"use_csr_serial_number": false,
"key_type": "any",
})
if err != nil {
t.Fatal(err)
}

_, err = CBWrite(b, s, "roles/sntrue", map[string]interface{}{
"allow_any_name": true,
"enforce_hostnames": false,
"allowed_serial_numbers": "foo*",
"use_csr_serial_number": true,
"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)

// Try signing the cert with use_csr_serial_number=True.
_, err = CBWrite(b, s, "sign/sntrue", map[string]interface{}{
"common_name": "localhost",
"csr": csrPem,
})
if err == nil {
t.Fatal("expected error")
}

// The serial number in the request should take precedence.
_, err = CBWrite(b, s, "sign/sntrue", map[string]interface{}{
"common_name": "localhost",
"csr": csrPem,
"serial_number": "foobar",
})

// Try signing the cert with use_csr_serial_number=False.
_, err = CBWrite(b, s, "sign/snfalse", map[string]interface{}{
"common_name": "localhost",
"csr": csrPem,
})
if err != nil {
t.Fatal(err)
}
}

func TestBackend_URI_SANs(t *testing.T) {
t.Parallel()
b, s := CreateBackendWithStorage(t)
Expand Down Expand Up @@ -3662,6 +3727,7 @@ func TestReadWriteDeleteRoles(t *testing.T) {
expectedData := map[string]interface{}{
"key_type": "rsa",
"use_csr_sans": true,
"use_csr_serial_number": true,
"client_flag": true,
"allowed_serial_numbers": []interface{}{},
"generate_lease": false,
Expand Down
2 changes: 1 addition & 1 deletion builtin/logical/pki/issuing/issue_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ 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 {
if role.UseCSRSerialNumber && ridSerialNumber == "" && csr != nil {
ridSerialNumber = csr.Subject.SerialNumber
}

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"`
UseCSRSerialNumber bool `json:"use_csr_serial_number"`
KeyType string `json:"key_type"`
KeyBits int `json:"key_bits"`
UsePSS bool `json:"use_pss"`
Expand Down Expand Up @@ -113,6 +114,7 @@ func (r *RoleEntry) ToResponseData() map[string]interface{} {
"email_protection_flag": r.EmailProtectionFlag,
"use_csr_common_name": r.UseCSRCommonName,
"use_csr_sans": r.UseCSRSANs,
"use_csr_serial_number": r.UseCSRSerialNumber,
"key_type": r.KeyType,
"key_bits": r.KeyBits,
"signature_bits": r.SignatureBits,
Expand Down Expand Up @@ -374,6 +376,7 @@ func SignVerbatimRoleWithOpts(opts ...RoleModifier) *RoleEntry {
KeyType: "any",
UseCSRCommonName: true,
UseCSRSANs: true,
UseCSRSerialNumber: true,
AllowedOtherSANs: []string{"*"},
AllowedSerialNumbers: []string{"*"},
AllowedURISANs: []string{"*"},
Expand Down
3 changes: 3 additions & 0 deletions builtin/logical/pki/path_issue_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,9 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d
if role.UseCSRSANs && data.Get("alt_names").(string) != "" {
resp.AddWarning("the alt_names field was provided but the role is set with \"use_csr_sans\" set to true")
}
if role.UseCSRSerialNumber && data.Get("serial_number").(string) != "" {
resp.AddWarning("the serial_number field was provided but the role is set with \"use_csr_serial_number\" set to true")
}
}

resp = addWarnings(resp, warnings)
Expand Down
20 changes: 20 additions & 0 deletions builtin/logical/pki/path_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,13 @@ include the Common Name (cn); use use_csr_common_name
for that. Defaults to true.`,
},

"use_csr_serial_number": {
Type: framework.TypeBool,
Required: true,
Description: `If set, when used with a signing profile,
the serial number from the CSR's subject will be used. Defaults to true.`,
},

"ou": {
Type: framework.TypeCommaStringSlice,
Description: `If set, OU (OrganizationalUnit) will be set to
Expand Down Expand Up @@ -674,6 +681,17 @@ for that. Defaults to true.`,
},
},

"use_csr_serial_number": {
Type: framework.TypeBool,
Default: true,
Description: `If set, when used with a signing profile,
the serial number from the CSR's subject will be used. Defaults to true.`,
DisplayAttrs: &framework.DisplayAttributes{
Name: "Use CSR Subject Serial Number",
Value: true,
},
},

"ou": {
Type: framework.TypeCommaStringSlice,
Description: `If set, OU (OrganizationalUnit) will be set to
Expand Down Expand Up @@ -962,6 +980,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),
UseCSRSerialNumber: data.Get("use_csr_serial_number").(bool),
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 @@ -1162,6 +1181,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),
UseCSRSerialNumber: getWithExplicitDefault(data, "use_csr_serial_number", oldEntry.UseCSRSerialNumber).(bool),
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: "use_csr_serial_number",
Before: false,
Patched: true,
},
{
Field: "ou",
Before: []string{"crypto"},
Expand Down
3 changes: 3 additions & 0 deletions changelog/25709.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
COMPONENT: Add `use_csr_serial_number` to PKI roles
```
Loading