Repository: smallstep/cli
Pull request #307 (2020-07-02 20:23:58)

Key encipherment
### Description

This PR disables key encipherment for leaf certificates without RSA keys.

See:
https://github.com/golang/go/issues/36499
https://tools.ietf.org/html/draft-ietf-lamps-5480-ku-clarifications-02


Patch:
diff --git a/command/certificate/remote.go b/command/certificate/remote.go
index 08bbce70..b070d6f2 100644
--- a/command/certificate/remote.go
+++ b/command/certificate/remote.go
@@ -3,6 +3,7 @@ package certificate
 import (
 	"crypto/tls"
 	"crypto/x509"
+	"net"
 	"net/url"
 	"strings"
 
@@ -34,8 +35,8 @@ func getPeerCertificates(addr, serverName, roots string, insecure bool) ([]*x509
 			return nil, errors.Wrapf(err, "failure to load root certificate pool from input path '%s'", roots)
 		}
 	}
-	if !strings.Contains(addr, ":") {
-		addr += ":443"
+	if _, _, err := net.SplitHostPort(addr); err != nil {
+		addr = net.JoinHostPort(addr, "443")
 	}
 	tlsConfig := &tls.Config{RootCAs: rootCAs}
 	if insecure {
diff --git a/crypto/x509util/leafProfile.go b/crypto/x509util/leafProfile.go
index aa8213eb..50a8e0f1 100644
--- a/crypto/x509util/leafProfile.go
+++ b/crypto/x509util/leafProfile.go
@@ -71,7 +71,10 @@ func defaultLeafTemplate(sub pkix.Name, iss pkix.Name) *x509.Certificate {
 		IsCA:      false,
 		NotBefore: notBefore,
 		NotAfter:  notBefore.Add(DefaultCertValidity),
-		KeyUsage:  x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
+		// KeyEncipherment MUST only be used for RSA keys. At signing time we
+		// will check the type of the key and remove the KeyEncipherment if
+		// necessary.
+		KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
 		ExtKeyUsage: []x509.ExtKeyUsage{
 			x509.ExtKeyUsageServerAuth,
 			x509.ExtKeyUsageClientAuth,
diff --git a/crypto/x509util/profile.go b/crypto/x509util/profile.go
index 7867acd8..7da1ee69 100644
--- a/crypto/x509util/profile.go
+++ b/crypto/x509util/profile.go
@@ -3,6 +3,7 @@ package x509util
 import (
 	"crypto"
 	"crypto/rand"
+	"crypto/rsa"
 	"crypto/sha1"
 	"crypto/x509"
 	"crypto/x509/pkix"
@@ -407,7 +408,8 @@ func (b *base) GenerateDefaultKeyPair() error {
 // CreateCertificate creates an x509 Certificate using the configuration stored
 // in the profile.
 func (b *base) CreateCertificate() ([]byte, error) {
-	if b.SubjectPublicKey() == nil {
+	pub := b.SubjectPublicKey()
+	if pub == nil {
 		return nil, errors.Errorf("Profile does not have subject public key. Need to call 'profile.GenerateKeyPair(...)' or use setters to populate keys")
 	}
 	if b.issPriv == nil {
@@ -420,7 +422,16 @@ func (b *base) CreateCertificate() ([]byte, error) {
 		sub.ExtraExtensions = append(sub.ExtraExtensions, b.ext...)
 	}
 
-	bytes, err := x509.CreateCertificate(rand.Reader, sub, iss, b.SubjectPublicKey(), b.issPriv)
+	// Remove KeyEncipherment and DataEncipherment for non-rsa keys.
+	// See:
+	// https://github.com/golang/go/issues/36499
+	// https://tools.ietf.org/html/draft-ietf-lamps-5480-ku-clarifications-02
+	if _, ok := pub.(*rsa.PublicKey); !ok {
+		sub.KeyUsage &= ^x509.KeyUsageKeyEncipherment
+		sub.KeyUsage &= ^x509.KeyUsageDataEncipherment
+	}
+
+	bytes, err := x509.CreateCertificate(rand.Reader, sub, iss, pub, b.issPriv)
 	return bytes, errors.WithStack(err)
 }
 


Test patch:
diff --git a/crypto/x509util/profile_test.go b/crypto/x509util/profile_test.go
new file mode 100644
index 00000000..3ea5bb68
--- /dev/null
+++ b/crypto/x509util/profile_test.go
@@ -0,0 +1,95 @@
+package x509util
+
+import (
+	"crypto/ecdsa"
+	"crypto/ed25519"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"encoding/pem"
+	"io/ioutil"
+	"testing"
+)
+
+func mustParseRSAKey(t *testing.T, filename string) *rsa.PrivateKey {
+	t.Helper()
+
+	b, err := ioutil.ReadFile("test_files/noPasscodeCa.key")
+	if err != nil {
+		t.Fatal(err)
+	}
+	block, _ := pem.Decode(b)
+	if block == nil {
+		t.Fatalf("error decoding %s", filename)
+	}
+	key, err := x509.ParsePKCS1PrivateKey(block.Bytes)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return key
+}
+
+func Test_base_CreateCertificate_KeyEncipherment(t *testing.T) {
+	// Issuer
+	iss := mustParseCertificate(t, "test_files/noPasscodeCa.crt")
+	issPriv := mustParseRSAKey(t, "test_files/noPasscodeCa.key")
+
+	mustCreateLeaf := func(key interface{}) Profile {
+		p, err := NewLeafProfile("test.smallstep.com", iss, issPriv, WithPublicKey(key))
+		if err != nil {
+			t.Fatal(err)
+		}
+		return p
+	}
+
+	// Keys and certs
+	ecdsaKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ecdsaProfile := mustCreateLeaf(ecdsaKey.Public())
+
+	rsaKey, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		t.Fatal(err)
+	}
+	rsaProfile := mustCreateLeaf(rsaKey.Public())
+
+	ed25519PubKey, _, err := ed25519.GenerateKey(rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ed25519Profile := mustCreateLeaf(ed25519PubKey)
+
+	tests := []struct {
+		name                string
+		profile             Profile
+		wantKeyEncipherment bool
+	}{
+		{"ecdsa", ecdsaProfile, false},
+		{"rsa", rsaProfile, true},
+		{"ed25519", ed25519Profile, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := tt.profile.CreateCertificate()
+			if err != nil {
+				t.Errorf("base.CreateCertificate() error = %v", err)
+				return
+			}
+			cert, err := x509.ParseCertificate(got)
+			if err != nil {
+				t.Errorf("error parsing certificate: %v", err)
+			} else {
+				ku := cert.KeyUsage & x509.KeyUsageKeyEncipherment
+				switch {
+				case tt.wantKeyEncipherment && ku == 0:
+					t.Errorf("base.CreateCertificate() keyUsage = %x, want %x", cert.KeyUsage, x509.KeyUsageDigitalSignature|x509.KeyUsageKeyEncipherment)
+				case !tt.wantKeyEncipherment && ku != 0:
+					t.Errorf("base.CreateCertificate() keyUsage = %x, want %x", cert.KeyUsage, x509.KeyUsageDigitalSignature)
+				}
+			}
+		})
+	}
+}