Merge pull request #506 from simias/stack

Implemented a generic Stack API and use it to deal with StackOf(X509) and StackOf(GENERAL_NAME)

5 files changed, 371 insertions, 136 deletions

diff --git a/openssl/src/lib.rs b/openssl/src/lib.rs
index d053606f..03f702b5 100644
--- a/openssl/src/lib.rs
+++ b/openssl/src/lib.rs
@@ -81,6 +81,7 @@ pub mod ssl;
 pub mod symm;
 pub mod version;
 pub mod x509;
+pub mod stack;
 #[cfg(any(ossl102, ossl110))]
 mod verify;
 
diff --git a/openssl/src/ssl/connector.rs b/openssl/src/ssl/connector.rs
index b2e00a81..c95b0fa1 100644
--- a/openssl/src/ssl/connector.rs
+++ b/openssl/src/ssl/connector.rs
@@ -256,7 +256,8 @@ mod verify {
     use std::str;
 
     use nid;
-    use x509::{X509StoreContext, X509, GeneralNames, X509Name};
+    use x509::{X509StoreContext, X509, X509Name, GeneralName};
+    use stack::Stack;
     use types::Ref;
 
     pub fn verify_callback(domain: &str,
@@ -275,15 +276,16 @@ mod verify {
 
     fn verify_hostname(domain: &str, cert: &Ref<X509>) -> bool {
         match cert.subject_alt_names() {
-            Some(names) => verify_subject_alt_names(domain, &names),
+            Some(names) => verify_subject_alt_names(domain, names),
             None => verify_subject_name(domain, &cert.subject_name()),
         }
     }
 
-    fn verify_subject_alt_names(domain: &str, names: &GeneralNames) -> bool {
+    fn verify_subject_alt_names(domain: &str,
+                                names: Stack<GeneralName>) -> bool {
         let ip = domain.parse();
 
-        for name in names {
+        for name in &names {
             match ip {
                 Ok(ip) => {
                     if let Some(actual) = name.ipaddress() {
diff --git a/openssl/src/stack.rs b/openssl/src/stack.rs
new file mode 100644
index 00000000..2cc53b5f
--- /dev/null
+++ b/openssl/src/stack.rs
@@ -0,0 +1,319 @@
+use std::ops::{Deref, DerefMut, Index, IndexMut};
+use std::iter;
+use std::borrow::Borrow;
+use std::convert::AsRef;
+
+#[cfg(ossl110)]
+use libc::c_int;
+
+use ffi;
+use types::{OpenSslType, Ref};
+
+/// Trait implemented by stackable types. This must *only* be
+/// implemented on opaque types that can be directly casted into their
+/// `CType`.
+pub trait Stackable: OpenSslType {
+    /// C stack type for this element. Generally called
+    /// `stack_st_{ELEMENT_TYPE}`, normally hidden by the
+    /// `STACK_OF(ELEMENT_TYPE)` macro in the OpenSSL API.
+    type StackType;
+}
+
+/// An owned stack of `T`.
+pub struct Stack<T: Stackable>(*mut T::StackType);
+
+impl<T: Stackable> Stack<T> {
+    /// Return a new Stack<T>, taking ownership of the handle
+    pub unsafe fn from_ptr(stack: *mut T::StackType) -> Stack<T> {
+        Stack(stack)
+    }
+}
+
+impl<T: Stackable> Drop for Stack<T> {
+    #[cfg(ossl10x)]
+    fn drop(&mut self) {
+        unsafe {
+            loop {
+                let ptr = ffi::sk_pop(self.as_stack());
+
+                if ptr.is_null() {
+                    break;
+                }
+
+                // Build the owned version of the object just to run
+                // its `drop` implementation and delete the item.
+                T::from_ptr(ptr as *mut _);
+            }
+
+            ffi::sk_free(self.0 as *mut _);
+        }
+    }
+
+    #[cfg(ossl110)]
+    fn drop(&mut self) {
+        unsafe {
+            loop {
+                let ptr = ffi::OPENSSL_sk_pop(self.as_stack());
+
+                if ptr.is_null() {
+                    break;
+                }
+
+                // Build the owned version of the object just to run
+                // its `drop` implementation and delete the item.
+                T::from_ptr(ptr as *mut _);
+            }
+
+            ffi::OPENSSL_sk_free(self.0 as *mut _);
+        }
+    }
+}
+
+impl<T: Stackable> AsRef<Ref<Stack<T>>> for Stack<T> {
+    fn as_ref(&self) -> &Ref<Stack<T>> {
+        &*self
+    }
+}
+
+impl<T: Stackable> Borrow<Ref<Stack<T>>> for Stack<T> {
+    fn borrow(&self) -> &Ref<Stack<T>> {
+        &*self
+    }
+}
+
+unsafe impl<T: Stackable> OpenSslType for Stack<T> {
+    type CType = T::StackType;
+
+    unsafe fn from_ptr(ptr: *mut T::StackType) -> Stack<T> {
+        Stack(ptr)
+    }
+
+    fn as_ptr(&self) -> *mut T::StackType {
+        self.0
+    }
+}
+
+impl<T: Stackable> Deref for Stack<T> {
+    type Target = Ref<Stack<T>>;
+
+    fn deref(&self) -> &Ref<Stack<T>> {
+        unsafe { Ref::from_ptr(self.0) }
+    }
+}
+
+impl<T: Stackable> DerefMut for Stack<T> {
+    fn deref_mut(&mut self) -> &mut ::types::Ref<Stack<T>> {
+        unsafe { Ref::from_ptr_mut(self.0) }
+    }
+}
+
+impl<T: Stackable> Ref<Stack<T>> {
+    /// OpenSSL stack types are just a (kinda) typesafe wrapper around
+    /// a `_STACK` object. We can therefore safely cast it and access
+    /// the `_STACK` members without having to worry about the real
+    /// layout of `T::StackType`.
+    ///
+    /// If that sounds unsafe then keep in mind that's exactly how the
+    /// OpenSSL 1.1.0 new C stack code works.
+    #[cfg(ossl10x)]
+    fn as_stack(&self) -> *mut ffi::_STACK {
+        self.as_ptr() as *mut _
+    }
+
+    /// OpenSSL 1.1.0 replaced the stack macros with a functions and
+    /// only exposes an opaque OPENSSL_STACK struct
+    /// publicly.
+    #[cfg(ossl110)]
+    fn as_stack(&self) -> *mut ffi::OPENSSL_STACK {
+        self.as_ptr() as *mut _
+    }
+
+    /// Returns the number of items in the stack
+    pub fn len(&self) -> usize {
+        self._len()
+    }
+
+    #[cfg(ossl10x)]
+    fn _len(&self) -> usize {
+        unsafe { (*self.as_stack()).num as usize }
+    }
+
+    #[cfg(ossl110)]
+    fn _len(&self) -> usize {
+        unsafe { ffi::OPENSSL_sk_num(self.as_stack()) as usize }
+    }
+
+    pub fn iter(&self) -> Iter<T> {
+        // Unfortunately we can't simply convert the stack into a
+        // slice and use that because OpenSSL 1.1.0 doesn't directly
+        // expose the stack data (we have to use `OPENSSL_sk_value`
+        // instead). We have to rewrite the entire iteration framework
+        // instead.
+
+        Iter {
+            stack: self,
+            pos: 0,
+        }
+    }
+
+    pub fn iter_mut(&mut self) -> IterMut<T> {
+        IterMut {
+            stack: self,
+            pos: 0,
+        }
+    }
+
+    /// Returns a reference to the element at the given index in the
+    /// stack or `None` if the index is out of bounds
+    pub fn get(&self, idx: usize) -> Option<&Ref<T>> {
+        if idx >= self.len() {
+            return None;
+        }
+
+        unsafe {
+            let r = Ref::from_ptr(self._get(idx));
+
+            Some(r)
+        }
+    }
+
+    /// Returns a mutable reference to the element at the given index in the
+    /// stack or `None` if the index is out of bounds
+    pub fn get_mut(&mut self, idx: usize) -> Option<&mut Ref<T>> {
+        if idx >= self.len() {
+            return None;
+        }
+
+        unsafe {
+            Some(Ref::from_ptr_mut(self._get(idx)))
+        }
+    }
+
+    #[cfg(ossl10x)]
+    unsafe fn _get(&self, idx: usize) -> *mut T::CType {
+        *(*self.as_stack()).data.offset(idx as isize) as *mut _
+    }
+
+    #[cfg(ossl110)]
+    unsafe fn _get(&self, idx: usize) -> *mut T::CType {
+        ffi::OPENSSL_sk_value(self.as_stack(), idx as c_int) as *mut _
+    }
+}
+
+impl<T: Stackable> Index<usize> for Ref<Stack<T>> {
+    type Output = Ref<T>;
+
+    fn index(&self, index: usize) -> &Ref<T> {
+        self.get(index).unwrap()
+    }
+}
+
+impl<T: Stackable> IndexMut<usize> for Ref<Stack<T>> {
+    fn index_mut(&mut self, index: usize) -> &mut Ref<T> {
+        self.get_mut(index).unwrap()
+    }
+}
+
+impl<'a, T: Stackable> iter::IntoIterator for &'a Ref<Stack<T>> {
+    type Item = &'a Ref<T>;
+    type IntoIter = Iter<'a, T>;
+
+    fn into_iter(self) -> Iter<'a, T> {
+        self.iter()
+    }
+}
+
+impl<'a, T: Stackable> iter::IntoIterator for &'a mut Ref<Stack<T>> {
+    type Item = &'a mut Ref<T>;
+    type IntoIter = IterMut<'a, T>;
+
+    fn into_iter(self) -> IterMut<'a, T> {
+        self.iter_mut()
+    }
+}
+
+impl<'a, T: Stackable> iter::IntoIterator for &'a Stack<T> {
+    type Item = &'a Ref<T>;
+    type IntoIter = Iter<'a, T>;
+
+    fn into_iter(self) -> Iter<'a, T> {
+        self.iter()
+    }
+}
+
+impl<'a, T: Stackable> iter::IntoIterator for &'a mut Stack<T> {
+    type Item = &'a mut Ref<T>;
+    type IntoIter = IterMut<'a, T>;
+
+    fn into_iter(self) -> IterMut<'a, T> {
+        self.iter_mut()
+    }
+}
+
+/// An iterator over the stack's contents.
+pub struct Iter<'a, T: Stackable>
+    where T: 'a {
+    stack: &'a Ref<Stack<T>>,
+    pos: usize,
+}
+
+impl<'a, T: Stackable> iter::Iterator for Iter<'a, T> {
+    type Item = &'a Ref<T>;
+
+    fn next(&mut self) -> Option<&'a Ref<T>> {
+        let n = self.stack.get(self.pos);
+
+        if n.is_some() {
+            self.pos += 1;
+        }
+
+        n
+    }
+
+    fn size_hint(&self) -> (usize, Option<usize>) {
+        let rem = self.stack.len() - self.pos;
+
+        (rem, Some(rem))
+    }
+}
+
+impl<'a, T: Stackable> iter::ExactSizeIterator for Iter<'a, T> {
+}
+
+/// A mutable iterator over the stack's contents.
+pub struct IterMut<'a, T: Stackable + 'a> {
+    stack: &'a mut Ref<Stack<T>>,
+    pos: usize,
+}
+
+impl<'a, T: Stackable> iter::Iterator for IterMut<'a, T> {
+    type Item = &'a mut Ref<T>;
+
+    fn next(&mut self) -> Option<&'a mut Ref<T>> {
+        if self.pos >= self.stack.len() {
+            None
+        } else {
+            // Rust won't allow us to get a mutable reference into
+            // `stack` in this situation since it can't statically
+            // guarantee that we won't return several references to
+            // the same object, so we have to use unsafe code for
+            // mutable iterators.
+            let n = unsafe {
+                Some(Ref::from_ptr_mut(self.stack._get(self.pos)))
+            };
+
+            self.pos += 1;
+
+            n
+        }
+    }
+
+    fn size_hint(&self) -> (usize, Option<usize>) {
+        let rem = self.stack.len() - self.pos;
+
+        (rem, Some(rem))
+    }
+}
+
+impl<'a, T: Stackable> iter::ExactSizeIterator for IterMut<'a, T> {
+}
diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs
index 28f2542f..e27b03d1 100644
--- a/openssl/src/x509/mod.rs
+++ b/openssl/src/x509/mod.rs
@@ -1,11 +1,10 @@
-use libc::{c_char, c_int, c_long, c_ulong, c_void};
+use libc::{c_char, c_int, c_long, c_ulong};
 use std::borrow::Borrow;
 use std::cmp;
 use std::collections::HashMap;
 use std::error::Error;
 use std::ffi::{CStr, CString};
 use std::fmt;
-use std::marker::PhantomData;
 use std::mem;
 use std::ptr;
 use std::slice;
@@ -21,6 +20,7 @@ use error::ErrorStack;
 use ffi;
 use nid::Nid;
 use types::{OpenSslType, Ref};
+use stack::{Stack, Stackable};
 
 #[cfg(ossl10x)]
 use ffi::{X509_set_notBefore, X509_set_notAfter, ASN1_STRING_data};
@@ -67,6 +67,28 @@ impl Ref<X509StoreContext> {
     pub fn error_depth(&self) -> u32 {
         unsafe { ffi::X509_STORE_CTX_get_error_depth(self.as_ptr()) as u32 }
     }
+
+    pub fn get_chain(&self) -> Option<&Ref<Stack<X509>>> {
+        unsafe {
+            let chain = self._get_chain();
+
+            if chain.is_null() {
+                return None;
+            }
+
+            Some(Ref::from_ptr(chain))
+        }
+    }
+
+    #[cfg(ossl110)]
+    unsafe fn _get_chain(&self) -> *mut ffi::stack_st_X509 {
+        ffi::X509_STORE_CTX_get0_chain(self.as_ptr())
+    }
+
+    #[cfg(ossl10x)]
+    unsafe fn _get_chain(&self) -> *mut ffi::stack_st_X509 {
+        ffi::X509_STORE_CTX_get_chain(self.as_ptr())
+    }
 }
 
 #[allow(non_snake_case)]
@@ -346,7 +368,7 @@ impl Ref<X509> {
     }
 
     /// Returns this certificate's SAN entries, if they exist.
-    pub fn subject_alt_names(&self) -> Option<GeneralNames> {
+    pub fn subject_alt_names(&self) -> Option<Stack<GeneralName>> {
         unsafe {
             let stack = ffi::X509_get_ext_d2i(self.as_ptr(),
                                               ffi::NID_subject_alt_name,
@@ -356,7 +378,7 @@ impl Ref<X509> {
                 return None;
             }
 
-            Some(GeneralNames { stack: stack as *mut _ })
+            Some(Stack::from_ptr(stack as *mut _))
         }
     }
 
@@ -468,6 +490,10 @@ impl Borrow<Ref<X509>> for X509 {
         &*self
     }
 }
+ 
+impl Stackable for X509 {
+    type StackType = ffi::stack_st_X509;
+}
 
 type_!(X509Name, ffi::X509_NAME, ffi::X509_NAME_free);
 
@@ -678,135 +704,18 @@ impl X509VerifyError {
     }
 }
 
-/// A collection of OpenSSL `GENERAL_NAME`s.
-pub struct GeneralNames {
-    stack: *mut ffi::stack_st_GENERAL_NAME,
-}
+type_!(GeneralName, ffi::GENERAL_NAME, ffi::GENERAL_NAME_free);
 
-impl Drop for GeneralNames {
-    #[cfg(ossl10x)]
-    fn drop(&mut self) {
-        unsafe {
-            // This transmute is dubious but it's what openssl itself does...
-            let free: unsafe extern "C" fn(*mut ffi::GENERAL_NAME) = ffi::GENERAL_NAME_free;
-            let free: unsafe extern "C" fn(*mut c_void) = mem::transmute(free);
-            ffi::sk_pop_free(&mut (*self.stack).stack, Some(free));
-        }
-    }
-
-    #[cfg(ossl110)]
-    fn drop(&mut self) {
-        unsafe {
-            // This transmute is dubious but it's what openssl itself does...
-            let free: unsafe extern "C" fn(*mut ffi::GENERAL_NAME) = ffi::GENERAL_NAME_free;
-            let free: unsafe extern "C" fn(*mut c_void) = mem::transmute(free);
-            ffi::OPENSSL_sk_pop_free(self.stack as *mut _, Some(free));
-        }
-    }
-}
-
-impl GeneralNames {
-    /// Returns the number of `GeneralName`s in this structure.
-    pub fn len(&self) -> usize {
-        self._len()
-    }
-
-    #[cfg(ossl10x)]
-    fn _len(&self) -> usize {
-        unsafe { (*self.stack).stack.num as usize }
-    }
-
-    #[cfg(ossl110)]
-    fn _len(&self) -> usize {
-        unsafe { ffi::OPENSSL_sk_num(self.stack as *const _) as usize }
-    }
-
-    /// Returns the specified `GeneralName`.
-    ///
-    /// # Panics
-    ///
-    /// Panics if `idx` is not less than `len()`.
-    pub fn get<'a>(&'a self, idx: usize) -> GeneralName<'a> {
-        unsafe {
-            assert!(idx < self.len());
-            GeneralName {
-                name: self._get(idx),
-                m: PhantomData,
-            }
-        }
-    }
-
-    #[cfg(ossl10x)]
-    unsafe fn _get(&self, idx: usize) -> *const ffi::GENERAL_NAME {
-        *(*self.stack).stack.data.offset(idx as isize) as *const ffi::GENERAL_NAME
-    }
-
-    #[cfg(ossl110)]
-    unsafe fn _get(&self, idx: usize) -> *const ffi::GENERAL_NAME {
-        ffi::OPENSSL_sk_value(self.stack as *const _, idx as c_int) as *mut _
-    }
-
-    /// Returns an iterator over the `GeneralName`s in this structure.
-    pub fn iter(&self) -> GeneralNamesIter {
-        GeneralNamesIter {
-            names: self,
-            idx: 0,
-        }
-    }
-}
-
-impl<'a> IntoIterator for &'a GeneralNames {
-    type Item = GeneralName<'a>;
-    type IntoIter = GeneralNamesIter<'a>;
-
-    fn into_iter(self) -> GeneralNamesIter<'a> {
-        self.iter()
-    }
-}
-
-/// An iterator over OpenSSL `GENERAL_NAME`s.
-pub struct GeneralNamesIter<'a> {
-    names: &'a GeneralNames,
-    idx: usize,
-}
-
-impl<'a> Iterator for GeneralNamesIter<'a> {
-    type Item = GeneralName<'a>;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        if self.idx < self.names.len() {
-            let name = self.names.get(self.idx);
-            self.idx += 1;
-            Some(name)
-        } else {
-            None
-        }
-    }
-
-    fn size_hint(&self) -> (usize, Option<usize>) {
-        let size = self.names.len() - self.idx;
-        (size, Some(size))
-    }
-}
-
-impl<'a> ExactSizeIterator for GeneralNamesIter<'a> {}
-
-/// An OpenSSL `GENERAL_NAME`.
-pub struct GeneralName<'a> {
-    name: *const ffi::GENERAL_NAME,
-    m: PhantomData<&'a ()>,
-}
-
-impl<'a> GeneralName<'a> {
+impl Ref<GeneralName> {
     /// Returns the contents of this `GeneralName` if it is a `dNSName`.
     pub fn dnsname(&self) -> Option<&str> {
         unsafe {
-            if (*self.name).type_ != ffi::GEN_DNS {
+            if (*self.as_ptr()).type_ != ffi::GEN_DNS {
                 return None;
             }
 
-            let ptr = ASN1_STRING_data((*self.name).d as *mut _);
-            let len = ffi::ASN1_STRING_length((*self.name).d as *mut _);
+            let ptr = ASN1_STRING_data((*self.as_ptr()).d as *mut _);
+            let len = ffi::ASN1_STRING_length((*self.as_ptr()).d as *mut _);
 
             let slice = slice::from_raw_parts(ptr as *const u8, len as usize);
             // dNSNames are stated to be ASCII (specifically IA5). Hopefully
@@ -819,18 +728,22 @@ impl<'a> GeneralName<'a> {
     /// Returns the contents of this `GeneralName` if it is an `iPAddress`.
     pub fn ipaddress(&self) -> Option<&[u8]> {
         unsafe {
-            if (*self.name).type_ != ffi::GEN_IPADD {
+            if (*self.as_ptr()).type_ != ffi::GEN_IPADD {
                 return None;
             }
 
-            let ptr = ASN1_STRING_data((*self.name).d as *mut _);
-            let len = ffi::ASN1_STRING_length((*self.name).d as *mut _);
+            let ptr = ASN1_STRING_data((*self.as_ptr()).d as *mut _);
+            let len = ffi::ASN1_STRING_length((*self.as_ptr()).d as *mut _);
 
             Some(slice::from_raw_parts(ptr as *const u8, len as usize))
         }
     }
 }
 
+impl Stackable for GeneralName {
+    type StackType = ffi::stack_st_GENERAL_NAME;
+}
+
 #[test]
 fn test_negative_serial() {
     // I guess that's enough to get a random negative number
diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs
index db93c3a8..2527d538 100644
--- a/openssl/src/x509/tests.rs
+++ b/openssl/src/x509/tests.rs
@@ -152,10 +152,10 @@ fn test_subject_alt_name() {
 
     let subject_alt_names = cert.subject_alt_names().unwrap();
     assert_eq!(3, subject_alt_names.len());
-    assert_eq!(Some("foobar.com"), subject_alt_names.get(0).dnsname());
-    assert_eq!(subject_alt_names.get(1).ipaddress(),
+    assert_eq!(Some("foobar.com"), subject_alt_names[0].dnsname());
+    assert_eq!(subject_alt_names[1].ipaddress(),
                Some(&[127, 0, 0, 1][..]));
-    assert_eq!(subject_alt_names.get(2).ipaddress(),
+    assert_eq!(subject_alt_names[2].ipaddress(),
                Some(&b"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x01"[..]));
 }