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

[WaLyong Cho]

On 11/06/2014 11:54 PM, Lennart Poettering wrote:
> On Tue, 04.11.14 17:35, WaLyong Cho (walyong.cho at 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
>