From 3037d144e84886e5798f3c67e7c75d775ec79f25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maxime=20B=C3=A9lair?= Date: Sun, 5 Jan 2025 10:33:16 +0100 Subject: [PATCH] UBUNTU: SAUCE: Revert "UBUNTU: SAUCE: apparmor4.0.0 [81/90]: apparmor: convert easy uses of unconfined() to label_mediates()" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Commit 11bd800e8f528 "UBUNTU: SAUCE: apparmor4.0.0 [88/90]: apparmor: add fine grained ipv4/ipv6 mediation" introduced minor changes in apparmor_socket_create, hence creating a conflict in the revert. As the bug is unrelated to this function, we keep the current version (11bd800e8f528) of apparmor_socket_create to fix this conflict. Signed-off-by: Maxime Bélair Acked-by: Stefan Bader Acked-by: Mehmet Basaran Signed-off-by: Koichiro Den --- 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 b2937dce4b8f..90a17e21b879 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) { @@ -757,7 +757,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 @@ -770,7 +770,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; @@ -910,8 +910,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 */ @@ -949,7 +949,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"; @@ -1199,20 +1199,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 @@ -1253,9 +1247,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, @@ -1276,9 +1269,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, @@ -1383,8 +1375,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) { @@ -1410,7 +1401,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 */ @@ -1506,8 +1497,7 @@ check: * 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 */