[apparmor] [patch 1/2] [2.8] mod_apparmor: try uri hat after AADefaultHatName, not before

[Seth Arnold]

On Tue, Jul 08, 2014 at 10:55:43AM -0700, Steve Beattie wrote:
> Bug: https://bugs.launchpad.net/bugs/1322778
> 
> Between the apparmor 2.8.2 and 2.8.3, a bug was fixed in mod_apparmor
> (in 2.8 revno 2120) 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. Additionally, because the 2.8 mod_apparmor attempts each hat
> individually (rather than use the aa_change_hatv like trunk's
> mod_apparmor), each attempt with the URI-based hatname is logged by the
> kernel portion of apparmor, making system logs particularly noisy those
> same users.
> 
> 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 2120 was fixed.
> 
> Signed-off-by: Steve Beattie <steve at nxnw.org>

Well, okay, you and John talked me into it. It's still a pity to see our
nice shiny design sullied by a bug, but so be it.

Acked-by: Seth Arnold <seth.arnold at canonical.com>

Thanks

> 
> ---
>  changehat/mod_apparmor/mod_apparmor.c   |   16 ++++++++--------
>  changehat/mod_apparmor/mod_apparmor.pod |   15 +++++++--------
>  2 files changed, 15 insertions(+), 16 deletions(-)
> 
> Index: b/changehat/mod_apparmor/mod_apparmor.c
> ===================================================================
> --- a/changehat/mod_apparmor/mod_apparmor.c
> +++ b/changehat/mod_apparmor/mod_apparmor.c
> @@ -161,14 +161,6 @@ immunix_enter_hat (request_rec *r)
>  	}
>      }
>  
> -    ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "calling change_hat [uri] %s", r->uri);
> -    sd_ret = aa_change_hat(r->uri, magic_token);
> -    if (sd_ret < 0) {
> -    	aa_change_hat(NULL, magic_token);
> -    } else {
> -	    return OK;
> -    }
> -
>      if (scfg) {
>      	ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r, "Dumping scfg info: "
>      	          "scfg='0x%lx' scfg->hat_name='%s'",
> @@ -186,6 +178,14 @@ immunix_enter_hat (request_rec *r)
>  	}
>      }
>  
> +    ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "calling change_hat [uri] %s", r->uri);
> +    sd_ret = aa_change_hat(r->uri, magic_token);
> +    if (sd_ret < 0) {
> +    	aa_change_hat(NULL, magic_token);
> +    } else {
> +	    return OK;
> +    }
> +
>      ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "calling change_hat DEFAULT_URI");
>      sd_ret = aa_change_hat(DEFAULT_URI_HAT, magic_token);
>      if (sd_ret < 0) aa_change_hat(NULL, magic_token);
> Index: b/changehat/mod_apparmor/mod_apparmor.pod
> ===================================================================
> --- a/changehat/mod_apparmor/mod_apparmor.pod
> +++ b/changehat/mod_apparmor/mod_apparmor.pod
> @@ -72,11 +72,10 @@ behavior 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 or hat named by the URI. If the AADefaultHatName hat does not
> -exist, it falls back to the DEFAULT_URI hat if it exists (as described
> -above).
> +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. If the AADefaultHatName hat does not exist,
> +then it falls back to the behavior described above.
>  
>  =back
>  
> @@ -96,11 +95,11 @@ 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 if it has been defined
> +2. try to aa_change_hat(2) into an AADefaultHatName hat if it has been defined
>  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
>  
> 
> 
> -- 
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140708/e5a9758a/attachment-0001.pgp>