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

Tyler Hicks tyhicks at canonical.com
Mon Jun 1 18:04:11 UTC 2015


On 2015-06-01 11:01:12, John Johansen wrote:
> 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
> 
> aa_query_file_path()
> and
> aa_query_file_path_len()
> 
> 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
> query.

Good point. I like the two proposed names.

Tyler
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150601/bf0e75f8/attachment.pgp>


More information about the AppArmor mailing list