diff options
author | Didier Wenzek <didier.wenzek@free.fr> | 2022-07-15 10:56:44 +0100 |
---|---|---|
committer | Didier Wenzek <didier.wenzek@free.fr> | 2022-07-15 10:56:44 +0100 |
commit | 9eff5cff05dd7f5f803ba6618ae619967cb0ca9f (patch) | |
tree | 9089342c59f39a947162c615df66e9ddf1cea7e9 /crates/core | |
parent | a8c01e665a5cfef160f50adef46544ef00e04016 (diff) |
Remove the `tedge` dependency on `UserManager`
The UserManager was used to run the `tedge` command as the `tedge` user
becomming `root` or `mosquitto` only when required.
In practice, this introduced more issues than benefits.
Notably, this forces to have a `tedge` user to run the command.
While switching to a different user is required only to create a
certificate under mosquitto.
The behavior is unchanged. Particularly, errors are ignored when giving
ownership of the certificate to mosquitto
(for instance, if the `tedge cert create` command is not run with `sudo`).
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Diffstat (limited to 'crates/core')
-rw-r--r-- | crates/core/tedge/Cargo.toml | 1 | ||||
-rw-r--r-- | crates/core/tedge/src/cli/certificate/cli.rs | 2 | ||||
-rw-r--r-- | crates/core/tedge/src/cli/certificate/create.rs | 34 | ||||
-rw-r--r-- | crates/core/tedge/src/cli/certificate/error.rs | 4 | ||||
-rw-r--r-- | crates/core/tedge/src/cli/certificate/remove.rs | 5 | ||||
-rw-r--r-- | crates/core/tedge/src/cli/connect/c8y_direct_connection.rs | 18 | ||||
-rw-r--r-- | crates/core/tedge/src/cli/connect/cli.rs | 12 | ||||
-rw-r--r-- | crates/core/tedge/src/cli/connect/command.rs | 5 | ||||
-rw-r--r-- | crates/core/tedge/src/cli/connect/error.rs | 5 | ||||
-rw-r--r-- | crates/core/tedge/src/cli/disconnect/cli.rs | 10 | ||||
-rw-r--r-- | crates/core/tedge/src/command.rs | 3 | ||||
-rw-r--r-- | crates/core/tedge/src/main.rs | 7 | ||||
-rw-r--r-- | crates/core/tedge/src/system_services/manager.rs | 7 | ||||
-rw-r--r-- | crates/core/tedge/src/system_services/managers/general_manager.rs | 10 |
14 files changed, 30 insertions, 93 deletions
diff --git a/crates/core/tedge/Cargo.toml b/crates/core/tedge/Cargo.toml index 6c65ac3c..b169f4da 100644 --- a/crates/core/tedge/Cargo.toml +++ b/crates/core/tedge/Cargo.toml @@ -27,7 +27,6 @@ rustls_0_19 = {package = "rustls", version = "0.19.0" } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" tedge_config = { path = "../../common/tedge_config" } -tedge_users = { path = "../../common/tedge_users" } tedge_utils = { path = "../../common/tedge_utils" } thiserror = "1.0" toml = "0.5" diff --git a/crates/core/tedge/src/cli/certificate/cli.rs b/crates/core/tedge/src/cli/certificate/cli.rs index a3e9db64..dcc286cd 100644 --- a/crates/core/tedge/src/cli/certificate/cli.rs +++ b/crates/core/tedge/src/cli/certificate/cli.rs @@ -35,7 +35,6 @@ impl BuildCommand for TEdgeCertCli { id, cert_path: config.query(DeviceCertPathSetting)?, key_path: config.query(DeviceKeyPathSetting)?, - user_manager: context.user_manager, }; cmd.into_boxed() } @@ -51,7 +50,6 @@ impl BuildCommand for TEdgeCertCli { let cmd = RemoveCertCmd { cert_path: config.query(DeviceCertPathSetting)?, key_path: config.query(DeviceKeyPathSetting)?, - user_manager: context.user_manager, }; cmd.into_boxed() } diff --git a/crates/core/tedge/src/cli/certificate/create.rs b/crates/core/tedge/src/cli/certificate/create.rs index 560c2a8f..2d1589c9 100644 --- a/crates/core/tedge/src/cli/certificate/create.rs +++ b/crates/core/tedge/src/cli/certificate/create.rs @@ -7,7 +7,6 @@ use std::{ path::Path, }; use tedge_config::*; -use tedge_users::UserManager; use tedge_utils::paths::{set_permission, validate_parent_dir_exists}; /// Create a self-signed device certificate @@ -20,9 +19,6 @@ pub struct CreateCertCmd { /// The path where the device private key will be stored pub key_path: FilePath, - - /// The UserManager required to change effective user id - pub user_manager: UserManager, } impl Command for CreateCertCmd { @@ -39,17 +35,16 @@ impl Command for CreateCertCmd { impl CreateCertCmd { fn create_test_certificate(&self, config: &NewCertificateConfig) -> Result<(), CertError> { - let _user_guard = self.user_manager.become_user(tedge_users::BROKER_USER)?; - validate_parent_dir_exists(&self.cert_path).map_err(CertError::CertPathError)?; validate_parent_dir_exists(&self.key_path).map_err(CertError::KeyPathError)?; let cert = KeyCertPair::new_selfsigned_certificate(config, &self.id)?; - // Creating files with permission 644 - let mut cert_file = create_new_file(&self.cert_path) - .map_err(|err| err.cert_context(self.cert_path.clone()))?; - let mut key_file = create_new_file(&self.key_path) + // Creating files with permission 644 owned by the MQTT broker + let mut cert_file = + create_new_file(&self.cert_path, crate::BROKER_USER, crate::BROKER_GROUP) + .map_err(|err| err.cert_context(self.cert_path.clone()))?; + let mut key_file = create_new_file(&self.key_path, crate::BROKER_USER, crate::BROKER_GROUP) .map_err(|err| err.key_context(self.key_path.clone()))?; let cert_pem = cert.certificate_pem_string()?; @@ -76,8 +71,18 @@ impl CreateCertCmd { } } -fn create_new_file(path: impl AsRef<Path>) -> Result<File, CertError> { - Ok(OpenOptions::new().write(true).create_new(true).open(path)?) +fn create_new_file(path: impl AsRef<Path>, user: &str, group: &str) -> Result<File, CertError> { + let file = OpenOptions::new() + .write(true) + .create_new(true) + .open(path.as_ref())?; + + // Ignore errors - This was the behavior with the now deprecated user manager. + // - When `tedge cert create` is not run as root, a certificate is created but owned by the user running the command. + // - A better approach could be to remove this `chown` and run the command as mosquitto. + let _ = tedge_utils::file::change_user_and_group(path.as_ref(), user, group); + + Ok(file) } #[cfg(test)] @@ -85,7 +90,6 @@ mod tests { use super::*; use assert_matches::assert_matches; use std::fs; - use tedge_users::UserManager; use tempfile::*; #[test] @@ -99,7 +103,6 @@ mod tests { id: String::from(id), cert_path: cert_path.clone(), key_path: key_path.clone(), - user_manager: UserManager::new(), }; assert_matches!( @@ -127,7 +130,6 @@ mod tests { id: "my-device-id".into(), cert_path: cert_path.clone(), key_path: key_path.clone(), - user_manager: UserManager::new(), }; assert!(cmd @@ -149,7 +151,6 @@ mod tests { id: "my-device-id".into(), cert_path, key_path, - user_manager: UserManager::new(), }; let cert_error = cmd @@ -168,7 +169,6 @@ mod tests { id: "my-device-id".into(), cert_path, key_path, - user_manager: UserManager::new(), }; let cert_error = cmd diff --git a/crates/core/tedge/src/cli/certificate/error.rs b/crates/core/tedge/src/cli/certificate/error.rs index 8fffc417..8193350d 100644 --- a/crates/core/tedge/src/cli/certificate/error.rs +++ b/crates/core/tedge/src/cli/certificate/error.rs @@ -2,7 +2,6 @@ use reqwest::StatusCode; use std::error::Error; use tedge_config::FilePath; use tedge_config::{ConfigSettingError, TEdgeConfigError}; -use tedge_users::UserSwitchError; use tedge_utils::paths::PathsError; #[derive(thiserror::Error, Debug)] @@ -72,9 +71,6 @@ pub enum CertError { #[error(transparent)] UrlParseError(#[from] url::ParseError), - #[error(transparent)] - UserSwitchError(#[from] UserSwitchError), - #[error("HTTP Connection Problem: {msg} \nHint: {hint}")] CertificateValidationFailure { hint: String, msg: String }, diff --git a/crates/core/tedge/src/cli/certificate/remove.rs b/crates/core/tedge/src/cli/certificate/remove.rs index e46acfbf..8f978972 100644 --- a/crates/core/tedge/src/cli/certificate/remove.rs +++ b/crates/core/tedge/src/cli/certificate/remove.rs @@ -1,7 +1,6 @@ use super::error::CertError; use crate::command::Command; use tedge_config::*; -use tedge_users::UserManager; use tedge_utils::paths::ok_if_not_found; /// Remove the device certificate @@ -11,9 +10,6 @@ pub struct RemoveCertCmd { /// The path of the private key to be removed pub key_path: FilePath, - - /// The UserManager required to change effective user id. - pub user_manager: UserManager, } impl Command for RemoveCertCmd { @@ -29,7 +25,6 @@ impl Command for RemoveCertCmd { impl RemoveCertCmd { fn remove_certificate(&self) -> Result<(), CertError> { - let _user_guard = self.user_manager.become_user(tedge_users::BROKER_USER)?; std::fs::remove_file(&self.cert_path).or_else(ok_if_not_found)?; std::fs::remove_file(&self.key_path).or_else(ok_if_not_found)?; diff --git a/crates/core/tedge/src/cli/connect/c8y_direct_connection.rs b/crates/core/tedge/src/cli/connect/c8y_direct_connection.rs index bba72e6a..89400667 100644 --- a/crates/core/tedge/src/cli/connect/c8y_direct_connection.rs +++ b/crates/core/tedge/src/cli/connect/c8y_direct_connection.rs @@ -13,11 +13,8 @@ use std::path::PathBuf; use std::{fs::File, io::BufReader}; use tedge_config::FilePath; -use tedge_users::UserManager; - // Connect directly to the c8y cloud over mqtt and publish device create message. pub fn create_device_with_direct_connection( - user_manager: UserManager, bridge_config: &BridgeConfig, device_type: &str, ) -> Result<(), ConnectError> { @@ -37,7 +34,7 @@ pub fn create_device_with_direct_connection( bridge_config.bridge_root_cert_path.clone(), )?; - let pvt_key = read_pvt_key(user_manager, bridge_config.bridge_keyfile.clone())?; + let pvt_key = read_pvt_key(bridge_config.bridge_keyfile.clone())?; let cert_chain = read_cert_chain(bridge_config.bridge_certfile.clone())?; let _ = client_config.set_single_client_cert(cert_chain, pvt_key); @@ -139,12 +136,7 @@ fn add_root_cert( Ok(()) } -fn read_pvt_key( - user_manager: UserManager, - key_file: tedge_config::FilePath, -) -> Result<rustls_0_19::PrivateKey, ConnectError> { - // Become BROKER_USER to read the private key - let _user_guard = user_manager.become_user(tedge_users::BROKER_USER)?; +fn read_pvt_key(key_file: tedge_config::FilePath) -> Result<rustls_0_19::PrivateKey, ConnectError> { parse_pkcs8_key(key_file.clone()).or_else(|_| parse_rsa_key(key_file)) } @@ -213,7 +205,6 @@ mod tests { #[test] fn parse_supported_key() { - let user_manager = UserManager::new(); let key = concat!( "-----BEGIN RSA PRIVATE KEY-----\n", "MC4CAQ\n", @@ -221,14 +212,13 @@ mod tests { ); let mut temp_file = NamedTempFile::new().unwrap(); temp_file.write_all(key.as_bytes()).unwrap(); - let parsed_key = read_pvt_key(user_manager, temp_file.path().into()).unwrap(); + let parsed_key = read_pvt_key(temp_file.path().into()).unwrap(); let expected_pvt_key = rustls_0_19::PrivateKey(vec![48, 46, 2, 1]); assert_eq!(parsed_key, expected_pvt_key); } #[test] fn parse_unsupported_key() { - let user_manager = UserManager::new(); let key = concat!( "-----BEGIN DSA PRIVATE KEY-----\n", "MC4CAQ\n", @@ -236,7 +226,7 @@ mod tests { ); let mut temp_file = NamedTempFile::new().unwrap(); temp_file.write_all(key.as_bytes()).unwrap(); - let err = read_pvt_key(user_manager, temp_file.path().into()).unwrap_err(); + let err = read_pvt_key(temp_file.path().into()).unwrap_err(); assert!(matches!(err, ConnectError::UnknownPrivateKeyFormat)); } } diff --git a/crates/core/tedge/src/cli/connect/cli.rs b/crates/core/tedge/src/cli/connect/cli.rs index 654af945..7253d1a7 100644 --- a/crates/core/tedge/src/cli/connect/cli.rs +++ b/crates/core/tedge/src/cli/connect/cli.rs @@ -32,11 +32,7 @@ impl BuildCommand for TEdgeConnectOpt { cloud: Cloud::C8y, common_mosquitto_config: CommonMosquittoConfig::default(), is_test_connection, - service_manager: service_manager( - context.user_manager.clone(), - context.config_location.tedge_config_root_path, - )?, - user_manager: context.user_manager, + service_manager: service_manager(context.config_location.tedge_config_root_path)?, }, TEdgeConnectOpt::Az { is_test_connection } => ConnectCommand { config_location: context.config_location.clone(), @@ -44,11 +40,7 @@ impl BuildCommand for TEdgeConnectOpt { cloud: Cloud::Azure, common_mosquitto_config: CommonMosquittoConfig::default(), is_test_connection, - service_manager: service_manager( - context.user_manager.clone(), - context.config_location.tedge_config_root_path, - )?, - user_manager: context.user_manager, + service_manager: service_manager(context.config_location.tedge_config_root_path)?, }, } .into_boxed()) diff --git a/crates/core/tedge/src/cli/connect/command.rs b/crates/core/tedge/src/cli/connect/command.rs index 7da926d0..faed5fde 100644 --- a/crates/core/tedge/src/cli/connect/command.rs +++ b/crates/core/tedge/src/cli/connect/command.rs @@ -7,7 +7,6 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; use tedge_config::*; -use tedge_users::UserManager; use tedge_utils::paths::{create_directories, ok_if_not_found, DraftFile}; use which::which; @@ -26,7 +25,6 @@ pub struct ConnectCommand { pub common_mosquitto_config: CommonMosquittoConfig, pub is_test_connection: bool, pub service_manager: Arc<dyn SystemServiceManager>, - pub user_manager: UserManager, } pub enum DeviceStatus { @@ -135,7 +133,6 @@ impl Command for ConnectCommand { &bridge_config, &updated_mosquitto_config, self.service_manager.as_ref(), - self.user_manager.clone(), &self.config_location, &device_type, )?; @@ -389,7 +386,6 @@ fn new_bridge( bridge_config: &BridgeConfig, common_mosquitto_config: &CommonMosquittoConfig, service_manager: &dyn SystemServiceManager, - user_manager: UserManager, config_location: &TEdgeConfigLocation, device_type: &str, ) -> Result<(), ConnectError> { @@ -414,7 +410,6 @@ fn new_bridge( if bridge_config.cloud_name.eq("c8y") { println!("Creating the device in Cumulocity cloud.\n"); let () = c8y_direct_connection::create_device_with_direct_connection( - user_manager, bridge_config, device_type, )?; diff --git a/crates/core/tedge/src/cli/connect/error.rs b/crates/core/tedge/src/cli/connect/error.rs index 4dc9453e..07ddbfbc 100644 --- a/crates/core/tedge/src/cli/connect/error.rs +++ b/crates/core/tedge/src/cli/connect/error.rs @@ -1,5 +1,3 @@ -use tedge_users::UserSwitchError; - #[derive(thiserror::Error, Debug)] pub enum ConnectError { #[error("Couldn't load certificate, provide valid certificate path in configuration. Use 'tedge config --set'")] @@ -51,7 +49,4 @@ pub enum ConnectError { #[error("Could not parse certificate")] RumqttcCertificate, - - #[error(transparent)] - UserSwitchError(#[from] UserSwitchError), } diff --git a/crates/core/tedge/src/cli/disconnect/cli.rs b/crates/core/tedge/src/cli/disconnect/cli.rs index e13a9250..3afb8c26 100644 --- a/crates/core/tedge/src/cli/disconnect/cli.rs +++ b/crates/core/tedge/src/cli/disconnect/cli.rs @@ -22,10 +22,7 @@ impl BuildCommand for TEdgeDisconnectBridgeCli { cloud: Cloud::C8y, use_mapper: true, use_agent: true, - service_manager: service_manager( - context.user_manager.clone(), - context.config_location.tedge_config_root_path, - )?, + service_manager: service_manager(context.config_location.tedge_config_root_path)?, }, TEdgeDisconnectBridgeCli::Az => DisconnectBridgeCommand { config_location: context.config_location.clone(), @@ -33,10 +30,7 @@ impl BuildCommand for TEdgeDisconnectBridgeCli { cloud: Cloud::Azure, use_mapper: true, use_agent: false, - service_manager: service_manager( - context.user_manager.clone(), - context.config_location.tedge_config_root_path, - )?, + service_manager: service_manager(context.config_location.tedge_config_root_path)?, }, }; Ok(cmd.into_boxed()) diff --git a/crates/core/tedge/src/command.rs b/crates/core/tedge/src/command.rs index 0ad255f5..759afa4f 100644 --- a/crates/core/tedge/src/command.rs +++ b/crates/core/tedge/src/command.rs @@ -1,5 +1,3 @@ -use tedge_users::UserManager; - /// A trait to be implemented by all tedge sub-commands. /// /// A command encapsulates all the required parameters and provides an `execute()` method @@ -150,5 +148,4 @@ pub trait BuildCommand { pub struct BuildContext { pub config_repository: tedge_config::TEdgeConfigRepository, pub config_location: tedge_config::TEdgeConfigLocation, - pub user_manager: UserManager, } diff --git a/crates/core/tedge/src/main.rs b/crates/core/tedge/src/main.rs index e4dc777e..ea5ccd9a 100644 --- a/crates/core/tedge/src/main.rs +++ b/crates/core/tedge/src/main.rs @@ -5,7 +5,6 @@ use std::path::Path; use anyhow::Context; use clap::Parser; -use tedge_users::UserManager; mod cli; mod command; mod error; @@ -16,6 +15,9 @@ type ConfigError = crate::error::TEdgeError; use command::{BuildCommand, BuildContext}; +const BROKER_USER: &str = "mosquitto"; +const BROKER_GROUP: &str = "mosquitto"; + fn main() -> anyhow::Result<()> { let opt = cli::Opt::parse(); @@ -24,15 +26,12 @@ fn main() -> anyhow::Result<()> { return Ok(()); } - let user_manager = UserManager::new(); let tedge_config_location = tedge_config::TEdgeConfigLocation::from_custom_root(opt.config_dir); - let _user_guard = user_manager.become_user(tedge_users::TEDGE_USER)?; let config_repository = tedge_config::TEdgeConfigRepository::new(tedge_config_location.clone()); let build_context = BuildContext { config_repository, config_location: tedge_config_location, - user_manager, }; if let Some(tedge_opt) = opt.tedge { diff --git a/crates/core/tedge/src/system_services/manager.rs b/crates/core/tedge/src/system_services/manager.rs index 7c39ea8c..83b4cd56 100644 --- a/crates/core/tedge/src/system_services/manager.rs +++ b/crates/core/tedge/src/system_services/manager.rs @@ -2,7 +2,6 @@ use crate::system_services::*; use std::fmt::Debug; use std::path::PathBuf; use std::sync::Arc; -use tedge_users::UserManager; /// Abstraction over the system-provided facility that manages starting, stopping as well as other /// service-related management functions of system services. @@ -43,11 +42,7 @@ pub trait SystemServiceManager: Debug { } pub fn service_manager( - user_manager: UserManager, config_root: PathBuf, ) -> Result<Arc<dyn SystemServiceManager>, SystemServiceError> { - Ok(Arc::new(GeneralServiceManager::try_new( - user_manager, - config_root, - )?)) + Ok(Arc::new(GeneralServiceManager::try_new(config_root)?)) } diff --git a/crates/core/tedge/src/system_services/managers/general_manager.rs b/crates/core/tedge/src/system_services/managers/general_manager.rs index fad556e3..dd5f3f8c 100644 --- a/crates/core/tedge/src/system_services/managers/general_manager.rs +++ b/crates/core/tedge/src/system_services/managers/general_manager.rs @@ -5,20 +5,15 @@ use crate::system_services::{ use std::fmt; use std::path::PathBuf; use std::process::ExitStatus; -use tedge_users::{UserManager, ROOT_USER}; #[derive(Debug)] pub struct GeneralServiceManager { - user_manager: UserManager, init_config: InitConfig, config_path: String, } impl GeneralServiceManager { - pub fn try_new( - user_manager: UserManager, - config_root: PathBuf, - ) -> Result<Self, SystemServiceError> { + pub fn try_new(config_root: PathBuf) -> Result<Self, SystemServiceError> { let init_config = SystemConfig::try_new(config_root.clone())?.init; let config_path = config_root .join(SERVICE_CONFIG_FILE) @@ -26,7 +21,6 @@ impl GeneralServiceManager { .unwrap_or(SERVICE_CONFIG_FILE) .to_string(); Ok(Self { - user_manager, init_config, config_path, }) @@ -238,8 +232,6 @@ impl GeneralServiceManager { exec_command: ExecCommand, config_path: &str, ) -> Result<ServiceCommandExitStatus, SystemServiceError> { - let _root_guard = self.user_manager.become_user(ROOT_USER); - exec_command .to_command() .status() |