From 98bb376969953fc50b875aa3bb2815dd085463b5 Mon Sep 17 00:00:00 2001 From: Thijs Cadier Date: Thu, 3 Sep 2015 14:09:24 +0200 Subject: [PATCH] Tests and a fix for ssl connections Setting ssl options was broken because the memory for its content was released immediately. We now keep ownership of everything until the pool is dropped. --- .gitignore | 1 + src/client.rs | 104 +++++++++++++++++++++++++++++------------------- tests/client.rs | 42 ++++++++++++++++++- 3 files changed, 106 insertions(+), 41 deletions(-) diff --git a/.gitignore b/.gitignore index f60cb7b..09adaba 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ target Cargo.lock mongoc-sys/mongo-c-driver* mongoc-sys/src/bindings.rs +ssl_env_vars diff --git a/src/client.rs b/src/client.rs index 92c572e..8c96bbc 100644 --- a/src/client.rs +++ b/src/client.rs @@ -22,7 +22,10 @@ use super::read_prefs::ReadPrefs; // with this root bool, there must be a better way. pub struct ClientPool { root_instance: bool, - uri: Uri, + // Uri and SslOptions need to be present for the + // lifetime of this pool. + uri: Uri, + ssl_options: Option, inner: *mut bindings::mongoc_client_pool_t } @@ -39,7 +42,7 @@ impl ClientPool { pool_ptr }; match ssl_options { - Some(options) => { + Some(ref options) => { unsafe { bindings::mongoc_client_pool_set_ssl_opts( pool, @@ -51,8 +54,9 @@ impl ClientPool { }; ClientPool { root_instance: true, - uri: uri, // Become owner of uri so it doesn't go out of scope - inner: pool + uri: uri, + ssl_options: ssl_options, + inner: pool } } @@ -93,6 +97,7 @@ impl Clone for ClientPool { ClientPool { root_instance: false, uri: self.uri.clone(), + ssl_options: self.ssl_options.clone(), inner: self.inner.clone() } } @@ -111,11 +116,18 @@ impl Drop for ClientPool { pub struct SslOptions { inner: bindings::mongoc_ssl_opt_t, - pem_file: Option, - pem_password: Option, - ca_file: Option, - ca_dir: Option, - crl_file: Option, + // We need to store everything so both memory sticks around + // for the C driver and we can clone this struct. + pem_file: Option, + _pem_file_cstring: Option, + pem_password: Option, + _pem_password_cstring: Option, + ca_file: Option, + _ca_file_cstring: Option, + ca_dir: Option, + _ca_dir_cstring: Option, + crl_file: Option, + _crl_file_cstring: Option, weak_cert_validation: bool } @@ -128,37 +140,31 @@ impl SslOptions { crl_file: Option, weak_cert_validation: bool ) -> io::Result { + let pem_file_cstring = try!(Self::cstring_from_path(&pem_file)); + let pem_password_cstring = Self::cstring_from_string(&pem_password); + let ca_file_cstring = try!(Self::cstring_from_path(&ca_file)); + let ca_dir_cstring = try!(Self::cstring_from_path(&ca_dir)); + let crl_file_cstring = try!(Self::cstring_from_path(&crl_file)); + let ssl_options = bindings::mongoc_ssl_opt_t { - pem_file: match pem_file { - Some(ref f) => { - try!(File::open(f.as_path())); - Self::path_ptr(f) - }, + pem_file: match pem_file_cstring { + Some(ref f) => f.as_ptr(), None => ptr::null() }, - pem_pwd: match pem_password { - Some(ref password) => CString::new(password.clone()).unwrap().as_ptr(), + pem_pwd: match pem_password_cstring { + Some(ref password) => password.as_ptr(), None => ptr::null() }, - ca_file: match ca_file { - Some(ref f) => { - try!(File::open(f.as_path())); - Self::path_ptr(f) - }, + ca_file: match ca_file_cstring { + Some(ref f) => f.as_ptr(), None => ptr::null() }, - ca_dir: match ca_dir { - Some(ref f) => { - try!(File::open(f.as_path())); - Self::path_ptr(f) - }, + ca_dir: match ca_dir_cstring { + Some(ref f) => f.as_ptr(), None => ptr::null() }, - crl_file: match crl_file { - Some(ref f) => { - try!(File::open(f.as_path())); - Self::path_ptr(f) - }, + crl_file: match crl_file_cstring { + Some(ref f) => f.as_ptr(), None => ptr::null() }, weak_cert_validation: weak_cert_validation as u8, @@ -166,18 +172,36 @@ impl SslOptions { }; Ok(SslOptions { - inner: ssl_options, - pem_file: pem_file, - pem_password: pem_password, - ca_file: ca_file, - ca_dir: ca_dir, - crl_file: crl_file, - weak_cert_validation: weak_cert_validation + inner: ssl_options, + pem_file: pem_file, + _pem_file_cstring: pem_file_cstring, + pem_password: pem_password, + _pem_password_cstring: pem_password_cstring, + ca_file: ca_file, + _ca_file_cstring: ca_file_cstring, + ca_dir: ca_dir, + _ca_dir_cstring: ca_dir_cstring, + crl_file: crl_file, + _crl_file_cstring: crl_file_cstring, + weak_cert_validation: weak_cert_validation }) } - fn path_ptr(path: &PathBuf) -> *const i8 { - path.as_os_str().to_cstring().unwrap().as_ptr() + fn cstring_from_path(path: &Option) -> io::Result> { + match path { + &Some(ref p) => { + try!(File::open(p.as_path())); + Ok(Some(p.as_os_str().to_cstring().unwrap())) + }, + &None => Ok(None) + } + } + + fn cstring_from_string(path: &Option) -> Option { + match path { + &Some(ref p) => Some(CString::new(p.clone()).unwrap()), + &None => None + } } fn inner(&self) -> *const bindings::mongoc_ssl_opt_t { diff --git a/tests/client.rs b/tests/client.rs index 2f1c860..2dc19ab 100644 --- a/tests/client.rs +++ b/tests/client.rs @@ -1,3 +1,4 @@ +use std::env; use std::path::PathBuf; use std::thread; @@ -55,7 +56,6 @@ fn test_get_server_status() { #[test] fn test_new_pool_with_ssl_options() { - // We currently have no way to test full operations let uri = Uri::new("mongodb://localhost:27017/").unwrap(); let ssl_options = SslOptions::new( Some(PathBuf::from("./README.md")), @@ -80,3 +80,43 @@ fn test_ssl_options_nonexistent_file() { false ).is_err()); } + +#[test] +fn test_ssl_connection_success() { + // This is currently tested on a private replica set, will be skipped if env vars are not set. + + let uri = Uri::new(match env::var("MONGO_RUST_DRIVER_SSL_URI") { Ok(v) => v, Err(_) => return }).unwrap(); + let pem_file = PathBuf::from(match env::var("MONGO_RUST_DRIVER_SSL_PEM_FILE") { Ok(v) => v, Err(_) => return }); + let ca_file = PathBuf::from(match env::var("MONGO_RUST_DRIVER_SSL_CA_FILE") { Ok(v) => v, Err(_) => return }); + + let ssl_options = SslOptions::new( + Some(pem_file), + None, + Some(ca_file), + None, + None, + false + ).unwrap(); + + let pool = ClientPool::new(uri, Some(ssl_options)); + let client = pool.pop(); + let database = client.get_database("admin"); + + let result = database.command_simple(doc! { "ping" => 1 }, None).unwrap(); + assert!(result.contains_key("ok")); +} + +#[test] +fn test_ssl_connection_failure() { + // This connection should fail since the private replica set uses certificate + // based auth that we're not setting. Test will be skipped if env var is not set. + + let uri = Uri::new(match env::var("MONGO_RUST_DRIVER_SSL_URI") { Ok(v) => v, Err(_) => return }).unwrap(); + + let pool = ClientPool::new(uri, None); + let client = pool.pop(); + let database = client.get_database("admin"); + + let result = database.command_simple(doc! { "ping" => 1 }, None); + assert!(result.is_err()); +}