Merge pull request #436 from smallstep/upgrade-crypto

Upgrade crypto and validate token id
2020-12-17 15:12:32 -08:00 · 2020-12-17 15:12:32 -08:00 · bae209741f
commit bae209741f
parent 1feb4fcb26 5017b7d21f
7 changed files with 55 additions and 46 deletions
--- a/authority/provisioner/aws.go
+++ b/authority/provisioner/aws.go
@ -284,7 +284,11 @@ func (p *AWS) GetTokenID(token string) (string, error) {
 		sum := sha256.Sum256([]byte(token))
 		return strings.ToLower(hex.EncodeToString(sum[:])), nil
 	}
-	return payload.ID, nil
+
+	// Use provisioner + instance-id as the identifier.
+	unique := fmt.Sprintf("%s.%s", p.GetID(), payload.document.InstanceID)
+	sum := sha256.Sum256([]byte(unique))
+	return strings.ToLower(hex.EncodeToString(sum[:])), nil
 }

 // GetName returns the name of the provisioner.
--- a/authority/provisioner/aws_test.go
+++ b/authority/provisioner/aws_test.go
@ -332,7 +332,7 @@ func TestAWS_authorizeToken(t *testing.T) {
 			p, err := generateAWS()
 			assert.FatalError(t, err)
 			tok, err := generateAWSToken(
-				"instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
+				p, "instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
 				"127.0.0.1", "us-west-1", time.Now(), badKey)
 			assert.FatalError(t, err)
 			return test{
@ -346,7 +346,7 @@ func TestAWS_authorizeToken(t *testing.T) {
 			p, err := generateAWS()
 			assert.FatalError(t, err)
 			tok, err := generateAWSToken(
-				"instance-id", awsIssuer, p.GetID(), "", "instance-id",
+				p, "instance-id", awsIssuer, p.GetID(), "", "instance-id",
 				"127.0.0.1", "us-west-1", time.Now(), key)
 			assert.FatalError(t, err)
 			return test{
@ -360,7 +360,7 @@ func TestAWS_authorizeToken(t *testing.T) {
 			p, err := generateAWS()
 			assert.FatalError(t, err)
 			tok, err := generateAWSToken(
-				"instance-id", awsIssuer, p.GetID(), p.Accounts[0], "",
+				p, "instance-id", awsIssuer, p.GetID(), p.Accounts[0], "",
 				"127.0.0.1", "us-west-1", time.Now(), key)
 			assert.FatalError(t, err)
 			return test{
@ -374,7 +374,7 @@ func TestAWS_authorizeToken(t *testing.T) {
 			p, err := generateAWS()
 			assert.FatalError(t, err)
 			tok, err := generateAWSToken(
-				"instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
+				p, "instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
 				"", "us-west-1", time.Now(), key)
 			assert.FatalError(t, err)
 			return test{
@ -388,7 +388,7 @@ func TestAWS_authorizeToken(t *testing.T) {
 			p, err := generateAWS()
 			assert.FatalError(t, err)
 			tok, err := generateAWSToken(
-				"instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
+				p, "instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
 				"127.0.0.1", "", time.Now(), key)
 			assert.FatalError(t, err)
 			return test{
@ -402,7 +402,7 @@ func TestAWS_authorizeToken(t *testing.T) {
 			p, err := generateAWS()
 			assert.FatalError(t, err)
 			tok, err := generateAWSToken(
-				"instance-id", "bad-issuer", p.GetID(), p.Accounts[0], "instance-id",
+				p, "instance-id", "bad-issuer", p.GetID(), p.Accounts[0], "instance-id",
 				"127.0.0.1", "us-west-1", time.Now(), key)
 			assert.FatalError(t, err)
 			return test{
@ -416,7 +416,7 @@ func TestAWS_authorizeToken(t *testing.T) {
 			p, err := generateAWS()
 			assert.FatalError(t, err)
 			tok, err := generateAWSToken(
-				"instance-id", awsIssuer, "bad-audience", p.Accounts[0], "instance-id",
+				p, "instance-id", awsIssuer, "bad-audience", p.Accounts[0], "instance-id",
 				"127.0.0.1", "us-west-1", time.Now(), key)
 			assert.FatalError(t, err)
 			return test{
@ -431,7 +431,7 @@ func TestAWS_authorizeToken(t *testing.T) {
 			assert.FatalError(t, err)
 			p.DisableCustomSANs = true
 			tok, err := generateAWSToken(
-				"foo", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
+				p, "foo", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
 				"127.0.0.1", "us-west-1", time.Now(), key)
 			assert.FatalError(t, err)
 			return test{
@ -445,7 +445,7 @@ func TestAWS_authorizeToken(t *testing.T) {
 			p, err := generateAWS()
 			assert.FatalError(t, err)
 			tok, err := generateAWSToken(
-				"instance-id", awsIssuer, p.GetID(), "foo", "instance-id",
+				p, "instance-id", awsIssuer, p.GetID(), "foo", "instance-id",
 				"127.0.0.1", "us-west-1", time.Now(), key)
 			assert.FatalError(t, err)
 			return test{
@ -460,7 +460,7 @@ func TestAWS_authorizeToken(t *testing.T) {
 			assert.FatalError(t, err)
 			p.InstanceAge = Duration{1 * time.Minute}
 			tok, err := generateAWSToken(
-				"instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
+				p, "instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
 				"127.0.0.1", "us-west-1", time.Now().Add(-1*time.Minute), key)
 			assert.FatalError(t, err)
 			return test{
@ -470,24 +470,11 @@ func TestAWS_authorizeToken(t *testing.T) {
 				err:   errors.New("aws.authorizeToken; aws identity document pendingTime is too old"),
 			}
 		},
-		"fail/identityCert": func(t *testing.T) test {
-			p, err := generateAWS()
-			p.IIDRoots = "testdata/certs/aws.crt"
-			assert.FatalError(t, err)
-			tok, err := generateAWSToken(
-				"instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
-				"127.0.0.1", "us-west-1", time.Now(), key)
-			assert.FatalError(t, err)
-			return test{
-				p:     p,
-				token: tok,
-			}
-		},
 		"ok": func(t *testing.T) test {
 			p, err := generateAWS()
 			assert.FatalError(t, err)
 			tok, err := generateAWSToken(
-				"instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
+				p, "instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
 				"127.0.0.1", "us-west-1", time.Now(), key)
 			assert.FatalError(t, err)
 			return test{
@ -500,7 +487,20 @@ func TestAWS_authorizeToken(t *testing.T) {
 			p.IIDRoots = "testdata/certs/aws-test.crt"
 			assert.FatalError(t, err)
 			tok, err := generateAWSToken(
-				"instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
+				p, "instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
+				"127.0.0.1", "us-west-1", time.Now(), key)
+			assert.FatalError(t, err)
+			return test{
+				p:     p,
+				token: tok,
+			}
+		},
+		"ok/identityCert2": func(t *testing.T) test {
+			p, err := generateAWS()
+			p.IIDRoots = "testdata/certs/aws.crt"
+			assert.FatalError(t, err)
+			tok, err := generateAWSToken(
+				p, "instance-id", awsIssuer, p.GetID(), p.Accounts[0], "instance-id",
 				"127.0.0.1", "us-west-1", time.Now(), key)
 			assert.FatalError(t, err)
 			return test{
@ -575,51 +575,51 @@ func TestAWS_AuthorizeSign(t *testing.T) {
 	assert.FatalError(t, err)

 	t4, err := generateAWSToken(
-		"instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "instance-id",
+		p1, "instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "instance-id",
 		"127.0.0.1", "us-west-1", time.Now(), key)
 	assert.FatalError(t, err)
 	failSubject, err := generateAWSToken(
-		"bad-subject", awsIssuer, p2.GetID(), p2.Accounts[0], "instance-id",
+		p2, "bad-subject", awsIssuer, p2.GetID(), p2.Accounts[0], "instance-id",
 		"127.0.0.1", "us-west-1", time.Now(), key)
 	assert.FatalError(t, err)
 	failIssuer, err := generateAWSToken(
-		"instance-id", "bad-issuer", p1.GetID(), p1.Accounts[0], "instance-id",
+		p1, "instance-id", "bad-issuer", p1.GetID(), p1.Accounts[0], "instance-id",
 		"127.0.0.1", "us-west-1", time.Now(), key)
 	assert.FatalError(t, err)
 	failAudience, err := generateAWSToken(
-		"instance-id", awsIssuer, "bad-audience", p1.Accounts[0], "instance-id",
+		p1, "instance-id", awsIssuer, "bad-audience", p1.Accounts[0], "instance-id",
 		"127.0.0.1", "us-west-1", time.Now(), key)
 	assert.FatalError(t, err)
 	failAccount, err := generateAWSToken(
-		"instance-id", awsIssuer, p1.GetID(), "", "instance-id",
+		p1, "instance-id", awsIssuer, p1.GetID(), "", "instance-id",
 		"127.0.0.1", "us-west-1", time.Now(), key)
 	assert.FatalError(t, err)
 	failInstanceID, err := generateAWSToken(
-		"instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "",
+		p1, "instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "",
 		"127.0.0.1", "us-west-1", time.Now(), key)
 	assert.FatalError(t, err)
 	failPrivateIP, err := generateAWSToken(
-		"instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "instance-id",
+		p1, "instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "instance-id",
 		"", "us-west-1", time.Now(), key)
 	assert.FatalError(t, err)
 	failRegion, err := generateAWSToken(
-		"instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "instance-id",
+		p1, "instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "instance-id",
 		"127.0.0.1", "", time.Now(), key)
 	assert.FatalError(t, err)
 	failExp, err := generateAWSToken(
-		"instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "instance-id",
+		p1, "instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "instance-id",
 		"127.0.0.1", "us-west-1", time.Now().Add(-360*time.Second), key)
 	assert.FatalError(t, err)
 	failNbf, err := generateAWSToken(
-		"instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "instance-id",
+		p1, "instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "instance-id",
 		"127.0.0.1", "us-west-1", time.Now().Add(360*time.Second), key)
 	assert.FatalError(t, err)
 	failKey, err := generateAWSToken(
-		"instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "instance-id",
+		p1, "instance-id", awsIssuer, p1.GetID(), p1.Accounts[0], "instance-id",
 		"127.0.0.1", "us-west-1", time.Now(), badKey)
 	assert.FatalError(t, err)
 	failInstanceAge, err := generateAWSToken(
-		"instance-id", awsIssuer, p2.GetID(), p2.Accounts[0], "instance-id",
+		p2, "instance-id", awsIssuer, p2.GetID(), p2.Accounts[0], "instance-id",
 		"127.0.0.1", "us-west-1", time.Now().Add(-1*time.Minute), key)
 	assert.FatalError(t, err)

--- a/authority/provisioner/options_test.go
+++ b/authority/provisioner/options_test.go
@ -127,7 +127,7 @@ func TestTemplateOptions(t *testing.T) {
 }`)}, false},
 		{"okCustomTemplate", args{&Options{X509: &X509Options{Template: x509util.DefaultIIDLeafTemplate}}, data}, x509util.Options{
 			CertBuffer: bytes.NewBufferString(`{
-	"subject": {"commonName":"foo"},
+	"subject": {"commonName": "foo"},
 	"sans": [{"type":"dns","value":"foo.com"}],
 	"keyUsage": ["digitalSignature"],
 	"extKeyUsage": ["serverAuth", "clientAuth"]
@ -189,7 +189,7 @@ func TestCustomTemplateOptions(t *testing.T) {
 }`)}, false},
 		{"okIID", args{nil, data, x509util.DefaultIIDLeafTemplate, SignOptions{}}, x509util.Options{
 			CertBuffer: bytes.NewBufferString(`{
-	"subject": {"commonName":"foo"},
+	"subject": {"commonName": "foo"},
 	"sans": [{"type":"dns","value":"foo.com"}],
 	"keyUsage": ["digitalSignature"],
 	"extKeyUsage": ["serverAuth", "clientAuth"]
--- a/authority/provisioner/ssh_options_test.go
+++ b/authority/provisioner/ssh_options_test.go
@ -39,8 +39,8 @@ func TestCustomSSHTemplateOptions(t *testing.T) {
 		}, false},
 		{"okNoData", args{nil, nil, sshutil.DefaultTemplate, SignSSHOptions{}}, sshutil.Options{
 			CertBuffer: bytes.NewBufferString(`{
-	"type": "<no value>",
-	"keyId": "<no value>",
+	"type": null,
+	"keyId": null,
 	"principals": null,
 	"extensions": null,
 	"criticalOptions": null
--- a/authority/provisioner/utils_test.go
+++ b/authority/provisioner/utils_test.go
@ -13,6 +13,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"time"

 	"github.com/pkg/errors"
@ -910,7 +911,7 @@ func generateGCPToken(sub, iss, aud, instanceID, instanceName, projectID, zone s
 	return jose.Signed(sig).Claims(claims).CompactSerialize()
 }

-func generateAWSToken(sub, iss, aud, accountID, instanceID, privateIP, region string, iat time.Time, key crypto.Signer) (string, error) {
+func generateAWSToken(p *AWS, sub, iss, aud, accountID, instanceID, privateIP, region string, iat time.Time, key crypto.Signer) (string, error) {
 	doc, err := json.MarshalIndent(awsInstanceIdentityDocument{
 		AccountID:        accountID,
 		Architecture:     "x86_64",
@ -946,8 +947,12 @@ func generateAWSToken(sub, iss, aud, accountID, instanceID, privateIP, region st
 		return "", err
 	}

+	unique := fmt.Sprintf("%s.%s", p.GetID(), instanceID)
+	sum = sha256.Sum256([]byte(unique))
+
 	claims := awsPayload{
 		Claims: jose.Claims{
+			ID:        strings.ToLower(hex.EncodeToString(sum[:])),
 			Subject:   sub,
 			Issuer:    iss,
 			IssuedAt:  jose.NewNumericDate(iat),
--- a/go.mod
+++ b/go.mod
@ -20,7 +20,7 @@ require (
 	github.com/smallstep/nosql v0.3.0
 	github.com/urfave/cli v1.22.4
 	go.step.sm/cli-utils v0.1.0
-	go.step.sm/crypto v0.7.1
+	go.step.sm/crypto v0.7.3
 	golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897
 	golang.org/x/net v0.0.0-20201021035429-f5854403a974
 	google.golang.org/api v0.33.0
--- a/go.sum
+++ b/go.sum
@ -293,8 +293,8 @@ go.step.sm/cli-utils v0.1.0 h1:uuQ73MuAh5P5Eg+3zfqwrtlTLx5DWSfGqGCrSSjYqdk=
 go.step.sm/cli-utils v0.1.0/go.mod h1:+t4qCp5NO+080DdGkJxEh3xL5S4TcYC2JTPLMM72b6Y=
 go.step.sm/crypto v0.6.1 h1:nJoRFGrGNf/mKVVMdWnfLbBfIFt/z4NdJlSL5nipQMQ=
 go.step.sm/crypto v0.6.1/go.mod h1:AKS4yMZVZD4EGjpSkY4eibuMenrvKCscb+BpWMet8c0=
-go.step.sm/crypto v0.7.1 h1:AX9rCGDvSHxfYy117+i7TqNkjhRxuybI584woKqP54c=
-go.step.sm/crypto v0.7.1/go.mod h1:AKS4yMZVZD4EGjpSkY4eibuMenrvKCscb+BpWMet8c0=
+go.step.sm/crypto v0.7.3 h1:uWkT0vsaZVixgn5x6Ojqittry9PiyVn2ihEYG/qOxV8=
+go.step.sm/crypto v0.7.3/go.mod h1:AKS4yMZVZD4EGjpSkY4eibuMenrvKCscb+BpWMet8c0=
 golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=