Skip to content

Add CSR common name to SANs if no other SANs are defined in CSR #1172

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

Merged
dopey merged 6 commits into from
May 9, 2024
Merged

Add CSR common name to SANs if no other SANs are defined in CSR #1172

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
22dce0e
Add CSR common name to SANs if no other SANs are defined in CSR
dopey May 7, 2024
9e6ac49
Add flag to enable and disable default CN -> SAN behavior
dopey May 7, 2024
542a219
omit-cn-san
dopey May 7, 2024
c870540
Merge branch 'master' into max/common-name-name
dopey May 7, 2024
25633cc
Update command/certificate/sign.go
dopey May 7, 2024
71a8a36
Do not add empty common name to SANs
dopey May 9, 2024
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
32 changes: 28 additions & 4 deletions command/certificate/sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ func signCommand() cli.Command {
Action: cli.ActionFunc(signAction),
Usage: "sign a certificate signing request (CSR)",
UsageText: `**step certificate sign** <csr-file> <crt-file> <key-file>
[**--profile**=<profile>] [**--template**=<file>]
[**--set**=<key=value>] [**--set-file**=<file>]
[**--profile**=<profile>] [**--template**=<file>]
[**--set**=<key=value>] [**--set-file**=<file>] [**--add-cn-san**]
[**--password-file**=<file>] [**--path-len**=<maximum>]
[**--not-before**=<time|duration>] [**--not-after**=<time|duration>]
[**--bundle**]`,
Expand Down Expand Up @@ -79,6 +79,11 @@ Sign a CSR with custom validity and bundle the new certificate with the issuer:
$ step certificate sign --bundle --not-before -1m --not-after 16h leaf.csr issuer.crt issuer.key
'''

Sign a CSR but do not add the Common Name to the SANs extension of the certificate:
'''
$ step certificate sign --add-cn-san=false leaf.csr issuer.crt issuer.key
'''

Sign an intermediate ca:
'''
$ step certificate sign --profile intermediate-ca intermediate.csr issuer.crt issuer.key
Expand Down Expand Up @@ -174,6 +179,12 @@ $ step certificate sign \
flags.Template,
flags.TemplateSet,
flags.TemplateSetFile,
cli.BoolTFlag{
Name: "add-cn-san",
Usage: `Ensure that Common Name from CSR subject is added to Subject Alternative
Name (SAN) extension of the certificate. This flag is enabled by default. To
disable default behavior pass the flag as '--add-cn-san=false'.`,
},

@maraino maraino May 7, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's do something like --omit-cn-san as @hslatman suggested.

flags.PasswordFile,
cli.StringFlag{
Name: "not-before",
Expand Down Expand Up @@ -327,7 +338,7 @@ func signAction(ctx *cli.Context) error {
}

// Create certificate template from csr.
data := createTemplateData(csr, maxPathLen)
data := createTemplateData(csr, maxPathLen, ctx.Bool("add-cn-san"))
data.SetUserData(userData)
tpl, err := x509util.NewCertificate(csr, x509util.WithTemplate(template, data))
if err != nil {
Expand Down Expand Up @@ -424,7 +435,7 @@ func validateIssuer(crt *x509.Certificate, profile string, maxPathLen int) error
// createTemplateData create a new template data with subject and sans based on
// the information in the certificate request, and the maxPathLen for
// intermediate certificates.
func createTemplateData(cr *x509.CertificateRequest, maxPathLen int) x509util.TemplateData {
func createTemplateData(cr *x509.CertificateRequest, maxPathLen int, addCNSAN bool) x509util.TemplateData {
var sans []string
sans = append(sans, cr.DNSNames...)
sans = append(sans, cr.EmailAddresses...)
Expand All @@ -435,6 +446,19 @@ func createTemplateData(cr *x509.CertificateRequest, maxPathLen int) x509util.Te
sans = append(sans, v.String())
}

if addCNSAN {
cnInSANs := false
for _, v := range sans {
if v == cr.Subject.CommonName {
cnInSANs = true
break
}
}
if !cnInSANs {
sans = append(sans, cr.Subject.CommonName)
}
Comment thread
dopey marked this conversation as resolved.
Outdated
}

@maraino maraino May 7, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we want to do this only if there are no SANs.


data := x509util.NewTemplateData()
data.SetCertificateRequest(cr)
data.Set("MaxPathLen", maxPathLen)
Expand Down