From f645165ee2b41f9c15d9a7d8f3eec56000b7e445 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Mon, 5 Mar 2018 19:25:01 -0800 Subject: [PATCH 01/38] Remove the x509 module-level example The example generated a bogus certificate that was missing a serial number, a validity range, etc. Generating a correct x509 certificate is complex enough that doing it correctly is too long to be a reasonable doc example. There's already a more complete example in the examples directory that handles things more correctly. Closes #859 --- openssl/src/x509/mod.rs | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index a4bbb5f0..9638e6a3 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -6,39 +6,6 @@ //! data with the included public key. `X509` certificates are used in many //! Internet protocols, including SSL/TLS, which is the basis for HTTPS, //! the secure protocol for browsing the web. -//! -//! # Example -//! -//! Build an `X509` certificate and use a generated RSA key to sign it. -//! -//! ```rust -//! -//! extern crate openssl; -//! -//! use openssl::x509::{X509, X509Name}; -//! use openssl::pkey::PKey; -//! use openssl::hash::MessageDigest; -//! use openssl::rsa::Rsa; -//! use openssl::nid::Nid; -//! -//! fn main() { -//! let rsa = Rsa::generate(2048).unwrap(); -//! let pkey = PKey::from_rsa(rsa).unwrap(); -//! -//! let mut name = X509Name::builder().unwrap(); -//! name.append_entry_by_nid(Nid::COMMONNAME, "foobar.com").unwrap(); -//! let name = name.build(); -//! -//! let mut builder = X509::builder().unwrap(); -//! builder.set_version(2).unwrap(); -//! builder.set_subject_name(&name).unwrap(); -//! builder.set_issuer_name(&name).unwrap(); -//! builder.set_pubkey(&pkey).unwrap(); -//! builder.sign(&pkey, MessageDigest::sha256()).unwrap(); -//! -//! let certificate: X509 = builder.build(); -//! } -//! ``` use libc::{c_int, c_long}; use ffi; From eb6296e892b168ef5f0908271443b646d742d724 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sun, 19 Mar 2017 00:25:45 -0700 Subject: [PATCH 02/38] add verify_cert and store_context_builder --- openssl-sys/src/lib.rs | 4 ++++ openssl/src/x509/tests.rs | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/openssl-sys/src/lib.rs b/openssl-sys/src/lib.rs index 77f69188..85ab03f7 100644 --- a/openssl-sys/src/lib.rs +++ b/openssl-sys/src/lib.rs @@ -2605,6 +2605,8 @@ extern "C" { pub fn X509_sign(x: *mut X509, pkey: *mut EVP_PKEY, md: *const EVP_MD) -> c_int; pub fn X509_get_pubkey(x: *mut X509) -> *mut EVP_PKEY; pub fn X509_to_X509_REQ(x: *mut X509, pkey: *mut EVP_PKEY, md: *const EVP_MD) -> *mut X509_REQ; + #[cfg(not(any(ossl101, libressl)))] + pub fn X509_verify_cert(ctx: *mut X509_STORE_CTX) -> c_int; pub fn X509_verify_cert_error_string(n: c_long) -> *const c_char; pub fn X509_get1_ocsp(x: *mut X509) -> *mut stack_st_OPENSSL_STRING; pub fn X509_check_issued(issuer: *mut X509, subject: *mut X509) -> c_int; @@ -2638,6 +2640,8 @@ extern "C" { pub fn X509_STORE_add_cert(store: *mut X509_STORE, x: *mut X509) -> c_int; pub fn X509_STORE_set_default_paths(store: *mut X509_STORE) -> c_int; + pub fn X509_STORE_CTX_new() -> *mut X509_STORE_CTX; + pub fn X509_STORE_CTX_init(ctx: *mut X509_STORE_CTX, store: *mut X509_STORE, x509: *mut X509, chain: *mut stack_st_X509) -> c_int; pub fn X509_STORE_CTX_free(ctx: *mut X509_STORE_CTX); pub fn X509_STORE_CTX_get_current_cert(ctx: *mut X509_STORE_CTX) -> *mut X509; pub fn X509_STORE_CTX_get_error(ctx: *mut X509_STORE_CTX) -> c_int; diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 6f6b430a..e91a039b 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -291,3 +291,20 @@ fn clone_x509() { let cert = X509::from_pem(cert).unwrap(); cert.clone(); } + +#[test] +fn test_verify_cert() { + let cert = include_bytes!("../../test/cert.pem"); + let cert = X509::from_pem(cert).unwrap(); + let ca = include_bytes!("../../test/root-ca.pem"); + let ca = X509::from_pem(ca).unwrap(); + + let mut store_bldr = X509StoreBuilder::new().unwrap(); + store_bldr.add_cert(ca); + let store = store_bldr.build(); + + let store_ctx_bldr = X509StoreContext::builder().unwrap(); + let store_ctx = store_ctx_bldr.build(store, cert, Stack::new().unwrap()).unwrap(); + + store_ctx.verify_cert().unwrap(); +} From 3595ff9e5170f6ef396f579e7e366ebe6ecb0a1f Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sun, 19 Mar 2017 00:47:22 -0700 Subject: [PATCH 03/38] Fix memory mgmt --- openssl/src/x509/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index e91a039b..a94c40bd 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -304,7 +304,7 @@ fn test_verify_cert() { let store = store_bldr.build(); let store_ctx_bldr = X509StoreContext::builder().unwrap(); - let store_ctx = store_ctx_bldr.build(store, cert, Stack::new().unwrap()).unwrap(); + let store_ctx = store_ctx_bldr.build(&store, &cert, &Stack::new().unwrap()).unwrap(); store_ctx.verify_cert().unwrap(); } From 847fac25f8c85a6cb0327f361cb310a93e2bada7 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sun, 19 Mar 2017 00:55:42 -0700 Subject: [PATCH 04/38] properly version library functions --- openssl/src/x509/mod.rs | 7 +++++++ openssl/src/x509/tests.rs | 1 + 2 files changed, 8 insertions(+) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 9638e6a3..1e7c4448 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -86,6 +86,13 @@ impl X509StoreContextRef { } } + #[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))] + pub fn verify_cert(self) -> Result, ErrorStack> { + unsafe { + cvt(ffi::X509_verify_cert(self.as_ptr())).map(|_| ()) + } + } + /// Returns the error code of the context. /// /// This corresponds to [`X509_STORE_CTX_get_error`]. diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index a94c40bd..2b9d5dd3 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -285,6 +285,7 @@ fn signature() { assert_eq!(algorithm.object().to_string(), "sha256WithRSAEncryption"); } +#[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))] #[test] fn clone_x509() { let cert = include_bytes!("../../test/cert.pem"); From 35cad33d518f3b6ecff1a01a4707b35ab834342e Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sun, 19 Mar 2017 12:06:41 -0700 Subject: [PATCH 05/38] fix error check --- openssl-sys/src/lib.rs | 1 - openssl/src/x509/mod.rs | 4 ++-- openssl/src/x509/tests.rs | 5 ++--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/openssl-sys/src/lib.rs b/openssl-sys/src/lib.rs index 85ab03f7..c29b60e8 100644 --- a/openssl-sys/src/lib.rs +++ b/openssl-sys/src/lib.rs @@ -2605,7 +2605,6 @@ extern "C" { pub fn X509_sign(x: *mut X509, pkey: *mut EVP_PKEY, md: *const EVP_MD) -> c_int; pub fn X509_get_pubkey(x: *mut X509) -> *mut EVP_PKEY; pub fn X509_to_X509_REQ(x: *mut X509, pkey: *mut EVP_PKEY, md: *const EVP_MD) -> *mut X509_REQ; - #[cfg(not(any(ossl101, libressl)))] pub fn X509_verify_cert(ctx: *mut X509_STORE_CTX) -> c_int; pub fn X509_verify_cert_error_string(n: c_long) -> *const c_char; pub fn X509_get1_ocsp(x: *mut X509) -> *mut stack_st_OPENSSL_STRING; diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 1e7c4448..6133e1a3 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -86,11 +86,11 @@ impl X509StoreContextRef { } } - #[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))] pub fn verify_cert(self) -> Result, ErrorStack> { unsafe { - cvt(ffi::X509_verify_cert(self.as_ptr())).map(|_| ()) + try!(cvt(ffi::X509_verify_cert(self.as_ptr())).map(|_| ())) } + Ok(self.error()) } /// Returns the error code of the context. diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 2b9d5dd3..b6303ade 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -285,7 +285,6 @@ fn signature() { assert_eq!(algorithm.object().to_string(), "sha256WithRSAEncryption"); } -#[cfg(any(all(feature = "v102", ossl102), all(feature = "v110", ossl110)))] #[test] fn clone_x509() { let cert = include_bytes!("../../test/cert.pem"); @@ -301,11 +300,11 @@ fn test_verify_cert() { let ca = X509::from_pem(ca).unwrap(); let mut store_bldr = X509StoreBuilder::new().unwrap(); - store_bldr.add_cert(ca); + store_bldr.add_cert(ca).unwrap(); let store = store_bldr.build(); let store_ctx_bldr = X509StoreContext::builder().unwrap(); let store_ctx = store_ctx_bldr.build(&store, &cert, &Stack::new().unwrap()).unwrap(); - store_ctx.verify_cert().unwrap(); + assert!(store_ctx.verify_cert().unwrap().is_none()); } From 910386027dea776c3b5034d216169cefde912e45 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sun, 19 Mar 2017 12:12:57 -0700 Subject: [PATCH 06/38] add comment about consuming self in verify_cert --- openssl/src/x509/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 6133e1a3..cb5eca40 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -86,6 +86,9 @@ impl X509StoreContextRef { } } + /// Verifies the certificate associated in the `build()` method + /// + /// This consumes self as the `X509StoreContext` must be reinitialized subsequent to any cally to verify. pub fn verify_cert(self) -> Result, ErrorStack> { unsafe { try!(cvt(ffi::X509_verify_cert(self.as_ptr())).map(|_| ())) From d8a11973e2c9ccc5a806936edb2cccf28332bc5e Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Mon, 20 Mar 2017 22:28:15 -0700 Subject: [PATCH 07/38] convert to raw pass-through methods --- openssl/src/x509/mod.rs | 26 +++++++++++++++++++++----- openssl/src/x509/tests.rs | 4 ++-- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index cb5eca40..52907110 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -86,14 +86,20 @@ impl X509StoreContextRef { } } - /// Verifies the certificate associated in the `build()` method + /// Initializes the store context to verify the certificate. /// - /// This consumes self as the `X509StoreContext` must be reinitialized subsequent to any cally to verify. - pub fn verify_cert(self) -> Result, ErrorStack> { + /// This Context can only be used once, subsequent to any validation, the context must be reinitialized. + /// + /// # Arguments + /// + /// * `trust` - a store of the trusted chain of certificates, or CAs, to validated the certificate + /// * `cert` - certificate to validate + /// * `cert_chain` - the certificates chain + pub fn init(&self, trust: &store::X509StoreRef, cert: &X509Ref, cert_chain: &StackRef) -> Result<(), ErrorStack> { unsafe { - try!(cvt(ffi::X509_verify_cert(self.as_ptr())).map(|_| ())) + cvt(ffi::X509_STORE_CTX_init(self.as_ptr(), trust.as_ptr(), cert.as_ptr(), cert_chain.as_ptr())) + .map(|_| ()) } - Ok(self.error()) } /// Returns the error code of the context. @@ -105,6 +111,16 @@ impl X509StoreContextRef { unsafe { X509VerifyResult::from_raw(ffi::X509_STORE_CTX_get_error(self.as_ptr())) } } + /// Verifies the certificate associated in the `init()` method + /// + /// This consumes self as the `X509StoreContext` must be reinitialized subsequent to any cally to verify. + pub fn verify_cert(&self) -> Result, ErrorStack> { + unsafe { + try!(cvt(ffi::X509_verify_cert(self.as_ptr())).map(|_| ())) + } + Ok(self.error()) + } + /// Set the error code of the context. /// /// This corresponds to [`X509_STORE_CTX_set_error`]. diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index b6303ade..6ef4f18e 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -303,8 +303,8 @@ fn test_verify_cert() { store_bldr.add_cert(ca).unwrap(); let store = store_bldr.build(); - let store_ctx_bldr = X509StoreContext::builder().unwrap(); - let store_ctx = store_ctx_bldr.build(&store, &cert, &Stack::new().unwrap()).unwrap(); + let store_ctx = X509StoreContext::new().unwrap(); + store_ctx.init(&store, &cert, &Stack::new().unwrap()).unwrap(); assert!(store_ctx.verify_cert().unwrap().is_none()); } From 2251a6f2b6da97a2e07f230787cb63c549e6940c Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Tue, 21 Mar 2017 08:59:56 +0000 Subject: [PATCH 08/38] Little tweaks --- openssl/src/x509/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 52907110..dcd4296a 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -88,14 +88,14 @@ impl X509StoreContextRef { /// Initializes the store context to verify the certificate. /// - /// This Context can only be used once, subsequent to any validation, the context must be reinitialized. + /// The context must be re-initialized before each call to `verify_cert`. /// /// # Arguments /// /// * `trust` - a store of the trusted chain of certificates, or CAs, to validated the certificate /// * `cert` - certificate to validate - /// * `cert_chain` - the certificates chain - pub fn init(&self, trust: &store::X509StoreRef, cert: &X509Ref, cert_chain: &StackRef) -> Result<(), ErrorStack> { + /// * `cert_chain` - the certificate's chain + pub fn init(&mut self, trust: &store::X509StoreRef, cert: &X509Ref, cert_chain: &StackRef) -> Result<(), ErrorStack> { unsafe { cvt(ffi::X509_STORE_CTX_init(self.as_ptr(), trust.as_ptr(), cert.as_ptr(), cert_chain.as_ptr())) .map(|_| ()) @@ -113,7 +113,7 @@ impl X509StoreContextRef { /// Verifies the certificate associated in the `init()` method /// - /// This consumes self as the `X509StoreContext` must be reinitialized subsequent to any cally to verify. + /// The context must be re-initialized before each call to this method. pub fn verify_cert(&self) -> Result, ErrorStack> { unsafe { try!(cvt(ffi::X509_verify_cert(self.as_ptr())).map(|_| ())) From 3187366cc5fb8619dd496b9bfccaba8c66c6923f Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Thu, 23 Mar 2017 21:37:42 -0700 Subject: [PATCH 09/38] restructure to self contained function --- openssl/src/x509/mod.rs | 15 +++++++++++---- openssl/src/x509/tests.rs | 5 +---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index dcd4296a..5dd12b0e 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -112,13 +112,20 @@ impl X509StoreContextRef { } /// Verifies the certificate associated in the `init()` method + /// * `cert_chain` - the certificates chain /// - /// The context must be re-initialized before each call to this method. - pub fn verify_cert(&self) -> Result, ErrorStack> { + /// # Result + /// + /// The Result must be `Some(None)` to be a valid certificate, otherwise the cert is not valid. + pub fn verify_cert(trust: &store::X509StoreRef, cert: &X509Ref, cert_chain: &StackRef) -> Result, ErrorStack> { unsafe { - try!(cvt(ffi::X509_verify_cert(self.as_ptr())).map(|_| ())) + ffi::init(); + let context = try!(cvt_p(ffi::X509_STORE_CTX_new()).map(|p| X509StoreContext(p))); + try!(cvt(ffi::X509_STORE_CTX_init(context.as_ptr(), trust.as_ptr(), cert.as_ptr(), cert_chain.as_ptr())) + .map(|_| ())); + try!(cvt(ffi::X509_verify_cert(context.as_ptr())).map(|_| ())); + Ok(context.error()) } - Ok(self.error()) } /// Set the error code of the context. diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 6ef4f18e..05baac12 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -303,8 +303,5 @@ fn test_verify_cert() { store_bldr.add_cert(ca).unwrap(); let store = store_bldr.build(); - let store_ctx = X509StoreContext::new().unwrap(); - store_ctx.init(&store, &cert, &Stack::new().unwrap()).unwrap(); - - assert!(store_ctx.verify_cert().unwrap().is_none()); + assert!(X509StoreContext::verify_cert(&store, &cert, &Stack::new().unwrap()).unwrap().is_none()); } From a1cfde765a2a63798411ce4d518b8d32c085ffbf Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Thu, 23 Mar 2017 22:11:23 -0700 Subject: [PATCH 10/38] add cleanup ffi to store context --- openssl-sys/src/lib.rs | 1 + openssl/src/x509/mod.rs | 8 ++++++-- openssl/src/x509/tests.rs | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/openssl-sys/src/lib.rs b/openssl-sys/src/lib.rs index c29b60e8..3e5b3dd6 100644 --- a/openssl-sys/src/lib.rs +++ b/openssl-sys/src/lib.rs @@ -2640,6 +2640,7 @@ extern "C" { pub fn X509_STORE_set_default_paths(store: *mut X509_STORE) -> c_int; pub fn X509_STORE_CTX_new() -> *mut X509_STORE_CTX; + pub fn X509_STORE_CTX_cleanup(ctx: *mut X509_STORE_CTX); pub fn X509_STORE_CTX_init(ctx: *mut X509_STORE_CTX, store: *mut X509_STORE, x509: *mut X509, chain: *mut stack_st_X509) -> c_int; pub fn X509_STORE_CTX_free(ctx: *mut X509_STORE_CTX); pub fn X509_STORE_CTX_get_current_cert(ctx: *mut X509_STORE_CTX) -> *mut X509; diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 5dd12b0e..0cfa8ada 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -117,14 +117,18 @@ impl X509StoreContextRef { /// # Result /// /// The Result must be `Some(None)` to be a valid certificate, otherwise the cert is not valid. - pub fn verify_cert(trust: &store::X509StoreRef, cert: &X509Ref, cert_chain: &StackRef) -> Result, ErrorStack> { + pub fn verify_cert(trust: store::X509Store, cert: X509, cert_chain: Stack) -> Result, ErrorStack> { unsafe { ffi::init(); let context = try!(cvt_p(ffi::X509_STORE_CTX_new()).map(|p| X509StoreContext(p))); try!(cvt(ffi::X509_STORE_CTX_init(context.as_ptr(), trust.as_ptr(), cert.as_ptr(), cert_chain.as_ptr())) .map(|_| ())); try!(cvt(ffi::X509_verify_cert(context.as_ptr())).map(|_| ())); - Ok(context.error()) + + let result = Ok(context.error()); + ffi::X509_STORE_CTX_cleanup(context.as_ptr()); + + result } } diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 05baac12..96d45742 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -303,5 +303,5 @@ fn test_verify_cert() { store_bldr.add_cert(ca).unwrap(); let store = store_bldr.build(); - assert!(X509StoreContext::verify_cert(&store, &cert, &Stack::new().unwrap()).unwrap().is_none()); + assert!(X509StoreContext::verify_cert(store, cert, Stack::new().unwrap()).unwrap().is_none()); } From 6abac82f13c80a2726fc2e9a0f6913357e93a985 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sun, 26 Mar 2017 00:16:27 -0700 Subject: [PATCH 11/38] cleanup and add negative test --- openssl/src/x509/mod.rs | 13 ++++++++----- openssl/src/x509/tests.rs | 16 +++++++++++++++- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 0cfa8ada..6bb58dbd 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -117,18 +117,21 @@ impl X509StoreContextRef { /// # Result /// /// The Result must be `Some(None)` to be a valid certificate, otherwise the cert is not valid. - pub fn verify_cert(trust: store::X509Store, cert: X509, cert_chain: Stack) -> Result, ErrorStack> { + pub fn verify_cert(trust: store::X509Store, cert: X509, cert_chain: Stack) -> Result<(), ErrorStack> { unsafe { ffi::init(); let context = try!(cvt_p(ffi::X509_STORE_CTX_new()).map(|p| X509StoreContext(p))); try!(cvt(ffi::X509_STORE_CTX_init(context.as_ptr(), trust.as_ptr(), cert.as_ptr(), cert_chain.as_ptr())) .map(|_| ())); + + mem::forget(trust); + mem::forget(cert); + mem::forget(cert_chain); + + // verify_cert returns an error `<= 0` if there was a validation error try!(cvt(ffi::X509_verify_cert(context.as_ptr())).map(|_| ())); - let result = Ok(context.error()); - ffi::X509_STORE_CTX_cleanup(context.as_ptr()); - - result + Ok(()) } } diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 96d45742..7ea91432 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -303,5 +303,19 @@ fn test_verify_cert() { store_bldr.add_cert(ca).unwrap(); let store = store_bldr.build(); - assert!(X509StoreContext::verify_cert(store, cert, Stack::new().unwrap()).unwrap().is_none()); + assert!(X509StoreContext::verify_cert(store, cert, Stack::new().unwrap()).is_ok()); +} + +#[test] +fn test_verify_fails() { + let cert = include_bytes!("../../test/cert.pem"); + let cert = X509::from_pem(cert).unwrap(); + let ca = include_bytes!("../../test/alt_name_cert.pem"); + let ca = X509::from_pem(ca).unwrap(); + + let mut store_bldr = X509StoreBuilder::new().unwrap(); + store_bldr.add_cert(ca).unwrap(); + let store = store_bldr.build(); + + assert!(X509StoreContext::verify_cert(store, cert, Stack::new().unwrap()).is_err()); } From 53adf0e6a4975c775dc1f18875d6ebc972650b92 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sun, 26 Mar 2017 00:20:49 -0700 Subject: [PATCH 12/38] delay return until after forgets --- openssl/src/x509/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 6bb58dbd..6023b5a9 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -121,13 +121,15 @@ impl X509StoreContextRef { unsafe { ffi::init(); let context = try!(cvt_p(ffi::X509_STORE_CTX_new()).map(|p| X509StoreContext(p))); - try!(cvt(ffi::X509_STORE_CTX_init(context.as_ptr(), trust.as_ptr(), cert.as_ptr(), cert_chain.as_ptr())) - .map(|_| ())); + let init_result = cvt(ffi::X509_STORE_CTX_init(context.as_ptr(), trust.as_ptr(), cert.as_ptr(), cert_chain.as_ptr())) + .map(|_| ()); mem::forget(trust); mem::forget(cert); mem::forget(cert_chain); + try!(init_result); + // verify_cert returns an error `<= 0` if there was a validation error try!(cvt(ffi::X509_verify_cert(context.as_ptr())).map(|_| ())); From 888f4ccaabafd8eed7cdc69be127dc700c7bbf54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 7 Mar 2018 15:07:55 +0100 Subject: [PATCH 13/38] Fixes the implementation of `X509StoreContextRef::verify_cert` The certificate, the store and the certificates chain does not need to be consumed by `verify_cert` and instead are taken as references. We also call `X509_STORE_CTX_cleanup`, after the verification succeeded. --- openssl/src/x509/mod.rs | 61 +++++++++++++++++++-------------------- openssl/src/x509/tests.rs | 11 +++++-- 2 files changed, 38 insertions(+), 34 deletions(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 6023b5a9..e2c87731 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -67,6 +67,18 @@ impl X509StoreContext { pub fn ssl_idx() -> Result, ErrorStack> { unsafe { cvt_n(ffi::SSL_get_ex_data_X509_STORE_CTX_idx()).map(|idx| Index::from_raw(idx)) } } + + /// Creates a new `X509StoreContext` instance. + /// + /// This corresponds to [`X509_STORE_CTX_new`]. + /// + /// [`X509_STORE_CTX_new`]: https://www.openssl.org/docs/man1.1.0/crypto/X509_STORE_CTX_new.html + pub fn new() -> Result { + unsafe { + ffi::init(); + cvt_p(ffi::X509_STORE_CTX_new()).map(|p| X509StoreContext(p)) + } + } } impl X509StoreContextRef { @@ -86,22 +98,6 @@ impl X509StoreContextRef { } } - /// Initializes the store context to verify the certificate. - /// - /// The context must be re-initialized before each call to `verify_cert`. - /// - /// # Arguments - /// - /// * `trust` - a store of the trusted chain of certificates, or CAs, to validated the certificate - /// * `cert` - certificate to validate - /// * `cert_chain` - the certificate's chain - pub fn init(&mut self, trust: &store::X509StoreRef, cert: &X509Ref, cert_chain: &StackRef) -> Result<(), ErrorStack> { - unsafe { - cvt(ffi::X509_STORE_CTX_init(self.as_ptr(), trust.as_ptr(), cert.as_ptr(), cert_chain.as_ptr())) - .map(|_| ()) - } - } - /// Returns the error code of the context. /// /// This corresponds to [`X509_STORE_CTX_get_error`]. @@ -111,27 +107,30 @@ impl X509StoreContextRef { unsafe { X509VerifyResult::from_raw(ffi::X509_STORE_CTX_get_error(self.as_ptr())) } } - /// Verifies the certificate associated in the `init()` method - /// * `cert_chain` - the certificates chain + /// Verifies a certificate with the given certificate store. + /// * `trust` - The certificate store with the trusted certificates. + /// * `cert` - The certificate that should be verified. + /// * `cert_chain` - The certificates chain. + /// + /// This corresponds to [`X509_STORE_CTX_init`] followed by [`X509_verify_cert`] and + /// [`X509_STORE_CTX_cleanup`]. + /// + /// [`X509_STORE_CTX_init`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_init.html + /// [`X509_verify_cert`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_verify_cert.html + /// [`X509_STORE_CTX_cleanup`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_cleanup.html /// /// # Result /// - /// The Result must be `Some(None)` to be a valid certificate, otherwise the cert is not valid. - pub fn verify_cert(trust: store::X509Store, cert: X509, cert_chain: Stack) -> Result<(), ErrorStack> { + /// The Result must be `Ok(())` to be a valid certificate, otherwise the cert is not valid. + pub fn verify_cert(&mut self, trust: &store::X509StoreRef, cert: &X509Ref, + cert_chain: &StackRef) -> Result<(), ErrorStack> { unsafe { - ffi::init(); - let context = try!(cvt_p(ffi::X509_STORE_CTX_new()).map(|p| X509StoreContext(p))); - let init_result = cvt(ffi::X509_STORE_CTX_init(context.as_ptr(), trust.as_ptr(), cert.as_ptr(), cert_chain.as_ptr())) - .map(|_| ()); + cvt(ffi::X509_STORE_CTX_init(self.as_ptr(), trust.as_ptr(), + cert.as_ptr(), cert_chain.as_ptr()))?; - mem::forget(trust); - mem::forget(cert); - mem::forget(cert_chain); + cvt(ffi::X509_verify_cert(self.as_ptr()))?; - try!(init_result); - - // verify_cert returns an error `<= 0` if there was a validation error - try!(cvt(ffi::X509_verify_cert(context.as_ptr())).map(|_| ())); + ffi::X509_STORE_CTX_cleanup(self.as_ptr()); Ok(()) } diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 7ea91432..31805956 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -7,9 +7,10 @@ use nid::Nid; use pkey::{PKey, Private}; use rsa::Rsa; use stack::Stack; -use x509::{X509, X509Name, X509Req, X509VerifyResult}; +use x509::{X509, X509Name, X509Req, X509VerifyResult, X509StoreContext}; use x509::extension::{AuthorityKeyIdentifier, BasicConstraints, ExtendedKeyUsage, KeyUsage, SubjectAlternativeName, SubjectKeyIdentifier}; +use x509::store::X509StoreBuilder; fn pkey() -> PKey { let rsa = Rsa::generate(2048).unwrap(); @@ -298,12 +299,14 @@ fn test_verify_cert() { let cert = X509::from_pem(cert).unwrap(); let ca = include_bytes!("../../test/root-ca.pem"); let ca = X509::from_pem(ca).unwrap(); + let chain = Stack::new().unwrap(); let mut store_bldr = X509StoreBuilder::new().unwrap(); store_bldr.add_cert(ca).unwrap(); let store = store_bldr.build(); - assert!(X509StoreContext::verify_cert(store, cert, Stack::new().unwrap()).is_ok()); + let mut context = X509StoreContext::new().unwrap(); + assert!(context.verify_cert(&store, &cert, &chain).is_ok()); } #[test] @@ -312,10 +315,12 @@ fn test_verify_fails() { let cert = X509::from_pem(cert).unwrap(); let ca = include_bytes!("../../test/alt_name_cert.pem"); let ca = X509::from_pem(ca).unwrap(); + let chain = Stack::new().unwrap(); let mut store_bldr = X509StoreBuilder::new().unwrap(); store_bldr.add_cert(ca).unwrap(); let store = store_bldr.build(); - assert!(X509StoreContext::verify_cert(store, cert, Stack::new().unwrap()).is_err()); + let mut context = X509StoreContext::new().unwrap(); + assert!(context.verify_cert(&store, &cert, &chain).is_err()); } From 9a8a1c752b05c0421a032f7f835e8f95ce15df47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 7 Mar 2018 18:42:13 +0100 Subject: [PATCH 14/38] Adds `PKeyRef::get_id` to get the OID of a key --- openssl-sys/src/lib.rs | 1 + openssl/src/pkey.rs | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/openssl-sys/src/lib.rs b/openssl-sys/src/lib.rs index 77f69188..78b683f4 100644 --- a/openssl-sys/src/lib.rs +++ b/openssl-sys/src/lib.rs @@ -2023,6 +2023,7 @@ extern "C" { pub fn EVP_PKEY_get1_DH(k: *mut EVP_PKEY) -> *mut DH; pub fn EVP_PKEY_get1_EC_KEY(k: *mut EVP_PKEY) -> *mut EC_KEY; pub fn EVP_PKEY_cmp(a: *const EVP_PKEY, b: *const EVP_PKEY) -> c_int; + pub fn EVP_PKEY_id(pkey: *const EVP_PKEY) -> c_int; pub fn EVP_PKEY_new_mac_key( type_: c_int, e: *mut ENGINE, diff --git a/openssl/src/pkey.rs b/openssl/src/pkey.rs index 8f7c4e4e..252f0e22 100644 --- a/openssl/src/pkey.rs +++ b/openssl/src/pkey.rs @@ -70,6 +70,16 @@ pub enum Public {} /// A tag type indicating that a key has private components. pub enum Private {} +/// The OIDs that identify the type of a key. +#[derive(PartialEq, Debug)] +pub enum OID { + RSA, + HMAC, + DSA, + DH, + EC, +} + /// A trait indicating that a key has parameters. pub unsafe trait HasParams {} @@ -155,6 +165,25 @@ impl PKeyRef { Ok(EcKey::from_ptr(ec_key)) } } + + /// Returns `Some(OID)` as the type of this key or `None`, if the OID is supported by this + /// library. + /// + /// This corresponds to [`EVP_PKEY_id`]. + /// + /// [`EVP_PKEY_id`]: https://www.openssl.org/docs/man1.1.0/crypto/EVP_PKEY_id.html + pub fn get_id(&self) -> Option { + unsafe { + match ffi::EVP_PKEY_id(self.as_ptr()) { + ffi::EVP_PKEY_RSA => Some(OID::RSA), + ffi::EVP_PKEY_HMAC => Some(OID::HMAC), + ffi::EVP_PKEY_DSA => Some(OID::DSA), + ffi::EVP_PKEY_DH => Some(OID::DH), + ffi::EVP_PKEY_EC => Some(OID::EC), + _ => None, + } + } + } } impl PKeyRef @@ -531,6 +560,7 @@ mod tests { let rsa = Rsa::generate(2048).unwrap(); let pkey = PKey::from_rsa(rsa).unwrap(); pkey.rsa().unwrap(); + assert_eq!(pkey.get_id().unwrap(), OID::RSA); assert!(pkey.dsa().is_err()); } @@ -539,6 +569,7 @@ mod tests { let dsa = Dsa::generate(2048).unwrap(); let pkey = PKey::from_dsa(dsa).unwrap(); pkey.dsa().unwrap(); + assert_eq!(pkey.get_id().unwrap(), OID::DSA); assert!(pkey.rsa().is_err()); } @@ -548,6 +579,7 @@ mod tests { let dh = Dh::params_from_pem(dh).unwrap(); let pkey = PKey::from_dh(dh).unwrap(); pkey.dh().unwrap(); + assert_eq!(pkey.get_id().unwrap(), OID::DH); assert!(pkey.rsa().is_err()); } @@ -556,6 +588,7 @@ mod tests { let ec_key = EcKey::from_curve_name(Nid::X9_62_PRIME256V1).unwrap(); let pkey = PKey::from_ec_key(ec_key).unwrap(); pkey.ec_key().unwrap(); + assert_eq!(pkey.get_id().unwrap(), OID::EC); assert!(pkey.rsa().is_err()); } } From 84a5ce76079ab210272be3da49c93ebd5ab75524 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 7 Mar 2018 20:43:12 +0100 Subject: [PATCH 15/38] Adds RSA PKCS1 PSS padding --- openssl-sys/src/lib.rs | 1 + openssl/src/rsa.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/openssl-sys/src/lib.rs b/openssl-sys/src/lib.rs index 78b683f4..73918d7d 100644 --- a/openssl-sys/src/lib.rs +++ b/openssl-sys/src/lib.rs @@ -1200,6 +1200,7 @@ pub const RSA_SSLV23_PADDING: c_int = 2; pub const RSA_NO_PADDING: c_int = 3; pub const RSA_PKCS1_OAEP_PADDING: c_int = 4; pub const RSA_X931_PADDING: c_int = 5; +pub const RSA_PKCS1_PSS_PADDING: c_int = 6; pub const SHA_LBLOCK: c_int = 16; diff --git a/openssl/src/rsa.rs b/openssl/src/rsa.rs index 02240948..6a591b69 100644 --- a/openssl/src/rsa.rs +++ b/openssl/src/rsa.rs @@ -63,6 +63,7 @@ impl Padding { pub const NONE: Padding = Padding(ffi::RSA_NO_PADDING); pub const PKCS1: Padding = Padding(ffi::RSA_PKCS1_PADDING); pub const PKCS1_OAEP: Padding = Padding(ffi::RSA_PKCS1_OAEP_PADDING); + pub const PKCS1_PSS: Padding = Padding(ffi::RSA_PKCS1_PSS_PADDING); } generic_foreign_type_and_impl_send_sync! { From 724dd6f830d64de6fc1e6f55b8b0a9f630d928f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 7 Mar 2018 20:43:28 +0100 Subject: [PATCH 16/38] Adds more functions to `Verifier`/`Signer` for RSA keys --- openssl-sys/src/lib.rs | 37 ++++++++++++++++++ openssl/src/sign.rs | 87 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/openssl-sys/src/lib.rs b/openssl-sys/src/lib.rs index 73918d7d..3eb9533b 100644 --- a/openssl-sys/src/lib.rs +++ b/openssl-sys/src/lib.rs @@ -228,9 +228,24 @@ pub const EVP_PKEY_EC: c_int = NID_X9_62_id_ecPublicKey; pub const EVP_PKEY_ALG_CTRL: c_int = 0x1000; pub const EVP_PKEY_CTRL_RSA_PADDING: c_int = EVP_PKEY_ALG_CTRL + 1; +pub const EVP_PKEY_CTRL_RSA_PSS_SALTLEN: c_int = EVP_PKEY_ALG_CTRL + 2; +pub const EVP_PKEY_CTRL_RSA_MGF1_MD: c_int = EVP_PKEY_ALG_CTRL + 5; pub const EVP_PKEY_CTRL_GET_RSA_PADDING: c_int = EVP_PKEY_ALG_CTRL + 6; +pub const EVP_PKEY_OP_SIGN: c_int = 1 << 3; +pub const EVP_PKEY_OP_VERIFY: c_int = 1 << 4; +pub const EVP_PKEY_OP_VERIFYRECOVER: c_int = 1 << 5; +pub const EVP_PKEY_OP_SIGNCTX: c_int = 1 << 6; +pub const EVP_PKEY_OP_VERIFYCTX: c_int = 1 << 7; +pub const EVP_PKEY_OP_ENCRYPT: c_int = 1 << 8; +pub const EVP_PKEY_OP_DECRYPT: c_int = 1 << 9; + +pub const EVP_PKEY_OP_TYPE_SIG: c_int = EVP_PKEY_OP_SIGN | EVP_PKEY_OP_VERIFY + | EVP_PKEY_OP_VERIFYRECOVER | EVP_PKEY_OP_SIGNCTX | EVP_PKEY_OP_VERIFYCTX; + +pub const EVP_PKEY_OP_TYPE_CRYPT: c_int = EVP_PKEY_OP_ENCRYPT | EVP_PKEY_OP_DECRYPT; + pub const EVP_CTRL_GCM_SET_IVLEN: c_int = 0x9; pub const EVP_CTRL_GCM_GET_TAG: c_int = 0x10; pub const EVP_CTRL_GCM_SET_TAG: c_int = 0x11; @@ -1478,6 +1493,28 @@ pub unsafe fn EVP_PKEY_CTX_get_rsa_padding(ctx: *mut EVP_PKEY_CTX, ppad: *mut c_ ) } +pub unsafe fn EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx: *mut EVP_PKEY_CTX, len: c_int) -> c_int { + EVP_PKEY_CTX_ctrl( + ctx, + EVP_PKEY_RSA, + EVP_PKEY_OP_SIGN | EVP_PKEY_OP_VERIFY, + EVP_PKEY_CTRL_RSA_PSS_SALTLEN, + len, + ptr::null_mut(), + ) +} + +pub unsafe fn EVP_PKEY_CTX_set_rsa_mgf1_md(ctx: *mut EVP_PKEY_CTX, md: *mut EVP_MD) -> c_int { + EVP_PKEY_CTX_ctrl( + ctx, + EVP_PKEY_RSA, + EVP_PKEY_OP_TYPE_SIG | EVP_PKEY_OP_TYPE_CRYPT, + EVP_PKEY_CTRL_RSA_MGF1_MD, + 0, + md as *mut c_void, + ) +} + pub unsafe fn SSL_CTX_set_mode(ctx: *mut SSL_CTX, op: c_long) -> c_long { SSL_CTX_ctrl(ctx, SSL_CTRL_MODE, op, ptr::null_mut()) } diff --git a/openssl/src/sign.rs b/openssl/src/sign.rs index a61d883b..9de9bcf7 100644 --- a/openssl/src/sign.rs +++ b/openssl/src/sign.rs @@ -66,6 +66,7 @@ use foreign_types::ForeignTypeRef; use std::io::{self, Write}; use std::marker::PhantomData; use std::ptr; +use libc::c_int; use {cvt, cvt_p}; use hash::MessageDigest; @@ -160,6 +161,38 @@ impl<'a> Signer<'a> { } } + /// Sets the RSA PSS salt length. + /// + /// This is only useful for RSA keys. + /// + /// This corresponds to [`EVP_PKEY_CTX_set_rsa_pss_saltlen`]. + /// + /// [`EVP_PKEY_CTX_set_rsa_pss_saltlen`]: https://www.openssl.org/docs/man1.1.0/crypto/EVP_PKEY_CTX_set_rsa_pss_saltlen.html + pub fn set_rsa_pss_saltlen(&mut self, len: c_int) -> Result<(), ErrorStack> { + unsafe { + cvt(ffi::EVP_PKEY_CTX_set_rsa_pss_saltlen( + self.pctx, + len, + )).map(|_| ()) + } + } + + /// Sets the RSA MGF1 algorithm. + /// + /// This is only useful for RSA keys. + /// + /// This corresponds to [`EVP_PKEY_CTX_set_rsa_mgf1_md`]. + /// + /// [`EVP_PKEY_CTX_set_rsa_mgf1_md`]: https://www.openssl.org/docs/manmaster/man7/RSA-PSS.html + pub fn set_rsa_mgf1_md(&mut self, md: MessageDigest) -> Result<(), ErrorStack> { + unsafe { + cvt(ffi::EVP_PKEY_CTX_set_rsa_mgf1_md( + self.pctx, + md.as_ptr() as *mut _, + )).map(|_| ()) + } + } + /// Feeds more data into the `Signer`. /// /// OpenSSL documentation at [`EVP_DigestUpdate`]. @@ -320,6 +353,38 @@ impl<'a> Verifier<'a> { } } + /// Sets the RSA PSS salt length. + /// + /// This is only useful for RSA keys. + /// + /// This corresponds to [`EVP_PKEY_CTX_set_rsa_pss_saltlen`]. + /// + /// [`EVP_PKEY_CTX_set_rsa_pss_saltlen`]: https://www.openssl.org/docs/man1.1.0/crypto/EVP_PKEY_CTX_set_rsa_pss_saltlen.html + pub fn set_rsa_pss_saltlen(&mut self, len: c_int) -> Result<(), ErrorStack> { + unsafe { + cvt(ffi::EVP_PKEY_CTX_set_rsa_pss_saltlen( + self.pctx, + len, + )).map(|_| ()) + } + } + + /// Sets the RSA MGF1 algorithm. + /// + /// This is only useful for RSA keys. + /// + /// This corresponds to [`EVP_PKEY_CTX_set_rsa_mgf1_md`]. + /// + /// [`EVP_PKEY_CTX_set_rsa_mgf1_md`]: https://www.openssl.org/docs/manmaster/man7/RSA-PSS.html + pub fn set_rsa_mgf1_md(&mut self, md: MessageDigest) -> Result<(), ErrorStack> { + unsafe { + cvt(ffi::EVP_PKEY_CTX_set_rsa_mgf1_md( + self.pctx, + md.as_ptr() as *mut _, + )).map(|_| ()) + } + } + /// Feeds more data into the `Verifier`. /// /// OpenSSL documentation at [`EVP_DigestUpdate`]. @@ -559,4 +624,26 @@ mod test { verifier.update(b"hello world").unwrap(); assert!(verifier.verify(&signature).unwrap()); } + + #[test] + fn rsa_sign_verify() { + let key = include_bytes!("../test/rsa.pem"); + let private_key = Rsa::private_key_from_pem(key).unwrap(); + let pkey = PKey::from_rsa(private_key).unwrap(); + + let mut signer = Signer::new(MessageDigest::sha256(), &pkey).unwrap(); + signer.set_rsa_padding(Padding::PKCS1_PSS).unwrap(); + assert_eq!(signer.rsa_padding().unwrap(), Padding::PKCS1_PSS); + signer.set_rsa_pss_saltlen(-1).unwrap(); + signer.set_rsa_mgf1_md(MessageDigest::sha256()).unwrap(); + signer.update(&Vec::from_hex(INPUT).unwrap()).unwrap(); + let signature = signer.sign_to_vec().unwrap(); + + let mut verifier = Verifier::new(MessageDigest::sha256(), &pkey).unwrap(); + verifier.set_rsa_padding(Padding::PKCS1_PSS).unwrap(); + verifier.set_rsa_pss_saltlen(-1).unwrap(); + verifier.set_rsa_mgf1_md(MessageDigest::sha256()).unwrap(); + verifier.update(&Vec::from_hex(INPUT).unwrap()).unwrap(); + assert!(verifier.verify(&signature).unwrap()); + } } From 2d6cd9eb1633ad80636d273f823b64d723a4be76 Mon Sep 17 00:00:00 2001 From: Rohit Aggarwal Date: Thu, 8 Mar 2018 09:44:05 +0000 Subject: [PATCH 17/38] Exposed some of ECDSA functions --- openssl-sys/src/ossl10x.rs | 26 ++++++ openssl-sys/src/ossl110.rs | 23 +++++ openssl/src/ecdsa.rs | 179 +++++++++++++++++++++++++++++++++++++ openssl/src/lib.rs | 1 + 4 files changed, 229 insertions(+) create mode 100644 openssl/src/ecdsa.rs diff --git a/openssl-sys/src/ossl10x.rs b/openssl-sys/src/ossl10x.rs index f8ff7193..78c76b6e 100644 --- a/openssl-sys/src/ossl10x.rs +++ b/openssl-sys/src/ossl10x.rs @@ -128,6 +128,12 @@ pub struct DSA { pub engine: *mut ::ENGINE, } +#[repr(C)] +pub struct ECDSA_SIG { + pub r: *mut BIGNUM, + pub s: *mut BIGNUM +} + #[repr(C)] pub struct EVP_PKEY { pub type_: c_int, @@ -823,6 +829,26 @@ extern "C" { ); pub fn CRYPTO_set_id_callback(func: unsafe extern "C" fn() -> c_ulong); + pub fn ECDSA_SIG_new() -> *mut ECDSA_SIG; + pub fn ECDSA_SIG_free(sig: *mut ECDSA_SIG); + pub fn i2d_ECDSA_SIG(sig: *const ECDSA_SIG, pp: *mut *mut c_uchar) -> c_int; + pub fn d2i_ECDSA_SIG(sig: *mut *mut ECDSA_SIG, pp: *mut *const c_uchar, len: c_long) -> *mut ECDSA_SIG; + pub fn ECDSA_size(eckey: *const ::EC_KEY) -> c_int; + pub fn ECDSA_sign(_type: c_int, dgst: *const c_uchar, dgstlen: c_int, + sig: *mut c_uchar, siglen: *mut c_uint, eckey: *mut ::EC_KEY) -> c_int; + pub fn ECDSA_do_sign(dgst: *const c_uchar, dgst_len: c_int, eckey: *mut ::EC_KEY) -> *mut ECDSA_SIG; + pub fn ECDSA_verify(_type: c_int, dgst: *const c_uchar, dgstlen: c_int, + sig: *const c_uchar, siglen: c_int, eckey: *mut ::EC_KEY) -> c_int; + pub fn ECDSA_do_verify(dgst: *const c_uchar, dgst_len: c_int, + sig: *const ECDSA_SIG, eckey: *mut ::EC_KEY) -> c_int; + pub fn ECDSA_do_sign_ex(dgst: *const c_uchar, dgstlen: c_int, + kinv: *const BIGNUM, rp: *const BIGNUM, + eckey: *mut ::EC_KEY) -> *mut ECDSA_SIG; + pub fn ECDSA_sign_setup(eckey: *mut ::EC_KEY, ctx: *mut ::BN_CTX, kinv: *mut *mut BIGNUM, rp: *mut *mut BIGNUM) -> c_int; + pub fn ECDSA_sign_ex(_type: c_int, dgst: *const c_uchar, dgstlen: c_int, + sig: *mut c_uchar, siglen: *mut c_uint, + kinv: *const BIGNUM, rp: *const BIGNUM, eckey: *mut ::EC_KEY) -> c_int; + pub fn ERR_load_crypto_strings(); pub fn RSA_generate_key( diff --git a/openssl-sys/src/ossl110.rs b/openssl-sys/src/ossl110.rs index b02c296d..d6f4eb99 100644 --- a/openssl-sys/src/ossl110.rs +++ b/openssl-sys/src/ossl110.rs @@ -8,6 +8,7 @@ pub enum BIO_METHOD {} pub enum CRYPTO_EX_DATA {} pub enum DH {} pub enum DSA {} +pub enum ECDSA_SIG {} pub enum EVP_CIPHER {} pub enum EVP_MD_CTX {} pub enum EVP_PKEY {} @@ -363,4 +364,26 @@ extern "C" { ) -> *mut PKCS12; pub fn X509_REQ_get_version(req: *const X509_REQ) -> c_long; pub fn X509_REQ_get_subject_name(req: *const X509_REQ) -> *mut ::X509_NAME; + + pub fn ECDSA_SIG_new() -> *mut ECDSA_SIG; + pub fn ECDSA_SIG_free(sig: *mut ECDSA_SIG); + pub fn ECDSA_SIG_get0(sig: *const ECDSA_SIG, pr: *mut *const BIGNUM, ps: *mut *const BIGNUM); + pub fn ECDSA_SIG_set0(sig: *mut ECDSA_SIG, pr: *mut BIGNUM, ps: *mut BIGNUM) -> c_int; + pub fn i2d_ECDSA_SIG(sig: *const ECDSA_SIG, pp: *mut *mut c_uchar) -> c_int; + pub fn d2i_ECDSA_SIG(sig: *mut *mut ECDSA_SIG, pp: *mut *const c_uchar, len: c_long) -> *mut ECDSA_SIG; + pub fn ECDSA_size(eckey: *const ::EC_KEY) -> c_int; + pub fn ECDSA_sign(_type: c_int, dgst: *const c_uchar, dgstlen: c_int, + sig: *mut c_uchar, siglen: *mut c_uint, eckey: *mut ::EC_KEY) -> c_int; + pub fn ECDSA_do_sign(dgst: *const c_uchar, dgst_len: c_int, eckey: *mut ::EC_KEY) -> *mut ECDSA_SIG; + pub fn ECDSA_verify(_type: c_int, dgst: *const c_uchar, dgstlen: c_int, + sig: *const c_uchar, siglen: c_int, eckey: *mut ::EC_KEY) -> c_int; + pub fn ECDSA_do_verify(dgst: *const c_uchar, dgst_len: c_int, + sig: *const ECDSA_SIG, eckey: *mut ::EC_KEY) -> c_int; + pub fn ECDSA_do_sign_ex(dgst: *const c_uchar, dgstlen: c_int, + kinv: *const BIGNUM, rp: *const BIGNUM, + eckey: *mut ::EC_KEY) -> *mut ECDSA_SIG; + pub fn ECDSA_sign_setup(eckey: *mut ::EC_KEY, ctx: *mut ::BN_CTX, kinv: *mut *mut BIGNUM, rp: *mut *mut BIGNUM) -> c_int; + pub fn ECDSA_sign_ex(_type: c_int, dgst: *const c_uchar, dgstlen: c_int, + sig: *mut c_uchar, siglen: *mut c_uint, + kinv: *const BIGNUM, rp: *const BIGNUM, eckey: *mut ::EC_KEY) -> c_int; } diff --git a/openssl/src/ecdsa.rs b/openssl/src/ecdsa.rs new file mode 100644 index 00000000..50cb0b62 --- /dev/null +++ b/openssl/src/ecdsa.rs @@ -0,0 +1,179 @@ +//! Low level Elliptic Curve Digital Signature Algorithm (ECDSA) functions. +//! + + +use bn::{BigNum, BigNumRef}; +use {cvt, cvt_n, cvt_p}; +use ec::EcKey; +use error::ErrorStack; +use ffi; +use foreign_types::{ForeignType, ForeignTypeRef}; +use pkey::{Private, Public}; +use std::mem; + + +foreign_type_and_impl_send_sync! { + type CType = ffi::ECDSA_SIG; + fn drop = ffi::ECDSA_SIG_free; + + /// A low level interface to ECDSA + /// + /// OpenSSL documentation at [`ECDSA_sign`] + /// + /// [`ECDSA_sign`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_sign.html + pub struct EcdsaSig; + /// Reference to [`EcdsaSig`] + /// + /// [`EcdsaSig`]: struct.EcdsaSig.html + pub struct EcdsaSigRef; +} + +impl EcdsaSig { + + /// Computes a digital signature of the `dgstlen` bytes hash value `data` using the private EC key eckey. + /// Some example values associated with `dgstlen` are: for SHA-1, it is 20; for SHA-256 it is 32 etc. + /// + /// OpenSSL documentation at [`ECDSA_do_sign`] + /// + /// [`ECDSA_do_sign`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_do_sign.html + pub fn sign(data: &[u8], dgstlen: i32, eckey: &EcKey) -> Result { + unsafe { + let sig = cvt_p(ffi::ECDSA_do_sign(data.as_ptr(), dgstlen, eckey.as_ptr()))?; + Ok(EcdsaSig::from_ptr(sig as *mut _)) + } + } + + /// Returns a new `EcdsaSig` by setting the `r` and `s` values associated with a + /// ECDSA signature. + /// + /// OpenSSL documentation at [`ECDSA_SIG_set0`] + /// + /// [`ECDSA_SIG_set0`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_SIG_set0.html + pub fn from_private_components(r: BigNum, s: BigNum) -> Result { + unsafe { + let sig = cvt_p(ffi::ECDSA_SIG_new())?; + cvt(compat::set_numbers(sig, r.as_ptr(), s.as_ptr()))?; + mem::forget((r, s)); + Ok(EcdsaSig::from_ptr(sig as *mut _)) + } + } + + /// Verifies if the signature is a valid ECDSA signature using the given public key + /// + /// OpenSSL documentation at [`ECDSA_do_verify`] + /// + /// [`ECDSA_do_verify`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_do_verify.html + pub fn verify(&self, data: &[u8], dgstlen: i32, eckey: &EcKey) -> Result { + unsafe { + let x = cvt_n(ffi::ECDSA_do_verify(data.as_ptr(), dgstlen, self.as_ptr(), eckey.as_ptr()))?; + Ok(x == 1) + } + } + + /// Returns internal components: `r` and `s` of a `EcdsaSig`. (See X9.62 or FIPS 186-2) + /// + /// OpenSSL documentation at [`ECDSA_SIG_get0`] + /// + /// [`ECDSA_SIG_get0`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_SIG_get0.html + pub fn private_components(&self) -> (Option<&BigNumRef>, Option<&BigNumRef>) { + unsafe { + let xs = compat::get_numbers(self.as_ptr()); + let r = if xs[0].is_null() { None } else { Some(BigNumRef::from_ptr(xs[0] as *mut _)) }; + let s = if xs[1].is_null() { None } else { Some(BigNumRef::from_ptr(xs[1] as *mut _)) }; + (r, s) + } + } + +} + +#[cfg(ossl110)] +mod compat { + use std::ptr; + + use libc::c_int; + use ffi::{self, BIGNUM, ECDSA_SIG}; + + pub unsafe fn set_numbers(sig: *mut ECDSA_SIG, r: *mut BIGNUM, s: *mut BIGNUM) -> c_int { + ffi::ECDSA_SIG_set0(sig, r, s) + } + + pub unsafe fn get_numbers(sig: *mut ECDSA_SIG) -> [*const BIGNUM; 2] { + let (mut r, mut s) = (ptr::null(), ptr::null()); + ffi::ECDSA_SIG_get0(sig, &mut r, &mut s); + [r, s] + } +} + +#[cfg(ossl10x)] +mod compat { + use libc::c_int; + use ffi::{BIGNUM, ECDSA_SIG}; + + pub unsafe fn set_numbers(sig: *mut ECDSA_SIG, r: *mut BIGNUM, s: *mut BIGNUM) -> c_int { + (*sig).r = r; + (*sig).s = s; + 1 + } + + pub unsafe fn get_numbers(sig: *mut ECDSA_SIG) -> [*const BIGNUM; 2] { + [(*sig).r, (*sig).s] + } + +} + +#[cfg(test)] +mod test { + use nid::Nid; + use ec::EcGroup; + use super::*; + + fn get_public_key(group: &EcGroup, x: &EcKey) -> Result, ErrorStack> { + let public_key_point = x.public_key(); + Ok(EcKey::from_public_key(group, public_key_point)?) + } + + #[test] + fn sign_and_verify() { + let group = EcGroup::from_curve_name(Nid::X9_62_PRIME256V1).unwrap(); + let private_key = EcKey::generate(&group).unwrap(); + let public_key = get_public_key(&group, &private_key).unwrap(); + + let private_key2 = EcKey::generate(&group).unwrap(); + let public_key2 = get_public_key(&group, &private_key2).unwrap(); + + let data = String::from("hello"); + let res = EcdsaSig::sign(data.as_bytes(), 32, &private_key).unwrap(); + + // Signature can be verified using the correct data & correct public key + let verification = res.verify(data.as_bytes(), 32, &public_key).unwrap(); + assert!(verification); + + // Signature will not be verified using the incorrect data but the correct public key + let verification2 = res.verify(String::from("hello2").as_bytes(), 32, &public_key).unwrap(); + assert!(verification2 == false); + + // Signature will not be verified using the correct data but the incorrect public key + let verification3 = res.verify(data.as_bytes(), 32, &public_key2).unwrap(); + assert!(verification3 == false); + } + + #[test] + fn check_private_components() { + let group = EcGroup::from_curve_name(Nid::X9_62_PRIME256V1).unwrap(); + let private_key = EcKey::generate(&group).unwrap(); + let public_key = get_public_key(&group, &private_key).unwrap(); + let data = String::from("hello"); + let res = EcdsaSig::sign(data.as_bytes(), 32, &private_key).unwrap(); + + let verification = res.verify(data.as_bytes(), 32, &public_key).unwrap(); + assert!(verification); + + let x = res.private_components(); + let r = x.0.unwrap().to_owned().unwrap(); + let s = x.1.unwrap().to_owned().unwrap(); + + let res2 = EcdsaSig::from_private_components(r, s).unwrap(); + let verification2 = res2.verify(data.as_bytes(), 32, &public_key).unwrap(); + assert!(verification2); + } +} \ No newline at end of file diff --git a/openssl/src/lib.rs b/openssl/src/lib.rs index 321a301f..e4b621ef 100644 --- a/openssl/src/lib.rs +++ b/openssl/src/lib.rs @@ -39,6 +39,7 @@ pub mod derive; pub mod dh; pub mod dsa; pub mod ec; +pub mod ecdsa; pub mod error; pub mod ex_data; #[cfg(not(libressl))] From 810ddeb4cad73b5f44d2faa88e3439e45fe3bf66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 8 Mar 2018 12:08:39 +0100 Subject: [PATCH 18/38] Moves `cleanup` into its own function --- openssl/src/x509/mod.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index e2c87731..107bfe69 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -108,16 +108,16 @@ impl X509StoreContextRef { } /// Verifies a certificate with the given certificate store. + /// For successive calls to this function, it is required to call `cleanup` in beforehand. + /// /// * `trust` - The certificate store with the trusted certificates. /// * `cert` - The certificate that should be verified. /// * `cert_chain` - The certificates chain. /// - /// This corresponds to [`X509_STORE_CTX_init`] followed by [`X509_verify_cert`] and - /// [`X509_STORE_CTX_cleanup`]. + /// This corresponds to [`X509_STORE_CTX_init`] followed by [`X509_verify_cert`]. /// /// [`X509_STORE_CTX_init`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_init.html /// [`X509_verify_cert`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_verify_cert.html - /// [`X509_STORE_CTX_cleanup`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_cleanup.html /// /// # Result /// @@ -129,13 +129,22 @@ impl X509StoreContextRef { cert.as_ptr(), cert_chain.as_ptr()))?; cvt(ffi::X509_verify_cert(self.as_ptr()))?; - - ffi::X509_STORE_CTX_cleanup(self.as_ptr()); Ok(()) } } + /// Cleans-up the context. + /// + /// This corresponds to [`X509_STORE_CTX_cleanup`]. + /// + /// [`X509_STORE_CTX_cleanup`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_cleanup.html + pub fn cleanup(&mut self) { + unsafe { + ffi::X509_STORE_CTX_cleanup(self.as_ptr()); + } + } + /// Set the error code of the context. /// /// This corresponds to [`X509_STORE_CTX_set_error`]. From 1a0b085377815c7377f63434bbd18d6da7903ad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 8 Mar 2018 12:10:29 +0100 Subject: [PATCH 19/38] Extends the test to verify the certificate two times --- openssl/src/x509/tests.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 31805956..8e89c3a0 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -307,6 +307,8 @@ fn test_verify_cert() { let mut context = X509StoreContext::new().unwrap(); assert!(context.verify_cert(&store, &cert, &chain).is_ok()); + context.cleanup(); + assert!(context.verify_cert(&store, &cert, &chain).is_ok()); } #[test] From b0ea53184d980357222ca0619985f6318f4974da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 8 Mar 2018 12:24:37 +0100 Subject: [PATCH 20/38] Switches to newtype wrapper for Oid --- openssl/src/pkey.rs | 49 +++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/openssl/src/pkey.rs b/openssl/src/pkey.rs index 252f0e22..76ecef7c 100644 --- a/openssl/src/pkey.rs +++ b/openssl/src/pkey.rs @@ -70,14 +70,26 @@ pub enum Public {} /// A tag type indicating that a key has private components. pub enum Private {} -/// The OIDs that identify the type of a key. -#[derive(PartialEq, Debug)] -pub enum OID { - RSA, - HMAC, - DSA, - DH, - EC, +/// The Oids that identify the type of a key. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub struct Oid(c_int); + +impl Oid { + /// Creates a `Oid` from an integer representation. + pub fn from_raw(value: c_int) -> Oid { + Oid(value) + } + + /// Returns the integer representation of `Oid`. + pub fn as_raw(&self) -> c_int { + self.0 + } + + pub const RSA: Oid = Oid(ffi::EVP_PKEY_RSA); + pub const HMAC: Oid = Oid(ffi::EVP_PKEY_HMAC); + pub const DSA: Oid = Oid(ffi::EVP_PKEY_DSA); + pub const DH: Oid = Oid(ffi::EVP_PKEY_DH); + pub const EC: Oid = Oid(ffi::EVP_PKEY_EC); } /// A trait indicating that a key has parameters. @@ -166,22 +178,15 @@ impl PKeyRef { } } - /// Returns `Some(OID)` as the type of this key or `None`, if the OID is supported by this + /// Returns `Some(Oid)` as the type of this key or `None`, if the Oid is supported by this /// library. /// /// This corresponds to [`EVP_PKEY_id`]. /// /// [`EVP_PKEY_id`]: https://www.openssl.org/docs/man1.1.0/crypto/EVP_PKEY_id.html - pub fn get_id(&self) -> Option { + pub fn oid(&self) -> Oid { unsafe { - match ffi::EVP_PKEY_id(self.as_ptr()) { - ffi::EVP_PKEY_RSA => Some(OID::RSA), - ffi::EVP_PKEY_HMAC => Some(OID::HMAC), - ffi::EVP_PKEY_DSA => Some(OID::DSA), - ffi::EVP_PKEY_DH => Some(OID::DH), - ffi::EVP_PKEY_EC => Some(OID::EC), - _ => None, - } + Oid::from_raw(ffi::EVP_PKEY_id(self.as_ptr())) } } } @@ -560,7 +565,7 @@ mod tests { let rsa = Rsa::generate(2048).unwrap(); let pkey = PKey::from_rsa(rsa).unwrap(); pkey.rsa().unwrap(); - assert_eq!(pkey.get_id().unwrap(), OID::RSA); + assert_eq!(pkey.oid(), Oid::RSA); assert!(pkey.dsa().is_err()); } @@ -569,7 +574,7 @@ mod tests { let dsa = Dsa::generate(2048).unwrap(); let pkey = PKey::from_dsa(dsa).unwrap(); pkey.dsa().unwrap(); - assert_eq!(pkey.get_id().unwrap(), OID::DSA); + assert_eq!(pkey.oid(), Oid::DSA); assert!(pkey.rsa().is_err()); } @@ -579,7 +584,7 @@ mod tests { let dh = Dh::params_from_pem(dh).unwrap(); let pkey = PKey::from_dh(dh).unwrap(); pkey.dh().unwrap(); - assert_eq!(pkey.get_id().unwrap(), OID::DH); + assert_eq!(pkey.oid(), Oid::DH); assert!(pkey.rsa().is_err()); } @@ -588,7 +593,7 @@ mod tests { let ec_key = EcKey::from_curve_name(Nid::X9_62_PRIME256V1).unwrap(); let pkey = PKey::from_ec_key(ec_key).unwrap(); pkey.ec_key().unwrap(); - assert_eq!(pkey.get_id().unwrap(), OID::EC); + assert_eq!(pkey.oid(), Oid::EC); assert!(pkey.rsa().is_err()); } } From 55ffc9b2e415c3286c8d903b7c0284c834e4a13d Mon Sep 17 00:00:00 2001 From: Rohit Aggarwal Date: Thu, 8 Mar 2018 11:54:19 +0000 Subject: [PATCH 21/38] Add support LibreSSL and remove OpenSSL binding which we aren't using --- openssl-sys/src/lib.rs | 6 ++++++ openssl-sys/src/libressl/mod.rs | 6 ++++++ openssl-sys/src/ossl10x.rs | 20 -------------------- openssl-sys/src/ossl110.rs | 19 ------------------- 4 files changed, 12 insertions(+), 39 deletions(-) diff --git a/openssl-sys/src/lib.rs b/openssl-sys/src/lib.rs index 77f69188..4a73ff7f 100644 --- a/openssl-sys/src/lib.rs +++ b/openssl-sys/src/lib.rs @@ -1876,6 +1876,12 @@ extern "C" { ctx: *mut BN_CTX, ) -> c_int; + pub fn ECDSA_SIG_new() -> *mut ECDSA_SIG; + pub fn ECDSA_SIG_free(sig: *mut ECDSA_SIG); + pub fn ECDSA_do_verify(dgst: *const c_uchar, dgst_len: c_int, + sig: *const ECDSA_SIG, eckey: *mut ::EC_KEY) -> c_int; + pub fn ECDSA_do_sign(dgst: *const c_uchar, dgst_len: c_int, eckey: *mut ::EC_KEY) -> *mut ECDSA_SIG; + pub fn ERR_peek_last_error() -> c_ulong; pub fn ERR_get_error() -> c_ulong; pub fn ERR_get_error_line_data( diff --git a/openssl-sys/src/libressl/mod.rs b/openssl-sys/src/libressl/mod.rs index 8454a78a..c47363ca 100644 --- a/openssl-sys/src/libressl/mod.rs +++ b/openssl-sys/src/libressl/mod.rs @@ -133,6 +133,12 @@ pub struct DSA { pub engine: *mut ::ENGINE, } +#[repr(C)] +pub struct ECDSA_SIG { + pub r: *mut ::BIGNUM, + pub s: *mut ::BIGNUM +} + #[repr(C)] pub struct EVP_PKEY { pub type_: c_int, diff --git a/openssl-sys/src/ossl10x.rs b/openssl-sys/src/ossl10x.rs index 78c76b6e..09185160 100644 --- a/openssl-sys/src/ossl10x.rs +++ b/openssl-sys/src/ossl10x.rs @@ -829,26 +829,6 @@ extern "C" { ); pub fn CRYPTO_set_id_callback(func: unsafe extern "C" fn() -> c_ulong); - pub fn ECDSA_SIG_new() -> *mut ECDSA_SIG; - pub fn ECDSA_SIG_free(sig: *mut ECDSA_SIG); - pub fn i2d_ECDSA_SIG(sig: *const ECDSA_SIG, pp: *mut *mut c_uchar) -> c_int; - pub fn d2i_ECDSA_SIG(sig: *mut *mut ECDSA_SIG, pp: *mut *const c_uchar, len: c_long) -> *mut ECDSA_SIG; - pub fn ECDSA_size(eckey: *const ::EC_KEY) -> c_int; - pub fn ECDSA_sign(_type: c_int, dgst: *const c_uchar, dgstlen: c_int, - sig: *mut c_uchar, siglen: *mut c_uint, eckey: *mut ::EC_KEY) -> c_int; - pub fn ECDSA_do_sign(dgst: *const c_uchar, dgst_len: c_int, eckey: *mut ::EC_KEY) -> *mut ECDSA_SIG; - pub fn ECDSA_verify(_type: c_int, dgst: *const c_uchar, dgstlen: c_int, - sig: *const c_uchar, siglen: c_int, eckey: *mut ::EC_KEY) -> c_int; - pub fn ECDSA_do_verify(dgst: *const c_uchar, dgst_len: c_int, - sig: *const ECDSA_SIG, eckey: *mut ::EC_KEY) -> c_int; - pub fn ECDSA_do_sign_ex(dgst: *const c_uchar, dgstlen: c_int, - kinv: *const BIGNUM, rp: *const BIGNUM, - eckey: *mut ::EC_KEY) -> *mut ECDSA_SIG; - pub fn ECDSA_sign_setup(eckey: *mut ::EC_KEY, ctx: *mut ::BN_CTX, kinv: *mut *mut BIGNUM, rp: *mut *mut BIGNUM) -> c_int; - pub fn ECDSA_sign_ex(_type: c_int, dgst: *const c_uchar, dgstlen: c_int, - sig: *mut c_uchar, siglen: *mut c_uint, - kinv: *const BIGNUM, rp: *const BIGNUM, eckey: *mut ::EC_KEY) -> c_int; - pub fn ERR_load_crypto_strings(); pub fn RSA_generate_key( diff --git a/openssl-sys/src/ossl110.rs b/openssl-sys/src/ossl110.rs index d6f4eb99..1902af75 100644 --- a/openssl-sys/src/ossl110.rs +++ b/openssl-sys/src/ossl110.rs @@ -365,25 +365,6 @@ extern "C" { pub fn X509_REQ_get_version(req: *const X509_REQ) -> c_long; pub fn X509_REQ_get_subject_name(req: *const X509_REQ) -> *mut ::X509_NAME; - pub fn ECDSA_SIG_new() -> *mut ECDSA_SIG; - pub fn ECDSA_SIG_free(sig: *mut ECDSA_SIG); pub fn ECDSA_SIG_get0(sig: *const ECDSA_SIG, pr: *mut *const BIGNUM, ps: *mut *const BIGNUM); pub fn ECDSA_SIG_set0(sig: *mut ECDSA_SIG, pr: *mut BIGNUM, ps: *mut BIGNUM) -> c_int; - pub fn i2d_ECDSA_SIG(sig: *const ECDSA_SIG, pp: *mut *mut c_uchar) -> c_int; - pub fn d2i_ECDSA_SIG(sig: *mut *mut ECDSA_SIG, pp: *mut *const c_uchar, len: c_long) -> *mut ECDSA_SIG; - pub fn ECDSA_size(eckey: *const ::EC_KEY) -> c_int; - pub fn ECDSA_sign(_type: c_int, dgst: *const c_uchar, dgstlen: c_int, - sig: *mut c_uchar, siglen: *mut c_uint, eckey: *mut ::EC_KEY) -> c_int; - pub fn ECDSA_do_sign(dgst: *const c_uchar, dgst_len: c_int, eckey: *mut ::EC_KEY) -> *mut ECDSA_SIG; - pub fn ECDSA_verify(_type: c_int, dgst: *const c_uchar, dgstlen: c_int, - sig: *const c_uchar, siglen: c_int, eckey: *mut ::EC_KEY) -> c_int; - pub fn ECDSA_do_verify(dgst: *const c_uchar, dgst_len: c_int, - sig: *const ECDSA_SIG, eckey: *mut ::EC_KEY) -> c_int; - pub fn ECDSA_do_sign_ex(dgst: *const c_uchar, dgstlen: c_int, - kinv: *const BIGNUM, rp: *const BIGNUM, - eckey: *mut ::EC_KEY) -> *mut ECDSA_SIG; - pub fn ECDSA_sign_setup(eckey: *mut ::EC_KEY, ctx: *mut ::BN_CTX, kinv: *mut *mut BIGNUM, rp: *mut *mut BIGNUM) -> c_int; - pub fn ECDSA_sign_ex(_type: c_int, dgst: *const c_uchar, dgstlen: c_int, - sig: *mut c_uchar, siglen: *mut c_uint, - kinv: *const BIGNUM, rp: *const BIGNUM, eckey: *mut ::EC_KEY) -> c_int; } From a5ba1a00077f1eb91db34ca6109a935661bd41a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 8 Mar 2018 16:17:32 +0100 Subject: [PATCH 22/38] Adds `RsaPssSaltlen` enum to encode the special values --- openssl/src/sign.rs | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/openssl/src/sign.rs b/openssl/src/sign.rs index 9de9bcf7..488e7291 100644 --- a/openssl/src/sign.rs +++ b/openssl/src/sign.rs @@ -79,6 +79,29 @@ use ffi::{EVP_MD_CTX_free, EVP_MD_CTX_new}; #[cfg(any(ossl101, ossl102))] use ffi::{EVP_MD_CTX_create as EVP_MD_CTX_new, EVP_MD_CTX_destroy as EVP_MD_CTX_free}; +/// Salt lengths that must be used with `set_rsa_pss_saltlen`. +pub enum RsaPssSaltlen { + /// The salt length is set to the digest length. + /// Corresponds to the special value `-1`. + DigestLength, + /// The salt length is set to the maximum permissible value. + /// Corresponds to the special value `-2`. + MaximumLength, + /// Sets the salt length to the given value. + Custom(u32) +} + +impl RsaPssSaltlen { + /// Returns the integer representation of `Oid`. + fn as_raw(&self) -> c_int { + match *self { + RsaPssSaltlen::DigestLength => -1, + RsaPssSaltlen::MaximumLength => -2, + RsaPssSaltlen::Custom(val) => val as c_int, + } + } +} + /// A type which computes cryptographic signatures of data. pub struct Signer<'a> { md_ctx: *mut ffi::EVP_MD_CTX, @@ -168,11 +191,11 @@ impl<'a> Signer<'a> { /// This corresponds to [`EVP_PKEY_CTX_set_rsa_pss_saltlen`]. /// /// [`EVP_PKEY_CTX_set_rsa_pss_saltlen`]: https://www.openssl.org/docs/man1.1.0/crypto/EVP_PKEY_CTX_set_rsa_pss_saltlen.html - pub fn set_rsa_pss_saltlen(&mut self, len: c_int) -> Result<(), ErrorStack> { + pub fn set_rsa_pss_saltlen(&mut self, len: RsaPssSaltlen) -> Result<(), ErrorStack> { unsafe { cvt(ffi::EVP_PKEY_CTX_set_rsa_pss_saltlen( self.pctx, - len, + len.as_raw(), )).map(|_| ()) } } @@ -360,11 +383,11 @@ impl<'a> Verifier<'a> { /// This corresponds to [`EVP_PKEY_CTX_set_rsa_pss_saltlen`]. /// /// [`EVP_PKEY_CTX_set_rsa_pss_saltlen`]: https://www.openssl.org/docs/man1.1.0/crypto/EVP_PKEY_CTX_set_rsa_pss_saltlen.html - pub fn set_rsa_pss_saltlen(&mut self, len: c_int) -> Result<(), ErrorStack> { + pub fn set_rsa_pss_saltlen(&mut self, len: RsaPssSaltlen) -> Result<(), ErrorStack> { unsafe { cvt(ffi::EVP_PKEY_CTX_set_rsa_pss_saltlen( self.pctx, - len, + len.as_raw(), )).map(|_| ()) } } @@ -451,7 +474,7 @@ mod test { use std::iter; use hash::MessageDigest; - use sign::{Signer, Verifier}; + use sign::{Signer, Verifier, RsaPssSaltlen}; use ec::{EcGroup, EcKey}; use nid::Nid; use rsa::{Padding, Rsa}; @@ -634,14 +657,14 @@ mod test { let mut signer = Signer::new(MessageDigest::sha256(), &pkey).unwrap(); signer.set_rsa_padding(Padding::PKCS1_PSS).unwrap(); assert_eq!(signer.rsa_padding().unwrap(), Padding::PKCS1_PSS); - signer.set_rsa_pss_saltlen(-1).unwrap(); + signer.set_rsa_pss_saltlen(RsaPssSaltlen::DigestLength).unwrap(); signer.set_rsa_mgf1_md(MessageDigest::sha256()).unwrap(); signer.update(&Vec::from_hex(INPUT).unwrap()).unwrap(); let signature = signer.sign_to_vec().unwrap(); let mut verifier = Verifier::new(MessageDigest::sha256(), &pkey).unwrap(); verifier.set_rsa_padding(Padding::PKCS1_PSS).unwrap(); - verifier.set_rsa_pss_saltlen(-1).unwrap(); + verifier.set_rsa_pss_saltlen(RsaPssSaltlen::DigestLength).unwrap(); verifier.set_rsa_mgf1_md(MessageDigest::sha256()).unwrap(); verifier.update(&Vec::from_hex(INPUT).unwrap()).unwrap(); assert!(verifier.verify(&signature).unwrap()); From d4de2a408f578e6f974468dc448c0c63030087e0 Mon Sep 17 00:00:00 2001 From: Rohit Aggarwal Date: Thu, 8 Mar 2018 16:12:35 +0000 Subject: [PATCH 23/38] Use examples listed in OpenSSL docs for testing --- openssl-sys/src/lib.rs | 4 ++-- openssl/src/ecdsa.rs | 30 ++++++++++++++++++++---------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/openssl-sys/src/lib.rs b/openssl-sys/src/lib.rs index 4a73ff7f..564c7eae 100644 --- a/openssl-sys/src/lib.rs +++ b/openssl-sys/src/lib.rs @@ -1879,8 +1879,8 @@ extern "C" { pub fn ECDSA_SIG_new() -> *mut ECDSA_SIG; pub fn ECDSA_SIG_free(sig: *mut ECDSA_SIG); pub fn ECDSA_do_verify(dgst: *const c_uchar, dgst_len: c_int, - sig: *const ECDSA_SIG, eckey: *mut ::EC_KEY) -> c_int; - pub fn ECDSA_do_sign(dgst: *const c_uchar, dgst_len: c_int, eckey: *mut ::EC_KEY) -> *mut ECDSA_SIG; + sig: *const ECDSA_SIG, eckey: *mut EC_KEY) -> c_int; + pub fn ECDSA_do_sign(dgst: *const c_uchar, dgst_len: c_int, eckey: *mut EC_KEY) -> *mut ECDSA_SIG; pub fn ERR_peek_last_error() -> c_ulong; pub fn ERR_get_error() -> c_ulong; diff --git a/openssl/src/ecdsa.rs b/openssl/src/ecdsa.rs index 50cb0b62..a8f6cbbd 100644 --- a/openssl/src/ecdsa.rs +++ b/openssl/src/ecdsa.rs @@ -1,5 +1,5 @@ //! Low level Elliptic Curve Digital Signature Algorithm (ECDSA) functions. -//! +//! use bn::{BigNum, BigNumRef}; @@ -127,6 +127,16 @@ mod test { use ec::EcGroup; use super::*; + #[cfg(ossl10x)] + static CURVE_IDENTIFER: Nid = Nid::SECP192K1; + #[cfg(ossl10x)] + static DGST_LEN: i32 = 20; + + #[cfg(ossl110)] + static CURVE_IDENTIFER: Nid = Nid::X9_62_PRIME256V1; + #[cfg(ossl110)] + static DGST_LEN: i32 = 32; + fn get_public_key(group: &EcGroup, x: &EcKey) -> Result, ErrorStack> { let public_key_point = x.public_key(); Ok(EcKey::from_public_key(group, public_key_point)?) @@ -134,7 +144,7 @@ mod test { #[test] fn sign_and_verify() { - let group = EcGroup::from_curve_name(Nid::X9_62_PRIME256V1).unwrap(); + let group = EcGroup::from_curve_name(CURVE_IDENTIFER).unwrap(); let private_key = EcKey::generate(&group).unwrap(); let public_key = get_public_key(&group, &private_key).unwrap(); @@ -142,30 +152,30 @@ mod test { let public_key2 = get_public_key(&group, &private_key2).unwrap(); let data = String::from("hello"); - let res = EcdsaSig::sign(data.as_bytes(), 32, &private_key).unwrap(); + let res = EcdsaSig::sign(data.as_bytes(), DGST_LEN, &private_key).unwrap(); // Signature can be verified using the correct data & correct public key - let verification = res.verify(data.as_bytes(), 32, &public_key).unwrap(); + let verification = res.verify(data.as_bytes(), DGST_LEN, &public_key).unwrap(); assert!(verification); // Signature will not be verified using the incorrect data but the correct public key - let verification2 = res.verify(String::from("hello2").as_bytes(), 32, &public_key).unwrap(); + let verification2 = res.verify(String::from("hello2").as_bytes(), DGST_LEN, &public_key).unwrap(); assert!(verification2 == false); // Signature will not be verified using the correct data but the incorrect public key - let verification3 = res.verify(data.as_bytes(), 32, &public_key2).unwrap(); + let verification3 = res.verify(data.as_bytes(), DGST_LEN, &public_key2).unwrap(); assert!(verification3 == false); } #[test] fn check_private_components() { - let group = EcGroup::from_curve_name(Nid::X9_62_PRIME256V1).unwrap(); + let group = EcGroup::from_curve_name(CURVE_IDENTIFER).unwrap(); let private_key = EcKey::generate(&group).unwrap(); let public_key = get_public_key(&group, &private_key).unwrap(); let data = String::from("hello"); - let res = EcdsaSig::sign(data.as_bytes(), 32, &private_key).unwrap(); + let res = EcdsaSig::sign(data.as_bytes(), DGST_LEN, &private_key).unwrap(); - let verification = res.verify(data.as_bytes(), 32, &public_key).unwrap(); + let verification = res.verify(data.as_bytes(), DGST_LEN, &public_key).unwrap(); assert!(verification); let x = res.private_components(); @@ -173,7 +183,7 @@ mod test { let s = x.1.unwrap().to_owned().unwrap(); let res2 = EcdsaSig::from_private_components(r, s).unwrap(); - let verification2 = res2.verify(data.as_bytes(), 32, &public_key).unwrap(); + let verification2 = res2.verify(data.as_bytes(), DGST_LEN, &public_key).unwrap(); assert!(verification2); } } \ No newline at end of file From 4b4d3120180648fbed3581f1d9be3dab10199637 Mon Sep 17 00:00:00 2001 From: Rohit Aggarwal Date: Thu, 8 Mar 2018 16:46:31 +0000 Subject: [PATCH 24/38] Another try at using the correct curve --- openssl/src/ecdsa.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/openssl/src/ecdsa.rs b/openssl/src/ecdsa.rs index a8f6cbbd..10f5f893 100644 --- a/openssl/src/ecdsa.rs +++ b/openssl/src/ecdsa.rs @@ -127,16 +127,9 @@ mod test { use ec::EcGroup; use super::*; - #[cfg(ossl10x)] - static CURVE_IDENTIFER: Nid = Nid::SECP192K1; - #[cfg(ossl10x)] + static CURVE_IDENTIFER: Nid = Nid::X9_62_PRIME192V1; static DGST_LEN: i32 = 20; - #[cfg(ossl110)] - static CURVE_IDENTIFER: Nid = Nid::X9_62_PRIME256V1; - #[cfg(ossl110)] - static DGST_LEN: i32 = 32; - fn get_public_key(group: &EcGroup, x: &EcKey) -> Result, ErrorStack> { let public_key_point = x.public_key(); Ok(EcKey::from_public_key(group, public_key_point)?) From 0a38b5a9efcc1bf60f42f8925832cd07f1a159d3 Mon Sep 17 00:00:00 2001 From: Rohit Aggarwal Date: Thu, 8 Mar 2018 16:56:40 +0000 Subject: [PATCH 25/38] Try out another curve --- openssl/src/ecdsa.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openssl/src/ecdsa.rs b/openssl/src/ecdsa.rs index 10f5f893..f7386861 100644 --- a/openssl/src/ecdsa.rs +++ b/openssl/src/ecdsa.rs @@ -127,7 +127,7 @@ mod test { use ec::EcGroup; use super::*; - static CURVE_IDENTIFER: Nid = Nid::X9_62_PRIME192V1; + static CURVE_IDENTIFER: Nid = Nid::X9_62_C2TNB191V1; static DGST_LEN: i32 = 20; fn get_public_key(group: &EcGroup, x: &EcKey) -> Result, ErrorStack> { From 9e2755abae98ca4c48dff58e5d8c6fb42a093916 Mon Sep 17 00:00:00 2001 From: Rohit Aggarwal Date: Thu, 8 Mar 2018 17:10:09 +0000 Subject: [PATCH 26/38] Get curves for OpenSSL tests itself --- openssl/src/ecdsa.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/openssl/src/ecdsa.rs b/openssl/src/ecdsa.rs index f7386861..8ab24e91 100644 --- a/openssl/src/ecdsa.rs +++ b/openssl/src/ecdsa.rs @@ -127,9 +127,14 @@ mod test { use ec::EcGroup; use super::*; - static CURVE_IDENTIFER: Nid = Nid::X9_62_C2TNB191V1; static DGST_LEN: i32 = 20; + #[cfg(not(osslconf = "OPENSSL_NO_EC2M"))] + static CURVE_IDENTIFER: Nid = Nid::X9_62_PRIME192V1; + + #[cfg(osslconf = "OPENSSL_NO_EC2M")] + static CURVE_IDENTIFER: Nid = Nid::X9_62_C2TNB191V1; + fn get_public_key(group: &EcGroup, x: &EcKey) -> Result, ErrorStack> { let public_key_point = x.public_key(); Ok(EcKey::from_public_key(group, public_key_point)?) From 8461129456411bcfa738cc251cea2fd88e3fe1c9 Mon Sep 17 00:00:00 2001 From: Rohit Aggarwal Date: Thu, 8 Mar 2018 17:42:15 +0000 Subject: [PATCH 27/38] Changes as per PR feedback --- openssl/src/ecdsa.rs | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/openssl/src/ecdsa.rs b/openssl/src/ecdsa.rs index 8ab24e91..24f43431 100644 --- a/openssl/src/ecdsa.rs +++ b/openssl/src/ecdsa.rs @@ -4,7 +4,7 @@ use bn::{BigNum, BigNumRef}; use {cvt, cvt_n, cvt_p}; -use ec::EcKey; +use ec::EcKeyRef; use error::ErrorStack; use ffi; use foreign_types::{ForeignType, ForeignTypeRef}; @@ -36,7 +36,7 @@ impl EcdsaSig { /// OpenSSL documentation at [`ECDSA_do_sign`] /// /// [`ECDSA_do_sign`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_do_sign.html - pub fn sign(data: &[u8], dgstlen: i32, eckey: &EcKey) -> Result { + pub fn sign(data: &[u8], dgstlen: i32, eckey: &EcKeyRef) -> Result { unsafe { let sig = cvt_p(ffi::ECDSA_do_sign(data.as_ptr(), dgstlen, eckey.as_ptr()))?; Ok(EcdsaSig::from_ptr(sig as *mut _)) @@ -63,24 +63,36 @@ impl EcdsaSig { /// OpenSSL documentation at [`ECDSA_do_verify`] /// /// [`ECDSA_do_verify`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_do_verify.html - pub fn verify(&self, data: &[u8], dgstlen: i32, eckey: &EcKey) -> Result { + pub fn verify(&self, data: &[u8], dgstlen: i32, eckey: &EcKeyRef) -> Result { unsafe { let x = cvt_n(ffi::ECDSA_do_verify(data.as_ptr(), dgstlen, self.as_ptr(), eckey.as_ptr()))?; Ok(x == 1) } } - /// Returns internal components: `r` and `s` of a `EcdsaSig`. (See X9.62 or FIPS 186-2) + /// Returns internal component: `r` of a `EcdsaSig`. (See X9.62 or FIPS 186-2) /// /// OpenSSL documentation at [`ECDSA_SIG_get0`] /// /// [`ECDSA_SIG_get0`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_SIG_get0.html - pub fn private_components(&self) -> (Option<&BigNumRef>, Option<&BigNumRef>) { + pub fn private_component_r(&self) -> Option<&BigNumRef> { unsafe { let xs = compat::get_numbers(self.as_ptr()); let r = if xs[0].is_null() { None } else { Some(BigNumRef::from_ptr(xs[0] as *mut _)) }; + r + } + } + + /// Returns internal components: `s` of a `EcdsaSig`. (See X9.62 or FIPS 186-2) + /// + /// OpenSSL documentation at [`ECDSA_SIG_get0`] + /// + /// [`ECDSA_SIG_get0`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_SIG_get0.html + pub fn private_component_s(&self) -> Option<&BigNumRef> { + unsafe { + let xs = compat::get_numbers(self.as_ptr()); let s = if xs[1].is_null() { None } else { Some(BigNumRef::from_ptr(xs[1] as *mut _)) }; - (r, s) + s } } @@ -125,6 +137,7 @@ mod compat { mod test { use nid::Nid; use ec::EcGroup; + use ec::EcKey; use super::*; static DGST_LEN: i32 = 20; @@ -176,9 +189,8 @@ mod test { let verification = res.verify(data.as_bytes(), DGST_LEN, &public_key).unwrap(); assert!(verification); - let x = res.private_components(); - let r = x.0.unwrap().to_owned().unwrap(); - let s = x.1.unwrap().to_owned().unwrap(); + let r = res.private_component_r().unwrap().to_owned().unwrap(); + let s = res.private_component_s().unwrap().to_owned().unwrap(); let res2 = EcdsaSig::from_private_components(r, s).unwrap(); let verification2 = res2.verify(data.as_bytes(), DGST_LEN, &public_key).unwrap(); From 7ab650098cc31223600a13570f7849889bb54d4b Mon Sep 17 00:00:00 2001 From: Rohit Aggarwal Date: Fri, 9 Mar 2018 10:39:58 +0000 Subject: [PATCH 28/38] Remove unneeded paramter --- openssl/src/ecdsa.rs | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/openssl/src/ecdsa.rs b/openssl/src/ecdsa.rs index 24f43431..b220350c 100644 --- a/openssl/src/ecdsa.rs +++ b/openssl/src/ecdsa.rs @@ -30,15 +30,14 @@ foreign_type_and_impl_send_sync! { impl EcdsaSig { - /// Computes a digital signature of the `dgstlen` bytes hash value `data` using the private EC key eckey. - /// Some example values associated with `dgstlen` are: for SHA-1, it is 20; for SHA-256 it is 32 etc. + /// Computes a digital signature of the hash value `data` using the private EC key eckey. /// /// OpenSSL documentation at [`ECDSA_do_sign`] /// /// [`ECDSA_do_sign`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_do_sign.html - pub fn sign(data: &[u8], dgstlen: i32, eckey: &EcKeyRef) -> Result { + pub fn sign(data: &[u8], eckey: &EcKeyRef) -> Result { unsafe { - let sig = cvt_p(ffi::ECDSA_do_sign(data.as_ptr(), dgstlen, eckey.as_ptr()))?; + let sig = cvt_p(ffi::ECDSA_do_sign(data.as_ptr(), data.len() as i32, eckey.as_ptr()))?; Ok(EcdsaSig::from_ptr(sig as *mut _)) } } @@ -63,9 +62,9 @@ impl EcdsaSig { /// OpenSSL documentation at [`ECDSA_do_verify`] /// /// [`ECDSA_do_verify`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_do_verify.html - pub fn verify(&self, data: &[u8], dgstlen: i32, eckey: &EcKeyRef) -> Result { + pub fn verify(&self, data: &[u8], eckey: &EcKeyRef) -> Result { unsafe { - let x = cvt_n(ffi::ECDSA_do_verify(data.as_ptr(), dgstlen, self.as_ptr(), eckey.as_ptr()))?; + let x = cvt_n(ffi::ECDSA_do_verify(data.as_ptr(), data.len() as i32, self.as_ptr(), eckey.as_ptr()))?; Ok(x == 1) } } @@ -140,8 +139,6 @@ mod test { use ec::EcKey; use super::*; - static DGST_LEN: i32 = 20; - #[cfg(not(osslconf = "OPENSSL_NO_EC2M"))] static CURVE_IDENTIFER: Nid = Nid::X9_62_PRIME192V1; @@ -163,18 +160,18 @@ mod test { let public_key2 = get_public_key(&group, &private_key2).unwrap(); let data = String::from("hello"); - let res = EcdsaSig::sign(data.as_bytes(), DGST_LEN, &private_key).unwrap(); + let res = EcdsaSig::sign(data.as_bytes(), &private_key).unwrap(); // Signature can be verified using the correct data & correct public key - let verification = res.verify(data.as_bytes(), DGST_LEN, &public_key).unwrap(); + let verification = res.verify(data.as_bytes(), &public_key).unwrap(); assert!(verification); // Signature will not be verified using the incorrect data but the correct public key - let verification2 = res.verify(String::from("hello2").as_bytes(), DGST_LEN, &public_key).unwrap(); + let verification2 = res.verify(String::from("hello2").as_bytes(), &public_key).unwrap(); assert!(verification2 == false); // Signature will not be verified using the correct data but the incorrect public key - let verification3 = res.verify(data.as_bytes(), DGST_LEN, &public_key2).unwrap(); + let verification3 = res.verify(data.as_bytes(), &public_key2).unwrap(); assert!(verification3 == false); } @@ -184,16 +181,16 @@ mod test { let private_key = EcKey::generate(&group).unwrap(); let public_key = get_public_key(&group, &private_key).unwrap(); let data = String::from("hello"); - let res = EcdsaSig::sign(data.as_bytes(), DGST_LEN, &private_key).unwrap(); + let res = EcdsaSig::sign(data.as_bytes(), &private_key).unwrap(); - let verification = res.verify(data.as_bytes(), DGST_LEN, &public_key).unwrap(); + let verification = res.verify(data.as_bytes(), &public_key).unwrap(); assert!(verification); let r = res.private_component_r().unwrap().to_owned().unwrap(); let s = res.private_component_s().unwrap().to_owned().unwrap(); let res2 = EcdsaSig::from_private_components(r, s).unwrap(); - let verification2 = res2.verify(data.as_bytes(), DGST_LEN, &public_key).unwrap(); + let verification2 = res2.verify(data.as_bytes(), &public_key).unwrap(); assert!(verification2); } } \ No newline at end of file From a5d7f8a718bc16d8c7986ea13b8585af8f20a648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sat, 10 Mar 2018 00:15:03 +0100 Subject: [PATCH 29/38] Moves store context init into its own function --- openssl/src/x509/mod.rs | 28 ++++++++++++++++++---------- openssl/src/x509/tests.rs | 9 ++++++--- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 107bfe69..55e5c75d 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -107,30 +107,38 @@ impl X509StoreContextRef { unsafe { X509VerifyResult::from_raw(ffi::X509_STORE_CTX_get_error(self.as_ptr())) } } - /// Verifies a certificate with the given certificate store. + /// Initializes this context with the given certificate, certificates chain and certificate + /// store. /// For successive calls to this function, it is required to call `cleanup` in beforehand. /// /// * `trust` - The certificate store with the trusted certificates. /// * `cert` - The certificate that should be verified. /// * `cert_chain` - The certificates chain. /// - /// This corresponds to [`X509_STORE_CTX_init`] followed by [`X509_verify_cert`]. + /// This corresponds to [`X509_STORE_CTX_init`]. /// /// [`X509_STORE_CTX_init`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_init.html + pub fn init(&mut self, trust: &store::X509StoreRef, cert: &X509Ref, + cert_chain: &StackRef) -> Result<(), ErrorStack> { + unsafe { + cvt(ffi::X509_STORE_CTX_init(self.as_ptr(), trust.as_ptr(), + cert.as_ptr(), cert_chain.as_ptr())).map(|_| ()) + } + } + + /// Verifies the stored certificate. + /// It is required to call `init` in beforehand, to initialize the required values. + /// + /// This corresponds to [`X509_verify_cert`]. + /// /// [`X509_verify_cert`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_verify_cert.html /// /// # Result /// /// The Result must be `Ok(())` to be a valid certificate, otherwise the cert is not valid. - pub fn verify_cert(&mut self, trust: &store::X509StoreRef, cert: &X509Ref, - cert_chain: &StackRef) -> Result<(), ErrorStack> { + pub fn verify_cert(&mut self) -> Result<(), ErrorStack> { unsafe { - cvt(ffi::X509_STORE_CTX_init(self.as_ptr(), trust.as_ptr(), - cert.as_ptr(), cert_chain.as_ptr()))?; - - cvt(ffi::X509_verify_cert(self.as_ptr()))?; - - Ok(()) + cvt(ffi::X509_verify_cert(self.as_ptr())).map(|_| ()) } } diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 8e89c3a0..9b630d0e 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -306,9 +306,11 @@ fn test_verify_cert() { let store = store_bldr.build(); let mut context = X509StoreContext::new().unwrap(); - assert!(context.verify_cert(&store, &cert, &chain).is_ok()); + assert!(context.init(&store, &cert, &chain).is_ok()); + assert!(context.verify_cert().is_ok()); context.cleanup(); - assert!(context.verify_cert(&store, &cert, &chain).is_ok()); + assert!(context.init(&store, &cert, &chain).is_ok()); + assert!(context.verify_cert().is_ok()); } #[test] @@ -324,5 +326,6 @@ fn test_verify_fails() { let store = store_bldr.build(); let mut context = X509StoreContext::new().unwrap(); - assert!(context.verify_cert(&store, &cert, &chain).is_err()); + assert!(context.init(&store, &cert, &chain).is_ok()); + assert!(context.verify_cert().is_err()); } From 7fe3fabf245a1e65ad8441dc8c00f1b3ab28d70d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sat, 10 Mar 2018 00:26:20 +0100 Subject: [PATCH 30/38] Switches to new type wrapper for RsaPssSaltlen --- openssl/src/pkey.rs | 3 +-- openssl/src/sign.rs | 35 +++++++++++++++++------------------ 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/openssl/src/pkey.rs b/openssl/src/pkey.rs index 76ecef7c..506c6db7 100644 --- a/openssl/src/pkey.rs +++ b/openssl/src/pkey.rs @@ -178,8 +178,7 @@ impl PKeyRef { } } - /// Returns `Some(Oid)` as the type of this key or `None`, if the Oid is supported by this - /// library. + /// Returns the Oid that represents the type of this key. /// /// This corresponds to [`EVP_PKEY_id`]. /// diff --git a/openssl/src/sign.rs b/openssl/src/sign.rs index 488e7291..d4c36cad 100644 --- a/openssl/src/sign.rs +++ b/openssl/src/sign.rs @@ -80,26 +80,25 @@ use ffi::{EVP_MD_CTX_free, EVP_MD_CTX_new}; use ffi::{EVP_MD_CTX_create as EVP_MD_CTX_new, EVP_MD_CTX_destroy as EVP_MD_CTX_free}; /// Salt lengths that must be used with `set_rsa_pss_saltlen`. -pub enum RsaPssSaltlen { - /// The salt length is set to the digest length. - /// Corresponds to the special value `-1`. - DigestLength, - /// The salt length is set to the maximum permissible value. - /// Corresponds to the special value `-2`. - MaximumLength, - /// Sets the salt length to the given value. - Custom(u32) -} +pub struct RsaPssSaltlen(c_int); impl RsaPssSaltlen { - /// Returns the integer representation of `Oid`. + /// Returns the integer representation of `RsaPssSaltlen`. fn as_raw(&self) -> c_int { - match *self { - RsaPssSaltlen::DigestLength => -1, - RsaPssSaltlen::MaximumLength => -2, - RsaPssSaltlen::Custom(val) => val as c_int, - } + self.0 } + + /// Sets the salt length to the given value. + pub fn custom(val: c_int) -> RsaPssSaltlen { + RsaPssSaltlen(val) + } + + /// The salt length is set to the digest length. + /// Corresponds to the special value `-1`. + pub const DIGEST_LENGTH: RsaPssSaltlen = RsaPssSaltlen(-1); + /// The salt length is set to the maximum permissible value. + /// Corresponds to the special value `-2`. + pub const MAXIMUM_LENGTH: RsaPssSaltlen = RsaPssSaltlen(-2); } /// A type which computes cryptographic signatures of data. @@ -657,14 +656,14 @@ mod test { let mut signer = Signer::new(MessageDigest::sha256(), &pkey).unwrap(); signer.set_rsa_padding(Padding::PKCS1_PSS).unwrap(); assert_eq!(signer.rsa_padding().unwrap(), Padding::PKCS1_PSS); - signer.set_rsa_pss_saltlen(RsaPssSaltlen::DigestLength).unwrap(); + signer.set_rsa_pss_saltlen(RsaPssSaltlen::DIGEST_LENGTH).unwrap(); signer.set_rsa_mgf1_md(MessageDigest::sha256()).unwrap(); signer.update(&Vec::from_hex(INPUT).unwrap()).unwrap(); let signature = signer.sign_to_vec().unwrap(); let mut verifier = Verifier::new(MessageDigest::sha256(), &pkey).unwrap(); verifier.set_rsa_padding(Padding::PKCS1_PSS).unwrap(); - verifier.set_rsa_pss_saltlen(RsaPssSaltlen::DigestLength).unwrap(); + verifier.set_rsa_pss_saltlen(RsaPssSaltlen::DIGEST_LENGTH).unwrap(); verifier.set_rsa_mgf1_md(MessageDigest::sha256()).unwrap(); verifier.update(&Vec::from_hex(INPUT).unwrap()).unwrap(); assert!(verifier.verify(&signature).unwrap()); From 245f5f3a11f9ef7a18131b0341d329089a29b3e1 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Fri, 9 Mar 2018 22:14:50 -0800 Subject: [PATCH 31/38] Impl Sync and Send for various types Closes #865 --- openssl/src/derive.rs | 3 +++ openssl/src/hash.rs | 6 ++++++ openssl/src/sign.rs | 6 ++++++ openssl/src/symm.rs | 7 ++++++- 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/openssl/src/derive.rs b/openssl/src/derive.rs index fb7bbf14..30d7dc25 100644 --- a/openssl/src/derive.rs +++ b/openssl/src/derive.rs @@ -11,6 +11,9 @@ use pkey::{HasPrivate, HasPublic, PKeyRef}; /// A type used to derive a shared secret between two keys. pub struct Deriver<'a>(*mut ffi::EVP_PKEY_CTX, PhantomData<&'a ()>); +unsafe impl<'a> Sync for Deriver<'a> {} +unsafe impl<'a> Send for Deriver<'a> {} + impl<'a> Deriver<'a> { /// Creates a new `Deriver` using the provided private key. /// diff --git a/openssl/src/hash.rs b/openssl/src/hash.rs index 103a7ae3..c6d4c862 100644 --- a/openssl/src/hash.rs +++ b/openssl/src/hash.rs @@ -49,6 +49,9 @@ impl MessageDigest { } } +unsafe impl Sync for MessageDigest {} +unsafe impl Send for MessageDigest {} + #[derive(PartialEq, Copy, Clone)] enum State { Reset, @@ -99,6 +102,9 @@ pub struct Hasher { state: State, } +unsafe impl Sync for Hasher {} +unsafe impl Send for Hasher {} + impl Hasher { /// Creates a new `Hasher` with the specified hash type. pub fn new(ty: MessageDigest) -> Result { diff --git a/openssl/src/sign.rs b/openssl/src/sign.rs index a61d883b..8911a69a 100644 --- a/openssl/src/sign.rs +++ b/openssl/src/sign.rs @@ -85,6 +85,9 @@ pub struct Signer<'a> { _p: PhantomData<&'a ()>, } +unsafe impl<'a> Sync for Signer<'a> {} +unsafe impl<'a> Send for Signer<'a> {} + impl<'a> Drop for Signer<'a> { fn drop(&mut self) { // pkey_ctx is owned by the md_ctx, so no need to explicitly free it. @@ -244,6 +247,9 @@ pub struct Verifier<'a> { pkey_pd: PhantomData<&'a ()>, } +unsafe impl<'a> Sync for Verifier<'a> {} +unsafe impl<'a> Send for Verifier<'a> {} + impl<'a> Drop for Verifier<'a> { fn drop(&mut self) { // pkey_ctx is owned by the md_ctx, so no need to explicitly free it. diff --git a/openssl/src/symm.rs b/openssl/src/symm.rs index 630f4ab6..1cb2ef81 100644 --- a/openssl/src/symm.rs +++ b/openssl/src/symm.rs @@ -223,6 +223,9 @@ impl Cipher { } } +unsafe impl Sync for Cipher {} +unsafe impl Send for Cipher {} + /// Represents a symmetric cipher context. /// /// Padding is enabled by default. @@ -288,6 +291,9 @@ pub struct Crypter { block_size: usize, } +unsafe impl Sync for Crypter {} +unsafe impl Send for Crypter {} + impl Crypter { /// Creates a new `Crypter`. The initialisation vector, `iv`, is not necesarry for certain /// types of `Cipher`. @@ -963,7 +969,6 @@ mod tests { #[test] fn test_des_ede3_cbc() { - let pt = "54686973206973206120746573742e"; let ct = "6f2867cfefda048a4046ef7e556c7132"; let key = "7cb66337f3d3c0fe7cb66337f3d3c0fe7cb66337f3d3c0fe"; From bc304565e740d0c093e902285501fe6790c1143e Mon Sep 17 00:00:00 2001 From: Rohit Aggarwal Date: Sat, 10 Mar 2018 16:29:54 +0000 Subject: [PATCH 32/38] Arguments should be BigNumRef and not BigNum --- openssl/src/ecdsa.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/openssl/src/ecdsa.rs b/openssl/src/ecdsa.rs index b220350c..2161ecbf 100644 --- a/openssl/src/ecdsa.rs +++ b/openssl/src/ecdsa.rs @@ -2,14 +2,13 @@ //! -use bn::{BigNum, BigNumRef}; +use bn::BigNumRef; use {cvt, cvt_n, cvt_p}; use ec::EcKeyRef; use error::ErrorStack; use ffi; use foreign_types::{ForeignType, ForeignTypeRef}; use pkey::{Private, Public}; -use std::mem; foreign_type_and_impl_send_sync! { @@ -48,11 +47,10 @@ impl EcdsaSig { /// OpenSSL documentation at [`ECDSA_SIG_set0`] /// /// [`ECDSA_SIG_set0`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_SIG_set0.html - pub fn from_private_components(r: BigNum, s: BigNum) -> Result { + pub fn from_private_components(r: &BigNumRef, s: &BigNumRef) -> Result { unsafe { let sig = cvt_p(ffi::ECDSA_SIG_new())?; cvt(compat::set_numbers(sig, r.as_ptr(), s.as_ptr()))?; - mem::forget((r, s)); Ok(EcdsaSig::from_ptr(sig as *mut _)) } } @@ -189,7 +187,7 @@ mod test { let r = res.private_component_r().unwrap().to_owned().unwrap(); let s = res.private_component_s().unwrap().to_owned().unwrap(); - let res2 = EcdsaSig::from_private_components(r, s).unwrap(); + let res2 = EcdsaSig::from_private_components(&r, &s).unwrap(); let verification2 = res2.verify(data.as_bytes(), &public_key).unwrap(); assert!(verification2); } From 562fe79f4c2c2df129c63eae40787486f63d4d47 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Sat, 10 Mar 2018 08:53:46 -0800 Subject: [PATCH 33/38] Add one more set of impls --- openssl/src/ssl/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openssl/src/ssl/mod.rs b/openssl/src/ssl/mod.rs index 5431df13..d7d98949 100644 --- a/openssl/src/ssl/mod.rs +++ b/openssl/src/ssl/mod.rs @@ -305,6 +305,9 @@ impl SslMethod { } } +unsafe impl Sync for SslMethod {} +unsafe impl Send for SslMethod {} + bitflags! { /// Options controling the behavior of certificate verification. pub struct SslVerifyMode: i32 { From 67640ed5991cbb5a57ae836658e36ef0e2750669 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Sat, 10 Mar 2018 16:26:01 -0800 Subject: [PATCH 34/38] Bump 1.1.1 to pre2 --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index af6536da..4363e376 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -92,7 +92,7 @@ macos_job: &MACOS_JOB openssl_111: &OPENSSL_111 LIBRARY: openssl - VERSION: 1.1.1-pre1 + VERSION: 1.1.1-pre2 openssl_110: &OPENSSL_110 LIBRARY: openssl VERSION: 1.1.0g From c0a4bc42021725d8ac3e2a7854c795c904eaa3d9 Mon Sep 17 00:00:00 2001 From: Rohit Aggarwal Date: Sun, 11 Mar 2018 07:41:22 +0000 Subject: [PATCH 35/38] Revert previous commit --- openssl/src/ecdsa.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/openssl/src/ecdsa.rs b/openssl/src/ecdsa.rs index 2161ecbf..b220350c 100644 --- a/openssl/src/ecdsa.rs +++ b/openssl/src/ecdsa.rs @@ -2,13 +2,14 @@ //! -use bn::BigNumRef; +use bn::{BigNum, BigNumRef}; use {cvt, cvt_n, cvt_p}; use ec::EcKeyRef; use error::ErrorStack; use ffi; use foreign_types::{ForeignType, ForeignTypeRef}; use pkey::{Private, Public}; +use std::mem; foreign_type_and_impl_send_sync! { @@ -47,10 +48,11 @@ impl EcdsaSig { /// OpenSSL documentation at [`ECDSA_SIG_set0`] /// /// [`ECDSA_SIG_set0`]: https://www.openssl.org/docs/man1.1.0/crypto/ECDSA_SIG_set0.html - pub fn from_private_components(r: &BigNumRef, s: &BigNumRef) -> Result { + pub fn from_private_components(r: BigNum, s: BigNum) -> Result { unsafe { let sig = cvt_p(ffi::ECDSA_SIG_new())?; cvt(compat::set_numbers(sig, r.as_ptr(), s.as_ptr()))?; + mem::forget((r, s)); Ok(EcdsaSig::from_ptr(sig as *mut _)) } } @@ -187,7 +189,7 @@ mod test { let r = res.private_component_r().unwrap().to_owned().unwrap(); let s = res.private_component_s().unwrap().to_owned().unwrap(); - let res2 = EcdsaSig::from_private_components(&r, &s).unwrap(); + let res2 = EcdsaSig::from_private_components(r, s).unwrap(); let verification2 = res2.verify(data.as_bytes(), &public_key).unwrap(); assert!(verification2); } From d7a7c379a85965fa2a49849d9e424c2c11035d89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sun, 11 Mar 2018 11:29:45 +0100 Subject: [PATCH 36/38] Changes `init` to take a closure which is called with the initialized context After calling the closure, we automatically cleanup the context. This is required, because otherwise we could have dangling references in the context. --- openssl/src/x509/mod.rs | 32 +++++++++++++++++++++++++------- openssl/src/x509/tests.rs | 10 +++------- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 55e5c75d..146a77b0 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -108,21 +108,39 @@ impl X509StoreContextRef { } /// Initializes this context with the given certificate, certificates chain and certificate - /// store. - /// For successive calls to this function, it is required to call `cleanup` in beforehand. + /// store. After initializing the context, the `with_context` closure is called with the prepared + /// context. As long as the closure is running, the context stays initialized and can be used + /// to e.g. verify a certificate. The context will be cleaned up, after the closure finished. /// /// * `trust` - The certificate store with the trusted certificates. /// * `cert` - The certificate that should be verified. /// * `cert_chain` - The certificates chain. + /// * `with_context` - The closure that is called with the initialized context. /// - /// This corresponds to [`X509_STORE_CTX_init`]. + /// This corresponds to [`X509_STORE_CTX_init`] before calling `with_context` and to + /// [`X509_STORE_CTX_cleanup`] after calling `with_context`. /// /// [`X509_STORE_CTX_init`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_init.html - pub fn init(&mut self, trust: &store::X509StoreRef, cert: &X509Ref, - cert_chain: &StackRef) -> Result<(), ErrorStack> { + /// [`X509_STORE_CTX_cleanup`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_cleanup.html + pub fn init(&mut self, trust: &store::X509StoreRef, cert: &X509Ref, + cert_chain: &StackRef, with_context: F) -> Result + where + F: FnOnce(&mut X509StoreContextRef) -> Result + { + struct Cleanup<'a>(&'a mut X509StoreContextRef); + + impl<'a> Drop for Cleanup<'a> { + fn drop(&mut self) { + self.0.cleanup(); + } + } + unsafe { cvt(ffi::X509_STORE_CTX_init(self.as_ptr(), trust.as_ptr(), - cert.as_ptr(), cert_chain.as_ptr())).map(|_| ()) + cert.as_ptr(), cert_chain.as_ptr()))?; + + let cleanup = Cleanup(self); + with_context(cleanup.0) } } @@ -147,7 +165,7 @@ impl X509StoreContextRef { /// This corresponds to [`X509_STORE_CTX_cleanup`]. /// /// [`X509_STORE_CTX_cleanup`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_cleanup.html - pub fn cleanup(&mut self) { + fn cleanup(&mut self) { unsafe { ffi::X509_STORE_CTX_cleanup(self.as_ptr()); } diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 9b630d0e..e3c726ae 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -306,11 +306,8 @@ fn test_verify_cert() { let store = store_bldr.build(); let mut context = X509StoreContext::new().unwrap(); - assert!(context.init(&store, &cert, &chain).is_ok()); - assert!(context.verify_cert().is_ok()); - context.cleanup(); - assert!(context.init(&store, &cert, &chain).is_ok()); - assert!(context.verify_cert().is_ok()); + assert!(context.init(&store, &cert, &chain, |c| c.verify_cert()).is_ok()); + assert!(context.init(&store, &cert, &chain, |c| c.verify_cert()).is_ok()); } #[test] @@ -326,6 +323,5 @@ fn test_verify_fails() { let store = store_bldr.build(); let mut context = X509StoreContext::new().unwrap(); - assert!(context.init(&store, &cert, &chain).is_ok()); - assert!(context.verify_cert().is_err()); + assert!(context.init(&store, &cert, &chain, |c| c.verify_cert()).is_err()); } From 40e59db37c63eab3977f6670b9e5fc23a1e2aad9 Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Sun, 11 Mar 2018 13:29:01 -0700 Subject: [PATCH 37/38] Rename Oid to Id --- openssl/src/pkey.rs | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/openssl/src/pkey.rs b/openssl/src/pkey.rs index 506c6db7..66ff33d6 100644 --- a/openssl/src/pkey.rs +++ b/openssl/src/pkey.rs @@ -70,26 +70,26 @@ pub enum Public {} /// A tag type indicating that a key has private components. pub enum Private {} -/// The Oids that identify the type of a key. +/// An identifier of a kind of key. #[derive(Debug, Copy, Clone, PartialEq, Eq)] -pub struct Oid(c_int); +pub struct Id(c_int); -impl Oid { - /// Creates a `Oid` from an integer representation. - pub fn from_raw(value: c_int) -> Oid { - Oid(value) +impl Id { + /// Creates a `Id` from an integer representation. + pub fn from_raw(value: c_int) -> Id { + Id(value) } - /// Returns the integer representation of `Oid`. + /// Returns the integer representation of the `Id`. pub fn as_raw(&self) -> c_int { self.0 } - pub const RSA: Oid = Oid(ffi::EVP_PKEY_RSA); - pub const HMAC: Oid = Oid(ffi::EVP_PKEY_HMAC); - pub const DSA: Oid = Oid(ffi::EVP_PKEY_DSA); - pub const DH: Oid = Oid(ffi::EVP_PKEY_DH); - pub const EC: Oid = Oid(ffi::EVP_PKEY_EC); + pub const RSA: Id = Id(ffi::EVP_PKEY_RSA); + pub const HMAC: Id = Id(ffi::EVP_PKEY_HMAC); + pub const DSA: Id = Id(ffi::EVP_PKEY_DSA); + pub const DH: Id = Id(ffi::EVP_PKEY_DH); + pub const EC: Id = Id(ffi::EVP_PKEY_EC); } /// A trait indicating that a key has parameters. @@ -178,14 +178,14 @@ impl PKeyRef { } } - /// Returns the Oid that represents the type of this key. + /// Returns the `Id` that represents the type of this key. /// /// This corresponds to [`EVP_PKEY_id`]. /// /// [`EVP_PKEY_id`]: https://www.openssl.org/docs/man1.1.0/crypto/EVP_PKEY_id.html - pub fn oid(&self) -> Oid { + pub fn id(&self) -> Id { unsafe { - Oid::from_raw(ffi::EVP_PKEY_id(self.as_ptr())) + Id::from_raw(ffi::EVP_PKEY_id(self.as_ptr())) } } } @@ -564,7 +564,7 @@ mod tests { let rsa = Rsa::generate(2048).unwrap(); let pkey = PKey::from_rsa(rsa).unwrap(); pkey.rsa().unwrap(); - assert_eq!(pkey.oid(), Oid::RSA); + assert_eq!(pkey.id(), Id::RSA); assert!(pkey.dsa().is_err()); } @@ -573,7 +573,7 @@ mod tests { let dsa = Dsa::generate(2048).unwrap(); let pkey = PKey::from_dsa(dsa).unwrap(); pkey.dsa().unwrap(); - assert_eq!(pkey.oid(), Oid::DSA); + assert_eq!(pkey.id(), Id::DSA); assert!(pkey.rsa().is_err()); } @@ -583,7 +583,7 @@ mod tests { let dh = Dh::params_from_pem(dh).unwrap(); let pkey = PKey::from_dh(dh).unwrap(); pkey.dh().unwrap(); - assert_eq!(pkey.oid(), Oid::DH); + assert_eq!(pkey.id(), Id::DH); assert!(pkey.rsa().is_err()); } @@ -592,7 +592,7 @@ mod tests { let ec_key = EcKey::from_curve_name(Nid::X9_62_PRIME256V1).unwrap(); let pkey = PKey::from_ec_key(ec_key).unwrap(); pkey.ec_key().unwrap(); - assert_eq!(pkey.oid(), Oid::EC); + assert_eq!(pkey.id(), Id::EC); assert!(pkey.rsa().is_err()); } } From 4ee7e0d3a9398cf9deb9f72fd978fd3cab86db0e Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Sun, 11 Mar 2018 14:06:57 -0700 Subject: [PATCH 38/38] Tweak verify_cert's signature The call can fail either due to an invalid cert or an internal error, and we should distinguish between the two. --- openssl/src/x509/mod.rs | 50 +++++++++++++++++++-------------------- openssl/src/x509/tests.rs | 26 ++++++++++++-------- 2 files changed, 40 insertions(+), 36 deletions(-) diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 146a77b0..ef4b57e5 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -122,22 +122,33 @@ impl X509StoreContextRef { /// /// [`X509_STORE_CTX_init`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_init.html /// [`X509_STORE_CTX_cleanup`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_cleanup.html - pub fn init(&mut self, trust: &store::X509StoreRef, cert: &X509Ref, - cert_chain: &StackRef, with_context: F) -> Result + pub fn init( + &mut self, + trust: &store::X509StoreRef, + cert: &X509Ref, + cert_chain: &StackRef, + with_context: F, + ) -> Result where - F: FnOnce(&mut X509StoreContextRef) -> Result + F: FnOnce(&mut X509StoreContextRef) -> Result, { struct Cleanup<'a>(&'a mut X509StoreContextRef); impl<'a> Drop for Cleanup<'a> { fn drop(&mut self) { - self.0.cleanup(); + unsafe { + ffi::X509_STORE_CTX_cleanup(self.0.as_ptr()); + } } } unsafe { - cvt(ffi::X509_STORE_CTX_init(self.as_ptr(), trust.as_ptr(), - cert.as_ptr(), cert_chain.as_ptr()))?; + cvt(ffi::X509_STORE_CTX_init( + self.as_ptr(), + trust.as_ptr(), + cert.as_ptr(), + cert_chain.as_ptr(), + ))?; let cleanup = Cleanup(self); with_context(cleanup.0) @@ -145,30 +156,17 @@ impl X509StoreContextRef { } /// Verifies the stored certificate. - /// It is required to call `init` in beforehand, to initialize the required values. + /// + /// Returns `true` if verification succeeds. The `error` method will return the specific + /// validation error if the certificate was not valid. + /// + /// This will only work inside of a call to `init`. /// /// This corresponds to [`X509_verify_cert`]. /// /// [`X509_verify_cert`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_verify_cert.html - /// - /// # Result - /// - /// The Result must be `Ok(())` to be a valid certificate, otherwise the cert is not valid. - pub fn verify_cert(&mut self) -> Result<(), ErrorStack> { - unsafe { - cvt(ffi::X509_verify_cert(self.as_ptr())).map(|_| ()) - } - } - - /// Cleans-up the context. - /// - /// This corresponds to [`X509_STORE_CTX_cleanup`]. - /// - /// [`X509_STORE_CTX_cleanup`]: https://www.openssl.org/docs/man1.0.2/crypto/X509_STORE_CTX_cleanup.html - fn cleanup(&mut self) { - unsafe { - ffi::X509_STORE_CTX_cleanup(self.as_ptr()); - } + pub fn verify_cert(&mut self) -> Result { + unsafe { cvt_n(ffi::X509_verify_cert(self.as_ptr())).map(|n| n != 0) } } /// Set the error code of the context. diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index e3c726ae..ecc7f7de 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -7,7 +7,7 @@ use nid::Nid; use pkey::{PKey, Private}; use rsa::Rsa; use stack::Stack; -use x509::{X509, X509Name, X509Req, X509VerifyResult, X509StoreContext}; +use x509::{X509, X509Name, X509Req, X509StoreContext, X509VerifyResult}; use x509::extension::{AuthorityKeyIdentifier, BasicConstraints, ExtendedKeyUsage, KeyUsage, SubjectAlternativeName, SubjectKeyIdentifier}; use x509::store::X509StoreBuilder; @@ -242,15 +242,11 @@ fn test_stack_from_pem() { assert_eq!(certs.len(), 2); assert_eq!( - hex::encode(certs[0] - .fingerprint(MessageDigest::sha1()) - .unwrap()), + hex::encode(certs[0].fingerprint(MessageDigest::sha1()).unwrap()), "59172d9313e84459bcff27f967e79e6e9217e584" ); assert_eq!( - hex::encode(certs[1] - .fingerprint(MessageDigest::sha1()) - .unwrap()), + hex::encode(certs[1].fingerprint(MessageDigest::sha1()).unwrap()), "c0cbdf7cdd03c9773e5468e1f6d2da7d5cbb1875" ); } @@ -306,8 +302,16 @@ fn test_verify_cert() { let store = store_bldr.build(); let mut context = X509StoreContext::new().unwrap(); - assert!(context.init(&store, &cert, &chain, |c| c.verify_cert()).is_ok()); - assert!(context.init(&store, &cert, &chain, |c| c.verify_cert()).is_ok()); + assert!( + context + .init(&store, &cert, &chain, |c| c.verify_cert()) + .unwrap() + ); + assert!( + context + .init(&store, &cert, &chain, |c| c.verify_cert()) + .unwrap() + ); } #[test] @@ -323,5 +327,7 @@ fn test_verify_fails() { let store = store_bldr.build(); let mut context = X509StoreContext::new().unwrap(); - assert!(context.init(&store, &cert, &chain, |c| c.verify_cert()).is_err()); + assert!(!context + .init(&store, &cert, &chain, |c| c.verify_cert()) + .unwrap()); }