[PATCH] pam_loginuid: cleanup loginuid buffer initialization
by Dmitry V. Levin
* modules/pam_loginuid/pam_loginuid.c (set_loginuid): Move loginuid
buffer initialization closer to its first use.
---
modules/pam_loginuid/pam_loginuid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/modules/pam_loginuid/pam_loginuid.c b/modules/pam_loginuid/pam_loginuid.c
index d258422..c476f7b 100644
--- a/modules/pam_loginuid/pam_loginuid.c
+++ b/modules/pam_loginuid/pam_loginuid.c
@@ -69,7 +69,6 @@ static int set_loginuid(pam_handle_t *pamh, uid_t uid)
close(fd);
}
- count = snprintf(loginuid, sizeof(loginuid), "%lu", (unsigned long)uid);
fd = open("/proc/self/loginuid", O_NOFOLLOW|O_RDWR);
if (fd < 0) {
if (errno == ENOENT) {
@@ -82,6 +81,7 @@ static int set_loginuid(pam_handle_t *pamh, uid_t uid)
return rc;
}
+ count = snprintf(loginuid, sizeof(loginuid), "%lu", (unsigned long)uid);
if (pam_modutil_read(fd, buf, sizeof(buf)) == count &&
memcmp(buf, loginuid, count) == 0) {
rc = PAM_SUCCESS;
--
ldv
10 years, 3 months
[PATCH] pam_loginuid: Always return PAM_IGNORE in userns
by Stéphane Graber
The previous patch to support user namespaces works fine with containers
that are started from a desktop/terminal session but fails when dealing
with containers that were started from a remote session such as ssh.
I haven't looked at the exact reason for that in the kernel but on the
userspace side of things, the difference is that containers started from
an ssh session will happily let pam open /proc/self/loginuid read-write,
will let it read its content but will then fail with EPERM when trying
to write to it.
So to make the userns support bullet proof, this commit moves the userns
check earlier in the function (which means a small performance impact as
it'll now happen everytime on kernels that have userns support) and will
set rc = PAM_IGNORE instead of rc = PAM_ERROR.
The rest of the code is still executed in the event that PAM is run on a
future kernel where we have some kind of audit namespace that includes a
working loginuid.
Signed-off-by: Stéphane Graber <stgraber(a)ubuntu.com>
---
modules/pam_loginuid/pam_loginuid.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/modules/pam_loginuid/pam_loginuid.c b/modules/pam_loginuid/pam_loginuid.c
index 54ae6f0..d258422 100644
--- a/modules/pam_loginuid/pam_loginuid.c
+++ b/modules/pam_loginuid/pam_loginuid.c
@@ -58,21 +58,22 @@ static int set_loginuid(pam_handle_t *pamh, uid_t uid)
static const char host_uid_map[] = " 0 0 4294967295\n";
char uid_map[sizeof(host_uid_map)];
+ /* loginuid in user namespaces currently isn't writable and in some
+ case, not even readable, so consider any failure as ignorable (but try
+ anyway, in case we hit a kernel which supports it). */
+ fd = open("/proc/self/uid_map", O_RDONLY);
+ if (fd >= 0) {
+ count = pam_modutil_read(fd, uid_map, sizeof(uid_map));
+ if (strncmp(uid_map, host_uid_map, count) != 0)
+ rc = PAM_IGNORE;
+ close(fd);
+ }
+
count = snprintf(loginuid, sizeof(loginuid), "%lu", (unsigned long)uid);
fd = open("/proc/self/loginuid", O_NOFOLLOW|O_RDWR);
if (fd < 0) {
if (errno == ENOENT) {
rc = PAM_IGNORE;
- } else if (errno == EACCES) {
- fd = open("/proc/self/uid_map", O_RDONLY);
- if (fd >= 0) {
- count = pam_modutil_read(fd, uid_map, sizeof(uid_map));
- if (strncmp(uid_map, host_uid_map, count) != 0)
- rc = PAM_IGNORE;
- close(fd);
- }
- if (rc != PAM_IGNORE)
- errno = EACCES;
}
if (rc != PAM_IGNORE) {
pam_syslog(pamh, LOG_ERR,
--
1.8.5.3
10 years, 3 months
[PATCH] Read pam_umask "usergroups" option from /etc/login.defs
by Steve Langasek
Ubuntu has been carrying a patch for a while to support reading the
USERGROUPS_ENAB setting from /etc/login.defs, so that it doesn't have to be
passed as an argument to pam_umask. I think this change is appropriate for
including upstream.
Do folks agree this is correct? I wonder if the USERGROUPS_ENAB setting
should really only apply to UMASK settings in /etc/login.defs, or if it
should be honored regardless of the source of the umask. (The current
behavior is the most conservative, but I'm not sure it's actually the most
natural.)
Also, while reviewing this patch, I noticed that the manpage is incorrect
about the precedence of umask settings. It currently says the order is:
· umask= argument
· umask= entry in the user's GECOS field
· UMASK= entry from /etc/default/login
· UMASK entry from /etc/login.defs (influenced by USERGROUPS_ENAB in
/etc/login.defs)
Looking at the code, the actual order is:
· umask= entry in the user's GECOS field
· umask= argument
· UMASK entry from /etc/login.defs (influenced by USERGROUPS_ENAB in
/etc/login.defs)
· UMASK= entry from /etc/default/login
We should certainly fix it so the code and the documentation match; I'm not
sure that either is currently correct, though. I think the intended order
is probably:
· umask= entry in the user's GECOS field
· umask= argument
· UMASK= entry from /etc/default/login
· UMASK entry from /etc/login.defs (influenced by USERGROUPS_ENAB in
/etc/login.defs)
Thoughts?
Thanks,
--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
slangasek(a)ubuntu.com vorlon(a)debian.org
10 years, 3 months
[PATCH] pam_namespace: don't use bashisms in default namespace.init script
by Steve Langasek
* modules/pam_namespace/pam_namespace.c: call setuid() before execing the
namespace init script, so that scripts run with maximum privilege regardless
of the shell implementation.
* modules/pam_namespace/namespace.init: drop the '-p' bashism from the
shebang line
This is not a POSIX standard option, it's a bashism. The bash manpage says
that it's used to prevent the effective user id from being reset to the real
user id on startup, and to ignore certain unsafe variables from the
environment.
In the case of pam_namespace, the -p is not necessary for environment
sanitizing because the PAM module (properly) sanitizes the environment
before execing the script.
The stated reason given in CVS history for passing -p is to "preserve euid
when called from setuid apps (su, newrole)." This should be done more
portably, by calling setuid() before spawning the shell.
Signed-off-by: Steve Langasek <vorlon(a)debian.org>
Bug-Debian: http://bugs.debian.org/624842
---
modules/pam_namespace/namespace.init | 2 +-
modules/pam_namespace/pam_namespace.c | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/modules/pam_namespace/namespace.init b/modules/pam_namespace/namespace.init
index 9ab5806..67d4aa2 100755
--- a/modules/pam_namespace/namespace.init
+++ b/modules/pam_namespace/namespace.init
@@ -1,4 +1,4 @@
-#!/bin/sh -p
+#!/bin/sh
# It receives polydir path as $1, the instance path as $2,
# a flag whether the instance dir was newly created (0 - no, 1 - yes) in $3,
# and user name in $4.
diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c
index e0d5e30..7c85b5b 100644
--- a/modules/pam_namespace/pam_namespace.c
+++ b/modules/pam_namespace/pam_namespace.c
@@ -1205,6 +1205,9 @@ static int inst_init(const struct polydir_s *polyptr, const char *ipath,
_exit(1);
}
#endif
+ /* Pass maximum privs when we exec() */
+ (void) setuid (geteuid ());
+
if (execle(init_script, init_script,
polyptr->dir, ipath, newdir?"1":"0", idata->user, NULL, envp) < 0)
_exit(1);
--
1.7.10.4
10 years, 3 months
[PATCH] pam_loginuid: Ignore failure in user namespaces
by Stéphane Graber
When running pam_loginuid in a container using the user namespaces, even
uid 0 isn't allowed to set the loginuid property.
This change catches the EACCES from opening loginuid, checks if the user
is in the host namespace (by comparing the uid_map with the host's one)
and only if that's the case, sets rc to 1.
Should uid_map not exist to be unreadable for some reason, it'll be
assumed that the process is running on the host's namespace.
The initial reason behind this change was failure to ssh into an
unprivileged container (using a 3.13 kernel and current LXC) when using
a standard pam profile for sshd (which requires success from
pam_loginuid).
I believe this solution doesn't have any drawback and will allow people
to use unprivileged containers normally. An alternative would be to have
all distros set pam_loginuid as optional but that'd be bad for any of
the other potential failure case which people may care about.
There has also been some discussions to get some of the audit features
tied with the user namespaces but currently none of that has been merged
upstream and the currently proposed implementation doesn't cover
loginuid (nor is it clear how this should even work when loginuid is set
as immutable after initial write).
---
modules/pam_loginuid/pam_loginuid.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/modules/pam_loginuid/pam_loginuid.c b/modules/pam_loginuid/pam_loginuid.c
index a903845..977cbea 100644
--- a/modules/pam_loginuid/pam_loginuid.c
+++ b/modules/pam_loginuid/pam_loginuid.c
@@ -53,11 +53,26 @@ static int set_loginuid(pam_handle_t *pamh, uid_t uid)
{
int fd, count, rc = 0;
char loginuid[24], buf[24];
+ char uid_map[256];
count = snprintf(loginuid, sizeof(loginuid), "%lu", (unsigned long)uid);
fd = open("/proc/self/loginuid", O_NOFOLLOW|O_RDWR);
if (fd < 0) {
- if (errno != ENOENT) {
+ if (errno == EACCES) {
+ rc = 1;
+ fd = open("/proc/self/uid_map", O_RDONLY);
+ if (fd >= 0) {
+ count = pam_modutil_read(fd, uid_map, sizeof(uid_map));
+ if (strncmp(uid_map, " 0 0 4294967295\n",
+ count) != 0)
+ rc = 0;
+ close(fd);
+ }
+ if (rc == 1)
+ pam_syslog(pamh, LOG_ERR,
+ "Cannot open /proc/self/loginuid: Permission denied");
+ }
+ else if (errno != ENOENT) {
rc = 1;
pam_syslog(pamh, LOG_ERR,
"Cannot open /proc/self/loginuid: %m");
--
1.8.5.1
10 years, 3 months
[PATCH] pam_loginuid: Ignore failure in user namespaces (v2)
by Stéphane Graber
When running pam_loginuid in a container using the user namespaces, even
uid 0 isn't allowed to set the loginuid property.
This change catches the EACCES from opening loginuid, checks if the user
is in the host namespace (by comparing the uid_map with the host's one)
and only if that's the case, sets rc to 1.
Should uid_map not exist to be unreadable for some reason, it'll be
assumed that the process is running on the host's namespace.
The initial reason behind this change was failure to ssh into an
unprivileged container (using a 3.13 kernel and current LXC) when using
a standard pam profile for sshd (which requires success from
pam_loginuid).
I believe this solution doesn't have any drawback and will allow people
to use unprivileged containers normally. An alternative would be to have
all distros set pam_loginuid as optional but that'd be bad for any of
the other potential failure case which people may care about.
There has also been some discussions to get some of the audit features
tied with the user namespaces but currently none of that has been merged
upstream and the currently proposed implementation doesn't cover
loginuid (nor is it clear how this should even work when loginuid is set
as immutable after initial write).
---
modules/pam_loginuid/pam_loginuid.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/modules/pam_loginuid/pam_loginuid.c b/modules/pam_loginuid/pam_loginuid.c
index a903845..60f11f7 100644
--- a/modules/pam_loginuid/pam_loginuid.c
+++ b/modules/pam_loginuid/pam_loginuid.c
@@ -47,18 +47,33 @@
/*
* This function writes the loginuid to the /proc system. It returns
- * 0 on success and 1 on failure.
+ * the relevant PAM return value.
*/
static int set_loginuid(pam_handle_t *pamh, uid_t uid)
{
- int fd, count, rc = 0;
+ int fd, count, rc = PAM_SUCCESS;
char loginuid[24], buf[24];
+ char uid_map[256];
count = snprintf(loginuid, sizeof(loginuid), "%lu", (unsigned long)uid);
fd = open("/proc/self/loginuid", O_NOFOLLOW|O_RDWR);
if (fd < 0) {
- if (errno != ENOENT) {
- rc = 1;
+ if (errno == EACCES) {
+ rc = PAM_SESSION_ERR;
+ fd = open("/proc/self/uid_map", O_RDONLY);
+ if (fd >= 0) {
+ count = pam_modutil_read(fd, uid_map, sizeof(uid_map));
+ if (strncmp(uid_map, " 0 0 4294967295\n",
+ count) != 0)
+ rc = PAM_IGNORE;
+ close(fd);
+ }
+ if (rc == PAM_IGNORE)
+ pam_syslog(pamh, LOG_ERR,
+ "Cannot open /proc/self/loginuid: Permission denied");
+ }
+ else if (errno != ENOENT) {
+ rc = PAM_IGNORE;
pam_syslog(pamh, LOG_ERR,
"Cannot open /proc/self/loginuid: %m");
}
@@ -69,7 +84,7 @@ static int set_loginuid(pam_handle_t *pamh, uid_t uid)
goto done; /* already correct */
if (lseek(fd, 0, SEEK_SET) == -1 || (ftruncate(fd, 0) == -1 ||
pam_modutil_write(fd, loginuid, count) != count))
- rc = 1;
+ rc = PAM_SESSION_ERR;
done:
close(fd);
return rc;
@@ -170,6 +185,7 @@ _pam_loginuid(pam_handle_t *pamh, int flags UNUSED,
{
const char *user = NULL;
struct passwd *pwd;
+ int ret;
#ifdef HAVE_LIBAUDIT
int require_auditd = 0;
#endif
@@ -188,9 +204,10 @@ _pam_loginuid(pam_handle_t *pamh, int flags UNUSED,
return PAM_SESSION_ERR;
}
- if (set_loginuid(pamh, pwd->pw_uid)) {
+ ret = set_loginuid(pamh, pwd->pw_uid);
+ if (ret != PAM_SUCCESS) {
pam_syslog(pamh, LOG_ERR, "set_loginuid failed\n");
- return PAM_SESSION_ERR;
+ return ret;
}
#ifdef HAVE_LIBAUDIT
--
1.8.5.2
10 years, 4 months