[apparmor] restrictions on profile names

[Tyler Hicks]

On 04/01/2017 10:51 PM, John Johansen wrote:
> There has been work upstream to bring generic LSM stacking to the
> Linux kernel. If this happens it will require changes to apparmor,
> specifically around the proc/<pid>/attr interfaces that apparmor
> shares with other lsms. Currently only a single LSM can use these
> interfaces, however with full lsm stacking there will need to be some
> form of sharing. The current design has these interfaces being
> multiplexed by using lsm name tagging
> 
> eg.
> 
>  > cat /proc/self/attr/current
>  lsm1='v1'lsm2='v2'lsm3='v3'
> 
> while multiplexing these interfaces is not the only possibility, any
> option will require changes to apparmor, and I would like to propose
> we begin making changes that will ensure apparmor is ready no matter
> what solution is chosen.

What are the chances that the stacking patches will be merged soon?
They've been discussed for years. I'd hate for us to make interface
changes for something that still isn't close to being merged.

> AppArmor's problem with these interfaces
> comes down to it allows arbitrary characters to be read and written,
> making it impossible for the generic infrastructure to parse out what
> is/should be apparmor's. The problem can be split into two distinct
> parts, what apparmor returns when the interface is read and what
> apparmor allows/requires be written to the interface.
> 
> The read problem is basically what characters are allowed in the
> profiles name. Currently apparmor allows any character except \000 as
> part of a profile name that begins with / (any valid file path), other
> profile names are more restricted but the exact restrictions are
> inconsistent and have never been formalized. Arbitrary characters for
> a profile name are problematic as 3rd partly applications and generic
> infrastructure must be able to cope and display the label (eg. ps,
> pstree, ...). These applications do not handle the apparmor label well
> when there are control characters etc, embedded. Nor does it make it
> easy for a generic infrastructure to be able to tease apart the
> apparmor label from other lsm labels.
> 
> So I propose we standardize on a set of printable characters for
> profile names. This will mean that profiles should be defined with a
> name separate from their attachment, ie.
> 
>   profile foo /usr/bin/bar { ... }

I agree that this is a good change. I've always been uneasy about how
the profile names are used throughout a number of userspace projects
even though they're not guaranteed to be printable.

> but even in cases where this is not the case we can have the parser
> convert the name to a valid set using a well defined
> transform. Somewhat like the standard we already employ for profile
> files in /etc/apparmor.d/.  However even with such support we should
> encourage the movement to none attachment based profile names, as they
> are easier to write policy around and shorter (which is beneficial
> when stacking is used).

What's the justification for performing a transform? Why couldn't we
simply not allow non-printable names and let the profile author handle
the transform in a way that's known and predictable to them?

> The write issue is harder, in that \000 is allowed, even required, and
> we use the size of the write to determine how much input their really
> is. This is required to continue to allow interfaces like change_hatv
> to continue to work, so instead of restricting the write input I
> propose we move the writes to a custom apparmor interface. For most
> applications the details can be hidden behind libapparmor. Basically
> we would provide
> 
>   apparmor/current
>   apparmor/exec
>   apparmor/prev
> 
> files in apparmorfs. Since writes are already restricted to be done
> only by the current task, we don't loose anything by not writing to
> the tasks attr. Once this interface is up, and the library is using
> it, we can deprecate writing to the proc interface in apparmor,
> defaulting to reject writes, but providing a sysctl to allow them. In
> this way we can find and migrate direct users of the current interface
> to the new one without breaking them.

IMO, this is the most disruptive change discussed in this email and I
think we need to think carefully about it. Is aa_change_hatv() the only
thing affected? If so, I think I'd prefer libapparmor's aa_change_hatv()
implementation to be updated to iterate over the list of hats and
effectively do an aa_change_hat() on each one of them instead of handing
the raw byte sequence, which contains NUL's, over to the kernel.

Tyler

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170508/70348ee1/attachment.pgp>