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.

Sign up for GitHub

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

added a flag to make common name optional if desired #3940

Merged
merged 5 commits into from
Feb 9, 2018
Merged

added a flag to make common name optional if desired #3940

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
adff74e
added a flag to make common name optional if desired
vishalnayak Feb 8, 2018
5de071a
Cover one more case where cn can be empty
vishalnayak Feb 8, 2018
efdf0d9
remove skipping when empty; instead check for emptiness before callin…
vishalnayak Feb 8, 2018
d95977b
Merge branch 'master' into pki-cn
jefferai Feb 9, 2018
ae579f2
Add verification before adding to DNS names to also fix #3918
jefferai Feb 9, 2018
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
20 changes: 17 additions & 3 deletions builtin/logical/pki/backend_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,17 @@ func TestPKI_RequireCN(t *testing.T) {
"max_ttl": "2h",
})

// Issue a cert with require_cn set to true and with common name supplied
// Issue a cert with require_cn set to true and with common name supplied.
// It should succeed.
resp, err = client.Logical().Write("pki/issue/example", map[string]interface{}{
"common_name": "foobar.com",
})
if err != nil {
t.Fatal(err)
}

// Issue a cert with require_cn set to true and with out supplying the common name
// Issue a cert with require_cn set to true and with out supplying the
// common name. It should error out.
resp, err = client.Logical().Write("pki/issue/example", map[string]interface{}{})
if err == nil {
t.Fatalf("expected an error due to missing common_name")
Expand All @@ -107,7 +109,19 @@ func TestPKI_RequireCN(t *testing.T) {
"require_cn": false,
})

// Issue a cert with require_cn set to true and with out supplying the common name
// Issue a cert with require_cn set to false and without supplying the
// common name. It should succeed.
resp, err = client.Logical().Write("pki/issue/example", map[string]interface{}{})
if err != nil {
t.Fatal(err)
}

if resp.Data["certificate"] == "" {
t.Fatalf("expected a cert to be generated")
}

// Issue a cert with require_cn set to false and with a common name. It
// should succeed.
resp, err = client.Logical().Write("pki/issue/example", map[string]interface{}{})
if err != nil {
t.Fatal(err)
Expand Down
10 changes: 7 additions & 3 deletions builtin/logical/pki/cert_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,10 @@ func fetchCertBySerial(ctx context.Context, req *logical.Request, prefix, serial
// If one does not pass, it is returned in the string argument.
func validateNames(req *logical.Request, names []string, role *roleEntry) string {
for _, name := range names {
if name == "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should happen... Don't see a reason why we would want empty names in that list.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

// This might happen when common_name is empty
continue
}
sanitizedName := name
emailDomain := name
isEmail := false
Expand Down Expand Up @@ -621,9 +625,9 @@ func generateCreationBundle(b *backend,
if csr != nil && role.UseCSRCommonName {
cn = csr.Subject.CommonName
}
if cn == "" && role.RequireCN {
if cn == "" {
cn = data.Get("common_name").(string)
if cn == "" {
if cn == "" && role.RequireCN {
return nil, errutil.UserError{Err: `the common_name field is required, or must be provided in a CSR with "use_csr_common_name" set to true, unless "require_cn" is set to false`}
}
}
Expand All @@ -633,7 +637,7 @@ func generateCreationBundle(b *backend,
emailAddresses = csr.EmailAddresses
}

if !data.Get("exclude_cn_from_sans").(bool) {
if cn != "" && !data.Get("exclude_cn_from_sans").(bool) {
if strings.Contains(cn, "@") {
// Note: emails are not disallowed if the role's email protection
// flag is false, because they may well be included for
Expand Down