[apparmor] [PATCH 7/6] libapparmor: Strip a trailing newline character in aa_splitcon(3)

[John Johansen]

On 05/19/2015 07:32 AM, Tyler Hicks wrote:
> Adjust the internal splitcon() function to strip a single trailing
> newline character when the bool strip_newline argument is true.
> 
> aa_getprocattr_raw(2) needs to set strip_newline to true since the
> kernel appends a newline character to the end of the AppArmor contexts
> read from /proc/>PID>/attr/current.
> 
> aa_splitcon(3) also sets strip_newline to true since it is unknown
> whether the context is originated from a location that appends a newline
> or not.
> 
> aa_getpeercon_raw(2) does not set strip_newline to true since it is
> unexpected for the kernel to append a newline to the the buffer returned
> from getsockopt(2).
> 
> This patch also creates tests specifically for splitcon() and updates
> the aa_splitcon(3) man page.
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> ---
>  libraries/libapparmor/doc/aa_splitcon.pod |   7 ++
>  libraries/libapparmor/src/kernel.c        |  50 ++++++----
>  libraries/libapparmor/src/tst_kernel.c    | 161 ++++++++++++++++++++++++------
>  3 files changed, 166 insertions(+), 52 deletions(-)
> 
> diff --git a/libraries/libapparmor/doc/aa_splitcon.pod b/libraries/libapparmor/doc/aa_splitcon.pod
> index a85b619..2a46dd2 100644
> --- a/libraries/libapparmor/doc/aa_splitcon.pod
> +++ b/libraries/libapparmor/doc/aa_splitcon.pod
> @@ -40,6 +40,11 @@ terminated. The enforcement mode is also NUL terminated and the parenthesis
>  surrounding the mode are removed. If @mode is non-NULL, it will point to the
>  first character in the enforcement mode string on success.
>  
> +The Linux kernel's /proc/<PID>/attr/current interface appends a trailing
> +newline character to AppArmor contexts that are read from that file. If @con
> +contains a single trailing newline character, it will be stripped by
> +aa_splitcon() prior to all other processing.
> +
>  =head1 RETURN VALUE
>  
>  Returns a pointer to the first character in the label string. NULL is returned
> @@ -50,7 +55,9 @@ on error.
>   Context                        Label               Mode 
>   -----------------------------  ------------------  -------
>   unconfined                     unconfined          NULL
> + unconfined\n                   unconfined          NULL
>   /bin/ping (enforce)            /bin/ping           enforce
> + /bin/ping (enforce)\n          /bin/ping           enforce
>   /usr/sbin/rsyslogd (complain)  /usr/sbin/rsyslogd  complain
> 

so this has got me thinking as to what behavior we want to show here. First
what of for processes that have a profile in the unconfined mode

  /bin/ping (unconfined)           /bin/ping            unconfined?

or do we want to be hiding that info from the kernel? Generally I think it is useful
for introspection purposes to know that the profile name separate from the mode.
If we have
  /bin/ping (unconfined)
  /foo/bar  (unconfined)

they both get treated as unconfined for enforcement purposes but as soon as /bin/ping
is replaced with an enforcing profile, we get
  /bin/ping (enforce)
  /foo/bar  (unconfined)

from an introspection pov, it is nice to know that when /bin/ping is replaced it will
get enforced. It will be difficult for users if they see all the processes with
/bin/ping and /foo/bar lumped together as one confinement (unconfined) and then, when
replacing one of those profiles get half of them split back out.


with that said we still have the unconfined profile (which has a mode of unconfined that
is not reported), do we leave it as
  unconfined		unconfined		null
or do
  unconfined		unconfined		unconfined



So the other problem is stacked profiles
  /bin/foo//&/bin/bar	(EE)			EE?   or just enforce?
  /bin/A//&/bin/C	(EC)			EC?

kernel side currently we report an abbreviated single character mode for each profile
in the stack, but we could report a single mode if they are all the same like in the EE
case.