From a7fe0104c44837f666a2cd6909fc44a81e3ffb4a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 15 Jul 2020 17:30:29 -0700 Subject: [PATCH] Remove ACME restrictions and add proper template support. --- acme/common.go | 1 + acme/order.go | 32 ++++++++++++++++++++++---------- authority/provisioner/acme.go | 14 +++++--------- x509util/extensions.go | 19 ++++++++++++++----- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/acme/common.go b/acme/common.go index d8b2b7e4..073e14b1 100644 --- a/acme/common.go +++ b/acme/common.go @@ -18,6 +18,7 @@ type Provisioner interface { AuthorizeSign(ctx context.Context, token string) ([]provisioner.SignOption, error) GetName() string DefaultTLSCertDuration() time.Duration + GetOptions() *provisioner.ProvisionerOptions } // MockProvisioner for testing diff --git a/acme/order.go b/acme/order.go index 8642c6d1..cf9002dc 100644 --- a/acme/order.go +++ b/acme/order.go @@ -10,6 +10,7 @@ import ( "github.com/pkg/errors" "github.com/smallstep/certificates/authority/provisioner" + "github.com/smallstep/certificates/x509util" "github.com/smallstep/nosql" ) @@ -299,23 +300,23 @@ func (o *order) finalize(db nosql.DB, csr *x509.CertificateRequest, auth SignAut orderNames = uniqueLowerNames(orderNames) // Validate identifier names against CSR alternative names. + // + // Note that with certificate templates we are not going to check for the no + // presence of other SANs as they will only be set if the templates allows + // them. 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)) } + + sans := make([]x509util.SubjectAlternativeName, len(csr.DNSNames)) 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)) } - } - - if len(csr.IPAddresses) > 0 { - return nil, BadCSRErr(errors.Errorf("CSR contains IP Address SANs, but should only contain DNS Names")) - } - if len(csr.EmailAddresses) > 0 { - return nil, BadCSRErr(errors.Errorf("CSR contains Email Address SANs, but should only contain DNS Names")) - } - if len(csr.URIs) > 0 { - return nil, BadCSRErr(errors.Errorf("CSR contains URI SANs, but should only contain DNS Names")) + sans[i] = x509util.SubjectAlternativeName{ + Type: x509util.DNSType, + Value: csr.DNSNames[i], + } } // Get authorizations from the ACME provisioner. @@ -325,6 +326,17 @@ func (o *order) finalize(db nosql.DB, csr *x509.CertificateRequest, auth SignAut return nil, ServerInternalErr(errors.Wrapf(err, "error retrieving authorization options from ACME provisioner")) } + // Template data + data := x509util.NewTemplateData() + data.SetCommonName(csr.Subject.CommonName) + data.Set(x509util.SANsKey, sans) + + templateOptions, err := provisioner.TemplateOptions(p.GetOptions(), data) + if err != nil { + return nil, ServerInternalErr(errors.Wrapf(err, "error creating template options from ACME provisioner")) + } + signOps = append(signOps, templateOptions) + // Create and store a new certificate. certChain, err := auth.Sign(csr, provisioner.Options{ NotBefore: provisioner.NewTimeDuration(o.NotBefore), diff --git a/authority/provisioner/acme.go b/authority/provisioner/acme.go index f96fce2f..418f733c 100644 --- a/authority/provisioner/acme.go +++ b/authority/provisioner/acme.go @@ -3,12 +3,10 @@ package provisioner import ( "context" "crypto/x509" - "net/http" "time" "github.com/pkg/errors" "github.com/smallstep/certificates/errs" - "github.com/smallstep/certificates/x509util" ) // ACME is the acme provisioner type, an entity that can authorize the ACME @@ -48,6 +46,11 @@ func (p *ACME) GetEncryptedKey() (string, string, bool) { return "", "", false } +// GetOptions returns the configured provisioner options. +func (p *ACME) GetOptions() *ProvisionerOptions { + return p.Options +} + // DefaultTLSCertDuration returns the default TLS cert duration enforced by // the provisioner. func (p *ACME) DefaultTLSCertDuration() time.Duration { @@ -75,14 +78,7 @@ func (p *ACME) Init(config Config) (err error) { // in the ACME protocol. This method returns a list of modifiers / constraints // on the resulting certificate. func (p *ACME) AuthorizeSign(ctx context.Context, token string) ([]SignOption, error) { - // Certificate templates - templateOptions, err := TemplateOptions(p.Options, x509util.NewTemplateData()) - if err != nil { - return nil, errs.Wrap(http.StatusInternalServerError, err, "acme.AuthorizeSign") - } - return []SignOption{ - templateOptions, // modifiers / withOptions newProvisionerExtensionOption(TypeACME, p.Name, ""), newForceCNOption(p.ForceCN), diff --git a/x509util/extensions.go b/x509util/extensions.go index e627d11c..4f3cb13b 100644 --- a/x509util/extensions.go +++ b/x509util/extensions.go @@ -48,6 +48,15 @@ var ( ExtKeyUsageMicrosoftKernelCodeSigning = convertName("MicrosoftKernelCodeSigning") ) +// Names used and SubjectAlternativeNames types. +const ( + AutoType = "auto" + DNSType = "dns" + EmailType = "email" + IPType = "ip" + URIType = "uri" +) + // Extension is the JSON representation of a raw X.509 extensions. type Extension struct { ID ObjectIdentifier `json:"id"` @@ -106,21 +115,21 @@ type SubjectAlternativeName struct { func (s SubjectAlternativeName) Set(c *x509.Certificate) { switch strings.ToLower(s.Type) { - case "dns": + case DNSType: c.DNSNames = append(c.DNSNames, s.Value) - case "email": + case EmailType: c.EmailAddresses = append(c.EmailAddresses, s.Value) - case "ip": + case IPType: // The validation of the IP would happen in the unmarshaling, but just // to be sure we are only adding valid IPs. if ip := net.ParseIP(s.Value); ip != nil { c.IPAddresses = append(c.IPAddresses, ip) } - case "uri": + case URIType: if u, err := url.Parse(s.Value); err == nil { c.URIs = append(c.URIs, u) } - case "auto", "": + case "", AutoType: dnsNames, ips, emails, uris := SplitSANs([]string{s.Value}) c.DNSNames = append(c.DNSNames, dnsNames...) c.IPAddresses = append(c.IPAddresses, ips...)