Move cert-csr check to boulder/core and review fixes

This commit is contained in:
Roland Shoemaker 2015-06-02 17:56:28 +01:00
parent 80833a1513
commit 51890a9626
4 changed files with 109 additions and 71 deletions

View File

@ -10,7 +10,9 @@ import (
"encoding/base64"
"encoding/hex"
"encoding/json"
"fmt"
jose "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/square/go-jose"
"sort"
"strings"
"time"
)
@ -49,6 +51,40 @@ const (
IdentifierDNS = IdentifierType("dns")
)
func cmpStrSlice(a, b []string) bool {
if len(a) != len(b) {
return false
}
sort.Strings(a)
sort.Strings(b)
for i := range a {
if a[i] != b[i] {
return false
}
}
return true
}
func cmpExtKeyUsageSlice(a, b []x509.ExtKeyUsage) bool {
if len(a) != len(b) {
return false
}
intA := make([]int, len(a))
intB := make([]int, len(b))
for i := range a {
intA[i] = int(a[i])
intB[i] = int(b[i])
}
sort.Ints(intA)
sort.Ints(intB)
for i := range a {
if a[i] != b[i] {
return false
}
}
return true
}
// An AcmeIdentifier encodes an identifier that can
// be validated by ACME. The protocol allows for different
// types of identifier to be supported (DNS names, IP
@ -361,6 +397,57 @@ type Certificate struct {
Expires time.Time `db:"expires"`
}
func (cert Certificate) MatchesCSR(csr *x509.CertificateRequest, earliestExpiry time.Time) (err error) {
parsedCertificate, err := x509.ParseCertificate([]byte(cert.DER))
if err != nil {
return
}
// Check issued certificate matches what was expected from the CSR
hostNames := make([]string, len(csr.DNSNames))
copy(hostNames, csr.DNSNames)
if len(csr.Subject.CommonName) > 0 {
hostNames = append(hostNames, csr.Subject.CommonName)
}
hostNames = UniqueNames(hostNames)
if !KeyDigestEquals(parsedCertificate.PublicKey, csr.PublicKey) {
err = InternalServerError("Generated certificate public key doesn't match CSR public key")
return
}
if len(csr.Subject.CommonName) > 0 && parsedCertificate.Subject.CommonName != csr.Subject.CommonName {
err = InternalServerError("Generated certificate CommonName doesn't match CSR CommonName")
return
}
if !cmpStrSlice(parsedCertificate.DNSNames, hostNames) {
err = InternalServerError("Generated certificate DNSNames don't match CSR DNSNames")
return
}
if parsedCertificate.NotAfter.After(earliestExpiry) {
err = InternalServerError("Generated certificate expires before earliest expiration")
return
}
now := time.Now()
if now.Sub(parsedCertificate.NotBefore) > time.Hour*24 {
err = InternalServerError(fmt.Sprintf("Generated certificate is back dated %s", now.Sub(parsedCertificate.NotBefore)))
return
}
if !parsedCertificate.BasicConstraintsValid {
err = InternalServerError("Generated certificate doesn't have basic constraints set")
return
}
if parsedCertificate.IsCA {
err = InternalServerError("Generated certificate can sign other certificates")
return
}
if !cmpExtKeyUsageSlice(parsedCertificate.ExtKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}) {
err = InternalServerError("Generated certificate doesn't have correct key usage extensions")
return
}
return
}
// CertificateStatus structs are internal to the server. They represent the
// latest data about the status of the certificate, required for OCSP updating
// and for validating that the subscriber has accepted the certificate.

View File

@ -11,7 +11,6 @@ import (
"math/big"
"net/url"
"regexp"
"sort"
"strconv"
"time"
@ -47,20 +46,6 @@ func lastPathSegment(url core.AcmeURL) string {
return allButLastPathSegment.ReplaceAllString(url.Path, "")
}
func cmpStrSlice(a, b []string) bool {
if len(a) != len(b) {
return false
}
sort.Strings(a)
sort.Strings(b)
for i := range a {
if a[i] != b[i] {
return false
}
}
return true
}
type certificateRequestEvent struct {
ID string `json:",omitempty"`
Requester int64 `json:",omitempty"`
@ -278,6 +263,12 @@ func (ra *RegistrationAuthorityImpl) NewCertificate(req core.CertificateRequest,
return emptyCert, err
}
err = cert.MatchesCSR(csr, earliestExpiry)
if err != nil {
logEvent.Error = err.Error()
return emptyCert, err
}
parsedCertificate, err := x509.ParseCertificate([]byte(cert.DER))
if err != nil {
err = core.InternalServerError(err.Error())
@ -285,48 +276,6 @@ func (ra *RegistrationAuthorityImpl) NewCertificate(req core.CertificateRequest,
return emptyCert, err
}
// Check issued certificate matches what was expected from the CSR
hostNames := make([]string, len(csr.DNSNames))
copy(hostNames, csr.DNSNames)
if len(csr.Subject.CommonName) > 0 {
hostNames = append(hostNames, csr.Subject.CommonName)
}
hostNames = core.UniqueNames(hostNames)
if !core.KeyDigestEquals(parsedCertificate.PublicKey, csr.PublicKey) {
err = core.MalformedRequestError("Generated certificate public key doesn't match CSR public key")
return emptyCert, err
}
if len(csr.Subject.CommonName) > 0 && parsedCertificate.Subject.CommonName != csr.Subject.CommonName {
err = core.InternalServerError("Generated certificate CommonName doesn't match CSR CommonName")
logEvent.Error = err.Error()
return emptyCert, err
}
if !cmpStrSlice(parsedCertificate.Subject.Country, csr.Subject.Country) || !cmpStrSlice(parsedCertificate.Subject.Organization, csr.Subject.Organization) ||
!cmpStrSlice(parsedCertificate.Subject.OrganizationalUnit, csr.Subject.OrganizationalUnit) || !cmpStrSlice(parsedCertificate.Subject.Locality, csr.Subject.Locality) ||
!cmpStrSlice(parsedCertificate.Subject.Province, csr.Subject.Province) || !cmpStrSlice(parsedCertificate.Subject.StreetAddress, csr.Subject.StreetAddress) ||
!cmpStrSlice(parsedCertificate.Subject.PostalCode, csr.Subject.PostalCode) {
err = core.InternalServerError("Generated certificate Subject doesn't match CSR Subject")
logEvent.Error = err.Error()
return emptyCert, err
}
if !cmpStrSlice(parsedCertificate.DNSNames, hostNames) {
err = core.InternalServerError("Generated certificate DNSNames don't match CSR DNSNames")
logEvent.Error = err.Error()
return emptyCert, err
}
now = time.Now()
if now.After(parsedCertificate.NotAfter) {
err = core.InternalServerError("Generated certificate is already expired")
logEvent.Error = err.Error()
return emptyCert, err
}
if now.Before(parsedCertificate.NotBefore) {
err = core.InternalServerError(fmt.Sprintf("Generated certificate won't be valid for %s", parsedCertificate.NotBefore.Sub(now)))
logEvent.Error = err.Error()
return emptyCert, err
}
logEvent.SerialNumber = parsedCertificate.SerialNumber
logEvent.CommonName = parsedCertificate.Subject.CommonName
logEvent.NotBefore = parsedCertificate.NotBefore

View File

@ -15,7 +15,8 @@
"usages": [
"digital signature",
"key encipherment",
"server auth"
"server auth",
"client auth"
],
"backdate": "1h",
"is_ca": false,

View File

@ -82,16 +82,17 @@ eROL1ve1vmQF3kjrMPhhK2kr6qdWnTE5XlPllVSZFQenSTzj98AO
// * CN = lets-encrypt
// * DNSNames = not-an-example.com
// Used for NewCertificate tests
GoodTestCert = "308201293081d6a003020102020100300b06092a864886f70d01010b300d310b3" +
"0090603550406130255533020170d3039313131303233303030305a180f333030" +
"39313131303233303030305a300d310b3009060355040613025553305c300d060" +
"92a864886f70d0101010500034b003048024100ed3c7ce33530068c71e08ccefd" +
"4086da4ebd3a6b9796c342ed83346f94b74891286e0ec2d13205fff77c9a52400" +
"5b211210eb528186b30916fdcc7e754d57aed0203010001a321301f301d060355" +
"1d110416301482126e6f742d616e2d6578616d706c652e636f6d300b06092a864" +
"886f70d01010b0341009426b655bd955c281dc568a531c78bbe4c691c9eab3335" +
"7ff379450cba90100acfdaa68cb192f1492be263caae0dc1dce3808ba707999f5" +
"e77b709ec96c786ba"
GoodTestCert = "3082015930820105a003020102020100300b06092a864886f70d01010b300d310" +
"b30090603550406130255533022180f32303539313131303233303030305a180f" +
"32303539313131303233303030305a300d310b3009060355040613025553305c3" +
"00d06092a864886f70d0101010500034b003048024100c0103404870dd67f7038" +
"e9274a01cbe3bd15c64a3f48965b72020ee53f976d9d7441c6ba923be99918394" +
"ca6529ccd456edd46e5064afd5fc9ea8e5769f888150203010001a34e304c301d" +
"0603551d250416301406082b0601050507030106082b06010505070302300c060" +
"3551d130101ff04023000301d0603551d110416301482126e6f742d616e2d6578" +
"616d706c652e636f6d300b06092a864886f70d01010b034100439a944c819e2ee" +
"0fb906a5e4bd7272b4b976712d8c27ebdf5cfbf81930b43e0ff327577916d81c6" +
"612ae1e612036f06feac5db2cecc306c3b1cd71d5d1c4035"
)
type MockSA struct {
@ -136,7 +137,7 @@ func (sa *MockSA) GetRegistrationByKey(jwk jose.JsonWebKey) (core.Registration,
func (sa *MockSA) GetAuthorization(id string) (core.Authorization, error) {
if id == "valid" {
return core.Authorization{Status: core.StatusValid, RegistrationID: 1, Expires: time.Now().AddDate(0, 0, 1), Identifier: core.AcmeIdentifier{Type: "dns", Value: "not-an-example.com"}}, nil
return core.Authorization{Status: core.StatusValid, RegistrationID: 1, Expires: time.Now().AddDate(100, 0, 0), Identifier: core.AcmeIdentifier{Type: "dns", Value: "not-an-example.com"}}, nil
}
return core.Authorization{}, nil
}
@ -467,9 +468,9 @@ func TestIssueCertificate(t *testing.T) {
Method: "POST",
Body: makeBody(`
{
"payload":"eyJjc3IiOiJNSUgxTUlHaUFnRUFNQTB4Q3pBSkJnTlZCQVlUQWxWVE1Gd3dEUVlKS29aSWh2Y05BUUVCQlFBRFN3QXdTQUpCQU8wOGZPTTFNQWFNY2VDTXp2MUFodHBPdlRwcmw1YkRRdTJETkctVXQwaVJLRzRPd3RFeUJmXzNmSnBTUUFXeUVTRU90U2dZYXpDUmI5ekg1MVRWZXUwQ0F3RUFBYUF3TUM0R0NTcUdTSWIzRFFFSkRqRWhNQjh3SFFZRFZSMFJCQll3RklJU2JtOTBMV0Z1TFdWNFlXMXdiR1V1WTI5dE1Bc0dDU3FHU0liM0RRRUJDd05CQUNuS0lqOFB3SE1zRU9hT3hPQ00xN3kwSUQ1ZG52cnZ5SXRMRjMxWEw0QnUtYVhRQXFsTzQwam1kZjBWLXprOUhHcmE3RzhLSTFvTHJJWXl1Y1pKX0EwPSIsImF1dGhvcml6YXRpb25zIjpbInZhbGlkIl19",
"payload":"eyJjc3IiOiJNSUgxTUlHaUFnRUFNQTB4Q3pBSkJnTlZCQVlUQWxWVE1Gd3dEUVlKS29aSWh2Y05BUUVCQlFBRFN3QXdTQUpCQU1BUU5BU0hEZFpfY0RqcEowb0J5LU85RmNaS1AwaVdXM0lDRHVVX2wyMmRkRUhHdXBJNzZaa1lPVXltVXB6TlJXN2RSdVVHU3YxZnllcU9WMm40aUJVQ0F3RUFBYUF3TUM0R0NTcUdTSWIzRFFFSkRqRWhNQjh3SFFZRFZSMFJCQll3RklJU2JtOTBMV0Z1TFdWNFlXMXdiR1V1WTI5dE1Bc0dDU3FHU0liM0RRRUJDd05CQUxtVmhoNVQ4cTJVNm5RXzNFM2loMjdkYVUwQU1LM0VEMFVHSlc1Rzk5WTZRNm8zQ3UwNU05OWlFaXhQX0lmOVl6aDkydVBEYnlWWlRwaEliQnRsZ1dnPSIsImF1dGhvcml6YXRpb25zIjpbInZhbGlkIl19",
"protected":"eyJhbGciOiJSUzI1NiIsImp3ayI6eyJrdHkiOiJSU0EiLCJuIjoieU5XVmh0WUVLSlIyMXk5eHNIVi1QRF9iWXdiWFNlTnVGYWw0NnhZeFZmUkw1bXFoYTd2dHR2akJfdmM3WGcyUnZnQ3hIUENxb3hnTVBUekhyWlQ3NUxqQ3dJVzJLX2tsQllOOG9ZdlR3d21lU2tBejZ1dDdaeFB2LW5aYVQ1VEpoR2swTlQya2hfelNwZHJpRUpfM3ZXLW1xeFliYkJtcHZIcXNhMV96eDlmU3VIWWN0QVpKV3p4elVaWHlrYldNV1FacEVpRTBKNGFqajUxZkluRXpWbjdWeFYtbXpmTXlib1FqdWpQaDdhTkp4QVdTcTRvUUVKSkRnV3dTaDlsZXlvSm9QcE9OSHhoNW5FRTVBakUwMUZrR0lDU3hqcFpzRi13OGhPVEkzWFhvaFVkdTI5U2UyNmsyQjBQb2xEU3VqMEdJUVU2LVc5VGRMWFNqQmIyU3BRIiwiZSI6IkFBRUFBUSJ9fQ",
"signature":"VG7HOgjfmxPMndFKDr4gM-tDZ8RmGbY5ApTYE1mbHZBtOi5_igInsjcn6xGpIBcQMQk0jZXB4EDmzbrlUTmBOId5jJwynaKjw5ZPcwQdEhmffaJ9UNrPmGbOeUk0ZDDecO512LE3IKONaEFdLgsv7uTcDilrE0LeQbHogXWrK0UOZlx127MbmAdu1c6XSGquNNiEKrVqxNpo6df7YgeMpHgNV3bfMaAtaf2lt6HsFKPiNwtpHlQeGKmYbPUwdtonl0vja-k39tV4hGai93mNtFRSMLeIfaqCvIrDQjLUj9FHDDSoUiUmv3KFdzR0kzmVMyJtDkN-TtqxRizLu2eEfA"
"signature":"bEI5f4DsMMSoli3B3LJXjW6LMJWcUoP-0muvCzGpr8oB14Uvl0NN3ZagWviVViPy-BEE5gG8qPly8u53GbjeL5RbXvO84UuU7VAHQma67FjnpG1frxrSPKkPgYN1YWOIaAj6jOY5qhebl9DVewBfJ-gkxJqr0_7mqU1Jeucsu9q-O0pAE5v4ll_w7jjO8CJgLRPlpiSKHYjz2SnCxYrKaEF7ZZovH6OMfTsiK686u4R6i4X07ZCfY_qdQQYHdla0qUd3pDYPbHhgjXnj1xGiA6tt7WgN-MNZHgtWEXsa6q5CREBIN1hc6X38pZBpqzW4iFEDMhCWPZNhZXtILwwBrQ"
}
`),
})