[apparmor] [PATCH v2 21/42] parser: Add functions for features support tests
John Johansen
john.johansen at canonical.com
Fri Mar 20 11:44:42 UTC 2015
On 03/12/2015 03:12 PM, Tyler Hicks wrote:
> On 2015-03-12 04:17:56, John Johansen wrote:
>> 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>
Acked-by: John Johansen <john.johansen 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
>
> Hmm... I don't think that's possible since there's no 'continue' when we
> hit a bracket. The (*cur == component[i]) test will fail and i will be
> reset back to 0.
>
> Either way, this is way too complicated of a function for me to be
> sending out without tests. I apologize for that. I'll add tests,
> including the case you mentioned above, and then fix issues and resend
> this patch out.
>
>>> + 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.
>
> Yeah, I completely agree. I'll see if I can come up with something that
> works off of string lengths instead of NUL terminators.
>
> Tyler
>
>>> +
>>> + /* 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;
>>> }
>>>
>>
More information about the AppArmor
mailing list