diff --git a/Gopkg.lock b/Gopkg.lock index 451faf0f..b8b3dcb3 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -18,12 +18,9 @@ [[projects]] branch = "master" - digest = "1:5f3371d6e432e3c4e0ceabb14b5c0679827ce8b96bbd963fa923977782e8e99b" + digest = "1:454adc7f974228ff789428b6dc098638c57a64aa0718f0bd61e53d3cd39d7a75" name = "github.com/chzyer/readline" - packages = [ - ".", - "runes", - ] + packages = ["."] pruneopts = "UT" revision = "2972be24d48e78746da79ba8e24e8b488c9880de" @@ -214,8 +211,8 @@ revision = "de77670473b5492f5d0bce155b5c01534c2d13f7" [[projects]] - branch = "master" - digest = "1:9e42b7ce7fce33796a870bbbd4e5fdf7c25f3a243ca355d7d9be52f88e3526d5" + branch = "sans" + digest = "1:84a773da390eabc9a292221bbc2c16653093f8eb805a1b16f738ef3cd88df701" name = "github.com/smallstep/cli" packages = [ "command", @@ -234,7 +231,7 @@ "utils", ] pruneopts = "UT" - revision = "a633fd7f7eff2e8ba4026189241a60968b3767d6" + revision = "a1df67d396f730ff0e7bf1916cd27ae8d5255236" [[projects]] branch = "master" diff --git a/authority/authorize.go b/authority/authorize.go index 1c6fc65d..cfd52bb7 100644 --- a/authority/authorize.go +++ b/authority/authorize.go @@ -3,6 +3,7 @@ package authority import ( "crypto/x509" "encoding/asn1" + "net" "net/http" "net/url" "time" @@ -16,6 +17,12 @@ type idUsed struct { Subject string `json:"sub,omitempty"` } +// Claims extends jwt.Claims with step attributes. +type Claims struct { + jwt.Claims + SANS []string `json:"sans,omitempty"` +} + // matchesAudience returns true if A and B share at least one element. func matchesAudience(as, bs []string) bool { if len(bs) == 0 || len(as) == 0 { @@ -48,7 +55,7 @@ func stripPort(rawurl string) string { func (a *Authority) Authorize(ott string) ([]interface{}, error) { var ( errContext = map[string]interface{}{"ott": ott} - claims = jwt.Claims{} + claims = Claims{} ) // Validate payload @@ -113,10 +120,15 @@ func (a *Authority) Authorize(ott string) ([]interface{}, error) { http.StatusUnauthorized, errContext} } + dnsNames, ips := SplitSANS(claims.SANS) + if err != nil { + return nil, err + } + signOps := []interface{}{ &commonNameClaim{claims.Subject}, - &dnsNamesClaim{claims.Subject}, - &ipAddressesClaim{claims.Subject}, + &dnsNamesClaim{dnsNames}, + &ipAddressesClaim{ips}, p, } @@ -132,6 +144,26 @@ func (a *Authority) Authorize(ott string) ([]interface{}, error) { return signOps, nil } +// SplitSANS splits a slice of Subject Alternative Names into slices of +// IP Addresses and DNS Names. If an element is not an IP address, then it +// is bucketed as a DNS Name. +func SplitSANS(sans []string) (dnsNames []string, ips []net.IP) { + dnsNames = []string{} + ips = []net.IP{} + if sans == nil { + return + } + for _, san := range sans { + if ip := net.ParseIP(san); ip != nil { + ips = append(ips, ip) + } else { + // If not IP then assume DNSName. + dnsNames = append(dnsNames, san) + } + } + return +} + // authorizeRenewal tries to locate the step provisioner extension, and checks // if for the configured provisioner, the renewal is enabled or not. If the // extra extension cannot be found, authorize the renewal by default. diff --git a/authority/claims.go b/authority/claims.go index 4dbea684..2af4d358 100644 --- a/authority/claims.go +++ b/authority/claims.go @@ -1,7 +1,10 @@ package authority import ( + "bytes" + "fmt" "net" + "sort" "time" "github.com/pkg/errors" @@ -42,43 +45,47 @@ func (c *commonNameClaim) Valid(crt *x509.Certificate) error { } type dnsNamesClaim struct { - name string + names []string } // Valid checks that certificate request common name matches the one configured. func (c *dnsNamesClaim) Valid(crt *x509.Certificate) error { - if len(crt.DNSNames) == 0 { - return nil + sort.Strings(c.names) + sort.Strings(crt.DNSNames) + if len(c.names) != len(crt.DNSNames) { + fmt.Printf("len(c.names) = %+v, len(crt.DNSNames) = %+v\n", len(c.names), len(crt.DNSNames)) + return errors.Errorf("DNS names claim failed - got %s, want %s", crt.DNSNames, c.names) } - for _, name := range crt.DNSNames { - if name != c.name { - return errors.Errorf("DNS names claim failed - got %s, want %s", name, c.name) + for i := range c.names { + if c.names[i] != crt.DNSNames[i] { + fmt.Printf("c.names[i] = %+v, crt.DNSNames[i] = %+v\n", c.names[i], crt.DNSNames[i]) + return errors.Errorf("DNS names claim failed - got %s, want %s", crt.DNSNames, c.names) } } return nil } type ipAddressesClaim struct { - name string + ips []net.IP } // Valid checks that certificate request common name matches the one configured. func (c *ipAddressesClaim) Valid(crt *x509.Certificate) error { - if len(crt.IPAddresses) == 0 { - return nil + if len(c.ips) != len(crt.IPAddresses) { + return errors.Errorf("IP Addresses claim failed - got %v, want %v", crt.IPAddresses, c.ips) } - - // If it's an IP validate that only that ip is in IP addresses - if requestedIP := net.ParseIP(c.name); requestedIP != nil { - for _, ip := range crt.IPAddresses { - if !ip.Equal(requestedIP) { - return errors.Errorf("IP addresses claim failed - got %s, want %s", ip, requestedIP) - } + sort.Slice(c.ips, func(i, j int) bool { + return bytes.Compare(c.ips[i], c.ips[j]) < 0 + }) + sort.Slice(crt.IPAddresses, func(i, j int) bool { + return bytes.Compare(crt.IPAddresses[i], crt.IPAddresses[j]) < 0 + }) + for i := range c.ips { + if !c.ips[i].Equal(crt.IPAddresses[i]) { + return errors.Errorf("IP Addresses claim failed - got %v, want %v", crt.IPAddresses[i], c.ips[i]) } - return nil } - - return errors.Errorf("IP addresses claim failed - got %v, want none", crt.IPAddresses) + return nil } // certTemporalClaim validates the certificate temporal validity settings. diff --git a/authority/tls.go b/authority/tls.go index 750ced60..6ca433ff 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -129,10 +129,6 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts SignOptions, ext return nil, nil, &apiError{errors.Wrap(err, "sign: error converting x509 csr to stepx509 csr"), http.StatusInternalServerError, errContext} } - // DNSNames and IPAddresses are validated but to avoid duplications we will - // clean them as x509util.NewLeafProfileWithCSR will set the right values. - stepCSR.DNSNames = nil - stepCSR.IPAddresses = nil issIdentity := a.intermediateIdentity leaf, err := x509util.NewLeafProfileWithCSR(stepCSR, issIdentity.Crt, @@ -201,15 +197,15 @@ func (a *Authority) Renew(ocx *x509.Certificate) (*x509.Certificate, *x509.Certi ExtKeyUsage: oldCert.ExtKeyUsage, UnknownExtKeyUsage: oldCert.UnknownExtKeyUsage, BasicConstraintsValid: oldCert.BasicConstraintsValid, - IsCA: oldCert.IsCA, - MaxPathLen: oldCert.MaxPathLen, - MaxPathLenZero: oldCert.MaxPathLenZero, - OCSPServer: oldCert.OCSPServer, - IssuingCertificateURL: oldCert.IssuingCertificateURL, - DNSNames: oldCert.DNSNames, - EmailAddresses: oldCert.EmailAddresses, - IPAddresses: oldCert.IPAddresses, - URIs: oldCert.URIs, + IsCA: oldCert.IsCA, + MaxPathLen: oldCert.MaxPathLen, + MaxPathLenZero: oldCert.MaxPathLenZero, + OCSPServer: oldCert.OCSPServer, + IssuingCertificateURL: oldCert.IssuingCertificateURL, + DNSNames: oldCert.DNSNames, + EmailAddresses: oldCert.EmailAddresses, + IPAddresses: oldCert.IPAddresses, + URIs: oldCert.URIs, PermittedDNSDomainsCritical: oldCert.PermittedDNSDomainsCritical, PermittedDNSDomains: oldCert.PermittedDNSDomains, ExcludedDNSDomains: oldCert.ExcludedDNSDomains, diff --git a/ca/client.go b/ca/client.go index e138698f..f6eea62d 100644 --- a/ca/client.go +++ b/ca/client.go @@ -15,7 +15,6 @@ import ( "encoding/pem" "io" "io/ioutil" - "net" "net/http" "net/url" "strconv" @@ -23,6 +22,7 @@ import ( "github.com/pkg/errors" "github.com/smallstep/certificates/api" + "github.com/smallstep/certificates/authority" "gopkg.in/square/go-jose.v2/jwt" ) @@ -442,7 +442,7 @@ func CreateSignRequest(ott string) (*api.SignRequest, crypto.PrivateKey, error) if err != nil { return nil, nil, errors.Wrap(err, "error parsing ott") } - var claims jwt.Claims + var claims authority.Claims if err := token.UnsafeClaimsWithoutVerification(&claims); err != nil { return nil, nil, errors.Wrap(err, "error parsing ott") } @@ -452,17 +452,15 @@ func CreateSignRequest(ott string) (*api.SignRequest, crypto.PrivateKey, error) return nil, nil, errors.Wrap(err, "error generating key") } + dnsNames, ips := authority.SplitSANS(claims.SANS) + template := &x509.CertificateRequest{ Subject: pkix.Name{ CommonName: claims.Subject, }, SignatureAlgorithm: x509.ECDSAWithSHA256, - } - - if ip := net.ParseIP(claims.Subject); ip != nil { - template.IPAddresses = append(template.IPAddresses, ip) - } else { - template.DNSNames = append(template.DNSNames, claims.Subject) + DNSNames: dnsNames, + IPAddresses: ips, } csr, err := x509.CreateCertificateRequest(rand.Reader, template, pk)