[v2] tests: handle AppArmor userns mitigation

Message ID 20240528162121.93250-1-simon.chopin@canonical.com
State Changes Requested
Headers
Series [v2] tests: handle AppArmor userns mitigation |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed

Commit Message

Simon Chopin May 28, 2024, 4:21 p.m. UTC
  Support for flat denial of user namespace creation from AppArmor was
added in commit 59e0441d4a ("tests: gracefully handle AppArmor userns
containment"). However, AppArmor supports another mode where it allows
the namespace creation but denies privileged actions within that
namespace.

The mitigation mode will be enabled by default in Ubuntu 24.04.

This patch adds support for properly skipping the tests when that
happens (usually when calling mount() or writing to /proc/self/uid_map)

Further info:
* AppArmor user namespace restriction modes:
  https://gitlab.com/apparmor/apparmor/-/wikis/unprivileged_userns_restriction#two-types-of-restrictions
* Error codes for AppArmor-denied syscals:
  https://gitlab.com/apparmor/apparmor/-/wikis/manpage_apparmor.d.5#profile-mode

V2:
* Use xasprintf rather than sprintf into static buffers

Signed-off-by: Simon Chopin <simon.chopin@canonical.com>
---
 support/support_become_root.c                 | 42 ++++++++++++++-----
 support/test-container.c                      | 12 +++++-
 .../unix/sysv/linux/tst-getcwd-smallbuff.c    | 17 +++++---
 sysdeps/unix/sysv/linux/tst-pidfd_getpid.c    |  2 +-
 4 files changed, 55 insertions(+), 18 deletions(-)


base-commit: b62928f9070c6f3c5cc43a4cb89b4bfb950d7406
  

Comments

Florian Weimer June 3, 2024, 5:25 p.m. UTC | #1
* Simon Chopin:

> diff --git a/support/support_become_root.c b/support/support_become_root.c
> index 5920fc8236..51368507bc 100644
> --- a/support/support_become_root.c
> +++ b/support/support_become_root.c
> @@ -22,12 +22,41 @@

> +static void
> +write_id_map (int map_fd, unsigned long long src,
> +                      unsigned long long dst, unsigned long long range)
> +{
> +  char *map_buf = xasprintf ("%llu %llu %llu\n", src, dst, range);
> +  size_t size = strlen(map_buf);
> +  int ret = write (map_fd, map_buf, size);
> +  free(map_buf);

Sorry, just a few style nits.

Space before “(” in “strlen(” and “free(”.

> +  if (ret < 0)
> +    {
> +      if (errno == EPERM || errno == EACCES)
> +        {
> +          /* Likely a LSM deny.  */
> +          FAIL_UNSUPPORTED (
> +              "Could not write ID map file, check security settings: %m\n");

GNU style suggests:

         FAIL_UNSUPPORTED ("\
"Could not write ID map file, check security settings: %m\n");

Or:
         FAIL_UNSUPPORTED ("Could not write ID map file,"
                           " check security settings: %m\n");

In the latter case, the second line news to be indented so that the
string literals start in the same column.

> +  else if (ret < size)
> +    {
> +      /* Retrying would just fail with EPERM, see user_namespaces(7).  */
> +      FAIL_EXIT1 (
> +          "couldn't write the entire buffer at once to the ID file: %m\n");
> +    }

Likewise.

Rest looks good to me.

Thanks,
Florian
  

Patch

diff --git a/support/support_become_root.c b/support/support_become_root.c
index 5920fc8236..51368507bc 100644
--- a/support/support_become_root.c
+++ b/support/support_become_root.c
@@ -22,12 +22,41 @@ 
 #include <fcntl.h>
 #include <sched.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
+#include <support.h>
 #include <support/check.h>
 #include <support/xunistd.h>
 #include <unistd.h>
 
 #ifdef CLONE_NEWUSER
+static void
+write_id_map (int map_fd, unsigned long long src,
+                      unsigned long long dst, unsigned long long range)
+{
+  char *map_buf = xasprintf ("%llu %llu %llu\n", src, dst, range);
+  size_t size = strlen(map_buf);
+  int ret = write (map_fd, map_buf, size);
+  free(map_buf);
+  if (ret < 0)
+    {
+      if (errno == EPERM || errno == EACCES)
+        {
+          /* Likely a LSM deny.  */
+          FAIL_UNSUPPORTED (
+              "Could not write ID map file, check security settings: %m\n");
+        }
+      else
+        FAIL_EXIT1 ("Could not write ID map file: %m\n");
+    }
+  else if (ret < size)
+    {
+      /* Retrying would just fail with EPERM, see user_namespaces(7).  */
+      FAIL_EXIT1 (
+          "couldn't write the entire buffer at once to the ID file: %m\n");
+    }
+}
+
 /* The necessary steps to allow file creation in user namespaces.  */
 static void
 setup_uid_gid_mapping (uid_t original_uid, gid_t original_gid)
@@ -43,12 +72,7 @@  setup_uid_gid_mapping (uid_t original_uid, gid_t original_gid)
   /* We map our original UID to the same UID in the container so we
      own our own files normally.  Without that, file creation could
      fail with EOVERFLOW (sic!).  */
-  char buf[100];
-  int ret = snprintf (buf, sizeof (buf), "%llu %llu 1\n",
-                      (unsigned long long) original_uid,
-                      (unsigned long long) original_uid);
-  TEST_VERIFY_EXIT (ret < sizeof (buf));
-  xwrite (fd, buf, ret);
+  write_id_map (fd, original_uid, original_uid, 1);
   xclose (fd);
 
   /* Linux 3.19 introduced the setgroups file.  We need write "deny" to this
@@ -69,11 +93,7 @@  setup_uid_gid_mapping (uid_t original_uid, gid_t original_gid)
 
   /* Now map our own GID, like we did for the user ID.  */
   fd = xopen ("/proc/self/gid_map", O_WRONLY, 0);
-  ret = snprintf (buf, sizeof (buf), "%llu %llu 1\n",
-                  (unsigned long long) original_gid,
-                  (unsigned long long) original_gid);
-  TEST_VERIFY_EXIT (ret < sizeof (buf));
-  xwrite (fd, buf, ret);
+  write_id_map (fd, original_gid, original_gid, 1);
   xclose (fd);
 }
 #endif /* CLONE_NEWUSER */
diff --git a/support/test-container.c b/support/test-container.c
index ebcc722da5..fd45cbfa57 100644
--- a/support/test-container.c
+++ b/support/test-container.c
@@ -1133,7 +1133,17 @@  main (int argc, char **argv)
 
   /* Some systems, by default, all mounts leak out of the namespace.  */
   if (mount ("none", "/", NULL, MS_REC | MS_PRIVATE, NULL) != 0)
-    FAIL_EXIT1 ("could not create a private mount namespace\n");
+    {
+      int saved_errno = errno;
+      if (errno == EPERM || errno == EACCES)
+        {
+          check_for_unshare_hints (require_pidns);
+          FAIL_UNSUPPORTED ("could not create a private mount namespace "
+                            "(security policy?: %s)",
+                            strerror (saved_errno));
+        }
+      FAIL_EXIT1 ("could not create a private mount namespace\n");
+    }
 
   trymount (support_srcdir_root, new_srcdir_path);
   trymount (support_objdir_root, new_objdir_path);
diff --git a/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c
index 55362f6060..f564be6a63 100644
--- a/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c
+++ b/sysdeps/unix/sysv/linux/tst-getcwd-smallbuff.c
@@ -133,7 +133,13 @@  child_func (void * const arg)
   TEST_VERIFY_EXIT (ch == '1');
 
   if (mount ("/", MOUNT_NAME, NULL, MS_BIND | MS_REC, NULL))
-    FAIL_EXIT1 ("mount failed: %m\n");
+    {
+      /* Probably rejected by local security policy.  */
+      if (errno == EPERM || errno == EACCES)
+        FAIL_UNSUPPORTED ("mount failed (security policy?): %m\n");
+      else
+        FAIL_EXIT1 ("mount failed: %m\n");
+    }
   const int fd = xopen ("mpoint",
 			O_RDONLY | O_PATH | O_DIRECTORY | O_NOFOLLOW, 0);
 
@@ -194,10 +200,11 @@  do_test (void)
     pid_t pid = xfork ();
     if (pid == 0)
       {
-	if (unshare (CLONE_NEWUSER | CLONE_NEWNS) != 0)
-	  _exit (EXIT_UNSUPPORTED);
-	else
-	  _exit (0);
+        if (unshare (CLONE_NEWUSER | CLONE_NEWNS) != 0
+            || mount ("none", "/", NULL, MS_REC | MS_PRIVATE, NULL) != 0)
+          _exit (EXIT_UNSUPPORTED);
+        else
+          _exit (0);
       }
     int status;
     xwaitpid (pid, &status, 0);
diff --git a/sysdeps/unix/sysv/linux/tst-pidfd_getpid.c b/sysdeps/unix/sysv/linux/tst-pidfd_getpid.c
index ef62fbe941..89233ac4a1 100644
--- a/sysdeps/unix/sysv/linux/tst-pidfd_getpid.c
+++ b/sysdeps/unix/sysv/linux/tst-pidfd_getpid.c
@@ -72,7 +72,7 @@  do_test (void)
 	    /* This happens if we're trying to create a nested container,
 	       like if the build is running under podman, and we lack
 	       priviledges.  */
-	    if (errno  == EPERM)
+	    if (errno  == EPERM || errno == EACCES)
 	      _exit (EXIT_UNSUPPORTED);
 	    else
 	      _exit (EXIT_FAILURE);