[PATCH 4.9 0/1] selinux: allow dontauditx and auditallowx rules to take effect without allowx

[PATCH 4.9 0/1] selinux: allow dontauditx and auditallowx rules to take effect without allowx

[Alexander Grund]

This patch fixes an inconsistency, if not a clear bug, with the extended permissions.
To quote from the original discussion [1]:
> The behavior of dontauditx and auditallowx appears to be broken making them useless.

[1] https://lore.kernel.org/selinux/6a791504-7728-3026-17ee-c22cbff8c3d1@gmail.com/T/

bauen1 (1):
  selinux: allow dontauditx and auditallowx rules to take effect without allowx

 security/selinux/ss/services.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

-- 
2.25.1

[PATCH 4.9 1/1] selinux: allow dontauditx and auditallowx rules to take effect without allowx

[Alexander Grund]

From: bauen1 <j2468h@googlemail.com>

commit 44141f58e14317853698f994ca5c3785a0c230d0 upstream.

This allows for dontauditing very specific ioctls e.g. TCGETS without
dontauditing every ioctl or granting additional permissions.

Now either an allowx, dontauditx or auditallowx rules enables checking
for extended permissions.

Signed-off-by: Jonathan Hettwer <j2468h@gmail.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Alexander Grund <theflamefire89@gmail.com>
---
 security/selinux/ss/services.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 6ca297821d459..c27c3ce76abbc 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -610,9 +610,7 @@ void services_compute_xperms_drivers(
 					node->datum.u.xperms->driver);
 	}
 
-	/* If no ioctl commands are allowed, ignore auditallow and auditdeny */
-	if (node->key.specified & AVTAB_XPERMS_ALLOWED)
-		xperms->len = 1;
+	xperms->len = 1;
 }
 
 /*
-- 
2.25.1

Re: [PATCH 4.9 0/1] selinux: allow dontauditx and auditallowx rules to take effect without allowx

[Greg KH]

On Mon, Aug 08, 2022 at 12:20:48PM +0200, Alexander Grund wrote:
> This patch fixes an inconsistency, if not a clear bug, with the extended permissions.
> To quote from the original discussion [1]:
> > The behavior of dontauditx and auditallowx appears to be broken making them useless.
> 
> [1] https://lore.kernel.org/selinux/6a791504-7728-3026-17ee-c22cbff8c3d1@gmail.com/T/
> 
> bauen1 (1):
>   selinux: allow dontauditx and auditallowx rules to take effect without allowx
> 
>  security/selinux/ss/services.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

For obvious reasons, we can't take patches only for 4.9.y that are not
also in newer kernel branches.  You don't want to upgrade to 4.14.y and
have a regression, right?

So this would need to be backported to 4.14.y, 4.19.y, 5.4.y, and 5.10.y
before we could consider it.

BUT, as this is something that just never worked, why is it needed at
all?  Making it work is a "new feature", not really a bugfix for these
older kernels as it is not a regression.

I'll drop this from my queue, if you really think it needs to come back,
we need backports for all affected kernel branches.

thanks,

greg k-h

Re: [PATCH 4.9 0/1] selinux: allow dontauditx and auditallowx rules to take effect without allowx

[Alexander Grund]

> So this would need to be backported to 4.14.y, 4.19.y, 5.4.y, and 5.10.y
> before we could consider it.
> 
> BUT, as this is something that just never worked, why is it needed at
> all?  Making it work is a "new feature", not really a bugfix for these
> older kernels as it is not a regression.

I agree it is not a regression but following the original discussion on this 
I do think it is a bug worth fixing, see the already quoted:

> The behavior of dontauditx and auditallowx appears to be broken making them useless.

At least it is a pitfall for policy writers which can be easily avoided by this small fix.

I don't mind porting this to the other LTS releases if you agree on this judgement.
If so, what would the best/simplest way to do so? An own thread with [0/1] & [1/1] mails
for each LTS branch or e.g. replying to this thread with a patch for each of the LTS
branches?

If you don't agree I won't send those mails which is also fine.

Thanks,
Alex