From 7ec015325b0d900ddaf375b62f5a52d4231dc9a2 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Sun, 16 Oct 2016 21:07:17 -0700 Subject: [PATCH] Finish error overhaul --- openssl/src/bio.rs | 5 +- openssl/src/bn.rs | 35 ++++++----- openssl/src/dh.rs | 37 +++++++----- openssl/src/macros.rs | 81 ------------------------- openssl/src/x509/mod.rs | 119 +++++++++++++++++-------------------- openssl/src/x509/verify.rs | 9 ++- 6 files changed, 101 insertions(+), 185 deletions(-) diff --git a/openssl/src/bio.rs b/openssl/src/bio.rs index 22d2cee3..199fc0c8 100644 --- a/openssl/src/bio.rs +++ b/openssl/src/bio.rs @@ -4,6 +4,7 @@ use std::slice; use libc::c_int; use ffi; +use cvt_p; use error::ErrorStack; pub struct MemBioSlice<'a>(*mut ffi::BIO, PhantomData<&'a [u8]>); @@ -22,7 +23,7 @@ impl<'a> MemBioSlice<'a> { assert!(buf.len() <= c_int::max_value() as usize); let bio = unsafe { - try_ssl_null!(BIO_new_mem_buf(buf.as_ptr() as *const _, buf.len() as c_int)) + try!(cvt_p(BIO_new_mem_buf(buf.as_ptr() as *const _, buf.len() as c_int))) }; Ok(MemBioSlice(bio, PhantomData)) @@ -48,7 +49,7 @@ impl MemBio { ffi::init(); let bio = unsafe { - try_ssl_null!(ffi::BIO_new(ffi::BIO_s_mem())) + try!(cvt_p(ffi::BIO_new(ffi::BIO_s_mem()))) }; Ok(MemBio(bio)) } diff --git a/openssl/src/bn.rs b/openssl/src/bn.rs index 22924e67..62b30d96 100644 --- a/openssl/src/bn.rs +++ b/openssl/src/bn.rs @@ -408,7 +408,7 @@ impl<'a> BigNumRef<'a> { pub fn to_owned(&self) -> Result { unsafe { - let r = try_ssl_null!(ffi::BN_dup(self.as_ptr())); + let r = try!(cvt_p(ffi::BN_dup(self.as_ptr()))); Ok(BigNum::from_ptr(r)) } } @@ -553,7 +553,7 @@ impl BigNum { pub fn new() -> Result { unsafe { ffi::init(); - let v = try_ssl_null!(ffi::BN_new()); + let v = try!(cvt_p(ffi::BN_new())); Ok(BigNum::from_ptr(v)) } } @@ -561,27 +561,28 @@ impl BigNum { /// Creates a new `BigNum` with the given value. pub fn new_from(n: u32) -> Result { BigNum::new().and_then(|v| unsafe { - try_ssl!(ffi::BN_set_word(v.as_ptr(), n as ffi::BN_ULONG)); - Ok(v) + cvt(ffi::BN_set_word(v.as_ptr(), n as ffi::BN_ULONG)).map(|_| v) }) } /// Creates a `BigNum` from a decimal string. pub fn from_dec_str(s: &str) -> Result { - BigNum::new().and_then(|mut v| unsafe { + unsafe { let c_str = CString::new(s.as_bytes()).unwrap(); - try_ssl!(ffi::BN_dec2bn(&mut (v.0).0, c_str.as_ptr() as *const _)); - Ok(v) - }) + let mut bn = ptr::null_mut(); + try!(cvt(ffi::BN_dec2bn(&mut bn, c_str.as_ptr() as *const _))); + Ok(BigNum::from_ptr(bn)) + } } /// Creates a `BigNum` from a hexadecimal string. pub fn from_hex_str(s: &str) -> Result { - BigNum::new().and_then(|mut v| unsafe { + unsafe { let c_str = CString::new(s.as_bytes()).unwrap(); - try_ssl!(ffi::BN_hex2bn(&mut (v.0).0, c_str.as_ptr() as *const _)); - Ok(v) - }) + let mut bn = ptr::null_mut(); + try!(cvt(ffi::BN_hex2bn(&mut bn, c_str.as_ptr() as *const _))); + Ok(BigNum::from_ptr(bn)) + } } pub unsafe fn from_ptr(handle: *mut ffi::BIGNUM) -> BigNum { @@ -597,11 +598,13 @@ impl BigNum { /// assert_eq!(bignum, BigNum::new_from(0x120034).unwrap()); /// ``` pub fn new_from_slice(n: &[u8]) -> Result { - BigNum::new().and_then(|v| unsafe { - try_ssl_null!(ffi::BN_bin2bn(n.as_ptr(), n.len() as c_int, v.as_ptr())); - Ok(v) - }) + unsafe { + assert!(n.len() <= c_int::max_value() as usize); + cvt_p(ffi::BN_bin2bn(n.as_ptr(), n.len() as c_int, ptr::null_mut())) + .map(|p| BigNum::from_ptr(p)) + } } + /// Generates a prime number. /// /// # Parameters diff --git a/openssl/src/dh.rs b/openssl/src/dh.rs index 6d0800a1..fec6bd98 100644 --- a/openssl/src/dh.rs +++ b/openssl/src/dh.rs @@ -3,6 +3,7 @@ use error::ErrorStack; use bio::MemBioSlice; use std::ptr; +use {cvt, cvt_p}; use bn::BigNum; use std::mem; @@ -11,11 +12,11 @@ pub struct DH(*mut ffi::DH); impl DH { pub fn from_params(p: BigNum, g: BigNum, q: BigNum) -> Result { unsafe { - let dh = DH(try_ssl_null!(ffi::DH_new())); - try_ssl!(compat::DH_set0_pqg(dh.0, + let dh = DH(try!(cvt_p(ffi::DH_new()))); + try!(cvt(compat::DH_set0_pqg(dh.0, p.as_ptr(), q.as_ptr(), - g.as_ptr())); + g.as_ptr()))); mem::forget((p, g, q)); Ok(dh) } @@ -23,34 +24,38 @@ impl DH { pub fn from_pem(buf: &[u8]) -> Result { let mem_bio = try!(MemBioSlice::new(buf)); - let dh = unsafe { - ffi::PEM_read_bio_DHparams(mem_bio.as_ptr(), ptr::null_mut(), None, ptr::null_mut()) - }; - try_ssl_null!(dh); - Ok(DH(dh)) + unsafe { + cvt_p(ffi::PEM_read_bio_DHparams(mem_bio.as_ptr(), + ptr::null_mut(), + None, + ptr::null_mut())) + .map(DH) + } } #[cfg(feature = "openssl-102")] pub fn get_1024_160() -> Result { - let dh = try_ssl_null!(unsafe { ffi::DH_get_1024_160() }); - Ok(DH(dh)) + unsafe { + cvt_p(ffi::DH_get_1024_160()).map(DH) + } } #[cfg(feature = "openssl-102")] pub fn get_2048_224() -> Result { - let dh = try_ssl_null!(unsafe { ffi::DH_get_2048_224() }); - Ok(DH(dh)) + unsafe { + cvt_p(ffi::DH_get_2048_224()).map(DH) + } } #[cfg(feature = "openssl-102")] pub fn get_2048_256() -> Result { - let dh = try_ssl_null!(unsafe { ffi::DH_get_2048_256() }); - Ok(DH(dh)) + unsafe { + cvt_p(ffi::DH_get_2048_256()).map(DH) + } } pub unsafe fn as_ptr(&self) -> *mut ffi::DH { - let DH(n) = *self; - n + self.0 } } diff --git a/openssl/src/macros.rs b/openssl/src/macros.rs index 31c298fa..85175445 100644 --- a/openssl/src/macros.rs +++ b/openssl/src/macros.rs @@ -1,86 +1,5 @@ #![macro_use] -macro_rules! try_ssl_stream { - ($e:expr) => ( - match $e { - Ok(ok) => ok, - Err(err) => return Err(StreamError(err)) - } - ) -} - -/// Shortcut return with SSL error if something went wrong -macro_rules! try_ssl_if { - ($e:expr) => ( - if $e { - return Err(::error::ErrorStack::get().into()) - } - ) -} - -/// Shortcut return with SSL error if last error result is 0 -/// (default) -macro_rules! try_ssl{ - ($e:expr) => (try_ssl_if!($e == 0)) -} - -/// Shortcut return with SSL if got a null result -macro_rules! try_ssl_null{ - ($e:expr) => ({ - let t = $e; - try_ssl_if!(t == ptr::null_mut()); - t - }) -} - -/// Shortcut return with SSL error if last error result is -1 -/// (default for size) -macro_rules! try_ssl_returns_size{ - ($e:expr) => ( - if $e == -1 { - return Err(::error::ErrorStack::get().into()) - } else { - $e - } - ) -} - -/// Lifts current SSL error code into Result<(), Error> -/// if expression is true -/// Lifting is actually a shortcut of the following form: -/// -/// ```ignore -/// let _ = try!(something) -/// Ok(()) -/// ``` -macro_rules! lift_ssl_if{ - ($e:expr) => ( { - if $e { - Err(::error::ErrorStack::get().into()) - } else { - Ok(()) - } - }) -} - -/// Lifts current SSL error code into Result<(), Error> -/// if SSL returned 0 (default error indication) -macro_rules! lift_ssl { - ($e:expr) => (lift_ssl_if!($e == 0)) -} - -/// Lifts current SSL error code into Result<(), Error> -/// if SSL returned -1 (default size error indication) -macro_rules! lift_ssl_returns_size { - ($e:expr) => ( { - if $e == -1 { - Err(::error::ErrorStack::get().into()) - } else { - Ok($e) - } - }) -} - #[cfg(ossl10x)] macro_rules! CRYPTO_free { ($e:expr) => (::ffi::CRYPTO_free($e)) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index c429b486..1cd7471f 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -10,6 +10,7 @@ use std::slice; use std::collections::HashMap; use std::marker::PhantomData; +use {cvt, cvt_p}; use asn1::Asn1Time; use asn1::Asn1TimeRef; use bio::{MemBio, MemBioSlice}; @@ -251,25 +252,25 @@ impl X509Generator { let value = CString::new(value.as_bytes()).unwrap(); let ext = match exttype.get_nid() { Some(nid) => { - ffi::X509V3_EXT_conf_nid(ptr::null_mut(), - mem::transmute(&ctx), - nid as c_int, - value.as_ptr() as *mut c_char) + try!(cvt_p(ffi::X509V3_EXT_conf_nid(ptr::null_mut(), + mem::transmute(&ctx), + nid as c_int, + value.as_ptr() as *mut c_char))) } None => { let name = CString::new(exttype.get_name().unwrap().as_bytes()).unwrap(); - ffi::X509V3_EXT_conf(ptr::null_mut(), - mem::transmute(&ctx), - name.as_ptr() as *mut c_char, - value.as_ptr() as *mut c_char) + try!(cvt_p(ffi::X509V3_EXT_conf(ptr::null_mut(), + mem::transmute(&ctx), + name.as_ptr() as *mut c_char, + value.as_ptr() as *mut c_char))) } }; - let mut success = false; - if ext != ptr::null_mut() { - success = ffi::X509_add_ext(x509, ext, -1) != 0; + if ffi::X509_add_ext(x509, ext, -1) != 1 { ffi::X509_EXTENSION_free(ext); + Err(ErrorStack::get()) + } else { + Ok(()) } - lift_ssl_if!(!success) } } @@ -278,17 +279,18 @@ impl X509Generator { value: &str) -> Result<(), ErrorStack> { let value_len = value.len() as c_int; - lift_ssl!(unsafe { + unsafe { let key = CString::new(key.as_bytes()).unwrap(); let value = CString::new(value.as_bytes()).unwrap(); - ffi::X509_NAME_add_entry_by_txt(name, - key.as_ptr() as *const _, - ffi::MBSTRING_UTF8, - value.as_ptr() as *const _, - value_len, - -1, - 0) - }) + cvt(ffi::X509_NAME_add_entry_by_txt(name, + key.as_ptr() as *const _, + ffi::MBSTRING_UTF8, + value.as_ptr() as *const _, + value_len, + -1, + 0)) + .map(|_| ()) + } } fn random_serial() -> Result { @@ -308,32 +310,30 @@ impl X509Generator { } /// Sets the certificate public-key, then self-sign and return it - /// Note: That the bit-length of the private key is used (set_bitlength is ignored) pub fn sign(&self, p_key: &PKey) -> Result { ffi::init(); unsafe { - let x509 = try_ssl_null!(ffi::X509_new()); - let x509 = X509::from_ptr(x509); + let x509 = X509::from_ptr(try!(cvt_p(ffi::X509_new()))); - try_ssl!(ffi::X509_set_version(x509.as_ptr(), 2)); - try_ssl!(ffi::ASN1_INTEGER_set(ffi::X509_get_serialNumber(x509.as_ptr()), - try!(X509Generator::random_serial()))); + try!(cvt(ffi::X509_set_version(x509.as_ptr(), 2))); + try!(cvt(ffi::ASN1_INTEGER_set(ffi::X509_get_serialNumber(x509.as_ptr()), + try!(X509Generator::random_serial())))); let not_before = try!(Asn1Time::days_from_now(0)); let not_after = try!(Asn1Time::days_from_now(self.days)); - try_ssl!(X509_set_notBefore(x509.as_ptr(), not_before.as_ptr() as *const _)); + try!(cvt(X509_set_notBefore(x509.as_ptr(), not_before.as_ptr() as *const _))); // If prev line succeded - ownership should go to cert mem::forget(not_before); - try_ssl!(X509_set_notAfter(x509.as_ptr(), not_after.as_ptr() as *const _)); + try!(cvt(X509_set_notAfter(x509.as_ptr(), not_after.as_ptr() as *const _))); // If prev line succeded - ownership should go to cert mem::forget(not_after); - try_ssl!(ffi::X509_set_pubkey(x509.as_ptr(), p_key.as_ptr())); + try!(cvt(ffi::X509_set_pubkey(x509.as_ptr(), p_key.as_ptr()))); - let name = try_ssl_null!(ffi::X509_get_subject_name(x509.as_ptr())); + let name = try!(cvt_p(ffi::X509_get_subject_name(x509.as_ptr()))); let default = [("CN", "rust-openssl")]; let default_iter = &mut default.iter().map(|&(k, v)| (k, v)); @@ -347,7 +347,7 @@ impl X509Generator { for (key, val) in iter { try!(X509Generator::add_name_internal(name, &key, &val)); } - try_ssl!(ffi::X509_set_issuer_name(x509.as_ptr(), name)); + try!(cvt(ffi::X509_set_issuer_name(x509.as_ptr(), name))); for (exttype, ext) in self.extensions.iter() { try!(X509Generator::add_extension_internal(x509.as_ptr(), @@ -356,7 +356,7 @@ impl X509Generator { } let hash_fn = self.hash_type.as_ptr(); - try_ssl!(ffi::X509_sign(x509.as_ptr(), p_key.as_ptr(), hash_fn)); + try!(cvt(ffi::X509_sign(x509.as_ptr(), p_key.as_ptr(), hash_fn))); Ok(x509) } } @@ -369,18 +369,20 @@ impl X509Generator { }; unsafe { - let req = ffi::X509_to_X509_REQ(cert.as_ptr(), ptr::null_mut(), ptr::null()); - try_ssl_null!(req); + let req = try!(cvt_p(ffi::X509_to_X509_REQ(cert.as_ptr(), + ptr::null_mut(), + ptr::null()))); + let req = X509Req::from_ptr(req); let exts = compat::X509_get0_extensions(cert.as_ptr()); if exts != ptr::null_mut() { - try_ssl!(ffi::X509_REQ_add_extensions(req, exts as *mut _)); + try!(cvt(ffi::X509_REQ_add_extensions(req.as_ptr(), exts as *mut _))); } let hash_fn = self.hash_type.as_ptr(); - try_ssl!(ffi::X509_REQ_sign(req, p_key.as_ptr(), hash_fn)); + try!(cvt(ffi::X509_REQ_sign(req.as_ptr(), p_key.as_ptr(), hash_fn))); - Ok(X509Req::new(req)) + Ok(req) } } } @@ -394,12 +396,6 @@ impl<'a> X509Ref<'a> { X509Ref(x509, PhantomData) } - /// - #[deprecated(note = "renamed to `X509::from_ptr`", since = "0.8.1")] - pub unsafe fn new(x509: *mut ffi::X509) -> X509Ref<'a> { - X509Ref::from_ptr(x509) - } - pub fn as_ptr(&self) -> *mut ffi::X509 { self.0 } @@ -429,7 +425,7 @@ impl<'a> X509Ref<'a> { pub fn public_key(&self) -> Result { unsafe { - let pkey = try_ssl_null!(ffi::X509_get_pubkey(self.0)); + let pkey = try!(cvt_p(ffi::X509_get_pubkey(self.0))); Ok(PKey::from_ptr(pkey)) } } @@ -440,7 +436,7 @@ impl<'a> X509Ref<'a> { let evp = hash_type.as_ptr(); let mut len = ffi::EVP_MAX_MD_SIZE; let mut buf = vec![0u8; len as usize]; - try_ssl!(ffi::X509_digest(self.0, evp, buf.as_mut_ptr() as *mut _, &mut len)); + try!(cvt(ffi::X509_digest(self.0, evp, buf.as_mut_ptr() as *mut _, &mut len))); buf.truncate(len as usize); Ok(buf) } @@ -468,7 +464,7 @@ impl<'a> X509Ref<'a> { pub fn to_pem(&self) -> Result, ErrorStack> { let mem_bio = try!(MemBio::new()); unsafe { - try_ssl!(ffi::PEM_write_bio_X509(mem_bio.as_ptr(), self.0)); + try!(cvt(ffi::PEM_write_bio_X509(mem_bio.as_ptr(), self.0))); } Ok(mem_bio.get_buf().to_owned()) } @@ -492,18 +488,12 @@ impl X509 { X509(X509Ref::from_ptr(x509)) } - /// - #[deprecated(note = "renamed to `X509::from_ptr`", since = "0.8.1")] - pub unsafe fn new(x509: *mut ffi::X509) -> X509 { - X509::from_ptr(x509) - } - /// Reads a certificate from DER. pub fn from_der(buf: &[u8]) -> Result { unsafe { let mut ptr = buf.as_ptr(); 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)); + let x509 = try!(cvt_p(ffi::d2i_X509(ptr::null_mut(), &mut ptr, len))); Ok(X509::from_ptr(x509)) } } @@ -512,10 +502,10 @@ impl X509 { pub fn from_pem(buf: &[u8]) -> Result { let mem_bio = try!(MemBioSlice::new(buf)); unsafe { - let handle = try_ssl_null!(ffi::PEM_read_bio_X509(mem_bio.as_ptr(), - ptr::null_mut(), - None, - ptr::null_mut())); + let handle = try!(cvt_p(ffi::PEM_read_bio_X509(mem_bio.as_ptr(), + ptr::null_mut(), + None, + ptr::null_mut()))); Ok(X509::from_ptr(handle)) } } @@ -582,8 +572,7 @@ impl<'x> X509Name<'x> { pub struct X509Req(*mut ffi::X509_REQ); impl X509Req { - /// Creates new from handle - pub unsafe fn new(handle: *mut ffi::X509_REQ) -> X509Req { + pub unsafe fn from_ptr(handle: *mut ffi::X509_REQ) -> X509Req { X509Req(handle) } @@ -595,11 +584,11 @@ impl X509Req { pub fn from_pem(buf: &[u8]) -> Result { let mem_bio = try!(MemBioSlice::new(buf)); unsafe { - let handle = try_ssl_null!(ffi::PEM_read_bio_X509_REQ(mem_bio.as_ptr(), - ptr::null_mut(), - None, - ptr::null_mut())); - Ok(X509Req::new(handle)) + let handle = try!(cvt_p(ffi::PEM_read_bio_X509_REQ(mem_bio.as_ptr(), + ptr::null_mut(), + None, + ptr::null_mut()))); + Ok(X509Req::from_ptr(handle)) } } diff --git a/openssl/src/x509/verify.rs b/openssl/src/x509/verify.rs index 0fc1df3a..87287875 100644 --- a/openssl/src/x509/verify.rs +++ b/openssl/src/x509/verify.rs @@ -32,11 +32,10 @@ impl<'a> X509VerifyParamRef<'a> { pub fn set_host(&mut self, host: &str) -> Result<(), ErrorStack> { unsafe { - try_ssl!(ffi::X509_VERIFY_PARAM_set1_host(self.0, - host.as_ptr() as *const _, - host.len())) + cvt(ffi::X509_VERIFY_PARAM_set1_host(self.0, + host.as_ptr() as *const _, + host.len())) + .map(|_| ()) } - - Ok(()) } }