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

[John Johansen]

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