Merge pull request #260 from anxolerd/feat-force-cn-if-empty

[Feature] Force CommonName for certificates from ACME provisioner

acme/order_test.go

@@ -1143,7 +1143,7 @@ func TestOrderFinalize(t *testing.T) {
 				csr: csr,
 				sa: &mockSignAuth{
 					sign: func(csr *x509.CertificateRequest, pops provisioner.Options, signOps ...provisioner.SignOption) ([]*x509.Certificate, error) {
-						assert.Equals(t, len(signOps), 4)
+						assert.Equals(t, len(signOps), 5)
 						return []*x509.Certificate{crt, inter}, nil
 					},
 				},
@@ -1192,7 +1192,7 @@ func TestOrderFinalize(t *testing.T) {
 				csr: csr,
 				sa: &mockSignAuth{
 					sign: func(csr *x509.CertificateRequest, pops provisioner.Options, signOps ...provisioner.SignOption) ([]*x509.Certificate, error) {
-						assert.Equals(t, len(signOps), 4)
+						assert.Equals(t, len(signOps), 5)
 						return []*x509.Certificate{crt, inter}, nil
 					},
 				},
@@ -1239,7 +1239,7 @@ func TestOrderFinalize(t *testing.T) {
 				csr: csr,
 				sa: &mockSignAuth{
 					sign: func(csr *x509.CertificateRequest, pops provisioner.Options, signOps ...provisioner.SignOption) ([]*x509.Certificate, error) {
-						assert.Equals(t, len(signOps), 4)
+						assert.Equals(t, len(signOps), 5)
 						return []*x509.Certificate{crt, inter}, nil
 					},
 				},

authority/provisioner/acme.go

@@ -15,6 +15,7 @@ type ACME struct {
 	Type    string  `json:"type"`
 	Name    string  `json:"name"`
 	Claims  *Claims `json:"claims,omitempty"`
+	ForceCN bool    `json:"forceCN,omitempty"`
 	claimer *Claimer
 }
 
@@ -67,6 +68,7 @@ func (p *ACME) AuthorizeSign(ctx context.Context, token string) ([]SignOption, e
 	return []SignOption{
 		// modifiers / withOptions
 		newProvisionerExtensionOption(TypeACME, p.Name, ""),
+		newForceCNOption(p.ForceCN),
 		profileDefaultDuration(p.claimer.DefaultTLSCertDuration()),
 		// validators
 		defaultPublicKeyValidator{},

authority/provisioner/acme_test.go

@@ -168,7 +168,7 @@ func TestACME_AuthorizeSign(t *testing.T) {
 				}
 			} else {
 				if assert.Nil(t, tc.err) && assert.NotNil(t, opts) {
-					assert.Len(t, 4, opts)
+					assert.Len(t, 5, opts)
 					for _, o := range opts {
 						switch v := o.(type) {
 						case *provisionerExtensionOption:
@@ -176,6 +176,8 @@ func TestACME_AuthorizeSign(t *testing.T) {
 							assert.Equals(t, v.Name, tc.p.GetName())
 							assert.Equals(t, v.CredentialID, "")
 							assert.Len(t, 0, v.KeyValuePairs)
+						case *forceCNOption:
+							assert.Equals(t, v.ForceCN, tc.p.ForceCN)
 						case profileDefaultDuration:
 							assert.Equals(t, time.Duration(v), tc.p.claimer.DefaultTLSCertDuration())
 						case defaultPublicKeyValidator:

authority/provisioner/sign_options.go

@@ -316,6 +316,32 @@ type stepProvisionerASN1 struct {
 	KeyValuePairs []string `asn1:"optional,omitempty"`
 }
 
+type forceCNOption struct {
+	ForceCN bool
+}
+
+func newForceCNOption(forceCN bool) *forceCNOption {
+	return &forceCNOption{forceCN}
+}
+
+func (o *forceCNOption) Option(Options) x509util.WithOption {
+	return func(p x509util.Profile) error {
+		if !o.ForceCN {
+			// Forcing CN is disabled, do nothing to certificate
+			return nil
+		}
+		crt := p.Subject()
+		if crt.Subject.CommonName == "" {
+			if len(crt.DNSNames) > 0 {
+				crt.Subject.CommonName = crt.DNSNames[0]
+			} else {
+				return errors.New("Cannot force CN, DNSNames is empty")
+			}
+		}
+		return nil
+	}
+}
+
 type provisionerExtensionOption struct {
 	Type          int
 	Name          string

authority/provisioner/sign_options_test.go

@@ -344,6 +344,88 @@ func Test_validityValidator_Valid(t *testing.T) {
 	}
 }
 
+func Test_forceCN_Option(t *testing.T) {
+	type test struct {
+		so    Options
+		fcn   forceCNOption
+		cert  *x509.Certificate
+		valid func(*x509.Certificate)
+		err   error
+	}
+
+	tests := map[string]func() test{
+		"ok/CN-not-forced": func() test {
+			return test{
+				fcn: forceCNOption{false},
+				so:  Options{},
+				cert: &x509.Certificate{
+					Subject:  pkix.Name{},
+					DNSNames: []string{"acme.example.com", "step.example.com"},
+				},
+				valid: func(cert *x509.Certificate) {
+					assert.Equals(t, cert.Subject.CommonName, "")
+				},
+			}
+		},
+		"ok/CN-forced-and-set": func() test {
+			return test{
+				fcn: forceCNOption{true},
+				so:  Options{},
+				cert: &x509.Certificate{
+					Subject: pkix.Name{
+						CommonName: "Some Common Name",
+					},
+					DNSNames: []string{"acme.example.com", "step.example.com"},
+				},
+				valid: func(cert *x509.Certificate) {
+					assert.Equals(t, cert.Subject.CommonName, "Some Common Name")
+				},
+			}
+		},
+		"ok/CN-forced-and-not-set": func() test {
+			return test{
+				fcn: forceCNOption{true},
+				so:  Options{},
+				cert: &x509.Certificate{
+					Subject:  pkix.Name{},
+					DNSNames: []string{"acme.example.com", "step.example.com"},
+				},
+				valid: func(cert *x509.Certificate) {
+					assert.Equals(t, cert.Subject.CommonName, "acme.example.com")
+				},
+			}
+		},
+		"fail/CN-forced-and-empty-DNSNames": func() test {
+			return test{
+				fcn: forceCNOption{true},
+				so:  Options{},
+				cert: &x509.Certificate{
+					Subject:  pkix.Name{},
+					DNSNames: []string{},
+				},
+				err: errors.New("Cannot force CN, DNSNames is empty"),
+			}
+		},
+	}
+
+	for name, run := range tests {
+		t.Run(name, func(t *testing.T) {
+			tt := run()
+			prof := &x509util.Leaf{}
+			prof.SetSubject(tt.cert)
+			if err := tt.fcn.Option(tt.so)(prof); err != nil {
+				if assert.NotNil(t, tt.err) {
+					assert.HasPrefix(t, err.Error(), tt.err.Error())
+				}
+			} else {
+				if assert.Nil(t, tt.err) {
+					tt.valid(prof.Subject())
+				}
+			}
+		})
+	}
+}
+
 func Test_profileDefaultDuration_Option(t *testing.T) {
 	type test struct {
 		so    Options