From 7a783aa8c5989aed803a4e73983f6802d6dea3c7 Mon Sep 17 00:00:00 2001 From: Arash Sahebolamri Date: Mon, 30 Oct 2023 13:28:33 -0700 Subject: [PATCH] Add x509 `verify` methods that accept `expected_common_name` --- mbedtls/src/alloc.rs | 41 +++++++++++++++ mbedtls/src/x509/certificate.rs | 93 +++++++++++++++++++++++++++++++-- 2 files changed, 130 insertions(+), 4 deletions(-) diff --git a/mbedtls/src/alloc.rs b/mbedtls/src/alloc.rs index 552d9dcd5..8a73835eb 100644 --- a/mbedtls/src/alloc.rs +++ b/mbedtls/src/alloc.rs @@ -6,6 +6,7 @@ * option. This file may not be copied, modified, or distributed except * according to those terms. */ +use core::ffi::{c_char, CStr}; use core::fmt; use core::mem::ManuallyDrop; use core::ops::{Deref, DerefMut}; @@ -71,3 +72,43 @@ unsafe impl Sync for Box {} pub struct List { pub(crate) inner: Option>, } + +/// Modeled after std's [`CString`](https://doc.rust-lang.org/std/ffi/struct.CString.html) +pub struct CString { + /// Pointer to the allocated buffer + inner: NonNull, +} + +impl CString { + pub fn new(str: &str) -> Self { + unsafe { + let buff = crate::alloc::mbedtls_calloc(1, str.len() + 1) as *mut u8; + buff.copy_from(str.as_ptr(), str.len()); + *buff.add(str.len()) = 0; + Self { + inner: NonNull::new(buff).unwrap(), + } + } + } +} + +impl Drop for CString { + fn drop(&mut self) { + unsafe { crate::alloc::mbedtls_free(self.inner.as_ptr() as *mut c_void) } + } +} + +impl Deref for CString { + type Target = CStr; + + fn deref(&self) -> &Self::Target { + unsafe { CStr::from_ptr(self.inner.as_ptr() as *const c_char) } + } +} + +#[test] +fn test_c_string() { + let str = "spooky code here!"; + let c_str = CString::new(str); + assert_eq!(str, c_str.to_str().unwrap()) +} diff --git a/mbedtls/src/x509/certificate.rs b/mbedtls/src/x509/certificate.rs index 4b79104be..d49474285 100644 --- a/mbedtls/src/x509/certificate.rs +++ b/mbedtls/src/x509/certificate.rs @@ -13,7 +13,7 @@ use core::ptr::NonNull; use mbedtls_sys::types::raw_types::{c_char, c_void}; use mbedtls_sys::*; -use crate::alloc::{mbedtls_calloc, Box as MbedtlsBox, List as MbedtlsList}; +use crate::alloc::{mbedtls_calloc, Box as MbedtlsBox, CString, List as MbedtlsList}; #[cfg(not(feature = "std"))] use crate::alloc_prelude::*; use crate::error::{Error, IntoResult, Result}; @@ -224,6 +224,7 @@ impl Certificate { ca_crl: Option<&mut Crl>, err_info: Option<&mut String>, cb: Option, + expected_common_name: Option<&str>, ) -> Result<()> where F: VerifyCallback + 'static, @@ -236,13 +237,15 @@ impl Certificate { } else { (None, ::core::ptr::null_mut()) }; + + let cn = expected_common_name.map(|cn| CString::new(cn)); let mut flags = 0; let result = unsafe { x509_crt_verify( chain.inner_ffi_mut(), trust_ca.inner_ffi_mut(), ca_crl.map_or(::core::ptr::null_mut(), |crl| crl.handle_mut()), - ::core::ptr::null(), + cn.as_ref().map_or(::core::ptr::null(), |cn| cn.as_ptr()), &mut flags, f_vrfy, p_vrfy, @@ -250,6 +253,9 @@ impl Certificate { } .into_result(); + // Asserts cn is still alive here. Prevents bugs (e.g., forgetting to insert `.as_ref()` when using cn) + drop(cn); + if result.is_err() { if let Some(err_info) = err_info { let verify_info = crate::private::alloc_string_repeat(|buf, size| unsafe { @@ -270,7 +276,32 @@ impl Certificate { ca_crl: Option<&mut Crl>, err_info: Option<&mut String>, ) -> Result<()> { - Self::verify_ex(chain, trust_ca, ca_crl, err_info, None::<&dyn VerifyCallback>) + Self::verify_ex(chain, trust_ca, ca_crl, err_info, None::<&dyn VerifyCallback>, None) + } + + /// Like `verify`, optionally accepts an `expected_common_name` arg. + /// + /// * `expected_common_name` + /// (From mbedtls documentation) The expected Common Name. This will be checked to be present in the certificate’s + /// subjectAltNames extension or, if this extension is absent, as a CN component in its Subject name. DNS names + /// and IP addresses are fully supported, while the URI subtype is partially supported: only exact matching, + /// without any normalization procedures described in 7.4 of RFC5280, will result in a positive URI verification. + /// This may be `None` if the CN need not be verified. + pub fn verify_with_expected_common_name( + chain: &MbedtlsList, + trust_ca: &MbedtlsList, + ca_crl: Option<&mut Crl>, + err_info: Option<&mut String>, + expected_common_name: Option<&str>, + ) -> Result<()> { + Self::verify_ex( + chain, + trust_ca, + ca_crl, + err_info, + None::<&dyn VerifyCallback>, + expected_common_name, + ) } pub fn verify_with_callback( @@ -283,7 +314,29 @@ impl Certificate { where F: VerifyCallback + 'static, { - Self::verify_ex(chain, trust_ca, ca_crl, err_info, Some(cb)) + Self::verify_ex(chain, trust_ca, ca_crl, err_info, Some(cb), None) + } + + /// Like `verify_with_callback`, optionally accepts an `expected_common_name` arg. + /// + /// * `expected_common_name` + /// (From mbedtls documentation) The expected Common Name. This will be checked to be present in the certificate’s + /// subjectAltNames extension or, if this extension is absent, as a CN component in its Subject name. DNS names + /// and IP addresses are fully supported, while the URI subtype is partially supported: only exact matching, + /// without any normalization procedures described in 7.4 of RFC5280, will result in a positive URI verification. + /// This may be `None` if the CN need not be verified. + pub fn verify_with_callback_expected_common_name( + chain: &MbedtlsList, + trust_ca: &MbedtlsList, + ca_crl: Option<&mut Crl>, + err_info: Option<&mut String>, + cb: F, + expected_common_name: Option<&str>, + ) -> Result<()> + where + F: VerifyCallback + 'static, + { + Self::verify_ex(chain, trust_ca, ca_crl, err_info, Some(cb), expected_common_name) } } @@ -1489,4 +1542,36 @@ cYp0bH/RcPTC0Z+ZaqSWMtfxRrk63MJQF9EXpDCdvQRcTMD9D85DJrMKn8aumq0M ); assert_eq!(err, "The certificate has been revoked (is on a CRL)\n"); } + + #[test] + fn expected_common_name_test() { + const C_CERT: &'static str = concat!(include_str!("../../tests/data/certificate.crt"), "\0"); + const C_ROOT: &'static str = concat!(include_str!("../../tests/data/root.crt"), "\0"); + + let mut certs = MbedtlsList::new(); + certs.push(Certificate::from_pem(&C_CERT.as_bytes()).unwrap()); + let mut roots = MbedtlsList::new(); + roots.push(Certificate::from_pem(&C_ROOT.as_bytes()).unwrap()); + + let mut err = String::new(); + assert!( + Certificate::verify_with_expected_common_name(&certs, &roots, None, Some(&mut err), Some("example.com")).is_ok(), + ); + } + + #[test] + fn expected_common_name_wrong_name_test() { + const C_CERT: &'static str = concat!(include_str!("../../tests/data/certificate.crt"), "\0"); + const C_ROOT: &'static str = concat!(include_str!("../../tests/data/root.crt"), "\0"); + + let mut certs = MbedtlsList::new(); + certs.push(Certificate::from_pem(&C_CERT.as_bytes()).unwrap()); + let mut roots = MbedtlsList::new(); + roots.push(Certificate::from_pem(&C_ROOT.as_bytes()).unwrap()); + + let mut err = String::new(); + assert!( + Certificate::verify_with_expected_common_name(&certs, &roots, None, Some(&mut err), Some("notit.com")).is_err() + ); + } }