[PATCH][RESEND 2] Revert "AUDIT: Allow login in non-init namespaces"

[PATCH][RESEND 2] Revert "AUDIT: Allow login in non-init namespaces"

[Calvin Owens]

This reverts 543bc6a1a987 "AUDIT: Allow login in non-init namespaces".

This commit incorrectly assumes that libpam treats -ECONNREFUSED as
an indicator that audit is disabled, and -EPERM or any other error
as a fatal error that prevents the login from continuing.

The opposite is in fact true: -EPERM allows the login to continue,
and -ECONNREFUSED causes it to refuse the login. This behavior has
been unchanged in upstream linux-pam since at least 2008.

Reverting this change allows libpam to again work as expected in
non-init user namespaces.

Signed-off-by: Calvin Owens <calvinowens@fb.com>
Cc: stable@vger.kernel.org
---
Relevant code in linux-pam:
https://git.fedorahosted.org/cgit/linux-pam.git/tree/libpam/pam_audit.c#n56

 kernel/audit.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 80983df..656e8ce 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -640,18 +640,8 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
 	int err = 0;
 
 	/* Only support initial user namespace for now. */
-	/*
-	 * We return ECONNREFUSED because it tricks userspace into thinking
-	 * that audit was not configured into the kernel.  Lots of users
-	 * configure their PAM stack (because that's what the distro does)
-	 * to reject login if unable to send messages to audit.  If we return
-	 * ECONNREFUSED the PAM stack thinks the kernel does not have audit
-	 * configured in and will let login proceed.  If we return EPERM
-	 * userspace will reject all logins.  This should be removed when we
-	 * support non init namespaces!!
-	 */
 	if (current_user_ns() != &init_user_ns)
-		return -ECONNREFUSED;
+		return -EPERM;
 
 	switch (msg_type) {
 	case AUDIT_LIST:
-- 
2.1.1

Re: [PATCH][RESEND 2] Revert "AUDIT: Allow login in non-init namespaces"

[Paul Moore]

On Thursday, January 08, 2015 05:44:48 PM Calvin Owens wrote:
> This reverts 543bc6a1a987 "AUDIT: Allow login in non-init namespaces".
> 
> This commit incorrectly assumes that libpam treats -ECONNREFUSED as
> an indicator that audit is disabled, and -EPERM or any other error
> as a fatal error that prevents the login from continuing.
> 
> The opposite is in fact true: -EPERM allows the login to continue,
> and -ECONNREFUSED causes it to refuse the login. This behavior has
> been unchanged in upstream linux-pam since at least 2008.
> 
> Reverting this change allows libpam to again work as expected in
> non-init user namespaces.
> 
> Signed-off-by: Calvin Owens <calvinowens@fb.com>
> Cc: stable@vger.kernel.org
> ---
> Relevant code in linux-pam:
> https://git.fedorahosted.org/cgit/linux-pam.git/tree/libpam/pam_audit.c#n56
> 
>  kernel/audit.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 80983df..656e8ce 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -640,18 +640,8 @@ static int audit_netlink_ok(struct sk_buff *skb, u16
> msg_type) int err = 0;
> 
>  	/* Only support initial user namespace for now. */
> -	/*
> -	 * We return ECONNREFUSED because it tricks userspace into thinking
> -	 * that audit was not configured into the kernel.  Lots of users
> -	 * configure their PAM stack (because that's what the distro does)
> -	 * to reject login if unable to send messages to audit.  If we return
> -	 * ECONNREFUSED the PAM stack thinks the kernel does not have audit
> -	 * configured in and will let login proceed.  If we return EPERM
> -	 * userspace will reject all logins.  This should be removed when we
> -	 * support non init namespaces!!
> -	 */
>  	if (current_user_ns() != &init_user_ns)
> -		return -ECONNREFUSED;
> +		return -EPERM;

While I haven't had reason to test this code lately, last I knew it was 
working, what problems are you seeing Calvin?  Also, with what distribution?

-- 
paul moore
www.paul-moore.com

Re: [PATCH][RESEND 2] Revert "AUDIT: Allow login in non-init namespaces"

[Calvin Owens]

On Friday 01/09 at 15:33 -0500, Paul Moore wrote:
> On Thursday, January 08, 2015 05:44:48 PM Calvin Owens wrote:
> > This reverts 543bc6a1a987 "AUDIT: Allow login in non-init namespaces".
> > 
> > This commit incorrectly assumes that libpam treats -ECONNREFUSED as
> > an indicator that audit is disabled, and -EPERM or any other error
> > as a fatal error that prevents the login from continuing.
> > 
> > The opposite is in fact true: -EPERM allows the login to continue,
> > and -ECONNREFUSED causes it to refuse the login. This behavior has
> > been unchanged in upstream linux-pam since at least 2008.
> > 
> > Reverting this change allows libpam to again work as expected in
> > non-init user namespaces.
> > 
> > Signed-off-by: Calvin Owens <calvinowens@fb.com>
> > Cc: stable@vger.kernel.org
> > ---
> > Relevant code in linux-pam:
> > https://urldefense.proofpoint.com/v1/url?u=https://git.fedorahosted.org/cgit/linux-pam.git/tree/libpam/pam_audit.c%23n56&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=oEb120Cp%2FehdhuY2M7qjelK5yT8IPB5WC2nEG4obDh4%3D%0A&m=vyJMOR0vSvoBryKRT8mBRsUpUQElUD3Fa4ZBI5Vaf%2BA%3D%0A&s=4f34b8ee3ea296b034ab84da1d7f0b738cd431081878afd6f85203507a722ebe
> > 
> >  kernel/audit.c | 12 +-----------
> >  1 file changed, 1 insertion(+), 11 deletions(-)
> >
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 80983df..656e8ce 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -640,18 +640,8 @@ static int audit_netlink_ok(struct sk_buff *skb, u16
> > msg_type) int err = 0;
> > 
> >  	/* Only support initial user namespace for now. */
> > -	/*
> > -	 * We return ECONNREFUSED because it tricks userspace into thinking
> > -	 * that audit was not configured into the kernel.  Lots of users
> > -	 * configure their PAM stack (because that's what the distro does)
> > -	 * to reject login if unable to send messages to audit.  If we return
> > -	 * ECONNREFUSED the PAM stack thinks the kernel does not have audit
> > -	 * configured in and will let login proceed.  If we return EPERM
> > -	 * userspace will reject all logins.  This should be removed when we
> > -	 * support non init namespaces!!
> > -	 */
> >  	if (current_user_ns() != &init_user_ns)
> > -		return -ECONNREFUSED;
> > +		return -EPERM;
> 
> While I haven't had reason to test this code lately, last I knew it was 
> working, what problems are you seeing Calvin?  Also, with what distribution?

An SSH authentication setup in LXC containers on CentOS broke with the
original patch. My revert made it work again. I can dig up more detail
about the specific setup if you're interested.

Thanks,
Calvin
 
> -- 
> paul moore
> www.paul-moore.com
> 

Re: [PATCH][RESEND 2] Revert "AUDIT: Allow login in non-init namespaces"

[Paul Moore]

On Friday, January 09, 2015 01:24:51 PM Calvin Owens wrote:
> On Friday 01/09 at 15:33 -0500, Paul Moore wrote:
> > While I haven't had reason to test this code lately, last I knew it was
> > working, what problems are you seeing Calvin?  Also, with what
> > distribution?
>
> An SSH authentication setup in LXC containers on CentOS broke with the
> original patch. My revert made it work again. I can dig up more detail
> about the specific setup if you're interested.

Yes, please.  I believe it is the little details that are important for this 
issue.

-- 
paul moore
www.paul-moore.com

Re: [PATCH][RESEND 2] Revert "AUDIT: Allow login in non-init namespaces"

[Calvin Owens]

On Friday 01/09 at 16:36 -0500, Paul Moore wrote:
> On Friday, January 09, 2015 01:24:51 PM Calvin Owens wrote:
> > On Friday 01/09 at 15:33 -0500, Paul Moore wrote:
> > > While I haven't had reason to test this code lately, last I knew it was
> > > working, what problems are you seeing Calvin?  Also, with what
> > > distribution?
> >
> > An SSH authentication setup in LXC containers on CentOS broke with the
> > original patch. My revert made it work again. I can dig up more detail
> > about the specific setup if you're interested.
> 
> Yes, please.  I believe it is the little details that are important for this 
> issue.

Just drop this patch, I goofed up. Sorry for the noise.

(543bc6a1a987 was mistakingly applied to a kernel build without the
supporting patches mentioned in the commit message. It was stupid of me
not to check for that before bothering you...)

Thanks very much,
Calvin
 
> -- 
> paul moore
> www.paul-moore.com
> 

Re: [PATCH][RESEND 2] Revert "AUDIT: Allow login in non-init namespaces"

[Richard Guy Briggs]

On 15/01/08, Calvin Owens wrote:
> This reverts 543bc6a1a987 "AUDIT: Allow login in non-init namespaces".
> 
> This commit incorrectly assumes that libpam treats -ECONNREFUSED as
> an indicator that audit is disabled, and -EPERM or any other error
> as a fatal error that prevents the login from continuing.

Which netlink audit message type is actually failing?
Is it AUDIT_TTY_{G,S}ET or is it an AUDIT_USER_* message?  The former
requires CAP_AUDIT_CONTROL and both PID and user init namespace (for
now) and the latter requires CAP_AUDIT_WRITE and only user init
namespace.

> The opposite is in fact true: -EPERM allows the login to continue,
> and -ECONNREFUSED causes it to refuse the login. This behavior has
> been unchanged in upstream linux-pam since at least 2008.

So this sounds to me like standard PAM usage is inverted from PAM usage
in containers.

> Reverting this change allows libpam to again work as expected in
> non-init user namespaces.

However, that will break other things...

Do you have test cases to show this?

Currently:
If audit is not available, return ECONNREFUSED. (netlink_unicast_kernel)

If not in init user namespace, return ECONNREFUSED. (audit_netlink_ok)

If control message and not init PID ns, return EPERM (audit_netlink_ok)

If control message and not CAP_AUDIT_CONTROL, return EPERM (audit_netlink_ok)

If user log message and not CAP_AUDIT_WRITE, return EPERM (audit_netlink_ok)

If unrecognized message, return EINVAL (audit_netlink_ok)


Listening in non-init net namespaces caused EPERM to be returned by
audit instead of ECONNREFUSED by netlink due to lack of perms when the
sending process didn't have CAP_AUDIT_WRITE.  Fixed in docker bz1119849
	http://blog.oddbit.com/2014/07/21/tracking-down-a-kernel-bug-wit/


> Signed-off-by: Calvin Owens <calvinowens@fb.com>
> Cc: stable@vger.kernel.org
> ---
> Relevant code in linux-pam:
> https://git.fedorahosted.org/cgit/linux-pam.git/tree/libpam/pam_audit.c#n56

This code only checks for an error return of -EPERM when the userid is
non-root.  Is login running as root, or has it already forked and is
running as an unprivileged user at that point?  Audit doesn't care about
the UID even though many equate root (superuser) with full capabilities.
Audit only looks at capabilities and namespaces.  Is this relevant to PAM?

>  kernel/audit.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 80983df..656e8ce 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -640,18 +640,8 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
>  	int err = 0;
>  
>  	/* Only support initial user namespace for now. */
> -	/*
> -	 * We return ECONNREFUSED because it tricks userspace into thinking
> -	 * that audit was not configured into the kernel.  Lots of users
> -	 * configure their PAM stack (because that's what the distro does)
> -	 * to reject login if unable to send messages to audit.  If we return
> -	 * ECONNREFUSED the PAM stack thinks the kernel does not have audit
> -	 * configured in and will let login proceed.  If we return EPERM
> -	 * userspace will reject all logins.  This should be removed when we
> -	 * support non init namespaces!!
> -	 */
>  	if (current_user_ns() != &init_user_ns)
> -		return -ECONNREFUSED;
> +		return -EPERM;
>  
>  	switch (msg_type) {
>  	case AUDIT_LIST:

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

Re: [PATCH][RESEND 2] Revert "AUDIT: Allow login in non-init namespaces"

[Richard Guy Briggs]

On 15/01/13, Richard Guy Briggs wrote:
> On 15/01/08, Calvin Owens wrote:
> > This reverts 543bc6a1a987 "AUDIT: Allow login in non-init namespaces".
> > 
> > This commit incorrectly assumes that libpam treats -ECONNREFUSED as
> > an indicator that audit is disabled, and -EPERM or any other error
> > as a fatal error that prevents the login from continuing.
> 
> Which netlink audit message type is actually failing?
> Is it AUDIT_TTY_{G,S}ET or is it an AUDIT_USER_* message?  The former
> requires CAP_AUDIT_CONTROL and both PID and user init namespace (for
> now) and the latter requires CAP_AUDIT_WRITE and only user init
> namespace.
> 
> > The opposite is in fact true: -EPERM allows the login to continue,
> > and -ECONNREFUSED causes it to refuse the login. This behavior has
> > been unchanged in upstream linux-pam since at least 2008.
> 
> So this sounds to me like standard PAM usage is inverted from PAM usage
> in containers.
> 
> > Reverting this change allows libpam to again work as expected in
> > non-init user namespaces.
> 
> However, that will break other things...
> 
> Do you have test cases to show this?
> 
> Currently:
> If audit is not available, return ECONNREFUSED. (netlink_unicast_kernel)

In fact, the socket() call should fail before that with EPROTONOSUPPORT. (in netlink_create)

> If not in init user namespace, return ECONNREFUSED. (audit_netlink_ok)
> 
> If control message and not init PID ns, return EPERM (audit_netlink_ok)
> 
> If control message and not CAP_AUDIT_CONTROL, return EPERM (audit_netlink_ok)
> 
> If user log message and not CAP_AUDIT_WRITE, return EPERM (audit_netlink_ok)
> 
> If unrecognized message, return EINVAL (audit_netlink_ok)
> 
> 
> Listening in non-init net namespaces caused EPERM to be returned by
> audit instead of ECONNREFUSED by netlink due to lack of perms when the
> sending process didn't have CAP_AUDIT_WRITE.  Fixed in docker bz1119849
> 	http://blog.oddbit.com/2014/07/21/tracking-down-a-kernel-bug-wit/
> 
> 
> > Signed-off-by: Calvin Owens <calvinowens@fb.com>
> > Cc: stable@vger.kernel.org
> > ---
> > Relevant code in linux-pam:
> > https://git.fedorahosted.org/cgit/linux-pam.git/tree/libpam/pam_audit.c#n56
> 
> This code only checks for an error return of -EPERM when the userid is
> non-root.  Is login running as root, or has it already forked and is
> running as an unprivileged user at that point?  Audit doesn't care about
> the UID even though many equate root (superuser) with full capabilities.
> Audit only looks at capabilities and namespaces.  Is this relevant to PAM?
> 
> >  kernel/audit.c | 12 +-----------
> >  1 file changed, 1 insertion(+), 11 deletions(-)
> > 
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 80983df..656e8ce 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -640,18 +640,8 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
> >  	int err = 0;
> >  
> >  	/* Only support initial user namespace for now. */
> > -	/*
> > -	 * We return ECONNREFUSED because it tricks userspace into thinking
> > -	 * that audit was not configured into the kernel.  Lots of users
> > -	 * configure their PAM stack (because that's what the distro does)
> > -	 * to reject login if unable to send messages to audit.  If we return
> > -	 * ECONNREFUSED the PAM stack thinks the kernel does not have audit
> > -	 * configured in and will let login proceed.  If we return EPERM
> > -	 * userspace will reject all logins.  This should be removed when we
> > -	 * support non init namespaces!!
> > -	 */
> >  	if (current_user_ns() != &init_user_ns)
> > -		return -ECONNREFUSED;
> > +		return -EPERM;
> >  
> >  	switch (msg_type) {
> >  	case AUDIT_LIST:
> 
> - RGB
> 
> --
> Richard Guy Briggs <rbriggs@redhat.com>
> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545