[apparmor] [Patch] mod_apparmor: try uri hat after AADefaultHatName, not before

[John Johansen]

On 06/12/2014 05:46 PM, Seth Arnold wrote:
> On Thu, Jun 12, 2014 at 02:23:46PM -0700, Steve Beattie wrote:
>> Bug: https://bugs.launchpad.net/bugs/1322778
>>
>> In trunk revno 2335, a bug was fixed in mod_apparmor that corrected
>> the storage location for AADefaultHatName.  The incorrect storage
>> caused the hat specified by the AADefaultHatName keyword to be the
>> default value for AAHatName, and meant that if both an AAHatName and
>> an AADefaultHatName entry were given in a vhost, mod_apparmor would
>> not fall back to trying AADefaultHatName if the hat specified in
>> AAHatName did not exist in the apache apparmor profile.
>>
>> However, because the value specified in AADefaultHatName was the
>> default, if no AAHatName was specified, it would be attempted first,
>> before a hat based on the passed URI, rather than after as the
>> documentation stated and the code intended. By fixing the storage bug,
>> the attempted hat ordering now matched the documentation. But a number
>> of users came to rely on AADefaultHatName being attempted before
>> the URI. For trunk, this issue is less severe because mod_apparmor
>> passes a vector of hats to aa_change_hatv(), and thus missing URI
>> hats are not logged by the kernel apparmor bits. It still represents
>> a behavioral change to users, though.
>>
>> This patch re-adjusts the ordering so that the URI-based hat is
>> attempted after the hat specified by AADefaultHatName is attempted,
>> thus maintaining the actual behavior before the bug addressed in
>> revno 2335 was fixed.
>>
>> (Apologies that the manpage changes are represented as larger than the
>> actual changes made; I reflowed the paragraphs, which caused gratuitous
>> changes.)
>>
> 
> I don't like this much; if I've understood it correctly, this patch makes
> it impossible to have most URIs in an application use one hat and
> special-case a few URIs with another hat. AADefaultHatName and AAHatName
> are more or less identical, right?
> 
> That said, the implementation looks good.
> 
> Should we use this in a distro patch somewhere to fix a regression?
> 
Distro patch? As in fix it in something released or only apply changes in
the distro?

The problem is that mod_apparmor had a behavior that all profiles developed
for it depend on. A change was made based off of documentation (not
implementation) that broke a lot of people. As much as I don't like that
error, and the name confusion it causes, that is what exists.

At this point we need to accept the old behavior as the behavior and
move forward (fix the regression and the documentation). We can introduce
a new feature/hat type to introduce new behavior (ie. AAAfterURLHatName)
but we should stay backwards compatible with existing policy