On 01/23/2014 02:45 AM, Steve Beattie wrote: > This patch adds code that checks the resulting hat that apache gets > placed into, and verifies that if the apache configuration specified > that an AAHatName or AADefaultHatName should have been the resulting > hat. If it wasn't, emit a warning message to the apache log, as this > likely indicates a mismatch between the apache configuration and its > apparmor policy (i.e. why define AAHatName if you aren't going to > create the corresponding hat in the apparmor policy?) > > (Note for AADefaultHatName, a message is not logged if a defined > AAHatName would also apply or if there is a hat defined for the uri, > as each of those come first in the order of attempted hats.) > > Signed-off-by: Steve Beattie <[email protected]>
So a few comments inline, with that said I am not opposed to committing this as is. So with or with out the changes Acked-by: John Johansen <[email protected]> > --- > changehat/mod_apparmor/mod_apparmor.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > Index: b/changehat/mod_apparmor/mod_apparmor.c > =================================================================== > --- a/changehat/mod_apparmor/mod_apparmor.c > +++ b/changehat/mod_apparmor/mod_apparmor.c > @@ -137,6 +137,7 @@ immunix_enter_hat (request_rec *r) > ap_get_module_config (r->server->module_config, > &apparmor_module); > const char *aa_hat_array[5] = { NULL, NULL, NULL, NULL, NULL }; > int i = 0; > + char *aa_con, *aa_mode, *aa_hat; > > debug_dump_uri(r); > ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r, "in immunix_enter_hat (%s) > n:0x%lx p:0x%lx main:0x%lx", > @@ -191,6 +192,36 @@ immunix_enter_hat (request_rec *r) > ap_log_rerror(APLOG_MARK, APLOG_WARNING, errno, r, "aa_change_hatv > call failed"); > } > > + /* Check to see if a defined AAHatName or AADefaultHatName would > + * apply, but wasn't the hat we landed up in; report a warning if > + * that's the case. */ > + sd_ret = aa_getcon(&aa_con, &aa_mode); Do we always want to do this or only when a certain LOG or warning level is set? > + if (sd_ret < 0) { > + ap_log_rerror(APLOG_MARK, APLOG_WARNING, errno, r, "aa_getcon call > failed"); > + } else { > + ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r, > + "AA checks: aa_getcon result is '%s', mode '%s'", > aa_con, aa_mode); > + aa_hat = strstr(aa_con, "//"); This will break for nested profiles and stacking. That being said I think it is fine for all current deployments as we don't allow nesting beyond the first subprofile level in policy yet. And stacking will likely only be used between namespaces where aa_getcon will not report parent namespace info. But perhaps you could add a little TODO: comment something like /* TODO: update to use library find hat_name fn */ > + if (aa_hat != NULL && strcmp(aa_mode, "enforce") == 0) { > + aa_hat += 2; /* skip "//" */ > + ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r, > + "AA checks: apache is in hat '%s', mode '%s'", > aa_hat, aa_mode); > + if (dcfg != NULL && dcfg->hat_name != NULL) { > + if (strcmp(aa_hat, dcfg->hat_name) != 0) > + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, > + "AAHatName '%s' applies, but does not appear to be a > hat in the apache apparmor policy", > + dcfg->hat_name); > + } else if (scfg != NULL && scfg->hat_name != NULL) { > + if (strcmp(aa_hat, scfg->hat_name) != 0 && > + strcmp(aa_hat, r->uri) != 0) > + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, > + "AADefaultHatName '%s' applies, but does not appear to > be a hat in the apache apparmor policy", > + scfg->hat_name); > + } > + } > + free(aa_con); > + } > + > return OK; > } > > > > -- AppArmor mailing list [email protected] Modify settings or > unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor > -- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
