x/crypto/ssh: ParsePrivateKey errors out with encrypted private keys

RSA and DSA keys if encrypted have the phrase ENCRYPTED in their Proc-Type block header according to RFC 1421 Section 4.6.1.1. This CL checks for that phrase and errors out if we encounter it, since we don't yet have decryption of encrypted private keys. Fixes golang/go#6650 Change-Id: I5b157716a2f93557d289af5f62994234a2e7a0ed Reviewed-on: https://go-review.googlesource.com/29676 Reviewed-by: Han-Wen Nienhuys <[email protected]> Run-TryBot: Han-Wen Nienhuys <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
golang · Sep 29, 2016 · a20de3f · a20de3f
1 parent 7ac97eb
commit a20de3f
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 11 deletions.
diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go
@@ -464,7 +464,7 @@ func TestClientAuthNone(t *testing.T) {
 	go NewClientConn(c2, "", clientConfig)
 	serverConn, err := newServer(c1, serverConfig)
 	if err != nil {
-		t.Fatal("newServer: %v", err)
+		t.Fatalf("newServer: %v", err)
 	}
 	if serverConn.User() != user {
 		t.Fatalf("server: got %q, want %q", serverConn.User(), user)

diff --git a/ssh/keys.go b/ssh/keys.go
@@ -731,6 +731,14 @@ func ParsePrivateKey(pemBytes []byte) (Signer, error) {
 	return NewSignerFromKey(key)
 }
 
+// encryptedBlock tells whether a private key is
+// encrypted by examining its Proc-Type header
+// for a mention of ENCRYPTED
+// according to RFC 1421 Section 4.6.1.1.
+func encryptedBlock(block *pem.Block) bool {
+	return strings.Contains(block.Headers["Proc-Type"], "ENCRYPTED")
+}
+
 // ParseRawPrivateKey returns a private key from a PEM encoded private key. It
 // supports RSA (PKCS#1), DSA (OpenSSL), and ECDSA private keys.
 func ParseRawPrivateKey(pemBytes []byte) (interface{}, error) {
@@ -739,6 +747,10 @@ func ParseRawPrivateKey(pemBytes []byte) (interface{}, error) {
 		return nil, errors.New("ssh: no key found")
 	}
 
+	if encryptedBlock(block) {
+		return nil, errors.New("ssh: cannot decode encrypted private keys")
+	}
+
 	switch block.Type {
 	case "RSA PRIVATE KEY":
 		return x509.ParsePKCS1PrivateKey(block.Bytes)

diff --git a/ssh/keys_test.go b/ssh/keys_test.go
@@ -132,6 +132,22 @@ func TestParseECPrivateKey(t *testing.T) {
 	}
 }
 
+// See Issue https://github.com/golang/go/issues/6650.
+func TestParseEncryptedPrivateKeysFails(t *testing.T) {
+	const wantSubstring = "encrypted"
+	for i, tt := range testdata.PEMEncryptedKeys {
+		_, err := ParsePrivateKey(tt.PEMBytes)
+		if err == nil {
+			t.Errorf("#%d key %s: ParsePrivateKey successfully parsed, expected an error", i, tt.Name)
+			continue
+		}
+
+		if !strings.Contains(err.Error(), wantSubstring) {
+			t.Errorf("#%d key %s: got error %q, want substring %q", i, tt.Name, err, wantSubstring)
+		}
+	}
+}
+
 func TestParseDSA(t *testing.T) {
 	// We actually exercise the ParsePrivateKey codepath here, as opposed to
 	// using the ParseRawPrivateKey+NewSignerFromKey path that testdata_test.go
@@ -309,14 +325,14 @@ func TestInvalidEntry(t *testing.T) {
 }
 
 var knownHostsParseTests = []struct {
-	input     string
-	err       string
-
-	marker   string
-	comment  string
-	hosts    []string
-	rest     string
-} {
+	input string
+	err   string
+
+	marker  string
+	comment string
+	hosts   []string
+	rest    string
+}{
 	{
 		"",
 		"EOF",
@@ -375,13 +391,13 @@ var knownHostsParseTests = []struct {
 		"localhost,[host2:123]\tssh-rsa {RSAPUB}\tcomment comment",
 		"",
 
-		"", "comment comment", []string{"localhost","[host2:123]"}, "",
+		"", "comment comment", []string{"localhost", "[host2:123]"}, "",
 	},
 	{
 		"@marker \tlocalhost,[host2:123]\tssh-rsa {RSAPUB}",
 		"",
 
-		"marker", "", []string{"localhost","[host2:123]"}, "",
+		"marker", "", []string{"localhost", "[host2:123]"}, "",
 	},
 	{
 		"@marker \tlocalhost,[host2:123]\tssh-rsa aabbccdd",

diff --git a/ssh/testdata/keys.go b/ssh/testdata/keys.go
@@ -55,3 +55,66 @@ PLL8IEwvYu2wq+lpXfGQnNMbzYf9gspG0w==
 -----END EC PRIVATE KEY-----
 `),
 }
+
+var PEMEncryptedKeys = []struct {
+	Name          string
+	EncryptionKey string
+	PEMBytes      []byte
+}{
+	0: {
+		Name:          "rsa-encrypted",
+		EncryptionKey: "r54-G0pher_t3st$",
+		PEMBytes: []byte(`-----BEGIN RSA PRIVATE KEY-----
+Proc-Type: 4,ENCRYPTED
+DEK-Info: AES-128-CBC,3E1714DE130BC5E81327F36564B05462
+
+MqW88sud4fnWk/Jk3fkjh7ydu51ZkHLN5qlQgA4SkAXORPPMj2XvqZOv1v2LOgUV
+dUevUn8PZK7a9zbZg4QShUSzwE5k6wdB7XKPyBgI39mJ79GBd2U4W3h6KT6jIdWA
+goQpluxkrzr2/X602IaxLEre97FT9mpKC6zxKCLvyFWVIP9n3OSFS47cTTXyFr+l
+7PdRhe60nn6jSBgUNk/Q1lAvEQ9fufdPwDYY93F1wyJ6lOr0F1+mzRrMbH67NyKs
+rG8J1Fa7cIIre7ueKIAXTIne7OAWqpU9UDgQatDtZTbvA7ciqGsSFgiwwW13N+Rr
+hN8MkODKs9cjtONxSKi05s206A3NDU6STtZ3KuPDjFE1gMJODotOuqSM+cxKfyFq
+wxpk/CHYCDdMAVBSwxb/vraOHamylL4uCHpJdBHypzf2HABt+lS8Su23uAmL87DR
+yvyCS/lmpuNTndef6qHPRkoW2EV3xqD3ovosGf7kgwGJUk2ZpCLVteqmYehKlZDK
+r/Jy+J26ooI2jIg9bjvD1PZq+Mv+2dQ1RlDrPG3PB+rEixw6vBaL9x3jatCd4ej7
+XG7lb3qO9xFpLsx89tkEcvpGR+broSpUJ6Mu5LBCVmrvqHjvnDhrZVz1brMiQtU9
+iMZbgXqDLXHd6ERWygk7OTU03u+l1gs+KGMfmS0h0ZYw6KGVLgMnsoxqd6cFSKNB
+8Ohk9ZTZGCiovlXBUepyu8wKat1k8YlHSfIHoRUJRhhcd7DrmojC+bcbMIZBU22T
+Pl2ftVRGtcQY23lYd0NNKfebF7ncjuLWQGy+vZW+7cgfI6wPIbfYfP6g7QAutk6W
+KQx0AoX5woZ6cNxtpIrymaVjSMRRBkKQrJKmRp3pC/lul5E5P2cueMs1fj4OHTbJ
+lAUv88ywr+R+mRgYQlFW/XQ653f6DT4t6+njfO9oBcPrQDASZel3LjXLpjjYG/N5
++BWnVexuJX9ika8HJiFl55oqaKb+WknfNhk5cPY+x7SDV9ywQeMiDZpr0ffeYAEP
+LlwwiWRDYpO+uwXHSFF3+JjWwjhs8m8g99iFb7U93yKgBB12dCEPPa2ZeH9wUHMJ
+sreYhNuq6f4iWWSXpzN45inQqtTi8jrJhuNLTT543ErW7DtntBO2rWMhff3aiXbn
+Uy3qzZM1nPbuCGuBmP9L2dJ3Z5ifDWB4JmOyWY4swTZGt9AVmUxMIKdZpRONx8vz
+I9u9nbVPGZBcou50Pa0qTLbkWsSL94MNXrARBxzhHC9Zs6XNEtwN7mOuii7uMkVc
+adrxgknBH1J1N+NX/eTKzUwJuPvDtA+Z5ILWNN9wpZT/7ed8zEnKHPNUexyeT5g3
+uw9z9jH7ffGxFYlx87oiVPHGOrCXYZYW5uoZE31SCBkbtNuffNRJRKIFeipmpJ3P
+7bpAG+kGHMelQH6b+5K1Qgsv4tpuSyKeTKpPFH9Av5nN4P1ZBm9N80tzbNWqjSJm
+S7rYdHnuNEVnUGnRmEUMmVuYZnNBEVN/fP2m2SEwXcP3Uh7TiYlcWw10ygaGmOr7
+MvMLGkYgQ4Utwnd98mtqa0jr0hK2TcOSFir3AqVvXN3XJj4cVULkrXe4Im1laWgp
+-----END RSA PRIVATE KEY-----
+`),
+	},
+
+	1: {
+		Name:          "dsa-encrypted",
+		EncryptionKey: "qG0pher-dsa_t3st$",
+		PEMBytes: []byte(`-----BEGIN DSA PRIVATE KEY-----
+Proc-Type: 4,ENCRYPTED
+DEK-Info: AES-128-CBC,7CE7A6E4A647DC01AF860210B15ADE3E
+
+hvnBpI99Hceq/55pYRdOzBLntIEis02JFNXuLEydWL+RJBFDn7tA+vXec0ERJd6J
+G8JXlSOAhmC2H4uK3q2xR8/Y3yL95n6OIcjvCBiLsV+o3jj1MYJmErxP6zRtq4w3
+JjIjGHWmaYFSxPKQ6e8fs74HEqaeMV9ONUoTtB+aISmgaBL15Fcoayg245dkBvVl
+h5Kqspe7yvOBmzA3zjRuxmSCqKJmasXM7mqs3vIrMxZE3XPo1/fWKcPuExgpVQoT
+HkJZEoIEIIPnPMwT2uYbFJSGgPJVMDT84xz7yvjCdhLmqrsXgs5Qw7Pw0i0c0BUJ
+b7fDJ2UhdiwSckWGmIhTLlJZzr8K+JpjCDlP+REYBI5meB7kosBnlvCEHdw2EJkH
+0QDc/2F4xlVrHOLbPRFyu1Oi2Gvbeoo9EsM/DThpd1hKAlb0sF5Y0y0d+owv0PnE
+R/4X3HWfIdOHsDUvJ8xVWZ4BZk9Zk9qol045DcFCehpr/3hslCrKSZHakLt9GI58
+vVQJ4L0aYp5nloLfzhViZtKJXRLkySMKdzYkIlNmW1oVGl7tce5UCNI8Nok4j6yn
+IiHM7GBn+0nJoKTXsOGMIBe3ulKlKVxLjEuk9yivh/8=
+-----END DSA PRIVATE KEY-----
+`),
+	},
+}