[systemd-devel] [PATCH] smack: introduce new SmackLabelExec option

[Casey Schaufler]

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.cho at samsung.com) wrote:
>>
>>> On 11/07/2014 09:35 AM, Lennart Poettering wrote:
>>>> On Fri, 07.11.14 04:17, WaLyong Cho (walyong.cho at 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 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=.

The SmackExec= option is a good thing. Your way is sneaky. The SmackExec=
option is explicit and obvious in effect and intent. You can look in the
configuration files and see what's going to happen.

>
>> 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 process. So
> I thought SmackLabelExec more appropriate.
>
>> 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?
> I think the option name make very very us confuse. Even if we decide to
> any option that will make misunderstanding. Because the given label name
> is *NOT* applied to executed process. Just only be applied to child
> systemd process.
>
>> 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
>>