Skip to content

Commit 6456ccb

Browse files
jrjohansenjrjohansen
committed
apparmor: fix regression in fs based unix sockets when using old abi
Policy loaded using abi 7 socket mediation was not being applied correctly in all cases. In some cases with fs based unix sockets a subset of permissions where allowed when they should have been denied. This was happening because the check for if the socket was an fs based unix socket came before the abi check. But the abi check is where the correct path is selected, so having the fs unix socket check occur early would cause the wrong code path to be used. Fix this by pushing the fs unix to be done after the abi check. Fixes: dcd7a55 ("apparmor: gate make fine grained unix mediation behind v9 abi") Signed-off-by: John Johansen <[email protected]>
1 parent 50d56a1 commit 6456ccb

File tree

2 files changed

+71
-51
lines changed

2 files changed

+71
-51
lines changed

‎security/apparmor/af_unix.c

Lines changed: 71 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ static int profile_create_perm(struct aa_profile *profile, int family,
221221

222222
static int profile_sk_perm(struct aa_profile *profile,
223223
struct apparmor_audit_data *ad,
224-
u32 request, struct sock *sk)
224+
u32 request, struct sock *sk, struct path *path)
225225
{
226226
struct aa_ruleset *rules = list_first_entry(&profile->rules,
227227
typeof(*rules),
@@ -231,11 +231,15 @@ static int profile_sk_perm(struct aa_profile *profile,
231231

232232
AA_BUG(!profile);
233233
AA_BUG(!sk);
234-
AA_BUG(is_unix_fs(sk));
235234
AA_BUG(profile_unconfined(profile));
236235

237236
state = RULE_MEDIATES_v9NET(rules);
238237
if (state) {
238+
if (is_unix_fs(sk))
239+
return unix_fs_perm(ad->op, request, ad->subj_cred,
240+
&profile->label,
241+
&unix_sk(sk)->path);
242+
239243
state = match_to_sk(rules->policy, state, request, unix_sk(sk),
240244
&p, &ad->info);
241245

@@ -261,6 +265,9 @@ static int profile_bind_perm(struct aa_profile *profile, struct sock *sk,
261265

262266
state = RULE_MEDIATES_v9NET(rules);
263267
if (state) {
268+
if (is_unix_addr_fs(ad->net.addr, ad->net.addrlen))
269+
/* under v7-9 fs hook handles bind */
270+
return 0;
264271
/* bind for abstract socket */
265272
state = match_to_local(rules->policy, state, AA_MAY_BIND,
266273
sk->sk_type, sk->sk_protocol,
@@ -285,14 +292,18 @@ static int profile_listen_perm(struct aa_profile *profile, struct sock *sk,
285292

286293
AA_BUG(!profile);
287294
AA_BUG(!sk);
288-
AA_BUG(is_unix_fs(sk));
289295
AA_BUG(!ad);
290296
AA_BUG(profile_unconfined(profile));
291297

292298
state = RULE_MEDIATES_v9NET(rules);
293299
if (state) {
294300
__be16 b = cpu_to_be16(backlog);
295301

302+
if (is_unix_fs(sk))
303+
return unix_fs_perm(ad->op, AA_MAY_LISTEN,
304+
ad->subj_cred, &profile->label,
305+
&unix_sk(sk)->path);
306+
296307
state = match_to_cmd(rules->policy, state, AA_MAY_LISTEN,
297308
unix_sk(sk), CMD_LISTEN, &p, &ad->info);
298309
if (state && !p) {
@@ -319,12 +330,16 @@ static int profile_accept_perm(struct aa_profile *profile,
319330

320331
AA_BUG(!profile);
321332
AA_BUG(!sk);
322-
AA_BUG(is_unix_fs(sk));
323333
AA_BUG(!ad);
324334
AA_BUG(profile_unconfined(profile));
325335

326336
state = RULE_MEDIATES_v9NET(rules);
327337
if (state) {
338+
if (is_unix_fs(sk))
339+
return unix_fs_perm(ad->op, AA_MAY_ACCEPT,
340+
ad->subj_cred, &profile->label,
341+
&unix_sk(sk)->path);
342+
328343
state = match_to_sk(rules->policy, state, AA_MAY_ACCEPT,
329344
unix_sk(sk), &p, &ad->info);
330345

@@ -346,13 +361,16 @@ static int profile_opt_perm(struct aa_profile *profile, u32 request,
346361

347362
AA_BUG(!profile);
348363
AA_BUG(!sk);
349-
AA_BUG(is_unix_fs(sk));
350364
AA_BUG(!ad);
351365
AA_BUG(profile_unconfined(profile));
352366

353367
state = RULE_MEDIATES_v9NET(rules);
354368
if (state) {
355369
__be16 b = cpu_to_be16(optname);
370+
if (is_unix_fs(sk))
371+
return unix_fs_perm(ad->op, request,
372+
ad->subj_cred, &profile->label,
373+
&unix_sk(sk)->path);
356374

357375
state = match_to_cmd(rules->policy, state, request, unix_sk(sk),
358376
CMD_OPT, &p, &ad->info);
@@ -371,8 +389,9 @@ static int profile_opt_perm(struct aa_profile *profile, u32 request,
371389

372390
/* null peer_label is allowed, in which case the peer_sk label is used */
373391
static int profile_peer_perm(struct aa_profile *profile, u32 request,
374-
struct sock *sk, struct sockaddr_un *peer_addr,
375-
int peer_addrlen,
392+
struct sock *sk, struct path *path,
393+
struct sockaddr_un *peer_addr,
394+
int peer_addrlen, struct path *peer_path,
376395
struct aa_label *peer_label,
377396
struct apparmor_audit_data *ad)
378397
{
@@ -391,6 +410,12 @@ static int profile_peer_perm(struct aa_profile *profile, u32 request,
391410
if (state) {
392411
struct aa_profile *peerp;
393412

413+
if (peer_path)
414+
return unix_fs_perm(ad->op, request, ad->subj_cred,
415+
&profile->label, peer_path);
416+
else if (path)
417+
return unix_fs_perm(ad->op, request, ad->subj_cred,
418+
&profile->label, path);
394419
state = match_to_peer(rules->policy, state, request,
395420
unix_sk(sk),
396421
peer_addr, peer_addrlen, &p, &ad->info);
@@ -421,15 +446,18 @@ int aa_unix_create_perm(struct aa_label *label, int family, int type,
421446
return 0;
422447
}
423448

424-
int aa_unix_label_sk_perm(const struct cred *subj_cred, struct aa_label *label,
425-
const char *op, u32 request, struct sock *sk)
449+
static int aa_unix_label_sk_perm(const struct cred *subj_cred,
450+
struct aa_label *label,
451+
const char *op, u32 request, struct sock *sk,
452+
struct path *path)
426453
{
427454
if (!unconfined(label)) {
428455
struct aa_profile *profile;
429456
DEFINE_AUDIT_SK(ad, op, subj_cred, sk);
430457

431458
return fn_for_each_confined(label, profile,
432-
profile_sk_perm(profile, &ad, request, sk));
459+
profile_sk_perm(profile, &ad, request, sk,
460+
path));
433461
}
434462
return 0;
435463
}
@@ -441,12 +469,9 @@ int aa_unix_sock_perm(const char *op, u32 request, struct socket *sock)
441469
int error;
442470

443471
label = begin_current_label_crit_section();
444-
if (is_unix_fs(sock->sk))
445-
error = unix_fs_perm(op, request, current_cred(), label,
446-
&unix_sk(sock->sk)->path);
447-
else
448-
error = aa_unix_label_sk_perm(current_cred(), label, op,
449-
request, sock->sk);
472+
error = aa_unix_label_sk_perm(current_cred(), label, op,
473+
request, sock->sk,
474+
is_unix_fs(sock->sk) ? &unix_sk(sock->sk)->path : NULL);
450475
end_current_label_crit_section(label);
451476

452477
return error;
@@ -476,7 +501,7 @@ int aa_unix_bind_perm(struct socket *sock, struct sockaddr *addr,
476501

477502
label = begin_current_label_crit_section();
478503
/* fs bind is handled by mknod */
479-
if (!(unconfined(label) || is_unix_addr_fs(addr, addrlen))) {
504+
if (!unconfined(label)) {
480505
DEFINE_AUDIT_SK(ad, OP_BIND, current_cred(), sock->sk);
481506

482507
ad.net.addr = unix_addr(addr);
@@ -510,7 +535,7 @@ int aa_unix_listen_perm(struct socket *sock, int backlog)
510535
int error = 0;
511536

512537
label = begin_current_label_crit_section();
513-
if (!(unconfined(label) || is_unix_fs(sock->sk))) {
538+
if (!unconfined(label)) {
514539
DEFINE_AUDIT_SK(ad, OP_LISTEN, current_cred(), sock->sk);
515540

516541
error = fn_for_each_confined(label, profile,
@@ -531,7 +556,7 @@ int aa_unix_accept_perm(struct socket *sock, struct socket *newsock)
531556
int error = 0;
532557

533558
label = begin_current_label_crit_section();
534-
if (!(unconfined(label) || is_unix_fs(sock->sk))) {
559+
if (!unconfined(label)) {
535560
DEFINE_AUDIT_SK(ad, OP_ACCEPT, current_cred(), sock->sk);
536561

537562
error = fn_for_each_confined(label, profile,
@@ -564,12 +589,12 @@ int aa_unix_opt_perm(const char *op, u32 request, struct socket *sock,
564589
int error = 0;
565590

566591
label = begin_current_label_crit_section();
567-
if (!(unconfined(label) || is_unix_fs(sock->sk))) {
592+
if (!unconfined(label)) {
568593
DEFINE_AUDIT_SK(ad, op, current_cred(), sock->sk);
569594

570595
error = fn_for_each_confined(label, profile,
571-
profile_opt_perm(profile, request,
572-
sock->sk, optname, &ad));
596+
profile_opt_perm(profile, request, sock->sk,
597+
optname, &ad));
573598
}
574599
end_current_label_crit_section(label);
575600

@@ -578,8 +603,9 @@ int aa_unix_opt_perm(const char *op, u32 request, struct socket *sock,
578603

579604
static int unix_peer_perm(const struct cred *subj_cred,
580605
struct aa_label *label, const char *op, u32 request,
581-
struct sock *sk, struct sockaddr_un *peer_addr,
582-
int peer_addrlen, struct aa_label *peer_label)
606+
struct sock *sk, struct path *path,
607+
struct sockaddr_un *peer_addr, int peer_addrlen,
608+
struct path *peer_path, struct aa_label *peer_label)
583609
{
584610
struct aa_profile *profile;
585611
DEFINE_AUDIT_SK(ad, op, subj_cred, sk);
@@ -588,8 +614,9 @@ static int unix_peer_perm(const struct cred *subj_cred,
588614
ad.net.peer.addrlen = peer_addrlen;
589615

590616
return fn_for_each_confined(label, profile,
591-
profile_peer_perm(profile, request, sk,
592-
peer_addr, peer_addrlen, peer_label, &ad));
617+
profile_peer_perm(profile, request, sk, path,
618+
peer_addr, peer_addrlen, peer_path,
619+
peer_label, &ad));
593620
}
594621

595622
/**
@@ -604,27 +631,19 @@ int aa_unix_peer_perm(const struct cred *subj_cred,
604631
{
605632
struct unix_sock *peeru = unix_sk(peer_sk);
606633
struct unix_sock *u = unix_sk(sk);
634+
int plen;
635+
struct sockaddr_un *paddr = aa_sunaddr(unix_sk(peer_sk), &plen);
607636

608637
AA_BUG(!label);
609638
AA_BUG(!sk);
610639
AA_BUG(!peer_sk);
611640
AA_BUG(!peer_label);
612641

613-
if (is_unix_fs(aa_unix_sk(peeru))) {
614-
return unix_fs_perm(op, request, subj_cred, label,
615-
&peeru->path);
616-
} else if (is_unix_fs(aa_unix_sk(u))) {
617-
return unix_fs_perm(op, request, subj_cred, label, &u->path);
618-
} else if (!unconfined(label)) {
619-
int plen;
620-
struct sockaddr_un *paddr = aa_sunaddr(unix_sk(peer_sk),
621-
&plen);
622-
623-
return unix_peer_perm(subj_cred, label, op, request,
624-
sk, paddr, plen, peer_label);
625-
}
626-
627-
return 0;
642+
return unix_peer_perm(subj_cred, label, op, request, sk,
643+
is_unix_fs(sk) ? &u->path : NULL,
644+
paddr, plen,
645+
is_unix_fs(peer_sk) ? &peeru->path : NULL,
646+
peer_label);
628647
}
629648

630649
/* This fn is only checked if something has changed in the security
@@ -665,12 +684,9 @@ int aa_unix_file_perm(const struct cred *subj_cred, struct aa_label *label,
665684
if (is_sk_fs && peer_sk)
666685
sk_req = request;
667686
if (sk_req) {
668-
if (is_sk_fs)
669-
error = unix_fs_perm(op, sk_req, subj_cred, label,
670-
&path);
671-
else
672687
error = aa_unix_label_sk_perm(subj_cred, label, op,
673-
sk_req, sock->sk);
688+
sk_req, sock->sk,
689+
is_sk_fs ? &path : NULL);
674690
}
675691
if (!peer_sk)
676692
goto out;
@@ -683,7 +699,7 @@ int aa_unix_file_perm(const struct cred *subj_cred, struct aa_label *label,
683699
if (!is_sk_fs && is_unix_fs(peer_sk)) {
684700
last_error(error,
685701
unix_fs_perm(op, request, subj_cred, label,
686-
&peer_path));
702+
is_unix_fs(peer_sk) ? &peer_path : NULL));
687703
} else if (!is_sk_fs) {
688704
struct aa_sk_ctx *pctx = aa_sock(peer_sk);
689705

@@ -693,11 +709,18 @@ int aa_unix_file_perm(const struct cred *subj_cred, struct aa_label *label,
693709
last_error(error,
694710
xcheck(unix_peer_perm(subj_cred, label, op,
695711
MAY_READ | MAY_WRITE, sock->sk,
712+
is_sk_fs ? &path : NULL,
696713
peer_addr, peer_addrlen,
714+
is_unix_fs(peer_sk) ?
715+
&peer_path : NULL,
697716
pctx->label),
698717
unix_peer_perm(file->f_cred, pctx->label, op,
699718
MAY_READ | MAY_WRITE, peer_sk,
700-
addr, addrlen, label)));
719+
is_unix_fs(peer_sk) ?
720+
&peer_path : NULL,
721+
addr, addrlen,
722+
is_sk_fs ? &path : NULL,
723+
label)));
701724
}
702725
sock_put(peer_sk);
703726

‎security/apparmor/include/af_unix.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,6 @@ int aa_unix_peer_perm(const struct cred *subj_cred,
3636
struct aa_label *label, const char *op, u32 request,
3737
struct sock *sk, struct sock *peer_sk,
3838
struct aa_label *peer_label);
39-
int aa_unix_label_sk_perm(const struct cred *subj_cred,
40-
struct aa_label *label, const char *op, u32 request,
41-
struct sock *sk);
4239
int aa_unix_sock_perm(const char *op, u32 request, struct socket *sock);
4340
int aa_unix_create_perm(struct aa_label *label, int family, int type,
4441
int protocol);

0 commit comments

Comments
 (0)