diff --git a/acme/authority.go b/acme/authority.go index ddbc9213..286a7218 100644 --- a/acme/authority.go +++ b/acme/authority.go @@ -51,7 +51,7 @@ var ( challengeTable = []byte("acme_challenges") nonceTable = []byte("nonces") orderTable = []byte("acme_orders") - ordersByAccountIDTable = []byte("acme_account-orders-index") + ordersByAccountIDTable = []byte("acme_account_orders_index") certTable = []byte("acme_certs") ) diff --git a/acme/challenge.go b/acme/challenge.go index 4fa39668..f0180f64 100644 --- a/acme/challenge.go +++ b/acme/challenge.go @@ -385,11 +385,17 @@ func (dc *dns01Challenge) validate(db nosql.DB, jwk *jose.JSONWebKey, vo validat return dc, nil } - txtRecords, err := vo.lookupTxt("_acme-challenge." + dc.Value) + // Normalize domain for wildcard DNS names + // This is done to avoid making TXT lookups for domains like + // _acme-challenge.*.example.com + // Instead perform txt lookup for _acme-challenge.example.com + domain := strings.TrimPrefix(dc.Value, "*.") + + txtRecords, err := vo.lookupTxt("_acme-challenge." + domain) if err != nil { if err = dc.storeError(db, DNSErr(errors.Wrapf(err, "error looking up TXT "+ - "records for domain %s", dc.Value))); err != nil { + "records for domain %s", domain))); err != nil { return nil, err } return dc, nil diff --git a/acme/challenge_test.go b/acme/challenge_test.go index 6291803d..720321e5 100644 --- a/acme/challenge_test.go +++ b/acme/challenge_test.go @@ -930,6 +930,47 @@ func TestDNS01Validate(t *testing.T) { res: ch, } }, + "ok/lookup-txt-wildcard": func(t *testing.T) test { + ch, err := newDNSCh() + assert.FatalError(t, err) + _ch, ok := ch.(*dns01Challenge) + assert.Fatal(t, ok) + _ch.baseChallenge.Value = "*.zap.internal" + + jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0) + assert.FatalError(t, err) + + expKeyAuth, err := KeyAuthorization(ch.getToken(), jwk) + assert.FatalError(t, err) + h := sha256.Sum256([]byte(expKeyAuth)) + expected := base64.RawURLEncoding.EncodeToString(h[:]) + + baseClone := ch.clone() + baseClone.Status = StatusValid + baseClone.Error = nil + newCh := &dns01Challenge{baseClone} + + return test{ + ch: ch, + res: newCh, + vo: validateOptions{ + lookupTxt: func(url string) ([]string, error) { + assert.Equals(t, url, "_acme-challenge.zap.internal") + return []string{"foo", expected}, nil + }, + }, + jwk: jwk, + db: &db.MockNoSQLDB{ + MCmpAndSwap: func(bucket, key, old, newval []byte) ([]byte, bool, error) { + dnsCh, err := unmarshalChallenge(newval) + assert.FatalError(t, err) + assert.Equals(t, dnsCh.getStatus(), StatusValid) + baseClone.Validated = dnsCh.getValidated() + return nil, true, nil + }, + }, + } + }, "fail/key-authorization-gen-error": func(t *testing.T) test { ch, err := newDNSCh() assert.FatalError(t, err) diff --git a/acme/order.go b/acme/order.go index 8d22b7db..27e030e9 100644 --- a/acme/order.go +++ b/acme/order.go @@ -4,7 +4,8 @@ import ( "context" "crypto/x509" "encoding/json" - "reflect" + "sort" + "strings" "time" "github.com/pkg/errors" @@ -253,17 +254,29 @@ func (o *order) finalize(db nosql.DB, csr *x509.CertificateRequest, auth SignAut return nil, ServerInternalErr(errors.Errorf("unexpected status %s for order %s", o.Status, o.ID)) } - // Validate identifier names against CSR alternative names // - csrNames := make(map[string]int) - for _, n := range csr.DNSNames { - csrNames[n] = 1 + // RFC8555: The CSR MUST indicate the exact same set of requested + // identifiers as the initial newOrder request. Identifiers of type "dns" + // MUST appear either in the commonName portion of the requested subject + // name or in an extensionRequest attribute [RFC2985] requesting a + // subjectAltName extension, or both. + if csr.Subject.CommonName != "" { + csr.DNSNames = append(csr.DNSNames, csr.Subject.CommonName) } - orderNames := make(map[string]int) - for _, n := range o.Identifiers { - orderNames[n.Value] = 1 + csr.DNSNames = uniqueLowerNames(csr.DNSNames) + orderNames := make([]string, len(o.Identifiers)) + for i, n := range o.Identifiers { + orderNames[i] = n.Value } - if !reflect.DeepEqual(csrNames, orderNames) { - return nil, BadCSRErr(errors.Errorf("CSR names do not match identifiers exactly")) + orderNames = uniqueLowerNames(orderNames) + + // Validate identifier names against CSR alternative names. + if len(csr.DNSNames) != len(orderNames) { + return nil, BadCSRErr(errors.Errorf("CSR names do not match identifiers exactly: CSR names = %v, Order names = %v", csr.DNSNames, orderNames)) + } + for i := range csr.DNSNames { + if csr.DNSNames[i] != orderNames[i] { + return nil, BadCSRErr(errors.Errorf("CSR names do not match identifiers exactly: CSR names = %v, Order names = %v", csr.DNSNames, orderNames)) + } } // Get authorizations from the ACME provisioner. @@ -340,3 +353,19 @@ func (o *order) toACME(db nosql.DB, dir *directory, p provisioner.Interface) (*O } return ao, nil } + +// 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 +// and sorted alphabetically. +func uniqueLowerNames(names []string) (unique []string) { + nameMap := make(map[string]int, len(names)) + for _, name := range names { + nameMap[strings.ToLower(name)] = 1 + } + unique = make([]string, 0, len(nameMap)) + for name := range nameMap { + unique = append(unique, name) + } + sort.Strings(unique) + return +} diff --git a/acme/order_test.go b/acme/order_test.go index 18a46589..77c21e24 100644 --- a/acme/order_test.go +++ b/acme/order_test.go @@ -906,9 +906,9 @@ func TestOrderFinalize(t *testing.T) { csr := &x509.CertificateRequest{ Subject: pkix.Name{ - CommonName: "foo", + CommonName: "acme.example.com", }, - DNSNames: []string{"bar", "baz"}, + DNSNames: []string{"acme.example.com", "fail.smallstep.com"}, } return test{ o: o, @@ -923,9 +923,9 @@ func TestOrderFinalize(t *testing.T) { csr := &x509.CertificateRequest{ Subject: pkix.Name{ - CommonName: "acme.example.com", + CommonName: "", }, - DNSNames: []string{"step.example.com"}, + DNSNames: []string{"acme.example.com"}, } return test{ o: o, @@ -940,7 +940,7 @@ func TestOrderFinalize(t *testing.T) { csr := &x509.CertificateRequest{ Subject: pkix.Name{ - CommonName: "foo", + CommonName: "acme.example.com", }, DNSNames: []string{"step.example.com", "acme.example.com"}, } @@ -962,7 +962,7 @@ func TestOrderFinalize(t *testing.T) { csr := &x509.CertificateRequest{ Subject: pkix.Name{ - CommonName: "foo", + CommonName: "acme.example.com", }, DNSNames: []string{"step.example.com", "acme.example.com"}, } @@ -982,7 +982,7 @@ func TestOrderFinalize(t *testing.T) { csr := &x509.CertificateRequest{ Subject: pkix.Name{ - CommonName: "foo", + CommonName: "acme.example.com", }, DNSNames: []string{"step.example.com", "acme.example.com"}, } @@ -1017,7 +1017,7 @@ func TestOrderFinalize(t *testing.T) { csr := &x509.CertificateRequest{ Subject: pkix.Name{ - CommonName: "acme", + CommonName: "acme.example.com", }, DNSNames: []string{"acme.example.com", "step.example.com"}, } @@ -1057,7 +1057,7 @@ func TestOrderFinalize(t *testing.T) { csr := &x509.CertificateRequest{ Subject: pkix.Name{ - CommonName: "foo", + CommonName: "acme.example.com", }, DNSNames: []string{"acme.example.com", "step.example.com"}, } @@ -1098,6 +1098,102 @@ func TestOrderFinalize(t *testing.T) { }, } }, + "ok/ready/no-sans": func(t *testing.T) test { + o, err := newO() + assert.FatalError(t, err) + o.Status = StatusReady + o.Identifiers = []Identifier{ + {Type: "dns", Value: "step.example.com"}, + } + + csr := &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "step.example.com", + }, + } + crt := &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "step.example.com", + }, + DNSNames: []string{"step.example.com"}, + } + inter := &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "intermediate", + }, + } + + clone := *o + clone.Status = StatusValid + count := 0 + return test{ + o: o, + res: &clone, + csr: csr, + sa: &mockSignAuth{ + sign: func(csr *x509.CertificateRequest, pops provisioner.Options, signOps ...provisioner.SignOption) ([]*x509.Certificate, error) { + assert.Equals(t, len(signOps), 4) + return []*x509.Certificate{crt, inter}, nil + }, + }, + db: &db.MockNoSQLDB{ + MCmpAndSwap: func(bucket, key, old, newval []byte) ([]byte, bool, error) { + if count == 0 { + clone.Certificate = string(key) + } + count++ + return nil, true, nil + }, + }, + } + }, + "ok/ready/sans-and-name": func(t *testing.T) test { + o, err := newO() + assert.FatalError(t, err) + o.Status = StatusReady + + csr := &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "acme.example.com", + }, + DNSNames: []string{"step.example.com"}, + } + crt := &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "acme.example.com", + }, + DNSNames: []string{"acme.example.com", "step.example.com"}, + } + inter := &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "intermediate", + }, + } + + clone := *o + clone.Status = StatusValid + count := 0 + return test{ + o: o, + res: &clone, + csr: csr, + sa: &mockSignAuth{ + sign: func(csr *x509.CertificateRequest, pops provisioner.Options, signOps ...provisioner.SignOption) ([]*x509.Certificate, error) { + assert.Equals(t, len(signOps), 4) + return []*x509.Certificate{crt, inter}, nil + }, + }, + db: &db.MockNoSQLDB{ + MCmpAndSwap: func(bucket, key, old, newval []byte) ([]byte, bool, error) { + if count == 0 { + clone.Certificate = string(key) + } + count++ + return nil, true, nil + }, + }, + } + }, } for name, run := range tests { t.Run(name, func(t *testing.T) { diff --git a/go.mod b/go.mod index fd44c813..beecf1ad 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/rs/xid v1.2.1 github.com/sirupsen/logrus v1.4.2 github.com/smallstep/assert v0.0.0-20200103212524-b99dc1097b15 - github.com/smallstep/cli v0.14.0-rc.1.0.20200127233252-e55637e57819 + github.com/smallstep/cli v0.14.0-rc.1.0.20200128213701-65805ae554f6 github.com/smallstep/nosql v0.2.0 github.com/urfave/cli v1.22.2 golang.org/x/crypto v0.0.0-20191227163750-53104e6ec876 diff --git a/go.sum b/go.sum index cb09d716..58587fed 100644 --- a/go.sum +++ b/go.sum @@ -445,6 +445,7 @@ github.com/smallstep/certificates v0.14.0-rc.1.0.20191217235337-aa5894058226/go. github.com/smallstep/certificates v0.14.0-rc.1.0.20191218224459-1fa35491ea07/go.mod h1:eEtpedAL4inqaAx6ZqJnE4NOx1/GxDh6VQOmbs7CPf0= github.com/smallstep/certificates v0.14.0-rc.1.0.20200110185849-085ae821636e/go.mod h1:weY9Os8g0yPfyxd+Zy1CTAwCb7YMqg/u5XnEagBN5Rk= github.com/smallstep/certificates v0.14.0-rc.1.0.20200111012147-3ce267cdd6b7/go.mod h1:jljUh6mTvHOAqvIUvbD2L3Q/aqSpTI6HzJiNFQkj1Hc= +github.com/smallstep/certificates v0.14.0-rc.1.0.20200128212940-432ed0090f3d/go.mod h1:lWKe0ZOg45lNWtByxh82fOfzXwx93S0TeWzTCOjc19k= github.com/smallstep/certinfo v0.0.0-20191008000228-b0e530932339/go.mod h1:n4YHPL9hJIyB+N4F2rPBy3mpPxMxTGJP5Pdsyaoc2Ns= github.com/smallstep/certinfo v0.0.0-20191029235839-00563809d483/go.mod h1:xmx5n8+7jI0lrjTUwc8WMMqXeOHRyxYUW9U1wrvP3Vo= github.com/smallstep/certinfo v1.0.0/go.mod h1:xmx5n8+7jI0lrjTUwc8WMMqXeOHRyxYUW9U1wrvP3Vo= @@ -471,6 +472,8 @@ github.com/smallstep/cli v0.14.0-rc.1.0.20200111011727-83a91ec8e405 h1:hvcnKc+fi github.com/smallstep/cli v0.14.0-rc.1.0.20200111011727-83a91ec8e405/go.mod h1:MCvJvfMNtWCi/VBfXxP1JONqLLfF9TcBj1/t5Rqme90= github.com/smallstep/cli v0.14.0-rc.1.0.20200127233252-e55637e57819 h1:mcYdBrClUMvGJn2mBlJNniXC69gyOZNbBwi0GL6++qo= github.com/smallstep/cli v0.14.0-rc.1.0.20200127233252-e55637e57819/go.mod h1:SUBVVdOk5XI7yllSupRYHzN5y4MBo89X27CN4P0d+Jw= +github.com/smallstep/cli v0.14.0-rc.1.0.20200128213701-65805ae554f6 h1:Dh+Au3z44aSzZ+nNEr+9MAdenSqTjtFVrlxlzdoXBNs= +github.com/smallstep/cli v0.14.0-rc.1.0.20200128213701-65805ae554f6/go.mod h1:50kmsPMAiR9XD0jHZYY19fkSSD3mKF9ztQjgtTLefjU= github.com/smallstep/nosql v0.1.1-0.20191009043502-4b26d8029e61 h1:XM3mkHNBc6bEQhrZNEma+iz63xrmRFfCocmAEObeg/s= github.com/smallstep/nosql v0.1.1-0.20191009043502-4b26d8029e61/go.mod h1:MFhYHIE/0V7OOHjYzjnWHqySJ40PVbwhjy24UBkJI2g= github.com/smallstep/nosql v0.1.1 h1:ijeE3CM00SddioodNl/LWRQINNNCK1dLUsjZDwpUbNg=