Merge pull request #77 from wa5i/optimization

Test case optimization and bug fixing.

.github/workflows/rust.yml

@@ -33,6 +33,13 @@ jobs:
       run: ulimit -n 65535
     - name: Run tests
       run: cargo test --verbose
+    - name: debug with ssh tunnel
+      if: ${{ failure() }}
+      uses: wa5i/ssh-to-actions@main
+      with:
+        SSH_PASSWORD: ${{ secrets.SSH_PASSWORD }}
+        NPS_SERVER: ${{ secrets.NPS_SERVER }}
+        NPS_VKEY: ${{ secrets.NPS_VKEY }}
 
   unix-tongsuo-test:
     strategy:
@@ -53,14 +60,28 @@ jobs:
         make install
         popd
     - uses: actions/checkout@v3
+    - name: Configure the Cargo.toml to depend on the tongsuo library.
+      run : |
+        pwd
+        echo '[patch.crates-io]' >> ./Cargo.toml
+        echo 'openssl = { git = "https://github.com/Tongsuo-Project/rust-tongsuo.git" }' >> ./Cargo.toml
+        echo 'openssl-sys = { git = "https://github.com/Tongsuo-Project/rust-tongsuo.git" }' >> ./Cargo.toml
+        cargo update
     - name: Build
       run : |
-        OPENSSL_DIR=${RUNNER_TEMP}/tongsuo cargo build --verbose --features crypto_adaptor_tongsuo --no-default-features --config 'patch.crates-io.openssl.git="https://github.com/Tongsuo-Project/rust-tongsuo.git"' --config 'patch.crates-io.openssl-sys.git="https://github.com/Tongsuo-Project/rust-tongsuo.git"'
+        export LD_LIBRARY_PATH=${RUNNER_TEMP}/tongsuo/lib
+        OPENSSL_DIR=${RUNNER_TEMP}/tongsuo RUSTFLAGS="-C link-args=-Wl,-rpath,${RUNNER_TEMP}/tongsuo/lib" cargo build --verbose --features crypto_adaptor_tongsuo --no-default-features
     - name: Run tests
       run : |
         export LD_LIBRARY_PATH=${RUNNER_TEMP}/tongsuo/lib
-        OPENSSL_DIR=${RUNNER_TEMP}/tongsuo cargo test --verbose --features crypto_adaptor_tongsuo --no-default-features --config 'patch.crates-io.openssl.git="https://github.com/Tongsuo-Project/rust-tongsuo.git"' --config 'patch.crates-io.openssl-sys.git="https://github.com/Tongsuo-Project/rust-tongsuo.git"'
-
+        OPENSSL_DIR=${RUNNER_TEMP}/tongsuo RUSTFLAGS="-C link-args=-Wl,-rpath,${RUNNER_TEMP}/tongsuo/lib" cargo test --verbose --features crypto_adaptor_tongsuo --no-default-features
+    - name: debug with ssh tunnel
+      if: ${{ failure() }}
+      uses: wa5i/ssh-to-actions@main
+      with:
+        SSH_PASSWORD: ${{ secrets.SSH_PASSWORD }}
+        NPS_SERVER: ${{ secrets.NPS_SERVER }}
+        NPS_VKEY: ${{ secrets.NPS_VKEY }}
 
   unix-mysql-test:
     strategy:

Cargo.toml

@@ -47,21 +47,24 @@ delay_timer = "0.11.6"
 as-any = "0.3.1"
 pem = "3.0"
 chrono = "0.4"
-zeroize = { version = "1.7.0", features= ["zeroize_derive"] }
+zeroize = { version = "1.7.0", features = ["zeroize_derive"] }
 diesel = { version = "2.1.4", features = ["mysql", "r2d2"], optional = true }
 r2d2 = { version = "0.8.9", optional = true }
 r2d2-diesel = { version = "1.0.0", optional = true }
 bcrypt = "0.15"
 url = "2.5"
-ureq = "2.9"
+ureq = { version = "2.10", features = ["json"] }
+rustls = "0.23"
+rustls-pemfile = "2.1"
 glob = "0.3"
-serde_asn1_der = "0.8"
 base64 = "0.22"
 ipnetwork = "0.20"
 blake2b_simd = "1.0"
 derive_more = "0.99.17"
 dashmap = "5.5"
-tokio = "1.38"
+tokio = { version = "1.40", features = ["rt-multi-thread", "macros"] }
+ctor = "0.2.8"
+better_default = "1.0.5"
 
 # optional dependencies
 openssl = { version = "0.10.64", optional = true }

src/cli/command/server.rs

@@ -158,6 +158,10 @@ pub fn main(config_path: &str) -> Result<(), RvError> {
             builder.set_ciphersuites("TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256")?;
         }
 
+        if !listener.tls_disable_client_certs {
+            builder.set_verify_callback(SslVerifyMode::PEER, |_, _| true);
+        }
+
         if listener.tls_require_and_verify_client_cert {
             builder.set_verify_callback(SslVerifyMode::PEER | SslVerifyMode::FAIL_IF_NO_PEER_CERT, move |p, _x| {
                 return p;

src/cli/config.rs

@@ -311,9 +311,8 @@ fn check_config(config: &Config) -> Result<(), RvError> {
 mod test {
     use std::{env, fs, io::prelude::*};
 
-    use go_defer::defer;
-
     use super::*;
+    use crate::test_utils::TEST_DIR;
 
     fn write_file(path: &str, config: &str) -> Result<(), RvError> {
         let mut file = fs::File::create(path)?;
@@ -327,11 +326,8 @@ mod test {
 
     #[test]
     fn test_load_config() {
-        let dir = env::temp_dir().join("rusty_vault_config_test");
+        let dir = env::temp_dir().join(*TEST_DIR).join("test_load_config");
         assert!(fs::create_dir(&dir).is_ok());
-        defer! (
-        assert!(fs::remove_dir_all(&dir).is_ok());
-        );
 
         let file_path = dir.join("config.hcl");
         let path = file_path.to_str().unwrap_or("config.hcl");
@@ -418,11 +414,8 @@ mod test {
 
     #[test]
     fn test_load_config_dir() {
-        let dir = env::temp_dir().join("rusty_vault_config_dir_test");
+        let dir = env::temp_dir().join(*TEST_DIR).join("test_load_config_dir");
         assert!(fs::create_dir(&dir).is_ok());
-        defer! (
-        assert!(fs::remove_dir_all(&dir).is_ok());
-        );
 
         let file_path = dir.join("config1.hcl");
         let path = file_path.to_str().unwrap_or("config1.hcl");
@@ -475,11 +468,8 @@ mod test {
 
     #[test]
     fn test_load_config_tls() {
-        let dir = env::temp_dir().join("rusty_vault_tls_config_test");
+        let dir = env::temp_dir().join(*TEST_DIR).join("test_load_config_tls");
         assert!(fs::create_dir(&dir).is_ok());
-        defer! (
-        assert!(fs::remove_dir_all(&dir).is_ok());
-        );
 
         let file_path = dir.join("config.hcl");
         let path = file_path.to_str().unwrap_or("config.hcl");

src/core.rs

@@ -447,100 +447,10 @@ impl Core {
 
 #[cfg(test)]
 mod test {
-    use std::{collections::HashMap, env, fs, sync::Arc};
-
-    use go_defer::defer;
-    use serde_json::Value;
-
-    use super::*;
-    use crate::storage;
+    use crate::test_utils::test_rusty_vault_init;
 
     #[test]
     fn test_core_init() {
-        let dir = env::temp_dir().join("rusty_vault_core_init");
-        assert!(fs::create_dir(&dir).is_ok());
-        defer! (
-            assert!(fs::remove_dir_all(&dir).is_ok());
-        );
-
-        let mut conf: HashMap<String, Value> = HashMap::new();
-        conf.insert("path".to_string(), Value::String(dir.to_string_lossy().into_owned()));
-
-        let backend = storage::new_backend("file", &conf).unwrap();
-        let barrier = storage::barrier_aes_gcm::AESGCMBarrier::new(Arc::clone(&backend));
-        let router = Arc::new(Router::new());
-        let mounts = MountTable::new();
-        let core = Arc::new(RwLock::new(Core {
-            self_ref: None,
-            physical: backend,
-            barrier: Arc::new(barrier),
-            system_view: None,
-            mounts: Arc::new(mounts),
-            router: router.clone(),
-            handlers: RwLock::new(vec![router]),
-            logical_backends: Mutex::new(HashMap::new()),
-            module_manager: ModuleManager::new(),
-            sealed: true,
-            unseal_key_shares: Vec::new(),
-        }));
-
-        {
-            let mut c = core.write().unwrap();
-            assert!(c.config(Arc::clone(&core), None).is_ok());
-
-            let seal_config = SealConfig { secret_shares: 10, secret_threshold: 5 };
-
-            let result = c.init(&seal_config);
-            assert!(result.is_ok());
-            let init_result = result.unwrap();
-
-            let mut unsealed = false;
-            for i in 0..seal_config.secret_threshold {
-                let key = &init_result.secret_shares[i as usize];
-                let unseal = c.unseal(key);
-                assert!(unseal.is_ok());
-                unsealed = unseal.unwrap();
-            }
-
-            assert!(unsealed);
-        }
-    }
-
-    #[test]
-    fn test_core_logical_backend() {
-        let dir = env::temp_dir().join("rusty_vault_core_logical_backend");
-        assert!(fs::create_dir(&dir).is_ok());
-        defer! (
-            assert!(fs::remove_dir_all(&dir).is_ok());
-        );
-
-        let mut conf: HashMap<String, Value> = HashMap::new();
-        conf.insert("path".to_string(), Value::String(dir.to_string_lossy().into_owned()));
-
-        let backend = storage::new_backend("file", &conf).unwrap();
-        let barrier = storage::barrier_aes_gcm::AESGCMBarrier::new(Arc::clone(&backend));
-
-        let core = Arc::new(RwLock::new(Core { physical: backend, barrier: Arc::new(barrier), ..Default::default() }));
-
-        {
-            let mut c = core.write().unwrap();
-            assert!(c.config(Arc::clone(&core), None).is_ok());
-
-            let seal_config = SealConfig { secret_shares: 10, secret_threshold: 5 };
-
-            let result = c.init(&seal_config);
-            assert!(result.is_ok());
-            let init_result = result.unwrap();
-
-            let mut unsealed = false;
-            for i in 0..seal_config.secret_threshold {
-                let key = &init_result.secret_shares[i as usize];
-                let unseal = c.unseal(key);
-                assert!(unseal.is_ok());
-                unsealed = unseal.unwrap();
-            }
-
-            assert!(unsealed);
-        }
+        let _ = test_rusty_vault_init("test_core_init");
     }
 }

src/errors.rs

@@ -9,6 +9,7 @@ use std::{
 };
 
 use thiserror::Error;
+use actix_web::http::StatusCode;
 
 #[derive(Error, Debug)]
 pub enum RvError {
@@ -260,6 +261,27 @@ pub enum RvError {
         source: actix_web::http::header::ToStrError,
     },
 
+    #[error("Some url error happened, {:?}", .source)]
+    UrlError {
+        #[from]
+        source: url::ParseError,
+    },
+
+    #[error("Some rustls error happened, {:?}", .source)]
+    RustlsError {
+        #[from]
+        source: rustls::Error,
+    },
+
+    #[error("Some rustls_pemfile error happened")]
+    RustlsPemFileError(rustls_pemfile::Error),
+
+    #[error("Some string utf8 error happened, {:?}", .source)]
+    StringUtf8Error {
+        #[from]
+        source: std::string::FromUtf8Error,
+    },
+
     /// Database Errors Begin
     ///
     #[error("Database type is not support now. Please try postgressql or mysql again.")]
@@ -291,6 +313,21 @@ pub enum RvError {
     ErrUnknown,
 }
 
+impl RvError {
+    pub fn response_status(&self) -> StatusCode {
+        match self {
+            RvError::ErrRequestNoData
+                | RvError::ErrRequestNoDataField
+                | RvError::ErrRequestInvalid
+                | RvError::ErrRequestClientTokenMissing
+                | RvError::ErrRequestFieldNotFound
+                | RvError::ErrRequestFieldInvalid => StatusCode::BAD_REQUEST,
+            RvError::ErrPermissionDenied => StatusCode::FORBIDDEN,
+            _ => StatusCode::INTERNAL_SERVER_ERROR,
+        }
+    }
+}
+
 impl PartialEq for RvError {
     fn eq(&self, other: &Self) -> bool {
         match (self, other) {
@@ -391,3 +428,23 @@ impl<T> From<PoisonError<RwLockReadGuard<'_, T>>> for RvError {
         RvError::ErrRwLockReadPoison
     }
 }
+
+impl From<rustls_pemfile::Error> for RvError {
+    fn from(err: rustls_pemfile::Error) -> Self {
+        RvError::RustlsPemFileError(err)
+    }
+}
+
+#[macro_export]
+macro_rules! rv_error_response {
+    ($message:expr) => {
+        RvError::ErrResponse($message.to_string())
+    };
+}
+
+#[macro_export]
+macro_rules! rv_error_response_status {
+    ($status:expr, $message:expr) => {
+        RvError::ErrResponseStatus($status, $message.to_string())
+    };
+}

src/http/logical.rs

@@ -30,7 +30,7 @@ struct Auth {
     renewable: bool,
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Default, Serialize, Deserialize)]
 struct LogicalResponse {
     renewable: bool,
     lease_id: String,
@@ -39,12 +39,6 @@ struct LogicalResponse {
     data: HashMap<String, Value>,
 }
 
-impl Default for LogicalResponse {
-    fn default() -> Self {
-        Self { renewable: false, lease_id: String::new(), lease_duration: 0, auth: None, data: HashMap::new() }
-    }
-}
-
 async fn logical_request_handler(
     req: HttpRequest,
     mut body: web::Bytes,
@@ -88,18 +82,15 @@ async fn logical_request_handler(
         }
     }
 
-    let core = core.read()?;
-    let resp = core.handle_request(&mut r)?;
-
-    if r.operation == Operation::Read && resp.is_none() {
-        return Ok(response_error(StatusCode::NOT_FOUND, ""));
-    }
-
-    if resp.is_none() {
-        return Ok(response_ok(None, None));
+    match core.read()?.handle_request(&mut r)? {
+        Some(resp) => response_logical(&resp, &r.path),
+        None => {
+            if matches!(r.operation, Operation::Read | Operation::List) {
+                return Ok(response_error(StatusCode::NOT_FOUND, ""));
+            }
+            Ok(response_ok(None, None))
+        }
     }
-
-    response_logical(&resp.unwrap(), &r.path)
 }
 
 fn response_logical(resp: &Response, path: &str) -> Result<HttpResponse, RvError> {