Move CSR normalization/verification to their own methods (#1826)
* Split CSR testing and name hoisting into own functions, verify CSR in RA & CA * Move tests around and various other fixes * 1.5.3 doesn't have the needed stringer * Move functions to their own lib * Remove unused imports * Move MaxCNLength and BadSignatureAlgorithms to csr package * Always normalizeCSR in VerifyCSR and de-export it * Update comments
This commit is contained in:
Roland Bracewell Shoemaker
parent
5c175f0d9c
commit
5abe7e3cdf
|
|
@ -35,25 +35,10 @@ import (
|
|||
|
||||
"github.com/letsencrypt/boulder/cmd"
|
||||
"github.com/letsencrypt/boulder/core"
|
||||
csrlib "github.com/letsencrypt/boulder/csr"
|
||||
blog "github.com/letsencrypt/boulder/log"
|
||||
)
|
||||
|
||||
// This map is used to detect algorithms in crypto/x509 that
|
||||
// are no longer considered sufficiently strong.
|
||||
// * No MD2, MD5, or SHA-1
|
||||
// * No DSA
|
||||
//
|
||||
// SHA1WithRSA is allowed because there's still a fair bit of it
|
||||
// out there, but we should try to remove it soon.
|
||||
var badSignatureAlgorithms = map[x509.SignatureAlgorithm]bool{
|
||||
x509.UnknownSignatureAlgorithm: true,
|
||||
x509.MD2WithRSA: true,
|
||||
x509.MD5WithRSA: true,
|
||||
x509.DSAWithSHA1: true,
|
||||
x509.DSAWithSHA256: true,
|
||||
x509.ECDSAWithSHA1: true,
|
||||
}
|
||||
|
||||
// Miscellaneous PKIX OIDs that we need to refer to
|
||||
var (
|
||||
// X.509 Extensions
|
||||
|
|
@ -113,9 +98,6 @@ const (
|
|||
// Increments when CA handles a CSR requesting an extension other than those
|
||||
// listed above
|
||||
metricCSRExtensionOther = "CA.CSRExtensions.Other"
|
||||
|
||||
// Maximum length allowed for the common name. RFC 5280
|
||||
maxCNLength = 64
|
||||
)
|
||||
|
||||
type certificateStorage interface {
|
||||
|
|
@ -398,80 +380,10 @@ func (ca *CertificateAuthorityImpl) GenerateOCSP(ctx context.Context, xferObj co
|
|||
func (ca *CertificateAuthorityImpl) IssueCertificate(ctx context.Context, csr x509.CertificateRequest, regID int64) (core.Certificate, error) {
|
||||
emptyCert := core.Certificate{}
|
||||
|
||||
key, ok := csr.PublicKey.(crypto.PublicKey)
|
||||
if !ok {
|
||||
err := core.MalformedRequestError("Invalid public key in CSR.")
|
||||
if err := csrlib.VerifyCSR(&csr, ca.maxNames, &ca.keyPolicy, ca.PA, ca.forceCNFromSAN, regID); err != nil {
|
||||
// AUDIT[ Certificate Requests ] 11917fa4-10ef-4e0d-9105-bacbe7836a3c
|
||||
ca.log.AuditErr(err)
|
||||
return emptyCert, err
|
||||
}
|
||||
if err := ca.keyPolicy.GoodKey(key); err != nil {
|
||||
err = core.MalformedRequestError(fmt.Sprintf("Invalid public key in CSR: %s", err.Error()))
|
||||
// AUDIT[ Certificate Requests ] 11917fa4-10ef-4e0d-9105-bacbe7836a3c
|
||||
ca.log.AuditErr(err)
|
||||
return emptyCert, err
|
||||
}
|
||||
if badSignatureAlgorithms[csr.SignatureAlgorithm] {
|
||||
err := core.MalformedRequestError("Invalid signature algorithm in CSR")
|
||||
// AUDIT[ Certificate Requests ] 11917fa4-10ef-4e0d-9105-bacbe7836a3c
|
||||
ca.log.AuditErr(err)
|
||||
return emptyCert, err
|
||||
}
|
||||
|
||||
// Pull hostnames from CSR
|
||||
// Authorization is checked by the RA
|
||||
commonName := ""
|
||||
hostNames := make([]string, len(csr.DNSNames))
|
||||
copy(hostNames, csr.DNSNames)
|
||||
|
||||
if len(csr.Subject.CommonName) > 0 {
|
||||
commonName = strings.ToLower(csr.Subject.CommonName)
|
||||
hostNames = append(hostNames, commonName)
|
||||
}
|
||||
|
||||
if len(hostNames) == 0 {
|
||||
err := core.MalformedRequestError("Cannot issue a certificate without a hostname.")
|
||||
// AUDIT[ Certificate Requests ] 11917fa4-10ef-4e0d-9105-bacbe7836a3c
|
||||
ca.log.AuditErr(err)
|
||||
return emptyCert, err
|
||||
}
|
||||
|
||||
// Collapse any duplicate names. Note that this operation may re-order the names
|
||||
hostNames = core.UniqueLowerNames(hostNames)
|
||||
|
||||
if ca.forceCNFromSAN && commonName == "" {
|
||||
commonName = hostNames[0]
|
||||
}
|
||||
|
||||
if len(commonName) > maxCNLength {
|
||||
msg := fmt.Sprintf("Common name was longer than 64 bytes, was %d",
|
||||
len(csr.Subject.CommonName))
|
||||
err := core.MalformedRequestError(msg)
|
||||
// AUDIT[ Certificate Requests ] 11917fa4-10ef-4e0d-9105-bacbe7836a3c
|
||||
ca.log.AuditErr(err)
|
||||
return emptyCert, err
|
||||
}
|
||||
|
||||
if ca.maxNames > 0 && len(hostNames) > ca.maxNames {
|
||||
msg := fmt.Sprintf("Certificate request has %d names, maximum is %d.", len(hostNames), ca.maxNames)
|
||||
ca.log.Info(msg)
|
||||
return emptyCert, core.MalformedRequestError(msg)
|
||||
}
|
||||
|
||||
// Verify that names are allowed by policy
|
||||
var badNames []string
|
||||
for _, name := range hostNames {
|
||||
identifier := core.AcmeIdentifier{Type: core.IdentifierDNS, Value: name}
|
||||
if err := ca.PA.WillingToIssue(identifier, regID); err != nil {
|
||||
ca.log.AuditErr(err)
|
||||
badNames = append(badNames, name)
|
||||
}
|
||||
}
|
||||
if len(badNames) > 0 {
|
||||
err := core.MalformedRequestError(fmt.Sprintf("Policy forbids issuing for: %s", strings.Join(badNames, ", ")))
|
||||
// AUDIT[ Certificate Requests ] 11917fa4-10ef-4e0d-9105-bacbe7836a3c
|
||||
ca.log.AuditErr(err)
|
||||
return emptyCert, err
|
||||
return emptyCert, core.MalformedRequestError(err.Error())
|
||||
}
|
||||
|
||||
requestedExtensions, err := ca.extensionsFromCSR(&csr)
|
||||
|
|
@ -511,13 +423,13 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(ctx context.Context, csr x5
|
|||
serialHex := core.SerialToString(serialBigInt)
|
||||
|
||||
var profile string
|
||||
switch key.(type) {
|
||||
switch csr.PublicKey.(type) {
|
||||
case *rsa.PublicKey:
|
||||
profile = ca.rsaProfile
|
||||
case *ecdsa.PublicKey:
|
||||
profile = ca.ecdsaProfile
|
||||
default:
|
||||
err = core.InternalServerError(fmt.Sprintf("unsupported key type %T", key))
|
||||
err = core.InternalServerError(fmt.Sprintf("unsupported key type %T", csr.PublicKey))
|
||||
// AUDIT[ Certificate Requests ] 11917fa4-10ef-4e0d-9105-bacbe7836a3c
|
||||
ca.log.AuditErr(err)
|
||||
return emptyCert, err
|
||||
|
|
@ -527,9 +439,9 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(ctx context.Context, csr x5
|
|||
req := signer.SignRequest{
|
||||
Request: csrPEM,
|
||||
Profile: profile,
|
||||
Hosts: hostNames,
|
||||
Hosts: csr.DNSNames,
|
||||
Subject: &signer.Subject{
|
||||
CN: commonName,
|
||||
CN: csr.Subject.CommonName,
|
||||
},
|
||||
Serial: serialBigInt,
|
||||
Extensions: requestedExtensions,
|
||||
|
|
@ -539,7 +451,7 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(ctx context.Context, csr x5
|
|||
}
|
||||
|
||||
ca.log.AuditInfo(fmt.Sprintf("Signing: serial=[%s] names=[%s] csr=[%s]",
|
||||
serialHex, strings.Join(hostNames, ", "), csrPEM))
|
||||
serialHex, strings.Join(csr.DNSNames, ", "), csrPEM))
|
||||
|
||||
certPEM, err := issuer.eeSigner.Sign(req)
|
||||
ca.noteSignError(err)
|
||||
|
|
@ -551,7 +463,7 @@ func (ca *CertificateAuthorityImpl) IssueCertificate(ctx context.Context, csr x5
|
|||
}
|
||||
|
||||
ca.log.AuditInfo(fmt.Sprintf("Signing success: serial=[%s] names=[%s] csr=[%s] pem=[%s]",
|
||||
serialHex, strings.Join(hostNames, ", "), csrPEM,
|
||||
serialHex, strings.Join(csr.DNSNames, ", "), csrPEM,
|
||||
certPEM))
|
||||
|
||||
if len(certPEM) == 0 {
|
||||
|
|
|
|||
|
|
@ -36,12 +36,6 @@ var (
|
|||
// * DNSNames = not-example.com, www.not-example.com
|
||||
CNandSANCSR = mustRead("./testdata/cn_and_san.der.csr")
|
||||
|
||||
// CSR generated by Go:
|
||||
// * Random public key
|
||||
// * CN = not-example.com
|
||||
// * DNSNames = [none]
|
||||
NoSANCSR = mustRead("./testdata/no_san.der.csr")
|
||||
|
||||
// CSR generated by Go:
|
||||
// * Random public key
|
||||
// * C = US
|
||||
|
|
@ -56,12 +50,6 @@ var (
|
|||
// * DNSNames = [none]
|
||||
NoNameCSR = mustRead("./testdata/no_name.der.csr")
|
||||
|
||||
// CSR generated by Go:
|
||||
// * Random public key
|
||||
// * CN = [none]
|
||||
// * DNSNames = a.example.com, a.example.com
|
||||
DupeNameCSR = mustRead("./testdata/dupe_name.der.csr")
|
||||
|
||||
// CSR generated by Go:
|
||||
// * Random public key
|
||||
// * CN = [none]
|
||||
|
|
@ -74,13 +62,6 @@ var (
|
|||
// * DNSNames = not-example.com, www.not-example.com, mail.not-example.com
|
||||
ShortKeyCSR = mustRead("./testdata/short_key.der.csr")
|
||||
|
||||
// CSR generated by Go:
|
||||
// * Random public key
|
||||
// * CN = (none)
|
||||
// * DNSNames = not-example.com, www.not-example.com, mail.not-example.com
|
||||
// * Signature algorithm: SHA1WithRSA
|
||||
BadAlgorithmCSR = mustRead("./testdata/bad_algorithm.der.csr")
|
||||
|
||||
// CSR generated by Go:
|
||||
// * Random public key
|
||||
// * CN = CapiTalizedLetters.com
|
||||
|
|
@ -305,53 +286,36 @@ func TestIssueCertificate(t *testing.T) {
|
|||
sa := &mockSA{}
|
||||
ca.SA = sa
|
||||
|
||||
csrs := [][]byte{CNandSANCSR, NoSANCSR}
|
||||
for _, csrDER := range csrs {
|
||||
csr, _ := x509.ParseCertificateRequest(csrDER)
|
||||
csr, _ := x509.ParseCertificateRequest(CNandSANCSR)
|
||||
|
||||
// Sign CSR
|
||||
issuedCert, err := ca.IssueCertificate(ctx, *csr, 1001)
|
||||
test.AssertNotError(t, err, "Failed to sign certificate")
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
// Sign CSR
|
||||
issuedCert, err := ca.IssueCertificate(ctx, *csr, 1001)
|
||||
test.AssertNotError(t, err, "Failed to sign certificate")
|
||||
|
||||
// Verify cert contents
|
||||
cert, err := x509.ParseCertificate(issuedCert.DER)
|
||||
test.AssertNotError(t, err, "Certificate failed to parse")
|
||||
// Verify cert contents
|
||||
cert, err := x509.ParseCertificate(issuedCert.DER)
|
||||
test.AssertNotError(t, err, "Certificate failed to parse")
|
||||
|
||||
test.AssertEquals(t, cert.Subject.CommonName, "not-example.com")
|
||||
test.AssertEquals(t, cert.Subject.CommonName, "not-example.com")
|
||||
|
||||
switch len(cert.DNSNames) {
|
||||
case 1:
|
||||
if cert.DNSNames[0] != "not-example.com" {
|
||||
t.Errorf("Improper list of domain names %v", cert.DNSNames)
|
||||
}
|
||||
case 2:
|
||||
switch {
|
||||
case (cert.DNSNames[0] == "not-example.com" && cert.DNSNames[1] == "www.not-example.com"):
|
||||
t.Log("case 1")
|
||||
case (cert.DNSNames[0] == "www.not-example.com" && cert.DNSNames[1] == "not-example.com"):
|
||||
t.Log("case 2")
|
||||
default:
|
||||
t.Errorf("Improper list of domain names %v", cert.DNSNames)
|
||||
}
|
||||
|
||||
default:
|
||||
if len(cert.DNSNames) == 1 {
|
||||
if cert.DNSNames[0] != "not-example.com" {
|
||||
t.Errorf("Improper list of domain names %v", cert.DNSNames)
|
||||
} else {
|
||||
}
|
||||
|
||||
if len(cert.Subject.Country) > 0 {
|
||||
t.Errorf("Subject contained unauthorized values: %v", cert.Subject)
|
||||
}
|
||||
|
||||
// Verify that the cert got stored in the DB
|
||||
serialString := core.SerialToString(cert.SerialNumber)
|
||||
if cert.Subject.SerialNumber != serialString {
|
||||
t.Errorf("SerialNumber: want %#v, got %#v", serialString, cert.Subject.SerialNumber)
|
||||
}
|
||||
test.Assert(t, bytes.Equal(issuedCert.DER, sa.certificate.DER), "Retrieved cert not equal to issued cert.")
|
||||
t.Errorf("Improper list of domain names %v", cert.DNSNames)
|
||||
}
|
||||
|
||||
if len(cert.Subject.Country) > 0 {
|
||||
t.Errorf("Subject contained unauthorized values: %v", cert.Subject)
|
||||
}
|
||||
|
||||
// Verify that the cert got stored in the DB
|
||||
serialString := core.SerialToString(cert.SerialNumber)
|
||||
if cert.Subject.SerialNumber != serialString {
|
||||
t.Errorf("SerialNumber: want %#v, got %#v", serialString, cert.Subject.SerialNumber)
|
||||
}
|
||||
test.Assert(t, bytes.Equal(issuedCert.DER, sa.certificate.DER), "Retrieved cert not equal to issued cert.")
|
||||
}
|
||||
|
||||
// Test issuing when multiple issuers are present.
|
||||
|
|
@ -527,33 +491,6 @@ func TestRejectTooManyNames(t *testing.T) {
|
|||
test.Assert(t, ok, "Incorrect error type returned")
|
||||
}
|
||||
|
||||
func TestDeduplication(t *testing.T) {
|
||||
testCtx := setup(t)
|
||||
ca, err := NewCertificateAuthorityImpl(
|
||||
testCtx.caConfig,
|
||||
testCtx.fc,
|
||||
testCtx.stats,
|
||||
testCtx.issuers,
|
||||
testCtx.keyPolicy)
|
||||
test.AssertNotError(t, err, "Failed to create CA")
|
||||
ca.Publisher = &mocks.Publisher{}
|
||||
ca.PA = testCtx.pa
|
||||
ca.SA = &mockSA{}
|
||||
|
||||
// Test that the CA collapses duplicate names
|
||||
csr, _ := x509.ParseCertificateRequest(DupeNameCSR)
|
||||
cert, err := ca.IssueCertificate(ctx, *csr, 1001)
|
||||
test.AssertNotError(t, err, "Failed to gracefully handle a CSR with duplicate names")
|
||||
|
||||
parsedCert, err := x509.ParseCertificate(cert.DER)
|
||||
test.AssertNotError(t, err, "Error parsing certificate produced by CA")
|
||||
|
||||
correctName := "a.not-example.com"
|
||||
correctNames := len(parsedCert.DNSNames) == 1 &&
|
||||
parsedCert.DNSNames[0] == correctName
|
||||
test.Assert(t, correctNames, "Incorrect set of names in deduplicated certificate")
|
||||
}
|
||||
|
||||
func TestRejectValidityTooLong(t *testing.T) {
|
||||
testCtx := setup(t)
|
||||
ca, err := NewCertificateAuthorityImpl(
|
||||
|
|
@ -659,52 +596,6 @@ func TestLongCommonName(t *testing.T) {
|
|||
test.Assert(t, ok, "Incorrect error type returned")
|
||||
}
|
||||
|
||||
func TestRejectBadAlgorithm(t *testing.T) {
|
||||
testCtx := setup(t)
|
||||
ca, err := NewCertificateAuthorityImpl(
|
||||
testCtx.caConfig,
|
||||
testCtx.fc,
|
||||
testCtx.stats,
|
||||
testCtx.issuers,
|
||||
testCtx.keyPolicy)
|
||||
ca.Publisher = &mocks.Publisher{}
|
||||
ca.PA = testCtx.pa
|
||||
ca.SA = &mockSA{}
|
||||
|
||||
// Test that the CA rejects CSRs that would expire after the intermediate cert
|
||||
csr, _ := x509.ParseCertificateRequest(BadAlgorithmCSR)
|
||||
_, err = ca.IssueCertificate(ctx, *csr, 1001)
|
||||
test.AssertError(t, err, "Issued a certificate based on a CSR with a weak algorithm.")
|
||||
_, ok := err.(core.MalformedRequestError)
|
||||
test.Assert(t, ok, "Incorrect error type returned")
|
||||
}
|
||||
|
||||
func TestCapitalizedLetters(t *testing.T) {
|
||||
testCtx := setup(t)
|
||||
testCtx.caConfig.MaxNames = 3
|
||||
ca, err := NewCertificateAuthorityImpl(
|
||||
testCtx.caConfig,
|
||||
testCtx.fc,
|
||||
testCtx.stats,
|
||||
testCtx.issuers,
|
||||
testCtx.keyPolicy)
|
||||
ca.Publisher = &mocks.Publisher{}
|
||||
ca.PA = testCtx.pa
|
||||
ca.SA = &mockSA{}
|
||||
|
||||
csr, _ := x509.ParseCertificateRequest(CapitalizedCSR)
|
||||
cert, err := ca.IssueCertificate(ctx, *csr, 1001)
|
||||
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)
|
||||
t.Logf("subject serial number %#v", parsedCert.Subject.SerialNumber)
|
||||
}
|
||||
|
||||
func TestWrongSignature(t *testing.T) {
|
||||
testCtx := setup(t)
|
||||
testCtx.caConfig.MaxNames = 3
|
||||
|
|
|
|||
|
|
@ -61,7 +61,7 @@ func main() {
|
|||
|
||||
rai := ra.NewRegistrationAuthorityImpl(clock.Default(), logger, stats,
|
||||
dc, rateLimitPolicies, c.RA.MaxContactsPerRegistration, c.KeyPolicy(),
|
||||
c.RA.UseNewVARPC)
|
||||
c.RA.UseNewVARPC, c.RA.MaxNames, c.RA.DoNotForceCN)
|
||||
rai.PA = pa
|
||||
raDNSTimeout, err := time.ParseDuration(c.Common.DNSTimeout)
|
||||
cmd.FailOnError(err, "Couldn't parse RA DNS timeout")
|
||||
|
|
|
|||
|
|
@ -67,6 +67,9 @@ type Config struct {
|
|||
// before giving up. May be short-circuited by deadlines. A zero value
|
||||
// will be turned into 1.
|
||||
DNSTries int
|
||||
|
||||
MaxNames int
|
||||
DoNotForceCN bool
|
||||
}
|
||||
|
||||
SA struct {
|
||||
|
|
|
|||
|
|
@ -15,9 +15,10 @@ import (
|
|||
"sort"
|
||||
"testing"
|
||||
|
||||
"github.com/square/go-jose"
|
||||
|
||||
"github.com/letsencrypt/boulder/probs"
|
||||
"github.com/letsencrypt/boulder/test"
|
||||
"github.com/square/go-jose"
|
||||
)
|
||||
|
||||
// challenges.go
|
||||
|
|
|
|||
|
|
@ -0,0 +1,88 @@
|
|||
package csr
|
||||
|
||||
import (
|
||||
"crypto"
|
||||
"crypto/x509"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
"github.com/letsencrypt/boulder/core"
|
||||
)
|
||||
|
||||
// maxCNLength is the maximum length allowed for the common name as specified in RFC 5280
|
||||
const maxCNLength = 64
|
||||
|
||||
// This map is used to detect algorithms in crypto/x509 that
|
||||
// are no longer considered sufficiently strong.
|
||||
// * No MD2, MD5, or SHA-1
|
||||
// * No DSA
|
||||
//
|
||||
// SHA1WithRSA is allowed because there's still a fair bit of it
|
||||
// out there, but we should try to remove it soon.
|
||||
var badSignatureAlgorithms = map[x509.SignatureAlgorithm]bool{
|
||||
x509.UnknownSignatureAlgorithm: true,
|
||||
x509.MD2WithRSA: true,
|
||||
x509.MD5WithRSA: true,
|
||||
x509.DSAWithSHA1: true,
|
||||
x509.DSAWithSHA256: true,
|
||||
x509.ECDSAWithSHA1: true,
|
||||
}
|
||||
|
||||
// VerifyCSR checks the validity of a x509.CertificateRequest. Before doing checks it normalizes
|
||||
// the CSR which lowers the case of DNS names and subject CN, and if forceCNFromSAN is true it
|
||||
// will hoist a DNS name into the CN if it is empty.
|
||||
func VerifyCSR(csr *x509.CertificateRequest, maxNames int, keyPolicy *core.KeyPolicy, pa core.PolicyAuthority, forceCNFromSAN bool, regID int64) error {
|
||||
normalizeCSR(csr, forceCNFromSAN)
|
||||
key, ok := csr.PublicKey.(crypto.PublicKey)
|
||||
if !ok {
|
||||
return errors.New("invalid public key in CSR")
|
||||
}
|
||||
if err := keyPolicy.GoodKey(key); err != nil {
|
||||
return fmt.Errorf("invalid public key in CSR: %s", err)
|
||||
}
|
||||
if badSignatureAlgorithms[csr.SignatureAlgorithm] {
|
||||
// go1.6 provides a stringer for x509.SignatureAlgorithm but 1.5.x
|
||||
// does not
|
||||
return errors.New("signature algorithm not supported")
|
||||
}
|
||||
if err := csr.CheckSignature(); err != nil {
|
||||
return errors.New("invalid signature on CSR")
|
||||
}
|
||||
if len(csr.DNSNames) == 0 && csr.Subject.CommonName == "" {
|
||||
return errors.New("at least one DNS name is required")
|
||||
}
|
||||
if len(csr.Subject.CommonName) > maxCNLength {
|
||||
return fmt.Errorf("CN was longer than %d bytes", maxCNLength)
|
||||
}
|
||||
if maxNames > 0 && len(csr.DNSNames) > maxNames {
|
||||
return fmt.Errorf("CSR contains more than %d DNS names", maxNames)
|
||||
}
|
||||
badNames := []string{}
|
||||
for _, name := range csr.DNSNames {
|
||||
if err := pa.WillingToIssue(core.AcmeIdentifier{
|
||||
Type: core.IdentifierDNS,
|
||||
Value: name,
|
||||
}, regID); err != nil {
|
||||
badNames = append(badNames, name)
|
||||
}
|
||||
}
|
||||
if len(badNames) > 0 {
|
||||
return fmt.Errorf("policy forbids issuing for: %s", strings.Join(badNames, ", "))
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// normalizeCSR deduplicates and lowers the case of dNSNames and the subject CN.
|
||||
// If forceCNFromSAN is true it will also hoist a dNSName into the CN if it is empty.
|
||||
func normalizeCSR(csr *x509.CertificateRequest, forceCNFromSAN bool) {
|
||||
if forceCNFromSAN && csr.Subject.CommonName == "" {
|
||||
if len(csr.DNSNames) > 0 {
|
||||
csr.Subject.CommonName = csr.DNSNames[0]
|
||||
}
|
||||
} else if csr.Subject.CommonName != "" {
|
||||
csr.DNSNames = append(csr.DNSNames, csr.Subject.CommonName)
|
||||
}
|
||||
csr.Subject.CommonName = strings.ToLower(csr.Subject.CommonName)
|
||||
csr.DNSNames = core.UniqueLowerNames(csr.DNSNames)
|
||||
}
|
||||
|
|
@ -0,0 +1,172 @@
|
|||
package csr
|
||||
|
||||
import (
|
||||
"crypto/rand"
|
||||
"crypto/rsa"
|
||||
"crypto/x509"
|
||||
"crypto/x509/pkix"
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/square/go-jose"
|
||||
|
||||
"github.com/letsencrypt/boulder/core"
|
||||
"github.com/letsencrypt/boulder/test"
|
||||
)
|
||||
|
||||
var testingPolicy = &core.KeyPolicy{
|
||||
AllowRSA: true,
|
||||
AllowECDSANISTP256: true,
|
||||
AllowECDSANISTP384: true,
|
||||
}
|
||||
|
||||
type mockPA struct{}
|
||||
|
||||
func (pa *mockPA) ChallengesFor(identifier core.AcmeIdentifier, key *jose.JsonWebKey) (challenges []core.Challenge, combinations [][]int) {
|
||||
return
|
||||
}
|
||||
|
||||
func (pa *mockPA) WillingToIssue(id core.AcmeIdentifier, regID int64) error {
|
||||
if id.Value == "bad-name.com" {
|
||||
return errors.New("")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func TestVerifyCSR(t *testing.T) {
|
||||
private, err := rsa.GenerateKey(rand.Reader, 2048)
|
||||
test.AssertNotError(t, err, "error generating test key")
|
||||
signedReqBytes, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{PublicKey: private.PublicKey, SignatureAlgorithm: x509.SHA256WithRSA}, private)
|
||||
test.AssertNotError(t, err, "error generating test CSR")
|
||||
signedReq, err := x509.ParseCertificateRequest(signedReqBytes)
|
||||
test.AssertNotError(t, err, "error parsing test CSR")
|
||||
brokenSignedReq := new(x509.CertificateRequest)
|
||||
*brokenSignedReq = *signedReq
|
||||
brokenSignedReq.Signature = []byte{1, 1, 1, 1}
|
||||
signedReqWithHosts := new(x509.CertificateRequest)
|
||||
*signedReqWithHosts = *signedReq
|
||||
signedReqWithHosts.DNSNames = []string{"a.com", "b.com"}
|
||||
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"}
|
||||
|
||||
cases := []struct {
|
||||
csr *x509.CertificateRequest
|
||||
maxNames int
|
||||
keyPolicy *core.KeyPolicy
|
||||
pa core.PolicyAuthority
|
||||
regID int64
|
||||
expectedError error
|
||||
}{
|
||||
{
|
||||
&x509.CertificateRequest{},
|
||||
0,
|
||||
testingPolicy,
|
||||
&mockPA{},
|
||||
0,
|
||||
errors.New("invalid public key in CSR"),
|
||||
},
|
||||
{
|
||||
&x509.CertificateRequest{PublicKey: private.PublicKey},
|
||||
1,
|
||||
testingPolicy,
|
||||
&mockPA{},
|
||||
0,
|
||||
errors.New("signature algorithm not supported"),
|
||||
},
|
||||
{
|
||||
brokenSignedReq,
|
||||
1,
|
||||
testingPolicy,
|
||||
&mockPA{},
|
||||
0,
|
||||
errors.New("invalid signature on CSR"),
|
||||
},
|
||||
{
|
||||
signedReq,
|
||||
1,
|
||||
testingPolicy,
|
||||
&mockPA{},
|
||||
0,
|
||||
errors.New("at least one DNS name is required"),
|
||||
},
|
||||
{
|
||||
signedReqWithLongCN,
|
||||
1,
|
||||
testingPolicy,
|
||||
&mockPA{},
|
||||
0,
|
||||
errors.New("CN was longer than 64 bytes"),
|
||||
},
|
||||
{
|
||||
signedReqWithHosts,
|
||||
1,
|
||||
testingPolicy,
|
||||
&mockPA{},
|
||||
0,
|
||||
errors.New("CSR contains more than 1 DNS names"),
|
||||
},
|
||||
{
|
||||
signedReqWithBadName,
|
||||
1,
|
||||
testingPolicy,
|
||||
&mockPA{},
|
||||
0,
|
||||
errors.New("policy forbids issuing for: bad-name.com"),
|
||||
},
|
||||
}
|
||||
|
||||
for _, c := range cases {
|
||||
err := VerifyCSR(c.csr, c.maxNames, c.keyPolicy, c.pa, false, c.regID)
|
||||
test.AssertDeepEquals(t, c.expectedError, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeCSR(t *testing.T) {
|
||||
cases := []struct {
|
||||
csr *x509.CertificateRequest
|
||||
forceCN bool
|
||||
expectedCN string
|
||||
expectedNames []string
|
||||
}{
|
||||
{
|
||||
&x509.CertificateRequest{DNSNames: []string{"a.com"}},
|
||||
true,
|
||||
"a.com",
|
||||
[]string{"a.com"},
|
||||
},
|
||||
{
|
||||
&x509.CertificateRequest{Subject: pkix.Name{CommonName: "A.com"}, DNSNames: []string{"a.com"}},
|
||||
true,
|
||||
"a.com",
|
||||
[]string{"a.com"},
|
||||
},
|
||||
{
|
||||
&x509.CertificateRequest{DNSNames: []string{"a.com"}},
|
||||
false,
|
||||
"",
|
||||
[]string{"a.com"},
|
||||
},
|
||||
{
|
||||
&x509.CertificateRequest{DNSNames: []string{"a.com", "a.com"}},
|
||||
false,
|
||||
"",
|
||||
[]string{"a.com"},
|
||||
},
|
||||
{
|
||||
&x509.CertificateRequest{Subject: pkix.Name{CommonName: "A.com"}, DNSNames: []string{"B.com"}},
|
||||
false,
|
||||
"a.com",
|
||||
[]string{"a.com", "b.com"},
|
||||
},
|
||||
}
|
||||
for _, c := range cases {
|
||||
normalizeCSR(c.csr, c.forceCN)
|
||||
test.AssertEquals(t, c.expectedCN, c.csr.Subject.CommonName)
|
||||
test.AssertDeepEquals(t, c.expectedNames, c.expectedNames)
|
||||
}
|
||||
}
|
||||
|
|
@ -27,6 +27,7 @@ import (
|
|||
"github.com/letsencrypt/boulder/bdns"
|
||||
"github.com/letsencrypt/boulder/cmd"
|
||||
"github.com/letsencrypt/boulder/core"
|
||||
csrlib "github.com/letsencrypt/boulder/csr"
|
||||
blog "github.com/letsencrypt/boulder/log"
|
||||
)
|
||||
|
||||
|
|
@ -65,6 +66,8 @@ type RegistrationAuthorityImpl struct {
|
|||
lastIssuedCount *time.Time
|
||||
maxContactsPerReg int
|
||||
useNewVARPC bool
|
||||
maxNames int
|
||||
forceCNFromSAN bool
|
||||
|
||||
regByIPStats metrics.Scope
|
||||
pendAuthByRegIDStats metrics.Scope
|
||||
|
|
@ -73,7 +76,7 @@ type RegistrationAuthorityImpl struct {
|
|||
}
|
||||
|
||||
// NewRegistrationAuthorityImpl constructs a new RA object.
|
||||
func NewRegistrationAuthorityImpl(clk clock.Clock, logger blog.Logger, stats statsd.Statter, dc *DomainCheck, policies cmd.RateLimitConfig, maxContactsPerReg int, keyPolicy core.KeyPolicy, newVARPC bool) *RegistrationAuthorityImpl {
|
||||
func NewRegistrationAuthorityImpl(clk clock.Clock, logger blog.Logger, stats statsd.Statter, dc *DomainCheck, policies cmd.RateLimitConfig, maxContactsPerReg int, keyPolicy core.KeyPolicy, newVARPC bool, maxNames int, forceCNFromSAN bool) *RegistrationAuthorityImpl {
|
||||
// TODO(jmhodges): making RA take a "RA" stats.Scope, not Statter
|
||||
scope := metrics.NewStatsdScope(stats, "RA")
|
||||
ra := &RegistrationAuthorityImpl{
|
||||
|
|
@ -88,11 +91,12 @@ func NewRegistrationAuthorityImpl(clk clock.Clock, logger blog.Logger, stats sta
|
|||
maxContactsPerReg: maxContactsPerReg,
|
||||
keyPolicy: keyPolicy,
|
||||
useNewVARPC: newVARPC,
|
||||
|
||||
regByIPStats: scope.NewScope("RA", "RateLimit", "RegistrationsByIP"),
|
||||
pendAuthByRegIDStats: scope.NewScope("RA", "RateLimit", "PendingAuthorizationsByRegID"),
|
||||
certsForDomainStats: scope.NewScope("RA", "RateLimit", "CertificatesForDomain"),
|
||||
totalCertsStats: scope.NewScope("RA", "RateLimit", "TotalCertificates"),
|
||||
maxNames: maxNames,
|
||||
forceCNFromSAN: forceCNFromSAN,
|
||||
regByIPStats: scope.NewScope("RA", "RateLimit", "RegistrationsByIP"),
|
||||
pendAuthByRegIDStats: scope.NewScope("RA", "RateLimit", "PendingAuthorizationsByRegID"),
|
||||
certsForDomainStats: scope.NewScope("RA", "RateLimit", "CertificatesForDomain"),
|
||||
totalCertsStats: scope.NewScope("RA", "RateLimit", "TotalCertificates"),
|
||||
}
|
||||
return ra
|
||||
}
|
||||
|
|
@ -413,7 +417,7 @@ func (ra *RegistrationAuthorityImpl) MatchesCSR(cert core.Certificate, csr *x509
|
|||
err = core.InternalServerError("Generated certificate public key doesn't match CSR public key")
|
||||
return
|
||||
}
|
||||
if len(csr.Subject.CommonName) > 0 &&
|
||||
if !ra.forceCNFromSAN && len(csr.Subject.CommonName) > 0 &&
|
||||
parsedCertificate.Subject.CommonName != strings.ToLower(csr.Subject.CommonName) {
|
||||
err = core.InternalServerError("Generated certificate CommonName doesn't match CSR CommonName")
|
||||
return
|
||||
|
|
@ -528,9 +532,8 @@ func (ra *RegistrationAuthorityImpl) NewCertificate(ctx context.Context, req cor
|
|||
|
||||
// Verify the CSR
|
||||
csr := req.CSR
|
||||
if err = csr.CheckSignature(); err != nil {
|
||||
logEvent.Error = err.Error()
|
||||
err = core.UnauthorizedError("Invalid signature on CSR")
|
||||
if err := csrlib.VerifyCSR(csr, ra.maxNames, &ra.keyPolicy, ra.PA, ra.forceCNFromSAN, regID); err != nil {
|
||||
err = core.MalformedRequestError(err.Error())
|
||||
return emptyCert, err
|
||||
}
|
||||
|
||||
|
|
@ -540,9 +543,6 @@ func (ra *RegistrationAuthorityImpl) NewCertificate(ctx context.Context, req cor
|
|||
// Validate that authorization key is authorized for all domains
|
||||
names := make([]string, len(csr.DNSNames))
|
||||
copy(names, csr.DNSNames)
|
||||
if len(csr.Subject.CommonName) > 0 {
|
||||
names = append(names, csr.Subject.CommonName)
|
||||
}
|
||||
|
||||
if len(names) == 0 {
|
||||
err = core.UnauthorizedError("CSR has no names in it")
|
||||
|
|
|
|||
|
|
@ -215,7 +215,7 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut
|
|||
Threshold: 100,
|
||||
Window: cmd.ConfigDuration{Duration: 24 * 90 * time.Hour},
|
||||
},
|
||||
}, 1, testKeyPolicy, false)
|
||||
}, 1, testKeyPolicy, false, 0, true)
|
||||
ra.SA = ssa
|
||||
ra.VA = va
|
||||
ra.CA = ca
|
||||
|
|
|
|||
|
|
@ -171,6 +171,8 @@
|
|||
"useNewVARPC": true,
|
||||
"debugAddr": "localhost:8002",
|
||||
"hostnamePolicyFile": "test/hostname-policy.json",
|
||||
"maxNames": 1000,
|
||||
"doNotForceCN": true,
|
||||
"amqp": {
|
||||
"serverURLFile": "test/secrets/amqp_url",
|
||||
"insecure": true,
|
||||
|
|
|
|||
|
|
@ -165,6 +165,8 @@
|
|||
"dnsTries": 3,
|
||||
"debugAddr": "localhost:8002",
|
||||
"hostnamePolicyFile": "test/hostname-policy.json",
|
||||
"maxNames": 1000,
|
||||
"doNotForceCN": true,
|
||||
"amqp": {
|
||||
"serverURLFile": "test/secrets/amqp_url",
|
||||
"insecure": true,
|
||||
|
|
|
|||
|
|
@ -179,13 +179,13 @@ func (ra *MockRegistrationAuthority) OnValidationUpdate(ctx context.Context, aut
|
|||
return nil
|
||||
}
|
||||
|
||||
type MockPA struct{}
|
||||
type mockPA struct{}
|
||||
|
||||
func (pa *MockPA) ChallengesFor(identifier core.AcmeIdentifier, key *jose.JsonWebKey) (challenges []core.Challenge, combinations [][]int) {
|
||||
func (pa *mockPA) ChallengesFor(identifier core.AcmeIdentifier, key *jose.JsonWebKey) (challenges []core.Challenge, combinations [][]int) {
|
||||
return
|
||||
}
|
||||
|
||||
func (pa *MockPA) WillingToIssue(id core.AcmeIdentifier, regID int64) error {
|
||||
func (pa *mockPA) WillingToIssue(id core.AcmeIdentifier, regID int64) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
|
|
@ -578,12 +578,12 @@ func TestIssueCertificate(t *testing.T) {
|
|||
// TODO: Use a mock RA so we can test various conditions of authorized, not
|
||||
// authorized, etc.
|
||||
stats, _ := statsd.NewNoopClient(nil)
|
||||
ra := ra.NewRegistrationAuthorityImpl(fc, wfe.log, stats, nil, cmd.RateLimitConfig{}, 0, testKeyPolicy, false)
|
||||
ra := ra.NewRegistrationAuthorityImpl(fc, wfe.log, stats, nil, cmd.RateLimitConfig{}, 0, testKeyPolicy, false, 0, true)
|
||||
ra.SA = mocks.NewStorageAuthority(fc)
|
||||
ra.CA = &mocks.MockCA{
|
||||
PEM: mockCertPEM,
|
||||
}
|
||||
ra.PA = &MockPA{}
|
||||
ra.PA = &mockPA{}
|
||||
wfe.RA = ra
|
||||
responseWriter := httptest.NewRecorder()
|
||||
|
||||
|
|
@ -653,7 +653,7 @@ func TestIssueCertificate(t *testing.T) {
|
|||
}`, wfe.nonceService)))
|
||||
assertJSONEquals(t,
|
||||
responseWriter.Body.String(),
|
||||
`{"type":"urn:acme:error:unauthorized","detail":"Error creating new cert :: Invalid signature on CSR","status":403}`)
|
||||
`{"type":"urn:acme:error:malformed","detail":"Error creating new cert :: invalid signature on CSR","status":400}`)
|
||||
|
||||
// Valid, signed JWS body, payload has a valid CSR but no authorizations:
|
||||
// openssl req -outform der -new -nodes -key wfe/test/178.key -subj /CN=meep.com | b64url
|
||||
|
|
|
|||
Loading…
Reference in New Issue