[apparmor] [PATCH] libapparmor: ENOENT should only signify label not found in label queries

[Tyler Hicks]

On 2013-08-02 12:40:50, Tyler Hicks wrote:
> When doing label queries with aa_query_label(), there are a number of
> error conditions that can occur. Most of them indicate that the query
> could not happen. That may be because the application provided invalid
> input, memory allocation failed, the kernel doesn't support queries,
> libapparmor and the kernel may be out of sync, etc.
> 
> However, there's one special error condition that stands out from the
> rest. That is when the query was successful but the kernel was unable to
> find the subject label of the query.
> 
> This is a special error condition because it would be useful for
> applications that will be doing queries to do a test query during their
> initialization stage to make sure that their queries, the current
> version of libapparmor, and the current kernel can be expected to work
> well together later during runtime. If the test query fails, depending
> on the application's configuration, it may want to continue without
> AppArmor mediation or it may want to exit right then and there.
> 
> The significance of ENOENT for these test queries is that the
> application has no idea what a valid label may be in its early
> initialization phase. It isn't aware of what profiles are loaded and no
> other application has started talking to it. So, it will have to guess
> at a label name (maybe "default"?) for its test query and it needs to
> know if the query was successful even if the label doesn't exist.
> 
> This patch eliminates all potential return locations that may have errno
> set to ENOENT, except for the write() that sets ENOENT when the label
> isn't found.
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> ---
>  libraries/libapparmor/src/kernel_interface.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/libraries/libapparmor/src/kernel_interface.c b/libraries/libapparmor/src/kernel_interface.c
> index 34f9579..53961b2 100644
> --- a/libraries/libapparmor/src/kernel_interface.c
> +++ b/libraries/libapparmor/src/kernel_interface.c
> @@ -684,7 +684,9 @@ static void aafs_access_init_once(void)
>   * @audited: upon successful return, will be 1 if query should be audited and 0
>   *           if not
>   *
> - * Returns: 0 on success else -1 and sets errno
> + * Returns: 0 on success else -1 and sets errno. If -1 is returned and errno is
> + *          ENOENT, the query was successful but the label in the query string
> + *          could not be found by the kernel.
>   */
>  int aa_query_label(uint32_t mask, char *query, size_t size, int *allowed,
>  		   int *audited)
> @@ -708,8 +710,11 @@ int aa_query_label(uint32_t mask, char *query, size_t size, int *allowed,
>  	}
>  
>  	fd = open(aafs_access, O_RDWR);
> -	if (fd == -1)
> +	if (fd == -1) {
> +		if (errno == ENOENT)
> +			errno = EPROTONOSUPPORT;
>  		return -1;
> +	}
>  
>  	memcpy(query, AA_QUERY_CMD_LABEL, AA_QUERY_CMD_LABEL_SIZE);
>  	errno = 0;
> @@ -717,6 +722,12 @@ int aa_query_label(uint32_t mask, char *query, size_t size, int *allowed,
>  	if (ret != size) {
>  		if (ret >= 0)
>  			errno = EPROTO;
> +		/* IMPORTANT: This is the only valid place to return ENOENT. It
> +		 * indicates that the label cannot be found by the kernel but
> +		 * the query was successful. Applications may depend on this to
> +		 * check if their query strings and aa_query_label() work with
> +		 * the current kernel by doing a test query.
> +		 */

Unfortunately, this is slightly flawed due to the remaining read()
below. There's still a chance that the kernel returns an unexpected
query reply but that code wouldn't be reached if the label wasn't found
by the kernel. That may result in an out-of-sync libapparmor and kernel
even though aa_query_label() returned ENOENT.

If it isn't possible for applications to do a _full_ test query
(including final read() below), then I'm not sure that the test query
idea is worth implementing.

When I initially came up with this test query idea, I was hoping that I
could query the "unconfined" label, but that results in ENOENT.

John (or others) there's no label that I can rely on for the purpose of
doing a full test query, is there?

Tyler

>  		return -1;
>  	}
>  
> @@ -725,8 +736,7 @@ int aa_query_label(uint32_t mask, char *query, size_t size, int *allowed,
>  	(void)close(fd);
>  	errno = saved;
>  	if (ret != QUERY_LABEL_REPLY_LEN) {
> -		if (ret >= 0)
> -			errno = EPROTO;
> +		errno = EPROTO;
>  		return -1;
>  	}
>  
> -- 
> 1.8.3.2
> 
> 
> -- 
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20130802/7922fb08/attachment.pgp>