[apparmor] [PATCH v2 21/42] parser: Add functions for features support tests

[John Johansen]

On 03/06/2015 01:48 PM, Tyler Hicks wrote:
> Defines a function that can be called to test features support. It is
> string based which allows the support tests to work with new kernel
> features without any changes.
> 
> The use of global variables in the parser to store and check features
> support is still preserved. The parser should probably move over to
> passing the aa_features object around but that's left for later.
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> ---
>  parser/features.c    | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  parser/features.h    |   1 +
>  parser/parser_main.c |  52 ++++++++++-----------
>  3 files changed, 155 insertions(+), 26 deletions(-)
> 
> diff --git a/parser/features.c b/parser/features.c
> index 07d6e6c..50e0024 100644
> --- a/parser/features.c
> +++ b/parser/features.c
> @@ -17,6 +17,7 @@
>   */
>  
>  #include <errno.h>
> +#include <ctype.h>
>  #include <fcntl.h>
>  #include <stdio.h>
>  #include <string.h>
> @@ -163,6 +164,86 @@ static int load_features_file(const char *name, char *buffer, size_t size)
>  	return 0;
>  }
>  
> +static bool walk_one(const char **str, const char *component, bool is_top_level)
> +{
> +	const char *cur = *str;
> +	uint32_t bracket_count = 0;
> +	int i = 0;
> +
> +	/* Empty strings are not accepted */
> +	if (!*cur || !component[0])
> +		return false;
> +
> +	/**
> +	 * If @component is not top-level, the first character in the string to
> +	 * search MUST be '{'
> +	 */
> +	if (!is_top_level) {
> +		if (*cur != '{')
> +			return false;
> +
> +		cur++;
> +	}
> +
> +	/**
> +	 * This loop tries to find the @component in *@str. When this loops
> +	 * completes, cur will either point one character past the end of the
> +	 * matched @component or to the NUL terminator of *@str.
> +	 */
> +	while(*cur && component[i]) {
> +		if (!isascii(*cur)) {
> +			/* Only ASCII is expected */
> +			return false;
> +		} else if (*cur == '{') {
> +			/* There's a limit to the number of opening brackets */
> +			if (bracket_count == UINT32_MAX)
> +				return false;
> +
> +			bracket_count++;
> +		} else if (*cur == '}') {
> +			/* Check for unexpected closing brackets */
> +			if (bracket_count == 0)
> +				return false;
> +
> +			bracket_count--;
> +		}
> +
> +		/**
> +		 * Move to the next character in @component if we have a match
> +		 * and either @component is not top-level or, if @component is
> +		 * top-level, we're not inside of brackets
> +		 */
> +		if (*cur == component[i] &&
> +		    (!is_top_level || bracket_count == 0))
> +			i++;
> +		else
> +			i = 0;
> +
This isn't correct. IF we are !is_top_level then we can have a partial leading
match, hit a bracket, increase bracket_count and continue matching, so that
the component
  frontback 
will match either of
  front{back
  front}back

> +		cur++;
> +	}
> +
> +	/* A full match was not found if component[i] is non-NUL */
> +	if (component[i])
> +		return false;
> +
> +	/**
> +	 * This loop eats up valid (ASCII) characters until a non-bracket or
> +	 * non-space character is found so that *@str is properly set to call
> +	 * back into this function, if necessary
> +	 */
> +	while (*cur) {
> +		if (!isascii(*cur))
> +			return false;
> +		else if (*cur == '{' || *cur == '}' || !isspace(*cur))
> +			break;
> +
> +		cur++;
> +	}
> +
> +	*str = cur;
> +	return true;
> +}
> +
>  /**
>   * aa_features_new - create a new features based on a path
>   * @features: will point to the address of an allocated and initialized
> @@ -299,3 +380,50 @@ bool aa_features_is_equal(aa_features *features1, aa_features *features2)
>  	return features1 && features2 &&
>  	       strcmp(features1->string, features2->string) == 0;
>  }
> +
> +/**
> + * aa_features_supports - provides features support status
> + * @features: the features
> + * @str: the string representation of a feature to check
> + *
> + * Example @str values are "dbus/mask/send", "caps/mask/audit_read", and
> + * "policy/versions/v7".
> + *
> + * Returns: a bool specifying the support status of @str feature
> + */
> +bool aa_features_supports(aa_features *features, char *str)
> +{
> +	const char *features_string = features->string;
> +	char *components[32];
> +	char *saveptr = NULL;
> +	size_t i;
> +
> +	/* Empty strings are not accepted. Neither are leading '/' chars. */
> +	if (!str || str[0] == '/')
> +		return false;
> +
> +	/**
> +	 * Break @str into an array of components. For example,
> +	 * "mount/mask/mount" would turn into "mount" as the first component,
> +	 * "mask" as the second, and "mount" as the third
> +	 */
> +	for (i = 0; i < sizeof(components); i++) {
> +		components[i] = strtok_r(str, "/", &saveptr);
> +		if (!components[i])
> +			break;
> +
> +		str = NULL;
> +	}
I am not a fan of the use of strtok in this fn. It means const strings can
not be passed, nor could the same query string be used multiple times (not
that you generally should but ...).
I would rather see an api that can use constant strings, but at the very
least @str needs to document that it will be modified.

> +
> +	/* At least one valid token is required */
> +	if (!components[0])
> +		return false;
> +
> +	/* Ensure that all components are valid and found */
> +	for (i = 0; i < sizeof(components) && components[i]; i++) {
> +		if (!walk_one(&features_string, components[i], i == 0))
> +			return false;
> +	}
> +
> +	return true;
> +}
> diff --git a/parser/features.h b/parser/features.h
> index a0c177f..96d0ee9 100644
> --- a/parser/features.h
> +++ b/parser/features.h
> @@ -29,5 +29,6 @@ aa_features *aa_features_ref(aa_features *features);
>  void aa_features_unref(aa_features *features);
>  const char *aa_features_get_string(aa_features *features);
>  bool aa_features_is_equal(aa_features *features1, aa_features *features2);
> +bool aa_features_supports(aa_features *features, char *str);
>  
>  #endif /* __AA_FEATURES_H */
> diff --git a/parser/parser_main.c b/parser/parser_main.c
> index 3f8ca6a..2d220a7 100644
> --- a/parser/parser_main.c
> +++ b/parser/parser_main.c
> @@ -574,7 +574,17 @@ no_match:
>  
>  static void set_supported_features(void)
>  {
> -	const char *features_string;
> +	char file[] = "file";
> +	char network[] = "network";
> +	char af_unix[] = "network/af_unix";
> +	char mount[] = "mount";
> +	char dbus[] = "dbus";
> +	char signal[] = "signal";
> +	char ptrace[] = "ptrace";
> +	char set_load[] = "policy/set_load";
> +	char diff_encode[] = "policy/diff_encode";
> +	char v7[] = "policy/versions/v7";
> +	char v6[] = "policy/versions/v6";
>  
>  	/* has process_args() already assigned a match string? */
>  	if (!features && aa_features_new_from_kernel(&features) == -1) {
> @@ -582,33 +592,23 @@ static void set_supported_features(void)
>  		return;
>  	}
>  
> -	features_string = aa_features_get_string(features);
>  	perms_create = 1;
> -
> -	/* TODO: make this real parsing and config setting */
> -	if (strstr(features_string, "file {"))	/* pre policydb is file= */
> -		kernel_supports_policydb = 1;
> -	if (strstr(features_string, "v6"))
> -		kernel_abi_version = 6;
> -	if (strstr(features_string, "v7"))
> +	kernel_supports_policydb = aa_features_supports(features, file);
> +	kernel_supports_network = aa_features_supports(features, network);
> +	kernel_supports_unix = aa_features_supports(features, af_unix);
> +	kernel_supports_mount = aa_features_supports(features, mount);
> +	kernel_supports_dbus = aa_features_supports(features, dbus);
> +	kernel_supports_signal = aa_features_supports(features, signal);
> +	kernel_supports_ptrace = aa_features_supports(features, ptrace);
> +	kernel_supports_setload = aa_features_supports(features, set_load);
> +	kernel_supports_diff_encode = aa_features_supports(features, diff_encode);
> +
> +	if (aa_features_supports(features, v7))
>  		kernel_abi_version = 7;
> -	if (strstr(features_string, "set_load"))
> -		kernel_supports_setload = 1;
> -	if (strstr(features_string, "network"))
> -		kernel_supports_network = 1;
> -	if (strstr(features_string, "af_unix"))
> -		kernel_supports_unix = 1;
> -	if (strstr(features_string, "mount"))
> -		kernel_supports_mount = 1;
> -	if (strstr(features_string, "dbus"))
> -		kernel_supports_dbus = 1;
> -	if (strstr(features_string, "signal"))
> -		kernel_supports_signal = 1;
> -	if (strstr(features_string, "ptrace {"))
> -		kernel_supports_ptrace = 1;
> -	if (strstr(features_string, "diff_encode"))
> -		kernel_supports_diff_encode = 1;
> -	else if (dfaflags & DFA_CONTROL_DIFF_ENCODE)
> +	else if (aa_features_supports(features, v6))
> +		kernel_abi_version = 6;
> +
> +	if (!kernel_supports_diff_encode)
>  		/* clear diff_encode because it is not supported */
>  		dfaflags &= ~DFA_CONTROL_DIFF_ENCODE;
>  }
>