[PATCH] --secontext: Implement displaying of expected context upon mismatch

[Renaud Métrich]

See inline

On 12/29/21 00:45, Dmitry V. Levin wrote:
> On Tue, Dec 28, 2021 at 01:12:16PM +0100, Renaud Métrich wrote:
> [...]
>> diff --git a/m4/st_selinux.m4 b/m4/st_selinux.m4
>> index 8b524896d..09a15e9d2 100644
>> --- a/m4/st_selinux.m4
>> +++ b/m4/st_selinux.m4
>> @@ -48,6 +48,24 @@ AS_IF([test "x$with_libselinux" != xno],
>>   		     )
>>   		    ]
>>   	      )
>> +	      AC_CHECK_LIB([selinux],[selabel_open],
>> +		[libselinux_LIBS="-lselinux"
>> +		 enable_secontext=yes
>> +		],
>> +		[if test "x$with_libselinux" != xcheck; then
>> +		   AC_MSG_FAILURE([failed to find selabel_open in libselinux])
>> +		 fi
>> +		]
>> +	      )
>> +	      AC_CHECK_LIB([selinux],[selabel_lookup],
>> +		[libselinux_LIBS="-lselinux"
>> +		 enable_secontext=yes
>> +		],
>> +		[if test "x$with_libselinux" != xcheck; then
>> +		   AC_MSG_FAILURE([failed to find selabel_lookup in libselinux])
>> +		 fi
>> +		]
>> +	      )
>>   	      LDFLAGS="$saved_LDFLAGS"
>>   	     ],
>>   	     [AS_IF([test "x$with_libselinux" != xcheck],
> After commit v5.15~18 this could be re-written as follows:
>
> --- a/m4/st_selinux.m4
> +++ b/m4/st_selinux.m4
> @@ -35,7 +35,7 @@ AS_IF([test "x$with_libselinux" != xno],
>   	     [saved_LDFLAGS="$LDFLAGS"
>   	      LDFLAGS="$LDFLAGS $libselinux_LDFLAGS"
>   	      missing=
> -	      for func in getpidcon getfilecon; do
> +	      for func in getpidcon getfilecon selabel_open selabel_lookup; do
>   		AC_CHECK_LIB([selinux], [$func], [:],
>   			     [missing="$missing $func"])
>   	      done
OK
> [...]
>> +static int
>> +get_expected_filecontext(const char *path, char **result)
>> +{
>> +	static struct selabel_handle *hdl = NULL;
>> +	static bool disabled = false;
>> +
>> +	if (disabled)
>> +		return -1;
>> +
>> +	if (!hdl) {
>> +		hdl = selabel_open(SELABEL_CTX_FILE, NULL, 0);
>> +		if (!hdl) {
>> +			error_msg("Could not open SELinux database, disabling "
>> +				  "context mismatch checking: %s",
>> +				  strerror(errno));
> perror_msg should be used instead of error_msg(..., strerror(errno)).
OK
>> +			disabled = true;
>> +			return -1;
>> +		}
>> +	}
>> +
>> +	/*
>> +	 * We need to fully resolve the path, because selabel_lookup() isn't
>> +	 * smart enough to automatically resolve
>> +	 */
>> +
>> +	char *resolved = realpath(path, NULL);
>> +	if (!resolved)
>> +		return -1;
> realpath looks like overkill, it may issue quite a few syscalls while
> a simple readlink should be enough to resolve the symlink in /proc/.

Unfortunately readlink() is not working here because readlink() doesn't 
resolve fully but selabel_lookup() really requires knowing the path, 
because it just checks in its database for the corresponding regex.

Example:

$ cd /tmp
$ ln -s /home/rmetrich symlinkdir
$ touch /home/rmetrich/bar
$ ln -s /tmp/symlinkdir/bar
$ matchpathcon $(readlink bar)
/tmp/symlinkdir/bar	<<none>>

---> WRONG

$ matchpathcon $(realpath bar)
/home/rmetrich/bar	unconfined_u:object_r:user_home_t:s0

---> RIGHT

>> +	struct stat statbuf;
>> +	if (stat(resolved, &statbuf) == -1) {
>> +		free(resolved);
>> +		return -1;
>> +	}
Actually I think stat() is not needed, if realpath() fails because the 
file doesn't exist/isn't accessible, we will return in error as well.
> Maybe stat should be called before readlink.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20211229/69d8c31d/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20211229/69d8c31d/attachment.bin>