[apparmor] [PATCH 22/31] parser: Add functions for features parsing and support tests

John Johansen john.johansen at canonical.com
Thu Jan 22 18:11:54 UTC 2015


On 12/05/2014 04:22 PM, Tyler Hicks wrote:
> Defines a set of functions that can be called to test features support.
> 
> 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>

So the code is fine again, but I am questioning the API, do we really
want a separate fn call for each feature? Not that I am advocating for a
bitmap either, just trying to be sure we have shown the right direction

> ---
>  parser/features.c    | 157 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  parser/features.h    |  11 ++++
>  parser/parser_main.c |  40 ++++---------
>  3 files changed, 180 insertions(+), 28 deletions(-)
> 
> diff --git a/parser/features.c b/parser/features.c
> index 429eb21..3dbbd21 100644
> --- a/parser/features.c
> +++ b/parser/features.c
> @@ -37,8 +37,21 @@
>  
>  #define STRING_SIZE 8192
>  
> +#define SUPPORT_POLICYDB	(1<<1)
> +#define SUPPORT_ABI_V6		(1<<2)
> +#define SUPPORT_ABI_V7		(1<<3)
> +#define SUPPORT_SET_LOAD	(1<<4)
> +#define SUPPORT_NETWORK		(1<<5)
> +#define SUPPORT_AF_UNIX		(1<<6)
> +#define SUPPORT_MOUNT		(1<<7)
> +#define SUPPORT_DBUS		(1<<8)
> +#define SUPPORT_SIGNAL		(1<<9)
> +#define SUPPORT_PTRACE		(1<<10)
> +#define SUPPORT_DIFF_ENCODE	(1<<11)
> +
>  struct aa_features {
>  	unsigned int ref_count;
> +	uint16_t support;
>  	char string[STRING_SIZE];
>  };
>  
> @@ -143,6 +156,33 @@ static int load_features_file(const char *name, char *buffer, size_t size)
>  	return 0;
>  }
>  
> +static void parse_features(aa_features *features)
> +{
> +	/* TODO: make this real parsing and config setting */
> +	if (strstr(features->string, "file {"))	/* pre policydb is file= */
> +		features->support |= SUPPORT_POLICYDB;
> +	if (strstr(features->string, "v6"))
> +		features->support |= SUPPORT_ABI_V6;
> +	if (strstr(features->string, "v7"))
> +		features->support |= SUPPORT_ABI_V7;
> +	if (strstr(features->string, "set_load"))
> +		features->support |= SUPPORT_SET_LOAD;
> +	if (strstr(features->string, "network"))
> +		features->support |= SUPPORT_NETWORK;
> +	if (strstr(features->string, "af_unix"))
> +		features->support |= SUPPORT_AF_UNIX;
> +	if (strstr(features->string, "mount"))
> +		features->support |= SUPPORT_MOUNT;
> +	if (strstr(features->string, "dbus"))
> +		features->support |= SUPPORT_DBUS;
> +	if (strstr(features->string, "signal"))
> +		features->support |= SUPPORT_SIGNAL;
> +	if (strstr(features->string, "ptrace {"))
> +		features->support |= SUPPORT_PTRACE;
> +	if (strstr(features->string, "diff_encode"))
> +		features->support |= SUPPORT_DIFF_ENCODE;
> +}
> +
>  /**
>   * aa_features_new - create a new features based on a path
>   * @features: will point to the address of an allocated and initialized
> @@ -181,6 +221,7 @@ int aa_features_new(aa_features **features, const char *path)
>  		return -1;
>  	}
>  
> +	parse_features(f);
>  	*features = f;
>  
>  	return 0;
> @@ -216,6 +257,7 @@ int aa_features_new_from_string(aa_features **features,
>  
>  	memcpy(f->string, string, size);
>  	f->string[size] = '\0';
> +	parse_features(f);
>  	*features = f;
>  
>  	return 0;
> @@ -279,3 +321,118 @@ bool aa_features_is_equal(aa_features *features1, aa_features *features2)
>  	return features1 && features2 &&
>  	       strcmp(features1->string, features2->string) == 0;
>  }
> +
> +/**
> + * aa_features_supports_max_abi - the max supported ABI reported by features
> + * @features: the features
> + *
> + * Returns: an unsigned int specifying the max supported ABI
> + */
> +unsigned int aa_features_supports_max_abi(aa_features *features)
> +{
> +	if (features->support & SUPPORT_ABI_V7)
> +		return 7;
> +	else if (features->support & SUPPORT_ABI_V6)
> +		return 6;
> +
> +	return 5;
> +}
> +
> +/**
> + * aa_features_supports_policydb - provides features support status of policydb
> + * @features: the features
> + *
> + * Returns: true if policydb is supported, false if not
> + */
> +bool aa_features_supports_policydb(aa_features *features)
> +{
> +	return features->support & SUPPORT_POLICYDB;
> +}
> +
> +/**
> + * aa_features_supports_set_load - provides features support status of set_load
> + * @features: the features
> + *
> + * Returns: true if set_load is supported, false if not
> + */
> +bool aa_features_supports_set_load(aa_features *features)
> +{
> +	return features->support & SUPPORT_SET_LOAD;
> +}
> +
> +/**
> + * aa_features_supports_network - provides features support status of network
> + * @features: the features
> + *
> + * Returns: true if network is supported, false if not
> + */
> +bool aa_features_supports_network(aa_features *features)
> +{
> +	return features->support & SUPPORT_NETWORK;
> +}
> +
> +/**
> + * aa_features_supports_af_unix - provides features support status of af_unix
> + * @features: the features
> + *
> + * Returns: true if af_unix is supported, false if not
> + */
> +bool aa_features_supports_af_unix(aa_features *features)
> +{
> +	return features->support & SUPPORT_AF_UNIX;
> +}
> +
> +/**
> + * aa_features_supports_mount - provides features support status of mount
> + * @features: the features
> + *
> + * Returns: true if mount is supported, false if not
> + */
> +bool aa_features_supports_mount(aa_features *features)
> +{
> +	return features->support & SUPPORT_MOUNT;
> +}
> +
> +/**
> + * aa_features_supports_dbus - provides features support status of dbus
> + * @features: the features
> + *
> + * Returns: true if dbus is supported, false if not
> + */
> +bool aa_features_supports_dbus(aa_features *features)
> +{
> +	return features->support & SUPPORT_DBUS;
> +}
> +
> +/**
> + * aa_features_supports_signal - provides features support status of signal
> + * @features: the features
> + *
> + * Returns: true if signal is supported, false if not
> + */
> +bool aa_features_supports_signal(aa_features *features)
> +{
> +	return features->support & SUPPORT_SIGNAL;
> +}
> +
> +/**
> + * aa_features_supports_ptrace - provides features support status of ptrace
> + * @features: the features
> + *
> + * Returns: true if ptrace is supported, false if not
> + */
> +bool aa_features_supports_ptrace(aa_features *features)
> +{
> +	return features->support & SUPPORT_PTRACE;
> +}
> +
> +/**
> + * aa_features_supports_diff_encode - provides features support status of diff_encode
> + * @features: the features
> + *
> + * Returns: true if diff_encode is supported, false if not
> + */
> +bool aa_features_supports_diff_encode(aa_features *features)
> +{
> +	return features->support & SUPPORT_DIFF_ENCODE;
> +}
> diff --git a/parser/features.h b/parser/features.h
> index a0c177f..61adb12 100644
> --- a/parser/features.h
> +++ b/parser/features.h
> @@ -30,4 +30,15 @@ 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);
>  
> +unsigned int aa_features_supports_max_abi(aa_features *features);
> +bool aa_features_supports_policydb(aa_features *features);
> +bool aa_features_supports_set_load(aa_features *features);
> +bool aa_features_supports_network(aa_features *features);
> +bool aa_features_supports_af_unix(aa_features *features);
> +bool aa_features_supports_mount(aa_features *features);
> +bool aa_features_supports_dbus(aa_features *features);
> +bool aa_features_supports_signal(aa_features *features);
> +bool aa_features_supports_ptrace(aa_features *features);
> +bool aa_features_supports_diff_encode(aa_features *features);
> +
>  #endif /* __AA_FEATURES_H */
> diff --git a/parser/parser_main.c b/parser/parser_main.c
> index e5658a8..717062f 100644
> --- a/parser/parser_main.c
> +++ b/parser/parser_main.c
> @@ -556,8 +556,6 @@ int have_enough_privilege(void)
>  
>  static void set_supported_features(void)
>  {
> -	const char *features_string;
> -
>  	/* has process_args() already assigned a match string? */
>  	if (!features && aa_features_new_from_kernel(&features) == -1) {
>  		aa_match *match;
> @@ -574,33 +572,19 @@ 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_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)
> +	kernel_supports_policydb = aa_features_supports_policydb(features);
> +	kernel_abi_version = aa_features_supports_max_abi(features);
> +	kernel_supports_setload = aa_features_supports_set_load(features);
> +	kernel_supports_network = aa_features_supports_network(features);
> +	kernel_supports_unix = aa_features_supports_af_unix(features);
> +	kernel_supports_mount = aa_features_supports_mount(features);
> +	kernel_supports_dbus = aa_features_supports_dbus(features);
> +	kernel_supports_signal = aa_features_supports_signal(features);
> +	kernel_supports_ptrace = aa_features_supports_ptrace(features);
> +	kernel_supports_diff_encode = aa_features_supports_diff_encode(features);
> +
> +	if (!kernel_supports_diff_encode)
>  		/* clear diff_encode because it is not supported */
>  		dfaflags &= ~DFA_CONTROL_DIFF_ENCODE;
>  }
> 





More information about the AppArmor mailing list