letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

cert-checker: add support for ipAddress SANs (#8188) 

In cert-checker, inspect both the DNS Names and the IP Addresses
contained within the certificate being examined. Also add a check that
no other kinds of SANs exist in the certificate.

Fixes https://github.com/letsencrypt/boulder/issues/8183
This commit is contained in:
Aaron Gable 2025-05-16 16:22:56 -07:00 committed by GitHub
parent aaaf623d49
commit ac2dae70f2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 175 additions and 142 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

293
cmd/cert-checker/main.go
View File

@ -8,6 +8,7 @@ import (
"encoding/json" "encoding/json"
"flag" "flag"
"fmt" "fmt"
"net/netip"
"os" "os"
"regexp" "regexp"
"slices" "slices"
@ -78,7 +79,7 @@ func (r *report) dump() error {
type reportEntry struct { type reportEntry struct {
Valid bool `json:"valid"` Valid bool `json:"valid"`
DNSNames []string `json:"dnsNames"` SANs []string `json:"sans"`
Problems []string `json:"problems,omitempty"` Problems []string `json:"problems,omitempty"`
} }
@ -258,13 +259,13 @@ func (c *certChecker) getCerts(ctx context.Context) error {
func (c *certChecker) processCerts(ctx context.Context, wg *sync.WaitGroup, badResultsOnly bool) { func (c *certChecker) processCerts(ctx context.Context, wg *sync.WaitGroup, badResultsOnly bool) {
for cert := range c.certs { for cert := range c.certs {
dnsNames, problems := c.checkCert(ctx, cert) sans, problems := c.checkCert(ctx, cert)
valid := len(problems) == 0 valid := len(problems) == 0
c.rMu.Lock() c.rMu.Lock()
if !badResultsOnly || (badResultsOnly && !valid) { if !badResultsOnly || (badResultsOnly && !valid) {
c.issuedReport.Entries[cert.Serial] = reportEntry{ c.issuedReport.Entries[cert.Serial] = reportEntry{
Valid: valid, Valid: valid,
DNSNames: dnsNames, SANs: sans,
Problems: problems, Problems: problems,
} }
} }
@ -333,164 +334,196 @@ func (c *certChecker) checkValidations(ctx context.Context, cert *corepb.Certifi
return nil return nil
} }
// checkCert returns a list of DNS names in the certificate and a list of problems with the certificate. // checkCert returns a list of Subject Alternative Names in the certificate and a list of problems with the certificate.
func (c *certChecker) checkCert(ctx context.Context, cert *corepb.Certificate) ([]string, []string) { func (c *certChecker) checkCert(ctx context.Context, cert *corepb.Certificate) ([]string, []string) {
var dnsNames []string
var problems []string var problems []string
// Check that the digests match. // Check that the digests match.
if cert.Digest != core.Fingerprint256(cert.Der) { if cert.Digest != core.Fingerprint256(cert.Der) {
problems = append(problems, "Stored digest doesn't match certificate digest") problems = append(problems, "Stored digest doesn't match certificate digest")
} }
// Parse the certificate. // Parse the certificate.
parsedCert, err := zX509.ParseCertificate(cert.Der) parsedCert, err := zX509.ParseCertificate(cert.Der)
if err != nil { if err != nil {
problems = append(problems, fmt.Sprintf("Couldn't parse stored certificate: %s", err)) problems = append(problems, fmt.Sprintf("Couldn't parse stored certificate: %s", err))
} else { // This is a fatal error, we can't do any further processing.
dnsNames = parsedCert.DNSNames return nil, problems
// Run zlint checks. }
results := zlint.LintCertificateEx(parsedCert, c.lints)
for name, res := range results.Results { // Now that it's parsed, we can extract the SANs.
if res.Status <= lint.Pass { sans := slices.Clone(parsedCert.DNSNames)
continue for _, ip := range parsedCert.IPAddresses {
} sans = append(sans, ip.String())
prob := fmt.Sprintf("zlint %s: %s", res.Status, name) }
if res.Details != "" {
prob = fmt.Sprintf("%s %s", prob, res.Details) // Run zlint checks.
} results := zlint.LintCertificateEx(parsedCert, c.lints)
problems = append(problems, prob) for name, res := range results.Results {
if res.Status <= lint.Pass {
continue
} }
// Check if stored serial is correct. prob := fmt.Sprintf("zlint %s: %s", res.Status, name)
storedSerial, err := core.StringToSerial(cert.Serial) if res.Details != "" {
prob = fmt.Sprintf("%s %s", prob, res.Details)
}
problems = append(problems, prob)
}
// Check if stored serial is correct.
storedSerial, err := core.StringToSerial(cert.Serial)
if err != nil {
problems = append(problems, "Stored serial is invalid")
} else if parsedCert.SerialNumber.Cmp(storedSerial) != 0 {
problems = append(problems, "Stored serial doesn't match certificate serial")
}
// Check that we have the correct expiration time.
if !parsedCert.NotAfter.Equal(cert.Expires.AsTime()) {
problems = append(problems, "Stored expiration doesn't match certificate NotAfter")
}
// Check if basic constraints are set.
if !parsedCert.BasicConstraintsValid {
problems = append(problems, "Certificate doesn't have basic constraints set")
}
// Check that the cert isn't able to sign other certificates.
if parsedCert.IsCA {
problems = append(problems, "Certificate can sign other certificates")
}
// Check that the cert has a valid validity period. The validity
// period is computed inclusive of the whole final second indicated by
// notAfter.
validityDuration := parsedCert.NotAfter.Add(time.Second).Sub(parsedCert.NotBefore)
_, ok := c.acceptableValidityDurations[validityDuration]
if !ok {
problems = append(problems, "Certificate has unacceptable validity period")
}
// Check that the stored issuance time isn't too far back/forward dated.
if parsedCert.NotBefore.Before(cert.Issued.AsTime().Add(-6*time.Hour)) || parsedCert.NotBefore.After(cert.Issued.AsTime().Add(6*time.Hour)) {
problems = append(problems, "Stored issuance date is outside of 6 hour window of certificate NotBefore")
}
// Check that the cert doesn't contain any SANs of unexpected types.
if len(parsedCert.EmailAddresses) != 0 || len(parsedCert.URIs) != 0 {
problems = append(problems, "Certificate contains SAN of unacceptable type (email or URI)")
}
if parsedCert.Subject.CommonName != "" {
// Check if the CommonName is <= 64 characters.
if len(parsedCert.Subject.CommonName) > 64 {
problems = append(
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(sans, 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 DNS name and IP
// address in the SANs. We do not check the CommonName here, as (if it exists)
// we already checked that it is identical to one of the DNSNames in the SAN.
for _, name := range parsedCert.DNSNames {
err = c.pa.WillingToIssue(identifier.ACMEIdentifiers{identifier.NewDNS(name)})
if err != nil { if err != nil {
problems = append(problems, "Stored serial is invalid") problems = append(problems, fmt.Sprintf("Policy Authority isn't willing to issue for '%s': %s", name, err))
} else if parsedCert.SerialNumber.Cmp(storedSerial) != 0 { continue
problems = append(problems, "Stored serial doesn't match certificate serial")
} }
// Check that we have the correct expiration time. // For defense-in-depth, even if the PA was willing to issue for a name
if !parsedCert.NotAfter.Equal(cert.Expires.AsTime()) { // we double check it against a list of forbidden domains. This way even
problems = append(problems, "Stored expiration doesn't match certificate NotAfter") // if the hostnamePolicyFile malfunctions we will flag the forbidden
// domain matches
if forbidden, pattern := isForbiddenDomain(name); forbidden {
problems = append(problems, fmt.Sprintf(
"Policy Authority was willing to issue but domain '%s' matches "+
"forbiddenDomains entry %q", name, pattern))
} }
// Check if basic constraints are set. }
if !parsedCert.BasicConstraintsValid { for _, name := range parsedCert.IPAddresses {
problems = append(problems, "Certificate doesn't have basic constraints set") ip, ok := netip.AddrFromSlice(name)
}
// Check that the cert isn't able to sign other certificates.
if parsedCert.IsCA {
problems = append(problems, "Certificate can sign other certificates")
}
// Check that the cert has a valid validity period. The validity
// period is computed inclusive of the whole final second indicated by
// notAfter.
validityDuration := parsedCert.NotAfter.Add(time.Second).Sub(parsedCert.NotBefore)
_, ok := c.acceptableValidityDurations[validityDuration]
if !ok { if !ok {
problems = append(problems, "Certificate has unacceptable validity period") problems = append(problems, fmt.Sprintf("SANs contain malformed IP %q", name))
continue
} }
// Check that the stored issuance time isn't too far back/forward dated. err = c.pa.WillingToIssue(identifier.ACMEIdentifiers{identifier.NewIP(ip)})
if parsedCert.NotBefore.Before(cert.Issued.AsTime().Add(-6*time.Hour)) || parsedCert.NotBefore.After(cert.Issued.AsTime().Add(6*time.Hour)) {
problems = append(problems, "Stored issuance date is outside of 6 hour window of certificate NotBefore")
}
if parsedCert.Subject.CommonName != "" {
// Check if the CommonName is <= 64 characters.
if len(parsedCert.Subject.CommonName) > 64 {
problems = append(
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.
// We do not check the CommonName here, as (if it exists) we already checked
// that it is identical to one of the DNSNames in the SAN.
//
// TODO(#7311): We'll need to iterate over IP address identifiers too.
for _, name := range parsedCert.DNSNames {
err = c.pa.WillingToIssue(identifier.ACMEIdentifiers{identifier.NewDNS(name)})
if err != nil {
problems = append(problems, fmt.Sprintf("Policy Authority isn't willing to issue for '%s': %s", name, err))
} else {
// For defense-in-depth, even if the PA was willing to issue for a name
// we double check it against a list of forbidden domains. This way even
// if the hostnamePolicyFile malfunctions we will flag the forbidden
// domain matches
if forbidden, pattern := isForbiddenDomain(name); forbidden {
problems = append(problems, fmt.Sprintf(
"Policy Authority was willing to issue but domain '%s' matches "+
"forbiddenDomains entry %q", name, pattern))
}
}
}
// Check the cert has the correct key usage extensions
serverAndClient := slices.Equal(parsedCert.ExtKeyUsage, []zX509.ExtKeyUsage{zX509.ExtKeyUsageServerAuth, zX509.ExtKeyUsageClientAuth})
serverOnly := slices.Equal(parsedCert.ExtKeyUsage, []zX509.ExtKeyUsage{zX509.ExtKeyUsageServerAuth})
if !(serverAndClient || serverOnly) {
problems = append(problems, "Certificate has incorrect key usage extensions")
}
for _, ext := range parsedCert.Extensions {
_, ok := allowedExtensions[ext.Id.String()]
if !ok {
problems = append(problems, fmt.Sprintf("Certificate contains an unexpected extension: %s", ext.Id))
}
expectedContent, ok := expectedExtensionContent[ext.Id.String()]
if ok {
if !bytes.Equal(ext.Value, expectedContent) {
problems = append(problems, fmt.Sprintf("Certificate extension %s contains unexpected content: has %x, expected %x", ext.Id, ext.Value, expectedContent))
}
}
}
// Check that the cert has a good key. Note that this does not perform
// checks which rely on external resources such as weak or blocked key
// lists, or the list of blocked keys in the database. This only performs
// static checks, such as against the RSA key size and the ECDSA curve.
p, err := x509.ParseCertificate(cert.Der)
if err != nil { if err != nil {
problems = append(problems, fmt.Sprintf("Couldn't parse stored certificate: %s", err)) problems = append(problems, fmt.Sprintf("Policy Authority isn't willing to issue for '%s': %s", name, err))
continue
} }
}
// Check the cert has the correct key usage extensions
serverAndClient := slices.Equal(parsedCert.ExtKeyUsage, []zX509.ExtKeyUsage{zX509.ExtKeyUsageServerAuth, zX509.ExtKeyUsageClientAuth})
serverOnly := slices.Equal(parsedCert.ExtKeyUsage, []zX509.ExtKeyUsage{zX509.ExtKeyUsageServerAuth})
if !(serverAndClient || serverOnly) {
problems = append(problems, "Certificate has incorrect key usage extensions")
}
for _, ext := range parsedCert.Extensions {
_, ok := allowedExtensions[ext.Id.String()]
if !ok {
problems = append(problems, fmt.Sprintf("Certificate contains an unexpected extension: %s", ext.Id))
}
expectedContent, ok := expectedExtensionContent[ext.Id.String()]
if ok {
if !bytes.Equal(ext.Value, expectedContent) {
problems = append(problems, fmt.Sprintf("Certificate extension %s contains unexpected content: has %x, expected %x", ext.Id, ext.Value, expectedContent))
}
}
}
// Check that the cert has a good key. Note that this does not perform
// checks which rely on external resources such as weak or blocked key
// lists, or the list of blocked keys in the database. This only performs
// static checks, such as against the RSA key size and the ECDSA curve.
p, err := x509.ParseCertificate(cert.Der)
if err != nil {
problems = append(problems, fmt.Sprintf("Couldn't parse stored certificate: %s", err))
} else {
err = c.kp.GoodKey(ctx, p.PublicKey) err = c.kp.GoodKey(ctx, p.PublicKey)
if err != nil { if err != nil {
problems = append(problems, fmt.Sprintf("Key Policy isn't willing to issue for public key: %s", err)) problems = append(problems, fmt.Sprintf("Key Policy isn't willing to issue for public key: %s", err))
} }
}
precertDER, err := c.getPrecert(ctx, cert.Serial) precertDER, err := c.getPrecert(ctx, cert.Serial)
if err != nil {
// Log and continue, since we want the problems slice to only contains
// problems with the cert itself.
c.logger.Errf("fetching linting precertificate for %s: %s", cert.Serial, err)
atomic.AddInt64(&c.issuedReport.DbErrs, 1)
} else {
err = precert.Correspond(precertDER, cert.Der)
if err != nil { if err != nil {
// Log and continue, since we want the problems slice to only contains problems = append(problems, fmt.Sprintf("Certificate does not correspond to precert for %s: %s", cert.Serial, err))
// problems with the cert itself.
c.logger.Errf("fetching linting precertificate for %s: %s", cert.Serial, err)
atomic.AddInt64(&c.issuedReport.DbErrs, 1)
} else {
err = precert.Correspond(precertDER, cert.Der)
if err != nil {
problems = append(problems,
fmt.Sprintf("Certificate does not correspond to precert for %s: %s", cert.Serial, err))
}
} }
}
if features.Get().CertCheckerChecksValidations { if features.Get().CertCheckerChecksValidations {
idents := identifier.FromCert(p) idents := identifier.FromCert(p)
err = c.checkValidations(ctx, cert, idents) err = c.checkValidations(ctx, cert, idents)
if err != nil { if err != nil {
if features.Get().CertCheckerRequiresValidations { if features.Get().CertCheckerRequiresValidations {
problems = append(problems, err.Error()) problems = append(problems, err.Error())
} else { } else {
var identValues []string var identValues []string
for _, ident := range idents { for _, ident := range idents {
identValues = append(identValues, ident.Value) identValues = append(identValues, ident.Value)
}
c.logger.Errf("Certificate %s %s: %s", cert.Serial, identValues, err)
} }
c.logger.Errf("Certificate %s %s: %s", cert.Serial, identValues, err)
} }
} }
} }
return dnsNames, problems
return sans, problems
} }
type Config struct { type Config struct {

4
cmd/cert-checker/main_test.go
View File

@ -143,7 +143,7 @@ func TestCheckWildcardCert(t *testing.T) {
} }
} }
func TestCheckCertReturnsDNSNames(t *testing.T) { func TestCheckCertReturnsSANs(t *testing.T) {
saDbMap, err := sa.DBMapForTest(vars.DBConnSA) saDbMap, err := sa.DBMapForTest(vars.DBConnSA)
test.AssertNotError(t, err, "Couldn't connect to database") test.AssertNotError(t, err, "Couldn't connect to database")
saCleanup := test.ResetBoulderTestDatabase(t) saCleanup := test.ResetBoulderTestDatabase(t)
@ -171,7 +171,7 @@ func TestCheckCertReturnsDNSNames(t *testing.T) {
} }
names, problems := checker.checkCert(context.Background(), cert) names, problems := checker.checkCert(context.Background(), cert)
if !slices.Equal(names, []string{"quite_invalid.com", "al--so--wr--ong.com"}) { if !slices.Equal(names, []string{"quite_invalid.com", "al--so--wr--ong.com", "127.0.0.1"}) {
t.Errorf("didn't get expected DNS names. other problems: %s", strings.Join(problems, "\n")) t.Errorf("didn't get expected DNS names. other problems: %s", strings.Join(problems, "\n"))
} }
} }

20
cmd/cert-checker/testdata/quite_invalid.pem vendored
View File

@ -1,5 +1,5 @@
-----BEGIN CERTIFICATE----- -----BEGIN CERTIFICATE-----
MIIDUzCCAjugAwIBAgIILgLqdMwyzT4wDQYJKoZIhvcNAQELBQAwIDEeMBwGA1UE MIIDWTCCAkGgAwIBAgIILgLqdMwyzT4wDQYJKoZIhvcNAQELBQAwIDEeMBwGA1UE
AxMVbWluaWNhIHJvb3QgY2EgOTMzZTM5MB4XDTIxMTExMTIwMjMzMloXDTIzMTIx AxMVbWluaWNhIHJvb3QgY2EgOTMzZTM5MB4XDTIxMTExMTIwMjMzMloXDTIzMTIx
MTIwMjMzMlowHDEaMBgGA1UEAwwRcXVpdGVfaW52YWxpZC5jb20wggEiMA0GCSqG MTIwMjMzMlowHDEaMBgGA1UEAwwRcXVpdGVfaW52YWxpZC5jb20wggEiMA0GCSqG
SIb3DQEBAQUAA4IBDwAwggEKAoIBAQDi4jBbqMyvhMonDngNsvie9SHPB16mdpiy SIb3DQEBAQUAA4IBDwAwggEKAoIBAQDi4jBbqMyvhMonDngNsvie9SHPB16mdpiy
@ -7,14 +7,14 @@ Y/agreU84xUz/roKK07TpVmeqvwWvDkvHTFov7ytKdnCY+z/NXKJ3hNqflWCwU7h
Uk9TmpBp0vg+5NvalYul/+bq/B4qDhEvTBzAX3k/UYzd0GQdMyAbwXtG41f5cSK6 Uk9TmpBp0vg+5NvalYul/+bq/B4qDhEvTBzAX3k/UYzd0GQdMyAbwXtG41f5cSK6
cWTQYfJL3gGR5/KLoTz3/VemLgEgAP/CvgcUJPbQceQViiZ4opi9hFIfUqxX2NsD cWTQYfJL3gGR5/KLoTz3/VemLgEgAP/CvgcUJPbQceQViiZ4opi9hFIfUqxX2NsD
49klw8cDFu/BG2LEC+XtbdT8XevD0aGIOuYVr+Pa2mxb2QCDXu4tXOsDXH9Y/Cmk 49klw8cDFu/BG2LEC+XtbdT8XevD0aGIOuYVr+Pa2mxb2QCDXu4tXOsDXH9Y/Cmk
8103QbdB8Y+usOiHG/IXxK2q4J7QNPal4ER4/PGA06V0gwrjNH8BAgMBAAGjgZQw 8103QbdB8Y+usOiHG/IXxK2q4J7QNPal4ER4/PGA06V0gwrjNH8BAgMBAAGjgZow
gZEwDgYDVR0PAQH/BAQDAgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcD gZcwDgYDVR0PAQH/BAQDAgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcD
AjAMBgNVHRMBAf8EAjAAMB8GA1UdIwQYMBaAFNIcaCjv32YRafE065dZO57ONWuk AjAMBgNVHRMBAf8EAjAAMB8GA1UdIwQYMBaAFNIcaCjv32YRafE065dZO57ONWuk
MDEGA1UdEQQqMCiCEXF1aXRlX2ludmFsaWQuY29tghNhbC0tc28tLXdyLS1vbmcu MDcGA1UdEQQwMC6CEXF1aXRlX2ludmFsaWQuY29tghNhbC0tc28tLXdyLS1vbmcu
Y29tMA0GCSqGSIb3DQEBCwUAA4IBAQAjSv0o5G4VuLnnwHON4P53bLvGnYqaqYju Y29thwR/AAABMA0GCSqGSIb3DQEBCwUAA4IBAQAjSv0o5G4VuLnnwHON4P53bLvG
TEafi3hSgHAfBuhOQUVgwujoYpPp1w1fm5spfcbSwNNRte79HgV97kAuZ4R4RHk1 nYqaqYjuTEafi3hSgHAfBuhOQUVgwujoYpPp1w1fm5spfcbSwNNRte79HgV97kAu
5Xux1ITLalaHR/ilu002N0eJ7dFYawBgV2xMudULzohwmW2RjPJ5811iWwtiVf1b Z4R4RHk15Xux1ITLalaHR/ilu002N0eJ7dFYawBgV2xMudULzohwmW2RjPJ5811i
A3V5SZJWSJll1BhANBs7R0pBbyTSNHR470N8TGG0jfXqgTKd0xZaH91HrwEMo+96 WwtiVf1bA3V5SZJWSJll1BhANBs7R0pBbyTSNHR470N8TGG0jfXqgTKd0xZaH91H
llbfp90Y5OfHIfym/N1sH2hVgd+ZAkhiVEiNBWZlbSyOgbZ1cCBvBXg6TuwpQMZK rwEMo+96llbfp90Y5OfHIfym/N1sH2hVgd+ZAkhiVEiNBWZlbSyOgbZ1cCBvBXg6
9RWjlpni8yuzLGduPl8qHG1dqsUvbVqcG+WhHLbaZMNhiMfiWInL TuwpQMZK9RWjlpni8yuzLGduPl8qHG1dqsUvbVqcG+WhHLbaZMNhiMfiWInL
-----END CERTIFICATE----- -----END CERTIFICATE-----