From 6482f419b859c146467f06b63fb937dfab5f5caa Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Fri, 29 May 2020 21:34:20 -0700 Subject: [PATCH 1/5] Add Debug trait for X509 and other types. This currently leaves out at least two useful things: - The detailed SubjectPublicKeyInfo, e.g. the modulus of RSA keys. - Extensions. --- openssl/src/asn1.rs | 53 +++++++++++++++++++++++++++------- openssl/src/pkey.rs | 22 ++++++++++++++ openssl/src/stack.rs | 10 +++++++ openssl/src/x509/mod.rs | 60 +++++++++++++++++++++++++++++++++++++++ openssl/src/x509/tests.rs | 28 ++++++++++++++++++ 5 files changed, 162 insertions(+), 11 deletions(-) diff --git a/openssl/src/asn1.rs b/openssl/src/asn1.rs index 8c7f7b1a..43c92f1f 100644 --- a/openssl/src/asn1.rs +++ b/openssl/src/asn1.rs @@ -67,12 +67,16 @@ foreign_type_and_impl_send_sync! { impl fmt::Display for Asn1GeneralizedTimeRef { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { unsafe { - let mem_bio = MemBio::new()?; - cvt(ffi::ASN1_GENERALIZEDTIME_print( - mem_bio.as_ptr(), - self.as_ptr(), - ))?; - write!(f, "{}", str::from_utf8_unchecked(mem_bio.get_buf())) + match MemBio::new() { + Err(_) => f.write_str(""), + Ok(mem_bio) => match cvt(ffi::ASN1_GENERALIZEDTIME_print( + mem_bio.as_ptr(), + self.as_ptr(), + )) { + Err(_) => f.write_str(""), + Ok(_) => f.write_str(str::from_utf8_unchecked(mem_bio.get_buf())), + }, + } } } } @@ -207,13 +211,23 @@ impl<'a> PartialOrd for &'a Asn1TimeRef { impl fmt::Display for Asn1TimeRef { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { unsafe { - let mem_bio = MemBio::new()?; - cvt(ffi::ASN1_TIME_print(mem_bio.as_ptr(), self.as_ptr()))?; - write!(f, "{}", str::from_utf8_unchecked(mem_bio.get_buf())) + match MemBio::new() { + Err(_) => f.write_str("error"), + Ok(mem_bio) => match cvt(ffi::ASN1_TIME_print(mem_bio.as_ptr(), self.as_ptr())) { + Err(_) => f.write_str("error"), + Ok(_) => f.write_str(str::from_utf8_unchecked(mem_bio.get_buf())), + }, + } } } } +impl fmt::Debug for Asn1TimeRef { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str(&self.to_string()) + } +} + impl Asn1Time { fn new() -> Result { ffi::init(); @@ -389,6 +403,15 @@ impl Asn1StringRef { } } +impl fmt::Debug for Asn1StringRef { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + match self.as_utf8() { + Ok(openssl_string) => openssl_string.fmt(fmt), + Err(_) => fmt.write_str("error"), + } + } +} + foreign_type_and_impl_send_sync! { type CType = ffi::ASN1_INTEGER; fn drop = ffi::ASN1_INTEGER_free; @@ -527,12 +550,20 @@ impl fmt::Display for Asn1ObjectRef { self.as_ptr(), 0, ); - let s = str::from_utf8(&buf[..len as usize]).map_err(|_| fmt::Error)?; - fmt.write_str(s) + match str::from_utf8(&buf[..len as usize]) { + Err(_) => fmt.write_str("error"), + Ok(s) => fmt.write_str(s), + } } } } +impl fmt::Debug for Asn1ObjectRef { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + fmt.write_str(self.to_string().as_str()) + } +} + cfg_if! { if #[cfg(any(ossl110, libressl273))] { use ffi::ASN1_STRING_get0_data; diff --git a/openssl/src/pkey.rs b/openssl/src/pkey.rs index 2dfa95c6..7af13c09 100644 --- a/openssl/src/pkey.rs +++ b/openssl/src/pkey.rs @@ -49,6 +49,7 @@ use ffi; use foreign_types::{ForeignType, ForeignTypeRef}; use libc::{c_int, c_long}; use std::ffi::CString; +use std::fmt; use std::mem; use std::ptr; @@ -286,6 +287,27 @@ where } } +impl fmt::Debug for PKey { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + let alg = match self.id() { + Id::RSA => "RSA", + Id::HMAC => "HMAC", + Id::DSA => "DSA", + Id::DH => "DH", + Id::EC => "EC", + #[cfg(ossl111)] + Id::ED25519 => "Ed25519", + #[cfg(ossl111)] + Id::ED448 => "Ed448", + _ => "unknown", + }; + fmt.debug_struct("public_key") + .field("algorithm", &alg) + .finish() + // TODO: Print details for each specific type of key + } +} + impl Clone for PKey { fn clone(&self) -> PKey { PKeyRef::to_owned(self) diff --git a/openssl/src/stack.rs b/openssl/src/stack.rs index fc2eafae..985426d3 100644 --- a/openssl/src/stack.rs +++ b/openssl/src/stack.rs @@ -3,6 +3,7 @@ use foreign_types::{ForeignType, ForeignTypeRef, Opaque}; use libc::c_int; use std::borrow::Borrow; use std::convert::AsRef; +use std::fmt; use std::iter; use std::marker::PhantomData; use std::mem; @@ -43,6 +44,15 @@ pub struct Stack(*mut T::StackType); unsafe impl Send for Stack {} unsafe impl Sync for Stack {} +impl fmt::Debug for Stack +where + T: Stackable, + T::Ref: fmt::Debug, +{ + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + fmt.debug_list().entries(self).finish() + } +} impl Drop for Stack { fn drop(&mut self) { unsafe { diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 02063d0e..047102bd 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -671,6 +671,35 @@ impl Clone for X509 { } } +impl fmt::Debug for X509 { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + let serial = match &self.serial_number().to_bn() { + Ok(bn) => match bn.to_hex_str() { + Ok(hex) => hex.to_string(), + Err(_) => "".to_string(), + }, + Err(_) => "".to_string(), + }; + let mut debug_struct = formatter.debug_struct("X509"); + debug_struct.field("serial_number", &serial); + debug_struct.field("signature_algorithm", &self.signature_algorithm().object()); + debug_struct.field("issuer", &self.issuer_name()); + debug_struct.field("subject", &self.subject_name()); + if let Some(subject_alt_names) = &self.subject_alt_names() { + debug_struct.field("subject_alt_names", subject_alt_names); + } + debug_struct.field("not_before", &self.not_before()); + debug_struct.field("not_after", &self.not_after()); + + if let Ok(public_key) = &self.public_key() { + debug_struct.field("public_key", public_key); + }; + // TODO: Print extensions once they are supported on the X509 struct. + + debug_struct.finish() + } +} + impl AsRef for X509Ref { fn as_ref(&self) -> &X509Ref { self @@ -867,6 +896,12 @@ impl X509NameRef { } } +impl fmt::Debug for X509NameRef { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.debug_list().entries(self.entries()).finish() + } +} + /// A type to destructure and examine an `X509Name`. pub struct X509NameEntries<'a> { name: &'a X509NameRef, @@ -942,6 +977,12 @@ impl X509NameEntryRef { } } +impl fmt::Debug for X509NameEntryRef { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_fmt(format_args!("{:?} = {:?}", self.object(), self.data())) + } +} + /// A builder used to construct an `X509Req`. pub struct X509ReqBuilder(X509Req); @@ -1298,6 +1339,25 @@ impl GeneralNameRef { } } +impl fmt::Debug for GeneralNameRef { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + if let Some(email) = self.email() { + return formatter.write_str(email); + } + if let Some(dnsname) = self.dnsname() { + return formatter.write_str(dnsname); + } + if let Some(uri) = self.uri() { + return formatter.write_str(uri); + } + if let Some(ipaddress) = self.ipaddress() { + let result = String::from_utf8_lossy(ipaddress); + return formatter.write_str(&result); + } + Ok(()) + } +} + impl Stackable for GeneralName { type StackType = ffi::stack_st_GENERAL_NAME; } diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index a6baf45d..8db82fa5 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -31,6 +31,34 @@ fn test_cert_loading() { assert_eq!(hash_vec, &*fingerprint); } +#[test] +fn test_debug() { + let cert = include_bytes!("../../test/cert.pem"); + let cert = X509::from_pem(cert).unwrap(); + let debugged = format!("{:#?}", cert); + let expected = r#"X509 { + serial_number: "8771F7BDEE982FA5", + signature_algorithm: sha256WithRSAEncryption, + issuer: [ + countryName = "AU", + stateOrProvinceName = "Some-State", + organizationName = "Internet Widgits Pty Ltd", + ], + subject: [ + countryName = "AU", + stateOrProvinceName = "Some-State", + organizationName = "Internet Widgits Pty Ltd", + commonName = "foobar.com", + ], + not_before: Aug 14 17:00:03 2016 GMT, + not_after: Aug 12 17:00:03 2026 GMT, + public_key: public_key { + algorithm: "RSA", + }, +}"#; + assert_eq!(expected, debugged); +} + #[test] fn test_cert_issue_validity() { let cert = include_bytes!("../../test/cert.pem"); From 1aff5b9198ca510bbeed604a7c45c86c878a34ae Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Sun, 31 May 2020 20:03:37 -0700 Subject: [PATCH 2/5] Fixes in response to review feedback. --- openssl/src/asn1.rs | 30 +++++++++++++++++------------- openssl/src/pkey.rs | 4 +--- openssl/src/x509/mod.rs | 20 +++++++++----------- openssl/src/x509/tests.rs | 2 +- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/openssl/src/asn1.rs b/openssl/src/asn1.rs index 43c92f1f..2670ce77 100644 --- a/openssl/src/asn1.rs +++ b/openssl/src/asn1.rs @@ -67,15 +67,17 @@ foreign_type_and_impl_send_sync! { impl fmt::Display for Asn1GeneralizedTimeRef { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { unsafe { - match MemBio::new() { + let mem_bio = match MemBio::new() { + Err(_) => return f.write_str(""), + Ok(m) => m, + }; + let print_result = cvt(ffi::ASN1_GENERALIZEDTIME_print( + mem_bio.as_ptr(), + self.as_ptr(), + )); + match print_result { Err(_) => f.write_str(""), - Ok(mem_bio) => match cvt(ffi::ASN1_GENERALIZEDTIME_print( - mem_bio.as_ptr(), - self.as_ptr(), - )) { - Err(_) => f.write_str(""), - Ok(_) => f.write_str(str::from_utf8_unchecked(mem_bio.get_buf())), - }, + Ok(_) => f.write_str(str::from_utf8_unchecked(mem_bio.get_buf())), } } } @@ -211,12 +213,14 @@ impl<'a> PartialOrd for &'a Asn1TimeRef { impl fmt::Display for Asn1TimeRef { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { unsafe { - match MemBio::new() { + let mem_bio = match MemBio::new() { + Err(_) => return f.write_str("error"), + Ok(m) => m, + }; + let print_result = cvt(ffi::ASN1_TIME_print(mem_bio.as_ptr(), self.as_ptr())); + match print_result { Err(_) => f.write_str("error"), - Ok(mem_bio) => match cvt(ffi::ASN1_TIME_print(mem_bio.as_ptr(), self.as_ptr())) { - Err(_) => f.write_str("error"), - Ok(_) => f.write_str(str::from_utf8_unchecked(mem_bio.get_buf())), - }, + Ok(_) => f.write_str(str::from_utf8_unchecked(mem_bio.get_buf())), } } } diff --git a/openssl/src/pkey.rs b/openssl/src/pkey.rs index 7af13c09..c8be33ca 100644 --- a/openssl/src/pkey.rs +++ b/openssl/src/pkey.rs @@ -301,9 +301,7 @@ impl fmt::Debug for PKey { Id::ED448 => "Ed448", _ => "unknown", }; - fmt.debug_struct("public_key") - .field("algorithm", &alg) - .finish() + fmt.debug_struct("PKey").field("algorithm", &alg).finish() // TODO: Print details for each specific type of key } } diff --git a/openssl/src/x509/mod.rs b/openssl/src/x509/mod.rs index 047102bd..39190b7a 100644 --- a/openssl/src/x509/mod.rs +++ b/openssl/src/x509/mod.rs @@ -1342,19 +1342,17 @@ impl GeneralNameRef { impl fmt::Debug for GeneralNameRef { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { if let Some(email) = self.email() { - return formatter.write_str(email); - } - if let Some(dnsname) = self.dnsname() { - return formatter.write_str(dnsname); - } - if let Some(uri) = self.uri() { - return formatter.write_str(uri); - } - if let Some(ipaddress) = self.ipaddress() { + formatter.write_str(email) + } else if let Some(dnsname) = self.dnsname() { + formatter.write_str(dnsname) + } else if let Some(uri) = self.uri() { + formatter.write_str(uri) + } else if let Some(ipaddress) = self.ipaddress() { let result = String::from_utf8_lossy(ipaddress); - return formatter.write_str(&result); + formatter.write_str(&result) + } else { + formatter.write_str("(empty)") } - Ok(()) } } diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 8db82fa5..3e3650b9 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -52,7 +52,7 @@ fn test_debug() { ], not_before: Aug 14 17:00:03 2016 GMT, not_after: Aug 12 17:00:03 2026 GMT, - public_key: public_key { + public_key: PKey { algorithm: "RSA", }, }"#; From 01e229346cf66674556f6d6e9eb6f0c807f00907 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Mon, 1 Jun 2020 18:29:50 -0700 Subject: [PATCH 3/5] Write "error" when there is an error. --- openssl/src/asn1.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openssl/src/asn1.rs b/openssl/src/asn1.rs index 2670ce77..4140cd5e 100644 --- a/openssl/src/asn1.rs +++ b/openssl/src/asn1.rs @@ -68,7 +68,7 @@ impl fmt::Display for Asn1GeneralizedTimeRef { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { unsafe { let mem_bio = match MemBio::new() { - Err(_) => return f.write_str(""), + Err(_) => return f.write_str("error"), Ok(m) => m, }; let print_result = cvt(ffi::ASN1_GENERALIZEDTIME_print( @@ -76,7 +76,7 @@ impl fmt::Display for Asn1GeneralizedTimeRef { self.as_ptr(), )); match print_result { - Err(_) => f.write_str(""), + Err(_) => f.write_str("error"), Ok(_) => f.write_str(str::from_utf8_unchecked(mem_bio.get_buf())), } } From aedbe6537b90be9956ee23081b096c6ea283d592 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 3 Jun 2020 21:30:15 -0700 Subject: [PATCH 4/5] Make tests narrower. Evidently the behavior on different platforms is different with regards to whether the final element in a list gets a comma or not, so we can't do a fully-string comparison of the debug output. --- openssl/src/x509/tests.rs | 27 ++++++--------------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/openssl/src/x509/tests.rs b/openssl/src/x509/tests.rs index 3e3650b9..74b6aff0 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -36,27 +36,12 @@ fn test_debug() { let cert = include_bytes!("../../test/cert.pem"); let cert = X509::from_pem(cert).unwrap(); let debugged = format!("{:#?}", cert); - let expected = r#"X509 { - serial_number: "8771F7BDEE982FA5", - signature_algorithm: sha256WithRSAEncryption, - issuer: [ - countryName = "AU", - stateOrProvinceName = "Some-State", - organizationName = "Internet Widgits Pty Ltd", - ], - subject: [ - countryName = "AU", - stateOrProvinceName = "Some-State", - organizationName = "Internet Widgits Pty Ltd", - commonName = "foobar.com", - ], - not_before: Aug 14 17:00:03 2016 GMT, - not_after: Aug 12 17:00:03 2026 GMT, - public_key: PKey { - algorithm: "RSA", - }, -}"#; - assert_eq!(expected, debugged); + assert!(debugged.contains(r#"serial_number: "8771F7BDEE982FA5""#)); + assert!(debugged.contains(r#"signature_algorithm: sha256WithRSAEncryption"#)); + assert!(debugged.contains(r#"countryName = "AU""#)); + assert!(debugged.contains(r#"stateOrProvinceName = Some-State"#)); + assert!(debugged.contains(r#"not_before: Aug 14 17:00:03 2016 GMT"#)); + assert!(debugged.contains(r#"not_after: Aug 12 17:00:03 2026 GMT"#)); } #[test] From cbfdaa516d947bf9a78c8c20bf955dc0e94d9905 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 3 Jun 2020 23:32:03 -0700 Subject: [PATCH 5/5] Fix test. --- 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 74b6aff0..1f636b30 100644 --- a/openssl/src/x509/tests.rs +++ b/openssl/src/x509/tests.rs @@ -39,7 +39,7 @@ fn test_debug() { assert!(debugged.contains(r#"serial_number: "8771F7BDEE982FA5""#)); assert!(debugged.contains(r#"signature_algorithm: sha256WithRSAEncryption"#)); assert!(debugged.contains(r#"countryName = "AU""#)); - assert!(debugged.contains(r#"stateOrProvinceName = Some-State"#)); + assert!(debugged.contains(r#"stateOrProvinceName = "Some-State""#)); assert!(debugged.contains(r#"not_before: Aug 14 17:00:03 2016 GMT"#)); assert!(debugged.contains(r#"not_after: Aug 12 17:00:03 2026 GMT"#)); }