fix pam_namespace leaking the protect mounts to parent namespace (#755216)

This commit is contained in:
Tomas Mraz 2012-01-31 17:19:23 +01:00
parent 87d3951c7d
commit 92f3acf6be
3 changed files with 217 additions and 1 deletions

View File

@ -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
</arg>
<arg choice="opt">
- no_unmount_on_close
+ unmount_on_close
</arg>
<arg choice="opt">
use_current_context
@@ -195,16 +195,17 @@
<varlistentry>
<term>
- <option>no_unmount_on_close</option>
+ <option>unmount_on_close</option>
</term>
<listitem>
<para>
- 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.
</para>
</listitem>
</varlistentry>
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 */

View File

@ -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
<command>mount --make-rshared /</command> 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.
</para>
+ <para>
+ 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.
+ </para>
</listitem>
</varlistentry>
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;

View File

@ -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 <tmraz@redhat.com> 1.1.5-5
- fix pam_namespace leaking the protect mounts to parent namespace (#755216)
* Fri Jan 13 2012 Fedora Release Engineering <rel-eng@lists.fedoraproject.org> - 1.1.5-4
- Rebuilt for https://fedoraproject.org/wiki/Fedora_17_Mass_Rebuild