Merge pull request #930 from letsencrypt/lower_domains

clean up CSRs with capitalized letters

ca/certificate-authority.go

@@ -16,6 +16,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"math/big"
+	"strings"
 	"time"
 
 	"github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/jmhodges/clock"
@@ -223,7 +224,8 @@ func (ca *CertificateAuthorityImpl) RevokeCertificate(serial string, reasonCode
 }
 
 // IssueCertificate attempts to convert a CSR into a signed Certificate, while
-// enforcing all policies.
+// enforcing all policies. Names (domains) in the CertificateRequest will be
+// lowercased before storage.
 func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest, regID int64) (core.Certificate, error) {
 	emptyCert := core.Certificate{}
 	var err error
@@ -253,10 +255,10 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest
 	hostNames := make([]string, len(csr.DNSNames))
 	copy(hostNames, csr.DNSNames)
 	if len(csr.Subject.CommonName) > 0 {
-		commonName = csr.Subject.CommonName
-		hostNames = append(hostNames, csr.Subject.CommonName)
+		commonName = strings.ToLower(csr.Subject.CommonName)
+		hostNames = append(hostNames, commonName)
 	} else if len(hostNames) > 0 {
-		commonName = hostNames[0]
+		commonName = strings.ToLower(hostNames[0])
 	} else {
 		err = core.MalformedRequestError("Cannot issue a certificate without a hostname.")
 		// AUDIT[ Certificate Requests ] 11917fa4-10ef-4e0d-9105-bacbe7836a3c
@@ -265,7 +267,7 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(csr x509.CertificateRequest
 	}
 
 	// Collapse any duplicate names.  Note that this operation may re-order the names
-	hostNames = core.UniqueNames(hostNames)
+	hostNames = core.UniqueLowerNames(hostNames)
 	if ca.MaxNames > 0 && len(hostNames) > ca.MaxNames {
 		err = core.MalformedRequestError(fmt.Sprintf("Certificate request has %d > %d names", len(hostNames), ca.MaxNames))
 		ca.log.WarningErr(err)

ca/certificate-authority_test.go

@@ -11,6 +11,7 @@ import (
 	"encoding/asn1"
 	"fmt"
 	"io/ioutil"
+	"sort"
 	"testing"
 	"time"
 
@@ -64,7 +65,7 @@ var (
 	DupeNameCSR = mustRead("./testdata/dupe_name.der.csr")
 
 	// CSR generated by Go:
-	// * Random pulic key
+	// * Random public key
 	// * CN = [none]
 	// * DNSNames = not-example.com, www.not-example.com, mail.example.com
 	TooManyNameCSR = mustRead("./testdata/too_many_names.der.csr")
@@ -81,7 +82,14 @@ var (
 	// * DNSNames = not-example.com, www.not-example.com, mail.not-example.com
 	// * Signature algorithm: SHA1WithRSA
 	BadAlgorithmCSR = mustRead("./testdata/bad_algorithm.der.csr")
-	log             = mocks.UseMockLog()
+
+	// CSR generated by Go:
+	// * Random public key
+	// * CN = CapiTalizedLetters.com
+	// * DNSNames = moreCAPs.com, morecaps.com, evenMOREcaps.com, Capitalizedletters.COM
+	CapitalizedCSR = mustRead("./testdata/capitalized_cn_and_san.der.csr")
+
+	log = mocks.UseMockLog()
 )
 
 // CFSSL config
@@ -421,3 +429,24 @@ func TestRejectBadAlgorithm(t *testing.T) {
 	_, ok := err.(core.MalformedRequestError)
 	test.Assert(t, ok, "Incorrect error type returned")
 }
+
+func TestCapitalizedLetters(t *testing.T) {
+	ctx := setup(t)
+	defer ctx.cleanUp()
+	ctx.caConfig.MaxNames = 3
+	ca, err := NewCertificateAuthorityImpl(ctx.caConfig, ctx.fc, caCertFile)
+	ca.Publisher = &mocks.Publisher{}
+	ca.PA = ctx.pa
+	ca.SA = ctx.sa
+
+	csr, _ := x509.ParseCertificateRequest(CapitalizedCSR)
+	cert, err := ca.IssueCertificate(*csr, ctx.reg.ID)
+	test.AssertNotError(t, err, "Failed to gracefully handle a CSR with capitalized names")
+
+	parsedCert, err := x509.ParseCertificate(cert.DER)
+	test.AssertNotError(t, err, "Error parsing certificate produced by CA")
+	test.AssertEquals(t, "capitalizedletters.com", parsedCert.Subject.CommonName)
+	sort.Strings(parsedCert.DNSNames)
+	expected := []string{"capitalizedletters.com", "evenmorecaps.com", "morecaps.com"}
+	test.AssertDeepEquals(t, expected, parsedCert.DNSNames)
+}

ca/testdata/testcsr.go

@@ -0,0 +1,69 @@
+// Hack up the x509.CertificateRequest in here, run `go run testcsr.go`, and a
+// DER-encoded CertificateRequest will be printed to stdout.
+package main
+
+import (
+	"crypto/rand"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"log"
+	"os"
+)
+
+// A 2048-bit RSA private key
+var pemPrivateKey = `-----BEGIN RSA PRIVATE KEY-----
+MIIEowIBAAKCAQEA5cpXqfCaUDD+hf93j5jxbrhK4jrJAzfAEjeZj/Lx5Rv/7eEO
+uhS2DdCU2is82vR6yJ7EidUYVz/nUAjSTP7JIEsbyvfsfACABbqRyGltHlJnULVH
+y/EMjt9xKZf17T8tOLHVUEAJTxsvjKn4TMIQJTNrAqm/lNrUXmCIR41Go+3RBGC6
+YdAKEwcZMCzrjQGF06mC6/6xMmYMSMd6+VQRFIPpuPK/6BBp1Tgju2LleRC5uatj
+QcFOoilGkfh1RnZp3GJ7q58KaqHiPmjl31rkY5vS3LP7yfU5TRBcxCSG8l8LKuRt
+MArkbTEtj3PkDjbipL/SkLrZ28e5w9Egl4g1MwIDAQABAoIBABZqY5zPPK5f6SQ3
+JHmciMitL5jb9SncMV9VjyRMpa4cyh1xW9dpF81HMI4Ls7cELEoPuspbQDGaqTzU
+b3dVT1dYHFDzWF1MSzDD3162cg+IKE3mMSfCzt/NCiPtj+7hv86NAmr+pCnUVBIb
+rn4GXD7UwjaTSn4Bzr+aGREpxd9Nr0JdNQwxVHZ75A92vTihCfaXyMCjhW3JEpF9
+N89XehgidoGgtUxxeeb+WsO3nvVBpLv/HDxMTx/IDzvSA5nLlYMcqVzb7IJoeAQu
+og0WJKlniYzvIdoQ6/hGydAW5sKd0qWh0JPYs7uLKAWrdAWvrFAp7//fYKVamalU
+8pUu/WkCgYEA+tcTQ3qTnVh41O9YeM/7NULpIkuCAlR+PBRky294zho9nGQIPdaW
+VNvyqqjLaHaXJVokYHbU4hDk6RbrhoWVd4Po/5g9cUkT1f6nrdZGRkg4XOCzHWvV
+Yrqh3eYYX4bdiH5EhB78m0rrbjHfd7SF3cdYNzOUS2kJvCInYC6zPx8CgYEA6oRr
+UhZFuoqRsEb28ELM8sHvdIMA/C3aWCu+nUGQ4gHSEb4uvuOD/7tQNuCaBioiXVPM
+/4hjk9jHJcjYf5l33ANqIP7JiYAt4rzTWXF3iS6kQOhQhjksSlSnWqw0Uu1DtlpG
+rzeG1ZkBuwH7Bx0yj4sGSz5sAvyF44aRsE6AC20CgYEArafWO0ISDb1hMbFdo44B
+ELd45Pg3UluiZP+NZFWQ4cbC3pFWL1FvE+KNll5zK6fmLcLBKlM6QCOIBmKKvb+f
+YXVeCg0ghFweMmkxNqUAU8nN02bwOa8ctFQWmaOhPgkFN2iLEJjPMsdkRA6c8ad1
+gbtvNBAuWyKlzawrbGgISesCgYBkGEjGLINubx5noqJbQee/5U6S6CdPezKqV2Fw
+NT/ldul2cTn6d5krWYOPKKYU437vXokst8XooKm/Us41CAfEfCCcHKNgcLklAXsj
+ve5LOwEYQw+7ekORJjiX1tAuZN51wmpQ9t4x5LB8ZQgDrU6bPbdd/jKTw7xRtGoS
+Wi8EsQKBgG8iGy3+kVBIjKHxrN5jVs3vj/l/fQL0WRMLCMmVuDBfsKyy3f9n8R1B
+/KdwoyQFwsLOyr5vAjiDgpFurXQbVyH4GDFiJGS1gb6MNcinwSTpsbOLLV7zgibX
+A2NgiQ+UeWMia16dZVd6gGDlY3lQpeyLdsdDd+YppNfy9vedjbvT
+-----END RSA PRIVATE KEY-----`
+
+func main() {
+	block, _ := pem.Decode([]byte(pemPrivateKey))
+	rsaPriv, err := x509.ParsePKCS1PrivateKey(block.Bytes)
+	if err != nil {
+		log.Fatalf("Failed to parse private key: %s", err)
+	}
+
+	req := &x509.CertificateRequest{
+		Subject: pkix.Name{
+			CommonName: "CapiTalizedLetters.com",
+		},
+		DNSNames: []string{
+			"moreCAPs.com",
+			"morecaps.com",
+			"evenMOREcaps.com",
+			"Capitalizedletters.COM",
+		},
+	}
+	csr, err := x509.CreateCertificateRequest(rand.Reader, req, rsaPriv)
+	if err != nil {
+		log.Fatalf("unable to create CSR: %s", err)
+	}
+	_, err = os.Stdout.Write(csr)
+	if err != nil {
+		log.Fatalf("unable to write to stdout: %s", err)
+	}
+}

core/util.go

@@ -472,11 +472,12 @@ func GetBuildHost() (retID string) {
 	return
 }
 
-// UniqueNames returns the set of all unique names in the input.
-func UniqueNames(names []string) (unique []string) {
+// UniqueLowerNames returns the set of all unique names in the input after all
+// of them are lowercased. The returned names will be in their lowercased form.
+func UniqueLowerNames(names []string) (unique []string) {
 	nameMap := make(map[string]int, len(names))
 	for _, name := range names {
-		nameMap[name] = 1
+		nameMap[strings.ToLower(name)] = 1
 	}
 
 	unique = make([]string, 0, len(nameMap))

core/util_test.go

@@ -11,6 +11,7 @@ import (
 	"math"
 	"math/big"
 	"net/url"
+	"sort"
 	"testing"
 
 	"github.com/letsencrypt/boulder/Godeps/_workspace/src/github.com/letsencrypt/go-jose"
@@ -113,3 +114,9 @@ func TestAcmeURL(t *testing.T) {
 	a := (*AcmeURL)(u)
 	test.AssertEquals(t, s, a.String())
 }
+
+func TestUniqueLowerNames(t *testing.T) {
+	u := UniqueLowerNames([]string{"foobar.com", "fooBAR.com", "baz.com", "foobar.com", "bar.com", "bar.com"})
+	sort.Strings(u)
+	test.AssertDeepEquals(t, []string{"bar.com", "baz.com", "foobar.com"}, u)
+}

policy/policy-authority.go

@@ -64,7 +64,7 @@ const (
 	whitelistedPartnerRegID = -1
 )
 
-var dnsLabelRegexp = regexp.MustCompile("^[a-zA-Z0-9][a-zA-Z0-9-]{0,62}$")
+var dnsLabelRegexp = regexp.MustCompile("^[a-z0-9][a-z0-9-]{0,62}$")
 var punycodeRegexp = regexp.MustCompile("^xn--")
 
 func isDNSCharacter(ch byte) bool {
@@ -112,7 +112,8 @@ func (e SyntaxError) Error() string            { return "Syntax error" }
 func (e NonPublicError) Error() string         { return "Name does not end in a public suffix" }
 
 // WillingToIssue determines whether the CA is willing to issue for the provided
-// identifier.
+// identifier. It expects domains in id to be lowercase to prevent mismatched
+// cases breaking queries.
 //
 // We place several criteria on identifiers we are willing to issue for:
 //
@@ -132,8 +133,6 @@ func (e NonPublicError) Error() string         { return "Name does not end in a
 // XXX: Is there any need for this method to be constant-time?  We're
 //      going to refuse to issue anyway, but timing could leak whether
 //      names are on the blacklist.
-//
-// XXX: We should probably fold everything to lower-case somehow.
 func (pa PolicyAuthorityImpl) WillingToIssue(id core.AcmeIdentifier, regID int64) error {
 	if id.Type != core.IdentifierDNS {
 		return InvalidIdentifierError{}
@@ -146,7 +145,6 @@ func (pa PolicyAuthorityImpl) WillingToIssue(id core.AcmeIdentifier, regID int64
 		}
 	}
 
-	domain = strings.ToLower(domain)
 	if len(domain) > 255 {
 		return SyntaxError{}
 	}

policy/policy-authority_test.go

@@ -86,6 +86,9 @@ func TestWillingToIssue(t *testing.T) {
 		`zombocom`,
 		`localhost`,
 		`mail`,
+
+		// disallow capitalized letters for #927
+		`CapitalizedLetters.com`,
 	}
 
 	shouldBeNonPublic := []string{

ra/registration-authority.go

@@ -214,7 +214,8 @@ func validateContacts(contacts []*core.AcmeURL, resolver core.DNSResolver, stats
 	return
 }
 
-// NewAuthorization constuct a new Authz from a request.
+// NewAuthorization constuct a new Authz from a request. Values (domains) in
+// request.Identifier will be lowercased before storage.
 func (ra *RegistrationAuthorityImpl) NewAuthorization(request core.Authorization, regID int64) (authz core.Authorization, err error) {
 	reg, err := ra.SA.GetRegistration(regID)
 	if err != nil {
@@ -223,6 +224,7 @@ func (ra *RegistrationAuthorityImpl) NewAuthorization(request core.Authorization
 	}
 
 	identifier := request.Identifier
+	identifier.Value = strings.ToLower(identifier.Value)
 
 	// Check that the identifier is present and appropriate
 	if err = ra.PA.WillingToIssue(identifier, regID); err != nil {
@@ -303,7 +305,7 @@ func (ra *RegistrationAuthorityImpl) MatchesCSR(
 	if len(csr.Subject.CommonName) > 0 {
 		hostNames = append(hostNames, csr.Subject.CommonName)
 	}
-	hostNames = core.UniqueNames(hostNames)
+	hostNames = core.UniqueLowerNames(hostNames)
 
 	if !core.KeyDigestEquals(parsedCertificate.PublicKey, csr.PublicKey) {
 		err = core.InternalServerError("Generated certificate public key doesn't match CSR public key")

ra/registration-authority_test.go

@@ -409,6 +409,25 @@ func TestNewAuthorization(t *testing.T) {
 	t.Log("DONE TestNewAuthorization")
 }
 
+func TestNewAuthorizationCapitalLetters(t *testing.T) {
+	_, sa, ra, _, cleanUp := initAuthorities(t)
+	defer cleanUp()
+
+	authzReq := core.Authorization{
+		Identifier: core.AcmeIdentifier{
+			Type:  core.IdentifierDNS,
+			Value: "NOT-example.COM",
+		},
+	}
+	authz, err := ra.NewAuthorization(authzReq, Registration.ID)
+	test.AssertNotError(t, err, "NewAuthorization failed")
+	test.AssertEquals(t, "not-example.com", authz.Identifier.Value)
+
+	dbAuthz, err := sa.GetAuthorization(authz.ID)
+	test.AssertNotError(t, err, "Could not fetch authorization from database")
+	assertAuthzEqual(t, authz, dbAuthz)
+}
+
 func TestUpdateAuthorization(t *testing.T) {
 	va, sa, ra, _, cleanUp := initAuthorities(t)
 	defer cleanUp()