Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
On 09/11/14 02:08, Casey Schaufler wrote: 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. I think there's a much more fundamental reason why SMACK64EXEC would be a bad idea for dbus[1], which is that dbus-daemon has not been designed to distrust its caller and prevent privilege escalation from its caller's privileges to its own privileges. I think an executable can only safely be setuid, or other frameworks' equivalents of setuid (setcap, SMACK64EXEC, etc.), if the developers of that executable are fully aware that it is a privilege boundary in that way, and are designing it (and choosing its library dependencies!) with that in mind. This is not the case for dbus-daemon. It is entirely possible (although IMO unlikely) that, by coincidence, dbus-daemon does not currently have privilege escalations from its caller's privileges to its own privileges; but if we introduced such a thing (executing a command given by an environment variable, perhaps), I would not consider that to be a regression, because we never claimed it was suitable for that use. This is not unique to dbus; it applies equally to any project that releases executables. Note that dbus-daemon --system is designed to be a different sort of privilege boundary: it enforces differing privilege levels for applications that connect to it via D-Bus, and we do treat holes in that policy as security flaws. That still doesn't mean it is designed to be suitable for setuid. S [1] I assume you mean dbus-daemon, which is an executable; dbus is a package containing dbus-daemon, some other executables, and the libdbus library. There is no such thing as /usr/bin/dbus. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
On 11/10/2014 08:57 PM, Simon McVittie wrote: On 09/11/14 02:08, Casey Schaufler wrote: 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. I think there's a much more fundamental reason why SMACK64EXEC would be a bad idea for dbus[1], which is that dbus-daemon has not been designed to distrust its caller and prevent privilege escalation from its caller's privileges to its own privileges. I think an executable can only safely be setuid, or other frameworks' equivalents of setuid (setcap, SMACK64EXEC, etc.), if the developers of that executable are fully aware that it is a privilege boundary in that way, and are designing it (and choosing its library dependencies!) with that in mind. This is not the case for dbus-daemon. It is entirely possible (although IMO unlikely) that, by coincidence, dbus-daemon does not currently have privilege escalations from its caller's privileges to its own privileges; but if we introduced such a thing (executing a command given by an environment variable, perhaps), I would not consider that to be a regression, because we never claimed it was suitable for that use. This is not unique to dbus; it applies equally to any project that releases executables. Note that dbus-daemon --system is designed to be a different sort of privilege boundary: it enforces differing privilege levels for applications that connect to it via D-Bus, and we do treat holes in that policy as security flaws. That still doesn't mean it is designed to be suitable for setuid. S [1] I assume you mean dbus-daemon, which is an executable; dbus is a package containing dbus-daemon, some other executables, and the libdbus library. There is no such thing as /usr/bin/dbus. Ah..dbus-daemon was just a example as a well known daemon. Actually, this problem was occurred on email-service daemon. Currently, that has system::email SMACK label. Please forgot I'd mentioned about dbus-daemon. Currently, we are using daemon-daemon with _ label like other system daemon. I wonder about other guys also confused about this. Thanks WaLyong ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
On Fri, 07.11.14 10:03, Casey Schaufler (ca...@schaufler-ca.com) wrote: 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. OK! WaLyong, let's name it SmackLabel= then! Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
On 11/10/2014 10:26 PM, Lennart Poettering wrote: On Fri, 07.11.14 10:03, Casey Schaufler (ca...@schaufler-ca.com) wrote: 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. OK! WaLyong, let's name it SmackLabel= then! I think I had made you to bother. Excuse me, but I'm asking you again. And I think introducing new config should be careful. Hmm, I'm still confusing. We're already using SmackLabel= as socket config item. Yeah, it can possible as both exec/socket config. But each purposes are different. In socket config, this config is used to set SMACK64 of socket file. In exec config, this config is used to set child systemd attribute when User= config is given. And does we have to explain each man page? Or drop from socket package and move that to exec page? I'm not sure it make sense. Thanks WaLyong Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
On Tue, 11.11.14 00:43, WaLyong Cho (walyong@samsung.com) wrote: On 11/10/2014 10:26 PM, Lennart Poettering wrote: On Fri, 07.11.14 10:03, Casey Schaufler (ca...@schaufler-ca.com) wrote: 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. OK! WaLyong, let's name it SmackLabel= then! I think I had made you to bother. Excuse me, but I'm asking you again. And I think introducing new config should be careful. Hmm, I'm still confusing. We're already using SmackLabel= as socket config item. Yeah, it can possible as both exec/socket config. But each purposes are different. In socket config, this config is used to set SMACK64 of socket file. In exec config, this config is used to set child systemd attribute when User= config is given. And does we have to explain each man page? Or drop from socket package and move that to exec page? I'm not sure it make sense. Hmm, OK, so you might actually have a point. And this is because .socket units may carry ExecStartPre= command lines which are execute before we start listening to a socket. If we'd just have SmackLabel= then it would not be clear whether it applies as file system label to the socket fds, or if it applies as process label to the ExecStartPre= processes. Hmm, I guess I am fine with SmackLabelExec= then! Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
On 11/11/2014 04:10 AM, Lennart Poettering wrote: On Tue, 11.11.14 00:43, WaLyong Cho (walyong@samsung.com) wrote: On 11/10/2014 10:26 PM, Lennart Poettering wrote: On Fri, 07.11.14 10:03, Casey Schaufler (ca...@schaufler-ca.com) wrote: 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. OK! WaLyong, let's name it SmackLabel= then! I think I had made you to bother. Excuse me, but I'm asking you again. And I think introducing new config should be careful. Hmm, I'm still confusing. We're already using SmackLabel= as socket config item. Yeah, it can possible as both exec/socket config. But each purposes are different. In socket config, this config is used to set SMACK64 of socket file. In exec config, this config is used to set child systemd attribute when User= config is given. And does we have to explain each man page? Or drop from socket package and move that to exec page? I'm not sure it make sense. Hmm, OK, so you might actually have a point. And this is because .socket units may carry ExecStartPre= command lines which are execute before we start listening to a socket. If we'd just have SmackLabel= then it would not be clear whether it applies as file system label to the socket fds, or if it applies as process label to the ExecStartPre= processes. Hmm, I guess I am fine with SmackLabelExec= then! Hmm, I'd thouth about this again. The name SmackLabelExec= can be shown as the value will be set to the target processes what will be executed by child systemd. But acctually the label only be used to access the executable file. I think just read the SMACK64 of the executable file and set child systemd itself will reduce our naming pain. But Casey said it way is sneaky. How do you think? WaLyong Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
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. Precondition in SMACK side: 1. SMACK access permission checking is by-passed for root uid processes. 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. 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. 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.) The second. This method do *NOT* need any new option. Before systemd is calling enforce_user(), systemd still has root uid and can read SMACK64 of the file on ExecStart. So the child systemd process can set /proc/$PID/attr/current of itself to same label what was red. Then alwasy the child systemd process can access the given file on ExecStart=. 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=? I don't care much this. But I menthioned above. The given label is applied on child systemd. That is not file. That is running
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 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=? 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? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
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] smack: introduce new SmackLabelExec option
On Tue, 04.11.14 17:35, WaLyong Cho (walyong@samsung.com) wrote:
In case of systemd has _ label and run as root, if a service file
has User= option and the command line file has a special SMACK label
then systemd will fail to execute the command. Generally, SMACK label
is ignored for the root. But if a service has a User= then systemd
will call setresuid() in the child process. After then it no more
root. So it should have some of executable label for the command. To
set the SMACK64EXEC before the uid is changed introduce new
SmackLabelExec option.
Hmm, I am not sure I like the abbreviation of this. Can't we just call
this SmackLabel=?
+#ifdef HAVE_SMACK
+#include smack-util.h
+#endif
+
ifdeffing the include is unnecessary. YOu can just include it without
ifdef protectionn, there's nothing in it that we need to avoid pullin in.
#define SMACK_FLOOR_LABEL _
@@ -123,6 +124,31 @@ int mac_smack_apply_ip_in_fd(int fd, const char *label) {
return r;
}
+int mac_smack_apply_pid(pid_t pid, const char *label) {
+int r = 0;
+_cleanup_free_ char *path = NULL;
+
+assert(label);
+
+#ifdef HAVE_SMACK
+if (!mac_smack_use())
+return 0;
+
+if (pid)
+r = asprintf(path, /proc/%lu/attr/current, (unsigned
long) pid);
+else
+r = asprintf(path, /proc/self/attr/current);
+if (r 0)
+return -ENOMEM;
Please use procfs_file_alloca() for this. It makes this much nicer!
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
On 11/06/2014 11:54 PM, Lennart Poettering wrote:
On Tue, 04.11.14 17:35, WaLyong Cho (walyong@samsung.com) wrote:
In case of systemd has _ label and run as root, if a service file
has User= option and the command line file has a special SMACK label
then systemd will fail to execute the command. Generally, SMACK label
is ignored for the root. But if a service has a User= then systemd
will call setresuid() in the child process. After then it no more
root. So it should have some of executable label for the command. To
set the SMACK64EXEC before the uid is changed introduce new
SmackLabelExec option.
Hmm, I am not sure I like the abbreviation of this. Can't we just call
this SmackLabel=?
SmackLabel is already used as socket. Can we use that also here?
By the way, I hope to discuss about the naming of the SMACK options.
SmackLabel/SmackLabelIPIn/SmackLabelIPOut are. They are used in socket
group. According to SMACK description,
SMACK64/SMACK64EXEC/SMACK64MMAP/SMACK64TRANSMUTE/SMACK64IPIN/SMACK64IPOUT are
the origin attribute name. I think using origin name is most make sense.
If you agree, then in this case, SMACK64EXEC will be.
+#ifdef HAVE_SMACK
+#include smack-util.h
+#endif
+
ifdeffing the include is unnecessary. YOu can just include it without
ifdef protectionn, there's nothing in it that we need to avoid pullin in.
SELINUX/APPARMOR also use #ifdef. But can SMACK use without that?
#define SMACK_FLOOR_LABEL _
@@ -123,6 +124,31 @@ int mac_smack_apply_ip_in_fd(int fd, const char *label)
{
return r;
}
+int mac_smack_apply_pid(pid_t pid, const char *label) {
+int r = 0;
+_cleanup_free_ char *path = NULL;
+
+assert(label);
+
+#ifdef HAVE_SMACK
+if (!mac_smack_use())
+return 0;
+
+if (pid)
+r = asprintf(path, /proc/%lu/attr/current, (unsigned
long) pid);
+else
+r = asprintf(path, /proc/self/attr/current);
+if (r 0)
+return -ENOMEM;
Please use procfs_file_alloca() for this. It makes this much nicer!
Thanks for advising. I will change.
WaLyong
Lennart
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
On Fri, 07.11.14 03:18, WaLyong Cho (walyong@gmail.com) wrote: On 11/06/2014 11:54 PM, Lennart Poettering wrote: On Tue, 04.11.14 17:35, WaLyong Cho (walyong@samsung.com) wrote: In case of systemd has _ label and run as root, if a service file has User= option and the command line file has a special SMACK label then systemd will fail to execute the command. Generally, SMACK label is ignored for the root. But if a service has a User= then systemd will call setresuid() in the child process. After then it no more root. So it should have some of executable label for the command. To set the SMACK64EXEC before the uid is changed introduce new SmackLabelExec option. Hmm, I am not sure I like the abbreviation of this. Can't we just call this SmackLabel=? SmackLabel is already used as socket. Can we use that also here? Well, sure. I mean, SmackLabel= on a socket unit applies to the socket, and SmackLabel= on a service unit applies to the processes forked off, that feels quite natural I think. Overloading the field in this way appears to be quite appropriate to me in this case. By the way, I hope to discuss about the naming of the SMACK options. SmackLabel/SmackLabelIPIn/SmackLabelIPOut are. They are used in socket group. According to SMACK description, SMACK64/SMACK64EXEC/SMACK64MMAP/SMACK64TRANSMUTE/SMACK64IPIN/SMACK64IPOUT are the origin attribute name. I think using origin name is most make sense. If you agree, then in this case, SMACK64EXEC will be. What precisely is the SMACK64 label, and in which way does it differ from SMACK64EXEC? The former is the xattr field on files, the latter the current procfs file on processes? What is SMACK64MMAP for? Does any of the other labels apply to processes? Naming things is always one of the most difficult problems in computer science I guesss... In general we try to not do unnecessary abbreviations, especially for more exotic functionality. It's certainly a good idea to stay close to the low-level concepts, but then again dropping components of a low-level name doesn't sound too bad to me. ifdeffing the include is unnecessary. YOu can just include it without ifdef protectionn, there's nothing in it that we need to avoid pullin in. SELINUX/APPARMOR also use #ifdef. But can SMACK use without that? Well, they import system headers, but smack-util.h is not a system header, it's shipped in systemd itself... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
On 11/07/2014 03:30 AM, Lennart Poettering wrote: On Fri, 07.11.14 03:18, WaLyong Cho (walyong@gmail.com) wrote: On 11/06/2014 11:54 PM, Lennart Poettering wrote: On Tue, 04.11.14 17:35, WaLyong Cho (walyong@samsung.com) wrote: In case of systemd has _ label and run as root, if a service file has User= option and the command line file has a special SMACK label then systemd will fail to execute the command. Generally, SMACK label is ignored for the root. But if a service has a User= then systemd will call setresuid() in the child process. After then it no more root. So it should have some of executable label for the command. To set the SMACK64EXEC before the uid is changed introduce new SmackLabelExec option. Hmm, I am not sure I like the abbreviation of this. Can't we just call this SmackLabel=? SmackLabel is already used as socket. Can we use that also here? Well, sure. I mean, SmackLabel= on a socket unit applies to the socket, and SmackLabel= on a service unit applies to the processes forked off, that feels quite natural I think. Overloading the field in this way appears to be quite appropriate to me in this case. By the way, I hope to discuss about the naming of the SMACK options. SmackLabel/SmackLabelIPIn/SmackLabelIPOut are. They are used in socket group. According to SMACK description, SMACK64/SMACK64EXEC/SMACK64MMAP/SMACK64TRANSMUTE/SMACK64IPIN/SMACK64IPOUT are the origin attribute name. I think using origin name is most make sense. If you agree, then in this case, SMACK64EXEC will be. What precisely is the SMACK64 label, and in which way does it differ from SMACK64EXEC? The former is the xattr field on files, the latter the current procfs file on processes? What is SMACK64MMAP for? Does any of the other labels apply to processes? Sorry, I missed attaching URL: http://www.webcitation.org/6AqzohCXq 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. SMACK64MMAP Don't allow the file to be mmapped by a process whose Smack label does not allow all of the access permitted to a process with the label contained in this attribute. This is a very specific use case for shared libraries. SMACK64TRANSMUTE Can only have the value TRUE. If this attribute is present on a directory when an object is created in the directory and the Smack rule (more below) that permitted the write access to the directory includes the transmute (t) mode the object gets the label of the directory instead of the label of the creating process. If the object being created is a directory the SMACK64TRANSMUTE attribute is set as well. Maybe SMACK64MMAP/SMACK64TRANSMUTE wasn't used in systemd. So we don't need to discuss about SMACK64MMAP/SMACK64TRANSMUTE on here. I'm not an expert on SMACK, but if I add some explain, we generally assign three kind of label to filesystem object. We usally call them as label, execute label and transmute. transmute has only effect on directory. So we just need to think about label and execute label. Every filesystem objects have to have a label. That can be some string or be _ or *...but the execute label is excuted process's attribute label. That can be none. If a object has none execute label then the object will be run as caller processes label. (I think I'd confused. We should use SMACK64 or SmackLabel instead SMACK64EXEC or SmackLabelExec in here.) Auke, what do you think? Naming things is always one of the most difficult problems in computer science I guesss... In general we try to not do unnecessary abbreviations, especially for more exotic functionality. It's certainly a good idea to stay close to the low-level concepts, but then again dropping components of a low-level name doesn't sound too bad to me. ifdeffing the include is unnecessary. YOu can just include it without ifdef protectionn, there's nothing in it that we need to avoid pullin in. SELINUX/APPARMOR also use #ifdef. But can SMACK use without that? Well, they import system headers, but smack-util.h is not a system header, it's shipped in systemd itself... Lennart ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
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? (I think I'd confused. We should use SMACK64 or SmackLabel instead SMACK64EXEC or SmackLabelExec in here.) Auke, what do you think? Now I am even more confused than I was before... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
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
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 something what can has executable
rule for the label of ExecStart= file.
Regarding to new option's naming, we will set attribute of process(child
systemd). So that is subject. So I think SmackLabelExec= or SMACK64EXEC=
are more appropriate.
I think we can consider another method. If we don't want to add new
SMACK option then we can do like this. Read the access label of
executable file of ExecStart=. And just set the label to attribute of
fork()-ed child systemd process. Then we don't need to add new SMACK
option. I'm not sure this is right way.
Regarding to the predefined labels:
https://www.kernel.org/doc/Documentation/security/Smack.txt
There are some predefined labels:
_ Pronounced floor, a single underscore character.
^ Pronounced hat, a single circumflex character.
* Pronounced star, a single asterisk character.
? Pronounced huh, a single question mark character.
@ Pronounced web, a single at sign character.
Every task on a Smack system is assigned a label. System tasks, such as
init(8) and systems daemons, are run with the floor (_) label. User tasks
are assigned labels according to the specification found in the
/etc/smack/user configuration file.
Thanks for reading,
WaLyong
(I think I'd confused. We should use SMACK64 or SmackLabel instead
SMACK64EXEC or SmackLabelExec
[systemd-devel] [PATCH] smack: introduce new SmackLabelExec option
In case of systemd has _ label and run as root, if a service file
has User= option and the command line file has a special SMACK label
then systemd will fail to execute the command. Generally, SMACK label
is ignored for the root. But if a service has a User= then systemd
will call setresuid() in the child process. After then it no more
root. So it should have some of executable label for the command. To
set the SMACK64EXEC before the uid is changed introduce new
SmackLabelExec option.
---
man/systemd.exec.xml | 9 +++
src/core/dbus-execute.c | 19 +
src/core/execute.c| 14 ++
src/core/execute.h| 3 +++
src/core/load-fragment-gperf.gperf.m4 | 7 +++--
src/core/load-fragment.c | 50 +++
src/core/load-fragment.h | 1 +
src/shared/exit-status.h | 1 +
src/shared/smack-util.c | 26 ++
src/shared/smack-util.h | 1 +
10 files changed, 129 insertions(+), 2 deletions(-)
diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index e9af4ab..27e6fae 100644
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -1138,6 +1138,15 @@
/varlistentry
varlistentry
+termvarnameSmackLabelExec=/varname/term
+
+listitemparaSet the SMACK security
+label of the executed process. This directive
is ignored if SMACK is
+disabled. If prefixed by literal-/literal,
all errors will be
+ignored./para/listitem
+/varlistentry
+
+varlistentry
termvarnameIgnoreSIGPIPE=/varname/term
listitemparaTakes a boolean
diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index 9276da4..5c56824 100644
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -508,6 +508,24 @@ static int property_get_apparmor_profile(
return sd_bus_message_append(reply, (bs),
c-apparmor_profile_ignore, c-apparmor_profile);
}
+static int property_get_smack_exec_label(
+sd_bus *bus,
+const char *path,
+const char *interface,
+const char *property,
+sd_bus_message *reply,
+void *userdata,
+sd_bus_error *error) {
+
+ExecContext *c = userdata;
+
+assert(bus);
+assert(reply);
+assert(c);
+
+return sd_bus_message_append(reply, (bs),
c-smack_exec_label_ignore, c-smack_exec_label);
+}
+
static int property_get_personality(
sd_bus *bus,
const char *path,
@@ -636,6 +654,7 @@ const sd_bus_vtable bus_exec_vtable[] = {
SD_BUS_PROPERTY(UtmpIdentifier, s, NULL, offsetof(ExecContext,
utmp_id), SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY(SELinuxContext, (bs),
property_get_selinux_context, 0, SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY(AppArmorProfile, (bs),
property_get_apparmor_profile, 0, SD_BUS_VTABLE_PROPERTY_CONST),
+SD_BUS_PROPERTY(SmackLabelExec, (bs),
property_get_smack_exec_label, 0, SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY(IgnoreSIGPIPE, b, bus_property_get_bool,
offsetof(ExecContext, ignore_sigpipe), SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY(NoNewPrivileges, b, bus_property_get_bool,
offsetof(ExecContext, no_new_privileges), SD_BUS_VTABLE_PROPERTY_CONST),
SD_BUS_PROPERTY(SystemCallFilter, (bas),
property_get_syscall_filter, 0, SD_BUS_VTABLE_PROPERTY_CONST),
diff --git a/src/core/execute.c b/src/core/execute.c
index c41aec2..5222aee 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -90,6 +90,10 @@
#include seccomp-util.h
#endif
+#ifdef HAVE_SMACK
+#include smack-util.h
+#endif
+
#define IDLE_TIMEOUT_USEC (5*USEC_PER_SEC)
#define IDLE_TIMEOUT2_USEC (1*USEC_PER_SEC)
@@ -1617,6 +1621,16 @@ static int exec_child(ExecCommand *command,
}
}
+#ifdef HAVE_SMACK
+if (context-smack_exec_label) {
+err = mac_smack_apply_pid(0,
context-smack_exec_label);
+if (err 0) {
+*error = EXIT_SMACK_LABEL;
+return err;
+}
+}
+#endif
+
if (context-user) {
err = enforce_user(context, uid);
if (err 0) {
diff --git a/src/core/execute.h b/src/core/execute.h
index c45dde5..e6b9122 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -142,6 +142,9 @@ struct ExecContext {
bool apparmor_profile_ignore;
char *apparmor_profile;
+bool
