cert-checker: allow for empty common name (#7157)
Update cert-checker's logic to only ask the Policy Authority if we are willing to issue for the Subject Alternative Names in a cert, to avoid asking the PA about the Common Name when that CN is just the empty string. Add a new check to cert-checker to ensure that the Common Name is exactly equal to one of the SANs, to fill a theoretical gap created by loosening the check above. Fixes https://github.com/letsencrypt/boulder/issues/7156
This commit is contained in:
parent
279a4d539d
commit
6c9153aa76
|
|
@ -389,16 +389,25 @@ func (c *certChecker) checkCert(ctx context.Context, cert core.Certificate, igno
|
||||||
if parsedCert.NotBefore.Before(cert.Issued.Add(-6*time.Hour)) || parsedCert.NotBefore.After(cert.Issued.Add(6*time.Hour)) {
|
if parsedCert.NotBefore.Before(cert.Issued.Add(-6*time.Hour)) || parsedCert.NotBefore.After(cert.Issued.Add(6*time.Hour)) {
|
||||||
problems = append(problems, "Stored issuance date is outside of 6 hour window of certificate NotBefore")
|
problems = append(problems, "Stored issuance date is outside of 6 hour window of certificate NotBefore")
|
||||||
}
|
}
|
||||||
// Check if the CommonName is <= 64 characters.
|
if parsedCert.Subject.CommonName != "" {
|
||||||
if len(parsedCert.Subject.CommonName) > 64 {
|
// Check if the CommonName is <= 64 characters.
|
||||||
problems = append(
|
if len(parsedCert.Subject.CommonName) > 64 {
|
||||||
problems,
|
problems = append(
|
||||||
fmt.Sprintf("Certificate has common name >64 characters long (%d)", len(parsedCert.Subject.CommonName)),
|
problems,
|
||||||
)
|
fmt.Sprintf("Certificate has common name >64 characters long (%d)", len(parsedCert.Subject.CommonName)),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check that the CommonName is included in the SANs.
|
||||||
|
if !slices.Contains(parsedCert.DNSNames, parsedCert.Subject.CommonName) {
|
||||||
|
problems = append(problems, fmt.Sprintf("Certificate Common Name does not appear in Subject Alternative Names: %q !< %v",
|
||||||
|
parsedCert.Subject.CommonName, parsedCert.DNSNames))
|
||||||
|
}
|
||||||
}
|
}
|
||||||
// Check that the PA is still willing to issue for each name in DNSNames
|
// Check that the PA is still willing to issue for each name in DNSNames.
|
||||||
// + CommonName.
|
// We do not check the CommonName here, as (if it exists) we already checked
|
||||||
for _, name := range append(parsedCert.DNSNames, parsedCert.Subject.CommonName) {
|
// that it is identical to one of the DNSNames in the SAN.
|
||||||
|
for _, name := range parsedCert.DNSNames {
|
||||||
id := identifier.ACMEIdentifier{Type: identifier.DNS, Value: name}
|
id := identifier.ACMEIdentifier{Type: identifier.DNS, Value: name}
|
||||||
err = c.pa.WillingToIssueWildcards([]identifier.ACMEIdentifier{id})
|
err = c.pa.WillingToIssueWildcards([]identifier.ACMEIdentifier{id})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
|
||||||
|
|
@ -241,13 +241,12 @@ func TestCheckCert(t *testing.T) {
|
||||||
NotBefore: issued,
|
NotBefore: issued,
|
||||||
NotAfter: goodExpiry.AddDate(0, 0, 1), // Period too long
|
NotAfter: goodExpiry.AddDate(0, 0, 1), // Period too long
|
||||||
DNSNames: []string{
|
DNSNames: []string{
|
||||||
// longName should be flagged along with the long CN
|
|
||||||
longName,
|
|
||||||
"example-a.com",
|
"example-a.com",
|
||||||
"foodnotbombs.mil",
|
"foodnotbombs.mil",
|
||||||
// `dev-myqnapcloud.com` is included because it is an exact private
|
// `dev-myqnapcloud.com` is included because it is an exact private
|
||||||
// entry on the public suffix list
|
// entry on the public suffix list
|
||||||
"dev-myqnapcloud.com",
|
"dev-myqnapcloud.com",
|
||||||
|
// don't include longName in the SANs, so the unique CN gets flagged
|
||||||
},
|
},
|
||||||
SerialNumber: serial,
|
SerialNumber: serial,
|
||||||
BasicConstraintsValid: false,
|
BasicConstraintsValid: false,
|
||||||
|
|
@ -283,6 +282,7 @@ func TestCheckCert(t *testing.T) {
|
||||||
"Certificate has incorrect key usage extensions": 1,
|
"Certificate has incorrect key usage extensions": 1,
|
||||||
"Certificate has common name >64 characters long (65)": 1,
|
"Certificate has common name >64 characters long (65)": 1,
|
||||||
"Certificate contains an unexpected extension: 1.3.3.7": 1,
|
"Certificate contains an unexpected extension: 1.3.3.7": 1,
|
||||||
|
"Certificate Common Name does not appear in Subject Alternative Names: \"eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeexample.com\" !< [example-a.com foodnotbombs.mil dev-myqnapcloud.com]": 1,
|
||||||
}
|
}
|
||||||
for _, p := range problems {
|
for _, p := range problems {
|
||||||
_, ok := problemsMap[p]
|
_, ok := problemsMap[p]
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue