From c62087c4547fd623410fbba6fe4dd576b15ad7b9 Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Mon, 18 Sep 2023 11:23:06 +0200 Subject: [PATCH 1/4] Fix dictionary file paring in UsrPwd auth --- .../unicast/establishment/ext/auth/usrpwd.rs | 26 +++++++--- .../tests/unicast_authenticator.rs | 48 +++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs b/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs index 48d292cd32..004f34c0ed 100644 --- a/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs +++ b/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs @@ -21,7 +21,6 @@ use zenoh_buffers::{ writer::{DidntWrite, HasWriter, Writer}, }; use zenoh_codec::{RCodec, WCodec, Zenoh080}; -use zenoh_collections::Properties; use zenoh_config::UsrPwdConf; use zenoh_core::{bail, zasyncread, zerror, Error as ZError, Result as ZResult}; use zenoh_crypto::hmac; @@ -74,9 +73,24 @@ impl AuthUsrPwd { .map_err(|e| zerror!("{S} Invalid user-password dictionary file: {}.", e))?; // Populate the user-password dictionary - let mut ps = Properties::from(content); - for (user, password) in ps.drain() { - lookup.insert(user.as_bytes().to_owned(), password.as_bytes().to_owned()); + // The config file is expected to be in the form of: + // usr1:pwd1 + // usr2:pwd2 + // usr3:pwd3 + // I.e.: one : entry per line + for l in content.lines() { + let idx = l.find(':').ok_or_else(|| { + zerror!("{S} Invalid user-password dictionary file: invalid format.") + })?; + let user = l[..idx].as_bytes().to_owned(); + if user.is_empty() { + bail!("{S} Invalid user-password dictionary file: empty user.") + } + let password = l[idx + 1..].as_bytes().to_owned(); + if password.is_empty() { + bail!("{S} Invalid user-password dictionary file: empty password.") + } + lookup.insert(user, password); } log::debug!("{S} User-password dictionary has been configured."); } @@ -106,10 +120,10 @@ impl fmt::Debug for AuthUsrPwd { match self.credentials.as_ref() { Some(c) => write!( f, - "User: '{}', Password: '***'", + "User: '{}', Password: '***', ", String::from_utf8_lossy(&c.0) )?, - None => write!(f, "User: '', Password: ''")?, + None => write!(f, "User: '', Password: '', ")?, } write!(f, "Dictionary: {{")?; for (i, (u, _)) in self.lookup.iter().enumerate() { diff --git a/io/zenoh-transport/tests/unicast_authenticator.rs b/io/zenoh-transport/tests/unicast_authenticator.rs index 33b35d52d2..0d2b888c5b 100644 --- a/io/zenoh-transport/tests/unicast_authenticator.rs +++ b/io/zenoh-transport/tests/unicast_authenticator.rs @@ -407,10 +407,58 @@ async fn auth_pubkey(endpoint: &EndPoint, #[cfg(feature = "shared-memory")] shm_ #[cfg(feature = "auth_usrpwd")] async fn auth_usrpwd(endpoint: &EndPoint, #[cfg(feature = "shared-memory")] shm_transport: bool) { + use std::{fs::File, io::Write}; + use zenoh_config::UsrPwdConf; use zenoh_transport::test_helpers::make_basic_transport_manager_builder; use zenoh_transport::unicast::establishment::ext::auth::AuthUsrPwd; use zenoh_transport::TransportManager; + /* [CONFIG] */ + let f1 = "zenoh-test-auth-usrpwd.txt"; + + let mut config = UsrPwdConf::default(); + config.set_user(Some("usr1".to_owned())).unwrap(); + config.set_password(Some("pwd1".to_owned())).unwrap(); + config.set_dictionary_file(Some(f1.to_owned())).unwrap(); + + macro_rules! zconfig { + () => { + File::options() + .create(true) + .write(true) + .truncate(true) + .open(f1) + .unwrap() + }; + } + // Valid config + let mut c = zconfig!(); + writeln!(c, "usr1:pwd1").unwrap(); + drop(c); + assert!(AuthUsrPwd::from_config(&config).await.unwrap().is_some()); + // Invalid config + let mut c = zconfig!(); + writeln!(c, "usr1").unwrap(); + drop(c); + assert!(AuthUsrPwd::from_config(&config).await.is_err()); + // Empty password + let mut c = zconfig!(); + writeln!(c, "usr1:").unwrap(); + drop(c); + assert!(AuthUsrPwd::from_config(&config).await.is_err()); + // Empty user + let mut c = zconfig!(); + writeln!(c, ":pwd1").unwrap(); + drop(c); + assert!(AuthUsrPwd::from_config(&config).await.is_err()); + // Empty user and password + let mut c = zconfig!(); + writeln!(c, ":").unwrap(); + drop(c); + assert!(AuthUsrPwd::from_config(&config).await.is_err()); + + let _ = std::fs::remove_file(f1); + /* [CLIENT] */ let client01_id = ZenohId::try_from([2]).unwrap(); let user01 = "user01".to_string(); From 9922940791d0f14a48d8582515fda5e5add6a2f9 Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Mon, 18 Sep 2023 12:07:58 +0200 Subject: [PATCH 2/4] Move UsrPwd config test in dedicated test --- .../unicast/establishment/ext/auth/usrpwd.rs | 59 +++++++++++++++++++ .../tests/unicast_authenticator.rs | 48 --------------- 2 files changed, 59 insertions(+), 48 deletions(-) diff --git a/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs b/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs index 004f34c0ed..4787020958 100644 --- a/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs +++ b/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs @@ -442,3 +442,62 @@ impl<'a> AcceptFsm for AuthUsrPwdFsm<'a> { Ok(Some(ZExtUnit::new())) } } + +mod tests { + #[test] + fn authenticator_usrpwd_config() { + async fn inner() { + use super::AuthUsrPwd; + use std::{fs::File, io::Write}; + use zenoh_config::UsrPwdConf; + + /* [CONFIG] */ + let f1 = "zenoh-test-auth-usrpwd.txt"; + + let mut config = UsrPwdConf::default(); + config.set_user(Some("usr1".to_owned())).unwrap(); + config.set_password(Some("pwd1".to_owned())).unwrap(); + config.set_dictionary_file(Some(f1.to_owned())).unwrap(); + + macro_rules! zconfig { + () => { + File::options() + .create(true) + .write(true) + .truncate(true) + .open(f1) + .unwrap() + }; + } + // Valid config + let mut c = zconfig!(); + writeln!(c, "usr1:pwd1").unwrap(); + drop(c); + assert!(AuthUsrPwd::from_config(&config).await.unwrap().is_some()); + // Invalid config + let mut c = zconfig!(); + writeln!(c, "usr1").unwrap(); + drop(c); + assert!(AuthUsrPwd::from_config(&config).await.is_err()); + // Empty password + let mut c = zconfig!(); + writeln!(c, "usr1:").unwrap(); + drop(c); + assert!(AuthUsrPwd::from_config(&config).await.is_err()); + // Empty user + let mut c = zconfig!(); + writeln!(c, ":pwd1").unwrap(); + drop(c); + assert!(AuthUsrPwd::from_config(&config).await.is_err()); + // Empty user and password + let mut c = zconfig!(); + writeln!(c, ":").unwrap(); + drop(c); + assert!(AuthUsrPwd::from_config(&config).await.is_err()); + + let _ = std::fs::remove_file(f1); + } + + async_std::task::block_on(inner()); + } +} diff --git a/io/zenoh-transport/tests/unicast_authenticator.rs b/io/zenoh-transport/tests/unicast_authenticator.rs index 0d2b888c5b..33b35d52d2 100644 --- a/io/zenoh-transport/tests/unicast_authenticator.rs +++ b/io/zenoh-transport/tests/unicast_authenticator.rs @@ -407,58 +407,10 @@ async fn auth_pubkey(endpoint: &EndPoint, #[cfg(feature = "shared-memory")] shm_ #[cfg(feature = "auth_usrpwd")] async fn auth_usrpwd(endpoint: &EndPoint, #[cfg(feature = "shared-memory")] shm_transport: bool) { - use std::{fs::File, io::Write}; - use zenoh_config::UsrPwdConf; use zenoh_transport::test_helpers::make_basic_transport_manager_builder; use zenoh_transport::unicast::establishment::ext::auth::AuthUsrPwd; use zenoh_transport::TransportManager; - /* [CONFIG] */ - let f1 = "zenoh-test-auth-usrpwd.txt"; - - let mut config = UsrPwdConf::default(); - config.set_user(Some("usr1".to_owned())).unwrap(); - config.set_password(Some("pwd1".to_owned())).unwrap(); - config.set_dictionary_file(Some(f1.to_owned())).unwrap(); - - macro_rules! zconfig { - () => { - File::options() - .create(true) - .write(true) - .truncate(true) - .open(f1) - .unwrap() - }; - } - // Valid config - let mut c = zconfig!(); - writeln!(c, "usr1:pwd1").unwrap(); - drop(c); - assert!(AuthUsrPwd::from_config(&config).await.unwrap().is_some()); - // Invalid config - let mut c = zconfig!(); - writeln!(c, "usr1").unwrap(); - drop(c); - assert!(AuthUsrPwd::from_config(&config).await.is_err()); - // Empty password - let mut c = zconfig!(); - writeln!(c, "usr1:").unwrap(); - drop(c); - assert!(AuthUsrPwd::from_config(&config).await.is_err()); - // Empty user - let mut c = zconfig!(); - writeln!(c, ":pwd1").unwrap(); - drop(c); - assert!(AuthUsrPwd::from_config(&config).await.is_err()); - // Empty user and password - let mut c = zconfig!(); - writeln!(c, ":").unwrap(); - drop(c); - assert!(AuthUsrPwd::from_config(&config).await.is_err()); - - let _ = std::fs::remove_file(f1); - /* [CLIENT] */ let client01_id = ZenohId::try_from([2]).unwrap(); let user01 = "user01".to_string(); From f50b60e1135728ba991d9e00bf130134fdeaa9bf Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Mon, 18 Sep 2023 12:09:36 +0200 Subject: [PATCH 3/4] Move UsrPwd config test in dedicated test --- .../src/unicast/establishment/ext/auth/usrpwd.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs b/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs index 4787020958..e4fdd2b527 100644 --- a/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs +++ b/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs @@ -498,6 +498,9 @@ mod tests { let _ = std::fs::remove_file(f1); } - async_std::task::block_on(inner()); + async_std::task::block_on(async { + zasync_executor_init!(); + inner().await; + }); } } From 383f8a61bea087383d83bb6f31edb4e393db7b0b Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Mon, 18 Sep 2023 12:09:55 +0200 Subject: [PATCH 4/4] Move UsrPwd config test in dedicated test --- io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs b/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs index e4fdd2b527..521986ae00 100644 --- a/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs +++ b/io/zenoh-transport/src/unicast/establishment/ext/auth/usrpwd.rs @@ -446,6 +446,8 @@ impl<'a> AcceptFsm for AuthUsrPwdFsm<'a> { mod tests { #[test] fn authenticator_usrpwd_config() { + use zenoh_core::zasync_executor_init; + async fn inner() { use super::AuthUsrPwd; use std::{fs::File, io::Write};