Fix SslContext::add_extra_chain_cert

SSL_CTX_add_extra_chain_cert assumes ownership of the certificate, so
the method really needs to take an X509 by value. Work around this by
manually cloning the cert.

This method has been around for over a year but I'm guessing nobody
actually used it since it produces a nice double free into segfault!
This commit is contained in:
Steven Fackler 2016-08-17 19:30:57 -07:00
parent 80ed1ef8ab
commit cd69343d67
4 changed files with 28 additions and 3 deletions

View File

@ -1073,6 +1073,7 @@ extern "C" {
pub fn X509_REQ_add_extensions(req: *mut X509_REQ, exts: *mut stack_st_X509_EXTENSION) -> c_int; pub fn X509_REQ_add_extensions(req: *mut X509_REQ, exts: *mut stack_st_X509_EXTENSION) -> c_int;
pub fn X509_REQ_sign(x: *mut X509_REQ, pkey: *mut EVP_PKEY, md: *const EVP_MD) -> c_int; pub fn X509_REQ_sign(x: *mut X509_REQ, pkey: *mut EVP_PKEY, md: *const EVP_MD) -> c_int;
pub fn d2i_X509(a: *mut *mut X509, pp: *mut *mut c_uchar, length: c_long) -> *mut X509;
pub fn i2d_X509_bio(b: *mut BIO, x: *mut X509) -> c_int; pub fn i2d_X509_bio(b: *mut BIO, x: *mut X509) -> c_int;
pub fn i2d_X509_REQ_bio(b: *mut BIO, x: *mut X509_REQ) -> c_int; pub fn i2d_X509_REQ_bio(b: *mut BIO, x: *mut X509_REQ) -> c_int;

View File

@ -535,9 +535,14 @@ impl<'a> SslContextRef<'a> {
/// Adds a certificate to the certificate chain presented together with the /// Adds a certificate to the certificate chain presented together with the
/// certificate specified using set_certificate() /// certificate specified using set_certificate()
pub fn add_extra_chain_cert(&mut self, cert: &X509Ref) -> Result<(), ErrorStack> { pub fn add_extra_chain_cert(&mut self, cert: &X509Ref) -> Result<(), ErrorStack> {
wrap_ssl_result(unsafe { // FIXME this should really just take an X509 by value
ffi::SSL_CTX_add_extra_chain_cert(self.as_ptr(), cert.as_ptr()) as c_int let der = try!(cert.to_der());
}) let cert = try!(X509::from_der(&der));
unsafe {
try_ssl!(ffi::SSL_CTX_add_extra_chain_cert(self.as_ptr(), cert.as_ptr()));
}
mem::forget(cert);
Ok(())
} }
/// Specifies the file that contains private key /// Specifies the file that contains private key

View File

@ -1081,3 +1081,11 @@ fn default_verify_paths() {
assert!(result.starts_with(b"HTTP/1.0")); assert!(result.starts_with(b"HTTP/1.0"));
assert!(result.ends_with(b"</HTML>\r\n") || result.ends_with(b"</html>")); assert!(result.ends_with(b"</HTML>\r\n") || result.ends_with(b"</html>"));
} }
#[test]
fn add_extra_chain_cert() {
let cert = include_bytes!("../../../test/cert.pem");
let cert = X509::from_pem(cert).unwrap();
let mut ctx = SslContext::new(SslMethod::Sslv23).unwrap();
ctx.add_extra_chain_cert(&cert).unwrap();
}

View File

@ -1,4 +1,5 @@
use libc::{c_char, c_int, c_long, c_ulong, c_void}; use libc::{c_char, c_int, c_long, c_ulong, c_void};
use std::cmp;
use std::ffi::CString; use std::ffi::CString;
use std::mem; use std::mem;
use std::ptr; use std::ptr;
@ -492,6 +493,16 @@ impl X509 {
X509::from_ptr(x509) X509::from_ptr(x509)
} }
/// Reads a certificate from DER.
pub fn from_der(buf: &[u8]) -> Result<X509, ErrorStack> {
unsafe {
let mut ptr = buf.as_ptr() as *mut _;
let len = cmp::min(buf.len(), c_long::max_value() as usize) as c_long;
let x509 = try_ssl_null!(ffi::d2i_X509(ptr::null_mut(), &mut ptr, len));
Ok(X509::from_ptr(x509))
}
}
/// Reads a certificate from PEM. /// Reads a certificate from PEM.
pub fn from_pem(buf: &[u8]) -> Result<X509, ErrorStack> { pub fn from_pem(buf: &[u8]) -> Result<X509, ErrorStack> {
let mem_bio = try!(MemBioSlice::new(buf)); let mem_bio = try!(MemBioSlice::new(buf));