Repository: smallstep/cli
Pull request #323 (2020-07-15 19:44:40)

Generate SubjectKeyId according to spec
### Description

This PR changes the way the subject key id is generated to comply with the recommendation in the RFC 5280:

In https://tools.ietf.org/html/rfc5280#section-4.2.1.2 specifies
```
(1) The keyIdentifier is composed of the 160-bit SHA-1 hash of the
    value of the BIT STRING subjectPublicKey (excluding the tag,
    length, and number of unused bits).
```

A public key is defined as:
```
SubjectPublicKeyInfo  ::=  SEQUENCE  {
        algorithm            AlgorithmIdentifier,
        subjectPublicKey     BIT STRING  
}
```

The PR also includes the upgrade of https://github.com/smallstep/truststore that includes a fix for Java.

Related to smallstep/certificates#321


Patch:
diff --git a/crypto/x509util/profile.go b/crypto/x509util/profile.go
index a21dab1a..76e7e105 100644
--- a/crypto/x509util/profile.go
+++ b/crypto/x509util/profile.go
@@ -332,12 +332,11 @@ func newProfile(p Profile, sub, iss *x509.Certificate, issPriv crypto.PrivateKey
 	}
 
 	if sub.SubjectKeyId == nil {
-		pubBytes, err := x509.MarshalPKIXPublicKey(p.SubjectPublicKey())
+		id, err := generateSubjectKeyID(p.SubjectPublicKey())
 		if err != nil {
-			return nil, errors.Wrap(err, "failed to marshal public key to bytes")
+			return nil, err
 		}
-		hash := sha1.Sum(pubBytes)
-		sub.SubjectKeyId = hash[:] // takes slice over the whole array
+		sub.SubjectKeyId = id
 	}
 
 	if sub.SerialNumber == nil {
@@ -498,3 +497,28 @@ func (b *base) CreateWriteCertificate(crtOut, keyOut, pass string) ([]byte, erro
 	}
 	return crtBytes, nil
 }
+
+// subjectPublicKeyInfo is a PKIX public key structure defined in RFC 5280.
+type subjectPublicKeyInfo struct {
+	Algorithm        pkix.AlgorithmIdentifier
+	SubjectPublicKey asn1.BitString
+}
+
+// generateSubjectKeyID generates the key identifier according the the RFC 5280
+// section 4.2.1.2.
+//
+// The keyIdentifier is composed of the 160-bit SHA-1 hash of the value of the
+// BIT STRING subjectPublicKey (excluding the tag, length, and number of unused
+// bits).
+func generateSubjectKeyID(pub crypto.PublicKey) ([]byte, error) {
+	b, err := x509.MarshalPKIXPublicKey(pub)
+	if err != nil {
+		return nil, errors.Wrap(err, "error marshaling public key")
+	}
+	var info subjectPublicKeyInfo
+	if _, err = asn1.Unmarshal(b, &info); err != nil {
+		return nil, errors.Wrap(err, "error unmarshaling public key")
+	}
+	hash := sha1.Sum(info.SubjectPublicKey.Bytes)
+	return hash[:], nil
+}
diff --git a/go.mod b/go.mod
index a7eaa9f1..a9c96a1b 100644
--- a/go.mod
+++ b/go.mod
@@ -18,7 +18,7 @@ require (
 	github.com/smallstep/assert v0.0.0-20200103212524-b99dc1097b15
 	github.com/smallstep/certificates v0.14.5
 	github.com/smallstep/certinfo v1.3.0
-	github.com/smallstep/truststore v0.9.3
+	github.com/smallstep/truststore v0.9.6
 	github.com/smallstep/zcrypto v0.0.0-20200203191936-fbc32cf76bce
 	github.com/smallstep/zlint v0.0.0-20180727184541-d84eaafe274f
 	github.com/stretchr/testify v1.5.1
diff --git a/go.sum b/go.sum
index 9038f0f6..3c780407 100644
--- a/go.sum
+++ b/go.sum
@@ -526,6 +526,8 @@ github.com/smallstep/nosql v0.3.0 h1:V1X5vfDsDt89499h3jZFUlR4VnnsYYs5tXaQZ0w8z5U
 github.com/smallstep/nosql v0.3.0/go.mod h1:QG7gNOpidifn99MjZaiNbm7HPesIyBd97F/OfacNz8Q=
 github.com/smallstep/truststore v0.9.3 h1:YAbCOZN4PrGApIPwUd95b71blpRJ/ZlbM9o6XwUugjo=
 github.com/smallstep/truststore v0.9.3/go.mod h1:PRSkpRIhAYBK/KLWkHNgRdYgzWMEy45bN7PSJCfKKGE=
+github.com/smallstep/truststore v0.9.6 h1:vNzEJmaJL0XOZD8uouXLmYu4/aP1UQ/wHUopH3qKeYA=
+github.com/smallstep/truststore v0.9.6/go.mod h1:HwHKRcBi0RUxxw1LYDpTRhYC4jZUuxPpkHdVonlkoDM=
 github.com/smallstep/zcrypto v0.0.0-20200203191936-fbc32cf76bce h1:cZMeOsWtXts169P82wfUkTCjReFOUTHVWKu/o5rv3YQ=
 github.com/smallstep/zcrypto v0.0.0-20200203191936-fbc32cf76bce/go.mod h1:+F24VU3UCxfVFvvqgm5jNUFQOm/L6ed13ImwWGFgg/g=
 github.com/smallstep/zlint v0.0.0-20180727184541-d84eaafe274f h1:mZxeNDk2+xg4eTEuR4y6z2+i6HBvcWjtdscDiUmBRzc=
@@ -877,6 +879,7 @@ honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.1-2019.2.3 h1:3JgtbtFHMiCmsznwGVTUWbgGov+pVqnlf1dEJTNAXeM=
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
+howett.net/plist v0.0.0-20181124034731-591f970eefbb/go.mod h1:vMygbs4qMhSZSc4lCUl2OEE+rDiIIJAIdR4m7MiMcm0=
 howett.net/plist v0.0.0-20200419221736-3b63eb3a43b5 h1:AQkaJpH+/FmqRjmXZPELom5zIERYZfwTjnHpfoVMQEc=
 howett.net/plist v0.0.0-20200419221736-3b63eb3a43b5/go.mod h1:vMygbs4qMhSZSc4lCUl2OEE+rDiIIJAIdR4m7MiMcm0=
 mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed h1:WX1yoOaKQfddO/mLzdV4wptyWgoH/6hwLs7QHTixo0I=


Test patch:
diff --git a/crypto/x509util/profile_test.go b/crypto/x509util/profile_test.go
index 1d3a6e8c..449351a1 100644
--- a/crypto/x509util/profile_test.go
+++ b/crypto/x509util/profile_test.go
@@ -1,6 +1,7 @@
 package x509util
 
 import (
+	"crypto"
 	"crypto/ecdsa"
 	"crypto/ed25519"
 	"crypto/elliptic"
@@ -13,6 +14,7 @@ import (
 	"io/ioutil"
 	"net"
 	"net/url"
+	"reflect"
 	"testing"
 
 	"github.com/pkg/errors"
@@ -37,6 +39,23 @@ func mustParseRSAKey(t *testing.T, filename string) *rsa.PrivateKey {
 	return key
 }
 
+func decodeCertificateFile(t *testing.T, filename string) *x509.Certificate {
+	t.Helper()
+	b, err := ioutil.ReadFile(filename)
+	if err != nil {
+		t.Fatal(err)
+	}
+	block, _ := pem.Decode(b)
+	if block == nil {
+		t.Fatal("error decoding pem")
+	}
+	crt, err := x509.ParseCertificate(block.Bytes)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return crt
+}
+
 type basicConstraints struct {
 	IsCA       bool `asn1:"optional"`
 	MaxPathLen int  `asn1:"optional,default:-1"`
@@ -396,3 +415,36 @@ func Test_base_CreateCertificate_KeyEncipherment(t *testing.T) {
 		})
 	}
 }
+
+func Test_generateSubjectKeyID(t *testing.T) {
+	ecdsaCrt := decodeCertificateFile(t, "test_files/google.crt")
+	rsaCrt := decodeCertificateFile(t, "test_files/smallstep.crt")
+	ed25519Crt := decodeCertificateFile(t, "test_files/ed25519.crt")
+
+	type args struct {
+		pub crypto.PublicKey
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    []byte
+		wantErr bool
+	}{
+		{"ecdsa", args{ecdsaCrt.PublicKey}, ecdsaCrt.SubjectKeyId, false},
+		{"rsa", args{rsaCrt.PublicKey}, rsaCrt.SubjectKeyId, false},
+		{"ed25519", args{ed25519Crt.PublicKey}, ed25519Crt.SubjectKeyId, false},
+		{"fail", args{[]byte("fail")}, nil, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := generateSubjectKeyID(tt.args.pub)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("generateSubjectKeyID() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("generateSubjectKeyID() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
diff --git a/crypto/x509util/test_files/ed25519.crt b/crypto/x509util/test_files/ed25519.crt
new file mode 100644
index 00000000..ef83f0dd
--- /dev/null
+++ b/crypto/x509util/test_files/ed25519.crt
@@ -0,0 +1,10 @@
+-----BEGIN CERTIFICATE-----
+MIIBWzCCAQ2gAwIBAgIUDIPYISuCyyOYI2Pi95eKQ1vzvZIwBQYDK2VwMCMxITAf
+BgNVBAMMGEVkMjU1MTkgdGVzdCBjZXJ0aWZpY2F0ZTAeFw0xOTA1MDYxNzI3MTZa
+Fw0xOTA2MDUxNzI3MTZaMCMxITAfBgNVBAMMGEVkMjU1MTkgdGVzdCBjZXJ0aWZp
+Y2F0ZTAqMAUGAytlcAMhADYpxWwNTxRsgdD/ddNqcF9pzQ9NZtXamH6CSYmjijz6
+o1MwUTAdBgNVHQ4EFgQUCTs6nUop2JX/aL57Q1Ry4K2i464wHwYDVR0jBBgwFoAU
+CTs6nUop2JX/aL57Q1Ry4K2i464wDwYDVR0TAQH/BAUwAwEB/zAFBgMrZXADQQBT
+pVgcLDsqnqydTqUdX11tprUI3hKC85cgrvrYmPQagzJrkfUkHcQgfyziTdoTO21U
+GtKoKNxgudT0eEs8HJEA
+-----END CERTIFICATE-----
\ No newline at end of file
diff --git a/crypto/x509util/test_files/google.crt b/crypto/x509util/test_files/google.crt
new file mode 100644
index 00000000..c7465328
--- /dev/null
+++ b/crypto/x509util/test_files/google.crt
@@ -0,0 +1,28 @@
+-----BEGIN CERTIFICATE-----
+MIIExjCCA66gAwIBAgIRAP1vPiSYwlsdCAAAAABH8DMwDQYJKoZIhvcNAQELBQAw
+QjELMAkGA1UEBhMCVVMxHjAcBgNVBAoTFUdvb2dsZSBUcnVzdCBTZXJ2aWNlczET
+MBEGA1UEAxMKR1RTIENBIDFPMTAeFw0yMDA2MTcxNDMxMjJaFw0yMDA5MDkxNDMx
+MjJaMGgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQH
+Ew1Nb3VudGFpbiBWaWV3MRMwEQYDVQQKEwpHb29nbGUgTExDMRcwFQYDVQQDEw53
+d3cuZ29vZ2xlLmNvbTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABOFYzxk3Ghpj
+cPvTNffIAqyY4p62NFaISj/X66RMGO7rs0RyM2I4ch1hiEP/alWb/+81BzA+R1nK
+w0ZKwy86Kh6jggJaMIICVjAOBgNVHQ8BAf8EBAMCB4AwEwYDVR0lBAwwCgYIKwYB
+BQUHAwEwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUjqNsRxKnCgdblFHWKj9y+TUG
+RSwwHwYDVR0jBBgwFoAUmNH4bhDrz5vsYJ8YkBug630J/SswaAYIKwYBBQUHAQEE
+XDBaMCsGCCsGAQUFBzABhh9odHRwOi8vb2NzcC5wa2kuZ29vZy9ndHMxbzFjb3Jl
+MCsGCCsGAQUFBzAChh9odHRwOi8vcGtpLmdvb2cvZ3NyMi9HVFMxTzEuY3J0MBkG
+A1UdEQQSMBCCDnd3dy5nb29nbGUuY29tMCEGA1UdIAQaMBgwCAYGZ4EMAQICMAwG
+CisGAQQB1nkCBQMwMwYDVR0fBCwwKjAooCagJIYiaHR0cDovL2NybC5wa2kuZ29v
+Zy9HVFMxTzFjb3JlLmNybDCCAQIGCisGAQQB1nkCBAIEgfMEgfAA7gB1ALIeBcyL
+os2KIE6HZvkruYolIGdr2vpw57JJUy3vi5BeAAABcsLn/z4AAAQDAEYwRAIgTB4w
+h0IyoPg5FFwxva6DkCumsXIIhKmHO1xk6HrOpDECIA0c5Y7Yq2mgGu03QPdbPRLb
+dBF5ISGltoW1ni2LkLIsAHUAXqdz+d9WwOe1Nkh90EngMnqRmgyEoRIShBh1loFx
+RVgAAAFywuf/bgAABAMARjBEAiBUcGdSwAP/13zLGH4xcJ5l7rYoif6lb7Ymv/4u
+intIHQIga1qNo582mt6FkPUYcKNq77MR9MPQYjJj+SQg266bbacwDQYJKoZIhvcN
+AQELBQADggEBAFKf5klhZEz9FwJlsu6/vnAn9lUKM2hKVKxO4B8T4f6PuY6dlZ3T
+g6mvdtsiLjAZ8v6WL6Bn0v15kTh9RhrKwEwVbLLfoXZbQBygV9B/k+HOCgMEb8U3
+Nn7chIP1kTJcyy3BW7TSU0WODmNuEusX0MQcs5TNl/omwdKkzpB6NjXWhnwgzCeD
+1nT33vvq1pwgg28ncIBJkqUpDca7hBDQjx1HYEq00euF5/zDWLV98mH8ORPSC2i2
+N7e0OMNT3q2HJRtHor5USuzehR86u3Aie90Z6jBvG/kNsGSgjUAdj/PAlVBj9GcF
+KtLwFt/ee1wps0/VGR+gqkoJps0BW8Tv+1k=
+-----END CERTIFICATE-----
\ No newline at end of file
diff --git a/crypto/x509util/test_files/smallstep.crt b/crypto/x509util/test_files/smallstep.crt
new file mode 100644
index 00000000..7cc4b49f
--- /dev/null
+++ b/crypto/x509util/test_files/smallstep.crt
@@ -0,0 +1,31 @@
+-----BEGIN CERTIFICATE-----
+MIIFZTCCBE2gAwIBAgISBPRFhEdtcBsNSxHgkoSkHpU/MA0GCSqGSIb3DQEBCwUA
+MEoxCzAJBgNVBAYTAlVTMRYwFAYDVQQKEw1MZXQncyBFbmNyeXB0MSMwIQYDVQQD
+ExpMZXQncyBFbmNyeXB0IEF1dGhvcml0eSBYMzAeFw0yMDA2MTYwNzE2NTZaFw0y
+MDA5MTQwNzE2NTZaMBoxGDAWBgNVBAMMDyouc21hbGxzdGVwLmNvbTCCASIwDQYJ
+KoZIhvcNAQEBBQADggEPADCCAQoCggEBAMgKbXAbD84RScmR3QGVlT27U09ihM6X
+XkVp4Ht4fbfm2SvpmOm7kLt5EB3f+0/I3enVsupCoULisUOQpEaoY9wXTjSHRSKl
+X3QPzyb8eJrwltxeo0qC5SidaVl2lXcSpKrKMvbp5qfd4hLhAJudEldmTFMQjzou
+/FouhzDpP4UDzf8Kc9b8Px27Qw7hg/888lJwCIcTAIVgmHUOxZKmjsHe0rHTNTYi
+mE+76udBPyG/Kis+ld2nuufqI/gPrD6gkm5pqSekt057zbk9oJTdBZTuAtaPGlER
+bH4G4fbkoT6j0tD7qafeebKATGBtzsK8gqwTZ2Uh4jzd5Ppukg2x4gkCAwEAAaOC
+AnMwggJvMA4GA1UdDwEB/wQEAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYB
+BQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUFlxnRkxa25R8P1zsLqtVMHBC
+MlUwHwYDVR0jBBgwFoAUqEpqYwR93brm0Tm3pkVl7/Oo7KEwbwYIKwYBBQUHAQEE
+YzBhMC4GCCsGAQUFBzABhiJodHRwOi8vb2NzcC5pbnQteDMubGV0c2VuY3J5cHQu
+b3JnMC8GCCsGAQUFBzAChiNodHRwOi8vY2VydC5pbnQteDMubGV0c2VuY3J5cHQu
+b3JnLzApBgNVHREEIjAggg8qLnNtYWxsc3RlcC5jb22CDXNtYWxsc3RlcC5jb20w
+TAYDVR0gBEUwQzAIBgZngQwBAgEwNwYLKwYBBAGC3xMBAQEwKDAmBggrBgEFBQcC
+ARYaaHR0cDovL2Nwcy5sZXRzZW5jcnlwdC5vcmcwggEEBgorBgEEAdZ5AgQCBIH1
+BIHyAPAAdgDwlaRZ8gDRgkAQLS+TiI6tS/4dR+OZ4dA0prCoqo6ycwAAAXK8M+PT
+AAAEAwBHMEUCIFjO361bM1BiXp9Nexw1KxJX34bI98JkBsHkSz3S+nSZAiEAuBy6
+KBUpYNLT71aPoVWtprZnxThUmKq2gm2Jltp5gMgAdgCyHgXMi6LNiiBOh2b5K7mK
+JSBna9r6cOeySVMt74uQXgAAAXK8M+PAAAAEAwBHMEUCIQCIqfGGfeGQHtYnVO4J
+UPTqIbNia+Lrbfw9HARElQw4iAIgSspUzNwYHY4cU/BdpfHRaTGzddrTFXhEoCQt
+0pbaG0wwDQYJKoZIhvcNAQELBQADggEBAJJgEB70PUChWsbLUyFk1udDPePanU5O
+zzSViB5+fesWppG9IDnYnL6KSI+l1jP9jl5Zym79SQ2gSC3xwkswpT8X+Drzup/7
+UV7T5NbHpzkZfg2itsx407yuxoW2L7zihZ4CWm1bodUbPKWNYJAZFrcxqcydwmFn
+pvZMoJDP3PW0hejz+gsLe9ZrtXb8q1BLFupHfx+bTx476qhRyL+e2fDi5wQz1Qs2
+jVZGsvv/cljvlRSLZ0NVPeKcRN50f7UQ7OYw+JLR2K5+K1H7oHiu+iUX4F2/PGGm
+Tb4HBKgzkJSsS5b3pEp+2U77m9KOBhsiZOVvQ2ECgJ9/eDiy0QCqEyI=
+-----END CERTIFICATE-----
\ No newline at end of file