From 1db67dfe7857bd3af4606a3b63bdfde465306999 Mon Sep 17 00:00:00 2001 From: Marcel Guzik Date: Tue, 3 Sep 2024 14:07:00 +0200 Subject: [PATCH 1/2] Make sudo processes spawned by thin-edge non-interactive When thin-edge spawns processes using sudo without `--non-inteactive` flag, sudo printed the following message: ``` sudo: a terminal is required to read the password; either use the -S option to read from standard input or configure an askpass helper ``` The flag makes sudo not ask for a password and instead fail immediately if a password is required. Signed-off-by: Marcel Guzik --- crates/common/tedge_config/src/sudo.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/common/tedge_config/src/sudo.rs b/crates/common/tedge_config/src/sudo.rs index 4ecca4221f..f0e62a6016 100644 --- a/crates/common/tedge_config/src/sudo.rs +++ b/crates/common/tedge_config/src/sudo.rs @@ -54,6 +54,8 @@ impl SudoCommandBuilder { match which::which(self.sudo_program.as_ref()) { Ok(sudo) => { let mut c = Command::new(sudo); + // non-interactive + c.arg("-n"); c.arg(program); c } From f8e94a5555136df1f01ac4e27b4494622188b20d Mon Sep 17 00:00:00 2001 From: Marcel Guzik Date: Mon, 2 Sep 2024 17:58:38 +0200 Subject: [PATCH 2/2] fix: Remove hardcoded tedge-write path API for spawning a tedge-write process no longer uses a hardcoded `/usr/bin/tedge-write` path. The path was hardcoded because of the wrong assumption of how `sudo` works: when provided with a command without the full path, e.g. `sudo tedge-write /etc/config1`, `sudo` will internally look for `tedge-write` in $PATH and then compare path with its sudoers entry, so it's not necessary to use full path for binaries used with `sudo`. Signed-off-by: Marcel Guzik --- crates/core/tedge_write/src/api.rs | 8 ++- crates/core/tedge_write/src/lib.rs | 2 +- .../configuration_operation.robot | 63 +++++++++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/crates/core/tedge_write/src/api.rs b/crates/core/tedge_write/src/api.rs index 5af924d1c4..1be8d6887f 100644 --- a/crates/core/tedge_write/src/api.rs +++ b/crates/core/tedge_write/src/api.rs @@ -6,6 +6,8 @@ use camino::Utf8Path; use std::process::Command; use tedge_config::SudoCommandBuilder; +use crate::TEDGE_WRITE_BINARY; + /// Options for copying files using a `tedge-write` process. #[derive(Debug, PartialEq)] pub struct CopyOptions<'a> { @@ -57,7 +59,11 @@ impl<'a> CopyOptions<'a> { } fn command(&self) -> anyhow::Result { - let mut command = self.sudo.command(crate::TEDGE_WRITE_PATH); + // if tedge-write is in PATH of tedge process, use it, if not, defer PATH lookup to sudo + let tedge_write_binary = + which::which_global(TEDGE_WRITE_BINARY).unwrap_or(TEDGE_WRITE_BINARY.into()); + + let mut command = self.sudo.command(tedge_write_binary); let from_reader = std::fs::File::open(self.from) .with_context(|| format!("could not open file for reading '{}'", self.from))?; diff --git a/crates/core/tedge_write/src/lib.rs b/crates/core/tedge_write/src/lib.rs index 68ff64cd60..88818e41af 100644 --- a/crates/core/tedge_write/src/lib.rs +++ b/crates/core/tedge_write/src/lib.rs @@ -20,7 +20,7 @@ //! //! https://github.com/thin-edge/thin-edge.io/issues/2456 -const TEDGE_WRITE_PATH: &str = "/usr/bin/tedge-write"; +const TEDGE_WRITE_BINARY: &str = "tedge-write"; pub mod bin; diff --git a/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot b/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot index 1642a77082..820d3e1ae8 100644 --- a/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot +++ b/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot @@ -54,6 +54,13 @@ Set Configuration when file exists and tedge run by root Binary file (Child Device) ${CHILD_SN} ${PARENT_SN}:device:${CHILD_SN} CONFIG1_BINARY /etc/binary-config1.tar.gz ${CURDIR}/binary-config1.tar.gz 664 root:root delete_file_before=${false} ... agent_as_root=${true} +Set Configuration when tedge-write is in another location + [Template] Set Configuration from Device with tedge-write at another location + Text file (Main Device) ${PARENT_SN} ${PARENT_SN} CONFIG1 /etc/config1.json ${CURDIR}/config1-version2.json 664 root:root delete_file_before=${false} + Binary file (Main Device) ${PARENT_SN} ${PARENT_SN} CONFIG1_BINARY /etc/binary-config1.tar.gz ${CURDIR}/binary-config1.tar.gz 664 root:root delete_file_before=${false} + Text file (Child Device) ${CHILD_SN} ${PARENT_SN}:device:${CHILD_SN} CONFIG1 /etc/config1.json ${CURDIR}/config1-version2.json 664 root:root delete_file_before=${false} + Binary file (Child Device) ${CHILD_SN} ${PARENT_SN}:device:${CHILD_SN} CONFIG1_BINARY /etc/binary-config1.tar.gz ${CURDIR}/binary-config1.tar.gz 664 root:root delete_file_before=${false} + Set configuration with broken url [Template] Set Configuration from URL Main Device ${PARENT_SN} ${PARENT_SN} CONFIG1 /etc/config1.json invalid://hellö.zip @@ -294,6 +301,62 @@ Set Configuration from Device END END +Set Configuration from Device with tedge-write at another location + [Documentation] + ... Check if config_update still works if `tedge-write` binary is present at another location. For that we need + ... to make sure that other location is in $PATH and that this new $PATH is inherited by tedge-agent, so for the + ... purposes of the test we change $PATH at the tedge-agent systemd service level. We also add a sudoers entry + ... with new path of tedge-write so sudo correcly elevates permissions. + [Arguments] + ... ${test_desc} + ... ${device} + ... ${external_id} + ... ${config_type} + ... ${device_file} + ... ${file} + ... ${permission} + ... ${ownership} + ... ${delete_file_before}=${true} + [Setup] + [Teardown] + + Set Device Context ${device} + + # Have /opt/tedge/bin in $PATH of tedge-agent + Execute Command mkdir -p /etc/systemd/system/tedge-agent.service.d + Execute Command cmd=echo "[Service]\nEnvironment=\\"PATH=/opt/tedge/bin:$PATH\\"" > /etc/systemd/system/tedge-agent.service.d/10-override-path.conf + Execute Command systemctl daemon-reload + Restart Service tedge-agent + + # put tedge-write in /opt/tedge/bin + Execute Command mkdir -p /opt/tedge/bin + Execute Command mv /usr/bin/tedge-write /opt/tedge/bin/ + Execute Command + ... echo 'tedge ALL \= (ALL) NOPASSWD: /opt/tedge/bin/tedge-write' > /etc/sudoers.d/20-tedge-opt + + TRY + Set Configuration from Device + ... ${test_desc} + ... ${device} + ... ${external_id} + ... ${config_type} + ... ${device_file} + ... ${file} + ... ${permission} + ... ${ownership} + ... ${delete_file_before} + FINALLY + # cleanup + Set Device Context ${device} + + Execute Command mv /opt/tedge/bin/tedge-write /usr/bin/ + Execute Command rm /etc/sudoers.d/20-tedge-opt + + Execute Command rm -r /etc/systemd/system/tedge-agent.service.d + Execute Command systemctl daemon-reload + Restart Service tedge-agent + END + Set Configuration from URL [Arguments] ${test_desc} ${device} ${external_id} ${config_type} ${device_file} ${config_url}