From 02b109bf0417026fe7825ef5abd3e7bf4595241f Mon Sep 17 00:00:00 2001 From: Mathijs van de Nes Date: Thu, 10 Sep 2015 12:47:26 +0200 Subject: [PATCH 1/4] Check _fromstr function for success --- openssl/src/crypto/pkey.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/openssl/src/crypto/pkey.rs b/openssl/src/crypto/pkey.rs index 48308381..5d4bd7f1 100644 --- a/openssl/src/crypto/pkey.rs +++ b/openssl/src/crypto/pkey.rs @@ -110,11 +110,16 @@ impl PKey { } } - fn _fromstr(&mut self, s: &[u8], f: unsafe extern "C" fn(*const *mut ffi::RSA, *const *const u8, c_uint) -> *mut ffi::RSA) { + fn _fromstr(&mut self, s: &[u8], f: unsafe extern "C" fn(*const *mut ffi::RSA, *const *const u8, c_uint) -> *mut ffi::RSA) -> bool { unsafe { let rsa = ptr::null_mut(); f(&rsa, &s.as_ptr(), s.len() as c_uint); - ffi::EVP_PKEY_set1_RSA(self.evp, rsa); + if !rsa.is_null() { + ffi::EVP_PKEY_set1_RSA(self.evp, rsa) == 1 + } + else { + false + } } } @@ -148,8 +153,9 @@ impl PKey { * Loads a serialized form of the public key, as produced by save_pub(). */ pub fn load_pub(&mut self, s: &[u8]) { - self._fromstr(s, ffi::d2i_RSA_PUBKEY); - self.parts = Parts::Public; + if self._fromstr(s, ffi::d2i_RSA_PUBKEY) { + self.parts = Parts::Public; + } } /** @@ -164,8 +170,9 @@ impl PKey { * save_priv(). */ pub fn load_priv(&mut self, s: &[u8]) { - self._fromstr(s, ffi::d2i_RSAPrivateKey); - self.parts = Parts::Both; + if self._fromstr(s, ffi::d2i_RSAPrivateKey) { + self.parts = Parts::Both; + } } /// Stores private key as a PEM From 0eb2f0ecfacf25f1055312ec9a715f25398d527d Mon Sep 17 00:00:00 2001 From: Mathijs van de Nes Date: Thu, 10 Sep 2015 12:58:52 +0200 Subject: [PATCH 2/4] Check rsa.is_null() before passing it to RSA_size RSA_size will cause an segmentation fault if it is null --- openssl/src/crypto/pkey.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/openssl/src/crypto/pkey.rs b/openssl/src/crypto/pkey.rs index 5d4bd7f1..980e8216 100644 --- a/openssl/src/crypto/pkey.rs +++ b/openssl/src/crypto/pkey.rs @@ -205,7 +205,13 @@ impl PKey { */ pub fn size(&self) -> usize { unsafe { - ffi::RSA_size(ffi::EVP_PKEY_get1_RSA(self.evp)) as usize + let rsa = ffi::EVP_PKEY_get1_RSA(self.evp); + if rsa.is_null() { + 0 + } + else { + ffi::RSA_size(rsa) as usize + } } } @@ -244,6 +250,9 @@ impl PKey { pub fn max_data(&self) -> usize { unsafe { let rsa = ffi::EVP_PKEY_get1_RSA(self.evp); + if rsa.is_null() { + return 0; + } let len = ffi::RSA_size(rsa); // 41 comes from RSA_public_encrypt(3) for OAEP @@ -254,6 +263,9 @@ impl PKey { pub fn encrypt_with_padding(&self, s: &[u8], padding: EncryptionPadding) -> Vec { unsafe { let rsa = ffi::EVP_PKEY_get1_RSA(self.evp); + if rsa.is_null() { + panic!("Could not get RSA key for encryption"); + } let len = ffi::RSA_size(rsa); assert!(s.len() < self.max_data()); @@ -279,6 +291,9 @@ impl PKey { pub fn decrypt_with_padding(&self, s: &[u8], padding: EncryptionPadding) -> Vec { unsafe { let rsa = ffi::EVP_PKEY_get1_RSA(self.evp); + if rsa.is_null() { + panic!("Could not get RSA key for decryption"); + } let len = ffi::RSA_size(rsa); assert_eq!(s.len() as c_int, ffi::RSA_size(rsa)); @@ -337,6 +352,9 @@ impl PKey { unsafe { let rsa = ffi::EVP_PKEY_get1_RSA(self.evp); let len = ffi::RSA_size(rsa); + if rsa.is_null() { + panic!("Could not get RSA key for signing"); + } let mut r = repeat(0u8).take(len as usize + 1).collect::>(); let mut len = 0; @@ -360,6 +378,9 @@ impl PKey { pub fn verify_with_hash(&self, h: &[u8], s: &[u8], hash: hash::Type) -> bool { unsafe { let rsa = ffi::EVP_PKEY_get1_RSA(self.evp); + if rsa.is_null() { + panic!("Could not get RSA key for verification"); + } let rv = ffi::RSA_verify( openssl_hash_nid(hash), From 3be32528e5b36ff80d5df9c569027bb8c8bcffe5 Mon Sep 17 00:00:00 2001 From: Mathijs van de Nes Date: Fri, 11 Sep 2015 09:23:51 +0200 Subject: [PATCH 3/4] Add tests to ensure a panic occurs instead of segv --- openssl/src/crypto/pkey.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/openssl/src/crypto/pkey.rs b/openssl/src/crypto/pkey.rs index 980e8216..d59a9cfb 100644 --- a/openssl/src/crypto/pkey.rs +++ b/openssl/src/crypto/pkey.rs @@ -560,4 +560,36 @@ mod tests { assert!(priv_key.windows(11).any(|s| s == b"PRIVATE KEY")); assert!(pub_key.windows(10).any(|s| s == b"PUBLIC KEY")); } + + #[test] + #[should_panic(expected = "Could not get RSA key for encryption")] + fn test_nokey_encrypt() { + let mut pkey = super::PKey::new(); + pkey.load_pub(&[]); + pkey.encrypt(&[]); + } + + #[test] + #[should_panic(expected = "Could not get RSA key for decryption")] + fn test_nokey_decrypt() { + let mut pkey = super::PKey::new(); + pkey.load_priv(&[]); + pkey.decrypt(&[]); + } + + #[test] + #[should_panic(expected = "Could not get RSA key for signing")] + fn test_nokey_sign() { + let mut pkey = super::PKey::new(); + pkey.load_priv(&[]); + pkey.sign(&[]); + } + + #[test] + #[should_panic(expected = "Could not get RSA key for verification")] + fn test_nokey_verify() { + let mut pkey = super::PKey::new(); + pkey.load_pub(&[]); + pkey.verify(&[], &[]); + } } From 87d5c0e4291051676d8e88513310e9eaa81aca0b Mon Sep 17 00:00:00 2001 From: Mathijs van de Nes Date: Fri, 11 Sep 2015 09:24:24 +0200 Subject: [PATCH 4/4] Fix one call to RSA_size found by tests --- openssl/src/crypto/pkey.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openssl/src/crypto/pkey.rs b/openssl/src/crypto/pkey.rs index d59a9cfb..5bf0e81e 100644 --- a/openssl/src/crypto/pkey.rs +++ b/openssl/src/crypto/pkey.rs @@ -351,10 +351,10 @@ impl PKey { pub fn sign_with_hash(&self, s: &[u8], hash: hash::Type) -> Vec { unsafe { let rsa = ffi::EVP_PKEY_get1_RSA(self.evp); - let len = ffi::RSA_size(rsa); if rsa.is_null() { panic!("Could not get RSA key for signing"); } + let len = ffi::RSA_size(rsa); let mut r = repeat(0u8).take(len as usize + 1).collect::>(); let mut len = 0;