From 404e9682b1f78f9750c7ad4582c0f6daac26cadf Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Tue, 18 Oct 2016 10:15:21 -0700 Subject: [PATCH] Improve error messages. (#2256) Quote rejected hostnames. Include term "global" when rejecting based on global rate limit. Fixes #2252 --- csr/csr.go | 2 +- csr/csr_test.go | 28 ++++++++++++++-------------- ra/ra.go | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/csr/csr.go b/csr/csr.go index c44562b3b..ed3dcb328 100644 --- a/csr/csr.go +++ b/csr/csr.go @@ -80,7 +80,7 @@ func VerifyCSR(csr *x509.CertificateRequest, maxNames int, keyPolicy *goodkey.Ke Type: core.IdentifierDNS, Value: name, }); err != nil { - badNames = append(badNames, name) + badNames = append(badNames, fmt.Sprintf("%q", name)) } } if len(badNames) > 0 { diff --git a/csr/csr_test.go b/csr/csr_test.go index 776ce65e7..5c6156954 100644 --- a/csr/csr_test.go +++ b/csr/csr_test.go @@ -28,7 +28,7 @@ func (pa *mockPA) ChallengesFor(identifier core.AcmeIdentifier) (challenges []co } func (pa *mockPA) WillingToIssue(id core.AcmeIdentifier) error { - if id.Value == "bad-name.com" { + if id.Value == "bad-name.com" || id.Value == "other-bad-name.com" { return errors.New("") } return nil @@ -50,9 +50,9 @@ func TestVerifyCSR(t *testing.T) { signedReqWithLongCN := new(x509.CertificateRequest) *signedReqWithLongCN = *signedReq signedReqWithLongCN.Subject.CommonName = strings.Repeat("a", maxCNLength+1) - signedReqWithBadName := new(x509.CertificateRequest) - *signedReqWithBadName = *signedReq - signedReqWithBadName.DNSNames = []string{"bad-name.com"} + signedReqWithBadNames := new(x509.CertificateRequest) + *signedReqWithBadNames = *signedReq + signedReqWithBadNames.DNSNames = []string{"bad-name.com", "other-bad-name.com"} signedReqWithEmailAddress := new(x509.CertificateRequest) *signedReqWithEmailAddress = *signedReq signedReqWithEmailAddress.EmailAddresses = []string{"foo@bar.com"} @@ -70,7 +70,7 @@ func TestVerifyCSR(t *testing.T) { }{ { &x509.CertificateRequest{}, - 0, + 100, testingPolicy, &mockPA{}, 0, @@ -78,7 +78,7 @@ func TestVerifyCSR(t *testing.T) { }, { &x509.CertificateRequest{PublicKey: private.PublicKey}, - 1, + 100, testingPolicy, &mockPA{}, 0, @@ -86,7 +86,7 @@ func TestVerifyCSR(t *testing.T) { }, { brokenSignedReq, - 1, + 100, testingPolicy, &mockPA{}, 0, @@ -94,7 +94,7 @@ func TestVerifyCSR(t *testing.T) { }, { signedReq, - 1, + 100, testingPolicy, &mockPA{}, 0, @@ -102,7 +102,7 @@ func TestVerifyCSR(t *testing.T) { }, { signedReqWithLongCN, - 1, + 100, testingPolicy, &mockPA{}, 0, @@ -117,16 +117,16 @@ func TestVerifyCSR(t *testing.T) { errors.New("CSR contains more than 1 DNS names"), }, { - signedReqWithBadName, - 1, + signedReqWithBadNames, + 100, testingPolicy, &mockPA{}, 0, - errors.New("policy forbids issuing for: bad-name.com"), + errors.New("policy forbids issuing for: \"bad-name.com\", \"other-bad-name.com\""), }, { signedReqWithEmailAddress, - 1, + 100, testingPolicy, &mockPA{}, 0, @@ -134,7 +134,7 @@ func TestVerifyCSR(t *testing.T) { }, { signedReqWithIPAddress, - 1, + 100, testingPolicy, &mockPA{}, 0, diff --git a/ra/ra.go b/ra/ra.go index faba686eb..3cd31754e 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -749,7 +749,7 @@ func (ra *RegistrationAuthorityImpl) checkLimits(ctx context.Context, names []st domains := strings.Join(names, ",") ra.totalCertsStats.Inc("Exceeded", 1) ra.log.Info(fmt.Sprintf("Rate limit exceeded, TotalCertificates, regID: %d, domains: %s, totalIssued: %d", regID, domains, totalIssued)) - return core.RateLimitedError("Certificate issuance limit reached") + return core.RateLimitedError("Global certificate issuance limit reached. Try again in an hour.") } ra.totalCertsStats.Inc("Pass", 1) }