[systemd-devel] [PATCH v2 1/2] Smack - relabel directories and files created by systemd

[Łukasz Stelmach]

It was <2014-02-19 śro 14:30>, when Lennart Poettering wrote:
> On Wed, 19.02.14 14:07, Łukasz Stelmach (l.stelmach at samsung.com) wrote:
>
>> From: Casey Schaufler <casey at schaufler-ca.com>
>> 
>> Systemd creates directories in /dev. These directories will
>> get the label of systemd, which is the label of the System
>> domain, which is not accessable to everyone. Relabel the
>> directories, files and symlinks created so that they can be
>> generally used.
>> 
>> Signed-off-by: Casey Schaufler <casey.schaufler at intel.com>
>> Signed-off-by: Łukasz Stelmach <l.stelmach at samsung.com>
>> ---
>>  src/shared/label.c |   60 +++++++++++++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 57 insertions(+), 3 deletions(-)
>> 
>> diff --git a/src/shared/label.c b/src/shared/label.c
>> index 4a26ba9..9a1916a 100644
>> --- a/src/shared/label.c
>> +++ b/src/shared/label.c
>> @@ -41,6 +41,48 @@
>>  static struct selabel_handle *label_hnd = NULL;
>>  
>>  #endif
>> +#ifdef HAVE_SMACK
>> +#include <sys/xattr.h>
>> +#include <string.h>
>
> No includes in the middle of files please for normal API stuff.
>
> Also, these files are not smack-specific. In order to avoid superfluous
> #ifdefs, and to avoid uplicate inclusions later on, please just add
> these to the top of the file, and include string.h unconditionally, and
> xattr.h only if HAVE_XATTR is defined...
>
>> +#define FLOOR_LABEL	"_"
>> +#define STAR_LABEL	"*"
>
> hmm, could we rename these to SMACK_LABEL_FLOOR and SMACK_LABEL_STAR?
> That way they have a namespaced common namespace.
>
>> +
>> +static void smack_relabel_in_dev(const char *path) {
>> +        struct stat sb;
>> +        const char *label;
>> +        int r;
>> +
>> +        /*
>> +         * Path must be in /dev and must exist
>> +         */
>> +        if (!path_equal(path, "/dev") &&
>> +            !path_startswith(path, "/dev"))
>> +                return;
>> +
>> +        r = lstat(path, &sb);
>> +        if (r < 0)
>> +                return;
>> +
>> +        /*
>> +         * Label directories and character devices "*".
>> +         * Label symlinks "_".
>> +         * Don't change anything else.
>> +         */
>> +        if (S_ISDIR(sb.st_mode))
>> +                label = STAR_LABEL;
>> +        else if (S_ISLNK(sb.st_mode))
>> +                label = FLOOR_LABEL;
>> +        else if (S_ISCHR(sb.st_mode))
>> +                label = STAR_LABEL;
>> +        else
>> +                return;
>> +
>> +        r = setxattr(path, "security.SMACK64", label, strlen(label), 0);
>> +        if (r < 0)
>> +                log_error("Smack relabeling \"%s\" %s", path, strerror(errno));
>> +        return;
>
> This return is unnecessary...
>
> That said, I think it find it nicer if this call would actually return
> an error, so that the caller decides whether it wants to ignore it, not
> the function internally.
>
> Also, please move the #ifdef HAVE_SMACK checks inside of this function
> and make it a NOP on non-SMACK builds. That way we only have one #ifdef
> check for this and not one for each invocation of the function. The
> compiler should be smart away to suppress the function if it empty.

I am not sure about that. If we want smack_relabel_in_dev() to return a
value and call it from label_fix()

--8<---------------cut here---------------start------------->8---
int label_fix(const char *path, bool ignore_enoent, bool ignore_erofs) {
        int r = 0;

#ifdef HAVE_SELINUX
[...]
#endif
        smack_relabel_in_dev(path);

        return r;
}
--8<---------------cut here---------------end--------------->8---

then it seems better to write

--8<---------------cut here---------------start------------->8---
#elif defined(HAVE_SMACK)
        r = smack_relabel_in_dev(path);
#endif
--8<---------------cut here---------------end--------------->8---

and be able to add support for a yet undetermined security framework
below assuming systemd can have support for only one fw compiled in. How
to have support for more than one security fw reasonably compiled in? (I
think this is the moment to create the pattern).

-- 
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 489 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20140219/cb3be733/attachment.pgp>