Skip to content

Commit e074176

Browse files
jrjohansenAndrea Righi
authored andcommitted
UBUNTU: SAUCE: apparmor4.0.0 [61/76]: prompt - refactor to moving caching to uresponse
BugLink: https://bugs.launchpad.net/bugs/2028253 To be able to be more responsive to prompt return values refactor the code more to make caching operations distinct from user interaction and move adding to the cache into the uresps side, so actions don't have to be passed back to the paused process. This is a step towards allowing response that will use a named response instead of permissions and also allow for reuse of knotif for notifications, heartbeat and canceled. Signed-off-by: John Johansen <[email protected]> (cherry picked from https://gitlab.com/jjohansen/apparmor-kernel) Signed-off-by: Andrea Righi <[email protected]>
1 parent 06d52d7 commit e074176

File tree

3 files changed

+238
-133
lines changed

3 files changed

+238
-133
lines changed

security/apparmor/file.c

Lines changed: 54 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,16 @@ static void file_audit_cb(struct audit_buffer *ab, void *va)
7979
}
8080
}
8181

82+
// ??? differentiate between
83+
// cached - allow : no audit == 1
84+
// cached - deny : no audit < 0
85+
// cached - complain : no audit
86+
// cached - partial : audit missing part : as miss
87+
// not cached = 0
8288
static int check_cache(struct aa_profile *profile,
83-
struct apparmor_audit_data *ad,
84-
struct aa_perms *perms)
89+
struct apparmor_audit_data *ad)
8590
{
86-
struct aa_audit_node *node = NULL;
8791
struct aa_audit_node *hit;
88-
bool cache_response;
89-
int err;
9092

9193
AA_BUG(!profile);
9294
ad->subj_label = &profile->label; // normally set in aa_audit
@@ -116,34 +118,48 @@ static int check_cache(struct aa_profile *profile,
116118
/* continue to do prompt */
117119
} else {
118120
AA_DEBUG(DEBUG_UPCALL, "cache hit");
119-
ad->error = 0;
120121
aa_put_audit_node(hit);
121-
/* do audit */
122-
return 0;
122+
/* don't audit: if its in the cache already audited */
123+
return 1;
123124
}
124125
aa_put_audit_node(hit);
125126
hit = NULL;
126127
} else {
127128
AA_DEBUG(DEBUG_UPCALL, "cache miss");
128129
}
130+
131+
return 0;
132+
}
133+
134+
// error - immediate return
135+
// - debug message do audit
136+
// caching is handled on listener task side
137+
static int check_user(struct aa_profile *profile,
138+
struct apparmor_audit_data *ad,
139+
struct aa_perms *perms)
140+
{
141+
struct aa_audit_node *node = NULL;
142+
int err;
143+
129144
/* assume we are going to dispatch */
130145
node = aa_dup_audit_data(ad, GFP_KERNEL);
131146
if (!node) {
132147
AA_DEBUG(DEBUG_UPCALL,
133148
"notifcation failed to duplicate with error -ENOMEM\n");
134149
/* do audit */
135-
return 0;
150+
return -ENOMEM;
136151
}
137152

138153
get_task_struct(current);
139154
node->data.subjtsk = current;
140155
node->data.type = AUDIT_APPARMOR_USER;
141156
node->data.request = ad->request;
142157
node->data.denied = ad->request & ~perms->allow;
143-
err = aa_do_notification(APPARMOR_NOTIF_OP, node, &cache_response);
158+
err = aa_do_notification(APPARMOR_NOTIF_OP, node);
144159
put_task_struct(node->data.subjtsk);
145160

146161
if (err) {
162+
// do we want to do something special with -ERESTARTSYS
147163
AA_DEBUG(DEBUG_UPCALL, "notifcation failed with error %d\n",
148164
ad->error);
149165
goto return_to_audit;
@@ -154,33 +170,9 @@ static int check_cache(struct aa_profile *profile,
154170
ad->denied = node->data.denied;
155171
ad->error = node->data.error;
156172

157-
if (cache_response) {
158-
/* TODO: shouldn't add until after auditing it, or at
159-
* least having a refcount. Fix once removing entry is
160-
* allowed
161-
*/
162-
AA_DEBUG(DEBUG_UPCALL, "inserting cache entry requ 0x%x denied 0x%x",
163-
node->data.request, node->data.denied);
164-
hit = aa_audit_cache_insert(&profile->learning_cache,
165-
node);
166-
AA_DEBUG(DEBUG_UPCALL, "cache insert %s: name %s node %s\n",
167-
hit != node ? "lost race" : "",
168-
hit->data.name, node->data.name);
169-
if (hit != node) {
170-
AA_DEBUG(DEBUG_UPCALL, "updating existing cache entry");
171-
aa_audit_cache_update_ent(&profile->learning_cache,
172-
hit, &node->data);
173-
aa_put_audit_node(hit);
174-
} else {
175-
176-
AA_DEBUG(DEBUG_UPCALL, "inserted into cache");
177-
}
178-
/* now to audit */
179-
} /* cache_response */
180-
181173
return_to_audit:
182174
aa_put_audit_node(node);
183-
return 0;
175+
return err;
184176
}
185177

186178
/**
@@ -220,14 +212,33 @@ int aa_audit_file(const struct cred *subj_cred,
220212
ad.common.u.tsk = NULL;
221213
ad.subjtsk = NULL;
222214

223-
if (unlikely(ad.error) && ((prompt && USER_MODE(profile)) ||
224-
((request & perms->prompt) &&
225-
((request & (perms->prompt |
226-
perms->allow)) == request)))) {
227-
err = check_cache(profile, &ad, perms);
228-
if (err)
229-
/* only happens if already cached */
230-
return err;
215+
ad.denied = denied_perms(perms, ad.request);
216+
217+
if (unlikely(ad.error)) {
218+
u32 implicit_deny;
219+
220+
/* learning cache - not audit dedup yet */
221+
err = check_cache(profile, &ad);
222+
if (err != 0)
223+
/* cached */
224+
return ad.error;
225+
226+
implicit_deny = (ad.request & ~perms->allow) & ~perms->deny;
227+
if (USER_MODE(profile))
228+
perms->prompt = ALL_PERMS_MASK;
229+
230+
/* don't prompt
231+
* - if explicit deny
232+
* - if implicit_deny is not entirely covered by prompt
233+
* as no point asking user to just deny it anyway.
234+
*/
235+
if (prompt && !(request & perms->deny) &&
236+
(perms->prompt & implicit_deny) == implicit_deny) {
237+
err = check_user(profile, &ad, perms);
238+
if (err == -ERESTARTSYS)
239+
/* are there other errors we should bail on */
240+
return err;
241+
}
231242
}
232243

233244
if (likely(!ad.error)) {
@@ -260,7 +271,6 @@ int aa_audit_file(const struct cred *subj_cred,
260271
return ad.error;
261272
}
262273

263-
ad.denied = ad.request & ~perms->allow;
264274
err = aa_audit(type, profile, &ad, file_audit_cb);
265275
return err;
266276
}

security/apparmor/include/notify.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ struct aa_listener_proxy {
5050
struct list_head nslist;
5151
};
5252

53+
#define KNOTIF_PULSE
54+
#define KNOTIF_PENDING
55+
#define KNOTIF_CANCELLED
5356
/* need to split knofif into audit_proxy
5457
* prompt notifications only go to first taker so no need for completion
5558
* in the proxy, it increases size of proxy in non-prompt case
@@ -60,14 +63,14 @@ struct aa_knotif {
6063
struct completion ready;
6164
u64 id;
6265
u16 ntype;
66+
u16 flags;
6367
};
6468

6569
void aa_free_listener_proxy(struct aa_listener_proxy *proxy);
6670
bool aa_register_listener_proxy(struct aa_listener *listener, struct aa_ns *ns);
6771
struct aa_listener *aa_new_listener(struct aa_ns *ns, gfp_t gfp);
6872
struct aa_knotif *__aa_find_notif(struct aa_listener *listener, u64 id);
69-
int aa_do_notification(u16 ntype, struct aa_audit_node *node,
70-
bool *cache_response);
73+
int aa_do_notification(u16 ntype, struct aa_audit_node *node);
7174

7275
long aa_listener_unotif_recv(struct aa_listener *listener, void __user *buf,
7376
u16 max_size);

0 commit comments

Comments
 (0)