cleanup & improve code style

Author: Taowyoo

mbedtls/src/pk/mod.rs

@@ -163,7 +163,7 @@ define!(
 // B. Verifying thread safety.
 //
 // 1. Calls towards the specific Pk implementation are done via function pointers.
-// 
+//
 // - Example call towards Pk:
 //    ../../../mbedtls-sys/vendor/library/ssl_srv.c:3707 - mbedtls_pk_decrypt( private_key, p, len, ...
 // - This calls a generic function pointer via:
@@ -174,7 +174,7 @@ define!(
 // - The function pointers are defined via function:
 //      ../../../mbedtls-sys/vendor/crypto/library/pk.c:115 - mbedtls_pk_info_from_type
 // - They are as follows: mbedtls_rsa_info / mbedtls_eckey_info / mbedtls_ecdsa_info
-// - These are defined in: 
+// - These are defined in:
 //       ../../../mbedtls-sys/vendor/crypto/library/pk_wrap.c:196
 //
 // C. Checking types one by one.
@@ -201,7 +201,34 @@ define!(
 //
 // - Only used when creating/freeing - which is safe by design - eckey_alloc_wrap / eckey_free_wrap
 //
-// 3. ECDSA - code uses mbedtls_pk wrappers. In this case code goes through ECKEY logic above. (mbedtls_pk_parse_key intentionally never calls mbedtls_pk_info_from_type with MBEDTLS_PK_ECDSA)
+// 3. ECDSA: mbedtls_ecdsa_info at ../../../mbedtls-sys/vendor/crypto/library/pk_wrap.c:729
+// This does not use internal locks but avoids interior mutability.
+//
+// - Const access / copies context to stack based variables:
+//   ecdsa_verify_wrap: ../../../mbedtls-sys/vendor/crypto/library/pk_wrap.c:544
+//       This copies the public key on the stack - in buf[] and copies the group id and nbits.
+//       That is done via: mbedtls_pk_write_pubkey( &p, buf, &key ) where key.pk_ctx = ctx;
+//       And the key is a const parameter to mbedtls_pk_write_pubkey - ../../../mbedtls-sys/vendor/crypto/library/pkwrite.c:158
+//
+// - Const access with additional notes due to call stacks involved.
+//   ecdsa_sign_wrap: ../../../mbedtls-sys/vendor/crypto/library/pk_wrap.c:657
+//
+//       mbedtls_ecdsa_write_signature ../../../mbedtls-sys/vendor/crypto/library/ecdsa.c:688
+//           mbedtls_ecdsa_write_signature_restartable ../../../mbedtls-sys/vendor/crypto/library/ecdsa.c:640
+//               MBEDTLS_ECDSA_DETERMINISTIC is not defined.
+//               MBEDTLS_ECDSA_SIGN_ALT is not defined.
+//               Passes grp to: ecdsa_sign_restartable: ../../../mbedtls-sys/vendor/crypto/library/ecdsa.c:253
+//                    Const access to group - reads parameters, passed as const to mbedtls_ecp_gen_privkey,
+//                    mbedtls_ecp_mul_restartable: ../../../mbedtls-sys/vendor/crypto/library/ecp.c:2351
+//                        MBEDTLS_ECP_INTERNAL_ALT is not defined. (otherwise it might not be safe depending on ecp_init/ecp_free) ../../../mbedtls-sys/build/config.rs:131
+//                        Passes as const to: mbedtls_ecp_check_privkey / mbedtls_ecp_check_pubkey / mbedtls_ecp_get_type( grp
+//
+// - Ignored due to not defined: ecdsa_verify_rs_wrap, ecdsa_sign_rs_wrap, ecdsa_rs_alloc, ecdsa_rs_free
+//   (Undefined - MBEDTLS_ECP_RESTARTABLE - ../../../mbedtls-sys/build/config.rs:173)
+//
+// - Only const access to context: eckey_check_pair
+//
+// - Only used when creating/freeing - which is safe by design: ecdsa_alloc_wrap, ecdsa_free_wrap
 //
 unsafe impl Sync for Pk {}
 
@@ -900,7 +927,7 @@ impl Pk {
         if hash.len() == 0 || sig.len() == 0 {
             return Err(Error::PkBadInputData)
         }
-        
+
         unsafe {
             pk_verify(
                 &mut self.inner,
@@ -1270,7 +1297,7 @@ iy6KC991zzvaWY/Ys+q/84Afqa+0qJKQnPuy/7F5GkVdQA/lfbhi
             let mut dummy_sig = [];
             assert_eq!(pk.sign(digest, data, &mut dummy_sig, &mut crate::test_support::rand::test_rng()).unwrap_err(), Error::PkBadInputData);
             assert_eq!(pk.sign(digest, &[], &mut signature, &mut crate::test_support::rand::test_rng()).unwrap_err(), Error::PkBadInputData);
-            
+
             assert_eq!(pk.sign_deterministic(digest, data, &mut dummy_sig, &mut crate::test_support::rand::test_rng()).unwrap_err(), Error::PkBadInputData);
             assert_eq!(pk.sign_deterministic(digest, &[], &mut signature, &mut crate::test_support::rand::test_rng()).unwrap_err(), Error::PkBadInputData);
 

mbedtls/src/ssl/context.rs

@@ -20,24 +20,21 @@ use {
     std::marker::Unpin,
     std::pin::Pin,
     std::task::{Context as TaskContext, Poll},
+    tokio::io::{AsyncRead, AsyncWrite, ReadBuf},
+    crate::ssl::async_utils::IoAdapter,
 };
 
 use mbedtls_sys::types::raw_types::{c_int, c_uchar, c_void};
 use mbedtls_sys::types::size_t;
 use mbedtls_sys::*;
 
-#[cfg(all(feature = "std", feature = "async"))]
-use tokio::io::{AsyncRead, AsyncWrite, ReadBuf};
-
 #[cfg(not(feature = "std"))]
 use crate::alloc_prelude::*;
 use crate::alloc::{List as MbedtlsList};
 use crate::error::{Error, Result, IntoResult};
 use crate::pk::Pk;
 use crate::private::UnsafeFrom;
 use crate::ssl::config::{Config, Version, AuthMode};
-#[cfg(all(feature = "std", feature = "async"))]
-use crate::ssl::async_utils::IoAdapter;
 use crate::x509::{Certificate, Crl, VerifyError};
 
 pub trait IoCallback {
@@ -199,7 +196,7 @@ define!(
     struct HandshakeContext {
         handshake_ca_cert: Option<Arc<MbedtlsList<Certificate>>>,
         handshake_crl: Option<Arc<Crl>>,
-        
+
         handshake_cert: Vec<Arc<MbedtlsList<Certificate>>>,
         handshake_pk: Vec<Arc<Pk>>,
     };
@@ -213,10 +210,10 @@ define!(
 pub struct Context<T> {
     // Base structure used in SNI callback where we cannot determine the io type.
     inner: HandshakeContext,
-    
+
     // config is used read-only for multiple contexts and is immutable once configured.
-    config: Arc<Config>, 
-    
+    config: Arc<Config>,
+
     // Must be held in heap and pointer to it as pointer is sent to MbedSSL and can't be re-allocated.
     io: Option<Box<T>>,
 
@@ -240,14 +237,10 @@ impl<'a, T> Into<*mut ssl_context> for &'a mut Context<T> {
     }
 }
 
-#[cfg(all(feature = "std", feature = "async"))]
-pub type AsyncContext<T> = Context<IoAdapter<T>>;
-
-
 impl<T> Context<T> {
     pub fn new(config: Arc<Config>) -> Self {
         let mut inner = ssl_context::default();
-        
+
         unsafe {
             ssl_init(&mut inner);
             ssl_setup(&mut inner, (&*config).into());
@@ -258,7 +251,7 @@ impl<T> Context<T> {
                 inner,
                 handshake_ca_cert: None,
                 handshake_crl: None,
-                
+
                 handshake_cert: vec![],
                 handshake_pk: vec![],
             },
@@ -268,11 +261,11 @@ impl<T> Context<T> {
             client_transport_id: None,
         }
     }
-    
+
     pub(crate) fn handle(&self) -> &::mbedtls_sys::ssl_context {
         self.inner.handle()
     }
-    
+
     pub(crate) fn handle_mut(&mut self) -> &mut ::mbedtls_sys::ssl_context {
         self.inner.handle_mut()
     }
@@ -385,23 +378,23 @@ impl<T> Context<T> {
     pub fn config(&self) -> &Arc<Config> {
         &self.config
     }
-    
+
     pub fn close(&mut self) {
         unsafe {
             ssl_close_notify(self.into());
             ssl_set_bio(self.into(), ::core::ptr::null_mut(), None, None, None);
             self.io = None;
         }
     }
-    
+
     pub fn io(&self) -> Option<&T> {
         self.io.as_ref().map(|v| &**v)
     }
-    
+
     pub fn io_mut(&mut self) -> Option<&mut T> {
         self.io.as_mut().map(|v| &mut **v)
     }
-    
+
     /// Return the minor number of the negotiated TLS version
     pub fn minor_version(&self) -> i32 {
         self.handle().minor_ver
@@ -433,15 +426,15 @@ impl<T> Context<T> {
 
 
     // Session specific functions
-    
+
     /// Return the 16-bit ciphersuite identifier.
     /// All assigned ciphersuites are listed by the IANA in
     /// <https://www.iana.org/assignments/tls-parameters/tls-parameters.txt>
     pub fn ciphersuite(&self) -> Result<u16> {
         if self.handle().session.is_null() {
             return Err(Error::SslBadInputData);
         }
-        
+
         Ok(unsafe { self.handle().session.as_ref().unwrap().ciphersuite as u16 })
     }
 
@@ -578,12 +571,12 @@ impl HandshakeContext {
         self.handshake_ca_cert = None;
         self.handshake_crl = None;
     }
-    
+
     pub fn set_authmode(&mut self, am: AuthMode) -> Result<()> {
         if self.inner.handshake as *const _ == ::core::ptr::null() {
             return Err(Error::SslBadInputData);
         }
-        
+
         unsafe { ssl_set_hs_authmode(self.into(), am as i32) }
         Ok(())
     }
@@ -637,6 +630,9 @@ impl HandshakeContext {
     }
 }
 
+#[cfg(all(feature = "std", feature = "async"))]
+pub type AsyncContext<T> = Context<IoAdapter<T>>;
+
 #[cfg(all(feature = "std", feature = "async"))]
 pub trait IoAsyncCallback {
     unsafe extern "C" fn call_recv_async(user_data: *mut c_void, data: *mut c_uchar, len: size_t) -> c_int where Self: Sized;
@@ -700,7 +696,7 @@ impl<T> std::future::Future for HandshakeFuture<'_, T> {
     fn poll(mut self: Pin<&mut Self>, ctx: &mut TaskContext) -> std::task::Poll<Self::Output> {
         self.0.io_mut().ok_or(Error::NetInvalidContext)?
                        .ecx.set(ctx);
-        
+
         let result = match self.0.handshake() {
             Err(Error::SslWantRead) |
             Err(Error::SslWantWrite) => {
@@ -709,9 +705,9 @@ impl<T> std::future::Future for HandshakeFuture<'_, T> {
             Err(e) => Poll::Ready(Err(e)),
             Ok(()) => Poll::Ready(Ok(()))
         };
-        
+
         self.0.io_mut().map(|v| v.ecx.clear());
-        
+
         result
     }
 }
@@ -741,7 +737,7 @@ impl<T: AsyncRead + AsyncWrite + Unpin + 'static> AsyncContext<T> {
             );
 
             self.io = Some(io);
-            self.inner.reset_handshake();            
+            self.inner.reset_handshake();
         }
 
         HandshakeFuture(self).await
@@ -762,7 +758,7 @@ impl<T: AsyncRead> AsyncRead for Context<IoAdapter<T>> {
 
         self.io_mut().ok_or(IoError::new(IoErrorKind::Other, "stream has been shutdown"))?
                      .ecx.set(cx);
-        
+
         let result = match unsafe { ssl_read((&mut *self).into(), buf.initialize_unfilled().as_mut_ptr(), buf.initialize_unfilled().len()).into_result() } {
             Err(Error::SslPeerCloseNotify) => Poll::Ready(Ok(())),
             Err(Error::SslWantRead) => Poll::Pending,
@@ -798,10 +794,10 @@ impl<T: AsyncWrite + Unpin> AsyncWrite for Context<IoAdapter<T>> {
             io.write_tracker.adjust_buf(buf)
         }?;
 
-        
+
         self.io_mut().ok_or(IoError::new(IoErrorKind::Other, "stream has been shutdown"))?
                      .ecx.set(cx);
-        
+
         let result = match unsafe { ssl_write((&mut *self).into(), buf.as_ptr(), buf.len()).into_result() } {
             Err(Error::SslPeerCloseNotify) => Poll::Ready(Ok(0)),
             Err(Error::SslWantWrite) => Poll::Pending,
@@ -868,7 +864,7 @@ mod tests {
 
     use crate::ssl::context::{HandshakeContext, Context};
     use crate::tests::TestTrait;
-    
+
     #[test]
     fn handshakecontext_sync() {
         assert!(!TestTrait::<dyn Sync, HandshakeContext>::new().impls_trait(), "HandshakeContext must be !Sync");
@@ -884,7 +880,7 @@ mod tests {
             unimplemented!()
         }
     }
-    
+
     #[cfg(feature = "std")]
     impl Write for NonSendStream {
         fn write(&mut self, _: &[u8]) -> IoResult<usize> {
@@ -906,7 +902,7 @@ mod tests {
             unimplemented!()
         }
     }
-    
+
     #[cfg(feature = "std")]
     impl Write for SendStream {
         fn write(&mut self, _: &[u8]) -> IoResult<usize> {

mbedtls/tests/ssl_conf_ca_cb.rs

@@ -18,7 +18,7 @@ use mbedtls::pk::Pk;
 use mbedtls::rng::CtrDrbg;
 use mbedtls::ssl::config::{Endpoint, Preset, Transport};
 use mbedtls::ssl::{Config, Context};
-use mbedtls::x509::{Certificate};
+use mbedtls::x509::Certificate;
 use mbedtls::Result as TlsResult;
 use mbedtls::ssl::config::CaCallback;
 
@@ -58,7 +58,6 @@ mod test {
     use std::thread;
     use crate::support::net::create_tcp_pair;
     use crate::support::keys;
-    use mbedtls::x509::Certificate;
     use mbedtls::Error;
     use mbedtls::alloc::List as MbedtlsList;