On 01/23/2014 02:33 PM, Steve Beattie wrote: > On Thu, Jan 23, 2014 at 03:49:51AM -0800, John Johansen wrote: >> 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? > > Hrm, maybe. It's only useful to do the work when APLOG_WARNING is being > reported, so it could test on that, but that's a log level I'd almost > certainly expect to be enabled by default. We could adjust the log > level lower as well, I guess, but I fear that it's not nearly as useful > to carry the code if it's at a level that isn't reported by default. > > It kind of points to a minor deficiency in aa_change_hatv()'s > interface, in that you know you successfully changed to hat or not, > but not which one. > yep, its actually a problem I tried to come up with a solution for but didn't like the option presented with the proc interface. Basically the only thing we could do is return an error code, and map a specific range of error codes to being success, and subtracting the base of the range to find the hat number.
But to be change_hat compatible we need to return the size consumed, at least if its the first hat that matched. Its doable but ugly and I'd rather a different solution. -- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
