public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] tests: handle AppArmor userns mitigation
@ 2024-05-28 16:21 Simon Chopin
  2024-06-03 17:25 ` Florian Weimer
  0 siblings, 1 reply; 2+ messages in thread
From: Simon Chopin @ 2024-05-28 16:21 UTC (permalink / raw)
  To: libc-alpha; +Cc: Adhemerval Zanella Netto, Simon Chopin

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(-)

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);

base-commit: b62928f9070c6f3c5cc43a4cb89b4bfb950d7406
-- 
2.43.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] tests: handle AppArmor userns mitigation
  2024-05-28 16:21 [PATCH v2] tests: handle AppArmor userns mitigation Simon Chopin
@ 2024-06-03 17:25 ` Florian Weimer
  0 siblings, 0 replies; 2+ messages in thread
From: Florian Weimer @ 2024-06-03 17:25 UTC (permalink / raw)
  To: Simon Chopin; +Cc: libc-alpha, Adhemerval Zanella Netto

* 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


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-06-03 17:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-28 16:21 [PATCH v2] tests: handle AppArmor userns mitigation Simon Chopin
2024-06-03 17:25 ` Florian Weimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).