X Tutup
Skip to content

Commit 0df44e5

Browse files
jmhodgesjmhodges
committed
clean up CSRs with capitalized letters
This change lowercases domains before they are stored in the database and makes policy.WillingToIssue reject any domains with uppercase letters. Fixes letsencrypt#927.
1 parent a3d7717 commit 0df44e5

10 files changed

+147
-17
lines changed

‎ca/certificate-authority.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"fmt"
1717
"io/ioutil"
1818
"math/big"
19+
"strings"
1920
"time"
2021

2122
"github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock"
@@ -223,7 +224,8 @@ func (ca *CertificateAuthorityImpl) RevokeCertificate(serial string, reasonCode
223224
}
224225

225226
// IssueCertificate attempts to convert a CSR into a signed Certificate, while
226-
// enforcing all policies.
227+
// enforcing all policies. Names (domains) in the CertificateRequest will be
228+
// lowercased before storage.
227229
func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest, regID int64) (core.Certificate, error) {
228230
emptyCert := core.Certificate{}
229231
var err error
@@ -253,10 +255,10 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest
253255
hostNames := make([]string, len(csr.DNSNames))
254256
copy(hostNames, csr.DNSNames)
255257
if len(csr.Subject.CommonName) > 0 {
256-
commonName = csr.Subject.CommonName
257-
hostNames = append(hostNames, csr.Subject.CommonName)
258+
commonName = strings.ToLower(csr.Subject.CommonName)
259+
hostNames = append(hostNames, commonName)
258260
} else if len(hostNames) > 0 {
259-
commonName = hostNames[0]
261+
commonName = strings.ToLower(hostNames[0])
260262
} else {
261263
err = core.MalformedRequestError("Cannot issue a certificate without a hostname.")
262264
// AUDIT[ Certificate Requests ] 11917fa4-10ef-4e0d-9105-bacbe7836a3c
@@ -265,7 +267,7 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest
265267
}
266268

267269
// Collapse any duplicate names. Note that this operation may re-order the names
268-
hostNames = core.UniqueNames(hostNames)
270+
hostNames = core.UniqueLowerNames(hostNames)
269271
if ca.MaxNames > 0 && len(hostNames) > ca.MaxNames {
270272
err = core.MalformedRequestError(fmt.Sprintf("Certificate request has %d > %d names", len(hostNames), ca.MaxNames))
271273
ca.log.WarningErr(err)

‎ca/certificate-authority_test.go‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"encoding/asn1"
1212
"fmt"
1313
"io/ioutil"
14+
"sort"
1415
"testing"
1516
"time"
1617

@@ -64,7 +65,7 @@ var (
6465
DupeNameCSR = mustRead("./testdata/dupe_name.der.csr")
6566

6667
// CSR generated by Go:
67-
// * Random pulic key
68+
// * Random public key
6869
// * CN = [none]
6970
// * DNSNames = not-example.com, www.not-example.com, mail.example.com
7071
TooManyNameCSR = mustRead("./testdata/too_many_names.der.csr")
@@ -81,7 +82,14 @@ var (
8182
// * DNSNames = not-example.com, www.not-example.com, mail.not-example.com
8283
// * Signature algorithm: SHA1WithRSA
8384
BadAlgorithmCSR = mustRead("./testdata/bad_algorithm.der.csr")
84-
log = mocks.UseMockLog()
85+
86+
// CSR generated by Go:
87+
// * Random public key
88+
// * CN = CapiTalizedLetters.com
89+
// * DNSNames = moreCAPs.com, morecaps.com, evenMOREcaps.com, Capitalizedletters.COM
90+
CapitalizedCSR = mustRead("./testdata/capitalized_cn_and_san.der.csr")
91+
92+
log = mocks.UseMockLog()
8593
)
8694

8795
// CFSSL config
@@ -421,3 +429,24 @@ func TestRejectBadAlgorithm(t *testing.T) {
421429
_, ok := err.(core.MalformedRequestError)
422430
test.Assert(t, ok, "Incorrect error type returned")
423431
}
432+
433+
func TestCapitalizedLetters(t *testing.T) {
434+
ctx := setup(t)
435+
defer ctx.cleanUp()
436+
ctx.caConfig.MaxNames = 3
437+
ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, caCertFile)
438+
ca.Publisher = &mocks.Publisher{}
439+
ca.PA = ctx.pa
440+
ca.SA = ctx.sa
441+
442+
csr, _ := x509.ParseCertificateRequest(CapitalizedCSR)
443+
cert, err := ca.IssueCertificate(*csr, ctx.reg.ID)
444+
test.AssertNotError(t, err, "Failed to gracefully handle a CSR with capitalized names")
445+
446+
parsedCert, err := x509.ParseCertificate(cert.DER)
447+
test.AssertNotError(t, err, "Error parsing certificate produced by CA")
448+
test.AssertEquals(t, "capitalizedletters.com", parsedCert.Subject.CommonName)
449+
sort.Strings(parsedCert.DNSNames)
450+
expected := []string{"capitalizedletters.com", "evenmorecaps.com", "morecaps.com"}
451+
test.AssertDeepEquals(t, expected, parsedCert.DNSNames)
452+
}

‎ca/testdata/capitalized_cn_and_san.der.csr‎

716 Bytes
Binary file not shown.

‎ca/testdata/testcsr.go‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Hack up the x509.CertificateRequest in here, run `go run testcsr.go`, and a
2+
// DER-encoded CertificateRequest will be printed to stdout.
3+
package main
4+
5+
import (
6+
"crypto/rand"
7+
"crypto/x509"
8+
"crypto/x509/pkix"
9+
"encoding/pem"
10+
"log"
11+
"os"
12+
)
13+
14+
// A 2048-bit RSA private key
15+
var pemPrivateKey = `-----BEGIN RSA PRIVATE KEY-----
16+
MIIEowIBAAKCAQEA5cpXqfCaUDD+hf93j5jxbrhK4jrJAzfAEjeZj/Lx5Rv/7eEO
17+
uhS2DdCU2is82vR6yJ7EidUYVz/nUAjSTP7JIEsbyvfsfACABbqRyGltHlJnULVH
18+
y/EMjt9xKZf17T8tOLHVUEAJTxsvjKn4TMIQJTNrAqm/lNrUXmCIR41Go+3RBGC6
19+
YdAKEwcZMCzrjQGF06mC6/6xMmYMSMd6+VQRFIPpuPK/6BBp1Tgju2LleRC5uatj
20+
QcFOoilGkfh1RnZp3GJ7q58KaqHiPmjl31rkY5vS3LP7yfU5TRBcxCSG8l8LKuRt
21+
MArkbTEtj3PkDjbipL/SkLrZ28e5w9Egl4g1MwIDAQABAoIBABZqY5zPPK5f6SQ3
22+
JHmciMitL5jb9SncMV9VjyRMpa4cyh1xW9dpF81HMI4Ls7cELEoPuspbQDGaqTzU
23+
b3dVT1dYHFDzWF1MSzDD3162cg+IKE3mMSfCzt/NCiPtj+7hv86NAmr+pCnUVBIb
24+
rn4GXD7UwjaTSn4Bzr+aGREpxd9Nr0JdNQwxVHZ75A92vTihCfaXyMCjhW3JEpF9
25+
N89XehgidoGgtUxxeeb+WsO3nvVBpLv/HDxMTx/IDzvSA5nLlYMcqVzb7IJoeAQu
26+
og0WJKlniYzvIdoQ6/hGydAW5sKd0qWh0JPYs7uLKAWrdAWvrFAp7//fYKVamalU
27+
8pUu/WkCgYEA+tcTQ3qTnVh41O9YeM/7NULpIkuCAlR+PBRky294zho9nGQIPdaW
28+
VNvyqqjLaHaXJVokYHbU4hDk6RbrhoWVd4Po/5g9cUkT1f6nrdZGRkg4XOCzHWvV
29+
Yrqh3eYYX4bdiH5EhB78m0rrbjHfd7SF3cdYNzOUS2kJvCInYC6zPx8CgYEA6oRr
30+
UhZFuoqRsEb28ELM8sHvdIMA/C3aWCu+nUGQ4gHSEb4uvuOD/7tQNuCaBioiXVPM
31+
/4hjk9jHJcjYf5l33ANqIP7JiYAt4rzTWXF3iS6kQOhQhjksSlSnWqw0Uu1DtlpG
32+
rzeG1ZkBuwH7Bx0yj4sGSz5sAvyF44aRsE6AC20CgYEArafWO0ISDb1hMbFdo44B
33+
ELd45Pg3UluiZP+NZFWQ4cbC3pFWL1FvE+KNll5zK6fmLcLBKlM6QCOIBmKKvb+f
34+
YXVeCg0ghFweMmkxNqUAU8nN02bwOa8ctFQWmaOhPgkFN2iLEJjPMsdkRA6c8ad1
35+
gbtvNBAuWyKlzawrbGgISesCgYBkGEjGLINubx5noqJbQee/5U6S6CdPezKqV2Fw
36+
NT/ldul2cTn6d5krWYOPKKYU437vXokst8XooKm/Us41CAfEfCCcHKNgcLklAXsj
37+
ve5LOwEYQw+7ekORJjiX1tAuZN51wmpQ9t4x5LB8ZQgDrU6bPbdd/jKTw7xRtGoS
38+
Wi8EsQKBgG8iGy3+kVBIjKHxrN5jVs3vj/l/fQL0WRMLCMmVuDBfsKyy3f9n8R1B
39+
/KdwoyQFwsLOyr5vAjiDgpFurXQbVyH4GDFiJGS1gb6MNcinwSTpsbOLLV7zgibX
40+
A2NgiQ+UeWMia16dZVd6gGDlY3lQpeyLdsdDd+YppNfy9vedjbvT
41+
-----END RSA PRIVATE KEY-----`
42+
43+
func main() {
44+
block, _ := pem.Decode([]byte(pemPrivateKey))
45+
rsaPriv, err := x509.ParsePKCS1PrivateKey(block.Bytes)
46+
if err != nil {
47+
log.Fatalf("Failed to parse private key: %s", err)
48+
}
49+
50+
req := &x509.CertificateRequest{
51+
Subject: pkix.Name{
52+
CommonName: "CapiTalizedLetters.com",
53+
},
54+
DNSNames: []string{
55+
"moreCAPs.com",
56+
"morecaps.com",
57+
"evenMOREcaps.com",
58+
"Capitalizedletters.COM",
59+
},
60+
}
61+
csr, err := x509.CreateCertificateRequest(rand.Reader, req, rsaPriv)
62+
if err != nil {
63+
log.Fatalf("unable to create CSR: %s", err)
64+
}
65+
_, err = os.Stdout.Write(csr)
66+
if err != nil {
67+
log.Fatalf("unable to write to stdout: %s", err)
68+
}
69+
}

‎core/util.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -472,11 +472,12 @@ func GetBuildHost() (retID string) {
472472
return
473473
}
474474

475-
// UniqueNames returns the set of all unique names in the input.
476-
func UniqueNames(names []string) (unique []string) {
475+
// UniqueLowerNames returns the set of all unique names in the input after all
476+
// of them are lowercased. The returned names will be in their lowercased form.
477+
func UniqueLowerNames(names []string) (unique []string) {
477478
nameMap := make(map[string]int, len(names))
478479
for _, name := range names {
479-
nameMap[name] = 1
480+
nameMap[strings.ToLower(name)] = 1
480481
}
481482

482483
unique = make([]string, 0, len(nameMap))

‎core/util_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"math"
1212
"math/big"
1313
"net/url"
14+
"sort"
1415
"testing"
1516

1617
"github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/go-jose"
@@ -113,3 +114,9 @@ func TestAcmeURL(t *testing.T) {
113114
a := (*AcmeURL)(u)
114115
test.AssertEquals(t, s, a.String())
115116
}
117+
118+
func TestUniqueLowerNames(t *testing.T) {
119+
u := UniqueLowerNames([]string{"foobar.com", "fooBAR.com", "baz.com", "foobar.com", "bar.com", "bar.com"})
120+
sort.Strings(u)
121+
test.AssertDeepEquals(t, []string{"bar.com", "baz.com", "foobar.com"}, u)
122+
}

‎policy/policy-authority.go‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ const (
6464
whitelistedPartnerRegID = -1
6565
)
6666

67-
var dnsLabelRegexp = regexp.MustCompile("^[a-zA-Z0-9][a-zA-Z0-9-]{0,62}$")
67+
var dnsLabelRegexp = regexp.MustCompile("^[a-z0-9][a-z0-9-]{0,62}$")
6868
var punycodeRegexp = regexp.MustCompile("^xn--")
6969

7070
func isDNSCharacter(ch byte) bool {
@@ -112,7 +112,8 @@ func (e SyntaxError) Error() string { return "Syntax error" }
112112
func (e NonPublicError) Error() string { return "Name does not end in a public suffix" }
113113

114114
// WillingToIssue determines whether the CA is willing to issue for the provided
115-
// identifier.
115+
// identifier. It expects domains in id to be lowercase to prevent mismatched
116+
// cases breaking queries.
116117
//
117118
// We place several criteria on identifiers we are willing to issue for:
118119
//
@@ -132,8 +133,6 @@ func (e NonPublicError) Error() string { return "Name does not end in a
132133
// XXX: Is there any need for this method to be constant-time? We're
133134
// going to refuse to issue anyway, but timing could leak whether
134135
// names are on the blacklist.
135-
//
136-
// XXX: We should probably fold everything to lower-case somehow.
137136
func (pa PolicyAuthorityImpl) WillingToIssue(id core.AcmeIdentifier, regID int64) error {
138137
if id.Type != core.IdentifierDNS {
139138
return InvalidIdentifierError{}
@@ -146,7 +145,6 @@ func (pa PolicyAuthorityImpl) WillingToIssue(id core.AcmeIdentifier, regID int64
146145
}
147146
}
148147

149-
domain = strings.ToLower(domain)
150148
if len(domain) > 255 {
151149
return SyntaxError{}
152150
}

‎policy/policy-authority_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ func TestWillingToIssue(t *testing.T) {
8686
`zombocom`,
8787
`localhost`,
8888
`mail`,
89+
90+
// disallow capitalized letters for #927
91+
`CapitalizedLetters.com`,
8992
}
9093

9194
shouldBeNonPublic := []string{

‎ra/registration-authority.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,8 @@ func validateContacts(contacts []*core.AcmeURL, resolver core.DNSResolver, stats
186186
return
187187
}
188188

189-
// NewAuthorization constuct a new Authz from a request.
189+
// NewAuthorization constuct a new Authz from a request. Values (domains) in
190+
// request.Identifier will be lowercased before storage.
190191
func (ra *RegistrationAuthorityImpl) NewAuthorization(request core.Authorization, regID int64) (authz core.Authorization, err error) {
191192
reg, err := ra.SA.GetRegistration(regID)
192193
if err != nil {
@@ -195,6 +196,7 @@ func (ra *RegistrationAuthorityImpl) NewAuthorization(request core.Authorization
195196
}
196197

197198
identifier := request.Identifier
199+
identifier.Value = strings.ToLower(identifier.Value)
198200

199201
// Check that the identifier is present and appropriate
200202
if err = ra.PA.WillingToIssue(identifier, regID); err != nil {
@@ -275,7 +277,7 @@ func (ra *RegistrationAuthorityImpl) MatchesCSR(
275277
if len(csr.Subject.CommonName) > 0 {
276278
hostNames = append(hostNames, csr.Subject.CommonName)
277279
}
278-
hostNames = core.UniqueNames(hostNames)
280+
hostNames = core.UniqueLowerNames(hostNames)
279281

280282
if !core.KeyDigestEquals(parsedCertificate.PublicKey, csr.PublicKey) {
281283
err = core.InternalServerError("Generated certificate public key doesn't match CSR public key")

‎ra/registration-authority_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,25 @@ func TestNewAuthorization(t *testing.T) {
403403
t.Log("DONE TestNewAuthorization")
404404
}
405405

406+
func TestNewAuthorizationCapitalLetters(t *testing.T) {
407+
_, sa, ra, _, cleanUp := initAuthorities(t)
408+
defer cleanUp()
409+
410+
authzReq := core.Authorization{
411+
Identifier: core.AcmeIdentifier{
412+
Type: core.IdentifierDNS,
413+
Value: "NOT-example.COM",
414+
},
415+
}
416+
authz, err := ra.NewAuthorization(authzReq, Registration.ID)
417+
test.AssertNotError(t, err, "NewAuthorization failed")
418+
test.AssertEquals(t, "not-example.com", authz.Identifier.Value)
419+
420+
dbAuthz, err := sa.GetAuthorization(authz.ID)
421+
test.AssertNotError(t, err, "Could not fetch authorization from database")
422+
assertAuthzEqual(t, authz, dbAuthz)
423+
}
424+
406425
func TestUpdateAuthorization(t *testing.T) {
407426
va, sa, ra, _, cleanUp := initAuthorities(t)
408427
defer cleanUp()

0 commit comments

Comments
 (0)
X Tutup