From f92ac2477b3f857dfe2399a304033d9bcf3d1b3e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 6 Mar 2017 09:59:00 +0100 Subject: Add test to run into issue with stack.len() --- openssl/src/pkcs12.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'openssl/src') diff --git a/openssl/src/pkcs12.rs b/openssl/src/pkcs12.rs index 9f014af6..31864802 100644 --- a/openssl/src/pkcs12.rs +++ b/openssl/src/pkcs12.rs @@ -196,6 +196,17 @@ mod test { "c0cbdf7cdd03c9773e5468e1f6d2da7d5cbb1875"); } + #[test] + fn parse_empty_chain() { + let der = include_bytes!("../test/keystore-empty-chain.p12"); + let pkcs12 = Pkcs12::from_der(der).unwrap(); + let parsed = pkcs12.parse("cassandra").unwrap(); + + + assert_eq!(parsed.chain.len(), 0); + assert_eq!(parsed.chain.into_iter().collect::>().len(), 0); + } + #[test] fn create() { let subject_name = "ns.example.com"; -- cgit v1.2.3 From ec2685347c8eba32234a14c4fdc81d6ce83dc97d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 6 Mar 2017 10:14:39 +0100 Subject: Fix for empty stacks The culprit is that `sk_num(stack)` can return -1 as c_int if there is no stack allocated. Previously, thanks to unsafe casts, this would result in a isize::max() for len() and iteration size if there was no stack. Now this case is handled specifically, which fixes the issue. --- openssl/src/stack.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'openssl/src') diff --git a/openssl/src/stack.rs b/openssl/src/stack.rs index 268afde7..d9795a51 100644 --- a/openssl/src/stack.rs +++ b/openssl/src/stack.rs @@ -116,7 +116,7 @@ pub struct IntoIter { impl IntoIter { fn stack_len(&self) -> c_int { - unsafe { OPENSSL_sk_num(self.stack as *mut _) } + safe_stack_size(self.stack as *mut _) as c_int } } @@ -154,6 +154,15 @@ impl ExactSizeIterator for IntoIter {} pub struct StackRef(Opaque, PhantomData); +fn safe_stack_size(stack: *mut OPENSSL_STACK) -> usize { + let l = unsafe { OPENSSL_sk_num(stack) as isize }; + if l < 0 { + 0 + } else { + l as usize + } +} + impl ForeignTypeRef for StackRef { type CType = T::StackType; } @@ -165,7 +174,7 @@ impl StackRef { /// Returns the number of items in the stack pub fn len(&self) -> usize { - unsafe { OPENSSL_sk_num(self.as_stack()) as usize } + safe_stack_size(self.as_stack()) } pub fn iter(&self) -> Iter { -- cgit v1.2.3 From 463db85110658db729c722e6f2ef63fc67b4788b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 7 Mar 2017 07:39:25 +0100 Subject: Don't allow Stacks to be allocated with a null-ptr The latter must be seen as undefined behaviour, as it will cause the `sk_num` function to return -1 to indicate the error, which causes all kinds of issues. Thus there now is a panic to abort the program if stacks are initialized with a null-ptr, and special handling of that case when decoding a Pkcs file. --- openssl/src/pkcs12.rs | 9 +++++++-- openssl/src/stack.rs | 15 ++++----------- 2 files changed, 11 insertions(+), 13 deletions(-) (limited to 'openssl/src') diff --git a/openssl/src/pkcs12.rs b/openssl/src/pkcs12.rs index 31864802..31aae536 100644 --- a/openssl/src/pkcs12.rs +++ b/openssl/src/pkcs12.rs @@ -42,7 +42,12 @@ impl Pkcs12Ref { let pkey = PKey::from_ptr(pkey); let cert = X509::from_ptr(cert); - let chain = Stack::from_ptr(chain); + + let chain = if chain.is_null() { + try!(Stack::new()) + } else { + Stack::from_ptr(chain) + }; Ok(ParsedPkcs12 { pkey: pkey, @@ -80,6 +85,7 @@ impl Pkcs12 { pub struct ParsedPkcs12 { pub pkey: PKey, pub cert: X509, + // FIXME Make this Option in the next breaking release pub chain: Stack, } @@ -202,7 +208,6 @@ mod test { let pkcs12 = Pkcs12::from_der(der).unwrap(); let parsed = pkcs12.parse("cassandra").unwrap(); - assert_eq!(parsed.chain.len(), 0); assert_eq!(parsed.chain.into_iter().collect::>().len(), 0); } diff --git a/openssl/src/stack.rs b/openssl/src/stack.rs index d9795a51..6ac8264c 100644 --- a/openssl/src/stack.rs +++ b/openssl/src/stack.rs @@ -86,6 +86,8 @@ impl ForeignType for Stack { #[inline] unsafe fn from_ptr(ptr: *mut T::StackType) -> Stack { + assert!(!ptr.is_null(), "Must not instantiate a Stack from a null-ptr - use Stack::new() in \ + that case"); Stack(ptr) } @@ -116,7 +118,7 @@ pub struct IntoIter { impl IntoIter { fn stack_len(&self) -> c_int { - safe_stack_size(self.stack as *mut _) as c_int + unsafe { OPENSSL_sk_num(self.stack as *mut _) } } } @@ -154,15 +156,6 @@ impl ExactSizeIterator for IntoIter {} pub struct StackRef(Opaque, PhantomData); -fn safe_stack_size(stack: *mut OPENSSL_STACK) -> usize { - let l = unsafe { OPENSSL_sk_num(stack) as isize }; - if l < 0 { - 0 - } else { - l as usize - } -} - impl ForeignTypeRef for StackRef { type CType = T::StackType; } @@ -174,7 +167,7 @@ impl StackRef { /// Returns the number of items in the stack pub fn len(&self) -> usize { - safe_stack_size(self.as_stack()) + unsafe { OPENSSL_sk_num(self.as_stack()) as usize } } pub fn iter(&self) -> Iter { -- cgit v1.2.3