FIXMEs

README.md

@@ -21,6 +21,7 @@ and [hyper](https://github.com/hyperium/hyper) built on top of it.
  * `Ssl::new_from_ref` -> `Ssl::new()`.
  * `X509Builder::append_extension2` -> `X509Builder::append_extension`.
  * `X509Store` is now cheaply cloneable, but immutable. `SslContextBuilder.cert_store_mut()` can't be used after `.set_cert_store()`. Use `.set_cert_store_builder()` if you need `.cert_store_mut()`.
+ * `X509StoreBuilder::add_cert` takes a reference.
  * `hyper` 0.x support has been removed. Use `hyper` 1.x.
 
 ## Contribution

boring/src/ec.rs

@@ -267,8 +267,7 @@ impl EcPointRef {
         group: &EcGroupRef,
         q: &EcPointRef,
         m: &BigNumRef,
-        // FIXME should be &mut
-        ctx: &BigNumContextRef,
+        ctx: &mut BigNumContextRef,
     ) -> Result<(), ErrorStack> {
         unsafe {
             cvt(ffi::EC_POINT_mul(
@@ -287,8 +286,7 @@ impl EcPointRef {
         &mut self,
         group: &EcGroupRef,
         n: &BigNumRef,
-        // FIXME should be &mut
-        ctx: &BigNumContextRef,
+        ctx: &mut BigNumContextRef,
     ) -> Result<(), ErrorStack> {
         unsafe {
             cvt(ffi::EC_POINT_mul(
@@ -838,7 +836,7 @@ mod test {
         let mut ctx = BigNumContext::new().unwrap();
         let mut public_key = EcPoint::new(&group).unwrap();
         public_key
-            .mul_generator(&group, key.private_key(), &ctx)
+            .mul_generator(&group, key.private_key(), &mut ctx)
             .unwrap();
         assert!(public_key.eq(&group, key.public_key(), &mut ctx).unwrap());
     }
@@ -850,7 +848,7 @@ mod test {
         let one = BigNum::from_u32(1).unwrap();
         let mut ctx = BigNumContext::new().unwrap();
         let mut ecp = EcPoint::new(&group).unwrap();
-        ecp.mul_generator(&group, &one, &ctx).unwrap();
+        ecp.mul_generator(&group, &one, &mut ctx).unwrap();
         assert!(ecp.eq(&group, gen, &mut ctx).unwrap());
     }
 

boring/src/rsa.rs

@@ -416,7 +416,7 @@ impl Rsa<Public> {
     pub fn from_public_components(n: BigNum, e: BigNum) -> Result<Rsa<Public>, ErrorStack> {
         unsafe {
             let rsa = cvt_p(ffi::RSA_new())?;
-            RSA_set0_key(rsa, n.as_ptr(), e.as_ptr(), ptr::null_mut());
+            cvt(RSA_set0_key(rsa, n.as_ptr(), e.as_ptr(), ptr::null_mut()))?;
             mem::forget((n, e));
             Ok(Rsa::from_ptr(rsa))
         }
@@ -474,7 +474,7 @@ impl RsaPrivateKeyBuilder {
     pub fn new(n: BigNum, e: BigNum, d: BigNum) -> Result<RsaPrivateKeyBuilder, ErrorStack> {
         unsafe {
             let rsa = cvt_p(ffi::RSA_new())?;
-            RSA_set0_key(rsa, n.as_ptr(), e.as_ptr(), d.as_ptr());
+            cvt(RSA_set0_key(rsa, n.as_ptr(), e.as_ptr(), d.as_ptr()))?;
             mem::forget((n, e, d));
             Ok(RsaPrivateKeyBuilder {
                 rsa: Rsa::from_ptr(rsa),
@@ -485,12 +485,10 @@ impl RsaPrivateKeyBuilder {
     /// Sets the factors of the Rsa key.
     ///
     /// `p` and `q` are the first and second factors of `n`.
-    ///
-    // FIXME should be infallible
     #[corresponds(RSA_set0_factors)]
     pub fn set_factors(self, p: BigNum, q: BigNum) -> Result<RsaPrivateKeyBuilder, ErrorStack> {
         unsafe {
-            RSA_set0_factors(self.rsa.as_ptr(), p.as_ptr(), q.as_ptr());
+            cvt(RSA_set0_factors(self.rsa.as_ptr(), p.as_ptr(), q.as_ptr()))?;
             mem::forget((p, q));
         }
         Ok(self)
@@ -500,8 +498,6 @@ impl RsaPrivateKeyBuilder {
     ///
     /// `dmp1`, `dmq1`, and `iqmp` are the exponents and coefficient for
     /// CRT calculations which is used to speed up RSA operations.
-    ///
-    // FIXME should be infallible
     #[corresponds(RSA_set0_crt_params)]
     pub fn set_crt_params(
         self,
@@ -510,12 +506,12 @@ impl RsaPrivateKeyBuilder {
         iqmp: BigNum,
     ) -> Result<RsaPrivateKeyBuilder, ErrorStack> {
         unsafe {
-            RSA_set0_crt_params(
+            cvt(RSA_set0_crt_params(
                 self.rsa.as_ptr(),
                 dmp1.as_ptr(),
                 dmq1.as_ptr(),
                 iqmp.as_ptr(),
-            );
+            ))?;
             mem::forget((dmp1, dmq1, iqmp));
         }
         Ok(self)

boring/src/ssl/test/cert_compressor.rs

@@ -54,7 +54,7 @@ fn server_only_cert_compression() {
 
     let mut store = X509StoreBuilder::new().unwrap();
     let x509 = X509::from_pem(super::ROOT_CERT).unwrap();
-    store.add_cert(x509).unwrap();
+    store.add_cert(&x509).unwrap();
 
     let client = server.client();
 
@@ -67,7 +67,7 @@ fn client_only_cert_compression() {
 
     let mut store = X509StoreBuilder::new().unwrap();
     let x509 = X509::from_pem(super::ROOT_CERT).unwrap();
-    store.add_cert(x509).unwrap();
+    store.add_cert(&x509).unwrap();
 
     let mut client = server_builder.client();
     client
@@ -90,7 +90,7 @@ fn client_and_server_cert_compression() {
 
     let mut store = X509StoreBuilder::new().unwrap();
     let x509 = X509::from_pem(super::ROOT_CERT).unwrap();
-    store.add_cert(x509).unwrap();
+    store.add_cert(&x509).unwrap();
 
     let mut client = server.client();
     client

boring/src/ssl/test/mod.rs

@@ -317,18 +317,18 @@ fn test_mutable_store() {
     let cert2 = X509::from_pem(cert2).unwrap();
 
     let mut ctx = SslContext::builder(SslMethod::tls()).unwrap();
-    ctx.cert_store_mut().add_cert(cert.clone()).unwrap();
+    ctx.cert_store_mut().add_cert(&cert.clone()).unwrap();
     assert_eq!(1, ctx.cert_store().objects_len());
 
     ctx.set_cert_store_builder(X509StoreBuilder::new().unwrap());
     assert_eq!(0, ctx.cert_store().objects_len());
 
-    ctx.cert_store_mut().add_cert(cert.clone()).unwrap();
+    ctx.cert_store_mut().add_cert(&cert.clone()).unwrap();
     assert_eq!(1, ctx.cert_store().objects_len());
 
     let mut new_store = X509StoreBuilder::new().unwrap();
-    new_store.add_cert(cert).unwrap();
-    new_store.add_cert(cert2).unwrap();
+    new_store.add_cert(&cert).unwrap();
+    new_store.add_cert(&cert2).unwrap();
     let new_store = new_store.build();
     assert_eq!(2, new_store.objects_len());
 

boring/src/ssl/test/verify.rs

@@ -32,7 +32,7 @@ fn trusted_with_set_cert() {
 
     let mut store = X509StoreBuilder::new().unwrap();
     let x509 = X509::from_pem(super::ROOT_CERT).unwrap();
-    store.add_cert(x509).unwrap();
+    store.add_cert(&x509).unwrap();
 
     let mut client = server.client();
     client.ctx().set_verify(SslVerifyMode::PEER);

boring/src/x509/store.rs

@@ -36,15 +36,15 @@
 //!
 //! let certificate: X509 = builder.build();
 //! let mut builder = X509StoreBuilder::new().unwrap();
-//! let _ = builder.add_cert(certificate);
+//! let _ = builder.add_cert(&certificate);
 //! let store: X509Store = builder.build();
 //! ```
 
 use crate::error::ErrorStack;
 use crate::ffi;
 use crate::stack::StackRef;
 use crate::x509::verify::{X509VerifyFlags, X509VerifyParamRef};
-use crate::x509::{X509Object, X509};
+use crate::x509::{X509Object, X509Ref};
 use crate::{cvt, cvt_p};
 use foreign_types::{ForeignType, ForeignTypeRef};
 use openssl_macros::corresponds;
@@ -79,9 +79,8 @@ impl X509StoreBuilder {
 
 impl X509StoreBuilderRef {
     /// Adds a certificate to the certificate store.
-    // FIXME should take an &X509Ref
     #[corresponds(X509_STORE_add_cert)]
-    pub fn add_cert(&mut self, cert: X509) -> Result<(), ErrorStack> {
+    pub fn add_cert(&mut self, cert: &X509Ref) -> Result<(), ErrorStack> {
         unsafe { cvt(ffi::X509_STORE_add_cert(self.as_ptr(), cert.as_ptr())) }
     }
 

boring/src/x509/tests/mod.rs

@@ -451,7 +451,7 @@ fn test_verify_cert() {
     let chain = Stack::new().unwrap();
 
     let mut store_bldr = X509StoreBuilder::new().unwrap();
-    store_bldr.add_cert(ca).unwrap();
+    store_bldr.add_cert(&ca).unwrap();
     let store = store_bldr.build();
     let empty_store = X509StoreBuilder::new().unwrap().build();
 
@@ -484,7 +484,7 @@ fn test_verify_fails() {
     let chain = Stack::new().unwrap();
 
     let mut store_bldr = X509StoreBuilder::new().unwrap();
-    store_bldr.add_cert(ca).unwrap();
+    store_bldr.add_cert(&ca).unwrap();
     let store = store_bldr.build();
 
     let mut context = X509StoreContext::new().unwrap();

boring/src/x509/tests/trusted_first.rs

@@ -75,7 +75,7 @@ fn verify(
         let mut builder = X509StoreBuilder::new().unwrap();
 
         for cert in trusted {
-            builder.add_cert((**cert).to_owned()).unwrap();
+            builder.add_cert(cert).unwrap();
         }
 
         builder.build()