apparmor: ensure the target profile name is always audited
The target profile name was not being correctly audited in a few cases because the target variable was not being set and gotos passed the code to set it at apply: Since it is always based on new_profile just drop the target var and conditionally report based on new_profile. Signed-off-by: John Johansen <john.johansen@canonical.com> Acked-by: Seth Arnold <seth.arnold@canonical.com>
This commit is contained in:
@@ -346,7 +346,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
|
|||||||
file_inode(bprm->file)->i_uid,
|
file_inode(bprm->file)->i_uid,
|
||||||
file_inode(bprm->file)->i_mode
|
file_inode(bprm->file)->i_mode
|
||||||
};
|
};
|
||||||
const char *name = NULL, *target = NULL, *info = NULL;
|
const char *name = NULL, *info = NULL;
|
||||||
int error = 0;
|
int error = 0;
|
||||||
|
|
||||||
if (bprm->cred_prepared)
|
if (bprm->cred_prepared)
|
||||||
@@ -399,6 +399,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
|
|||||||
if (cxt->onexec) {
|
if (cxt->onexec) {
|
||||||
struct file_perms cp;
|
struct file_perms cp;
|
||||||
info = "change_profile onexec";
|
info = "change_profile onexec";
|
||||||
|
new_profile = aa_get_newest_profile(cxt->onexec);
|
||||||
if (!(perms.allow & AA_MAY_ONEXEC))
|
if (!(perms.allow & AA_MAY_ONEXEC))
|
||||||
goto audit;
|
goto audit;
|
||||||
|
|
||||||
@@ -413,7 +414,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
|
|||||||
|
|
||||||
if (!(cp.allow & AA_MAY_ONEXEC))
|
if (!(cp.allow & AA_MAY_ONEXEC))
|
||||||
goto audit;
|
goto audit;
|
||||||
new_profile = aa_get_newest_profile(cxt->onexec);
|
|
||||||
goto apply;
|
goto apply;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -445,10 +445,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
|
|||||||
if (!new_profile) {
|
if (!new_profile) {
|
||||||
error = -ENOMEM;
|
error = -ENOMEM;
|
||||||
info = "could not create null profile";
|
info = "could not create null profile";
|
||||||
} else {
|
} else
|
||||||
error = -EACCES;
|
error = -EACCES;
|
||||||
target = new_profile->base.hname;
|
|
||||||
}
|
|
||||||
perms.xindex |= AA_X_UNSAFE;
|
perms.xindex |= AA_X_UNSAFE;
|
||||||
} else
|
} else
|
||||||
/* fail exec */
|
/* fail exec */
|
||||||
@@ -459,7 +457,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
|
|||||||
* fail the exec.
|
* fail the exec.
|
||||||
*/
|
*/
|
||||||
if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) {
|
if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) {
|
||||||
aa_put_profile(new_profile);
|
|
||||||
error = -EPERM;
|
error = -EPERM;
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
}
|
||||||
@@ -474,10 +471,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
|
|||||||
|
|
||||||
if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) {
|
if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) {
|
||||||
error = may_change_ptraced_domain(new_profile);
|
error = may_change_ptraced_domain(new_profile);
|
||||||
if (error) {
|
if (error)
|
||||||
aa_put_profile(new_profile);
|
|
||||||
goto audit;
|
goto audit;
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Determine if secure exec is needed.
|
/* Determine if secure exec is needed.
|
||||||
@@ -498,7 +493,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
|
|||||||
bprm->unsafe |= AA_SECURE_X_NEEDED;
|
bprm->unsafe |= AA_SECURE_X_NEEDED;
|
||||||
}
|
}
|
||||||
apply:
|
apply:
|
||||||
target = new_profile->base.hname;
|
|
||||||
/* when transitioning profiles clear unsafe personality bits */
|
/* when transitioning profiles clear unsafe personality bits */
|
||||||
bprm->per_clear |= PER_CLEAR_ON_SETID;
|
bprm->per_clear |= PER_CLEAR_ON_SETID;
|
||||||
|
|
||||||
@@ -506,15 +500,19 @@ x_clear:
|
|||||||
aa_put_profile(cxt->profile);
|
aa_put_profile(cxt->profile);
|
||||||
/* transfer new profile reference will be released when cxt is freed */
|
/* transfer new profile reference will be released when cxt is freed */
|
||||||
cxt->profile = new_profile;
|
cxt->profile = new_profile;
|
||||||
|
new_profile = NULL;
|
||||||
|
|
||||||
/* clear out all temporary/transitional state from the context */
|
/* clear out all temporary/transitional state from the context */
|
||||||
aa_clear_task_cxt_trans(cxt);
|
aa_clear_task_cxt_trans(cxt);
|
||||||
|
|
||||||
audit:
|
audit:
|
||||||
error = aa_audit_file(profile, &perms, GFP_KERNEL, OP_EXEC, MAY_EXEC,
|
error = aa_audit_file(profile, &perms, GFP_KERNEL, OP_EXEC, MAY_EXEC,
|
||||||
name, target, cond.uid, info, error);
|
name,
|
||||||
|
new_profile ? new_profile->base.hname : NULL,
|
||||||
|
cond.uid, info, error);
|
||||||
|
|
||||||
cleanup:
|
cleanup:
|
||||||
|
aa_put_profile(new_profile);
|
||||||
aa_put_profile(profile);
|
aa_put_profile(profile);
|
||||||
kfree(buffer);
|
kfree(buffer);
|
||||||
|
|
||||||
|
Reference in New Issue
Block a user