[SRU][N, O][PATCH 1/1] Backports a revert of "UBUNTU: SAUCE: apparmor4.0.0 [81/90]: apparmor: convert easy uses of unconfined() to label_mediates()"
Maxime Bélair
maxime.belair at canonical.com
Sun Nov 17 11:43:19 UTC 2024
BugLink: https://bugs.launchpad.net/bugs/2067900
Adding mediation classes in unconfined profiles caused nested profiles to be mediated, inside a container for example. This notably prevents the launching of Docker containers inside a LXC container due to pivot_root being blocked.
Backports a revert of dc757a645cfa82f6ac252365df20a36a9ff82760.
Signed-off-by: Maxime Bélair <maxime.belair at canonical.com>
---
security/apparmor/apparmorfs.c | 2 +-
security/apparmor/domain.c | 40 +++++++++++++---------------------
security/apparmor/file.c | 4 ++--
security/apparmor/ipc.c | 2 +-
security/apparmor/label.c | 8 +++----
security/apparmor/lsm.c | 16 +++++++-------
security/apparmor/mount.c | 3 ++-
security/apparmor/net.c | 2 +-
security/apparmor/task.c | 12 ++++++----
9 files changed, 42 insertions(+), 47 deletions(-)
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 822f2e6a96a7..ea5aedd90d14 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -828,7 +828,7 @@ static void profile_query_cb(struct aa_profile *profile, struct aa_perms *perms,
struct aa_perms tmp = { };
aa_state_t state = DFA_NOMATCH;
- if (!profile_mediates_safe(profile, *match_str))
+ if (profile_unconfined(profile))
return;
if (rules->file->dfa && *match_str == AA_CLASS_FILE) {
state = aa_dfa_match_len(rules->file->dfa,
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index dd457eaedab8..08c84b8bedfa 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -288,7 +288,7 @@ static int change_profile_perms(struct aa_profile *profile,
u32 request, aa_state_t start,
struct aa_perms *perms)
{
- if (!profile_mediates(profile, AA_CLASS_FILE)) {
+ if (profile_unconfined(profile)) {
perms->allow = AA_MAY_CHANGE_PROFILE | AA_MAY_ONEXEC;
perms->audit = perms->quiet = perms->kill = 0;
return 0;
@@ -655,7 +655,7 @@ static struct aa_label *profile_transition(const struct cred *subj_cred,
error = aa_path_name(&bprm->file->f_path, profile->path_flags, buffer,
&name, &info, profile->disconnected);
if (error) {
- if (!profile_mediates(profile, AA_CLASS_FILE) ||
+ if (profile_unconfined(profile) ||
(profile->label.flags & FLAG_IX_ON_NAME_ERROR)) {
AA_DEBUG(DEBUG_DOMAIN, "name lookup ix on error");
error = 0;
@@ -665,7 +665,7 @@ static struct aa_label *profile_transition(const struct cred *subj_cred,
goto audit;
}
- if (!profile_mediates(profile, AA_CLASS_FILE)) {
+ if (profile_unconfined(profile)) {
new = find_attach(bprm, profile->ns,
&profile->ns->base.profiles, name, &info);
if (new) {
@@ -752,7 +752,7 @@ static int profile_onexec(const struct cred *subj_cred,
AA_BUG(!bprm);
AA_BUG(!buffer);
- if (!profile_mediates(profile, AA_CLASS_FILE)) {
+ if (profile_unconfined(profile)) {
/* change_profile on exec already granted */
/*
* NOTE: Domain transitions from unconfined are allowed
@@ -765,7 +765,7 @@ static int profile_onexec(const struct cred *subj_cred,
error = aa_path_name(&bprm->file->f_path, profile->path_flags, buffer,
&xname, &info, profile->disconnected);
if (error) {
- if (!profile_mediates(profile, AA_CLASS_FILE) ||
+ if (profile_unconfined(profile) ||
(profile->label.flags & FLAG_IX_ON_NAME_ERROR)) {
AA_DEBUG(DEBUG_DOMAIN, "name lookup ix on error");
error = 0;
@@ -905,8 +905,8 @@ int apparmor_bprm_creds_for_exec(struct linux_binprm *bprm)
*
* Testing for unconfined must be done before the subset test
*/
- if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
- label_mediates(label, AA_CLASS_FILE) && !ctx->nnp)
+ if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) && !unconfined(label) &&
+ !ctx->nnp)
ctx->nnp = aa_get_label(label);
/* buffer freed below, name is pointer into buffer */
@@ -944,7 +944,7 @@ int apparmor_bprm_creds_for_exec(struct linux_binprm *bprm)
* aways results in a further reduction of permissions.
*/
if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) &&
- label_mediates(label, AA_CLASS_FILE) &&
+ !unconfined(label) &&
!aa_label_is_unconfined_subset(new, ctx->nnp)) {
error = -EPERM;
info = "no new privs";
@@ -1194,20 +1194,14 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags)
label = aa_get_newest_cred_label(subj_cred);
previous = aa_get_newest_label(ctx->previous);
- if (!label_mediates(label, AA_CLASS_FILE)) {
- info = "unconfined can not change_hat";
- error = -EPERM;
- goto fail;
- }
-
/*
* Detect no new privs being set, and store the label it
* occurred under. Ideally this would happen when nnp
* is set but there isn't a good way to do that yet.
*
- * Testing for mediation must be done before the subset test
+ * Testing for unconfined must be done before the subset test
*/
- if (task_no_new_privs(current) && !ctx->nnp)
+ if (task_no_new_privs(current) && !unconfined(label) && !ctx->nnp)
ctx->nnp = aa_get_label(label);
/* return -EPERM when unconfined doesn't have children to avoid
@@ -1248,9 +1242,8 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags)
/*
* no new privs prevents domain transitions that would
* reduce restrictions.
- * label_mediates(label, AA_CLASS_FILE) == true here
*/
- if (task_no_new_privs(current) &&
+ if (task_no_new_privs(current) && !unconfined(label) &&
!aa_label_is_unconfined_subset(new, ctx->nnp)) {
/* not an apparmor denial per se, so don't log it */
AA_DEBUG(DEBUG_DOMAIN,
@@ -1271,9 +1264,8 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags)
/*
* no new privs prevents domain transitions that would
* reduce restrictions.
- * label_mediates(label, AA_CLASS_FILE) == true here
*/
- if (task_no_new_privs(current) &&
+ if (task_no_new_privs(current) && !unconfined(label) &&
!aa_label_is_unconfined_subset(previous, ctx->nnp)) {
/* not an apparmor denial per se, so don't log it */
AA_DEBUG(DEBUG_DOMAIN,
@@ -1378,8 +1370,7 @@ int aa_change_profile(const char *fqname, int flags)
*
* Testing for unconfined must be done before the subset test
*/
- if (task_no_new_privs(current) &&
- label_mediates(label, AA_CLASS_FILE) && !ctx->nnp)
+ if (task_no_new_privs(current) && !unconfined(label) && !ctx->nnp)
ctx->nnp = aa_get_label(label);
if (!fqname || !*fqname) {
@@ -1405,7 +1396,7 @@ int aa_change_profile(const char *fqname, int flags)
/* This should move to a per profile test. Requires pushing build
* into callback
*/
- if (!stack && !label_mediates(label, AA_CLASS_FILE) &&
+ if (!stack && unconfined(label) &&
label == &labels_ns(label)->unconfined->label &&
aa_unprivileged_unconfined_restricted &&
/* TODO: refactor so this check is a fn */
@@ -1501,8 +1492,7 @@ int aa_change_profile(const char *fqname, int flags)
* no new privs prevents domain transitions that would
* reduce restrictions.
*/
- if (task_no_new_privs(current) &&
- label_mediates(label, AA_CLASS_FILE) &&
+ if (task_no_new_privs(current) && !unconfined(label) &&
!aa_label_is_unconfined_subset(new, ctx->nnp)) {
/* not an apparmor denial per se, so don't log it */
AA_DEBUG(DEBUG_DOMAIN,
diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index 3da8d79a7017..3871205bbad7 100644
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -381,7 +381,7 @@ int __aa_path_perm(const char *op, const struct cred *subj_cred,
typeof(*rules), list);
int e = 0;
- if (!profile_mediates(profile, AA_CLASS_FILE) ||
+ if (profile_unconfined(profile) ||
((flags & PATH_SOCK_COND) && !RULE_MEDIATES_AF(rules, AF_UNIX)))
return 0;
aa_str_perms(rules->file, rules->file->start[AA_CLASS_FILE],
@@ -403,7 +403,7 @@ static int profile_path_perm(const char *op, const struct cred *subj_cred,
const char *name;
int error;
- if (!profile_mediates(profile, AA_CLASS_FILE))
+ if (profile_unconfined(profile))
return 0;
error = path_name(op, subj_cred, &profile->label, path,
diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
index b7ea40d3de33..099ce3130063 100644
--- a/security/apparmor/ipc.c
+++ b/security/apparmor/ipc.c
@@ -88,7 +88,7 @@ static int profile_signal_perm(const struct cred *cred,
struct aa_perms perms;
aa_state_t state;
- if (!profile_mediates(profile, AA_CLASS_SIGNAL))
+ if (profile_unconfined(profile))
return 0;
ad->subj_cred = cred;
diff --git a/security/apparmor/label.c b/security/apparmor/label.c
index 25511d42cce8..ca30e6d0b1f0 100644
--- a/security/apparmor/label.c
+++ b/security/apparmor/label.c
@@ -203,13 +203,13 @@ static void accum_label_info(struct aa_label *new)
long u = FLAG_UNCONFINED;
int i;
- AA_BUG(!new || !new->vec);
+ AA_BUG(!new->vec);
/* size == 1 is a profile and flags must be set as part of creation */
if (new->size == 1)
- return;
+ return;
- for (i = 0; i < new->size; i++) {
+ for (i = 0; i < new->size; i++) {
u |= new->vec[i]->label.flags & (FLAG_DEBUG1 | FLAG_DEBUG2 |
FLAG_STALE);
if (!(u & new->vec[i]->label.flags & FLAG_UNCONFINED))
@@ -885,6 +885,7 @@ static struct aa_label *vec_create_and_insert_label(struct aa_profile **vec,
for (i = 0; i < len; i++)
new->vec[i] = aa_get_profile(vec[i]);
+
write_lock_irqsave(&ls->lock, flags);
label = __label_insert(ls, new, false);
write_unlock_irqrestore(&ls->lock, flags);
@@ -2093,7 +2094,6 @@ static struct aa_label *__label_update(struct aa_label *label)
AA_BUG(tmp == label);
goto remove;
}
-
if (labels_set(label) != labels_set(new)) {
write_unlock_irqrestore(&ls->lock, flags);
tmp = aa_label_insert(labels_set(new), new);
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 62dc21f8a6e2..b92b8dc10738 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -234,7 +234,7 @@ static int common_perm(const char *op, const struct path *path, u32 mask,
int error = 0;
label = __begin_current_label_crit_section();
- if (label_mediates(label, AA_CLASS_FILE))
+ if (!unconfined(label))
error = aa_path_perm(op, current_cred(), label, path, 0, mask,
cond);
__end_current_label_crit_section(label);
@@ -381,7 +381,7 @@ static int apparmor_path_link(struct dentry *old_dentry, const struct path *new_
return 0;
label = begin_current_label_crit_section();
- if (label_mediates(label, AA_CLASS_FILE))
+ if (!unconfined(label))
error = aa_path_link(current_cred(), label, old_dentry, new_dir,
new_dentry);
end_current_label_crit_section(label);
@@ -402,7 +402,7 @@ static int apparmor_path_rename(const struct path *old_dir, struct dentry *old_d
return 0;
label = begin_current_label_crit_section();
- if (label_mediates(label, AA_CLASS_FILE)) {
+ if (!unconfined(label)) {
struct mnt_idmap *idmap = mnt_idmap(old_dir->mnt);
vfsuid_t vfsuid;
struct path old_path = { .mnt = old_dir->mnt,
@@ -646,7 +646,7 @@ static int apparmor_file_open(struct file *file)
}
label = aa_get_newest_cred_label(file->f_cred);
- if (label_mediates(label, AA_CLASS_FILE)) {
+ if (!unconfined(label)) {
struct mnt_idmap *idmap = file_mnt_idmap(file);
struct inode *inode = file_inode(file);
vfsuid_t vfsuid;
@@ -916,7 +916,7 @@ static int apparmor_sb_mount(const char *dev_name, const struct path *path,
flags &= ~AA_MS_IGNORE_MASK;
label = __begin_current_label_crit_section();
- if (label_mediates(label, AA_CLASS_MOUNT)) {
+ if (!unconfined(label)) {
if (flags & MS_REMOUNT)
error = aa_remount(current_cred(), label, path, flags,
data);
@@ -946,7 +946,7 @@ static int apparmor_move_mount(const struct path *from_path,
int error = 0;
label = __begin_current_label_crit_section();
- if (label_mediates(label, AA_CLASS_MOUNT))
+ if (!unconfined(label))
error = aa_move_mount(current_cred(), label, from_path,
to_path);
__end_current_label_crit_section(label);
@@ -960,7 +960,7 @@ static int apparmor_sb_umount(struct vfsmount *mnt, int flags)
int error = 0;
label = __begin_current_label_crit_section();
- if (label_mediates(label, AA_CLASS_MOUNT))
+ if (!unconfined(label))
error = aa_umount(current_cred(), label, mnt, flags);
__end_current_label_crit_section(label);
@@ -974,7 +974,7 @@ static int apparmor_sb_pivotroot(const struct path *old_path,
int error = 0;
label = aa_get_current_label();
- if (label_mediates(label, AA_CLASS_MOUNT))
+ if (!unconfined(label))
error = aa_pivotroot(current_cred(), label, old_path, new_path);
aa_put_label(label);
diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index 74b7293ab971..49fe8da6fea4 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -678,7 +678,8 @@ static struct aa_label *build_pivotroot(const struct cred *subj_cred,
AA_BUG(!new_path);
AA_BUG(!old_path);
- if (!RULE_MEDIATES(rules, AA_CLASS_MOUNT))
+ if (profile_unconfined(profile) ||
+ !RULE_MEDIATES(rules, AA_CLASS_MOUNT))
return aa_get_newest_label(&profile->label);
error = aa_path_name(old_path, path_flags(profile, old_path),
diff --git a/security/apparmor/net.c b/security/apparmor/net.c
index 7e6c17fd7190..c91ce17d625d 100644
--- a/security/apparmor/net.c
+++ b/security/apparmor/net.c
@@ -224,7 +224,7 @@ static int aa_label_sk_perm(const struct cred *subj_cred,
AA_BUG(!label);
AA_BUG(!sk);
- if (ctx->label != kernel_t && label_mediates(label, AA_CLASS_NET)) {
+ if (ctx->label != kernel_t && !unconfined(label)) {
struct aa_profile *profile;
DEFINE_AUDIT_SK(ad, op, sk);
diff --git a/security/apparmor/task.c b/security/apparmor/task.c
index 56f7d7686c14..e24c59841693 100644
--- a/security/apparmor/task.c
+++ b/security/apparmor/task.c
@@ -245,8 +245,8 @@ static int profile_tracee_perm(const struct cred *cred,
struct aa_label *tracer, u32 request,
struct apparmor_audit_data *ad)
{
- if (!profile_mediates(tracee, AA_CLASS_PTRACE) ||
- !label_mediates(tracer, AA_CLASS_PTRACE))
+ if (profile_unconfined(tracee) || unconfined(tracer) ||
+ !ANY_RULE_MEDIATES(&tracee->rules, AA_CLASS_PTRACE))
return 0;
return profile_ptrace_perm(cred, tracee, tracer, request, ad);
@@ -257,11 +257,14 @@ static int profile_tracer_perm(const struct cred *cred,
struct aa_label *tracee, u32 request,
struct apparmor_audit_data *ad)
{
- if (profile_mediates(tracer, AA_CLASS_PTRACE))
+ if (profile_unconfined(tracer))
+ return 0;
+
+ if (ANY_RULE_MEDIATES(&tracer->rules, AA_CLASS_PTRACE))
return profile_ptrace_perm(cred, tracer, tracee, request, ad);
/* profile uses the old style capability check for ptrace */
- if (&tracer->label == tracee || !profile_mediates(tracer, AA_CLASS_CAP))
+ if (&tracer->label == tracee)
return 0;
ad->subj_label = &tracer->label;
@@ -409,6 +412,7 @@ struct aa_label *aa_profile_ns_perm(struct aa_profile *profile,
ad->request = request;
int error;
+
/* TODO: rework unconfined profile/dfa to mediate user ns, then
* we can drop the unconfined test
*/
--
2.43.0
More information about the kernel-team
mailing list