[apparmor] [PATCH 09/20] add helper fn to query file path permissions

John Johansen john.johansen at canonical.com
Mon Jun 1 18:01:12 UTC 2015

On 06/01/2015 10:47 AM, Tyler Hicks wrote:
> On 2015-06-01 10:18:34, John Johansen wrote:
>> On 06/01/2015 06:46 AM, Tyler Hicks wrote:
>>> On 2015-05-31 18:00:25, Christian Boltz wrote:
>>>> Hello,
>>>> Am Freitag, 29. Mai 2015 schrieb Tyler Hicks:
>>>>> On 2015-05-30 00:00:25, Christian Boltz wrote:
>>>>>> Am Freitag, 29. Mai 2015 schrieb Tyler Hicks:
>>>>>>> On 2015-05-29 01:39:15, John Johansen wrote:
>>>>>>>> +int aa_query_file(uint32_t mask, const char *label, const char
>>>>>>>> *path, +		  int *allowed, int *audited)
>>>>>>> I prefer that we require 'size_t label_len' and 'size_t path_len'
>>>>>>> parameters. The caller may already have the string lengths stored
>>>>>>> in
>>>>>>> variables, eliminating unnecessary calls to strlen(). Also, it
>>>>>>> allows
>>>>>>> for non-nul-terminated strings to be used.
>>>>>> You mean you want to call the function with path "foo\0" and
>>>>>> path_len
>>>>>> 12345?
>>>>>> Personally, I prefer an unnecessary strlen() call over an option to
>>>>>> allow someone to hand in invalid data (and, caused by that, possibly
>>>>>> doing funny[tm] things) ;-)
>>>>> You may not be aware that strlen() requires the string to be
>>>>> nul-terminated. If they wanted to shoot themselves in the foot or "do
>>>>> funny things" they could just pass in a non nul-terminated string to
>>>>> aa_query_file().
>>>> I don't know too much about C, but I already heard that strings must be 
>>>> \0-terminated ;-) and I'm not really surprised that strlen() needs the 
>>>> \0 to find the end.
>>>>> Also, libapparmor is in the process' address space. It makes no
>>>>> difference if we allow the caller to specify the string length or
>>>>> not...
>>>> So basically we have to decide if we
>>>> a) reduce the number of function parameters, which makes it easier for
>>>>    callers, but also means that the string _must be_ \0-terminated
>>>> b) have a parameter for the string len, which could possibly come with a
>>>>    wrong value
>>>> At least for non-C callers (we have perl/python/whatever bindings for 
>>>> libapparmor), option a) sounds better to me.
>>> The string length parameters really only make sense for the C API. The
>>> bindings for higher level languages probably shouldn't require string
>>> length parameters.
>> Well in the file path case, an embedded \0 is not allowed as part of the
>> query so passing the arg is a performance optimization.
>> These helper fns are just wrappers around the query api, so that apps
>> don't need to know the format of the queries.
>> I'm fine with putting in the length parameter but I find it weired that
>> we would want length in the C api and not in other languages. Also since
>> currently our other language apis are generated with swig, the length
>> parameter argument will likely carry over.
>> I'm fine with putting the length in as an arg, I just want to make sure
>> that is what we want to do
> int aa_query_file_len(uint32_t mask, const char *label, size_t label_len,
>                       const char *path, size_t path_len,
>                       int *allowed, int *audited)
> {
> 	...
> }
> int aa_query_file_nul(uint32_t mask, const char *label, const char *path,
>                       int *allowed, int *audited)
> {
>         return aa_query_file(mask, label, strlen(label), path,
>                              strlen(path), allowed, audited);
> }
we could, though I think


might be better names. I've started thinking we should be very explicit that
this is a path query, especially since we have been thinking about a file/fd

More information about the AppArmor mailing list