diff --git a/authority/authority.go b/authority/authority.go index 98dd68968..c894edbad 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -450,6 +450,16 @@ func (a *Authority) init() error { if len(a.intermediateX509Certs) == 0 { a.intermediateX509Certs = append(a.intermediateX509Certs, options.CertificateChain...) } + + // Validate that neither config.commonName nor authority.template.commonName + // collides with the issuing CA's CommonName. If they match, issued + // certificates (or the CA's own TLS certificate) will have Subject == Issuer + // and be incorrectly treated as self-signed by TLS stacks and validators. + if len(a.intermediateX509Certs) > 0 { + if err := a.validateCommonNames(a.intermediateX509Certs[0].Subject.CommonName); err != nil { + return err + } + } } a.x509CAService, err = cas.New(ctx, options) if err != nil { @@ -875,6 +885,28 @@ func (a *Authority) initLogf(format string, v ...any) { } } +// validateCommonNames returns an error if config.commonName or +// authority.template.commonName equals the issuing CA certificate's CommonName. +// When Subject.CommonName equals Issuer.CommonName the certificate looks +// self-signed to TLS stacks and certificate validators, which causes +// handshake failures and trust errors. +func (a *Authority) validateCommonNames(issuerCN string) error { + if issuerCN == "" { + return nil + } + if a.config.CommonName == issuerCN { + return fmt.Errorf("config commonName %q must not be equal to the issuing CA "+ + "certificate's CommonName; the CA's own TLS certificate would appear "+ + "self-signed — use a distinct name or leave commonName empty to use the default", issuerCN) + } + if tmpl := a.config.AuthorityConfig.Template; tmpl != nil && tmpl.CommonName == issuerCN { + return fmt.Errorf("authority.template.commonName %q must not be equal to the "+ + "issuing CA certificate's CommonName; issued leaf certificates would appear "+ + "self-signed — use a distinct name or remove authority.template.commonName", issuerCN) + } + return nil +} + // GetID returns the define authority id or a zero uuid. func (a *Authority) GetID() string { const zeroUUID = "00000000-0000-0000-0000-000000000000" diff --git a/authority/authority_test.go b/authority/authority_test.go index dca6cf14f..d2fcae5af 100644 --- a/authority/authority_test.go +++ b/authority/authority_test.go @@ -158,6 +158,31 @@ func TestAuthorityNew(t *testing.T) { err: errors.New(`error reading "wrong": no such file or directory`), } }, + "fail config commonName equals issuer CN": func(t *testing.T) *newTest { + c, err := LoadConfiguration("../ca/testdata/ca.json") + assert.FatalError(t, err) + // Intermediate CA in testdata has CN "smallstep Intermediate CA". + // Setting Config.CommonName to the same value would make the CA's + // own TLS certificate appear self-signed. + c.CommonName = "smallstep Intermediate CA" + return &newTest{ + config: c, + err: errors.New(`config commonName "smallstep Intermediate CA" must not be equal to the issuing CA certificate's CommonName`), + } + }, + "fail template commonName equals issuer CN": func(t *testing.T) *newTest { + c, err := LoadConfiguration("../ca/testdata/ca.json") + assert.FatalError(t, err) + // Setting authority.template.commonName to the intermediate CA's CN + // would make every issued leaf certificate appear self-signed. + c.AuthorityConfig.Template = &ASN1DN{ + CommonName: "smallstep Intermediate CA", + } + return &newTest{ + config: c, + err: errors.New(`authority.template.commonName "smallstep Intermediate CA" must not be equal to the issuing CA certificate's CommonName`), + } + }, } for name, genTestCase := range tests {