From 92f3acf6beb92a34bb9620483f20aff8f9d28346 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Tue, 31 Jan 2012 17:19:23 +0100 Subject: [PATCH] fix pam_namespace leaking the protect mounts to parent namespace (#755216) --- pam-1.1.5-namespace-no-unmount.patch | 93 ++++++++++++++++++++++ pam-1.1.5-namespace-rslave.patch | 114 +++++++++++++++++++++++++++ pam.spec | 11 ++- 3 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 pam-1.1.5-namespace-no-unmount.patch create mode 100644 pam-1.1.5-namespace-rslave.patch diff --git a/pam-1.1.5-namespace-no-unmount.patch b/pam-1.1.5-namespace-no-unmount.patch new file mode 100644 index 0000000..9e28397 --- /dev/null +++ b/pam-1.1.5-namespace-no-unmount.patch @@ -0,0 +1,93 @@ +diff --git a/modules/pam_namespace/pam_namespace.8.xml b/modules/pam_namespace/pam_namespace.8.xml +index 6ec3ad2..f0f80d3 100644 +--- a/modules/pam_namespace/pam_namespace.8.xml ++++ b/modules/pam_namespace/pam_namespace.8.xml +@@ -44,7 +44,7 @@ + ignore_instance_parent_mode + + +- no_unmount_on_close ++ unmount_on_close + + + use_current_context +@@ -195,16 +195,17 @@ + + + +- ++ + + + +- For certain trusted programs such as newrole, open session +- is called from a child process while the parent performs +- close session and pam end functions. For these commands +- use this option to instruct pam_close_session to not +- unmount the bind mounted polyinstantiated directory in the +- parent. ++ Explicitly unmount the polyinstantiated directories instead ++ of relying on automatic namespace destruction after the last ++ process in a namespace exits. This option should be used ++ only in case it is ensured by other means that there cannot be ++ any processes running in the private namespace left after the ++ session close. It is also useful only in case there are ++ multiple pam session calls in sequence from the same process. + + + +diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c +index 470f493..a40f05e 100644 +--- a/modules/pam_namespace/pam_namespace.c ++++ b/modules/pam_namespace/pam_namespace.c +@@ -2108,24 +2108,26 @@ PAM_EXTERN int pam_sm_close_session(pam_handle_t *pamh, int flags UNUSED, + idata.flags |= PAMNS_DEBUG; + if (strcmp(argv[i], "ignore_config_error") == 0) + idata.flags |= PAMNS_IGN_CONFIG_ERR; +- if (strcmp(argv[i], "no_unmount_on_close") == 0) +- idata.flags |= PAMNS_NO_UNMOUNT_ON_CLOSE; ++ if (strcmp(argv[i], "unmount_on_close") == 0) ++ idata.flags |= PAMNS_UNMOUNT_ON_CLOSE; + } + + if (idata.flags & PAMNS_DEBUG) + pam_syslog(idata.pamh, LOG_DEBUG, "close_session - start"); + + /* +- * For certain trusted programs such as newrole, open session +- * is called from a child process while the parent perfoms +- * close session and pam end functions. For these commands +- * pam_close_session should not perform the unmount of the +- * polyinstantiatied directory because it will result in +- * undoing of parents polyinstantiatiaion. These commands +- * will invoke pam_namespace with the "no_unmount_on_close" +- * argument. ++ * Normally the unmount is implicitly done when the last ++ * process in the private namespace exits. ++ * If it is ensured that there are no child processes left in ++ * the private namespace by other means and if there are ++ * multiple sessions opened and closed sequentially by the ++ * same process, the "unmount_on_close" option might be ++ * used to unmount the polydirs explicitly. + */ +- if (idata.flags & PAMNS_NO_UNMOUNT_ON_CLOSE) { ++ if (!(idata.flags & PAMNS_UNMOUNT_ON_CLOSE)) { ++ pam_set_data(idata.pamh, NAMESPACE_POLYDIR_DATA, NULL, NULL); ++ pam_set_data(idata.pamh, NAMESPACE_PROTECT_DATA, NULL, NULL); ++ + if (idata.flags & PAMNS_DEBUG) + pam_syslog(idata.pamh, LOG_DEBUG, "close_session - sucessful"); + return PAM_SUCCESS; +diff --git a/modules/pam_namespace/pam_namespace.h b/modules/pam_namespace/pam_namespace.h +index 6bca31c..1d0c11c 100644 +--- a/modules/pam_namespace/pam_namespace.h ++++ b/modules/pam_namespace/pam_namespace.h +@@ -101,7 +101,7 @@ + #define PAMNS_GEN_HASH 0x00002000 /* Generate md5 hash for inst names */ + #define PAMNS_IGN_CONFIG_ERR 0x00004000 /* Ignore format error in conf file */ + #define PAMNS_IGN_INST_PARENT_MODE 0x00008000 /* Ignore instance parent mode */ +-#define PAMNS_NO_UNMOUNT_ON_CLOSE 0x00010000 /* no unmount at session close */ ++#define PAMNS_UNMOUNT_ON_CLOSE 0x00010000 /* Unmount at session close */ + #define PAMNS_USE_CURRENT_CONTEXT 0x00020000 /* use getcon instead of getexeccon */ + #define PAMNS_USE_DEFAULT_CONTEXT 0x00040000 /* use get_default_context instead of getexeccon */ + #define PAMNS_MOUNT_PRIVATE 0x00080000 /* Make the polydir mounts private */ diff --git a/pam-1.1.5-namespace-rslave.patch b/pam-1.1.5-namespace-rslave.patch new file mode 100644 index 0000000..94265c0 --- /dev/null +++ b/pam-1.1.5-namespace-rslave.patch @@ -0,0 +1,114 @@ +diff -up Linux-PAM-1.1.5/modules/pam_namespace/pam_namespace.8.xml.rslave Linux-PAM-1.1.5/modules/pam_namespace/pam_namespace.8.xml +--- Linux-PAM-1.1.5/modules/pam_namespace/pam_namespace.8.xml.rslave 2011-06-21 11:04:56.000000000 +0200 ++++ Linux-PAM-1.1.5/modules/pam_namespace/pam_namespace.8.xml 2012-01-31 16:40:36.495716240 +0100 +@@ -246,12 +246,18 @@ + This option can be used on systems where the / mount point or + its submounts are made shared (for example with a + mount --make-rshared / command). +- The module will make the polyinstantiated directory mount points +- private. Normally the pam_namespace will try to detect the ++ The module will mark the whole directory tree so any mount and ++ unmount operations in the polyinstantiation namespace are private. ++ Normally the pam_namespace will try to detect the + shared / mount point and make the polyinstantiated directories + private automatically. This option has to be used just when + only a subtree is shared and / is not. + ++ ++ Note that mounts and unmounts done in the private namespace will not ++ affect the parent namespace if this option is used or when the ++ shared / mount point is autodetected. ++ + + + +diff -up Linux-PAM-1.1.5/modules/pam_namespace/pam_namespace.c.rslave Linux-PAM-1.1.5/modules/pam_namespace/pam_namespace.c +--- Linux-PAM-1.1.5/modules/pam_namespace/pam_namespace.c.rslave 2011-06-21 11:04:56.000000000 +0200 ++++ Linux-PAM-1.1.5/modules/pam_namespace/pam_namespace.c 2012-01-31 16:42:07.762506791 +0100 +@@ -1003,7 +1003,7 @@ static int protect_mount(int dfd, const + return 0; + } + +-static int protect_dir(const char *path, mode_t mode, int do_mkdir, int always, ++static int protect_dir(const char *path, mode_t mode, int do_mkdir, + struct instance_data *idata) + { + char *p = strdup(path); +@@ -1082,7 +1082,7 @@ static int protect_dir(const char *path, + } + } + +- if ((flags & O_NOFOLLOW) || always) { ++ if (flags & O_NOFOLLOW) { + /* we are inside user-owned dir - protect */ + if (protect_mount(rv, p, idata) == -1) { + save_errno = errno; +@@ -1124,7 +1124,7 @@ static int check_inst_parent(char *ipath + if (trailing_slash) + *trailing_slash = '\0'; + +- dfd = protect_dir(inst_parent, 0, 1, 0, idata); ++ dfd = protect_dir(inst_parent, 0, 1, idata); + + if (dfd == -1 || fstat(dfd, &instpbuf) < 0) { + pam_syslog(idata->pamh, LOG_ERR, +@@ -1259,7 +1259,7 @@ static int create_polydir(struct polydir + } + #endif + +- rc = protect_dir(dir, mode, 1, idata->flags & PAMNS_MOUNT_PRIVATE, idata); ++ rc = protect_dir(dir, mode, 1, idata); + if (rc == -1) { + pam_syslog(idata->pamh, LOG_ERR, + "Error creating directory %s: %m", dir); +@@ -1447,7 +1447,7 @@ static int ns_setup(struct polydir_s *po + pam_syslog(idata->pamh, LOG_DEBUG, + "Set namespace for directory %s", polyptr->dir); + +- retval = protect_dir(polyptr->dir, 0, 0, idata->flags & PAMNS_MOUNT_PRIVATE, idata); ++ retval = protect_dir(polyptr->dir, 0, 0, idata); + + if (retval < 0 && errno != ENOENT) { + pam_syslog(idata->pamh, LOG_ERR, "Polydir %s access error: %m", +@@ -1534,22 +1534,6 @@ static int ns_setup(struct polydir_s *po + goto error_out; + } + +- if (idata->flags & PAMNS_MOUNT_PRIVATE) { +- /* +- * Make the polyinstantiated dir private mount. This depends +- * on making the dir a mount point in the protect_dir call. +- */ +- if (mount(polyptr->dir, polyptr->dir, NULL, MS_PRIVATE|MS_REC, NULL) < 0) { +- pam_syslog(idata->pamh, LOG_ERR, "Error making %s a private mount, %m", +- polyptr->dir); +- goto error_out; +- } +- if (idata->flags & PAMNS_DEBUG) +- pam_syslog(idata->pamh, LOG_DEBUG, +- "Polyinstantiated directory %s made as private mount", polyptr->dir); +- +- } +- + /* + * Bind mount instance directory on top of the polyinstantiated + * directory to provide an instance of polyinstantiated directory +@@ -1720,6 +1704,18 @@ static int setup_namespace(struct instan + "Unable to unshare from parent namespace, %m"); + return PAM_SESSION_ERR; + } ++ if (idata->flags & PAMNS_MOUNT_PRIVATE) { ++ /* Remount / as SLAVE so that nothing mounted in the namespace ++ shows up in the parent */ ++ if (mount("/", "/", NULL, MS_SLAVE | MS_REC , NULL) < 0) { ++ pam_syslog(idata->pamh, LOG_ERR, ++ "Failed to mark / as a slave mount point, %m"); ++ return PAM_SESSION_ERR; ++ } ++ if (idata->flags & PAMNS_DEBUG) ++ pam_syslog(idata->pamh, LOG_DEBUG, ++ "The / mount point was marked as slave"); ++ } + } else { + del_polydir_list(idata->polydirs_ptr); + return PAM_SUCCESS; diff --git a/pam.spec b/pam.spec index 0c84c0d..a90ccc9 100644 --- a/pam.spec +++ b/pam.spec @@ -3,7 +3,7 @@ Summary: An extensible library which provides authentication for applications Name: pam Version: 1.1.5 -Release: 4%{?dist} +Release: 5%{?dist} # The library is BSD licensed with option to relicense as GPLv2+ # - this option is redundant as the BSD license allows that anyway. # pam_timestamp, pam_loginuid, and pam_console modules are GPLv2+. @@ -38,6 +38,10 @@ Patch10: pam-1.1.3-nouserenv.patch Patch11: pam-1.1.3-console-abstract.patch Patch12: pam-1.1.3-faillock-screensaver.patch Patch13: pam-1.1.5-limits-user.patch +# Committed to upstream git +Patch14: pam-1.1.5-namespace-rslave.patch +# Committed to upstream git +Patch15: pam-1.1.5-namespace-no-unmount.patch %define _sbindir /sbin %define _moduledir /%{_lib}/security @@ -110,6 +114,8 @@ mv pam-redhat-%{pam_redhat_version}/* modules %patch11 -p1 -b .abstract %patch12 -p1 -b .screensaver %patch13 -p1 -b .limits +%patch14 -p1 -b .rslave +%patch15 -p1 -b .no-unmount libtoolize -f autoreconf @@ -364,6 +370,9 @@ fi %doc doc/adg/*.txt doc/adg/html %changelog +* Tue Jan 31 2012 Tomas Mraz 1.1.5-5 +- fix pam_namespace leaking the protect mounts to parent namespace (#755216) + * Fri Jan 13 2012 Fedora Release Engineering - 1.1.5-4 - Rebuilt for https://fedoraproject.org/wiki/Fedora_17_Mass_Rebuild