Re: [RFC PATCH] selinux: add a fallback to defcontext for native labeling
Quoting Stephen Smalley (2018-09-25 09:39:55) > On 09/25/2018 12:03 PM, Paul Moore wrote: > > On Tue, Sep 25, 2018 at 9:58 AM Stephen Smalley wrote: > >> I'm inclined to just change the behavior for defcontext= unconditionally > >> and have it apply to both native and xattr labeling. If that's a no-go, > >> then the simplest solution is to just leave defcontext= behavior > >> unchanged for xattr labeling and only implement the new semantics for > >> native labeling. That's just a matter of adding a flag to > >> security_context_to_sid_default() and only setting it when calling from > >> selinux_inode_notifysecctx(). > > > > Neither option is very appealing to me, but that doesn't mean I'm saying > > "no". > > > > From a sanity and consistency point of view I think option #1 (change > > the defcontext behavior) is a better choice, and I tend to favor this > > consistency even with the understanding that it could result in some > > unexpected behavior for users. However, if we get complaints, I'm > > going to revert this without a second thought. > > In that case, I'd suggest splitting it into two patches; first one only > enables the new behavior for native labeling filesystems (as per the > above, via a flag to security_context_to_sid_default), and the second > patch drops the flag and does it unconditionally. Then you can always > revert the latter without affecting the former. > > > > > So to answer your question Taras, go ahead and prepare a patch so we > > can take a look. A bit of fair warning that it might get delayed > > until after the upcoming merge window since we are already at -rc5; I > > want this to have plenty of time in -next. > > > > Thanks guys. Thanks. I'll prepare patches is a few days. ___ 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: [RFC PATCH] selinux: add a fallback to defcontext for native labeling
On 09/25/2018 12:03 PM, Paul Moore wrote: On Tue, Sep 25, 2018 at 9:58 AM Stephen Smalley wrote: On 09/25/2018 01:45 AM, Taras Kondratiuk wrote: Quoting Paul Moore (2018-09-24 20:46:57) On Fri, Sep 21, 2018 at 10:39 AM Stephen Smalley wrote: On 09/20/2018 06:59 PM, Taras Kondratiuk wrote: Quoting Stephen Smalley (2018-09-20 07:49:12) On 09/19/2018 10:41 PM, Taras Kondratiuk wrote: Quoting Stephen Smalley (2018-09-19 12:00:33) On 09/19/2018 12:52 PM, Taras Kondratiuk wrote: ... IMO it would be more consistent if defcontext cover all "unlabeled" groups. It seems unlikely to me that somebody who currently uses defcontext can somehow rely on mapping invalid labels to unlabeled instead of default context. Yes, and that seems more consistent with the current documentation in the mount man page for defcontext=. I'd be inclined to change selinux_inode_notifysecctx() to call security_context_to_sid_default() directly instead of using selinux_inode_setsecurity() and change security_context_to_sid_core() and sidtab_search_core() as suggested above to save and use the def_sid instead of SECINITSID_UNLABELED always (initializing the context def_sid to SECINITSID_UNLABELED as the default). selinux_inode_setsecurity() we should leave unchanged, or if we change it at all, it should be more like the handling in selinux_inode_setxattr(). The notifysecctx hook is invoked by the filesystem to notify the security module of the file's existing security context, so in that case we always want the _default behavior, whereas the setsecurity hook is invoked by the vfs or the filesystem to set the security context of a file to a new value, so in that case we would only use the _force interface if the caller had CAP_MAC_ADMIN. Paul, what say you? NB This would be a user-visible behavior change for mounts specifying defcontext= on xattr filesystems; files with invalid contexts will then show up with the defcontext value instead of the unlabeled context. If that's too risky, then we'd need a flag or something to security_context_to_sid_default() to distinguish the behaviors and only set it when called from selinux_inode_notifysecctx(). Visible changes like this are always worrisome, even though I think it is safe to assume that the defcontext option is not widely used. I'd feel much better if this change was opt-in. Which brings about it's own problems. We have the policy capability functionality, but that is likely a poor fit for this as the policy capabilities are usually controlled by the Linux distribution while the mount options are set by the system's administrator when the filesystem is mounted. We could add a toggle somewhere in selinuxfs, but I really dislike that idea, and would prefer to find a different solution if possible. I'm not sure how much flak we would get for introducing a new mount option, but perhaps that is the best way to handle this: defcontext would continue to behave as it does now, but new option X would behave as mentioned in this thread. Thoughts? The new option X will also mean "default context", so it will be pretty hard to come up with a different but a sensible name. I'm afraid that having two options with hardly distinguishable semantics will be very confusing. What about a kernel config option that modifies the behavior of defcontext? First, the existing documentation for defcontext= leaves open the door to the proposed new behavior. From mount(8): "You can set the default security context for unlabeled files using defcontext= option. This overrides the value set for unlabeled files in the policy and requires a filesystem that supports xattr labeling." "Unlabeled files" could just mean files without any xattr, or it could mean all files that either lack an xattr or have an invalid one under the policy, since both sets of files are currently mapped to the unlabeled context. This may be true for the major SELinux policies being shipped by distributions, but can we say this holds true for *all* SELinux policies in use today? Second, under what conditions would this situation break existing userspace? The admin would have had to mount an xattr filesystem with defcontext= specified, and that filesystem would have to both contain files without any xattrs and files with invalid ones (otherwise how they are treated by the kernel is irrelevant), and the policy would need to distinguish access to files without xattrs vs files with invalid ones. Seems unlikely. I think you answered your own question. Yes, it is unlikely, but I *really* dislike changing visible behavior like this without some explicit action on behalf of the user/admin. We've done it in the past and it has left me uneasy each time. Third, the fact that policy maintainers remapped both SECINITSID_FILE (the default value for defcontext) and SECINITSID_UNLABELED (used for invalid contexts) to the unlabeled context (getting rid of file_t as a separate type, aliased to unlabeled_t) long ago
Re: [RFC PATCH] selinux: add a fallback to defcontext for native labeling
On Tue, Sep 25, 2018 at 9:58 AM Stephen Smalley wrote: > On 09/25/2018 01:45 AM, Taras Kondratiuk wrote: > > Quoting Paul Moore (2018-09-24 20:46:57) > >> On Fri, Sep 21, 2018 at 10:39 AM Stephen Smalley > >> wrote: > >>> On 09/20/2018 06:59 PM, Taras Kondratiuk wrote: > Quoting Stephen Smalley (2018-09-20 07:49:12) > > On 09/19/2018 10:41 PM, Taras Kondratiuk wrote: > >> Quoting Stephen Smalley (2018-09-19 12:00:33) > >>> On 09/19/2018 12:52 PM, Taras Kondratiuk wrote: > >> > >> ... > >> > IMO it would be more consistent if defcontext cover all "unlabeled" > groups. It seems unlikely to me that somebody who currently uses > defcontext can somehow rely on mapping invalid labels to unlabeled > instead of default context. > >>> > >>> Yes, and that seems more consistent with the current documentation in > >>> the mount man page for defcontext=. > >>> > >>> I'd be inclined to change selinux_inode_notifysecctx() to call > >>> security_context_to_sid_default() directly instead of using > >>> selinux_inode_setsecurity() and change security_context_to_sid_core() > >>> and sidtab_search_core() as suggested above to save and use the def_sid > >>> instead of SECINITSID_UNLABELED always (initializing the context def_sid > >>> to SECINITSID_UNLABELED as the default). selinux_inode_setsecurity() we > >>> should leave unchanged, or if we change it at all, it should be more > >>> like the handling in selinux_inode_setxattr(). The notifysecctx hook is > >>> invoked by the filesystem to notify the security module of the file's > >>> existing security context, so in that case we always want the _default > >>> behavior, whereas the setsecurity hook is invoked by the vfs or the > >>> filesystem to set the security context of a file to a new value, so in > >>> that case we would only use the _force interface if the caller had > >>> CAP_MAC_ADMIN. > >>> > >>> Paul, what say you? NB This would be a user-visible behavior change for > >>> mounts specifying defcontext= on xattr filesystems; files with invalid > >>> contexts will then show up with the defcontext value instead of the > >>> unlabeled context. If that's too risky, then we'd need a flag or > >>> something to security_context_to_sid_default() to distinguish the > >>> behaviors and only set it when called from selinux_inode_notifysecctx(). > >> > >> Visible changes like this are always worrisome, even though I think it > >> is safe to assume that the defcontext option is not widely used. I'd > >> feel much better if this change was opt-in. > >> > >> Which brings about it's own problems. We have the policy capability > >> functionality, but that is likely a poor fit for this as the policy > >> capabilities are usually controlled by the Linux distribution while > >> the mount options are set by the system's administrator when the > >> filesystem is mounted. We could add a toggle somewhere in selinuxfs, > >> but I really dislike that idea, and would prefer to find a different > >> solution if possible. I'm not sure how much flak we would get for > >> introducing a new mount option, but perhaps that is the best way to > >> handle this: defcontext would continue to behave as it does now, but > >> new option X would behave as mentioned in this thread. > >> > >> Thoughts? > > > > The new option X will also mean "default context", so it will be pretty > > hard to come up with a different but a sensible name. I'm afraid that > > having two options with hardly distinguishable semantics will be very > > confusing. > > > > What about a kernel config option that modifies the behavior of > > defcontext? > > First, the existing documentation for defcontext= leaves open the door > to the proposed new behavior. From mount(8): > "You can set the default security context for unlabeled files using > defcontext= option. This overrides the value set for unlabeled files in > the policy and requires a filesystem that supports xattr labeling." > > "Unlabeled files" could just mean files without any xattr, or it could > mean all files that either lack an xattr or have an invalid one under > the policy, since both sets of files are currently mapped to the > unlabeled context. This may be true for the major SELinux policies being shipped by distributions, but can we say this holds true for *all* SELinux policies in use today? > Second, under what conditions would this situation break existing > userspace? The admin would have had to mount an xattr filesystem with > defcontext= specified, and that filesystem would have to both contain > files without any xattrs and files with invalid ones (otherwise how they > are treated by the kernel is irrelevant), and the policy would need to > distinguish access to files without xattrs vs files with invalid ones. > Seems unlikely. I think you answered your own question. Yes, it is unlikely, but I *really* dislike changing visible behavior like this without some explicit action on behalf of the user/admin.
Re: [RFC PATCH] selinux: add a fallback to defcontext for native labeling
On Tue, Sep 25, 2018 at 1:45 AM Taras Kondratiuk wrote: > Quoting Paul Moore (2018-09-24 20:46:57) > > On Fri, Sep 21, 2018 at 10:39 AM Stephen Smalley wrote: > > > On 09/20/2018 06:59 PM, Taras Kondratiuk wrote: > > > > Quoting Stephen Smalley (2018-09-20 07:49:12) > > > >> On 09/19/2018 10:41 PM, Taras Kondratiuk wrote: > > > >>> Quoting Stephen Smalley (2018-09-19 12:00:33) > > > On 09/19/2018 12:52 PM, Taras Kondratiuk wrote: > > > > ... > > > > > > IMO it would be more consistent if defcontext cover all "unlabeled" > > > > groups. It seems unlikely to me that somebody who currently uses > > > > defcontext can somehow rely on mapping invalid labels to unlabeled > > > > instead of default context. > > > > > > Yes, and that seems more consistent with the current documentation in > > > the mount man page for defcontext=. > > > > > > I'd be inclined to change selinux_inode_notifysecctx() to call > > > security_context_to_sid_default() directly instead of using > > > selinux_inode_setsecurity() and change security_context_to_sid_core() > > > and sidtab_search_core() as suggested above to save and use the def_sid > > > instead of SECINITSID_UNLABELED always (initializing the context def_sid > > > to SECINITSID_UNLABELED as the default). selinux_inode_setsecurity() we > > > should leave unchanged, or if we change it at all, it should be more > > > like the handling in selinux_inode_setxattr(). The notifysecctx hook is > > > invoked by the filesystem to notify the security module of the file's > > > existing security context, so in that case we always want the _default > > > behavior, whereas the setsecurity hook is invoked by the vfs or the > > > filesystem to set the security context of a file to a new value, so in > > > that case we would only use the _force interface if the caller had > > > CAP_MAC_ADMIN. > > > > > > Paul, what say you? NB This would be a user-visible behavior change for > > > mounts specifying defcontext= on xattr filesystems; files with invalid > > > contexts will then show up with the defcontext value instead of the > > > unlabeled context. If that's too risky, then we'd need a flag or > > > something to security_context_to_sid_default() to distinguish the > > > behaviors and only set it when called from selinux_inode_notifysecctx(). > > > > Visible changes like this are always worrisome, even though I think it > > is safe to assume that the defcontext option is not widely used. I'd > > feel much better if this change was opt-in. > > > > Which brings about it's own problems. We have the policy capability > > functionality, but that is likely a poor fit for this as the policy > > capabilities are usually controlled by the Linux distribution while > > the mount options are set by the system's administrator when the > > filesystem is mounted. We could add a toggle somewhere in selinuxfs, > > but I really dislike that idea, and would prefer to find a different > > solution if possible. I'm not sure how much flak we would get for > > introducing a new mount option, but perhaps that is the best way to > > handle this: defcontext would continue to behave as it does now, but > > new option X would behave as mentioned in this thread. > > > > Thoughts? > > The new option X will also mean "default context", so it will be pretty > hard to come up with a different but a sensible name. I'm afraid that > having two options with hardly distinguishable semantics will be very > confusing. > > What about a kernel config option that modifies the behavior of > defcontext? A kernel config option would be going in the wrong direction; that would put it almost completely under the control of the distribution. -- 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 1/2] whitespace and spelling cleanup
Both patches were applied:
https://github.com/SELinuxProject/selinux/pull/100
On Mon, Sep 24, 2018 at 11:55 AM William Roberts
wrote:
> ack
>
> On Mon, Sep 24, 2018 at 11:12 AM Nick Kralevich via Selinux <
> selinux@tycho.nsa.gov> wrote:
>
>> Signed-off-by: Nick Kralevich
>> ---
>> libsepol/include/sepol/errcodes.h | 2 +-
>> secilc/secilc.c | 14 +++---
>> 2 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/libsepol/include/sepol/errcodes.h
>> b/libsepol/include/sepol/errcodes.h
>> index 0136564a..6e9ff316 100644
>> --- a/libsepol/include/sepol/errcodes.h
>> +++ b/libsepol/include/sepol/errcodes.h
>> @@ -12,7 +12,7 @@ extern "C" {
>> #define SEPOL_OK 0
>>
>> /* These first error codes are defined for compatibility with
>> - * previous version of libsepol. In the future, custome error
>> + * previous version of libsepol. In the future, custom error
>> * codes that don't map to system error codes should be defined
>> * outside of the range of system error codes.
>> */
>> diff --git a/secilc/secilc.c b/secilc/secilc.c
>> index 0be6975b..e1347205 100644
>> --- a/secilc/secilc.c
>> +++ b/secilc/secilc.c
>> @@ -1,16 +1,16 @@
>> /*
>> * Copyright 2011 Tresys Technology, LLC. All rights reserved.
>> - *
>> + *
>> * Redistribution and use in source and binary forms, with or without
>> * modification, are permitted provided that the following conditions
>> are met:
>> - *
>> + *
>> *1. Redistributions of source code must retain the above copyright
>> notice,
>> * this list of conditions and the following disclaimer.
>> - *
>> + *
>> *2. Redistributions in binary form must reproduce the above
>> copyright notice,
>> * this list of conditions and the following disclaimer in the
>> documentation
>> * and/or other materials provided with the distribution.
>> - *
>> + *
>> * THIS SOFTWARE IS PROVIDED BY TRESYS TECHNOLOGY, LLC ``AS IS'' AND ANY
>> EXPRESS
>> * OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
>> WARRANTIES OF
>> * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
>> IN NO
>> @@ -21,7 +21,7 @@
>> * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
>> NEGLIGENCE
>> * OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
>> EVEN IF
>> * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> - *
>> + *
>> * The views and conclusions contained in the software and documentation
>> are those
>> * of the authors and should not be interpreted as representing official
>> policies,
>> * either expressed or implied, of Tresys Technology, LLC.
>> @@ -259,7 +259,7 @@ int main(int argc, char *argv[])
>> fprintf(stderr, "Could not stat file: %s\n",
>> argv[i]);
>> goto exit;
>> }
>> - file_size = filedata.st_size;
>> + file_size = filedata.st_size;
>>
>> buffer = malloc(file_size);
>> rc = fread(buffer, file_size, 1, file);
>> @@ -347,7 +347,7 @@ int main(int argc, char *argv[])
>> fprintf(stderr, "Failed to open file_contexts file\n");
>> goto exit;
>> }
>> -
>> +
>> if (fwrite(fc_buf, sizeof(char), fc_size, file_contexts) !=
>> fc_size) {
>> fprintf(stderr, "Failed to write file_contexts file\n");
>> goto exit;
>> --
>> 2.19.0.444.g18242da7ef-goog
>>
>> ___
>> 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: [RFC PATCH] selinux: add a fallback to defcontext for native labeling
On 09/25/2018 01:45 AM, Taras Kondratiuk wrote: Quoting Paul Moore (2018-09-24 20:46:57) On Fri, Sep 21, 2018 at 10:39 AM Stephen Smalley wrote: On 09/20/2018 06:59 PM, Taras Kondratiuk wrote: Quoting Stephen Smalley (2018-09-20 07:49:12) On 09/19/2018 10:41 PM, Taras Kondratiuk wrote: Quoting Stephen Smalley (2018-09-19 12:00:33) On 09/19/2018 12:52 PM, Taras Kondratiuk wrote: ... IMO it would be more consistent if defcontext cover all "unlabeled" groups. It seems unlikely to me that somebody who currently uses defcontext can somehow rely on mapping invalid labels to unlabeled instead of default context. Yes, and that seems more consistent with the current documentation in the mount man page for defcontext=. I'd be inclined to change selinux_inode_notifysecctx() to call security_context_to_sid_default() directly instead of using selinux_inode_setsecurity() and change security_context_to_sid_core() and sidtab_search_core() as suggested above to save and use the def_sid instead of SECINITSID_UNLABELED always (initializing the context def_sid to SECINITSID_UNLABELED as the default). selinux_inode_setsecurity() we should leave unchanged, or if we change it at all, it should be more like the handling in selinux_inode_setxattr(). The notifysecctx hook is invoked by the filesystem to notify the security module of the file's existing security context, so in that case we always want the _default behavior, whereas the setsecurity hook is invoked by the vfs or the filesystem to set the security context of a file to a new value, so in that case we would only use the _force interface if the caller had CAP_MAC_ADMIN. Paul, what say you? NB This would be a user-visible behavior change for mounts specifying defcontext= on xattr filesystems; files with invalid contexts will then show up with the defcontext value instead of the unlabeled context. If that's too risky, then we'd need a flag or something to security_context_to_sid_default() to distinguish the behaviors and only set it when called from selinux_inode_notifysecctx(). Visible changes like this are always worrisome, even though I think it is safe to assume that the defcontext option is not widely used. I'd feel much better if this change was opt-in. Which brings about it's own problems. We have the policy capability functionality, but that is likely a poor fit for this as the policy capabilities are usually controlled by the Linux distribution while the mount options are set by the system's administrator when the filesystem is mounted. We could add a toggle somewhere in selinuxfs, but I really dislike that idea, and would prefer to find a different solution if possible. I'm not sure how much flak we would get for introducing a new mount option, but perhaps that is the best way to handle this: defcontext would continue to behave as it does now, but new option X would behave as mentioned in this thread. Thoughts? The new option X will also mean "default context", so it will be pretty hard to come up with a different but a sensible name. I'm afraid that having two options with hardly distinguishable semantics will be very confusing. What about a kernel config option that modifies the behavior of defcontext? First, the existing documentation for defcontext= leaves open the door to the proposed new behavior. From mount(8): "You can set the default security context for unlabeled files using defcontext= option. This overrides the value set for unlabeled files in the policy and requires a filesystem that supports xattr labeling." "Unlabeled files" could just mean files without any xattr, or it could mean all files that either lack an xattr or have an invalid one under the policy, since both sets of files are currently mapped to the unlabeled context. Second, under what conditions would this situation break existing userspace? The admin would have had to mount an xattr filesystem with defcontext= specified, and that filesystem would have to both contain files without any xattrs and files with invalid ones (otherwise how they are treated by the kernel is irrelevant), and the policy would need to distinguish access to files without xattrs vs files with invalid ones. Seems unlikely. Third, the fact that policy maintainers remapped both SECINITSID_FILE (the default value for defcontext) and SECINITSID_UNLABELED (used for invalid contexts) to the unlabeled context (getting rid of file_t as a separate type, aliased to unlabeled_t) long ago suggests that there is no meaningful difference there. I'm inclined to just change the behavior for defcontext= unconditionally and have it apply to both native and xattr labeling. If that's a no-go, then the simplest solution is to just leave defcontext= behavior unchanged for xattr labeling and only implement the new semantics for native labeling. That's just a matter of adding a flag to security_context_to_sid_default() and only setting it when calling from
Re: autorelabel loops in system executed 'semodule -d unconfined'
On Tue, Sep 25, 2018 at 07:19:23AM +0900, Shintaro Fujiwara wrote: > Hi, SELinux. > > I captured a picture saying this. > > rm: cannot remove '/.autorelabel' : Permission denied > > /.autorelabel could not be removed, so going into the loop, I guess. > > How can I autorelabel properly even if I delete unconfined module? This may or may not be a policy issue (see avc denials), but: Generally you want to do a full relabel in permissive mode. > > Thanks. > 2018年9月25日(火) 6:55 Shintaro Fujiwara : > > > > Hello, SELinux. > > > > I was playing with my F28 latest with 'semodle -d unconfined'. > > I executed this and relabeling starts even after finished relebeling > > and looks like going into the loop. > > # touch /.autorelabel > > # shtudown -r now > > > > I have attached a picure. > > > > Thanks. > > > > -- > Help analyzing sar file > https://github.com/intrajp/sar-analyzer > > LFS Scripts will make Linux From Scratch easy > https://github.com/intrajp/LFS-scripts-systemd > > SHIRASAGI-hardening Project > https://github.com/intrajp/shirasagi-hardening > > Linux Distribution Project > http://sourceforge.net/projects/pinkrabbitlinux/ > > Introducing hardrock and heavymetal > http://heavymetalhardrock.no-ip.info/ > > Open Source Software to manage SELinux at ease > http://sourceforge.net/projects/segatex/ > > Help SELinux administration > https://github.com/intrajp/segatex-ng > > network-magic ( Useful tool for network-administrators ) > https://github.com/intrajp/network-magic > > CMS（with PHP & PostgreSQL） > http://sourceforge.net/projects/webon/ > https://github.com/intrajp/irforum_jp > ___ > 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 security-next v3 07/29] LSM: Convert security_initcall() into DEFINE_LSM()
Instead of using argument-based initializers, switch to defining the
contents of struct lsm_info on a per-LSM basis. This also drops
the final use of the now inaccurate "initcall" naming.
Cc: John Johansen
Cc: James Morris
Cc: "Serge E. Hallyn"
Cc: Paul Moore
Cc: Stephen Smalley
Cc: Casey Schaufler
Cc: Tetsuo Handa
Cc: Mimi Zohar
Cc: linux-security-mod...@vger.kernel.org
Cc: selinux@tycho.nsa.gov
Signed-off-by: Kees Cook
---
include/linux/lsm_hooks.h | 6 --
security/apparmor/lsm.c| 4 +++-
security/integrity/iint.c | 4 +++-
security/selinux/hooks.c | 4 +++-
security/smack/smack_lsm.c | 4 +++-
security/tomoyo/tomoyo.c | 4 +++-
6 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index ad04761e5587..02ec717189f9 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2045,11 +2045,13 @@ struct lsm_info {
extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
-#define security_initcall(lsm) \
+#define DEFINE_LSM(lsm)
\
static struct lsm_info __lsm_##lsm \
__used __section(.lsm_info.init)\
__aligned(sizeof(unsigned long))\
- = { .init = lsm, }
+ = { \
+
+#define END_LSM }
#ifdef CONFIG_SECURITY_SELINUX_DISABLE
/*
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 8b8b70620bbe..7fa7b4464cf4 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1606,4 +1606,6 @@ static int __init apparmor_init(void)
return error;
}
-security_initcall(apparmor_init);
+DEFINE_LSM(apparmor)
+ .init = apparmor_init,
+END_LSM;
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 70d21b566955..20e60df929a3 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -175,7 +175,9 @@ static int __init integrity_iintcache_init(void)
0, SLAB_PANIC, init_once);
return 0;
}
-security_initcall(integrity_iintcache_init);
+DEFINE_LSM(integrity)
+ .init = integrity_iintcache_init,
+END_LSM;
/*
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad9a9b8e9979..469a90806bc6 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -7202,7 +7202,9 @@ void selinux_complete_init(void)
/* SELinux requires early initialization in order to label
all processes and objects when they are created. */
-security_initcall(selinux_init);
+DEFINE_LSM(selinux)
+ .init = selinux_init,
+END_LSM;
#if defined(CONFIG_NETFILTER)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 340fc30ad85d..1e1ace718e75 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4882,4 +4882,6 @@ static __init int smack_init(void)
* Smack requires early initialization in order to label
* all processes and objects when they are created.
*/
-security_initcall(smack_init);
+DEFINE_LSM(smack)
+ .init = smack_init,
+END_LSM;
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index 9f932e2d6852..a280d4eab456 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -550,4 +550,6 @@ static int __init tomoyo_init(void)
return 0;
}
-security_initcall(tomoyo_init);
+DEFINE_LSM(tomoyo)
+ .init = tomoyo_init,
+END_LSM;
--
2.17.1
___
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: [RFC PATCH] selinux: add a fallback to defcontext for native labeling
Quoting Stephen Smalley (2018-09-21 07:40:58)
> On 09/20/2018 06:59 PM, Taras Kondratiuk wrote:
> > Quoting Stephen Smalley (2018-09-20 07:49:12)
> >> On 09/19/2018 10:41 PM, Taras Kondratiuk wrote:
> >>> Quoting Stephen Smalley (2018-09-19 12:00:33)
> On 09/19/2018 12:52 PM, Taras Kondratiuk wrote:
> > When files on NFSv4 server are not properly labeled (label doesn't match
> > a policy on a client) they will end up with unlabeled_t type which is
> > too generic. We would like to be able to set a default context per
> > mount. 'defcontext' mount option looks like a nice solution, but it
> > doesn't seem to be fully implemented for native labeling. Default
> > context is stored, but is never used.
> >
> > The patch adds a fallback to a default context if a received context is
> > invalid. If the inode context is already initialized, then it is left
> > untouched to preserve a context set locally on a client.
>
> Can you explain the use case further? Why are you exporting a
> filesystem with security labeling enabled to a client that doesn't
> understand all of the labels used within it? Why wouldn't you just
> disable NFSv4 security labeling and/or use a regular context= mount to
> assign a single context to all files in the mount?
> >>>
> >>> Client and server are two embedded devices. They are part of a bigger
> >>> modular system and normally run the same SW version, so they understand
> >>> each others NFSv4 SELinux labels. But during migration from one SW
> >>> version to another a client and a server may run different versions for
> >>> some time.
> >>>
> >>> The immediate issue I'm trying to address is a migration between
> >>> releases with and without SELinux policy. A client (with policy) gets
> >>> initial SID labels from a server (without policy). Those labels are
> >>> considered invalid, so files remain unlabeled. This also causes lots of
> >>> errors from nfs_setsecurity() in dmesg.
> >>
> >> Why are you enabling SELinux on the server without loading a policy?
> >> Are you concerned about denials on the server? For that, you could
> >> always run it permissive until you are confident you have a working policy.
> >>
> >> Why are you enabling security_label in exports before you have a policy
> >> loaded? It doesn't appear that will ever work, since nfsd asks the
> >> security module for the labels, not the local filesystem directly.
> >>
> >> I understand that this doesn't fully address your use case, but it seems
> >> like you could avoid this particular situation altogether just by
> >> loading a policy (at which point your server can return actual contexts)
> >> or by removing security_label from your exports (at which point your
> >> server won't try returning contexts and the client will just apply the
> >> default for nfs) until you get to the point of loading a policy.
> >
> > It wasn't intentional configuration. Server is running v4.4.x kernel
> > that had security labels enabled by default for NFS 4.2. This was a bug:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1406885
> > Commit that introduced security_label 32ddd944a056 ("nfsd: opt in to
> > labeled nfs per export") appeared in v4.11 only.
> >
> > We hit this bug during migration to newer releases with SELinux, but the
> > older release is already in the field and we need to handle it.
>
> Ok, I understand the kernel bug, but not why your servers are in a state
> where SELinux is enabled but no policy is loaded. That is not a
> well-tested code path aside from early system initialization prior to
> first policy load by systemd/init. You would be better off either
> disabling SELinux on the servers entirely (thereby automatically
> disabling NFSv4.2 security labeling) or installing/loading a policy on
> the servers (thereby enabling them to produce valid security labels for
> the client at least to the extent that they agree on policy). If you
> are concerned about denials on the server, then you could always leave
> the servers permissive initially and collect audit logs until you are
> confident your policy is adequate.
It wasn't intentional either. The same kernel configuration is used
accross several products. We roll out SELinux for products one by one,
so some products don't have SELinux policy and /etc/selinux/config yet
while SELinux kernel config is enabled. In hindsight we should have
explicitly disabled SELinux in /etc/selinux/config for those products.
Since it is not a well-tested code I'm wondering shouldn't systemd/init
be disabling SELinux if config file or policy is missing?
>
> >>
> >>>
>
> To be clear, defcontext= doesn't work that way for local/FS_USE_XATTR
> filesystems. The context specified by it is only used for:
> 1) files that don't implement the xattr inode operations at all,
> 2) files that lack a security.selinux xattr,
> 3) the MLS portion of the context if it was missing
Re: [RFC PATCH] selinux: add a fallback to defcontext for native labeling
Quoting Paul Moore (2018-09-24 20:46:57) > On Fri, Sep 21, 2018 at 10:39 AM Stephen Smalley wrote: > > On 09/20/2018 06:59 PM, Taras Kondratiuk wrote: > > > Quoting Stephen Smalley (2018-09-20 07:49:12) > > >> On 09/19/2018 10:41 PM, Taras Kondratiuk wrote: > > >>> Quoting Stephen Smalley (2018-09-19 12:00:33) > > On 09/19/2018 12:52 PM, Taras Kondratiuk wrote: > > ... > > > > IMO it would be more consistent if defcontext cover all "unlabeled" > > > groups. It seems unlikely to me that somebody who currently uses > > > defcontext can somehow rely on mapping invalid labels to unlabeled > > > instead of default context. > > > > Yes, and that seems more consistent with the current documentation in > > the mount man page for defcontext=. > > > > I'd be inclined to change selinux_inode_notifysecctx() to call > > security_context_to_sid_default() directly instead of using > > selinux_inode_setsecurity() and change security_context_to_sid_core() > > and sidtab_search_core() as suggested above to save and use the def_sid > > instead of SECINITSID_UNLABELED always (initializing the context def_sid > > to SECINITSID_UNLABELED as the default). selinux_inode_setsecurity() we > > should leave unchanged, or if we change it at all, it should be more > > like the handling in selinux_inode_setxattr(). The notifysecctx hook is > > invoked by the filesystem to notify the security module of the file's > > existing security context, so in that case we always want the _default > > behavior, whereas the setsecurity hook is invoked by the vfs or the > > filesystem to set the security context of a file to a new value, so in > > that case we would only use the _force interface if the caller had > > CAP_MAC_ADMIN. > > > > Paul, what say you? NB This would be a user-visible behavior change for > > mounts specifying defcontext= on xattr filesystems; files with invalid > > contexts will then show up with the defcontext value instead of the > > unlabeled context. If that's too risky, then we'd need a flag or > > something to security_context_to_sid_default() to distinguish the > > behaviors and only set it when called from selinux_inode_notifysecctx(). > > Visible changes like this are always worrisome, even though I think it > is safe to assume that the defcontext option is not widely used. I'd > feel much better if this change was opt-in. > > Which brings about it's own problems. We have the policy capability > functionality, but that is likely a poor fit for this as the policy > capabilities are usually controlled by the Linux distribution while > the mount options are set by the system's administrator when the > filesystem is mounted. We could add a toggle somewhere in selinuxfs, > but I really dislike that idea, and would prefer to find a different > solution if possible. I'm not sure how much flak we would get for > introducing a new mount option, but perhaps that is the best way to > handle this: defcontext would continue to behave as it does now, but > new option X would behave as mentioned in this thread. > > Thoughts? The new option X will also mean "default context", so it will be pretty hard to come up with a different but a sensible name. I'm afraid that having two options with hardly distinguishable semantics will be very confusing. What about a kernel config option that modifies the behavior of defcontext? ___ 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 v4 00/19] LSM: Module stacking for SARA and Landlock
On 9/24/2018 10:53 AM, Tetsuo Handa wrote:
> On 2018/09/25 2:16, Casey Schaufler wrote:
>>> Not all of LKM-based LSMs use security blobs. And some of LKM-based LSMs
>>> might use security blobs for only a few objects. For example, AKARI uses
>>> inode security blob for remembering whether source address/port of an
>>> accept()ed socket was already checked, only during accept() operation and
>>> first socket operation on the accept()ed socket. Thus, there is no need
>>> to waste memory by assigning blobs for all inode objects.
>> The first question is why use an inode blob? Shouldn't you
>> be using a socket blob for this socket based information?
> Indeed. AKARI can as well use security_sk_free() using address of
> "struct sock" as a key.
>
>> If you only want information part of the time you can declare
>> a pointer sized blob and manage what hangs off that as you will.
>> I personally think that the added complexity of conditional
>> blob management is more pain than it's worth, but if you want
>> a really big blob, but only on occasion, I could see doing it.
> LKM based LSMs are too late for updating blob_sizes.* fields.
That is true with the code in this patch set. As I mentioned,
changing the blob handling to include a header with real use
information would be required.
> Even if they could, they after all have to somehow check whether
> corresponding init hook was called. That's checking for NULL.
Right.
> @@ -1202,11 +1183,11 @@ void security_file_free(struct file *file)
> {
> void *blob;
>
> + call_void_hook(file_free_security, file);
> +
> if (!lsm_file_cache)
> return;
>
> - call_void_hook(file_free_security, file);
> -
Why does this make sense? If the lsm_file_cache isn't
initialized you can't have allocated any file blobs,
no module can have initialized a file blob, hence there
can be nothing for the module to do.
>>> For modules (not limited to LKM-based LSMs) which want to use
>>> file blobs for only a few objects and avoid wasting memory by
>>> allocating file blobs to all file objects.
>>>
>>> Infrastructure based blob management fits well for LSM modules
>>> which want to assign blobs to all objects (like SELinux). But
>>> forcing infrastructure based blob management can become a huge
>>> waste of memory for LSM modules which want to assign blobs to
>>> only a few objects. Unconditionally calling file_free_security
>>> hook (as with other hooks) preserves a room for allowing the
>>> latter type of LSM modules without using infrastructure based
>>> blob management.
>> There is a hypothetical issue here, but that would require abuse
>> of the infrastructure. Having a file_free_security hook that doesn't
>> free a security blob allocated by file_alloc_security may coincidentaly
>> be useful, but that's not the intent of the hook.
>>
> The free hook might be used for freeing resources which were not allocated
> by alloc hook. Yama is using task_free hook without task_alloc hook.
> Someone might want to use file_free hook without file_alloc hook.
OK, you're correct. Checking for an initialized kmem_cache isn't appropriate.
___
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.
