Re: [systemd-devel] Supervisory Watchdog notification not working when using SmackProcessLabel
On 8/1/2018 3:18 AM, Martin Townsend wrote: > Hi, > > I have a service running with a SmackProcessLabel that uses the > supervisory watchdog feature, ie calls sd_notify(). The Watchdog > keeps resetting the service and I get the following in the journal > > Jul 27 11:36:11 kernel: audit: type=1400 audit(1532691371.270:34): > lsm=SMACK fn=smack_unix_may_send action=denied subject="apphealthd" > object="_" requested=w pid=466 comm="apphealthd" > path="/run/systemd/notify" > > /run/systemd/notify is a socket so I'm guessing sd_notify kicks the > watchdog by writing to this socket. The problem seems to be that the > socket is labelled with the floor label. What does "attr -S -g SMACK64 /run/systemd/notify" report? It should say that the attribute value is "*". If it doesn't, what kernel and systemd versions are you running? What is your base system? (Tizen, Fedora, ...) UDS socket access controls are done based on the processes involved, not the file system object. Do you have any services running with the floor ("_") label? > After looking through the code that sets up the notify socket I > quickly patched in some code to set SMACK64IPIN and IPOUT (not sure if > this one is required). > > @@ -728,7 +729,12 @@ static int manager_setup_notify(Manager *m) { > > m->notify_fd = fd; > fd = -1; > - > +r = mac_smack_apply_fd(m->notify_fd, SMACK_ATTR_IPIN, "*"); > +if (r < 0) > +log_error_errno(r, "mac_smack_apply_ip_in_fd: %m"); > +r = mac_smack_apply_fd(m->notify_fd, SMACK_ATTR_IPOUT, "@"); > +if (r < 0) > +log_error_errno(r, "mac_smack_apply_ip_out_fd: %m"); > log_debug("Using notification socket %s", m->notify_socket); > } > > And the audit message has gone. Your code above is the best way to ensure your service can talk to everyone. You have to be sure talking to everyone is really what you want to do. > Is there a better way of ensuring /run/systemd/notify can be accessed > by a service with a User defined SMACK label? or is this patch to > manager_setup_notify sufficient? You can always add access rules to allow communications. In this case it would be apphealthd _ w _ apphealthd w Generally you don't want to add rules for the floor ("_") label, but that's where you're seeing an issue. If you compile with SECURITY_SMACK_BRINGUP and use apphealthd _ wb _ apphealthd wb accesses permitted by these rules will be logged so that you can track down who you're talking to. > > Many Thanks in Advance, > Martin. > ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
On 11/9/2014 5:56 AM, WaLyong Cho wrote: On 11/08/2014 01:36 AM, Lennart Poettering wrote: On Fri, 07.11.14 15:43, WaLyong Cho (walyong@samsung.com) wrote: On 11/07/2014 09:35 AM, Lennart Poettering wrote: On Fri, 07.11.14 04:17, WaLyong Cho (walyong@gmail.com) wrote: SMACK64 Used to make access control decisions. In almost all cases the label given to a new filesystem object will be the label of the process that created it. SMACK64EXEC The Smack label of a process that execs a program file with this attribute set will run with this attribute's value. I am sorry, but I cannot parse this. The smack label will run with this attribute's value? smack labels run? That sentence makes no sense to me at all... Again, what kind of objects are SMACK64 and SMACK64EXEC applied to? files? processes? Sorry, I'm also confused. OK, I asked some about this to my security friend. And I add Casey Schaufler who the Author of smack. I hope my below explain it not wrong. But if not, please point out. Quoting the Wikipedia: In practice, a subject is usually a process or thread; objects are constructs such as files, directories, TCP/UDP ports, shared memory segments, IO devices etc. In case of SMACK, subject is SMACK64EXEC and object is SMACK64. Assume like this: /usr/bin/dbus-daemon has both label. SMACK64 is foo and SMACK64EXEC is bar. And current process (what will execute the /usr/bin/dbus-daemon) has foo label. Let's assume the current process So, here you are talking about *files* that have the SMACK64EXEC and SMACK64 type labels, while the *process* only as one generic label type. With your patch you want to introduce SmackLabelExec= now. It's a label applied to a *process* however, not to a *file*, right? This appears incompatible to me: I mean, if a process only has one single generic label, and doesn't distuingish between SMACK64 and SMACK64EXEC type labels, why would you call the option SmackLabelExec= and not the more generic SmackLabel=? Previous mail just a explaination for the difference SMACK64 and SMACK64EXEC. OK, well I think you also understood well about that. Now time to explain my patch. There was execution problems with User= option. Precondition in systemd: 1. systemd is running as root uid. 2. systemd has _ SMACK attribute. On Tizen 3 we set the Smack label of systemd to System. Precondition in SMACK side: 1. SMACK access permission checking is by-passed for root uid processes. Yes. That's the Linux privilege model. Problem case: 1. a system session systemd service has User= option. 2. The file on ExecStart= has some special access label and that can NOT be accessed by _ label. That's a configuration error. Set the SMACK64 attribute to _ on the binary. Is there a security reason to hide the binary? If you set the Smack label for the service before you set the UID Bob's your uncle. This isn't a problem if you think about the order you do things. The problem occurred procedure: 1. systemd(pid 1) is running as root uid. 2. fork()-ed child systemd still has root uid. (be fork()-ed in exec_spawn()) 3. fork()-ed child systemd process do permission appying. (in the exec_child()) 4. If the target service has User= option the child systemd process will call enforce_user(). 5. After exit of enforce_user() the child systemd no more root uid. 6. Finally, the child systemd process try to execve the given process. 7. (At my case:) the execution was failed because of SMACK access error. This is one very good reason to specify the Smack label of the service in the systemd configuration rather than using the SMACK64EXEC mechanism. If you let systemd do the work, like it does for UIDs and capabilities, this works seamlessly. Cause: The fork()-ed child systemd process has _ SMACK label what same with parent. And after enforce_user() the child has no more root uid. So that can not be by-passed. (As I wrote as above problem case:) The given process file has some special SMACK label and that can NOT be access-ed by _ attribute. So filnally the child systemd process can NOT access the given ExecStart= file. To resolve this, I tought two method. The first is what I sent patch. Introduce new SMACK option and the given label by new option is applied to child systemd process. Please, do *NOT* confuse. The given label is not applied to will be executed process on ExecStart. Just be applied to child systemd process to successfully access the file on ExecStart=. (The reason why I named the option name as SmackLabelExec, the given label by the option, is applied to child systemd. That is not file. That is currenlty running process. Not other reason.) This is unnecessarily complicated. Just have systemd set the label you want the service to run with and be done with it. The second. This method do *NOT* need any new option. Before systemd is calling enforce_user(), systemd still has root uid and can read
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
On 11/6/2014 10:43 PM, WaLyong Cho wrote: On 11/07/2014 09:35 AM, Lennart Poettering wrote: On Fri, 07.11.14 04:17, WaLyong Cho (walyong@gmail.com) wrote: SMACK64 Used to make access control decisions. In almost all cases the label given to a new filesystem object will be the label of the process that created it. SMACK64EXEC The Smack label of a process that execs a program file with this attribute set will run with this attribute's value. I am sorry, but I cannot parse this. The smack label will run with this attribute's value? smack labels run? That sentence makes no sense to me at all... Again, what kind of objects are SMACK64 and SMACK64EXEC applied to? files? processes? Sorry, I'm also confused. OK, I asked some about this to my security friend. And I add Casey Schaufler who the Author of smack. I hope my below explain it not wrong. But if not, please point out. Quoting the Wikipedia: In practice, a subject is usually a process or thread; objects are constructs such as files, directories, TCP/UDP ports, shared memory segments, IO devices etc. In case of SMACK, subject is SMACK64EXEC and object is SMACK64. The SMACK64 attribute of any object (file, directory, IPC object, ...) is used in access control decisions on that object. The SMACK64EXEC attribute is very much like the setuid bit on an executable. If the attribute is set (and it should almost *never* be set) on a program file the process execing the program will change its Smack label to the value in the SMACK64EXEC attribute. On execution: The existing process Smack label must allow eXec access to the program file, based on the Smack label of the process and the SMACK64 attribute of the program file. If there is no SMACK64EXEC attribute on the file the Smack label of the process is unchanged. If there is a SMACK64EXEC attribute the Smack label of the process changes to that value. Note that the SMACK64 attribute is present on all files and has no impact on the Smack label of the process. The SMACK63EXEC attribute is optional. Most program files do not have this attribute. It should be only used in cases where the Smack label of the program must always be a particular value. Thus, dbus is a fine example where SMACK64EXEC is a bad idea. Because you want a system bus and a user bus with different attributes you want it to get the Smack label at launch time, just like you do for UID and capability sets. Assume like this: /usr/bin/dbus-daemon has both label. SMACK64 is foo and SMACK64EXEC is bar. And current process (what will execute the /usr/bin/dbus-daemon) has foo label. Let's assume the current process is sh. Then /proc/$$/attr/current of sh is foo. There is no problems to sh execute the /usr/bin/dbus-daemon because the sh has same label with /usr/bin/dbus-daemon. Now /usr/bin/dbus-daemon will be executed by sh. But it has SMACK64EXEC label bar. So executed attr/current of /usr/bin/dbus-daemon is bar. If the sh has label waldo, then maybe two case can be existed. If label waldo has executable rule for label foo then the sh can execute the /usr/bin/dbus-daemon. But if label waldo has no rule for that, then sh can not execute the /usr/bin/dbus-daemon. EACCESS will be occurred. If label waldo has executable rule for label foo and /usr/bin/dbus-daemon was executed. Then the executed dbus-daemon process has attribute bar at /proc/{pid of dbus-daemon}/attr/current. # attr -l /bin/sleep Attribute SMACK64 has a 4 byte value for /bin/sleep Attribute SMACK64EXEC has a 3 byte value for /bin/sleep # attr -S -g SMACK64 /bin/sleep Attribute SMACK64 had a 4 byte value for /bin/sleep: foo # attr -S -g SMACK64EXEC /bin/sleep Attribute SMACK64EXEC had a 3 byte value for /bin/sleep: bar # cat /proc/$$/attr/current waldo # /bin/sleep 100 [1] 15263 sh-3.2# cat /proc/15263/attr/current bar Additionally, SMACK64EXEC can be none. Then the executed process inherit attribute from the caller process. So sh has attribute waldo and /bin/sleep has only SMACK64 foo then the executed /bin/sleep process has waldo attribute. # cat /proc/$$/attr/current waldo # attr -l /bin/sleep Attribute SMACK64 has a 4 byte value for /bin/sleep # attr -S -g SMACK64 /bin/sleep Attribute SMACK64 had a 4 byte value for /bin/sleep: foo # /bin/sleep 100 [1] 4509 # cat /proc/4509/attr/current waldo Back to the systemd execute problem with User= option. Exclude Netlabel, the access checking is ignored for all root uid processes. So there is no problems to execute the ExecStart= without User= option. But if a service has User= option and executable binary on ExecStart= has label foo then the fork()-ed systemd child process has no root uid. And the child systemd process has _ label.(see below predefined labels.) If _ has no executable rule for foo then access deny will be occurred. So to successfully execute the ExecStart=, the child systemd process have to has attribute
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
On 11/7/2014 8:36 AM, Lennart Poettering wrote: On Fri, 07.11.14 15:43, WaLyong Cho (walyong@samsung.com) wrote: On 11/07/2014 09:35 AM, Lennart Poettering wrote: On Fri, 07.11.14 04:17, WaLyong Cho (walyong@gmail.com) wrote: SMACK64 Used to make access control decisions. In almost all cases the label given to a new filesystem object will be the label of the process that created it. SMACK64EXEC The Smack label of a process that execs a program file with this attribute set will run with this attribute's value. I am sorry, but I cannot parse this. The smack label will run with this attribute's value? smack labels run? That sentence makes no sense to me at all... Again, what kind of objects are SMACK64 and SMACK64EXEC applied to? files? processes? Sorry, I'm also confused. OK, I asked some about this to my security friend. And I add Casey Schaufler who the Author of smack. I hope my below explain it not wrong. But if not, please point out. Quoting the Wikipedia: In practice, a subject is usually a process or thread; objects are constructs such as files, directories, TCP/UDP ports, shared memory segments, IO devices etc. In case of SMACK, subject is SMACK64EXEC and object is SMACK64. Assume like this: /usr/bin/dbus-daemon has both label. SMACK64 is foo and SMACK64EXEC is bar. And current process (what will execute the /usr/bin/dbus-daemon) has foo label. Let's assume the current process So, here you are talking about *files* that have the SMACK64EXEC and SMACK64 type labels, while the *process* only as one generic label type. With your patch you want to introduce SmackLabelExec= now. It's a label applied to a *process* however, not to a *file*, right? Yes, it would apply to the process, not the file. We're talking about the Smack attribute on the process, not a SMACK64EXEC attribute on the program file. This appears incompatible to me: I mean, if a process only has one single generic label, and doesn't distuingish between SMACK64 and SMACK64EXEC type labels, why would you call the option SmackLabelExec= and not the more generic SmackLabel=? It seemed like a good idea at the time. This really doesn't make sense to me. I have no understanding of SMACK, and I am not sure I want to understand it, and I figure I'd merge the patch regardless which name you pick for the option, but this is a bit too blatantly contradictory for me to completely ignore. Let my simplify my questions: a) Why would you call a setting that controls a label is written into /proc/$PID/attr/current SmackLabelExec= instead of just SmackLabel=? b) If SmackLabelExec= is appropriate (which it might well be, after all I really don't grok this), and SmackLabel= is a misnomer that would suggest that something different would happen than actually assumed, then what would an option by the name SmackLabel= for a service unit do differntly from one called SmackLabelExec? For both SELinux and AppArmor we now have simple options: SELinuxContext= and AppArmorProfile=. They only come in one flavour, and apply a label/profile to the process being executed and that's it. Why would SMACK be more complicated there so that SmackLabel= and SmackLabelExec= would even be a distinction? Calling it SmackLabel= instead of SmackLabelExec= would be fine as far as I'm concerned. SmackLabel= is more consistent with SELinuxContext= and AppArmorProfile=, as you point out. Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] SELINUX: add /sys/fs/selinux mount point to put selinuxfs
On 5/11/2011 7:43 AM, Greg KH wrote: On Wed, May 11, 2011 at 04:27:59PM +0200, Kay Sievers wrote: On Wed, May 11, 2011 at 15:54, Greg KH g...@kroah.com wrote: On Wed, May 11, 2011 at 01:22:42PM +0200, John Johansen wrote: On 05/11/2011 03:59 AM, Greg KH wrote: On Tue, May 10, 2011 at 03:55:24PM -0700, Casey Schaufler wrote: On 5/10/2011 3:34 PM, Greg KH wrote: From: Greg Kroah-Hartman gre...@suse.de In the interest of keeping userspace from having to create new root filesystems all the time, let's follow the lead of the other in-kernel filesystems and provide a proper mount point for it in sysfs. For selinuxfs, this mount point should be in /sys/fs/selinux/ It seems that we might want this to be an LSM interface standard. Is the call to kobject_create_and_add and associated cleanup all that's required? I would want Smack to follow the convention as well. You could always just create a subdir under /sys/security/ if you have your own filesystem, but I don't think that Smack has one, right? Is it going to get one? If so, we might want to revisit the idea of securityfs if no one is actually using it... resending, as this looks to have been lost AppArmor, IMA, and TOMOYO are using securityfs currently. Great, then it will not go anywhere. Just to get an idea how all this fits together. How can TPM bios and IMA/AppArmor share this directory? They have their own subdirs in there, or both just use the securityfs infrastructure and not their own filesystem on top? Only one security module is allowed to be loaded/active at any one point in time, so they can't step on each other. This is true today, but I seriously think we're going to break down this barrier before long. I see this as a significant reason to sort the location of LSM control filesystems. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] SELINUX: add /sys/fs/selinux mount point to put selinuxfs
On 5/11/2011 7:58 AM, Eric Paris wrote: On Wed, May 11, 2011 at 10:54 AM, John Johansen john.johan...@canonical.com wrote: On 05/11/2011 04:52 PM, Kay Sievers wrote: On Wed, May 11, 2011 at 16:43, Greg KH g...@kroah.com wrote: On Wed, May 11, 2011 at 04:27:59PM +0200, Kay Sievers wrote: On Wed, May 11, 2011 at 15:54, Greg KH g...@kroah.com wrote: On Wed, May 11, 2011 at 01:22:42PM +0200, John Johansen wrote: On 05/11/2011 03:59 AM, Greg KH wrote: On Tue, May 10, 2011 at 03:55:24PM -0700, Casey Schaufler wrote: On 5/10/2011 3:34 PM, Greg KH wrote: From: Greg Kroah-Hartman gre...@suse.de In the interest of keeping userspace from having to create new root filesystems all the time, let's follow the lead of the other in-kernel filesystems and provide a proper mount point for it in sysfs. For selinuxfs, this mount point should be in /sys/fs/selinux/ It seems that we might want this to be an LSM interface standard. Is the call to kobject_create_and_add and associated cleanup all that's required? I would want Smack to follow the convention as well. You could always just create a subdir under /sys/security/ if you have your own filesystem, but I don't think that Smack has one, right? Is it going to get one? If so, we might want to revisit the idea of securityfs if no one is actually using it... resending, as this looks to have been lost AppArmor, IMA, and TOMOYO are using securityfs currently. Great, then it will not go anywhere. Just to get an idea how all this fits together. How can TPM bios and IMA/AppArmor share this directory? They have their own subdirs in there, or both just use the securityfs infrastructure and not their own filesystem on top? Only one security module is allowed to be loaded/active at any one point in time, so they can't step on each other. Right, but what I don't understand is CONFIG_TCG_TPM, which seem to use securityfs, and is not a LSM. This and AppArmor/IMA can be used at the same time, can't it? They share securityfs then? AppArmor, Tomoyo and IMA all create their own subdirectoy under securityfs so this should not be a problem I guess the question is, should SELinux try to move to /sys/fs/selinux or /sys/security/selinux. The only minor issue I see with the later is that it requires both sysfs and securityfs to be mounted before you can mount selinuxfs, whereas the first only requires sysfs. Stephen, Casey, either of you have thoughts on the matter? I would prefer /sys/security for all LSMs, but if SELinux goes with /sys/fs Smack will likely follow on the theory that mirroring the current dominant LSM is more likely to please the masses than doing what the greatest number of LSMs are doing. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] SELINUX: add /sys/fs/selinux mount point to put selinuxfs
On 5/11/2011 1:40 PM, Eric Paris wrote: On Wed, May 11, 2011 at 4:22 PM, Greg KH g...@kroah.com wrote: On Wed, May 11, 2011 at 04:14:32PM -0400, Eric Paris wrote: On Wed, May 11, 2011 at 3:56 PM, Greg KH g...@kroah.com wrote: On Wed, May 11, 2011 at 10:14:40AM -0700, Casey Schaufler wrote: I would prefer /sys/security for all LSMs, but if SELinux goes with /sys/fs Smack will likely follow on the theory that mirroring the current dominant LSM is more likely to please the masses than doing what the greatest number of LSMs are doing. Is smack going to create its own filesystem like selinux has, or is it going to use securityfs? If securityfs, then stick with what you have. If you are going to create a new one, I'd be glad to work with you to add anything you might need to securityfs first, but if that doesn't work out, then yes, you could use /sys/fs/ for your new one. Pretty sure we already have a securty/smack/smackfs.c . Doh, you are right. Ok, I'll just shutup now then... greg k-h I'm willing to put in a day or two trying to move selinux to securityfs, but there is one thing I'm not sure how to handle, mainly when it comes to userspace backwards compat. Greg, maybe you can give me ideas. My biggest issue is how libselinux historically found selinuxfs. Given that people will likely use this library forever even with new kernels (*cough* akpm *cough*) I sorta feel like we need to continue to support it. The libselinux library had 3 tests. 1) check statfs(2) on /selinux and if it was selinuxfs magic we are done 2) check /proc/filesystems to see if selinuxfs exists, if not, selinux is disabled 3) check /proc/mounts and find if/where selinuxfs is mounted If we just move the selinuxfs mount around the filesystem it's no big deal. The old library will find it. It would be new userspace that mounted it in a new place, so that new userspace could make the check in (1) look at the new place. If we actually replace selinuxfs with securityfs step 2 is going to fail. I can probably fake it somehow and pretend that selinuxfs exists as an fs, but I don't really know how to fake /proc/mounts so even though /sys/kernel/security/* is actually securityfs /sys/kernel/security/selinux looks like selinuxfs in /proc/mounts. I guess maybe the way to do it is to make selinuxfs as it stands today a config option. I don't see a reason we couldn't implement a whole new set of selinux inodes in securityfs along with those in selinuxfs. Old userspace would mount selinuxfs on /selinux and would need the compat kernel option while new userspace would change it's tests to be: 1) check for /sys/kernel/security/selinux/something 2) check for securityfs and if not assume disabled 3) check /proc/mounts and find securityfs, then check for something I'd rather not have two complete implementations of selinuxfs but if it gets us to a good endgame I guess I'm willing to support it Isn't it about time we had /sys/kernel/lsm, which would contain the name of the active LSM? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] SELINUX: add /sys/fs/selinux mount point to put selinuxfs
On 5/10/2011 3:34 PM, Greg KH wrote: From: Greg Kroah-Hartman gre...@suse.de In the interest of keeping userspace from having to create new root filesystems all the time, let's follow the lead of the other in-kernel filesystems and provide a proper mount point for it in sysfs. For selinuxfs, this mount point should be in /sys/fs/selinux/ It seems that we might want this to be an LSM interface standard. Is the call to kobject_create_and_add and associated cleanup all that's required? I would want Smack to follow the convention as well. Cc: Stephen Smalley s...@tycho.nsa.gov Cc: James Morris jmor...@namei.org Cc: Eric Paris epa...@parisplace.org Cc: Lennart Poettering mzerq...@0pointer.de Cc: Daniel J Walsh dwa...@redhat.com Signed-off-by: Greg Kroah-Hartman gre...@suse.de --- Resend, due to lack of response. Note, patch is untested, I don't have any selinux-based machines here, sorry. diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index ea39cb7..2381c16 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -1897,6 +1897,7 @@ static struct file_system_type sel_fs_type = { }; struct vfsmount *selinuxfs_mount; +static struct kobject *selinuxfs_kobj; static int __init init_sel_fs(void) { @@ -1904,9 +1905,16 @@ static int __init init_sel_fs(void) if (!selinux_enabled) return 0; + + selinux_kobj = kobject_create_and_add(selinux, fs_kobj); + if (!selinux_kobj) + return -ENOMEM; + err = register_filesystem(sel_fs_type); - if (err) + if (err) { + kobject_put(selinux_kobj); return err; + } selinuxfs_mount = kern_mount(sel_fs_type); if (IS_ERR(selinuxfs_mount)) { @@ -1923,6 +1931,7 @@ __initcall(init_sel_fs); #ifdef CONFIG_SECURITY_SELINUX_DISABLE void exit_sel_fs(void) { + kobjext_put(selinux_kobj); unregister_filesystem(sel_fs_type); } #endif ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel -- To unsubscribe from this list: send the line unsubscribe linux-security-module in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel