diff --git a/ssh/keys.go b/ssh/keys.go index ef1bad731b..df4ebdada5 100644 --- a/ssh/keys.go +++ b/ssh/keys.go @@ -1232,16 +1232,27 @@ func ParseRawPrivateKeyWithPassphrase(pemBytes, passphrase []byte) (interface{}, return nil, fmt.Errorf("ssh: cannot decode encrypted private keys: %v", err) } + var result interface{} + switch block.Type { case "RSA PRIVATE KEY": - return x509.ParsePKCS1PrivateKey(buf) + result, err = x509.ParsePKCS1PrivateKey(buf) case "EC PRIVATE KEY": - return x509.ParseECPrivateKey(buf) + result, err = x509.ParseECPrivateKey(buf) case "DSA PRIVATE KEY": - return ParseDSAPrivateKey(buf) + result, err = ParseDSAPrivateKey(buf) default: - return nil, fmt.Errorf("ssh: unsupported key type %q", block.Type) + err = fmt.Errorf("ssh: unsupported key type %q", block.Type) } + // Because of deficiencies in the format, DecryptPEMBlock does not always + // detect an incorrect password. In these cases decrypted DER bytes is + // random noise. If the parsing of the key returns an asn1.StructuralError + // we return x509.IncorrectPasswordError. + if _, ok := err.(asn1.StructuralError); ok { + return nil, x509.IncorrectPasswordError + } + + return result, err } // ParseDSAPrivateKey returns a DSA private key from its ASN.1 DER encoding, as diff --git a/ssh/keys_test.go b/ssh/keys_test.go index 76d2338f43..3e18488f34 100644 --- a/ssh/keys_test.go +++ b/ssh/keys_test.go @@ -16,6 +16,7 @@ import ( "encoding/base64" "encoding/hex" "encoding/pem" + "errors" "fmt" "io" "reflect" @@ -221,6 +222,16 @@ func TestParseEncryptedPrivateKeysWithPassphrase(t *testing.T) { } } +func TestParseEncryptedPrivateKeysWithIncorrectPassphrase(t *testing.T) { + pem := testdata.PEMEncryptedKeys[0].PEMBytes + for i := 0; i < 4096; i++ { + _, err := ParseRawPrivateKeyWithPassphrase(pem, []byte(fmt.Sprintf("%d", i))) + if !errors.Is(err, x509.IncorrectPasswordError) { + t.Fatalf("expected error: %v, got: %v", x509.IncorrectPasswordError, err) + } + } +} + func TestParseDSA(t *testing.T) { // We actually exercise the ParsePrivateKey codepath here, as opposed to // using the ParseRawPrivateKey+NewSignerFromKey path that testdata_test.go