On 01/11/2016 05:02 PM, Seth Arnold wrote:
> Thanks for getting this started; some comments inline:
>
> On Mon, Jan 11, 2016 at 06:17:47PM -0600, Tyler Hicks wrote:
>> +=pod
>> +
>> +=head1 NAME
>> +
>> +aa_stack_profile, aa_stack_onexec - combine multiple profiles to confine a
>> task
>> +
>> +=head1 SYNOPSIS
>> +
>> +B<#include E<lt>sys/apparmor.hE<gt>>
>> +
>> +B<int aa_stack_profile(const char *profile);>
>> +
>> +B<int aa_stack_onexec(const char *profile);>
>> +
>> +Link with B<-lapparmor> when compiling.
>> +
>> +=head1 DESCRIPTION
>> +
>> +AppArmor supports stacking two or more profiles when confining a task. The
>> +result is an intersection of all profiles which are stacked. Stacking
>> profiles
>> +together is desirable when wanting to ensure that confinement will never
>> become
>> +more permissive. When changing between two profiles, as performed with
>> +aa_change_profile(2), there is always the possibility that the new profile
>> is
>> +more permissive than the old profile but that possibility is eliminated when
>> +using aa_stack_profile().
>> +
>> +To stack a profile with the current confinement context, a task can use the
>> +aa_stack_profile() function. The I<profile> parameter is a NUL-terminated
>> +string indicating a profile name that should be stacked with the current
>> +confinement.
>> +
>> +Calling aa_stack_profile("profile_a") while unconfined is equivalent to
>> calling
>> +aa_change_profile("profile_a") since the intersection of unconfined and
>> +"profile_a" is "profile_a". Calling aa_change_profile("profile_b") while
>
> Typo here, this should read aa_stack_profile("profile_b")
>
>> +confined by "profile_a" results in the task's confinement to be the
>> +intersection of "profile_a" and "profile_b". The resulting confinement
>> context
>> +will be represented as "profile_a//&profile_b" in audit log messages, the
>> +return value of aa_getcon(2), etc.
>> +
>> +Stacking another profile via aa_stack_profile() is permanent and the
>> process is not
>> +permitted to revert to the previous confinement context. Unlike
>
> I'm not sure this is true; aa_change_profile(), if properly allowed by
> policy, or Px rules in profiles, would both allow returning to previous
> confinement, right?
>
correct, change_profile and domain transition rules could result in the stack
collapsing. However all profiles in the stack would have to allow for the
same transition.
>> +aa_change_profile(2), confined programs wanting to use aa_stack_profile()
>> need
>> +no special rules in their profile to stack a new profile since the operation
>> +does not broaden the allowed permissions.
>> +
>> +Open file descriptors are not remediated after a call to aa_stack_profile()
>> +so the calling program must close(2) open file descriptors to ensure they
>> +are not available after calling aa_stack_profile().
>
> I'm not sure this is strictly true. While there's no plans for
> revalidating all open files at the time of aa_stack_profile(), I don't
> think we can promise (or should promise) that revalidation won't happen
> on future read(), write(), sendfile(), etc. calls. John has plans for
> what delegating authority looks like, and I don't want this paragraph
> here to complicate those plans.
>
Actually I can guarantee that revalidation will happen unless the stack is
a subset of the cached permissions. However a full revalidation may not happen
if profile_a is cached and profile_b is not then it only the access of
profile_b should be validated.
btw this is the case for change_profile as well, right now. With that said the
open fds will not be closed or duped to null like they would on an exec and
memory mapped fds are even more touchy. That is to say with out an exec barrier
their is a limited set of things we can do.
Revalidations in the read/write path slow things down a lot, and you are a lot
better off doing the close.
>> +The aa_stack_onexec() function is like the aa_stack_profile() function
>> +except it specifies that the stacking should take place on the next exec
>> +instead of immediately. The delayed profile change takes precedence over any
>> +exec transition rules within the confining profile. Delaying the stacking
>
> Hmm, this is an interesting point. There's something nice about the
> directness and clarity here but I think this may prematurely prevent some
> otherwise interesting uses of stacking.
>
please explain? You have the choice of stacking immediately or delaying to
the exec barrier. This is the same as is done with change_profile
> I'm also afraid that this kind of rule might also allow these APIs to be
> used by exploit code to unreasonably manipulate profile transitions in
> ways that aren't expected by policy authors.
>
how so?
the policy will have to allow stacking, and stacking is purely restrictive.
If the profile allows the stack, then and only then can it take priority
over the profiles exec transitions, just like with change_profile.
>> +boundary has a couple of advantages, it removes the need for stub transition
>> +profiles and the exec boundary is a natural security layer where potentially
>> +sensitive memory is unmapped.
>> +
>> +=head1 RETURN VALUE
>> +
>> +On success zero is returned. On error, -1 is returned, and
>> +errno(3) is set appropriately.
>> +
>> +=head1 ERRORS
>> +
>> +=over 4
>> +
>> +=item B<EINVAL>
>> +
>> +The AppArmor kernel module is not loaded, neither a profile nor a namespace
>> +was specified, or the communication via the F</proc/*/attr/current> file did
>> +not conform to protocol.
>
> Since AppArmor must be built-in these days we should probably reduce
> references to the old module style. (Nevermind the name "LSM"..)
>
meh, it is still a module, it is just a built-in module and can only be a
built-in. It is optional, can be enabled and disabled at boot time, has
a set of module parameters under /sys/modules/apparmor/ ...
>> +=item B<ENOMEM>
>> +
>> +Insufficient kernel memory was available.
>> +
>> +=item B<EPERM>
>> +
>> +The calling application is not confined by AppArmor, or the no_new_privs
>> +bit is set.
>
> When would "not confined" be returned as an error in these APIs?
> aa_stack_profile() was defined above to be the same as aa_change_profile()
> if the process was currently unconfined; aa_stack_onexec() might be
> different, but it feels like an artificial requirement. Am I overlooking
> something?
>
yeah this does feel wrong
>> +=item B<ENOENT>
>> +
>> +The specified profile does not exist, or is not visible from the current
>> +namespace.
>> +
>> +=back
>> +
>> +=head1 NOTES
>> +
>> +Using aa_stack_profile() and related libapparmor functions are the only way
>> to
>> +ensure compatibility between among varying kernel versions. However, there
>> may
>> +be some situations where libapparmor is not available and directly
>> interacting
>> +with the AppArmor filesystem is required to stack a profile.
>> +
>> +To immediately stack a profile named "profile_a", as performed with
>> +aa_stack_profile("profile_a"), the equivalent of this shell command can be
>> +used:
>> +
>> + $ echo -n "stackprofile profile_a" > /proc/self/attr/current
>> +
>> +To stack a profile named "profile_a" at the next exec, as performed with
>> +aa_stack_onexec("profile_a"), the equivalent of this shell command can be
>> used:
>> +
>> + $ echo -n "stackexec profile_a" > /proc/self/attr/exec
>> +
>> +These raw AppArmor filesystem operations must only be used when using
>> +libapparmor is not a viable option.
>> +
>> +=head1 EXAMPLE
>> +
>> +The following example shows a simple, if contrived, use of
>> +aa_stack_profile().
>> +
>> + #include <stdlib.h>
>> + #include <string.h>
>> + #include <sys/apparmor.h>
>> + #include <sys/types.h>
>> + #include <sys/stat.h>
>> + #include <fcntl.h>
>> + #include <stdio.h>
>> + #include <unistd.h>
>> +
>> + static void read_passwd()
>> + {
>> + int fd;
>> + char buf[10];
>> +
>> + if ((fd=open("/etc/passwd", O_RDONLY)) < 0) {
>> + perror("Failure opening /etc/passwd");
>> + _exit(1);
>> + }
>> +
>> + /* Verify that we can read /etc/passwd */
>> + memset(&buf, 0, 10);
>> + if (read(fd, &buf, 10) == -1) {
>> + perror("Failure reading /etc/passwd");
>> + _exit(1);
>> + }
>> + buf[9] = '\0';
>> + printf("/etc/passwd: %s\n", buf);
>> + close(fd);
>> + }
>> +
>> + int main(int argc, char * argv[])
>> + {
>> + printf("Before aa_stack_profile():\n");
>> + read_passwd();
>> +
>> + /* stack the "i_cant_be_trusted_anymore" profile, which
>> + * should not have read access to /etc/passwd. */
>> + if (aa_stack_profile("i_cant_be_trusted_anymore") < 0) {
>> + perror("Failure changing profile -- aborting");
>> + _exit(1);
>> + }
>> +
>> + printf("After aa_stack_profile():\n");
>> + read_passwd();
>> + _exit(0);
>> + }
>> +
>> +This code example requires a profile similar to the following to be loaded
>> +with apparmor_parser(8):
>> +
>> + # Confine stack_p to be able to read /etc/passwd and aa_stack_profile()
>> + # to the 'i_cant_be_trusted_anymore' profile.
>> + /tmp/stack_p {
>> + /etc/ld.so.cache mr,
>> + /lib/ld-*.so* mrix,
>> + /lib/libc*.so* mr,
>> +
>> + /etc/passwd r,
>> +
>> + # Needed for aa_stack_profile()
>> + /usr/lib/libapparmor*.so* mr,
>> + /proc/[0-9]*/attr/current w,
>> + }
>> +
>> +As well as the profile to stack:
>> +
>> + profile i_cant_be_trusted_anymore {
>> + /etc/ld.so.cache mr,
>> + /lib/ld-*.so* mrix,
>> + /lib/libc*.so* mr,
>> + }
>> +
>> +The output when run:
>> +
>> + $ /tmp/stack_p
>> + Before aa_stack_profile():
>> + /etc/passwd: root:x:0:
>> + After aa_stack_profile():
>> + Failure opening /etc/passwd: Permission denied
>> + $
>> +
>> +=head1 BUGS
>> +
>> +None known. If you find any, please report them at
>> +L<https://bugs.launchpad.net/apparmor/+filebug>. Note that using
>> +aa_stack_profile(2) without execve(2) provides no memory barriers between
>> +different areas of a program; if address space separation is required, then
>> +separate processes should be used.
>> +
>> +=head1 SEE ALSO
>> +
>> +apparmor(7), apparmor.d(5), apparmor_parser(8), aa_change_profile(2),
>> +aa_getcon(2) and L<http://wiki.apparmor.net>.
>> +
>> +=cut
>
> Thanks!
>
>
>
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
