only use tedge-write if no write permissions

Configuration file deployment was changed to always do a regular copy first, and only use tedge-write if copy failes due to lacking permissions. Signed-off-by: Marcel Guzik <[email protected]>
thin-edge · Aug 21, 2024 · e130de6 · e130de6
1 parent 9da30d3
commit e130de6
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 20 deletions.
diff --git a/crates/extensions/tedge_config_manager/src/actor.rs b/crates/extensions/tedge_config_manager/src/actor.rs
@@ -6,6 +6,7 @@ use log::error;
 use log::info;
 use serde_json::json;
 use std::collections::HashMap;
+use std::io::ErrorKind;
 use tedge_actors::fan_in_message_type;
 use tedge_actors::Actor;
 use tedge_actors::ChannelError;
@@ -30,6 +31,7 @@ use tedge_mqtt_ext::QoS;
 use tedge_mqtt_ext::Topic;
 use tedge_uploader_ext::UploadRequest;
 use tedge_uploader_ext::UploadResult;
+use tedge_utils::fs::AtomFileError;
 use tedge_write::CopyOptions;
 
 use crate::TedgeWriteStatus;
@@ -420,9 +422,8 @@ impl ConfigManagerActor {
     /// deployed.
     ///
     /// This function ensures that the configuration file under `dest` is overwritten by a new
-    /// version currently stored in a temporary directory under `src`. Depending on if
-    /// `use_tedge_write` is used, either a new `tedge-write` process is spawned, or a file is
-    /// copied directly.
+    /// version currently stored in a temporary directory under `src`. If we have no write
+    /// permissions, use tedge-write for permission elevation if `use_tedge_write` is enabled.
     fn deploy_config_file(
         &self,
         from: &Utf8Path,
@@ -435,25 +436,36 @@ impl ConfigManagerActor {
         let group = file_entry.file_permissions.group.as_deref();
 
         let to = Utf8PathBuf::from(&file_entry.path);
+        let src_file = std::fs::File::open(from)?;
 
-        match self.config.use_tedge_write.clone() {
-            TedgeWriteStatus::Disabled => {
-                let src_file = std::fs::File::open(from)?;
-                tedge_utils::fs::atomically_write_file_sync(&to, src_file)?;
+        // try doing a regular copy, return if success or error other than permissions
+        debug!("deploying config file from '{from}' to '{to}'");
+        let err = match tedge_utils::fs::atomically_write_file_sync(&to, src_file) {
+            Ok(()) => return Ok(to),
+            Err(err) => {
+                let AtomFileError::WriteError { source, .. } = &err;
+                if source.kind() != ErrorKind::PermissionDenied {
+                    return Err(err.into());
+                }
+                err
             }
+        };
+        // if we got permission denied and tedge-write is enabled, use it for privilege elevation
+        let TedgeWriteStatus::Enabled { sudo } = self.config.use_tedge_write.clone() else {
+            return Err(err.into());
+        };
 
-            TedgeWriteStatus::Enabled { sudo } => {
-                let options = CopyOptions {
-                    from,
-                    to: to.as_path(),
-                    sudo,
-                    mode,
-                    user,
-                    group,
-                };
-                options.copy()?;
-            }
-        }
+        debug!("using tedge-write for privilege elevation");
+
+        let options = CopyOptions {
+            from,
+            to: to.as_path(),
+            sudo,
+            mode,
+            user,
+            group,
+        };
+        options.copy()?;
 
         Ok(to)
     }

diff --git a/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot b/tests/RobotFramework/tests/cumulocity/configuration/configuration_operation.robot
@@ -30,7 +30,7 @@ Set Configuration when file does not exist
     Text file (Child Device)    ${CHILD_SN}    ${PARENT_SN}:device:${CHILD_SN}    CONFIG1    /etc/config1.json    ${CURDIR}/config1-version2.json    640    tedge:tedge    delete_file_before=${true}
     Binary file (Child Device)    ${CHILD_SN}    ${PARENT_SN}:device:${CHILD_SN}    CONFIG1_BINARY    /etc/binary-config1.tar.gz    ${CURDIR}/binary-config1.tar.gz    640    tedge:tedge    delete_file_before=${true}
 
-Set Configuration when file exists
+Set Configuration when file exists and tedge has no write permissions
     [Tags]    \#2972
     [Template]    Set Configuration from Device
     [Documentation]    If the configuration file already exists, it should be overwritten, but owner and permissions
@@ -40,6 +40,16 @@ Set Configuration when file exists
     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 when file exists and tedge has write permissions
+    [Tags]    \#2972
+    [Template]    Set Configuration from Device
+    [Documentation]    If the configuration file already exists, it should be overwritten, but owner and permissions
+    ...    should remain unchanged.
+    Text file (Main Device)    ${PARENT_SN}    ${PARENT_SN}    CONFIG1    /etc/config1.json    ${CURDIR}/config1-version2.json    664    tedge:tedge    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    tedge:tedge    delete_file_before=${false}
+    Text file (Child Device)    ${CHILD_SN}    ${PARENT_SN}:device:${CHILD_SN}    CONFIG1    /etc/config1.json    ${CURDIR}/config1-version2.json    664    tedge:tedge    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    tedge:tedge    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
@@ -242,12 +252,20 @@ Set Configuration from Device
     ...    ${permission}
     ...    ${ownership}
     ...    ${delete_file_before}=${true}
+    # to check that configuration file deploy doesnt use tedge-write when normal write would be
+    # fine, tedge-write is deleted so that it fails if it's attempted to be used by tedge-agent
+    ...    ${tedge_write}=${true}
 
     IF    ${delete_file_before}
         ThinEdgeIO.Set Device Context    ${device}
         Execute Command    rm -f ${device_file}
     END
 
+    IF    ${tedge_write} == ${false}
+        ThinEdgeIO.Set Device Context    ${device}
+        Execute Command    mv /usr/bin/tedge-write /usr/bin/tedge-write.bak
+    END
+
     Cumulocity.Set Device    ${external_id}
     ${config_url}=    Cumulocity.Create Inventory Binary    temp_file    ${config_type}    file=${file}
     ${operation}=    Cumulocity.Set Configuration    ${config_type}    url=${config_url}
@@ -257,6 +275,11 @@ Set Configuration from Device
     File Checksum Should Be Equal    ${device_file}    ${file}
     Path Should Have Permissions    ${device_file}    ${permission}    ${ownership}
 
+    IF    ${tedge_write} == ${false}
+        ThinEdgeIO.Set Device Context    ${device}
+        Execute Command    mv /usr/bin/tedge-write.bak /usr/bin/tedge-write
+    END
+
 Set Configuration from URL
     [Arguments]    ${test_desc}    ${device}    ${external_id}    ${config_type}    ${device_file}    ${config_url}