[fix] Ensure proper permissions are set for subsession app directories

First-level `apps_rw` subdirectories' permissions should be directly
copied from the corresponding `/etc/skel/apps_rw/*` subdirectories.

Change-Id: I2fe12c5350feece8c1264b2f4c92ef7509c81845

diff --git a/src/service/src/fs_helpers.cpp b/src/service/src/fs_helpers.cpp
index c87b3f7b279c5cc23d58d5451b1fcb67e69be67f..c052b826a77ebc7b25061206702425ab5aeb0d67 100644 (file)
--- a/src/service/src/fs_helpers.cpp
+++ b/src/service/src/fs_helpers.cpp
@@ -82,8 +82,21 @@ static void change_permissions(std::string_view path, mode_t mode)
                        + "` file/directory");
 }
 
+enum class Directory_Class {
+       App_Name,        /* `[SUBSESSION_DIR]/apps_rw/[APP_NAME]` only */
+       System_Share,    /* `[SUBSESSION_DIR]/apps_rw/[APP_NAME]/cache` and below
+                         * `[SUBSESSION_DIR]/apps_rw/[APP_NAME]/data` and below
+                         * `[SUBSESSION_DIR]/apps_rw/.shared/[APP_NAME]/cache` and below
+                         * `[SUBSESSION_DIR]/apps_rw/.shared/[APP_NAME]/data` and below */
+       Users,           /* Any other subdirectory */
+};
+
+static inline bool should_copy_perms_from_skel(Directory_Class type) {
+       return type == Directory_Class::App_Name;
+}
+
 static void set_ownership_and_perms(std::string_view src_path, std::string_view dest_path, const mode_t mode,
-       const int session_uid, const int gid)
+       const int session_uid, const int gid, Directory_Class type)
 {
        struct stat info;
        int ret = lstat(src_path.data(), &info);
@@ -99,7 +112,7 @@ static void set_ownership_and_perms(std::string_view src_path, std::string_view
        /* Symlinks don't have permissions, and we
         * don't want to change the underlying file's. */
        if (!S_ISLNK(info.st_mode))
-               change_permissions(dest_path, mode);
+               change_permissions(dest_path, should_copy_perms_from_skel(type) ? info.st_mode : mode);
 }
 
 static std::string get_smack_label(std::string_view src_path, smack_label_type type)
@@ -228,23 +241,27 @@ catch (std::exception const &ex) {
 /*
  * The following routine checks what Unix group and permissions should be
  * applied. The logic is as follows:
+ * `[SUBSESSION_DIR]/apps_rw - GROUP=system_share; PERMS=`drwxr-x---`;
+ * `[SUBSESSION_DIR]/apps_rw/[APP_NAME] - GROUP=users; PERMS copied from `/etc/skel/apps_rw/[APP_NAME]`
  * `[SUBSESSION_DIR]/apps_rw/[APP_NAME]/cache` and below         \
- * `[SUBSESSION_DIR]/apps_rw/[APP_NAME]/data` and below           \
+ * `[SUBSESSION_DIR]/apps_rw/[APP_NAME]/data` and below           \ Set explicitly as:
  * `[SUBSESSION_DIR]/apps_rw/.shared/[APP_NAME]/cache` and below  / GROUP=system_share; PERMS=rwxrwsr-x
  * `[SUBSESSION_DIR]/apps_rw/.shared/[APP_NAME]/data` and below  /
- * otherwise: GROUP=system; PERMS=rwxr-xr-x
+ * otherwise: GROUP=users; PERMS=rwxr-xr-x
  */
-static bool is_system_share (std::string_view path)
+
+static Directory_Class get_directory_type(std::string_view path)
 {
        auto first_slash = path.find_first_of('/');
        if (first_slash == std::string_view::npos)
-               return false;
+               return Directory_Class::App_Name;
+
        auto first_level_path = path.substr(0, first_slash);
        if (first_level_path == ".shared") {
                path.remove_prefix(first_slash + 1);
                first_slash = path.find_first_of('/');
                if (first_slash == std::string_view::npos)
-                       return false;
+                       return Directory_Class::Users;
        }
        path.remove_prefix(first_slash + 1);
 
@@ -252,9 +269,10 @@ static bool is_system_share (std::string_view path)
        if (second_slash != std::string_view::npos)
                path.remove_suffix(path.size() - second_slash);
 
-       return path == "cache"
-           || path == "data"
-       ;
+       if (path == "cache" || path == "data")
+               return Directory_Class::System_Share;
+
+       return Directory_Class::Users;
 }
 
 void add_user_subsession(const int session_uid, const std::string_view subsession_id)
@@ -332,15 +350,20 @@ void add_user_subsession(const int session_uid, const std::string_view subsessio
 
                        int gid;
                        mode_t mode;
-                       if (is_system_share(tmp_path)) {
+                       Directory_Class dir_type = get_directory_type(tmp_path);
+                       switch (dir_type) {
+                       case Directory_Class::System_Share:
                                mode = system_share_mode;
                                gid = system_share_gid;
-                       } else {
-                               mode = users_mode;
+                               break;
+                       case Directory_Class::Users:
+                       case Directory_Class::App_Name:
+                               mode = users_mode;      // N.B. won't be used for the first level subdirectory anyway
                                gid = users_gid;
+                               break;
                        }
 
-                       set_ownership_and_perms(s_path, d_path, mode, session_uid, gid);
+                       set_ownership_and_perms(s_path, d_path, mode, session_uid, gid, dir_type);
                        copy_smack_attributes(s_path, d_path);
                }
 

diff --git a/tests/integration/sessiond-integration-tests.sh b/tests/integration/sessiond-integration-tests.sh
index 6e9e8ce5abdc4704cebc46fbd03fdef8e647cacc..703e44a542eb220c8ff3948c5e7c9eb8583d4cfc 100755 (executable)
--- a/tests/integration/sessiond-integration-tests.sh
+++ b/tests/integration/sessiond-integration-tests.sh
@@ -143,9 +143,9 @@ for subsession in "${TEST_SUBSESSIONS[@]}"; do
                else
                        prefix_to_remove="$skeldir/"
                        prefix_removed_f="${templf/#"$prefix_to_remove"}"
-                       first_level=$(echo "$prefix_removed_f" | cut -d/ -f1)
-                       second_level=$(echo "$prefix_removed_f" | cut -d/ -f2)
-                       third_level=$(echo "$prefix_removed_f" | cut -d/ -f3)
+                       first_level=$(echo "${prefix_removed_f}/X_FILLING_X" | cut -d/ -f1)
+                       second_level=$(echo "${prefix_removed_f}/X_FILLING_X" | cut -d/ -f2)
+                       third_level=$(echo "${prefix_removed_f}/X_FILLING_X" | cut -d/ -f3)
 
                        if [ "$first_level" == ".shared" ]; then
                                dir_to_check="$third_level"
@@ -157,13 +157,16 @@ for subsession in "${TEST_SUBSESSIONS[@]}"; do
                                u_group="system_share"
                                u_perms="drwxrwsr-x"
                        fi
+
+                       # N.B. First-level subdirectories' permissions should be directly copied
+                       # from their corresponding /etc/skel/* subdirectories
+                       if [ "$second_level" == "X_FILLING_X" ]; then
+                               u_perms=$(stat --format '%A' "$templf")
+                       fi
+
+                       unset first_level second_level third_level
                fi
 
-               # N.B.: When I was performing tests on RPi4, I encountered
-               # an infamous exception to the above scheme of granting permissions.
-               # `xwalk-service` was the one the has the `rwxrwxr-x` permissions set,
-               # while all other home app directories followed the `rwxr-xr-x` scheme.
-               # This won't break the test, but it's good to keep that in mind.
                if [ "$user" != "$u_user" ] || [ "$group" != "$u_group" ] || [ "$perms" != "$u_perms" ]; then
                        echo "$f: incorrect permissions set!"
                        remove_test_users false