Cert-checker: Update certlint, add CN/SAN==PSL err ignore. (#3600)

* Update `globalsign/certlint` to d4a45be.

This commit updates the `github.com/globalsign/certlint` dependency to
the latest tip of master (d4a45be06892f3e664f69892aca79a48df510be0).

Unit tests are confirmed to pass:
```
$ go test ./...
ok    github.com/globalsign/certlint  3.816s
ok    github.com/globalsign/certlint/asn1 (cached)
?     github.com/globalsign/certlint/certdata [no test files]
?     github.com/globalsign/certlint/checks [no test files]
?     github.com/globalsign/certlint/checks/certificate/aiaissuers  [no
test files]
?     github.com/globalsign/certlint/checks/certificate/all [no test
files]
?     github.com/globalsign/certlint/checks/certificate/basicconstraints
[no test files]
?     github.com/globalsign/certlint/checks/certificate/extensions  [no
test files]
?     github.com/globalsign/certlint/checks/certificate/extkeyusage [no
test files]
ok    github.com/globalsign/certlint/checks/certificate/internal
(cached)
?     github.com/globalsign/certlint/checks/certificate/issuerdn  [no
test files]
?     github.com/globalsign/certlint/checks/certificate/keyusage  [no
test files]
?     github.com/globalsign/certlint/checks/certificate/publickey [no
test files]
?     github.com/globalsign/certlint/checks/certificate/publickey/goodkey
[no test files]
ok    github.com/globalsign/certlint/checks/certificate/publicsuffix
(cached)
?     github.com/globalsign/certlint/checks/certificate/revocation  [no
test files]
?     github.com/globalsign/certlint/checks/certificate/serialnumber
[no test files]
?     github.com/globalsign/certlint/checks/certificate/signaturealgorithm
[no test files]
ok    github.com/globalsign/certlint/checks/certificate/subject (cached)
ok    github.com/globalsign/certlint/checks/certificate/subjectaltname
(cached)
?     github.com/globalsign/certlint/checks/certificate/validity  [no
test files]
?     github.com/globalsign/certlint/checks/certificate/version [no test
files]
?     github.com/globalsign/certlint/checks/certificate/wildcard  [no
test files]
?     github.com/globalsign/certlint/checks/extensions/adobetimestamp
[no test files]
?     github.com/globalsign/certlint/checks/extensions/all  [no test
files]
?     github.com/globalsign/certlint/checks/extensions/authorityinfoaccess
[no test files]
?     github.com/globalsign/certlint/checks/extensions/authoritykeyid
[no test files]
?     github.com/globalsign/certlint/checks/extensions/basicconstraints
[no test files]
?     github.com/globalsign/certlint/checks/extensions/crldistributionpoints
[no test files]
?     github.com/globalsign/certlint/checks/extensions/ct [no test
files]
?     github.com/globalsign/certlint/checks/extensions/extkeyusage  [no
test files]
?     github.com/globalsign/certlint/checks/extensions/keyusage [no test
files]
?     github.com/globalsign/certlint/checks/extensions/nameconstraints
[no test files]
ok    github.com/globalsign/certlint/checks/extensions/ocspmuststaple
(cached)
?     github.com/globalsign/certlint/checks/extensions/ocspnocheck  [no
test files]
?     github.com/globalsign/certlint/checks/extensions/pdfrevocation
[no test files]
?     github.com/globalsign/certlint/checks/extensions/policyidentifiers
[no test files]
?     github.com/globalsign/certlint/checks/extensions/smimecapabilities
[no test files]
?     github.com/globalsign/certlint/checks/extensions/subjectaltname
[no test files]
?     github.com/globalsign/certlint/checks/extensions/subjectkeyid [no
test files]
ok    github.com/globalsign/certlint/errors (cached)
?     github.com/globalsign/certlint/examples/ct  [no test files]
?     github.com/globalsign/certlint/examples/specificchecks  [no test
files]
```

* Certchecker: Remove OCSP Must Staple err ignore, fix typos.

This commit removes the explicit ignore for OCSP Must Staple errors that
was added when the upstream `certlint` package didn't understand that
PKIX extension. That problem was resolved and so we can remove the
ignore from `cert-checker`.

This commit also fixes two typos that were fixed upstream and needed to
be reflected in expected error messages in the `certlint` unit test.

* Certchecker: Ignore Certlint CN/SAN == PSL errors.

`globalsign/certlint`, used by `cmd/cert-checker` to vet certs,
improperly flags certificates that have subj CN/SANs equal to a private
entry in the public suffix list as faulty.

This commit adds a regex that will skip errors that match the certlint
PSL error string. Prior to this workaround the addition of a private PSL
entry as a SAN in the `TestCheckCert` test cert fails the test:

```
--- FAIL: TestCheckCert (1.72s)
  main_test.go:221: Found unexpected problem 'Certificate subjectAltName
  "dev-myqnapcloud.com" equals "dev-myqnapcloud.com" from the public
  suffix list'.
```

With the workaround in place, the test passes again.
This commit is contained in:
Daniel McCarney 2018-04-04 12:20:43 -04:00 committed by GitHub
parent 8167abd5e3
commit 590dca0fe1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 143 additions and 57 deletions

82
Godeps/Godeps.json generated
View File

@ -92,159 +92,163 @@
},
{
"ImportPath": "github.com/globalsign/certlint/asn1",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/certdata",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/aiaissuers",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/all",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/basicconstraints",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/extensions",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/extkeyusage",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/internal",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/issuerdn",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/keyusage",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/publickey",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/publickey/goodkey",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/publicsuffix",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/revocation",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/serialnumber",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/signaturealgorithm",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/subject",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/subjectaltname",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/validity",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/version",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/certificate/wildcard",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/adobetimestamp",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/all",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/authorityinfoaccess",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/authoritykeyid",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/basicconstraints",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/crldistributionpoints",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/ct",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/extkeyusage",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/keyusage",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/nameconstraints",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/ocspmuststaple",
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/ocspnocheck",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/pdfrevocation",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/policyidentifiers",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/smimecapabilities",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/subjectaltname",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/checks/extensions/subjectkeyid",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/globalsign/certlint/errors",
"Rev": "6f978b7833fbea4afdf7953aa747c871048a4e9c"
"Rev": "d4a45be06892f3e664f69892aca79a48df510be0"
},
{
"ImportPath": "github.com/go-sql-driver/mysql",

View File

@ -36,14 +36,19 @@ const (
good = "valid"
bad = "invalid"
certlintCNError = "commonName field is deprecated"
certlintOCSPMustStapleError = "Certificate contains unknown extension (1.3.6.1.5.5.7.1.24)"
certlintCNError = "commonName field is deprecated"
filenameLayout = "20060102"
expectedValidityPeriod = time.Hour * 24 * 90
)
// certlintPSLErrPattern is a regex for matching Certlint error strings
// complaining about a CN or SAN matching a public suffix list entry.
var certlintPSLErrPattern = regexp.MustCompile(
`^Certificate (?:CommonName|subjectAltName) "[a-z0-9*][a-z0-9-.]+" ` +
`equals "[a-z0-9][a-z0-9-.]+" from the public suffix list$`)
// For defense-in-depth in addition to using the PA & its hostnamePolicy to
// check domain names we also perform a check against the regex's from the
// forbiddenDomains array
@ -222,10 +227,12 @@ func (c *certChecker) checkCert(cert core.Certificate) (problems []string) {
if err.Error() == certlintCNError {
continue
}
// Certlint doesn't presently understand the RFC 7633 OCSP Must Staple
// extension. While this is unaddressed in the upstream library we ignore
// this error like we ignore `certlintCNError`.
if err.Error() == certlintOCSPMustStapleError {
// certlint incorrectly flags certificates that have a subj. CN or SAN
// exactly equal to a *private* entry on the public suffix list. Since
// this is allowed and LE issues certificates for such names we ignore
// errors of this form until the upstream bug can be addressed. See
// https://github.com/globalsign/certlint/issues/17
if certlintPSLErrPattern.MatchString(err.Error()) {
continue
}
problems = append(problems, err.Error())

View File

@ -165,9 +165,18 @@ func TestCheckCert(t *testing.T) {
Subject: pkix.Name{
CommonName: longName,
},
NotBefore: issued,
NotAfter: goodExpiry.AddDate(0, 0, 1), // Period too long
DNSNames: []string{longName, "example-a.com", "foodnotbombs.mil", "*.foodnotbombs.mil"},
NotBefore: issued,
NotAfter: goodExpiry.AddDate(0, 0, 1), // Period too long
DNSNames: []string{
// longName should be flagged along with the long CN
longName,
"example-a.com",
"foodnotbombs.mil",
// `*.foodnotbombs.mil` should be flagged because the wildcard issuance feature is disabled
"*.foodnotbombs.mil",
// `dev-myqnapcloud.com` is included because it is an exact private
// entry on the public suffix list
"dev-myqnapcloud.com"},
SerialNumber: serial,
BasicConstraintsValid: false,
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
@ -202,7 +211,7 @@ func TestCheckCert(t *testing.T) {
"Certificate has incorrect key usage extensions": 1,
"Certificate has common name >64 characters long (65)": 1,
"Policy Authority isn't willing to issue for '*.foodnotbombs.mil': Wildcard names not supported": 1,
"commonName exeeding max lenght of 64": 1,
"commonName exceeding max length of 64": 1,
"Certificate contains unknown extension (1.3.3.7)": 1,
}
for _, p := range problems {

View File

@ -211,7 +211,7 @@ func isForbiddenString(b []byte) bool {
(r >= 58 && r <= 64) ||
(r >= 91 && r <= 96) ||
(r >= 123 && r <= 126)) {
// non metadata character inlcuded in value
// non metadata character included in value
return false
}
b = b[size:]

View File

@ -18,7 +18,7 @@ func (o *object) Equal(oid asn1.ObjectIdentifier) bool {
func (o *object) Valid(v interface{}) error {
if o.maxLength > 0 && len([]rune(v.(string))) > o.maxLength {
return fmt.Errorf("exeeding max lenght of %d", o.maxLength)
return fmt.Errorf("exceeding max length of %d", o.maxLength)
}
return nil
}

View File

@ -12,7 +12,7 @@ const checkName = "Subject Check"
func init() {
filter := &checks.Filter{
//Type: []string{"DV", "OV", "IV", "EV"},
//Type: []string{"DV", "OV", "IV", "EV"},
}
checks.RegisterCertificateCheck(checkName, filter, Check)
}
@ -57,7 +57,7 @@ func checkDN(vetting string, dn []pkix.AttributeTypeAndValue) *errors.Errors {
// Field related requirements
//
// Max field lenght:
// Max field length:
// https://www.itu.int/ITU-T/formal-language/itu-t/x/x520/2001/UpperBounds.html
for _, n := range dn {
switch {
@ -65,7 +65,7 @@ func checkDN(vetting string, dn []pkix.AttributeTypeAndValue) *errors.Errors {
// commonName
// If present, this field MUST contain a single IP address or FullyQualified Domain Name
case commonName.Equal(n.Type):
// report deprecated common name field as info untill not commenly used/accepted
// report deprecated common name field as info until not commenly used/accepted
e.Info("commonName field is deprecated")
// check if value is exceeding max length
@ -74,7 +74,7 @@ func checkDN(vetting string, dn []pkix.AttributeTypeAndValue) *errors.Errors {
}
case emailAddress.Equal(n.Type):
// report deprecated email address field as info untill not commenly used/accepted
// report deprecated email address field as info until not commenly used/accepted
e.Info("emailAddress field is deprecated")
// RFC5280: ub-emailaddress-length was changed from 128 to 255 in order to

View File

@ -11,6 +11,7 @@ import (
_ "github.com/globalsign/certlint/checks/extensions/extkeyusage"
_ "github.com/globalsign/certlint/checks/extensions/keyusage"
_ "github.com/globalsign/certlint/checks/extensions/nameconstraints"
_ "github.com/globalsign/certlint/checks/extensions/ocspmuststaple"
_ "github.com/globalsign/certlint/checks/extensions/ocspnocheck"
_ "github.com/globalsign/certlint/checks/extensions/pdfrevocation"
_ "github.com/globalsign/certlint/checks/extensions/policyidentifiers"

View File

@ -0,0 +1,65 @@
package ocspmuststaple
import (
"bytes"
"crypto/x509/pkix"
"encoding/asn1"
"fmt"
"github.com/globalsign/certlint/certdata"
"github.com/globalsign/certlint/checks"
"github.com/globalsign/certlint/errors"
)
const (
checkName = "OCSP Must Staple Extension Check"
certTypeErr = "OCSP Must Staple extension set in non end-entity/issuer certificate"
critExtErr = "OCSP Must Staple extension set critical"
)
var (
// RFC 7633 OID of the OCSP Must Staple TLS Extension Feature
extensionOid = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24}
// Expected extension value (DER encoded ASN.1 bytestring)
expectedExtensionValue = []uint8{0x30, 0x3, 0x2, 0x1, 0x5}
extValueErr = fmt.Sprintf(
"OCSP Must Staple extension had incorrect value. "+
"Should be ASN.1 DER %#v", expectedExtensionValue)
)
// RFC 7633 only defines this extension for PKIX end-entity certificates,
// certificate signing requests, and certificate signing certificates (CAs).
// We should not allow it for cert types like "OCSP", "PS", "CS", etc.
var allowedCertTypes = map[string]bool{
"DV": true,
"OV": true,
"EV": true,
"CA": true,
}
func init() {
// Register this check for the OCSP Must Staple extension OID.
checks.RegisterExtensionCheck(checkName, extensionOid, nil, Check)
}
// Check performs a strict verification on the extension according to the standard(s)
func Check(ex pkix.Extension, d *certdata.Data) *errors.Errors {
var e = errors.New(nil)
// If the cert type isn't one of the `allowedCertTypes`, return an error
if _, allowed := allowedCertTypes[d.Type]; !allowed {
e.Err(certTypeErr)
}
// Per RFC 7633 "The TLS feature extension SHOULD NOT be marked critical"
if ex.Critical {
e.Err(critExtErr)
}
// Check that the extension value is the expected slice of DER encoded ASN.1
if bytes.Compare(ex.Value, expectedExtensionValue) != 0 {
e.Err(extValueErr)
}
return e
}

View File

@ -2,7 +2,7 @@
package errors
import "fmt"
import "strconv"
const _Priority_name = "UnknownDebugInfoNoticeWarningErrorCriticalAlertEmergency"
@ -10,7 +10,7 @@ var _Priority_index = [...]uint8{0, 7, 12, 16, 22, 29, 34, 42, 47, 56}
func (i Priority) String() string {
if i < 0 || i >= Priority(len(_Priority_index)-1) {
return fmt.Sprintf("Priority(%d)", i)
return "Priority(" + strconv.FormatInt(int64(i), 10) + ")"
}
return _Priority_name[_Priority_index[i]:_Priority_index[i+1]]
}