ssh: reject unencrypted keys from ParsePrivateKeyWithPassphrase
The behavior of ParsePrivateKeyWithPassphrase when the key is unencrypted is unspecified. Currently, it just parses them like ParsePrivateKey, which is unlikely to be what anyone wants: for us to ignore a passphrase that they explicitly passed. It also makes the implementation of encrypted OpenSSH keys in the next CL more confused. Instead, make ParsePrivateKey return a PassphraseNeededError, so the application logic can be ParsePrivateKey -> detect encrypted key -> obtain passphrase -> ParsePrivateKeyWithPassphrase. That error will also let us return the public key for OpenSSH keys. Change-Id: Ife4fb2499ae538bef36e353adf9bc8e902662386 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/207599 Run-TryBot: Filippo Valsorda <filippo@golang.org> Run-TryBot: Han-Wen Nienhuys <hanwen@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
This commit is contained in:
committed by
Filippo Valsorda
parent
86a70503ff
commit
0a08dada0f
+35
-34
@@ -179,44 +179,45 @@ 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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Parse encrypted private keys with passphrase
|
||||
func TestParseEncryptedPrivateKeysWithPassphrase(t *testing.T) {
|
||||
data := []byte("sign me")
|
||||
for _, tt := range testdata.PEMEncryptedKeys {
|
||||
s, err := ParsePrivateKeyWithPassphrase(tt.PEMBytes, []byte(tt.EncryptionKey))
|
||||
if err != nil {
|
||||
t.Fatalf("ParsePrivateKeyWithPassphrase returned error: %s", err)
|
||||
continue
|
||||
}
|
||||
sig, err := s.Sign(rand.Reader, data)
|
||||
if err != nil {
|
||||
t.Fatalf("dsa.Sign: %v", err)
|
||||
}
|
||||
if err := s.PublicKey().Verify(data, sig); err != nil {
|
||||
t.Errorf("Verify failed: %v", err)
|
||||
}
|
||||
}
|
||||
t.Run(tt.Name, func(t *testing.T) {
|
||||
_, err := ParsePrivateKeyWithPassphrase(tt.PEMBytes, []byte("incorrect"))
|
||||
if err != x509.IncorrectPasswordError {
|
||||
t.Errorf("got %v want IncorrectPasswordError", err)
|
||||
}
|
||||
|
||||
tt := testdata.PEMEncryptedKeys[0]
|
||||
_, err := ParsePrivateKeyWithPassphrase(tt.PEMBytes, []byte("incorrect"))
|
||||
if err != x509.IncorrectPasswordError {
|
||||
t.Fatalf("got %v want IncorrectPasswordError", err)
|
||||
s, err := ParsePrivateKeyWithPassphrase(tt.PEMBytes, []byte(tt.EncryptionKey))
|
||||
if err != nil {
|
||||
t.Fatalf("ParsePrivateKeyWithPassphrase returned error: %s", err)
|
||||
}
|
||||
|
||||
sig, err := s.Sign(rand.Reader, data)
|
||||
if err != nil {
|
||||
t.Fatalf("Signer.Sign: %v", err)
|
||||
}
|
||||
if err := s.PublicKey().Verify(data, sig); err != nil {
|
||||
t.Errorf("Verify failed: %v", err)
|
||||
}
|
||||
|
||||
_, err = ParsePrivateKey(tt.PEMBytes)
|
||||
if err == nil {
|
||||
t.Fatalf("ParsePrivateKey succeeded, expected an error")
|
||||
}
|
||||
|
||||
if err, ok := err.(*PassphraseMissingError); !ok {
|
||||
t.Errorf("got error %q, want PassphraseMissingError", err)
|
||||
} else if tt.IncludesPublicKey {
|
||||
if err.PublicKey == nil {
|
||||
t.Fatalf("expected PassphraseMissingError.PublicKey not to be nil")
|
||||
}
|
||||
got, want := err.PublicKey.Marshal(), s.PublicKey().Marshal()
|
||||
if !bytes.Equal(got, want) {
|
||||
t.Errorf("error field %q doesn't match signer public key %q", got, want)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user