smack_new_label_from_path() counts terminating zero

[jobol]

The kernel LSM Smack may include the terminating zero in the returned label lengths.

This have to be handled by the Smack aware library libsmack.

[jarkkojs]

Yes, extended attributes contain data blobs. We use them in libsmack as strings. What is the problem here?

[jarkkojs]

I would like to add that in both branches labels go through get_label() internal function, which certainly does not count \0.

[jobol]

Please, check the function smack_new_label_from_path. It will return 1+length of the returned string.

[jarkkojs]

That's a regression then. I'll update the subject a bit.

[jarkkojs]

Comments are appreciated. I think this is the reasonable way to fix it because file attributes have additional issue that they are not validated necessarily when they are set.

[jobol]

To my eyes, the patch sounds to do the job. So it is good for me.

But... From a system designer there are issue with the way smack handles labels. There are two cases:

• case 1: the security is on and it use smack and the attribute "security.SMACK64", "security.SMACK64IPIN" or "security.SMACK64IPOUT".
• case 2: the security is off or it is not Smack or the attribute is "security.SMACK64EXEC" or "security.SMACK64TRANSMUTE" or "security.SMACK64MMAP".

In the first case (case 1), the LSM Smack is already validating labels, truncating it if invalid. For example "this is a label" will return "this" in a such way that the function getxattr would return 5 characters: "this\0".

In the second case (case 2), the function getxattr will return "this is a label" and you will fail to validate it. But the LSM Smack acts differently: for the label "security.SMACK64EXEC" for example, it will do as if the value is "this".

This issue is very theorical and it may not be a practical one if everyone use the smack utilities. But I don't want to forbid the use of libattr-devel and I can't validate every program avec CAP_MAC_ADMIN then... Suppose you write the shell script id.sh:

#!/bin/sh
id

and you run the program:

#include <sys/types.h>
#include <attr/xattr.h>
int main(){
 return setxattr("id.sh","security.SMACK64EXEC","this is a label",15,0)!=0;
}

then:

root:~> chsmack id.sh 
id.sh access="System" execute="this is a label"
root:~> ./id.sh 
uid=0(root) gid=0(root) groups=0(root),29(audio),6505(pulse-access),6506(pulse-rt) context=this

And you can see that chsmack prompts well "this is a label", what your current patch forbids. But that is for the label "security.SMACK64EXEC". That would be different with "security.SMACK64" (case 1).

[jarkkojs]

Yes, I'm aware of that issue. That's why we introduced validation into libsmack in the first place and this is good spot to make use of that. I hope that systemd would decide at some point to move into using libsmack because a lot of effort has been done (and will done) to provide clean API to the kernel interface.

It's good that in Tizen at least RPM plug-in uses this library, which should make sure that only validated labels get set for files.