Re: [PATCH] tests: handle AppArmor userns mitigation

[Simon Chopin <simon.chopin@canonical.com>]

On jeu. 21 mars 2024 13:19:41, Adhemerval Zanella Netto wrote:
> On 20/03/24 06:31, Simon Chopin wrote:
> > 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)
>
> As a side-note, is there a way to easily disable it? It would be good to
> avoid less test coverage.

This was already handled in 59e0441d4a, tests using test-container will
display a hint before exiting:

UNSUPPORTED: nss/tst-reload2
original exit status 77
To enable test-container, please run this as root:
  echo 0 > /proc/sys/kernel/apparmor_restrict_unprivileged_userns
error: test-container.c:1141: could not create a private mount
namespace (security policy?: Permission denied)

>
> >
> > 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
> >
> > Signed-off-by: Simon Chopin <simon.chopin@canonical.com>
> > ---
> >  support/support_become_root.c                 | 41 ++++++++++++++-----
> >  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, 54 insertions(+), 18 deletions(-)
> >
> > diff --git a/support/support_become_root.c b/support/support_become_root.c
> > index 5920fc8236..f744886037 100644
> > --- a/support/support_become_root.c
> > +++ b/support/support_become_root.c
> > @@ -28,6 +28,34 @@
> >  #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[100];
> > +  int size = snprintf (map_buf, sizeof (map_buf), "%llu %llu %llu\n", src, dst,
> > +                       range);
>
> Maybe use xasprintf here:
>
>   char *map_buf = xasprintf ("%llu %llu %llu\n", src, dst, range);
>   [...]
>   free (map_buf);
>
> > +  TEST_VERIFY_EXIT (size < sizeof (map_buf));
> > +  int ret = write (map_fd, map_buf, size);
> > +  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");
> > +    }
> > +}
> > +
>
> Ok.
>
> >  /* 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 +71,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 +92,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);
>
> Ok.
>
> > 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: 1ea051145612f199d8716ecdf78b084b00b5a727
>
> Ok.