'\0' termination on writes to /selinux/*

'\0' termination on writes to /selinux/*

[Joe Nall]

I was looking to mitigate https://github.com/SELinuxProject/selinux-kernel/issues/38 somewhat in our older RHEL6 systems for which a kernel update would be a big deal with a ENOMEM errno check and retry on the writes in libselinux. In looking at the code, it appears that whether the NUL termination is written out is inconsistent. 

grep write *c| grep -v swig| grep strlen
canonicalize_context.c:	ret = write(fd, buf, strlen(buf) + 1);
check_context.c:	ret = write(fd, con, strlen(con) + 1);
compute_av.c:	ret = write(fd, buf, strlen(buf));
compute_create.c:	ret = write(fd, buf, strlen(buf));
compute_member.c:	ret = write(fd, buf, strlen(buf));
compute_relabel.c:	ret = write(fd, buf, strlen(buf));
compute_user.c:	ret = write(fd, buf, strlen(buf));
disable.c:	ret = write(fd, buf, strlen(buf));
procattr.c:			ret = write(fd, context, strlen(context) + 1);
setenforce.c:	ret = write(fd, buf, strlen(buf));

Is the NUL required inconsistently, not really required, or is this a bug?

joe

Re: '\0' termination on writes to /selinux/*

[Stephen Smalley]

On 10/31/2018 03:28 PM, Joe Nall wrote:
> I was looking to mitigate https://github.com/SELinuxProject/selinux-kernel/issues/38 somewhat in our older RHEL6 systems for which a kernel update would be a big deal with a ENOMEM errno check and retry on the writes in libselinux. In looking at the code, it appears that whether the NUL termination is written out is inconsistent.
> 
> grep write *c| grep -v swig| grep strlen
> canonicalize_context.c:	ret = write(fd, buf, strlen(buf) + 1);
> check_context.c:	ret = write(fd, con, strlen(con) + 1);
> compute_av.c:	ret = write(fd, buf, strlen(buf));
> compute_create.c:	ret = write(fd, buf, strlen(buf));
> compute_member.c:	ret = write(fd, buf, strlen(buf));
> compute_relabel.c:	ret = write(fd, buf, strlen(buf));
> compute_user.c:	ret = write(fd, buf, strlen(buf));
> disable.c:	ret = write(fd, buf, strlen(buf));
> procattr.c:			ret = write(fd, context, strlen(context) + 1);
> setenforce.c:	ret = write(fd, buf, strlen(buf));
> 
> Is the NUL required inconsistently, not really required, or is this a bug?

The short answer is that the NUL is not required.

Historically, we originally required the caller to include the NUL as 
part of the security context (value and length); hence, the interfaces 
that are just writing a context are including the NUL in their size. 
However this is no longer required by the kernel; we changed our kernel 
code to accept it with or without the NUL when we migrated to using 
xattrs because the attr userspace tools did not include the NUL.  So in 
every mainline Linux kernel (>= 2.6.0), the NUL is not required.

In contrast, the other interfaces you mentioned above are either not 
passing security contexts at all or are passing multiple 
whitespace-separated arguments in a single string (which previously were 
separate arguments to a system call interface) and thus the contexts are 
being parsed out of that string.