From b59682cb9165753ac996f49982cf4ca4251ffba6 Mon Sep 17 00:00:00 2001 From: Richard Barnes Date: Thu, 28 May 2015 20:28:32 -0700 Subject: [PATCH] Add validity interval checking --- ca/certificate-authority.go | 12 ++++++++++-- ca/certificate-authority_test.go | 27 ++++++++++++++++++++------- core/interfaces.go | 5 +++-- ra/registration-authority.go | 7 ++++++- rpc/rpc-wrappers.go | 15 +++++++++------ 5 files changed, 48 insertions(+), 18 deletions(-) diff --git a/ca/certificate-authority.go b/ca/certificate-authority.go index e5bb35d96..f5bc8e908 100644 --- a/ca/certificate-authority.go +++ b/ca/certificate-authority.go @@ -205,7 +205,7 @@ func (ca *CertificateAuthorityImpl) RevokeCertificate(serial string, reasonCode // IssueCertificate attempts to convert a CSR into a signed Certificate, while // enforcing all policies. -func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest, regID int64) (core.Certificate, error) { +func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest, regID int64, earliestExpiry time.Time) (core.Certificate, error) { emptyCert := core.Certificate{} var err error // XXX Take in authorizations and verify that union covers CSR? @@ -228,12 +228,20 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest return emptyCert, err } - if ca.NotAfter.Before(time.Now().Add(ca.ValidityPeriod)) { + notAfter := time.Now().Add(ca.ValidityPeriod) + + if ca.NotAfter.Before(notAfter) { err = errors.New("Cannot issue a certificate that expires after the intermediate certificate.") ca.log.WarningErr(err) return emptyCert, err } + if earliestExpiry.Before(notAfter) { + err = errors.New(fmt.Sprintf("Cannot issue a certificate that expires after the shortest underlying authorization. [%v] [%v]", earliestExpiry, notAfter)) + ca.log.WarningErr(err) + return emptyCert, err + } + if len(hostNames) == 0 { hostNames = []string{commonName} } diff --git a/ca/certificate-authority_test.go b/ca/certificate-authority_test.go index 2afeef5a0..4771cd209 100644 --- a/ca/certificate-authority_test.go +++ b/ca/certificate-authority_test.go @@ -242,6 +242,10 @@ var DUPE_NAME_CSR_HEX = "3082018d3081f90201003016311430120603550403130b6578616d7 "5b7c0134e95b77a43a6b5789ff97b3262f949e75690314e417c4c2bd3d1f" + "7bedb21db1dd5dd4f71b82" +// Times for validity checking +var FarFuture = time.Date(2100, 1, 1, 0, 0, 0, 0, time.UTC) +var FarPast = time.Date(1950, 1, 1, 0, 0, 0, 0, time.UTC) + // CFSSL config const hostPort = "localhost:9000" const authKey = "79999d86250c367a2b517a1ae7d409c1" @@ -364,7 +368,7 @@ func TestRevoke(t *testing.T) { csrDER, _ := hex.DecodeString(CN_AND_SAN_CSR_HEX) csr, _ := x509.ParseCertificateRequest(csrDER) - certObj, err := ca.IssueCertificate(*csr, 1) + certObj, err := ca.IssueCertificate(*csr, 1, FarFuture) test.AssertNotError(t, err, "Failed to sign certificate") if err != nil { return @@ -404,7 +408,7 @@ func TestIssueCertificate(t *testing.T) { csr, _ := x509.ParseCertificateRequest(csrDER) // Sign CSR - certObj, err := ca.IssueCertificate(*csr, 1) + certObj, err := ca.IssueCertificate(*csr, 1, FarFuture) test.AssertNotError(t, err, "Failed to sign certificate") if err != nil { continue @@ -443,10 +447,19 @@ func TestIssueCertificate(t *testing.T) { test.Assert(t, certStatus.SubscriberApproved == false, "Subscriber shouldn't have approved cert yet.") } - // Test that the CA rejects CSRs with no names - csrDER, _ := hex.DecodeString(NO_NAME_CSR_HEX) + // Test that the CA rejects an otherwise valid request if the earliest + // expiry date is in the past + csrDER, _ := hex.DecodeString(CN_AND_SAN_CSR_HEX) csr, _ := x509.ParseCertificateRequest(csrDER) - _, err = ca.IssueCertificate(*csr, 1) + _, err = ca.IssueCertificate(*csr, 1, FarPast) + if err == nil { + t.Errorf("CA improperly agreed to create a certificate with too long a lifetime") + } + + // Test that the CA rejects CSRs with no names + csrDER, _ = hex.DecodeString(NO_NAME_CSR_HEX) + csr, _ = x509.ParseCertificateRequest(csrDER) + _, err = ca.IssueCertificate(*csr, 1, FarFuture) if err == nil { t.Errorf("CA improperly agreed to create a certificate with no name") } @@ -454,7 +467,7 @@ func TestIssueCertificate(t *testing.T) { // Test that the CA rejects CSRs with duplicate names csrDER, _ = hex.DecodeString(DUPE_NAME_CSR_HEX) csr, _ = x509.ParseCertificateRequest(csrDER) - _, err = ca.IssueCertificate(*csr, 1) + _, err = ca.IssueCertificate(*csr, 1, FarFuture) if err == nil { t.Errorf("CA improperly agreed to create a certificate with duplicate names") } @@ -463,7 +476,7 @@ func TestIssueCertificate(t *testing.T) { csrDER, _ = hex.DecodeString(NO_CN_CSR_HEX) csr, _ = x509.ParseCertificateRequest(csrDER) ca.NotAfter = time.Now() - _, err = ca.IssueCertificate(*csr, 1) + _, err = ca.IssueCertificate(*csr, 1, FarFuture) test.AssertEquals(t, err.Error(), "Cannot issue a certificate that expires after the intermediate certificate.") } diff --git a/core/interfaces.go b/core/interfaces.go index 106671b63..6a74a8382 100644 --- a/core/interfaces.go +++ b/core/interfaces.go @@ -9,6 +9,7 @@ import ( "crypto/x509" jose "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/square/go-jose" "net/http" + "time" ) // A WebFrontEnd object supplies methods that can be hooked into @@ -78,8 +79,8 @@ type ValidationAuthority interface { type CertificateAuthority interface { // [RegistrationAuthority] - IssueCertificate(x509.CertificateRequest, int64) (Certificate, error) - RevokeCertificate(string, int) error + IssueCertificate(x509.CertificateRequest, int64, time.Time) (Certificate, error) + RevokeCertificate(serial string) error } type PolicyAuthority interface { diff --git a/ra/registration-authority.go b/ra/registration-authority.go index 9da077432..3b6128eda 100644 --- a/ra/registration-authority.go +++ b/ra/registration-authority.go @@ -184,6 +184,7 @@ func (ra *RegistrationAuthorityImpl) NewCertificate(req core.CertificateRequest, // Gather authorized domains from the referenced authorizations authorizedDomains := map[string]bool{} verificationMethodSet := map[string]bool{} + earliestExpiry := time.Date(2100, 01, 01, 0, 0, 0, 0, time.UTC) now := time.Now() for _, url := range req.Authorizations { id := lastPathSegment(url) @@ -199,6 +200,10 @@ func (ra *RegistrationAuthorityImpl) NewCertificate(req core.CertificateRequest, continue } + if authz.Expires.Before(earliestExpiry) { + earliestExpiry = authz.Expires + } + for _, challenge := range authz.Challenges { if challenge.Status == core.StatusValid { verificationMethodSet[challenge.Type] = true @@ -233,7 +238,7 @@ func (ra *RegistrationAuthorityImpl) NewCertificate(req core.CertificateRequest, // Create the certificate and log the result var cert core.Certificate - if cert, err = ra.CA.IssueCertificate(*csr, regID); err != nil { + if cert, err = ra.CA.IssueCertificate(*csr, regID, earliestExpiry); err != nil { logEvent.Error = err.Error() return emptyCert, nil } diff --git a/rpc/rpc-wrappers.go b/rpc/rpc-wrappers.go index 24b2820f0..adecb59f2 100644 --- a/rpc/rpc-wrappers.go +++ b/rpc/rpc-wrappers.go @@ -10,6 +10,7 @@ import ( "encoding/json" "errors" "fmt" + "time" jose "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/square/go-jose" "github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/streadway/amqp" @@ -427,8 +428,9 @@ func NewCertificateAuthorityServer(serverQueue string, channel *amqp.Channel, im rpc.Handle(MethodIssueCertificate, func(req []byte) []byte { var icReq struct { - Bytes []byte - RegID int64 + Bytes []byte + RegID int64 + EarliestExpiry time.Time } err := json.Unmarshal(req, &icReq) if err != nil { @@ -442,7 +444,7 @@ func NewCertificateAuthorityServer(serverQueue string, channel *amqp.Channel, im return nil // XXX } - cert, err := impl.IssueCertificate(*csr, icReq.RegID) + cert, err := impl.IssueCertificate(*csr, icReq.RegID, icReq.EarliestExpiry) if err != nil { // AUDIT[ Error Conditions ] 9cc4d537-8534-4970-8665-4b382abe82f3 errorCondition(MethodIssueCertificate, err, csr) @@ -492,10 +494,11 @@ func NewCertificateAuthorityClient(clientQueue, serverQueue string, channel *amq return } -func (cac CertificateAuthorityClient) IssueCertificate(csr x509.CertificateRequest, regID int64) (cert core.Certificate, err error) { +func (cac CertificateAuthorityClient) IssueCertificate(csr x509.CertificateRequest, regID int64, earliestExpiry time.Time) (cert core.Certificate, err error) { var icReq struct { - Bytes []byte - RegID int64 + Bytes []byte + RegID int64 + EarliestExpiry time.Time } icReq.Bytes = csr.Raw icReq.RegID = regID