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? Thanks > Signed-off-by: Steve Beattie <[email protected]> > --- > changehat/mod_apparmor/mod_apparmor.c | 8 ++++---- > changehat/mod_apparmor/mod_apparmor.pod | 21 ++++++++++----------- > 2 files changed, 14 insertions(+), 15 deletions(-) > > Index: b/changehat/mod_apparmor/mod_apparmor.c > =================================================================== > --- a/changehat/mod_apparmor/mod_apparmor.c > +++ b/changehat/mod_apparmor/mod_apparmor.c > @@ -159,10 +159,6 @@ immunix_enter_hat (request_rec *r) > aa_hat_array[i++] = dcfg->hat_name; > } > > - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, > - "[uri] adding uri '%s' to aa_change_hat vector", r->uri); > - aa_hat_array[i++] = r->uri; > - > if (scfg) { > ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r, "Dumping scfg info: " > "scfg='0x%lx' scfg->hat_name='%s'", > @@ -184,6 +180,10 @@ immunix_enter_hat (request_rec *r) > } > > ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, > + "[uri] adding uri '%s' to aa_change_hat vector", r->uri); > + aa_hat_array[i++] = r->uri; > + > + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, > "[default] adding '%s' to aa_change_hat vector", > DEFAULT_URI_HAT); > aa_hat_array[i++] = DEFAULT_URI_HAT; > > Index: b/changehat/mod_apparmor/mod_apparmor.pod > =================================================================== > --- a/changehat/mod_apparmor/mod_apparmor.pod > +++ b/changehat/mod_apparmor/mod_apparmor.pod > @@ -72,14 +72,13 @@ behavior described above. > > =item B<AADefaultHatName> > > -AADefaultHatName allows you to specify a default hat to be used > -for virtual hosts and other Apache server directives, so that you > -can have different defaults for different virtual hosts. This can > -be overridden by the AAHatName directive and is checked for only if > -there isn't a matching AAHatName or hat named by the URI. The default > -value of AADefaultHatName is the ServerName for the server/vhost > -configuration. If the AADefaultHatName hat does not exist, it falls > -back to the DEFAULT_URI hat if it exists (as described above). > +AADefaultHatName allows you to specify a default hat to be used for > +virtual hosts and other Apache server directives, so that you can have > +different defaults for different virtual hosts. This can be overridden > +by the AAHatName directive and is checked for only if there isn't > +a matching AAHatName. The default value of AADefaultHatName is the > +ServerName for the server/vhost configuration. If the AADefaultHatName > +hat does not exist, then it falls back to the behavior described above. > > =back > > @@ -99,12 +98,12 @@ will: > 1. try to aa_change_hat(2) into a matching AAHatName hat if it exists and > applies, otherwise it will > > -2. try to aa_change_hat(2) into the URI itself, otherwise it will > - > -3. try to aa_change_hat(2) into an AADefaultHatName hat, either the > +2. try to aa_change_hat(2) into an AADefaultHatName hat, either the > ServerName (the default) or the configuration value specified by the > AADefaultHatName directive, for the server/vhost, otherwise it will > > +3. try to aa_change_hat(2) into the URI itself, otherwise it will > + > 4. try to aa_change_hat(2) into the DEFAULT_URI hat, if it exists, otherwise > it > will > > -- > Steve Beattie > <[email protected]> > http://NxNW.org/~steve/ > -- > AppArmor mailing list > [email protected] > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/apparmor
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
