From d25e7f64c29433f7abcc2abfaed7fb8f2d7af1bf Mon Sep 17 00:00:00 2001 From: max furman Date: Wed, 24 Jun 2020 09:58:40 -0700 Subject: [PATCH] wip --- acme/order_test.go | 57 ++++++++++ authority/provisioner/sign_options.go | 12 ++- authority/provisioner/sign_options_test.go | 116 ++++++++++++++++++++- authority/tls.go | 2 +- 4 files changed, 181 insertions(+), 6 deletions(-) diff --git a/acme/order_test.go b/acme/order_test.go index 90c8de84..f3894a2a 100644 --- a/acme/order_test.go +++ b/acme/order_test.go @@ -6,6 +6,7 @@ import ( "crypto/x509/pkix" "encoding/json" "fmt" + "net" "net/url" "testing" "time" @@ -1056,6 +1057,62 @@ func TestOrderFinalize(t *testing.T) { err: BadCSRErr(errors.Errorf("CSR names do not match identifiers exactly")), } }, + "fail/ready/no-ipAddresses": func(t *testing.T) test { + o, err := newO() + assert.FatalError(t, err) + o.Status = StatusReady + + csr := &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "", + }, + DNSNames: []string{"acme.example.com", "step.example.com"}, + IPAddresses: []net.IP{net.ParseIP("1.1.1.1")}, + } + return test{ + o: o, + csr: csr, + err: BadCSRErr(errors.Errorf("CSR contains IP Address SANs, but should only contain DNS Names")), + } + }, + "fail/ready/no-emailAddresses": func(t *testing.T) test { + o, err := newO() + assert.FatalError(t, err) + o.Status = StatusReady + + csr := &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "", + }, + DNSNames: []string{"acme.example.com", "step.example.com"}, + EmailAddresses: []string{"max@smallstep.com", "mariano@smallstep.com"}, + } + return test{ + o: o, + csr: csr, + err: BadCSRErr(errors.Errorf("CSR contains Email Address SANs, but should only contain DNS Names")), + } + }, + "fail/ready/no-URIs": func(t *testing.T) test { + o, err := newO() + assert.FatalError(t, err) + o.Status = StatusReady + + u, err := url.Parse("https://google.com") + assert.FatalError(t, err) + csr := &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "", + }, + DNSNames: []string{"acme.example.com", "step.example.com"}, + URIs: []*url.URL{u}, + } + return test{ + o: o, + csr: csr, + err: BadCSRErr(errors.Errorf("CSR contains URI SANs, but should only contain DNS Names")), + } + }, "fail/ready/provisioner-auth-sign-error": func(t *testing.T) test { o, err := newO() assert.FatalError(t, err) diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go index fd82470d..dac0bfb8 100644 --- a/authority/provisioner/sign_options.go +++ b/authority/provisioner/sign_options.go @@ -216,8 +216,12 @@ func (v urisValidator) Valid(req *x509.CertificateRequest) error { return nil } +// defaultsSANsValidator stores a set of SANs to eventually validate 1:1 against +// the SANs in an x509 certificate request. type defaultSANsValidator []string +// Valid verifies that the SANs stored in the validator match 1:1 with those +// requested in the x509 certificate request. func (v defaultSANsValidator) Valid(req *x509.CertificateRequest) (err error) { dnsNames, ips, emails, uris := x509util.SplitSANs(v) if err = dnsNamesValidator(dnsNames).Valid(req); err != nil { @@ -232,15 +236,15 @@ func (v defaultSANsValidator) Valid(req *x509.CertificateRequest) (err error) { return } -// ExtraExtensionsEnforcer enforces only those extra extensions that are strictly +// ExtraExtsEnforcer enforces only those extra extensions that are strictly // managed by step-ca. All other "extra extensions" are dropped. -type ExtraExtensionsEnforcer struct{} +type ExtraExtsEnforcer struct{} // Enforce removes all extensions except the step provisioner extension, if it // exists. If the step provisioner extension is not present, then remove all // extra extensions from the cert. -func (eee ExtraExtensionsEnforcer) Enforce(cert *x509.Certificate) error { - for _, ext := range cert.Extensions { +func (eee ExtraExtsEnforcer) Enforce(cert *x509.Certificate) error { + for _, ext := range cert.ExtraExtensions { if ext.Id.Equal(stepOIDProvisioner) { cert.ExtraExtensions = []pkix.Extension{ext} return nil diff --git a/authority/provisioner/sign_options_test.go b/authority/provisioner/sign_options_test.go index 6a8d3069..62e285e9 100644 --- a/authority/provisioner/sign_options_test.go +++ b/authority/provisioner/sign_options_test.go @@ -251,7 +251,7 @@ func Test_urisValidator_Valid(t *testing.T) { assert.FatalError(t, err) u2, err := url.Parse("https://google.com/index.html") assert.FatalError(t, err) - u3, err := url.Parse("https://foo.bar.baz") + u3, err := url.Parse("urn:uuid:ddfe62ba-7e99-4bc1-83b3-8f57fe3e9959") assert.FatalError(t, err) fu, err := url.Parse("https://unexpected.com") assert.FatalError(t, err) @@ -282,6 +282,120 @@ func Test_urisValidator_Valid(t *testing.T) { } } +func Test_defaultSANsValidator_Valid(t *testing.T) { + type test struct { + csr *x509.CertificateRequest + expectedSANs []string + err error + } + tests := map[string]func() test{ + "fail/dnsNamesValidator": func() test { + return test{ + csr: &x509.CertificateRequest{DNSNames: []string{"foo", "bar"}}, + expectedSANs: []string{"foo"}, + err: errors.New("certificate request does not contain the valid DNS names"), + } + }, + "fail/emailAddressesValidator": func() test { + return test{ + csr: &x509.CertificateRequest{EmailAddresses: []string{"max@fx.com", "mariano@fx.com"}}, + expectedSANs: []string{"dcow@fx.com"}, + err: errors.New("certificate request does not contain the valid Email Addresses"), + } + }, + "fail/ipAddressesValidator": func() test { + return test{ + csr: &x509.CertificateRequest{IPAddresses: []net.IP{net.ParseIP("1.1.1.1"), net.ParseIP("127.0.0.1")}}, + expectedSANs: []string{"127.0.0.1"}, + err: errors.New("IP Addresses claim failed"), + } + }, + "fail/urisValidator": func() test { + u1, err := url.Parse("https://google.com") + assert.FatalError(t, err) + u2, err := url.Parse("urn:uuid:ddfe62ba-7e99-4bc1-83b3-8f57fe3e9959") + assert.FatalError(t, err) + return test{ + csr: &x509.CertificateRequest{URIs: []*url.URL{u1, u2}}, + expectedSANs: []string{"urn:uuid:ddfe62ba-7e99-4bc1-83b3-8f57fe3e9959"}, + err: errors.New("URIs claim failed"), + } + }, + "ok": func() test { + u1, err := url.Parse("https://google.com") + assert.FatalError(t, err) + u2, err := url.Parse("urn:uuid:ddfe62ba-7e99-4bc1-83b3-8f57fe3e9959") + assert.FatalError(t, err) + return test{ + csr: &x509.CertificateRequest{ + DNSNames: []string{"foo", "bar"}, + EmailAddresses: []string{"max@fx.com", "mariano@fx.com"}, + IPAddresses: []net.IP{net.ParseIP("1.1.1.1"), net.ParseIP("127.0.0.1")}, + URIs: []*url.URL{u1, u2}, + }, + expectedSANs: []string{"foo", "127.0.0.1", "max@fx.com", "mariano@fx.com", "https://google.com", "1.1.1.1", "bar", "urn:uuid:ddfe62ba-7e99-4bc1-83b3-8f57fe3e9959"}, + } + }, + } + for name, run := range tests { + t.Run(name, func(t *testing.T) { + tt := run() + if err := defaultSANsValidator(tt.expectedSANs).Valid(tt.csr); err != nil { + if assert.NotNil(t, tt.err, fmt.Sprintf("expected no error, but got err = %s", err.Error())) { + assert.True(t, strings.Contains(err.Error(), tt.err.Error()), + fmt.Sprintf("want err = %s, but got err = %s", tt.err.Error(), err.Error())) + } + } else { + assert.Nil(t, tt.err, fmt.Sprintf("expected err = %s, but not ", tt.err)) + } + }) + } +} + +func Test_ExtraExtsEnforcer_Enforce(t *testing.T) { + e1 := pkix.Extension{Id: []int{1, 2, 3, 4, 5}, Critical: false, Value: []byte("foo")} + e2 := pkix.Extension{Id: []int{2, 2, 2}, Critical: false, Value: []byte("bar")} + stepExt := pkix.Extension{Id: stepOIDProvisioner, Critical: false, Value: []byte("baz")} + type test struct { + cert *x509.Certificate + check func(*x509.Certificate) + } + tests := map[string]func() test{ + "ok/empty-exts": func() test { + return test{ + cert: &x509.Certificate{}, + check: func(cert *x509.Certificate) { + assert.Equals(t, len(cert.ExtraExtensions), 0) + }, + } + }, + "ok/no-step-provisioner-ext": func() test { + return test{ + cert: &x509.Certificate{ExtraExtensions: []pkix.Extension{e1, e2}}, + check: func(cert *x509.Certificate) { + assert.Equals(t, len(cert.ExtraExtensions), 0) + }, + } + }, + "ok/step-provisioner-ext": func() test { + return test{ + cert: &x509.Certificate{ExtraExtensions: []pkix.Extension{e1, stepExt, e2}}, + check: func(cert *x509.Certificate) { + assert.Equals(t, len(cert.ExtraExtensions), 1) + assert.Equals(t, cert.ExtraExtensions[0], stepExt) + }, + } + }, + } + for name, run := range tests { + t.Run(name, func(t *testing.T) { + tt := run() + ExtraExtsEnforcer{}.Enforce(tt.cert) + tt.check(tt.cert) + }) + } +} + func Test_validityValidator_Valid(t *testing.T) { type test struct { cert *x509.Certificate diff --git a/authority/tls.go b/authority/tls.go index 551b7e15..ef5aa49f 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -64,7 +64,7 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Opti opts = []interface{}{errs.WithKeyVal("csr", csr), errs.WithKeyVal("signOptions", signOpts)} mods = []x509util.WithOption{withDefaultASN1DN(a.config.AuthorityConfig.Template)} certValidators = []provisioner.CertificateValidator{} - forcedModifiers = []provisioner.CertificateEnforcer{provisioner.ExtraExtensionsEnforcer{}} + forcedModifiers = []provisioner.CertificateEnforcer{provisioner.ExtraExtsEnforcer{}} ) // Set backdate with the configured value