Merge pull request #29 from smallstep/sans

Add SANs support

Gopkg.lock

@@ -18,12 +18,9 @@
 
 [[projects]]
   branch = "master"
-  digest = "1:5f3371d6e432e3c4e0ceabb14b5c0679827ce8b96bbd963fa923977782e8e99b"
+  digest = "1:454adc7f974228ff789428b6dc098638c57a64aa0718f0bd61e53d3cd39d7a75"
   name = "github.com/chzyer/readline"
-  packages = [
+  packages = ["."]
-    ".",
-    "runes",
-  ]
   pruneopts = "UT"
   revision = "2972be24d48e78746da79ba8e24e8b488c9880de"
 
@@ -214,8 +211,8 @@
   revision = "de77670473b5492f5d0bce155b5c01534c2d13f7"
 
 [[projects]]
-  branch = "master"
+  branch = "sans"
-  digest = "1:9e42b7ce7fce33796a870bbbd4e5fdf7c25f3a243ca355d7d9be52f88e3526d5"
+  digest = "1:4c9e30abfe7c119eb4d40287f6c23f854f3ad71c69206d8dc6402e1fef14ac88"
   name = "github.com/smallstep/cli"
   packages = [
     "command",
@@ -234,7 +231,7 @@
     "utils",
   ]
   pruneopts = "UT"
-  revision = "a633fd7f7eff2e8ba4026189241a60968b3767d6"
+  revision = "1379a62e0cf06b164d35e20a912d017ac8bad071"
 
 [[projects]]
   branch = "master"

Gopkg.toml

@@ -45,7 +45,7 @@ required = [
   name = "github.com/go-chi/chi"
 
 [[override]]
-  branch = "master"
+  branch = "sans"
   name = "github.com/smallstep/cli"
 
 [prune]

authority/authorize.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
+	"github.com/smallstep/cli/crypto/x509util"
 	"gopkg.in/square/go-jose.v2/jwt"
 )
 
@@ -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,21 @@ func (a *Authority) Authorize(ott string) ([]interface{}, error) {
 			http.StatusUnauthorized, errContext}
 	}
 
+	// NOTE: This is for backwards compatibility with older versions of cli
+	// and certificates. Older versions added the token subject as the only SAN
+	// in a CSR by default.
+	if len(claims.SANs) == 0 {
+		claims.SANs = []string{claims.Subject}
+	}
+	dnsNames, ips := x509util.SplitSANs(claims.SANs)
+	if err != nil {
+		return nil, err
+	}
+
 	signOps := []interface{}{
 		&commonNameClaim{claims.Subject},
-		&dnsNamesClaim{claims.Subject},
+		&dnsNamesClaim{dnsNames},
-		&ipAddressesClaim{claims.Subject},
+		&ipAddressesClaim{ips},
 		p,
 	}
 

authority/claims.go

@@ -2,6 +2,7 @@ package authority
 
 import (
 	"net"
+	"reflect"
 	"time"
 
 	"github.com/pkg/errors"
@@ -42,43 +43,43 @@ 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 {
+	tokMap := make(map[string]int)
-		return nil
+	for _, e := range c.names {
+		tokMap[e] = 1
 	}
-	for _, name := range crt.DNSNames {
+	crtMap := make(map[string]int)
-		if name != c.name {
+	for _, e := range crt.DNSNames {
-			return errors.Errorf("DNS names claim failed - got %s, want %s", name, c.name)
+		crtMap[e] = 1
 	}
+	if !reflect.DeepEqual(tokMap, crtMap) {
+		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 {
+	tokMap := make(map[string]int)
-		return nil
+	for _, e := range c.ips {
+		tokMap[e.String()] = 1
 	}
-
+	crtMap := make(map[string]int)
-	// If it's an IP validate that only that ip is in IP addresses
+	for _, e := range crt.IPAddresses {
-	if requestedIP := net.ParseIP(c.name); requestedIP != nil {
+		crtMap[e.String()] = 1
-		for _, ip := range crt.IPAddresses {
-			if !ip.Equal(requestedIP) {
-				return errors.Errorf("IP addresses claim failed - got %s, want %s", ip, requestedIP)
 	}
+	if !reflect.DeepEqual(tokMap, crtMap) {
+		return errors.Errorf("IP Addresses claim failed - got %v, want %v", crt.IPAddresses, c.ips)
 	}
 	return nil
-	}
-
-	return errors.Errorf("IP addresses claim failed - got %v, want none", crt.IPAddresses)
 }
 
 // certTemporalClaim validates the certificate temporal validity settings.

authority/claims_test.go

@@ -52,23 +52,28 @@ func TestIPAddressesClaim_Valid(t *testing.T) {
 		crt *x509.Certificate
 		err error
 	}{
-		"unexpected-ip": {
+		"unexpected-ip-in-crt": {
-			iac: &ipAddressesClaim{name: "127.0.0.1"},
+			iac: &ipAddressesClaim{ips: []net.IP{net.ParseIP("127.0.0.1")}},
 			crt: &x509.Certificate{IPAddresses: []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("1.1.1.1")}},
-			err: errors.New("IP addresses claim failed - got 1.1.1.1, want 127.0.0.1"),
+			err: errors.New("IP Addresses claim failed - got [127.0.0.1 1.1.1.1], want [127.0.0.1]"),
+		},
+		"missing-ip-in-crt": {
+			iac: &ipAddressesClaim{ips: []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("1.1.1.1")}},
+			crt: &x509.Certificate{IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}},
+			err: errors.New("IP Addresses claim failed - got [127.0.0.1], want [127.0.0.1 1.1.1.1]"),
 		},
 		"invalid-matcher-nonempty-ips": {
-			iac: &ipAddressesClaim{name: "invalid"},
+			iac: &ipAddressesClaim{ips: []net.IP{}},
 			crt: &x509.Certificate{IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}},
-			err: errors.New("IP addresses claim failed - got [127.0.0.1], want none"),
+			err: errors.New("IP Addresses claim failed - got [127.0.0.1], want []"),
 		},
 		"ok": {
-			iac: &ipAddressesClaim{name: "127.0.0.1"},
+			iac: &ipAddressesClaim{ips: []net.IP{net.ParseIP("127.0.0.1")}},
 			crt: &x509.Certificate{IPAddresses: []net.IP{net.ParseIP("127.0.0.1")}},
 		},
-		"ok-empty-ips": {
+		"ok-multiple-identical-ip-entries": {
-			iac: &ipAddressesClaim{name: "127.0.0.1"},
+			iac: &ipAddressesClaim{ips: []net.IP{net.ParseIP("127.0.0.1")}},
-			crt: &x509.Certificate{IPAddresses: []net.IP{}},
+			crt: &x509.Certificate{IPAddresses: []net.IP{net.ParseIP("127.0.0.1"), net.ParseIP("127.0.0.1"), net.ParseIP("127.0.0.1")}},
 		},
 	}
 
@@ -92,21 +97,22 @@ func TestDNSNamesClaim_Valid(t *testing.T) {
 		crt *x509.Certificate
 		err error
 	}{
-		"wrong-dns-name": {
+		"unexpected-dns-name-in-crt": {
-			dnc: &dnsNamesClaim{name: "foo"},
+			dnc: &dnsNamesClaim{names: []string{"foo"}},
 			crt: &x509.Certificate{DNSNames: []string{"foo", "bar"}},
-			err: errors.New("DNS names claim failed - got bar, want foo"),
+			err: errors.New("DNS names claim failed - got [foo bar], want [foo]"),
 		},
 		"ok": {
-			dnc: &dnsNamesClaim{name: "foo"},
+			dnc: &dnsNamesClaim{names: []string{"foo", "bar"}},
-			crt: &x509.Certificate{DNSNames: []string{"foo"}},
+			crt: &x509.Certificate{DNSNames: []string{"bar", "foo"}},
 		},
-		"ok-empty-dnsNames": {
+		"missing-dns-name-in-crt": {
-			dnc: &dnsNamesClaim{"foo"},
+			dnc: &dnsNamesClaim{names: []string{"foo", "bar"}},
-			crt: &x509.Certificate{},
+			crt: &x509.Certificate{DNSNames: []string{"foo"}},
+			err: errors.New("DNS names claim failed - got [foo], want [foo bar]"),
 		},
 		"ok-multiple-identical-dns-entries": {
-			dnc: &dnsNamesClaim{name: "foo"},
+			dnc: &dnsNamesClaim{names: []string{"foo"}},
 			crt: &x509.Certificate{DNSNames: []string{"foo", "foo", "foo"}},
 		},
 	}

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,

ca/bootstrap_test.go

@@ -94,6 +94,7 @@ func generateBootstrapToken(ca, subject, sha string) string {
 	cl := struct {
 		SHA string `json:"sha"`
 		jwt.Claims
+		SANS []string `json:"sans"`
 	}{
 		SHA: sha,
 		Claims: jwt.Claims{
@@ -104,6 +105,7 @@ func generateBootstrapToken(ca, subject, sha string) string {
 			Expiry:    jwt.NewNumericDate(now.Add(time.Minute)),
 			Audience:  []string{ca + "/sign"},
 		},
+		SANS: []string{subject},
 	}
 	raw, err := jwt.Signed(sig).Claims(cl).CompactSerialize()
 	if err != nil {

ca/ca_test.go

@@ -154,13 +154,19 @@ ZEp7knvU2psWRw==
 		"fail commonname-claim": func(t *testing.T) *signTest {
 			jti, err := randutil.ASCII(32)
 			assert.FatalError(t, err)
-			cl := jwt.Claims{
+			cl := struct {
+				jwt.Claims
+				SANS []string `json:"sans"`
+			}{
+				Claims: jwt.Claims{
 					Subject:   "invalid",
 					Issuer:    "step-cli",
 					NotBefore: jwt.NewNumericDate(now),
 					Expiry:    jwt.NewNumericDate(now.Add(time.Minute)),
 					Audience:  validAud,
 					ID:        jti,
+				},
+				SANS: []string{"invalid"},
 			}
 			raw, err := jwt.Signed(sig).Claims(cl).CompactSerialize()
 			assert.FatalError(t, err)
@@ -181,13 +187,52 @@ ZEp7knvU2psWRw==
 		"ok": func(t *testing.T) *signTest {
 			jti, err := randutil.ASCII(32)
 			assert.FatalError(t, err)
-			cl := jwt.Claims{
+			cl := struct {
+				jwt.Claims
+				SANS []string `json:"sans"`
+			}{
+				Claims: jwt.Claims{
 					Subject:   "test.smallstep.com",
 					Issuer:    "step-cli",
 					NotBefore: jwt.NewNumericDate(now),
 					Expiry:    jwt.NewNumericDate(now.Add(time.Minute)),
 					Audience:  validAud,
 					ID:        jti,
+				},
+				SANS: []string{"test.smallstep.com"},
+			}
+			raw, err := jwt.Signed(sig).Claims(cl).CompactSerialize()
+			assert.FatalError(t, err)
+			csr, err := getCSR(priv)
+			assert.FatalError(t, err)
+			body, err := json.Marshal(&api.SignRequest{
+				CsrPEM:    api.CertificateRequest{CertificateRequest: csr},
+				OTT:       raw,
+				NotBefore: now,
+				NotAfter:  leafExpiry,
+			})
+			assert.FatalError(t, err)
+			return &signTest{
+				ca:     ca,
+				body:   string(body),
+				status: http.StatusCreated,
+			}
+		},
+		"ok-backwards-compat-missing-subject-SAN": func(t *testing.T) *signTest {
+			jti, err := randutil.ASCII(32)
+			assert.FatalError(t, err)
+			cl := struct {
+				jwt.Claims
+				SANS []string `json:"sans"`
+			}{
+				Claims: jwt.Claims{
+					Subject:   "test.smallstep.com",
+					Issuer:    "step-cli",
+					NotBefore: jwt.NewNumericDate(now),
+					Expiry:    jwt.NewNumericDate(now.Add(time.Minute)),
+					Audience:  validAud,
+					ID:        jti,
+				},
 			}
 			raw, err := jwt.Signed(sig).Claims(cl).CompactSerialize()
 			assert.FatalError(t, err)

ca/client.go

@@ -15,7 +15,6 @@ import (
 	"encoding/pem"
 	"io"
 	"io/ioutil"
-	"net"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -23,6 +22,8 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/smallstep/certificates/api"
+	"github.com/smallstep/certificates/authority"
+	"github.com/smallstep/cli/crypto/x509util"
 	"gopkg.in/square/go-jose.v2/jwt"
 )
 
@@ -442,7 +443,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 +453,15 @@ func CreateSignRequest(ott string) (*api.SignRequest, crypto.PrivateKey, error)
 		return nil, nil, errors.Wrap(err, "error generating key")
 	}
 
+	dnsNames, ips := x509util.SplitSANs(claims.SANs)
+
 	template := &x509.CertificateRequest{
 		Subject: pkix.Name{
 			CommonName: claims.Subject,
 		},
 		SignatureAlgorithm: x509.ECDSAWithSHA256,
-	}
+		DNSNames:           dnsNames,
-
+		IPAddresses:        ips,
-	if ip := net.ParseIP(claims.Subject); ip != nil {
-		template.IPAddresses = append(template.IPAddresses, ip)
-	} else {
-		template.DNSNames = append(template.DNSNames, claims.Subject)
 	}
 
 	csr, err := x509.CreateCertificateRequest(rand.Reader, template, pk)

ca/tls_test.go

@@ -39,13 +39,19 @@ func generateOTT(subject string) string {
 	if err != nil {
 		panic(err)
 	}
-	cl := jwt.Claims{
+	cl := struct {
+		jwt.Claims
+		SANS []string `json:"sans"`
+	}{
+		Claims: jwt.Claims{
 			ID:        id,
 			Subject:   subject,
 			Issuer:    "mariano",
 			NotBefore: jwt.NewNumericDate(now),
 			Expiry:    jwt.NewNumericDate(now.Add(time.Minute)),
 			Audience:  []string{"https://127.0.0.1:0/sign"},
+		},
+		SANS: []string{subject},
 	}
 	raw, err := jwt.Signed(sig).Claims(cl).CompactSerialize()
 	if err != nil {