Re: [PATCH] selinux: refactor mls_context_to_sid() and make it stricter
On Fri, Aug 31, 2018 at 11:47 AM Jann Horn wrote: > On Thu, Aug 9, 2018 at 3:56 AM Paul Moore wrote: > > On Mon, Aug 6, 2018 at 5:19 PM Jann Horn wrote: ... > > In the case where we have a MLS policy loaded (pol->mls_enabled != 0) > > and scontext is empty (scontext[0] = '\0'), we could end up returning > > 0 couldn't we? It seems like we might want a quick check for this > > before we parse the low/high portions of the field into the rangep > > array. > > I don't think so. In the first loop iteration, `sensitivity` will be > an empty string, and so the hashtab_search() should return NULL, > leading to -EINVAL. Am I missing something? Looking at this again, no, I think you've got it right. My guess is that I just mistook the NULL sensitivity check at the top of the loop as getting triggered in this case, which isn't the case here. Sorry for the noise. > > As an aside, I believe my other comments on this patch still stand. > > It's a nice improvement but I think there are some other small things > > that need to be addressed. > > Is there anything I need to fix apart from the overly verbose comment > and the unnecessary curly braces? Nope. I wouldn't even bother with that brace/comment changes, those were minor nits and only worth changing if you needed to respin the patch for some other reason. Consider the patch merged, thanks! -- paul moore www.paul-moore.com ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH 1/2] semanage: "semanage user" does not use -s, fix documentation
Both "semanage user --help" and "man 8 semanage-user" state that
"semanage user" accepts option -s, but this is incorrect: -s is not
needed to specify the SELinux user on the command line, contrary to
"semanage login" for example. Fix the documention.
While at it, remove many spaces from the helptext of option --roles. I
do not know where they came from, but they were reduced to a single
space when displayed anyway.
Signed-off-by: Nicolas Iooss
---
python/semanage/semanage| 4 ++--
python/semanage/semanage-user.8 | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/python/semanage/semanage b/python/semanage/semanage
index 8d8a086094c9..e32d1e8ad387 100644
--- a/python/semanage/semanage
+++ b/python/semanage/semanage
@@ -53,7 +53,7 @@ usage_fcontext = "semanage fcontext [-h] [-n] [-N] [-S STORE]
["
usage_fcontext_dict = {' --add': ('(', '-t TYPE', '-f FTYPE', '-r RANGE', '-s
SEUSER', '|', '-e EQUAL', ')', 'FILE_SPEC',), ' --delete': ('(', '-t TYPE', '-f
FTYPE', '|', '-e EQUAL', ')', 'FILE_SPEC',), ' --modify': ('(', '-t TYPE', '-f
FTYPE', '-r RANGE', '-s SEUSER', '|', '-e EQUAL', ')', 'FILE_SPEC',), '
--list': ('[-C]',), ' --extract': ('',), ' --deleteall': ('',)}
usage_user = "semanage user [-h] [-n] [-N] [-S STORE] ["
-usage_user_dict = {' --add': ('(', '-L LEVEL', '-R ROLES', '-r RANGE', '-s
SEUSER', 'selinux_name'')'), ' --delete': ('selinux_name',), ' --modify': ('(',
'-L LEVEL', '-R ROLES', '-r RANGE', '-s SEUSER', 'selinux_name', ')'), '
--list': ('-C',), ' --extract': ('',), ' --deleteall': ('',)}
+usage_user_dict = {' --add': ('(', '-L LEVEL', '-R ROLES', '-r RANGE',
'SEUSER', ')'), ' --delete': ('SEUSER',), ' --modify': ('(', '-L LEVEL', '-R
ROLES', '-r RANGE', '-s SEUSER', 'SEUSER', ')'), ' --list': ('-C',), '
--extract': ('',), ' --deleteall': ('',)}
usage_port = "semanage port [-h] [-n] [-N] [-S STORE] ["
usage_port_dict = {' --add': ('-t TYPE', '-p PROTOCOL', '-r RANGE', '(',
'port_name', '|', 'port_range', ')'), ' --modify': ('-t TYPE', '-p PROTOCOL',
'-r RANGE', '(', 'port_name', '|', 'port_range', ')'), ' --delete': ('-p
PROTOCOL', '(', 'port_name', '|', 'port_range', ')'), ' --list': ('-C',), '
--extract': ('',), ' --deleteall': ('',)}
@@ -421,7 +421,7 @@ def setupUserParser(subparsers):
userParser.add_argument('-R', '--roles', default=[],
action=CheckRole,
help=_('''
-SELinux Roles. You must enclose multiple roles within quotes,
separate by spaces. Or specify -R multiple times.
+SELinux Roles. You must enclose multiple roles within quotes, separate by
spaces. Or specify -R multiple times.
'''))
userParser.add_argument('-P', '--prefix', default="user",
help=argparse.SUPPRESS)
userParser.add_argument('selinux_name', nargs='?', default=None,
help=_('selinux_name'))
diff --git a/python/semanage/semanage-user.8 b/python/semanage/semanage-user.8
index 30bc67052ed7..23fec698e042 100644
--- a/python/semanage/semanage-user.8
+++ b/python/semanage/semanage-user.8
@@ -2,7 +2,7 @@
.SH "NAME"
.B semanage\-user \- SELinux Policy Management SELinux User mapping tool
.SH "SYNOPSIS"
-.B semanage user [\-h] [\-n] [\-N] [\-S STORE] [ \-\-add ( \-L LEVEL \-R
ROLES \-r RANGE \-s SEUSER selinux_name) | \-\-delete selinux_name |
\-\-deleteall | \-\-extract | \-\-list [\-C] | \-\-modify ( \-L LEVEL \-R
ROLES \-r RANGE \-s SEUSER selinux_name ) ]
+.B semanage user [\-h] [\-n] [\-N] [\-S STORE] [ \-\-add ( \-L LEVEL \-R
ROLES \-r RANGE SEUSER) | \-\-delete SEUSER | \-\-deleteall | \-\-extract |
\-\-list [\-C] | \-\-modify ( \-L LEVEL \-R ROLES \-r RANGE SEUSER ) ]
.SH "DESCRIPTION"
semanage is used to configure certain elements of
--
2.18.0
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH 2/2] semanage: add a missing space in ibendport help
Currently, in:
# semanage ibendport --help
usage: semanage ibendport [-h] [-n] [-N] [-s STORE] [ --add -t TYPE
-z IBDEV_NAME -r RANGE ( port ) | --delete -z IBDEV_NAME -r RANGE(
port ) | --deleteall | --extract | --list -C | --modify -t TYPE -z
IBDEV_NAME -r RANGE ( port ) ]
... a space is missing between "RANGE" and "( port )" in the usage of
--delete. Add it by splitting the string correctly in the usage line
definition.
Signed-off-by: Nicolas Iooss
---
python/semanage/semanage | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python/semanage/semanage b/python/semanage/semanage
index e32d1e8ad387..f4be97507c18 100644
--- a/python/semanage/semanage
+++ b/python/semanage/semanage
@@ -62,7 +62,7 @@ usage_ibpkey = "semanage ibpkey [-h] [-n] [-N] [-s STORE] ["
usage_ibpkey_dict = {' --add': ('-t TYPE', '-x SUBNET_PREFIX', '-r RANGE',
'(', 'ibpkey_name', '|', 'pkey_range', ')'), ' --modify': ('-t TYPE', '-x
SUBNET_PREFIX', '-r RANGE', '(', 'ibpkey_name', '|', 'pkey_range', ')'), '
--delete': ('-x SUBNET_PREFIX', '(', 'ibpkey_name', '|', 'pkey_range', ')'), '
--list': ('-C',), ' --extract': ('',), ' --deleteall': ('',)}
usage_ibendport = "semanage ibendport [-h] [-n] [-N] [-s STORE] ["
-usage_ibendport_dict = {' --add': ('-t TYPE', '-z IBDEV_NAME', '-r RANGE',
'(', 'port', ')'), ' --modify': ('-t TYPE', '-z IBDEV_NAME', '-r RANGE', '(',
'port', ')'), ' --delete': ('-z IBDEV_NAME', '-r RANGE''(', 'port', ')'), '
--list': ('-C',), ' --extract': ('',), ' --deleteall': ('',)}
+usage_ibendport_dict = {' --add': ('-t TYPE', '-z IBDEV_NAME', '-r RANGE',
'(', 'port', ')'), ' --modify': ('-t TYPE', '-z IBDEV_NAME', '-r RANGE', '(',
'port', ')'), ' --delete': ('-z IBDEV_NAME', '-r RANGE', '(', 'port', ')'), '
--list': ('-C',), ' --extract': ('',), ' --deleteall': ('',)}
usage_node = "semanage node [-h] [-n] [-N] [-S STORE] ["
usage_node_dict = {' --add': ('-M NETMASK', '-p PROTOCOL', '-t TYPE', '-r
RANGE', 'node'), ' --modify': ('-M NETMASK', '-p PROTOCOL', '-t TYPE', '-r
RANGE', 'node'), ' --delete': ('-M NETMASK', '-p PROTOCOL', 'node'), ' --list':
('-C',), ' --extract': ('',), ' --deleteall': ('',)}
--
2.18.0
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: "semanage user" and -s option
On 09/05/2018 03:36 PM, Nicolas Iooss wrote: Hello, While reviewing the last patch sent by Vit Mojzis, I stumbled upon something that does not feel right in "semanage user". Both "semanage user --help" and "man 8 semanage-user" state: usage: semanage user [-h] [-n] [-N] [-S STORE] [ --add ( -L LEVEL -R ROLES -r RANGE -s SEUSER selinux_name) | ... I am wondering what are the meaning of "-s SEUSER" and "selinux_name" there. If I try to use "-s" option, semanage complains: semanage: error: unrecognized arguments: -s Therefore it seems that the usage would rather be "... --add ( -L LEVEL -R ROLES -r RANGE SEUSER)". Looking at the code, it seems that parser_add_seuser() is not used in setupUserParser() [1], and everything works fine when using "semanage user" without -s option. Am I missing something obvious, or should I write a patch which fixes the documentation? Sounds like a cut-and-paste error from the semanage login help and man page. The examples in the man page don't ever use -s to semanage user, nor does python/semanage/test-semanage.py or python/sepolicy/templates/*.py. Cheers, Nicolas [1] https://github.com/SELinuxProject/selinux/blob/libsemanage-2.8/python/semanage/semanage#L403 ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: "semanage user" and -s option
On Wed, Sep 05, 2018 at 09:36:35PM +0200, Nicolas Iooss wrote: > Hello, > > While reviewing the last patch sent by Vit Mojzis, I stumbled upon > something that does not feel right in "semanage user". Both "semanage > user --help" and "man 8 semanage-user" state: > > usage: semanage user [-h] [-n] [-N] [-S STORE] [ --add ( -L LEVEL -R > ROLES -r RANGE -s SEUSER selinux_name) | ... > > I am wondering what are the meaning of "-s SEUSER" and "selinux_name" > there. If I try to use "-s" option, semanage complains: > > semanage: error: unrecognized arguments: -s If i recall correctly think "-s selinux_name" is right. You cannot add a seuser withuser giving it a name. > > Therefore it seems that the usage would rather be "... --add ( -L > LEVEL -R ROLES -r RANGE SEUSER)". Looking at the code, it seems that > parser_add_seuser() is not used in setupUserParser() [1], and > everything works fine when using "semanage user" without -s option. Am > I missing something obvious, or should I write a patch which fixes the > documentation? > > Cheers, > Nicolas > > [1] > https://github.com/SELinuxProject/selinux/blob/libsemanage-2.8/python/semanage/semanage#L403 > > ___ > Selinux mailing list > Selinux@tycho.nsa.gov > To unsubscribe, send email to selinux-le...@tycho.nsa.gov. > To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov. -- Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 https://sks-keyservers.net/pks/lookup?op=get=0x3B6C5F1D2C7B6B02 Dominick Grift signature.asc Description: PGP signature ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH 1/1] libsemanage: use previous seuser when getting the previous name
I missed this bug in commit 9ec0ea143ab5 ("libsemanage: use previous
seuser when getting the previous name").
Signed-off-by: Nicolas Iooss
---
libsemanage/src/seusers_local.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libsemanage/src/seusers_local.c b/libsemanage/src/seusers_local.c
index 5fbb09e40b69..a79e2d3d230b 100644
--- a/libsemanage/src/seusers_local.c
+++ b/libsemanage/src/seusers_local.c
@@ -72,7 +72,7 @@ static int semanage_seuser_audit(semanage_handle_t * handle,
int rc = -1;
strcpy(msg, "login");
if (previous) {
- name = semanage_seuser_get_name(seuser);
+ name = semanage_seuser_get_name(previous);
psename = semanage_seuser_get_sename(previous);
pmls = semanage_seuser_get_mlsrange(previous);
proles = semanage_user_roles(handle, psename);
--
2.18.0
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libsemanage: Include user name in ROLE_REMOVE audit events
On Wed, Sep 5, 2018 at 10:01 PM Nicolas Iooss wrote:
>
> On Fri, Aug 24, 2018 at 1:16 PM Vit Mojzis wrote:
> >
> > Use "previous" user name when no new user is available in
> > semanage_seuser_audit. Otherwise "id=0" is logged instead of
> > "acct=user_name" ("id=0" is hard coded value).
> >
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1622045
>
> Hi,
> Thanks for the patch! Reviewing it took some time because I was quite
> unfamiliar with the audit logs generated by semanage and was wondering
> where "id=0" come from, and when I tried to use semanage login and
> semanage user to get these audit logs, I got surprised by something in
> semanage user documentation...
>
> Anyway, for the record, "id=0" comes from the 0 in the call to
> audit_log_semanage_message() that occurs in semanage_seuser_audit(),
> and according to libaudit source code [1], id is only used when name
> is NULL. So your patch looks good to me and I merged it.
>
Oops, on a closer inspection it seems that your patch is not so good:
when seuser is NULL, semanage_seuser_get_name(seuser) dereferences a
NULL pointer and crashes. This needed to be name =
semanage_seuser_get_name(previous); Sorry for this, I'll send a fix.
Nicolas
> [1]
> https://github.com/linux-audit/audit-userspace/blob/e42602b7b246ae62e7a12e9cd91f0ac37b1b1968/lib/audit_logging.c#L586
>
> > ---
> > libsemanage/src/seusers_local.c | 11 ++-
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/libsemanage/src/seusers_local.c
> > b/libsemanage/src/seusers_local.c
> > index 413ebddd..5fbb09e4 100644
> > --- a/libsemanage/src/seusers_local.c
> > +++ b/libsemanage/src/seusers_local.c
> > @@ -71,17 +71,18 @@ static int semanage_seuser_audit(semanage_handle_t *
> > handle,
> > const char *sep = "-";
> > int rc = -1;
> > strcpy(msg, "login");
> > + if (previous) {
> > + name = semanage_seuser_get_name(seuser);
> > + psename = semanage_seuser_get_sename(previous);
> > + pmls = semanage_seuser_get_mlsrange(previous);
> > + proles = semanage_user_roles(handle, psename);
> > + }
> > if (seuser) {
> > name = semanage_seuser_get_name(seuser);
> > sename = semanage_seuser_get_sename(seuser);
> > mls = semanage_seuser_get_mlsrange(seuser);
> > roles = semanage_user_roles(handle, sename);
> > }
> > - if (previous) {
> > - psename = semanage_seuser_get_sename(previous);
> > - pmls = semanage_seuser_get_mlsrange(previous);
> > - proles = semanage_user_roles(handle, psename);
> > - }
> > if (audit_type != AUDIT_ROLE_REMOVE) {
> > if (sename && (!psename || strcmp(psename, sename) != 0)) {
> > strcat(msg,sep);
> > --
> > 2.14.3
> >
> > ___
> > Selinux mailing list
> > Selinux@tycho.nsa.gov
> > To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> > To get help, send an email containing "help" to
> > selinux-requ...@tycho.nsa.gov.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] libsemanage: Include user name in ROLE_REMOVE audit events
On Fri, Aug 24, 2018 at 1:16 PM Vit Mojzis wrote:
>
> Use "previous" user name when no new user is available in
> semanage_seuser_audit. Otherwise "id=0" is logged instead of
> "acct=user_name" ("id=0" is hard coded value).
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1622045
Hi,
Thanks for the patch! Reviewing it took some time because I was quite
unfamiliar with the audit logs generated by semanage and was wondering
where "id=0" come from, and when I tried to use semanage login and
semanage user to get these audit logs, I got surprised by something in
semanage user documentation...
Anyway, for the record, "id=0" comes from the 0 in the call to
audit_log_semanage_message() that occurs in semanage_seuser_audit(),
and according to libaudit source code [1], id is only used when name
is NULL. So your patch looks good to me and I merged it.
Thanks,
Nicolas
[1]
https://github.com/linux-audit/audit-userspace/blob/e42602b7b246ae62e7a12e9cd91f0ac37b1b1968/lib/audit_logging.c#L586
> ---
> libsemanage/src/seusers_local.c | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/libsemanage/src/seusers_local.c b/libsemanage/src/seusers_local.c
> index 413ebddd..5fbb09e4 100644
> --- a/libsemanage/src/seusers_local.c
> +++ b/libsemanage/src/seusers_local.c
> @@ -71,17 +71,18 @@ static int semanage_seuser_audit(semanage_handle_t *
> handle,
> const char *sep = "-";
> int rc = -1;
> strcpy(msg, "login");
> + if (previous) {
> + name = semanage_seuser_get_name(seuser);
> + psename = semanage_seuser_get_sename(previous);
> + pmls = semanage_seuser_get_mlsrange(previous);
> + proles = semanage_user_roles(handle, psename);
> + }
> if (seuser) {
> name = semanage_seuser_get_name(seuser);
> sename = semanage_seuser_get_sename(seuser);
> mls = semanage_seuser_get_mlsrange(seuser);
> roles = semanage_user_roles(handle, sename);
> }
> - if (previous) {
> - psename = semanage_seuser_get_sename(previous);
> - pmls = semanage_seuser_get_mlsrange(previous);
> - proles = semanage_user_roles(handle, psename);
> - }
> if (audit_type != AUDIT_ROLE_REMOVE) {
> if (sename && (!psename || strcmp(psename, sename) != 0)) {
> strcat(msg,sep);
> --
> 2.14.3
>
> ___
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
"semanage user" and -s option
Hello, While reviewing the last patch sent by Vit Mojzis, I stumbled upon something that does not feel right in "semanage user". Both "semanage user --help" and "man 8 semanage-user" state: usage: semanage user [-h] [-n] [-N] [-S STORE] [ --add ( -L LEVEL -R ROLES -r RANGE -s SEUSER selinux_name) | ... I am wondering what are the meaning of "-s SEUSER" and "selinux_name" there. If I try to use "-s" option, semanage complains: semanage: error: unrecognized arguments: -s Therefore it seems that the usage would rather be "... --add ( -L LEVEL -R ROLES -r RANGE SEUSER)". Looking at the code, it seems that parser_add_seuser() is not used in setupUserParser() [1], and everything works fine when using "semanage user" without -s option. Am I missing something obvious, or should I write a patch which fixes the documentation? Cheers, Nicolas [1] https://github.com/SELinuxProject/selinux/blob/libsemanage-2.8/python/semanage/semanage#L403 ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: WARNING in apparmor_secid_to_secctx
On 9/5/2018 4:08 AM, Dmitry Vyukov wrote: > Thanks! I've re-enabled selinux on syzbot: > https://github.com/google/syzkaller/commit/196410e4f5665d4d2bf6c818d06f1c8d03cfa8cc > Now we will have instances with apparmor and with selinux. Any chance we could get a Smack instance as well? ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: WARNING in apparmor_secid_to_secctx
On Tue, Sep 4, 2018 at 7:53 AM, Dmitry Vyukov wrote: > Again, if you change wheezy to stretch here, it should reproduce the problem: > https://github.com/google/syzkaller/blob/master/tools/create-image.sh FWIW, I sent an update for the image version here: https://github.com/google/syzkaller/pull/708 -Kees -- Kees Cook Pixel Security ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: WARNING in apparmor_secid_to_secctx
On Wed, Sep 5, 2018 at 3:21 AM, Paul Moore wrote:
>> So why not ask for help from the SELinux community? I've cc'd the
>> selinux
>> list and a couple of folks involved in Debian selinux. I see a couple
>> of
>> options but I don't know your constraints for syzbot:
>>
>> 1) Run an instance of syzbot on a distro that supports SELinux enabled
>> out
>> of the box like Fedora. Then you don't have to fight with SELinux and
>> can
>> just focus on syzbot, while still testing SELinux enabled and enforcing.
>>
>> 2) Report the problems you are having with enabling SELinux on newer
>> Debian
>> to the selinux list and/or the Debian selinux package maintainers so
>> that
>> someone can help you resolve them.
>>
>> 3) Back-port the cgroup2 policy definitions to your wheezy policy,
>> rebuild
>> it, and install that. We could help provide guidance on that. I think
>> you'll need to rebuild the base policy on wheezy; in distributions with
>> modern SELinux userspace, one could do it just by adding a CIL module
>> locally.
>> >>>
>> >>>
>> >>> Thanks, Stephen!
>> >>>
>> >>> I would like to understand first if failing mount(2) for unknown fs is
>> >>> selinux bug or not. Because if it is and it is fixed, then it would
>> >>> resolve the problem without actually doing anything (well, at least on
>> >>> our side :)).
>> >>
>> >>
>> >> Yes, I think that's a selinux kernel regression, previously reported here:
>> >> https://lkml.org/lkml/2017/10/6/658
>> >>
>> >> Unfortunately I don't think it has been fixed upstream. Generally people
>> >> using SELinux with a newer kernel are also using a newer policy. That
>> >> said,
>> >> I agree it is a regression and ought to be fixed.
>> >
>> >
>> > How hard is it to fix it? We are on upstream head, so once it's in we
>> > are ready to go.
>> > Using multiple images is somewhat problematic (besides the fact that I
>> > don't know how to build a fedora image) because syzbot does not
>> > capture what image was used, and in the docs we just provide the
>> > single image, so people will start complaining that bugs don't
>> > reproduce but they are just using a wrong image.
>>
>> I'll take a look and see if I can provide a trivial fix.
>
> As a FYI, Stephen provided a patch and it has been merged into the
> selinux/next tree.
Thanks! I've re-enabled selinux on syzbot:
https://github.com/google/syzkaller/commit/196410e4f5665d4d2bf6c818d06f1c8d03cfa8cc
Now we will have instances with apparmor and with selinux.
> * git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
> * https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
>
> Author: Stephen Smalley
> Date: Tue Sep 4 16:51:36 2018 -0400
>
>selinux: fix mounting of cgroup2 under older policies
>
>commit 901ef845fa2469c ("selinux: allow per-file labeling for cgroupfs")
>broke mounting of cgroup2 under older SELinux policies which lacked
>a genfscon rule for cgroup2. This prevents mounting of cgroup2 even
>when SELinux is permissive.
>
>Change the handling when there is no genfscon rule in policy to
>just mark the inode unlabeled and not return an error to the caller.
>This permits mounting and access if allowed by policy, e.g. to
>unconfined domains.
>
>I also considered changing the behavior of security_genfs_sid() to
>never return -ENOENT, but the current behavior is relied upon by
>other callers to perform caller-specific handling.
>
>Fixes: 901ef845fa2469c ("selinux: allow per-file labeling for cgroupfs")
>CC:
>Reported-by: Dmitry Vyukov
>Reported-by: Waiman Long
>Signed-off-by: Stephen Smalley
>Tested-by: Waiman Long
>Signed-off-by: Paul Moore
>
> --
> paul moore
> www.paul-moore.com
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] selinux: fix mounting of cgroup2 under older policies
On 09/04/2018 04:51 PM, Stephen Smalley wrote:
> commit 901ef845fa2469c ("selinux: allow per-file labeling for cgroupfs")
> broke mounting of cgroup2 under older SELinux policies which lacked
> a genfscon rule for cgroup2. This prevents mounting of cgroup2 even
> when SELinux is permissive.
>
> Change the handling when there is no genfscon rule in policy to
> just mark the inode unlabeled and not return an error to the caller.
> This permits mounting and access if allowed by policy, e.g. to
> unconfined domains.
>
> I also considered changing the behavior of security_genfs_sid() to
> never return -ENOENT, but the current behavior is relied upon by
> other callers to perform caller-specific handling.
>
> Fixes: 901ef845fa2469c ("selinux: allow per-file labeling for cgroupfs")
> CC:
> Reported-by: Dmitry Vyukov
> Reported-by: Waiman Long
> Signed-off-by: Stephen Smalley
> ---
> security/selinux/hooks.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f78318af8254..58fee382a3bb 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1508,6 +1508,11 @@ static int selinux_genfs_get_sid(struct dentry *dentry,
> }
> rc = security_genfs_sid(_state, sb->s_type->name,
> path, tclass, sid);
> + if (rc == -ENOENT) {
> + /* No match in policy, mark as unlabeled. */
> + *sid = SECINITSID_UNLABELED;
> + rc = 0;
> + }
> }
> free_page((unsigned long)buffer);
> return rc;
I have tested this patch and it works in my case.
Tested-by: Waiman Long
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] selinux: fix mounting of cgroup2 under older policies
On Tue, Sep 4, 2018 at 6:18 PM Paul Moore wrote:
> On Tue, Sep 4, 2018 at 4:49 PM Stephen Smalley wrote:
> >
> > commit 901ef845fa2469c ("selinux: allow per-file labeling for cgroupfs")
> > broke mounting of cgroup2 under older SELinux policies which lacked
> > a genfscon rule for cgroup2. This prevents mounting of cgroup2 even
> > when SELinux is permissive.
> >
> > Change the handling when there is no genfscon rule in policy to
> > just mark the inode unlabeled and not return an error to the caller.
> > This permits mounting and access if allowed by policy, e.g. to
> > unconfined domains.
> >
> > I also considered changing the behavior of security_genfs_sid() to
> > never return -ENOENT, but the current behavior is relied upon by
> > other callers to perform caller-specific handling.
> >
> > Fixes: 901ef845fa2469c ("selinux: allow per-file labeling for cgroupfs")
> > CC:
> > Reported-by: Dmitry Vyukov
> > Reported-by: Waiman Long
> > Signed-off-by: Stephen Smalley
> > ---
> > security/selinux/hooks.c | 5 +
> > 1 file changed, 5 insertions(+)
>
> Looks like a reasonable approach to me, merged into selinux/next, thanks.
I probably should expand a bit on this as Stephen's stable CC marking
and the patch's inclusion in selinux/next (as opposed to
selinux/stable-4.19) don't quite match. I merged this into the next
branch because I didn't feel that this was a severe enough problem to
warrant immediate inclusion into the stable-4.19 branch and since this
is a user visible change I felt some additional time in the next
branch would be valuable. However, I did leave the CC stable marking
intact so that when this patch does hit Linus tree (expected during
the v4.20 merge window) it should get backported to the various stable
trees.
> As a FYI, since the US holiday and LSS-NA delayed the start of merging
> things into selinux/next I've updated selinux/next on top of v4.19-rc2
> instead of -rc1 this time around.
>
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index f78318af8254..58fee382a3bb 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -1508,6 +1508,11 @@ static int selinux_genfs_get_sid(struct dentry
> > *dentry,
> > }
> > rc = security_genfs_sid(_state, sb->s_type->name,
> > path, tclass, sid);
> > + if (rc == -ENOENT) {
> > + /* No match in policy, mark as unlabeled. */
> > + *sid = SECINITSID_UNLABELED;
> > + rc = 0;
> > + }
> > }
> > free_page((unsigned long)buffer);
> > return rc;
> > --
> > 2.14.4
>
> --
> paul moore
> www.paul-moore.com
--
paul moore
www.paul-moore.com
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
