Skip to content

Commit 88fec35

Browse files
jrjohansenjrjohansen
committed
apparmor: make sure unix socket labeling is correctly updated.
When a unix socket is passed into a different confinement domain make sure its cached mediation labeling is updated to correctly reflect which domains are using the socket. Fixes: c05e705 ("apparmor: add fine grained af_unix mediation") Signed-off-by: John Johansen <[email protected]>
1 parent 6456ccb commit 88fec35

File tree

6 files changed

+231
-62
lines changed

6 files changed

+231
-62
lines changed

‎security/apparmor/af_unix.c

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,67 @@ int aa_unix_peer_perm(const struct cred *subj_cred,
646646
peer_label);
647647
}
648648

649+
/* sk_plabel for comparison only */
650+
static void update_sk_ctx(struct sock *sk, struct aa_label *label,
651+
struct aa_label *plabel)
652+
{
653+
struct aa_label *l, *old;
654+
struct aa_sk_ctx *ctx = aa_sock(sk);
655+
bool update_sk;
656+
657+
rcu_read_lock();
658+
update_sk = (plabel &&
659+
(plabel != rcu_access_pointer(ctx->peer_lastupdate) ||
660+
!aa_label_is_subset(plabel, rcu_dereference(ctx->peer)))) ||
661+
!__aa_subj_label_is_cached(label, rcu_dereference(ctx->label));
662+
rcu_read_unlock();
663+
if (!update_sk)
664+
return;
665+
666+
spin_lock(&unix_sk(sk)->lock);
667+
old = rcu_dereference_protected(ctx->label,
668+
lockdep_is_held(&unix_sk(sk)->lock));
669+
l = aa_label_merge(old, label, GFP_ATOMIC);
670+
if (l) {
671+
if (l != old) {
672+
rcu_assign_pointer(ctx->label, l);
673+
aa_put_label(old);
674+
} else
675+
aa_put_label(l);
676+
}
677+
if (plabel && rcu_access_pointer(ctx->peer_lastupdate) != plabel) {
678+
old = rcu_dereference_protected(ctx->peer, lockdep_is_held(&unix_sk(sk)->lock));
679+
680+
if (old == plabel) {
681+
rcu_assign_pointer(ctx->peer_lastupdate, plabel);
682+
} else if (aa_label_is_subset(plabel, old)) {
683+
rcu_assign_pointer(ctx->peer_lastupdate, plabel);
684+
rcu_assign_pointer(ctx->peer, aa_get_label(plabel));
685+
aa_put_label(old);
686+
} /* else race or a subset - don't update */
687+
}
688+
spin_unlock(&unix_sk(sk)->lock);
689+
}
690+
691+
static void update_peer_ctx(struct sock *sk, struct aa_sk_ctx *ctx,
692+
struct aa_label *label)
693+
{
694+
struct aa_label *l, *old;
695+
696+
spin_lock(&unix_sk(sk)->lock);
697+
old = rcu_dereference_protected(ctx->peer,
698+
lockdep_is_held(&unix_sk(sk)->lock));
699+
l = aa_label_merge(old, label, GFP_ATOMIC);
700+
if (l) {
701+
if (l != old) {
702+
rcu_assign_pointer(ctx->peer, l);
703+
aa_put_label(old);
704+
} else
705+
aa_put_label(l);
706+
}
707+
spin_unlock(&unix_sk(sk)->lock);
708+
}
709+
649710
/* This fn is only checked if something has changed in the security
650711
* boundaries. Otherwise cached info off file is sufficient
651712
*/
@@ -655,6 +716,7 @@ int aa_unix_file_perm(const struct cred *subj_cred, struct aa_label *label,
655716
struct socket *sock = (struct socket *) file->private_data;
656717
struct sockaddr_un *addr, *peer_addr;
657718
int addrlen, peer_addrlen;
719+
struct aa_label *plabel = NULL;
658720
struct sock *peer_sk = NULL;
659721
u32 sk_req = request & ~NET_PEER_MASK;
660722
struct path path;
@@ -666,7 +728,6 @@ int aa_unix_file_perm(const struct cred *subj_cred, struct aa_label *label,
666728
AA_BUG(!sock->sk);
667729
AA_BUG(sock->sk->sk_family != PF_UNIX);
668730

669-
/* TODO: update sock label with new task label */
670731
/* investigate only using lock via unix_peer_get()
671732
* addr only needs the memory barrier, but need to investigate
672733
* path
@@ -701,8 +762,12 @@ int aa_unix_file_perm(const struct cred *subj_cred, struct aa_label *label,
701762
unix_fs_perm(op, request, subj_cred, label,
702763
is_unix_fs(peer_sk) ? &peer_path : NULL));
703764
} else if (!is_sk_fs) {
765+
struct aa_label *plabel;
704766
struct aa_sk_ctx *pctx = aa_sock(peer_sk);
705767

768+
rcu_read_lock();
769+
plabel = aa_get_label_rcu(&pctx->label);
770+
rcu_read_unlock();
706771
/* no fs check of aa_unix_peer_perm because conditions above
707772
* ensure they will never be done
708773
*/
@@ -713,18 +778,26 @@ int aa_unix_file_perm(const struct cred *subj_cred, struct aa_label *label,
713778
peer_addr, peer_addrlen,
714779
is_unix_fs(peer_sk) ?
715780
&peer_path : NULL,
716-
pctx->label),
717-
unix_peer_perm(file->f_cred, pctx->label, op,
781+
plabel),
782+
unix_peer_perm(file->f_cred, plabel, op,
718783
MAY_READ | MAY_WRITE, peer_sk,
719784
is_unix_fs(peer_sk) ?
720785
&peer_path : NULL,
721786
addr, addrlen,
722787
is_sk_fs ? &path : NULL,
723788
label)));
789+
if (!error && !__aa_subj_label_is_cached(plabel, label))
790+
update_peer_ctx(peer_sk, pctx, label);
724791
}
725792
sock_put(peer_sk);
726793

727794
out:
728795

796+
/* update peer cache to latest successful perm check */
797+
if (error == 0)
798+
update_sk_ctx(sock->sk, label, plabel);
799+
aa_put_label(plabel);
800+
729801
return error;
730802
}
803+

‎security/apparmor/file.c

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -561,19 +561,35 @@ static int __file_sock_perm(const char *op, const struct cred *subj_cred,
561561
return error;
562562
}
563563

564-
/* wrapper fn to indicate semantics of the check */
565-
static bool __subj_label_is_cached(struct aa_label *subj_label,
566-
struct aa_label *obj_label)
567-
{
568-
return aa_label_is_subset(obj_label, subj_label);
569-
}
570-
571564
/* for now separate fn to indicate semantics of the check */
572565
static bool __file_is_delegated(struct aa_label *obj_label)
573566
{
574567
return unconfined(obj_label);
575568
}
576569

570+
static bool __unix_needs_revalidation(struct file *file, struct aa_label *label,
571+
u32 request)
572+
{
573+
struct socket *sock = (struct socket *) file->private_data;
574+
575+
lockdep_assert_in_rcu_read_lock();
576+
577+
if (!S_ISSOCK(file_inode(file)->i_mode))
578+
return false;
579+
if (request & NET_PEER_MASK)
580+
return false;
581+
if (sock->sk->sk_family == PF_UNIX) {
582+
struct aa_sk_ctx *ctx = aa_sock(sock->sk);
583+
584+
if (rcu_access_pointer(ctx->peer) !=
585+
rcu_access_pointer(ctx->peer_lastupdate))
586+
return true;
587+
return !__aa_subj_label_is_cached(rcu_dereference(ctx->label),
588+
label);
589+
}
590+
return false;
591+
}
592+
577593
/**
578594
* aa_file_perm - do permission revalidation check & audit for @file
579595
* @op: operation being checked
@@ -612,14 +628,15 @@ int aa_file_perm(const char *op, const struct cred *subj_cred,
612628
*/
613629
denied = request & ~fctx->allow;
614630
if (unconfined(label) || __file_is_delegated(flabel) ||
615-
(!denied && __subj_label_is_cached(label, flabel))) {
631+
__unix_needs_revalidation(file, label, request) ||
632+
(!denied && __aa_subj_label_is_cached(label, flabel))) {
616633
rcu_read_unlock();
617634
goto done;
618635
}
619636

637+
/* slow path - revalidate access */
620638
flabel = aa_get_newest_label(flabel);
621639
rcu_read_unlock();
622-
/* TODO: label cross check */
623640

624641
if (file->f_path.mnt && path_mediated_fs(file->f_path.dentry))
625642
error = __file_path_perm(op, subj_cred, label, flabel, file,

‎security/apparmor/include/label.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,13 @@ static inline void aa_put_label(struct aa_label *l)
415415
kref_put(&l->count, aa_label_kref);
416416
}
417417

418+
/* wrapper fn to indicate semantics of the check */
419+
static inline bool __aa_subj_label_is_cached(struct aa_label *subj_label,
420+
struct aa_label *obj_label)
421+
{
422+
return aa_label_is_subset(obj_label, subj_label);
423+
}
424+
418425

419426
struct aa_proxy *aa_alloc_proxy(struct aa_label *l, gfp_t gfp);
420427
void aa_proxy_kref(struct kref *kref);

‎security/apparmor/include/net.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@
4747
#define NET_PEER_MASK (AA_MAY_SEND | AA_MAY_RECEIVE | AA_MAY_CONNECT | \
4848
AA_MAY_ACCEPT)
4949
struct aa_sk_ctx {
50-
struct aa_label *label;
51-
struct aa_label *peer;
50+
struct aa_label __rcu *label;
51+
struct aa_label __rcu *peer;
52+
struct aa_label __rcu *peer_lastupdate; /* ptr cmp only, no deref */
5253
};
5354

5455
static inline struct aa_sk_ctx *aa_sock(const struct sock *sk)

0 commit comments

Comments
 (0)