eiac/crypto
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

openpgp/packet: improve handling of short MPIs for RSA values

Browse Source 
MPIs are (supposed to be) stripped of leading zeroes. Avoid passing
such short values into crypto/rsa, even if it currently happens to work.

Change-Id: I5a5f4813b8358e83fcc2deeda1272d2733814542
Reviewed-on: https://go-review.googlesource.com/100844
Reviewed-by: Adam Langley <agl@golang.org>
This commit is contained in:
Filippo Valsorda
2018-03-16 18:01:49 +00:00
parent b4956d363a
commit 374053ea96
4 changed files with 60 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
openpgp/packet/encrypted_key.go
+2 -1
View File
@@ -78,7 +78,8 @@ func (e *EncryptedKey) Decrypt(priv *PrivateKey, config *Config) error {
// padding oracle attacks.
switch priv.PubKeyAlgo {
case PubKeyAlgoRSA, PubKeyAlgoRSAEncryptOnly:
b, err = rsa.DecryptPKCS1v15(config.Random(), priv.PrivateKey.(*rsa.PrivateKey), e.encryptedMPI1.bytes)
k := priv.PrivateKey.(*rsa.PrivateKey)
b, err = rsa.DecryptPKCS1v15(config.Random(), k, padToKeySize(&k.PublicKey, e.encryptedMPI1.bytes))
case PubKeyAlgoElGamal:
c1 := new(big.Int).SetBytes(e.encryptedMPI1.bytes)
c2 := new(big.Int).SetBytes(e.encryptedMPI2.bytes)
openpgp/packet/encrypted_key_test.go
+34 -29
View File
@@ -39,39 +39,44 @@ var encryptedKeyPriv = &PrivateKey{
}
func TestDecryptingEncryptedKey(t *testing.T) {
const encryptedKeyHex = "c18c032a67d68660df41c70104005789d0de26b6a50c985a02a13131ca829c413a35d0e6fa8d6842599252162808ac7439c72151c8c6183e76923fe3299301414d0c25a2f06a2257db3839e7df0ec964773f6e4c4ac7ff3b48c444237166dd46ba8ff443a5410dc670cb486672fdbe7c9dfafb75b4fea83af3a204fe2a7dfa86bd20122b4f3d2646cbeecb8f7be8"
const expectedKeyHex = "d930363f7e0308c333b9618617ea728963d8df993665ae7be1092d4926fd864b"
for i, encryptedKeyHex := range []string{
"c18c032a67d68660df41c70104005789d0de26b6a50c985a02a13131ca829c413a35d0e6fa8d6842599252162808ac7439c72151c8c6183e76923fe3299301414d0c25a2f06a2257db3839e7df0ec964773f6e4c4ac7ff3b48c444237166dd46ba8ff443a5410dc670cb486672fdbe7c9dfafb75b4fea83af3a204fe2a7dfa86bd20122b4f3d2646cbeecb8f7be8",
// MPI can be shorter than the length of the key.
"c18b032a67d68660df41c70103f8e520c52ae9807183c669ce26e772e482dc5d8cf60e6f59316e145be14d2e5221ee69550db1d5618a8cb002a719f1f0b9345bde21536d410ec90ba86cac37748dec7933eb7f9873873b2d61d3321d1cd44535014f6df58f7bc0c7afb5edc38e1a974428997d2f747f9a173bea9ca53079b409517d332df62d805564cffc9be6",
} {
const expectedKeyHex = "d930363f7e0308c333b9618617ea728963d8df993665ae7be1092d4926fd864b"
p, err := Read(readerFromHex(encryptedKeyHex))
if err != nil {
t.Errorf("error from Read: %s", err)
return
}
ek, ok := p.(*EncryptedKey)
if !ok {
t.Errorf("didn't parse an EncryptedKey, got %#v", p)
return
}
p, err := Read(readerFromHex(encryptedKeyHex))
if err != nil {
t.Errorf("#%d: error from Read: %s", i, err)
return
}
ek, ok := p.(*EncryptedKey)
if !ok {
t.Errorf("#%d: didn't parse an EncryptedKey, got %#v", i, p)
return
}
if ek.KeyId != 0x2a67d68660df41c7 || ek.Algo != PubKeyAlgoRSA {
t.Errorf("unexpected EncryptedKey contents: %#v", ek)
return
}
if ek.KeyId != 0x2a67d68660df41c7 || ek.Algo != PubKeyAlgoRSA {
t.Errorf("#%d: unexpected EncryptedKey contents: %#v", i, ek)
return
}
err = ek.Decrypt(encryptedKeyPriv, nil)
if err != nil {
t.Errorf("error from Decrypt: %s", err)
return
}
err = ek.Decrypt(encryptedKeyPriv, nil)
if err != nil {
t.Errorf("#%d: error from Decrypt: %s", i, err)
return
}
if ek.CipherFunc != CipherAES256 {
t.Errorf("unexpected EncryptedKey contents: %#v", ek)
return
}
if ek.CipherFunc != CipherAES256 {
t.Errorf("#%d: unexpected EncryptedKey contents: %#v", i, ek)
return
}
keyHex := fmt.Sprintf("%x", ek.Key)
if keyHex != expectedKeyHex {
t.Errorf("bad key, got %s want %x", keyHex, expectedKeyHex)
keyHex := fmt.Sprintf("%x", ek.Key)
if keyHex != expectedKeyHex {
t.Errorf("#%d: bad key, got %s want %s", i, keyHex, expectedKeyHex)
}
}
}
@@ -121,7 +126,7 @@ func TestEncryptingEncryptedKey(t *testing.T) {
keyHex := fmt.Sprintf("%x", ek.Key)
if keyHex != expectedKeyHex {
t.Errorf("bad key, got %s want %x", keyHex, expectedKeyHex)
t.Errorf("bad key, got %s want %s", keyHex, expectedKeyHex)
}
}
openpgp/packet/packet.go
+22 -10
View File
@@ -11,10 +11,12 @@ import (
"crypto/aes"
"crypto/cipher"
"crypto/des"
"golang.org/x/crypto/cast5"
"golang.org/x/crypto/openpgp/errors"
"crypto/rsa"
"io"
"math/big"
"golang.org/x/crypto/cast5"
"golang.org/x/crypto/openpgp/errors"
)
// readFull is the same as io.ReadFull except that reading zero bytes returns
@@ -500,19 +502,17 @@ func readMPI(r io.Reader) (mpi []byte, bitLength uint16, err error) {
numBytes := (int(bitLength) + 7) / 8
mpi = make([]byte, numBytes)
_, err = readFull(r, mpi)
return
}
// mpiLength returns the length of the given *big.Int when serialized as an
// MPI.
func mpiLength(n *big.Int) (mpiLengthInBytes int) {
mpiLengthInBytes = 2 /* MPI length */
mpiLengthInBytes += (n.BitLen() + 7) / 8
// According to RFC 4880 3.2. we should check that the MPI has no leading
// zeroes (at least when not an encrypted MPI?), but this implementation
// does generate leading zeroes, so we keep accepting them.
return
}
// writeMPI serializes a big integer to w.
func writeMPI(w io.Writer, bitLength uint16, mpiBytes []byte) (err error) {
// Note that we can produce leading zeroes, in violation of RFC 4880 3.2.
// Implementations seem to be tolerant of them, and stripping them would
// make it complex to guarantee matching re-serialization.
_, err = w.Write([]byte{byte(bitLength >> 8), byte(bitLength)})
if err == nil {
_, err = w.Write(mpiBytes)
@@ -525,6 +525,18 @@ func writeBig(w io.Writer, i *big.Int) error {
return writeMPI(w, uint16(i.BitLen()), i.Bytes())
}
// padToKeySize left-pads a MPI with zeroes to match the length of the
// specified RSA public.
func padToKeySize(pub *rsa.PublicKey, b []byte) []byte {
k := (pub.N.BitLen() + 7) / 8
if len(b) >= k {
return b
}
bb := make([]byte, k)
copy(bb[len(bb)-len(b):], b)
return bb
}
// CompressionAlgo Represents the different compression algorithms
// supported by OpenPGP (except for BZIP2, which is not currently
// supported). See Section 9.3 of RFC 4880.
openpgp/packet/public_key.go
+2 -2
View File
@@ -520,7 +520,7 @@ func (pk *PublicKey) VerifySignature(signed hash.Hash, sig *Signature) (err erro
switch pk.PubKeyAlgo {
case PubKeyAlgoRSA, PubKeyAlgoRSASignOnly:
rsaPublicKey, _ := pk.PublicKey.(*rsa.PublicKey)
err = rsa.VerifyPKCS1v15(rsaPublicKey, sig.Hash, hashBytes, sig.RSASignature.bytes)
err = rsa.VerifyPKCS1v15(rsaPublicKey, sig.Hash, hashBytes, padToKeySize(rsaPublicKey, sig.RSASignature.bytes))
if err != nil {
return errors.SignatureError("RSA verification failure")
}
@@ -571,7 +571,7 @@ func (pk *PublicKey) VerifySignatureV3(signed hash.Hash, sig *SignatureV3) (err
switch pk.PubKeyAlgo {
case PubKeyAlgoRSA, PubKeyAlgoRSASignOnly:
rsaPublicKey := pk.PublicKey.(*rsa.PublicKey)
if err = rsa.VerifyPKCS1v15(rsaPublicKey, sig.Hash, hashBytes, sig.RSASignature.bytes); err != nil {
if err = rsa.VerifyPKCS1v15(rsaPublicKey, sig.Hash, hashBytes, padToKeySize(rsaPublicKey, sig.RSASignature.bytes)); err != nil {
return errors.SignatureError("RSA verification failure")
}
return