Skip to content

Commit a20de3f

Browse files
odeke-emhanwen
authored andcommitted
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]>
1 parent 7ac97eb commit a20de3f

File tree

4 files changed

+102
-11
lines changed

4 files changed

+102
-11
lines changed

ssh/client_auth_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ func TestClientAuthNone(t *testing.T) {
464464
go NewClientConn(c2, "", clientConfig)
465465
serverConn, err := newServer(c1, serverConfig)
466466
if err != nil {
467-
t.Fatal("newServer: %v", err)
467+
t.Fatalf("newServer: %v", err)
468468
}
469469
if serverConn.User() != user {
470470
t.Fatalf("server: got %q, want %q", serverConn.User(), user)

ssh/keys.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,14 @@ func ParsePrivateKey(pemBytes []byte) (Signer, error) {
731731
return NewSignerFromKey(key)
732732
}
733733

734+
// encryptedBlock tells whether a private key is
735+
// encrypted by examining its Proc-Type header
736+
// for a mention of ENCRYPTED
737+
// according to RFC 1421 Section 4.6.1.1.
738+
func encryptedBlock(block *pem.Block) bool {
739+
return strings.Contains(block.Headers["Proc-Type"], "ENCRYPTED")
740+
}
741+
734742
// ParseRawPrivateKey returns a private key from a PEM encoded private key. It
735743
// supports RSA (PKCS#1), DSA (OpenSSL), and ECDSA private keys.
736744
func ParseRawPrivateKey(pemBytes []byte) (interface{}, error) {
@@ -739,6 +747,10 @@ func ParseRawPrivateKey(pemBytes []byte) (interface{}, error) {
739747
return nil, errors.New("ssh: no key found")
740748
}
741749

750+
if encryptedBlock(block) {
751+
return nil, errors.New("ssh: cannot decode encrypted private keys")
752+
}
753+
742754
switch block.Type {
743755
case "RSA PRIVATE KEY":
744756
return x509.ParsePKCS1PrivateKey(block.Bytes)

ssh/keys_test.go

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,22 @@ func TestParseECPrivateKey(t *testing.T) {
132132
}
133133
}
134134

135+
// See Issue https://github.com/golang/go/issues/6650.
136+
func TestParseEncryptedPrivateKeysFails(t *testing.T) {
137+
const wantSubstring = "encrypted"
138+
for i, tt := range testdata.PEMEncryptedKeys {
139+
_, err := ParsePrivateKey(tt.PEMBytes)
140+
if err == nil {
141+
t.Errorf("#%d key %s: ParsePrivateKey successfully parsed, expected an error", i, tt.Name)
142+
continue
143+
}
144+
145+
if !strings.Contains(err.Error(), wantSubstring) {
146+
t.Errorf("#%d key %s: got error %q, want substring %q", i, tt.Name, err, wantSubstring)
147+
}
148+
}
149+
}
150+
135151
func TestParseDSA(t *testing.T) {
136152
// We actually exercise the ParsePrivateKey codepath here, as opposed to
137153
// using the ParseRawPrivateKey+NewSignerFromKey path that testdata_test.go
@@ -309,14 +325,14 @@ func TestInvalidEntry(t *testing.T) {
309325
}
310326

311327
var knownHostsParseTests = []struct {
312-
input string
313-
err string
314-
315-
marker string
316-
comment string
317-
hosts []string
318-
rest string
319-
} {
328+
input string
329+
err string
330+
331+
marker string
332+
comment string
333+
hosts []string
334+
rest string
335+
}{
320336
{
321337
"",
322338
"EOF",
@@ -375,13 +391,13 @@ var knownHostsParseTests = []struct {
375391
"localhost,[host2:123]\tssh-rsa {RSAPUB}\tcomment comment",
376392
"",
377393

378-
"", "comment comment", []string{"localhost","[host2:123]"}, "",
394+
"", "comment comment", []string{"localhost", "[host2:123]"}, "",
379395
},
380396
{
381397
"@marker \tlocalhost,[host2:123]\tssh-rsa {RSAPUB}",
382398
"",
383399

384-
"marker", "", []string{"localhost","[host2:123]"}, "",
400+
"marker", "", []string{"localhost", "[host2:123]"}, "",
385401
},
386402
{
387403
"@marker \tlocalhost,[host2:123]\tssh-rsa aabbccdd",

ssh/testdata/keys.go

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,66 @@ PLL8IEwvYu2wq+lpXfGQnNMbzYf9gspG0w==
5555
-----END EC PRIVATE KEY-----
5656
`),
5757
}
58+
59+
var PEMEncryptedKeys = []struct {
60+
Name string
61+
EncryptionKey string
62+
PEMBytes []byte
63+
}{
64+
0: {
65+
Name: "rsa-encrypted",
66+
EncryptionKey: "r54-G0pher_t3st$",
67+
PEMBytes: []byte(`-----BEGIN RSA PRIVATE KEY-----
68+
Proc-Type: 4,ENCRYPTED
69+
DEK-Info: AES-128-CBC,3E1714DE130BC5E81327F36564B05462
70+
71+
MqW88sud4fnWk/Jk3fkjh7ydu51ZkHLN5qlQgA4SkAXORPPMj2XvqZOv1v2LOgUV
72+
dUevUn8PZK7a9zbZg4QShUSzwE5k6wdB7XKPyBgI39mJ79GBd2U4W3h6KT6jIdWA
73+
goQpluxkrzr2/X602IaxLEre97FT9mpKC6zxKCLvyFWVIP9n3OSFS47cTTXyFr+l
74+
7PdRhe60nn6jSBgUNk/Q1lAvEQ9fufdPwDYY93F1wyJ6lOr0F1+mzRrMbH67NyKs
75+
rG8J1Fa7cIIre7ueKIAXTIne7OAWqpU9UDgQatDtZTbvA7ciqGsSFgiwwW13N+Rr
76+
hN8MkODKs9cjtONxSKi05s206A3NDU6STtZ3KuPDjFE1gMJODotOuqSM+cxKfyFq
77+
wxpk/CHYCDdMAVBSwxb/vraOHamylL4uCHpJdBHypzf2HABt+lS8Su23uAmL87DR
78+
yvyCS/lmpuNTndef6qHPRkoW2EV3xqD3ovosGf7kgwGJUk2ZpCLVteqmYehKlZDK
79+
r/Jy+J26ooI2jIg9bjvD1PZq+Mv+2dQ1RlDrPG3PB+rEixw6vBaL9x3jatCd4ej7
80+
XG7lb3qO9xFpLsx89tkEcvpGR+broSpUJ6Mu5LBCVmrvqHjvnDhrZVz1brMiQtU9
81+
iMZbgXqDLXHd6ERWygk7OTU03u+l1gs+KGMfmS0h0ZYw6KGVLgMnsoxqd6cFSKNB
82+
8Ohk9ZTZGCiovlXBUepyu8wKat1k8YlHSfIHoRUJRhhcd7DrmojC+bcbMIZBU22T
83+
Pl2ftVRGtcQY23lYd0NNKfebF7ncjuLWQGy+vZW+7cgfI6wPIbfYfP6g7QAutk6W
84+
KQx0AoX5woZ6cNxtpIrymaVjSMRRBkKQrJKmRp3pC/lul5E5P2cueMs1fj4OHTbJ
85+
lAUv88ywr+R+mRgYQlFW/XQ653f6DT4t6+njfO9oBcPrQDASZel3LjXLpjjYG/N5
86+
+BWnVexuJX9ika8HJiFl55oqaKb+WknfNhk5cPY+x7SDV9ywQeMiDZpr0ffeYAEP
87+
LlwwiWRDYpO+uwXHSFF3+JjWwjhs8m8g99iFb7U93yKgBB12dCEPPa2ZeH9wUHMJ
88+
sreYhNuq6f4iWWSXpzN45inQqtTi8jrJhuNLTT543ErW7DtntBO2rWMhff3aiXbn
89+
Uy3qzZM1nPbuCGuBmP9L2dJ3Z5ifDWB4JmOyWY4swTZGt9AVmUxMIKdZpRONx8vz
90+
I9u9nbVPGZBcou50Pa0qTLbkWsSL94MNXrARBxzhHC9Zs6XNEtwN7mOuii7uMkVc
91+
adrxgknBH1J1N+NX/eTKzUwJuPvDtA+Z5ILWNN9wpZT/7ed8zEnKHPNUexyeT5g3
92+
uw9z9jH7ffGxFYlx87oiVPHGOrCXYZYW5uoZE31SCBkbtNuffNRJRKIFeipmpJ3P
93+
7bpAG+kGHMelQH6b+5K1Qgsv4tpuSyKeTKpPFH9Av5nN4P1ZBm9N80tzbNWqjSJm
94+
S7rYdHnuNEVnUGnRmEUMmVuYZnNBEVN/fP2m2SEwXcP3Uh7TiYlcWw10ygaGmOr7
95+
MvMLGkYgQ4Utwnd98mtqa0jr0hK2TcOSFir3AqVvXN3XJj4cVULkrXe4Im1laWgp
96+
-----END RSA PRIVATE KEY-----
97+
`),
98+
},
99+
100+
1: {
101+
Name: "dsa-encrypted",
102+
EncryptionKey: "qG0pher-dsa_t3st$",
103+
PEMBytes: []byte(`-----BEGIN DSA PRIVATE KEY-----
104+
Proc-Type: 4,ENCRYPTED
105+
DEK-Info: AES-128-CBC,7CE7A6E4A647DC01AF860210B15ADE3E
106+
107+
hvnBpI99Hceq/55pYRdOzBLntIEis02JFNXuLEydWL+RJBFDn7tA+vXec0ERJd6J
108+
G8JXlSOAhmC2H4uK3q2xR8/Y3yL95n6OIcjvCBiLsV+o3jj1MYJmErxP6zRtq4w3
109+
JjIjGHWmaYFSxPKQ6e8fs74HEqaeMV9ONUoTtB+aISmgaBL15Fcoayg245dkBvVl
110+
h5Kqspe7yvOBmzA3zjRuxmSCqKJmasXM7mqs3vIrMxZE3XPo1/fWKcPuExgpVQoT
111+
HkJZEoIEIIPnPMwT2uYbFJSGgPJVMDT84xz7yvjCdhLmqrsXgs5Qw7Pw0i0c0BUJ
112+
b7fDJ2UhdiwSckWGmIhTLlJZzr8K+JpjCDlP+REYBI5meB7kosBnlvCEHdw2EJkH
113+
0QDc/2F4xlVrHOLbPRFyu1Oi2Gvbeoo9EsM/DThpd1hKAlb0sF5Y0y0d+owv0PnE
114+
R/4X3HWfIdOHsDUvJ8xVWZ4BZk9Zk9qol045DcFCehpr/3hslCrKSZHakLt9GI58
115+
vVQJ4L0aYp5nloLfzhViZtKJXRLkySMKdzYkIlNmW1oVGl7tce5UCNI8Nok4j6yn
116+
IiHM7GBn+0nJoKTXsOGMIBe3ulKlKVxLjEuk9yivh/8=
117+
-----END DSA PRIVATE KEY-----
118+
`),
119+
},
120+
}

0 commit comments

Comments
 (0)