From 040d9e2bce0a5b321c402b79ee43a8e8d2fd3b06 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 23 Jan 2018 01:47:42 -0800 Subject: [PATCH 01/36] apparmor: fix display of .ns_name for containers The .ns_name should not be virtualized by the current ns view. It needs to report the ns base name as that is being used during startup as part of determining apparmor policy namespace support. BugLink: http://bugs.launchpad.net/bugs/1746463 Fixes: d9f02d9c237aa ("apparmor: fix display of ns name") Cc: Stable Reported-by: Serge Hallyn Tested-by: Serge Hallyn Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index d4fa04d91439..a23b0ca19fd0 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -1189,9 +1189,7 @@ static int seq_ns_level_show(struct seq_file *seq, void *v) static int seq_ns_name_show(struct seq_file *seq, void *v) { struct aa_label *label = begin_current_label_crit_section(); - - seq_printf(seq, "%s\n", aa_ns_name(labels_ns(label), - labels_ns(label), true)); + seq_printf(seq, "%s\n", labels_ns(label)->base.name); end_current_label_crit_section(label); return 0; From b5beb07ad32ab533027aa988d96a44965ec116f7 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 9 Feb 2018 04:57:39 -0800 Subject: [PATCH 02/36] apparmor: fix resource audit messages when auditing peer Resource auditing is using the peer field which is not available when the rlim data struct is used, because it is a different element of the same union. Accessing peer during resource auditing could cause garbage log entries or even oops the kernel. Move the rlim data block into the same struct as the peer field so they can be used together. CC: Fixes: 86b92cb782b3 ("apparmor: move resource checks to using labels") Signed-off-by: John Johansen --- security/apparmor/include/audit.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/security/apparmor/include/audit.h b/security/apparmor/include/audit.h index 4ac095118717..2ebc00a579fd 100644 --- a/security/apparmor/include/audit.h +++ b/security/apparmor/include/audit.h @@ -126,6 +126,10 @@ struct apparmor_audit_data { const char *target; kuid_t ouid; } fs; + struct { + int rlim; + unsigned long max; + } rlim; int signal; }; }; @@ -134,10 +138,6 @@ struct apparmor_audit_data { const char *ns; long pos; } iface; - struct { - int rlim; - unsigned long max; - } rlim; struct { const char *src_name; const char *type; From 98cf5bbff413eadf1b9cb195a7b80cc61c72a50e Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 1 Feb 2018 11:24:10 +0100 Subject: [PATCH 03/36] apparmor: fix logging of the existence test for signals The existence test is not being properly logged as the signal mapping maps it to the last entry in the named signal table. This is done to help catch bugs by making the 0 mapped signal value invalid so that we can catch the signal value not being filled in. When fixing the off-by-one comparision logic the reporting of the existence test was broken, because the logic behind the mapped named table was hidden. Fix this by adding a define for the name lookup and using it. Cc: Stable Fixes: f7dc4c9a855a1 ("apparmor: fix off-by-one comparison on MAXMAPPED_SIG") Signed-off-by: John Johansen --- security/apparmor/include/sig_names.h | 4 +++- security/apparmor/ipc.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/security/apparmor/include/sig_names.h b/security/apparmor/include/sig_names.h index 92e62fe95292..5ca47c50dfa7 100644 --- a/security/apparmor/include/sig_names.h +++ b/security/apparmor/include/sig_names.h @@ -2,6 +2,8 @@ #define SIGUNKNOWN 0 #define MAXMAPPED_SIG 35 +#define MAXMAPPED_SIGNAME (MAXMAPPED_SIG + 1) + /* provide a mapping of arch signal to internal signal # for mediation * those that are always an alias SIGCLD for SIGCLHD and SIGPOLL for SIGIO * map to the same entry those that may/or may not get a separate entry @@ -56,7 +58,7 @@ static const int sig_map[MAXMAPPED_SIG] = { }; /* this table is ordered post sig_map[sig] mapping */ -static const char *const sig_names[MAXMAPPED_SIG + 1] = { +static const char *const sig_names[MAXMAPPED_SIGNAME] = { "unknown", "hup", "int", diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c index b40678f3c1d5..586facd35f7c 100644 --- a/security/apparmor/ipc.c +++ b/security/apparmor/ipc.c @@ -174,7 +174,7 @@ static void audit_signal_cb(struct audit_buffer *ab, void *va) audit_signal_mask(ab, aad(sa)->denied); } } - if (aad(sa)->signal < MAXMAPPED_SIG) + if (aad(sa)->signal < MAXMAPPED_SIGNAME) audit_log_format(ab, " signal=%s", sig_names[aad(sa)->signal]); else audit_log_format(ab, " signal=rtmin+%d", From a6a52579e52b55448326db88bd9a5740e7c1a037 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sat, 3 Feb 2018 20:08:28 +0100 Subject: [PATCH 04/36] apparmor: split load data into management struct and data blob Splitting the management struct from the actual data blob will allow us in the future to do some sharing and other data reduction techniques like replacing the the raw data with compressed data. Prepare for this by separating the management struct from the data blob. Signed-off-by: John Johansen --- security/apparmor/include/policy_unpack.h | 2 +- security/apparmor/policy_unpack.c | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/security/apparmor/include/policy_unpack.h b/security/apparmor/include/policy_unpack.h index be6cd69ac319..8db4ab759e80 100644 --- a/security/apparmor/include/policy_unpack.h +++ b/security/apparmor/include/policy_unpack.h @@ -70,7 +70,7 @@ struct aa_loaddata { int abi; unsigned char *hash; - char data[]; + char *data; }; int aa_unpack(struct aa_loaddata *udata, struct list_head *lh, const char **ns); diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 59a1a25b7d43..ece0c246cfe6 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -164,8 +164,9 @@ static void do_loaddata_free(struct work_struct *work) } kzfree(d->hash); - kfree(d->name); - kvfree(d); + kzfree(d->name); + kvfree(d->data); + kzfree(d); } void aa_loaddata_kref(struct kref *kref) @@ -180,10 +181,16 @@ void aa_loaddata_kref(struct kref *kref) struct aa_loaddata *aa_loaddata_alloc(size_t size) { - struct aa_loaddata *d = kvzalloc(sizeof(*d) + size, GFP_KERNEL); + struct aa_loaddata *d; + d = kzalloc(sizeof(*d), GFP_KERNEL); if (d == NULL) return ERR_PTR(-ENOMEM); + d->data = kvzalloc(size, GFP_KERNEL); + if (!d->data) { + kfree(d); + return ERR_PTR(-ENOMEM); + } kref_init(&d->count); INIT_LIST_HEAD(&d->list); From cf65fabc2a2c8c12031678d86a2bd4a660865011 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 6 Sep 2017 02:53:15 -0700 Subject: [PATCH 05/36] apparmor: add first substr match to dfa Signed-off-by: John Johansen Acked-by: Seth Arnold --- security/apparmor/include/match.h | 4 + security/apparmor/match.c | 120 ++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h index add4c6726558..72b9b89670e6 100644 --- a/security/apparmor/include/match.h +++ b/security/apparmor/include/match.h @@ -129,6 +129,10 @@ unsigned int aa_dfa_match(struct aa_dfa *dfa, unsigned int start, const char *str); unsigned int aa_dfa_next(struct aa_dfa *dfa, unsigned int state, const char c); +unsigned int aa_dfa_match_until(struct aa_dfa *dfa, unsigned int start, + const char *str, const char **retpos); +unsigned int aa_dfa_matchn_until(struct aa_dfa *dfa, unsigned int start, + const char *str, int n, const char **retpos); void aa_dfa_free_kref(struct kref *kref); diff --git a/security/apparmor/match.c b/security/apparmor/match.c index 72c604350e80..6c6dc1a22f9a 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -457,3 +457,123 @@ unsigned int aa_dfa_next(struct aa_dfa *dfa, unsigned int state, return state; } + +/** + * aa_dfa_match_until - traverse @dfa until accept state or end of input + * @dfa: the dfa to match @str against (NOT NULL) + * @start: the state of the dfa to start matching in + * @str: the null terminated string of bytes to match against the dfa (NOT NULL) + * @retpos: first character in str after match OR end of string + * + * aa_dfa_match will match @str against the dfa and return the state it + * finished matching in. The final state can be used to look up the accepting + * label, or as the start state of a continuing match. + * + * Returns: final state reached after input is consumed + */ +unsigned int aa_dfa_match_until(struct aa_dfa *dfa, unsigned int start, + const char *str, const char **retpos) +{ + u16 *def = DEFAULT_TABLE(dfa); + u32 *base = BASE_TABLE(dfa); + u16 *next = NEXT_TABLE(dfa); + u16 *check = CHECK_TABLE(dfa); + u32 *accept = ACCEPT_TABLE(dfa); + unsigned int state = start, pos; + + if (state == 0) + return 0; + + /* current state is , matching character *str */ + if (dfa->tables[YYTD_ID_EC]) { + /* Equivalence class table defined */ + u8 *equiv = EQUIV_TABLE(dfa); + /* default is direct to next state */ + while (*str) { + pos = base_idx(base[state]) + equiv[(u8) *str++]; + if (check[pos] == state) + state = next[pos]; + else + state = def[state]; + if (accept[state]) + break; + } + } else { + /* default is direct to next state */ + while (*str) { + pos = base_idx(base[state]) + (u8) *str++; + if (check[pos] == state) + state = next[pos]; + else + state = def[state]; + if (accept[state]) + break; + } + } + + *retpos = str; + return state; +} + + +/** + * aa_dfa_matchn_until - traverse @dfa until accept or @n bytes consumed + * @dfa: the dfa to match @str against (NOT NULL) + * @start: the state of the dfa to start matching in + * @str: the string of bytes to match against the dfa (NOT NULL) + * @n: length of the string of bytes to match + * @retpos: first character in str after match OR str + n + * + * aa_dfa_match_len will match @str against the dfa and return the state it + * finished matching in. The final state can be used to look up the accepting + * label, or as the start state of a continuing match. + * + * This function will happily match again the 0 byte and only finishes + * when @n input is consumed. + * + * Returns: final state reached after input is consumed + */ +unsigned int aa_dfa_matchn_until(struct aa_dfa *dfa, unsigned int start, + const char *str, int n, const char **retpos) +{ + u16 *def = DEFAULT_TABLE(dfa); + u32 *base = BASE_TABLE(dfa); + u16 *next = NEXT_TABLE(dfa); + u16 *check = CHECK_TABLE(dfa); + u32 *accept = ACCEPT_TABLE(dfa); + unsigned int state = start, pos; + + *retpos = NULL; + if (state == 0) + return 0; + + /* current state is , matching character *str */ + if (dfa->tables[YYTD_ID_EC]) { + /* Equivalence class table defined */ + u8 *equiv = EQUIV_TABLE(dfa); + /* default is direct to next state */ + for (; n; n--) { + pos = base_idx(base[state]) + equiv[(u8) *str++]; + if (check[pos] == state) + state = next[pos]; + else + state = def[state]; + if (accept[state]) + break; + } + } else { + /* default is direct to next state */ + for (; n; n--) { + pos = base_idx(base[state]) + (u8) *str++; + if (check[pos] == state) + state = next[pos]; + else + state = def[state]; + if (accept[state]) + break; + } + } + + *retpos = str; + return state; +} From 6e0654d20ed9679cbf75a0ff7cd786e364f7f09a Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 6 Sep 2017 14:57:59 -0700 Subject: [PATCH 06/36] apparmor: use the dfa to do label parse string splitting The current split scheme is actually wrong in that it splits ///& where that is invalid and should fail. Use the dfa to do a proper bounded split without having to worry about getting the string processing right in code. Signed-off-by: John Johansen Acked-by: Seth Arnold --- security/apparmor/include/label.h | 25 +++++++ security/apparmor/include/match.h | 1 + security/apparmor/label.c | 12 +-- security/apparmor/match.c | 29 ++++++-- security/apparmor/stacksplitdfa.in | 114 +++++++++++++++++++++++++++++ 5 files changed, 170 insertions(+), 11 deletions(-) create mode 100644 security/apparmor/stacksplitdfa.in diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h index af22dcbbcb8a..80e9ba9d172c 100644 --- a/security/apparmor/include/label.h +++ b/security/apparmor/include/label.h @@ -330,6 +330,31 @@ void aa_label_printk(struct aa_label *label, gfp_t gfp); struct aa_label *aa_label_parse(struct aa_label *base, const char *str, gfp_t gfp, bool create, bool force_stack); +static inline const char *aa_label_strn_split(const char *str, int n) +{ + const char *pos; + unsigned int state; + + state = aa_dfa_matchn_until(stacksplitdfa, DFA_START, str, n, &pos); + if (!ACCEPT_TABLE(stacksplitdfa)[state]) + return NULL; + + return pos - 3; +} + +static inline const char *aa_label_str_split(const char *str) +{ + const char *pos; + unsigned int state; + + state = aa_dfa_match_until(stacksplitdfa, DFA_START, str, &pos); + if (!ACCEPT_TABLE(stacksplitdfa)[state]) + return NULL; + + return pos - 3; +} + + struct aa_perms; int aa_label_match(struct aa_profile *profile, struct aa_label *label, diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h index 72b9b89670e6..cd8aeab6ac57 100644 --- a/security/apparmor/include/match.h +++ b/security/apparmor/include/match.h @@ -101,6 +101,7 @@ struct aa_dfa { }; extern struct aa_dfa *nulldfa; +extern struct aa_dfa *stacksplitdfa; #define byte_to_byte(X) (X) diff --git a/security/apparmor/label.c b/security/apparmor/label.c index 324fe5c60f87..31e2f701d971 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -1815,7 +1815,9 @@ static int label_count_str_entries(const char *str) AA_BUG(!str); - for (split = strstr(str, "//&"); split; split = strstr(str, "//&")) { + for (split = aa_label_str_split(str); + split; + split = aa_label_str_split(str)) { count++; str = split + 3; } @@ -1859,7 +1861,7 @@ struct aa_label *aa_label_parse(struct aa_label *base, const char *str, DEFINE_VEC(profile, vec); struct aa_label *label, *currbase = base; int i, len, stack = 0, error; - char *split; + const char *split; AA_BUG(!base); AA_BUG(!str); @@ -1883,7 +1885,8 @@ struct aa_label *aa_label_parse(struct aa_label *base, const char *str, for (i = 0; i < stack; i++) vec[i] = aa_get_profile(base->vec[i]); - for (split = strstr(str, "//&"), i = stack; split && i < len; i++) { + for (split = aa_label_str_split(str), i = stack; + split && i < len; i++) { vec[i] = fqlookupn_profile(base, currbase, str, split - str); if (!vec[i]) goto fail; @@ -1894,7 +1897,7 @@ struct aa_label *aa_label_parse(struct aa_label *base, const char *str, if (vec[i]->ns != labels_ns(currbase)) currbase = &vec[i]->label; str = split + 3; - split = strstr(str, "//&"); + split = aa_label_str_split(str); } /* last element doesn't have a split */ if (i < len) { @@ -1930,7 +1933,6 @@ struct aa_label *aa_label_parse(struct aa_label *base, const char *str, goto out; } - /** * aa_labelset_destroy - remove all labels from the label set * @ls: label set to cleanup (NOT NULL) diff --git a/security/apparmor/match.c b/security/apparmor/match.c index 6c6dc1a22f9a..5d95caeddebc 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -30,6 +30,11 @@ static char nulldfa_src[] = { }; struct aa_dfa *nulldfa; +static char stacksplitdfa_src[] = { + #include "stacksplitdfa.in" +}; +struct aa_dfa *stacksplitdfa; + int aa_setup_dfa_engine(void) { int error; @@ -37,19 +42,31 @@ int aa_setup_dfa_engine(void) nulldfa = aa_dfa_unpack(nulldfa_src, sizeof(nulldfa_src), TO_ACCEPT1_FLAG(YYTD_DATA32) | TO_ACCEPT2_FLAG(YYTD_DATA32)); - if (!IS_ERR(nulldfa)) - return 0; + if (IS_ERR(nulldfa)) { + error = PTR_ERR(nulldfa); + nulldfa = NULL; + return error; + } - error = PTR_ERR(nulldfa); - nulldfa = NULL; + stacksplitdfa = aa_dfa_unpack(stacksplitdfa_src, + sizeof(stacksplitdfa_src), + TO_ACCEPT1_FLAG(YYTD_DATA32) | + TO_ACCEPT2_FLAG(YYTD_DATA32)); + if (IS_ERR(stacksplitdfa)) { + aa_put_dfa(nulldfa); + nulldfa = NULL; + error = PTR_ERR(stacksplitdfa); + stacksplitdfa = NULL; + return error; + } - return error; + return 0; } void aa_teardown_dfa_engine(void) { + aa_put_dfa(stacksplitdfa); aa_put_dfa(nulldfa); - nulldfa = NULL; } /** diff --git a/security/apparmor/stacksplitdfa.in b/security/apparmor/stacksplitdfa.in new file mode 100644 index 000000000000..4bddd10b62a9 --- /dev/null +++ b/security/apparmor/stacksplitdfa.in @@ -0,0 +1,114 @@ +/* 0x1 [^\000]*[^/\000]//& */ 0x1B, 0x5E, 0x78, 0x3D, 0x00, 0x00, +0x00, 0x18, 0x00, 0x00, 0x04, 0xD8, 0x00, 0x00, 0x6E, 0x6F, 0x74, +0x66, 0x6C, 0x65, 0x78, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x04, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, +0x00, 0x00, 0x00, 0x00, 0x07, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x02, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x06, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, +0x02, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, +0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x02, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x06, 0x00, 0x00, 0x00, 0x02, +0x00, 0x02, 0x00, 0x02, 0x00, 0x02, 0x00, 0x02, 0x00, 0x08, 0x00, +0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x05, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x03, 0x00, +0x04, 0x00, 0x01, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x02, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x01, 0x05, 0x00, 0x00, 0x00, 0x01, 0x00, +0x02, 0x00, 0x03, 0x00, 0x04, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x03, 0x00, 0x04, +0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00 From 95652cac83605d96cf3849e80e3e3f4dce74f5da Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 6 Sep 2017 16:33:56 -0700 Subject: [PATCH 07/36] apparmor: provide a bounded version of label_parse some label/context sources might not be guaranteed to be null terminiated provide a size bounded version of label parse to deal with these. Signed-off-by: John Johansen Acked-by: Seth Arnold --- security/apparmor/include/label.h | 3 +++ security/apparmor/label.c | 35 +++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/security/apparmor/include/label.h b/security/apparmor/include/label.h index 80e9ba9d172c..d871e7ff0952 100644 --- a/security/apparmor/include/label.h +++ b/security/apparmor/include/label.h @@ -327,6 +327,9 @@ void aa_label_audit(struct audit_buffer *ab, struct aa_label *label, gfp_t gfp); void aa_label_seq_print(struct seq_file *f, struct aa_label *label, gfp_t gfp); void aa_label_printk(struct aa_label *label, gfp_t gfp); +struct aa_label *aa_label_strn_parse(struct aa_label *base, const char *str, + size_t n, gfp_t gfp, bool create, + bool force_stack); struct aa_label *aa_label_parse(struct aa_label *base, const char *str, gfp_t gfp, bool create, bool force_stack); diff --git a/security/apparmor/label.c b/security/apparmor/label.c index 31e2f701d971..4721338ad551 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -1808,16 +1808,17 @@ void aa_label_printk(struct aa_label *label, gfp_t gfp) aa_put_ns(ns); } -static int label_count_str_entries(const char *str) +static int label_count_strn_entries(const char *str, size_t n) { + const char *end = str + n; const char *split; int count = 1; AA_BUG(!str); - for (split = aa_label_str_split(str); + for (split = aa_label_strn_split(str, end - str); split; - split = aa_label_str_split(str)) { + split = aa_label_strn_split(str, end - str)) { count++; str = split + 3; } @@ -1845,9 +1846,10 @@ static struct aa_profile *fqlookupn_profile(struct aa_label *base, } /** - * aa_label_parse - parse, validate and convert a text string to a label + * aa_label_strn_parse - parse, validate and convert a text string to a label * @base: base label to use for lookups (NOT NULL) * @str: null terminated text string (NOT NULL) + * @n: length of str to parse, will stop at \0 if encountered before n * @gfp: allocation type * @create: true if should create compound labels if they don't exist * @force_stack: true if should stack even if no leading & @@ -1855,19 +1857,23 @@ static struct aa_profile *fqlookupn_profile(struct aa_label *base, * Returns: the matching refcounted label if present * else ERRPTR */ -struct aa_label *aa_label_parse(struct aa_label *base, const char *str, - gfp_t gfp, bool create, bool force_stack) +struct aa_label *aa_label_strn_parse(struct aa_label *base, const char *str, + size_t n, gfp_t gfp, bool create, + bool force_stack) { DEFINE_VEC(profile, vec); struct aa_label *label, *currbase = base; int i, len, stack = 0, error; + const char *end = str + n; const char *split; AA_BUG(!base); AA_BUG(!str); - str = skip_spaces(str); - len = label_count_str_entries(str); + str = skipn_spaces(str, n); + if (str == NULL) + return ERR_PTR(-EINVAL); + len = label_count_strn_entries(str, end - str); if (*str == '&' || force_stack) { /* stack on top of base */ stack = base->size; @@ -1885,7 +1891,7 @@ struct aa_label *aa_label_parse(struct aa_label *base, const char *str, for (i = 0; i < stack; i++) vec[i] = aa_get_profile(base->vec[i]); - for (split = aa_label_str_split(str), i = stack; + for (split = aa_label_strn_split(str, end - str), i = stack; split && i < len; i++) { vec[i] = fqlookupn_profile(base, currbase, str, split - str); if (!vec[i]) @@ -1897,11 +1903,11 @@ struct aa_label *aa_label_parse(struct aa_label *base, const char *str, if (vec[i]->ns != labels_ns(currbase)) currbase = &vec[i]->label; str = split + 3; - split = aa_label_str_split(str); + split = aa_label_strn_split(str, end - str); } /* last element doesn't have a split */ if (i < len) { - vec[i] = fqlookupn_profile(base, currbase, str, strlen(str)); + vec[i] = fqlookupn_profile(base, currbase, str, end - str); if (!vec[i]) goto fail; } @@ -1933,6 +1939,13 @@ struct aa_label *aa_label_parse(struct aa_label *base, const char *str, goto out; } +struct aa_label *aa_label_parse(struct aa_label *base, const char *str, + gfp_t gfp, bool create, bool force_stack) +{ + return aa_label_strn_parse(base, str, strlen(str), gfp, create, + force_stack); +} + /** * aa_labelset_destroy - remove all labels from the label set * @ls: label set to cleanup (NOT NULL) From 71fa373b784e13eb8d68f41b68b9482241e5288c Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 11 Sep 2017 12:57:39 -0700 Subject: [PATCH 08/36] apparmor: cleanup add proper line wrapping to nulldfa.in nulldfa.in makes for a very long unwrapped line, which certain tools do not like. So add line breaks. Signed-off-by: John Johansen --- security/apparmor/nulldfa.in | 108 ++++++++++++++++++++++++++++++++++- 1 file changed, 107 insertions(+), 1 deletion(-) diff --git a/security/apparmor/nulldfa.in b/security/apparmor/nulldfa.in index 3cb38022902e..095f42a24cbc 100644 --- a/security/apparmor/nulldfa.in +++ b/security/apparmor/nulldfa.in @@ -1 +1,107 @@ -0x1B, 0x5E, 0x78, 0x3D, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x04, 0x90, 0x00, 0x00, 0x6E, 0x6F, 0x74, 0x66, 0x6C, 0x65, 0x78, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x07, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 +0x1B, 0x5E, 0x78, 0x3D, 0x00, 0x00, 0x00, 0x18, 0x00, 0x00, 0x04, +0x90, 0x00, 0x00, 0x6E, 0x6F, 0x74, 0x66, 0x6C, 0x65, 0x78, 0x00, +0x00, 0x00, 0x00, 0x01, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x07, 0x00, 0x04, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x04, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, +0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, +0x00, 0x00, 0x00, 0x08, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x03, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00 From 475bdda1f00074783e18403f3f38a36dd3488430 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 8 Sep 2017 01:13:41 -0700 Subject: [PATCH 09/36] apparmor: root view labels should not be under user control The root view of the label parse should not be exposed to user control. Signed-off-by: John Johansen Acked-by: Seth Arnold --- security/apparmor/label.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/security/apparmor/label.c b/security/apparmor/label.c index 4721338ad551..69c7451becef 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -1871,8 +1871,9 @@ struct aa_label *aa_label_strn_parse(struct aa_label *base, const char *str, AA_BUG(!str); str = skipn_spaces(str, n); - if (str == NULL) + if (str == NULL || (*str == '=' && base != &root_ns->unconfined->label)) return ERR_PTR(-EINVAL); + len = label_count_strn_entries(str, end - str); if (*str == '&' || force_stack) { /* stack on top of base */ @@ -1881,8 +1882,6 @@ struct aa_label *aa_label_strn_parse(struct aa_label *base, const char *str, if (*str == '&') str++; } - if (*str == '=') - base = &root_ns->unconfined->label; error = vec_setup(profile, vec, len, gfp); if (error) From 1d6583d9c6723d78e446dd203ffd974f6b85ab76 Mon Sep 17 00:00:00 2001 From: Pravin Shedge Date: Wed, 6 Dec 2017 23:05:59 +0530 Subject: [PATCH 10/36] security: apparmor: remove duplicate includes These duplicate includes have been found with scripts/checkincludes.pl but they have been removed manually to avoid removing false positives. Signed-off-by: Pravin Shedge Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 1 - 1 file changed, 1 deletion(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index a23b0ca19fd0..00fc4f9f7f14 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -33,7 +33,6 @@ #include "include/context.h" #include "include/crypto.h" #include "include/ipc.h" -#include "include/policy_ns.h" #include "include/label.h" #include "include/policy.h" #include "include/policy_ns.h" From 3dc6b1ce6861ebf40b68ab4b752a05584a1f99bf Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 12 Dec 2017 01:02:13 -0800 Subject: [PATCH 11/36] apparmor: make signal label match work when matching stacked labels Given a label with a profile stack of A//&B or A//&C ... A ptrace rule should be able to specify a generic trace pattern with a rule like signal send A//&**, however this is failing because while the correct label match routine is called, it is being done post label decomposition so it is always being done against a profile instead of the stacked label. To fix this refactor the cross check to pass the full peer label in to the label_match. Signed-off-by: John Johansen --- security/apparmor/ipc.c | 40 ++++++++++++---------------------------- 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c index 586facd35f7c..754f2ff8d355 100644 --- a/security/apparmor/ipc.c +++ b/security/apparmor/ipc.c @@ -184,50 +184,34 @@ static void audit_signal_cb(struct audit_buffer *ab, void *va) FLAGS_NONE, GFP_ATOMIC); } -/* TODO: update to handle compound name&name2, conditionals */ -static void profile_match_signal(struct aa_profile *profile, const char *label, - int signal, struct aa_perms *perms) -{ - unsigned int state; - - /* TODO: secondary cache check */ - state = aa_dfa_next(profile->policy.dfa, - profile->policy.start[AA_CLASS_SIGNAL], - signal); - state = aa_dfa_match(profile->policy.dfa, state, label); - aa_compute_perms(profile->policy.dfa, state, perms); -} - static int profile_signal_perm(struct aa_profile *profile, - struct aa_profile *peer, u32 request, + struct aa_label *peer, u32 request, struct common_audit_data *sa) { struct aa_perms perms; + unsigned int state; if (profile_unconfined(profile) || !PROFILE_MEDIATES(profile, AA_CLASS_SIGNAL)) return 0; - aad(sa)->peer = &peer->label; - profile_match_signal(profile, peer->base.hname, aad(sa)->signal, - &perms); + aad(sa)->peer = peer; + /* TODO: secondary cache check */ + state = aa_dfa_next(profile->policy.dfa, + profile->policy.start[AA_CLASS_SIGNAL], + aad(sa)->signal); + aa_label_match(profile, peer, state, false, request, &perms); aa_apply_modes_to_perms(profile, &perms); return aa_check_perms(profile, &perms, request, sa, audit_signal_cb); } -static int aa_signal_cross_perm(struct aa_profile *sender, - struct aa_profile *target, - struct common_audit_data *sa) -{ - return xcheck(profile_signal_perm(sender, target, MAY_WRITE, sa), - profile_signal_perm(target, sender, MAY_READ, sa)); -} - int aa_may_signal(struct aa_label *sender, struct aa_label *target, int sig) { + struct aa_profile *profile; DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_NONE, OP_SIGNAL); aad(&sa)->signal = map_signal_num(sig); - return xcheck_labels_profiles(sender, target, aa_signal_cross_perm, - &sa); + return xcheck_labels(sender, target, profile, + profile_signal_perm(profile, target, MAY_WRITE, &sa), + profile_signal_perm(profile, sender, MAY_READ, &sa)); } From 3acfd5f54ca16c15c36ac2f218357f2707b7edb8 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 1 Feb 2018 12:32:02 +0100 Subject: [PATCH 12/36] apparmor: audit unknown signal numbers Allow apparmor to audit the number of a signal that it does not provide a mapping for and is currently being reported only as unknown. Signed-off-by: John Johansen --- security/apparmor/include/audit.h | 5 ++++- security/apparmor/include/sig_names.h | 1 + security/apparmor/ipc.c | 10 +++++++--- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/security/apparmor/include/audit.h b/security/apparmor/include/audit.h index 2ebc00a579fd..41ad2c947bf4 100644 --- a/security/apparmor/include/audit.h +++ b/security/apparmor/include/audit.h @@ -130,7 +130,10 @@ struct apparmor_audit_data { int rlim; unsigned long max; } rlim; - int signal; + struct { + int signal; + int unmappedsig; + }; }; }; struct { diff --git a/security/apparmor/include/sig_names.h b/security/apparmor/include/sig_names.h index 5ca47c50dfa7..cbf7a997ed84 100644 --- a/security/apparmor/include/sig_names.h +++ b/security/apparmor/include/sig_names.h @@ -3,6 +3,7 @@ #define SIGUNKNOWN 0 #define MAXMAPPED_SIG 35 #define MAXMAPPED_SIGNAME (MAXMAPPED_SIG + 1) +#define SIGRT_BASE 128 /* provide a mapping of arch signal to internal signal # for mediation * those that are always an alias SIGCLD for SIGCLHD and SIGPOLL for SIGIO diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c index 754f2ff8d355..d7b137d4eb74 100644 --- a/security/apparmor/ipc.c +++ b/security/apparmor/ipc.c @@ -138,7 +138,7 @@ static inline int map_signal_num(int sig) if (sig > SIGRTMAX) return SIGUNKNOWN; else if (sig >= SIGRTMIN) - return sig - SIGRTMIN + 128; /* rt sigs mapped to 128 */ + return sig - SIGRTMIN + SIGRT_BASE; else if (sig < MAXMAPPED_SIG) return sig_map[sig]; return SIGUNKNOWN; @@ -174,11 +174,14 @@ static void audit_signal_cb(struct audit_buffer *ab, void *va) audit_signal_mask(ab, aad(sa)->denied); } } - if (aad(sa)->signal < MAXMAPPED_SIGNAME) + if (aad(sa)->signal == SIGUNKNOWN) + audit_log_format(ab, "signal=unknown(%d)", + aad(sa)->unmappedsig); + else if (aad(sa)->signal < MAXMAPPED_SIGNAME) audit_log_format(ab, " signal=%s", sig_names[aad(sa)->signal]); else audit_log_format(ab, " signal=rtmin+%d", - aad(sa)->signal - 128); + aad(sa)->signal - SIGRT_BASE); audit_log_format(ab, " peer="); aa_label_xaudit(ab, labels_ns(aad(sa)->label), aad(sa)->peer, FLAGS_NONE, GFP_ATOMIC); @@ -211,6 +214,7 @@ int aa_may_signal(struct aa_label *sender, struct aa_label *target, int sig) DEFINE_AUDIT_DATA(sa, LSM_AUDIT_DATA_NONE, OP_SIGNAL); aad(&sa)->signal = map_signal_num(sig); + aad(&sa)->unmappedsig = sig; return xcheck_labels(sender, target, profile, profile_signal_perm(profile, target, MAY_WRITE, &sa), profile_signal_perm(profile, sender, MAY_READ, &sa)); From 4d2f8ba3e3b76e34f84ae1de456934713e9e59af Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 19 Jan 2017 14:08:36 -0800 Subject: [PATCH 13/36] apparmor: rename task_ctx to the more accurate cred_ctx Signed-off-by: John Johansen --- security/apparmor/context.c | 38 ++++++++++++++--------------- security/apparmor/domain.c | 6 ++--- security/apparmor/include/context.h | 19 +++++++-------- security/apparmor/lsm.c | 26 ++++++++++---------- security/apparmor/policy.c | 2 +- 5 files changed, 45 insertions(+), 46 deletions(-) diff --git a/security/apparmor/context.c b/security/apparmor/context.c index c95f1ac6190b..89c03053303e 100644 --- a/security/apparmor/context.c +++ b/security/apparmor/context.c @@ -13,11 +13,11 @@ * License. * * - * AppArmor sets confinement on every task, via the the aa_task_ctx and - * the aa_task_ctx.label, both of which are required and are not allowed - * to be NULL. The aa_task_ctx is not reference counted and is unique + * AppArmor sets confinement on every task, via the the aa_cred_ctx and + * the aa_cred_ctx.label, both of which are required and are not allowed + * to be NULL. The aa_cred_ctx is not reference counted and is unique * to each cred (which is reference count). The label pointed to by - * the task_ctx is reference counted. + * the cred_ctx is reference counted. * * TODO * If a task uses change_hat it currently does not return to the old @@ -30,21 +30,21 @@ #include "include/policy.h" /** - * aa_alloc_task_context - allocate a new task_ctx + * aa_alloc_cred_ctx - allocate a new cred_ctx * @flags: gfp flags for allocation * * Returns: allocated buffer or NULL on failure */ -struct aa_task_ctx *aa_alloc_task_context(gfp_t flags) +struct aa_cred_ctx *aa_alloc_cred_ctx(gfp_t flags) { - return kzalloc(sizeof(struct aa_task_ctx), flags); + return kzalloc(sizeof(struct aa_cred_ctx), flags); } /** - * aa_free_task_context - free a task_ctx - * @ctx: task_ctx to free (MAYBE NULL) + * aa_free_cred_ctx - free a cred_ctx + * @ctx: cred_ctx to free (MAYBE NULL) */ -void aa_free_task_context(struct aa_task_ctx *ctx) +void aa_free_cred_ctx(struct aa_cred_ctx *ctx) { if (ctx) { aa_put_label(ctx->label); @@ -56,11 +56,11 @@ void aa_free_task_context(struct aa_task_ctx *ctx) } /** - * aa_dup_task_context - duplicate a task context, incrementing reference counts + * aa_dup_cred_ctx - duplicate a task context, incrementing reference counts * @new: a blank task context (NOT NULL) * @old: the task context to copy (NOT NULL) */ -void aa_dup_task_context(struct aa_task_ctx *new, const struct aa_task_ctx *old) +void aa_dup_cred_ctx(struct aa_cred_ctx *new, const struct aa_cred_ctx *old) { *new = *old; aa_get_label(new->label); @@ -93,7 +93,7 @@ struct aa_label *aa_get_task_label(struct task_struct *task) */ int aa_replace_current_label(struct aa_label *label) { - struct aa_task_ctx *ctx = current_ctx(); + struct aa_cred_ctx *ctx = current_cred_ctx(); struct cred *new; AA_BUG(!label); @@ -112,7 +112,7 @@ int aa_replace_current_label(struct aa_label *label) /* if switching to unconfined or a different label namespace * clear out context state */ - aa_clear_task_ctx_trans(ctx); + aa_clear_cred_ctx_trans(ctx); /* * be careful switching ctx->profile, when racing replacement it @@ -136,14 +136,14 @@ int aa_replace_current_label(struct aa_label *label) */ int aa_set_current_onexec(struct aa_label *label, bool stack) { - struct aa_task_ctx *ctx; + struct aa_cred_ctx *ctx; struct cred *new = prepare_creds(); if (!new) return -ENOMEM; ctx = cred_ctx(new); aa_get_label(label); - aa_clear_task_ctx_trans(ctx); + aa_clear_cred_ctx_trans(ctx); ctx->onexec = label; ctx->token = stack; @@ -163,7 +163,7 @@ int aa_set_current_onexec(struct aa_label *label, bool stack) */ int aa_set_current_hat(struct aa_label *label, u64 token) { - struct aa_task_ctx *ctx; + struct aa_cred_ctx *ctx; struct cred *new = prepare_creds(); if (!new) return -ENOMEM; @@ -201,7 +201,7 @@ int aa_set_current_hat(struct aa_label *label, u64 token) */ int aa_restore_previous_label(u64 token) { - struct aa_task_ctx *ctx; + struct aa_cred_ctx *ctx; struct cred *new = prepare_creds(); if (!new) return -ENOMEM; @@ -221,7 +221,7 @@ int aa_restore_previous_label(u64 token) ctx->label = aa_get_newest_label(ctx->previous); AA_BUG(!ctx->label); /* clear exec && prev information when restoring to previous context */ - aa_clear_task_ctx_trans(ctx); + aa_clear_cred_ctx_trans(ctx); commit_creds(new); return 0; diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 6a54d2ffa840..90967de96be0 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -779,7 +779,7 @@ static struct aa_label *handle_onexec(struct aa_label *label, */ int apparmor_bprm_set_creds(struct linux_binprm *bprm) { - struct aa_task_ctx *ctx; + struct aa_cred_ctx *ctx; struct aa_label *label, *new = NULL; struct aa_profile *profile; char *buffer = NULL; @@ -859,7 +859,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) done: /* clear out temporary/transitional state from the context */ - aa_clear_task_ctx_trans(ctx); + aa_clear_cred_ctx_trans(ctx); aa_put_label(label); put_buffers(buffer); @@ -1049,7 +1049,7 @@ static struct aa_label *change_hat(struct aa_label *label, const char *hats[], int aa_change_hat(const char *hats[], int count, u64 token, int flags) { const struct cred *cred; - struct aa_task_ctx *ctx; + struct aa_cred_ctx *ctx; struct aa_label *label, *previous, *new = NULL, *target = NULL; struct aa_profile *profile; struct aa_perms perms = {}; diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h index 6ae07e9aaa17..0622fcf2a695 100644 --- a/security/apparmor/include/context.h +++ b/security/apparmor/include/context.h @@ -23,10 +23,10 @@ #include "policy_ns.h" #define cred_ctx(X) ((X)->security) -#define current_ctx() cred_ctx(current_cred()) +#define current_cred_ctx() cred_ctx(current_cred()) /** - * struct aa_task_ctx - primary label for confined tasks + * struct aa_cred_ctx - primary label for confined tasks * @label: the current label (NOT NULL) * @exec: label to transition to on next exec (MAYBE NULL) * @previous: label the task may return to (MAYBE NULL) @@ -37,17 +37,16 @@ * * TODO: make so a task can be confined by a stack of contexts */ -struct aa_task_ctx { +struct aa_cred_ctx { struct aa_label *label; struct aa_label *onexec; struct aa_label *previous; u64 token; }; -struct aa_task_ctx *aa_alloc_task_context(gfp_t flags); -void aa_free_task_context(struct aa_task_ctx *ctx); -void aa_dup_task_context(struct aa_task_ctx *new, - const struct aa_task_ctx *old); +struct aa_cred_ctx *aa_alloc_cred_ctx(gfp_t flags); +void aa_free_cred_ctx(struct aa_cred_ctx *ctx); +void aa_dup_cred_ctx(struct aa_cred_ctx *new, const struct aa_cred_ctx *old); int aa_replace_current_label(struct aa_label *label); int aa_set_current_onexec(struct aa_label *label, bool stack); int aa_set_current_hat(struct aa_label *label, u64 token); @@ -65,7 +64,7 @@ struct aa_label *aa_get_task_label(struct task_struct *task); */ static inline struct aa_label *aa_cred_raw_label(const struct cred *cred) { - struct aa_task_ctx *ctx = cred_ctx(cred); + struct aa_cred_ctx *ctx = cred_ctx(cred); AA_BUG(!ctx || !ctx->label); return ctx->label; @@ -214,10 +213,10 @@ static inline struct aa_ns *aa_get_current_ns(void) } /** - * aa_clear_task_ctx_trans - clear transition tracking info from the ctx + * aa_clear_cred_ctx_trans - clear transition tracking info from the ctx * @ctx: task context to clear (NOT NULL) */ -static inline void aa_clear_task_ctx_trans(struct aa_task_ctx *ctx) +static inline void aa_clear_cred_ctx_trans(struct aa_cred_ctx *ctx) { aa_put_label(ctx->previous); aa_put_label(ctx->onexec); diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 9a65eeaf7dfa..0624eb2081f3 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -51,11 +51,11 @@ DEFINE_PER_CPU(struct aa_buffers, aa_buffers); */ /* - * free the associated aa_task_ctx and put its labels + * free the associated aa_cred_ctx and put its labels */ static void apparmor_cred_free(struct cred *cred) { - aa_free_task_context(cred_ctx(cred)); + aa_free_cred_ctx(cred_ctx(cred)); cred_ctx(cred) = NULL; } @@ -65,7 +65,7 @@ static void apparmor_cred_free(struct cred *cred) static int apparmor_cred_alloc_blank(struct cred *cred, gfp_t gfp) { /* freed by apparmor_cred_free */ - struct aa_task_ctx *ctx = aa_alloc_task_context(gfp); + struct aa_cred_ctx *ctx = aa_alloc_cred_ctx(gfp); if (!ctx) return -ENOMEM; @@ -75,18 +75,18 @@ static int apparmor_cred_alloc_blank(struct cred *cred, gfp_t gfp) } /* - * prepare new aa_task_ctx for modification by prepare_cred block + * prepare new aa_cred_ctx for modification by prepare_cred block */ static int apparmor_cred_prepare(struct cred *new, const struct cred *old, gfp_t gfp) { /* freed by apparmor_cred_free */ - struct aa_task_ctx *ctx = aa_alloc_task_context(gfp); + struct aa_cred_ctx *ctx = aa_alloc_cred_ctx(gfp); if (!ctx) return -ENOMEM; - aa_dup_task_context(ctx, cred_ctx(old)); + aa_dup_cred_ctx(ctx, cred_ctx(old)); cred_ctx(new) = ctx; return 0; } @@ -96,10 +96,10 @@ static int apparmor_cred_prepare(struct cred *new, const struct cred *old, */ static void apparmor_cred_transfer(struct cred *new, const struct cred *old) { - const struct aa_task_ctx *old_ctx = cred_ctx(old); - struct aa_task_ctx *new_ctx = cred_ctx(new); + const struct aa_cred_ctx *old_ctx = cred_ctx(old); + struct aa_cred_ctx *new_ctx = cred_ctx(new); - aa_dup_task_context(new_ctx, old_ctx); + aa_dup_cred_ctx(new_ctx, old_ctx); } static int apparmor_ptrace_access_check(struct task_struct *child, @@ -577,7 +577,7 @@ static int apparmor_getprocattr(struct task_struct *task, char *name, int error = -ENOENT; /* released below */ const struct cred *cred = get_task_cred(task); - struct aa_task_ctx *ctx = cred_ctx(cred); + struct aa_cred_ctx *ctx = cred_ctx(cred); struct aa_label *label = NULL; if (strcmp(name, "current") == 0) @@ -678,7 +678,7 @@ static int apparmor_setprocattr(const char *name, void *value, static void apparmor_bprm_committing_creds(struct linux_binprm *bprm) { struct aa_label *label = aa_current_raw_label(); - struct aa_task_ctx *new_ctx = cred_ctx(bprm->cred); + struct aa_cred_ctx *new_ctx = cred_ctx(bprm->cred); /* bail out if unconfined or not changing profile */ if ((new_ctx->label->proxy == label->proxy) || @@ -1024,9 +1024,9 @@ static int param_set_mode(const char *val, const struct kernel_param *kp) static int __init set_init_ctx(void) { struct cred *cred = (struct cred *)current->real_cred; - struct aa_task_ctx *ctx; + struct aa_cred_ctx *ctx; - ctx = aa_alloc_task_context(GFP_KERNEL); + ctx = aa_alloc_cred_ctx(GFP_KERNEL); if (!ctx) return -ENOMEM; diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index b0b58848c248..c505d517fa3c 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -845,7 +845,7 @@ static struct aa_profile *update_to_newest_parent(struct aa_profile *new) * @udata: serialized data stream (NOT NULL) * * unpack and replace a profile on the profile list and uses of that profile - * by any aa_task_ctx. If the profile does not exist on the profile list + * by any aa_cred_ctx. If the profile does not exist on the profile list * it is added. * * Returns: size of data consumed else error code on failure. From 3b529a7600d834f450ac244f43a7c082687284b4 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 20 Jan 2017 01:59:25 -0800 Subject: [PATCH 14/36] apparmor: move task domain change info to task security The task domain change info is task specific and its and abuse of the cred to store the information in there. Now that a task->security field exists store it in the proper place. Signed-off-by: John Johansen --- security/apparmor/context.c | 93 +++++++++++++++++++---------- security/apparmor/domain.c | 14 +++-- security/apparmor/include/context.h | 31 ++++++---- security/apparmor/lsm.c | 48 +++++++++++++-- 4 files changed, 133 insertions(+), 53 deletions(-) diff --git a/security/apparmor/context.c b/security/apparmor/context.c index 89c03053303e..432672b18945 100644 --- a/security/apparmor/context.c +++ b/security/apparmor/context.c @@ -48,8 +48,6 @@ void aa_free_cred_ctx(struct aa_cred_ctx *ctx) { if (ctx) { aa_put_label(ctx->label); - aa_put_label(ctx->previous); - aa_put_label(ctx->onexec); kzfree(ctx); } @@ -64,8 +62,6 @@ void aa_dup_cred_ctx(struct aa_cred_ctx *new, const struct aa_cred_ctx *old) { *new = *old; aa_get_label(new->label); - aa_get_label(new->previous); - aa_get_label(new->onexec); } /** @@ -85,6 +81,43 @@ struct aa_label *aa_get_task_label(struct task_struct *task) return p; } +/** + * aa_alloc_task_ctx - allocate a new task_ctx + * @flags: gfp flags for allocation + * + * Returns: allocated buffer or NULL on failure + */ +struct aa_task_ctx *aa_alloc_task_ctx(gfp_t flags) +{ + return kzalloc(sizeof(struct aa_task_ctx), flags); +} + +/** + * aa_free_task_ctx - free a task_ctx + * @ctx: task_ctx to free (MAYBE NULL) + */ +void aa_free_task_ctx(struct aa_task_ctx *ctx) +{ + if (ctx) { + aa_put_label(ctx->previous); + aa_put_label(ctx->onexec); + + kzfree(ctx); + } +} + +/** + * aa_dup_task_ctx - duplicate a task context, incrementing reference counts + * @new: a blank task context (NOT NULL) + * @old: the task context to copy (NOT NULL) + */ +void aa_dup_task_ctx(struct aa_task_ctx *new, const struct aa_task_ctx *old) +{ + *new = *old; + aa_get_label(new->previous); + aa_get_label(new->onexec); +} + /** * aa_replace_current_label - replace the current tasks label * @label: new label (NOT NULL) @@ -112,7 +145,7 @@ int aa_replace_current_label(struct aa_label *label) /* if switching to unconfined or a different label namespace * clear out context state */ - aa_clear_cred_ctx_trans(ctx); + aa_clear_task_ctx_trans(current_task_ctx()); /* * be careful switching ctx->profile, when racing replacement it @@ -136,18 +169,13 @@ int aa_replace_current_label(struct aa_label *label) */ int aa_set_current_onexec(struct aa_label *label, bool stack) { - struct aa_cred_ctx *ctx; - struct cred *new = prepare_creds(); - if (!new) - return -ENOMEM; + struct aa_task_ctx *ctx = current_task_ctx(); - ctx = cred_ctx(new); aa_get_label(label); - aa_clear_cred_ctx_trans(ctx); + aa_put_label(ctx->onexec); ctx->onexec = label; ctx->token = stack; - commit_creds(new); return 0; } @@ -163,28 +191,31 @@ int aa_set_current_onexec(struct aa_label *label, bool stack) */ int aa_set_current_hat(struct aa_label *label, u64 token) { + struct aa_task_ctx *tctx = current_task_ctx(); struct aa_cred_ctx *ctx; struct cred *new = prepare_creds(); + if (!new) return -ENOMEM; AA_BUG(!label); ctx = cred_ctx(new); - if (!ctx->previous) { + if (!tctx->previous) { /* transfer refcount */ - ctx->previous = ctx->label; - ctx->token = token; - } else if (ctx->token == token) { + tctx->previous = ctx->label; + tctx->token = token; + } else if (tctx->token == token) { aa_put_label(ctx->label); } else { /* previous_profile && ctx->token != token */ abort_creds(new); return -EACCES; } + ctx->label = aa_get_newest_label(label); /* clear exec on switching context */ - aa_put_label(ctx->onexec); - ctx->onexec = NULL; + aa_put_label(tctx->onexec); + tctx->onexec = NULL; commit_creds(new); return 0; @@ -201,28 +232,28 @@ int aa_set_current_hat(struct aa_label *label, u64 token) */ int aa_restore_previous_label(u64 token) { + struct aa_task_ctx *tctx = current_task_ctx(); struct aa_cred_ctx *ctx; - struct cred *new = prepare_creds(); + struct cred *new; + + if (tctx->token != token) + return -EACCES; + /* ignore restores when there is no saved label */ + if (!tctx->previous) + return 0; + + new = prepare_creds(); if (!new) return -ENOMEM; - ctx = cred_ctx(new); - if (ctx->token != token) { - abort_creds(new); - return -EACCES; - } - /* ignore restores when there is no saved label */ - if (!ctx->previous) { - abort_creds(new); - return 0; - } aa_put_label(ctx->label); - ctx->label = aa_get_newest_label(ctx->previous); + ctx->label = aa_get_newest_label(tctx->previous); AA_BUG(!ctx->label); /* clear exec && prev information when restoring to previous context */ - aa_clear_cred_ctx_trans(ctx); + aa_clear_task_ctx_trans(tctx); commit_creds(new); + return 0; } diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 90967de96be0..b90759a765b5 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -780,6 +780,7 @@ static struct aa_label *handle_onexec(struct aa_label *label, int apparmor_bprm_set_creds(struct linux_binprm *bprm) { struct aa_cred_ctx *ctx; + struct aa_task_ctx *tctx; struct aa_label *label, *new = NULL; struct aa_profile *profile; char *buffer = NULL; @@ -795,15 +796,17 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) return 0; ctx = cred_ctx(bprm->cred); + tctx = current_task_ctx(); AA_BUG(!ctx); + AA_BUG(!tctx); label = aa_get_newest_label(ctx->label); /* buffer freed below, name is pointer into buffer */ get_buffers(buffer); /* Test for onexec first as onexec override other x transitions. */ - if (ctx->onexec) - new = handle_onexec(label, ctx->onexec, ctx->token, + if (tctx->onexec) + new = handle_onexec(label, tctx->onexec, tctx->token, bprm, buffer, &cond, &unsafe); else new = fn_label_build(label, profile, GFP_ATOMIC, @@ -858,9 +861,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) ctx->label = new; done: - /* clear out temporary/transitional state from the context */ - aa_clear_cred_ctx_trans(ctx); - aa_put_label(label); put_buffers(buffer); @@ -1050,6 +1050,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) { const struct cred *cred; struct aa_cred_ctx *ctx; + struct aa_task_ctx *tctx; struct aa_label *label, *previous, *new = NULL, *target = NULL; struct aa_profile *profile; struct aa_perms perms = {}; @@ -1070,8 +1071,9 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) /* released below */ cred = get_current_cred(); ctx = cred_ctx(cred); + tctx = current_task_ctx(); label = aa_get_newest_cred_label(cred); - previous = aa_get_newest_label(ctx->previous); + previous = aa_get_newest_label(tctx->previous); if (unconfined(label)) { info = "unconfined can not change_hat"; diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h index 0622fcf2a695..c3b51d88275b 100644 --- a/security/apparmor/include/context.h +++ b/security/apparmor/include/context.h @@ -25,20 +25,24 @@ #define cred_ctx(X) ((X)->security) #define current_cred_ctx() cred_ctx(current_cred()) +#define task_ctx(X) ((X)->security) +#define current_task_ctx() (task_ctx(current)) + /** * struct aa_cred_ctx - primary label for confined tasks * @label: the current label (NOT NULL) - * @exec: label to transition to on next exec (MAYBE NULL) - * @previous: label the task may return to (MAYBE NULL) - * @token: magic value the task must know for returning to @previous - * - * Contains the task's current label (which could change due to - * change_hat). Plus the hat_magic needed during change_hat. - * - * TODO: make so a task can be confined by a stack of contexts */ struct aa_cred_ctx { struct aa_label *label; +}; + +/** + * struct aa_task_ctx - information for current task label change + * @onexec: profile to transition to on next exec (MAY BE NULL) + * @previous: profile the task may return to (MAY BE NULL) + * @token: magic value the task must know for returning to @previous_profile + */ +struct aa_task_ctx { struct aa_label *onexec; struct aa_label *previous; u64 token; @@ -47,6 +51,11 @@ struct aa_cred_ctx { struct aa_cred_ctx *aa_alloc_cred_ctx(gfp_t flags); void aa_free_cred_ctx(struct aa_cred_ctx *ctx); void aa_dup_cred_ctx(struct aa_cred_ctx *new, const struct aa_cred_ctx *old); + +struct aa_task_ctx *aa_alloc_task_ctx(gfp_t flags); +void aa_free_task_ctx(struct aa_task_ctx *ctx); +void aa_dup_task_ctx(struct aa_task_ctx *new, const struct aa_task_ctx *old); + int aa_replace_current_label(struct aa_label *label); int aa_set_current_onexec(struct aa_label *label, bool stack); int aa_set_current_hat(struct aa_label *label, u64 token); @@ -213,11 +222,13 @@ static inline struct aa_ns *aa_get_current_ns(void) } /** - * aa_clear_cred_ctx_trans - clear transition tracking info from the ctx + * aa_clear_task_ctx_trans - clear transition tracking info from the ctx * @ctx: task context to clear (NOT NULL) */ -static inline void aa_clear_cred_ctx_trans(struct aa_cred_ctx *ctx) +static inline void aa_clear_task_ctx_trans(struct aa_task_ctx *ctx) { + AA_BUG(!ctx); + aa_put_label(ctx->previous); aa_put_label(ctx->onexec); ctx->previous = NULL; diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 0624eb2081f3..a1d63d93b862 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -102,6 +102,27 @@ static void apparmor_cred_transfer(struct cred *new, const struct cred *old) aa_dup_cred_ctx(new_ctx, old_ctx); } +static void apparmor_task_free(struct task_struct *task) +{ + + aa_free_task_ctx(task_ctx(task)); + task_ctx(task) = NULL; +} + +static int apparmor_task_alloc(struct task_struct *task, + unsigned long clone_flags) +{ + struct aa_task_ctx *new = aa_alloc_task_ctx(GFP_KERNEL); + + if (!new) + return -ENOMEM; + + aa_dup_task_ctx(new, current_task_ctx()); + task_ctx(task) = new; + + return 0; +} + static int apparmor_ptrace_access_check(struct task_struct *child, unsigned int mode) { @@ -577,15 +598,16 @@ static int apparmor_getprocattr(struct task_struct *task, char *name, int error = -ENOENT; /* released below */ const struct cred *cred = get_task_cred(task); + struct aa_task_ctx *tctx = current_task_ctx(); struct aa_cred_ctx *ctx = cred_ctx(cred); struct aa_label *label = NULL; if (strcmp(name, "current") == 0) label = aa_get_newest_label(ctx->label); - else if (strcmp(name, "prev") == 0 && ctx->previous) - label = aa_get_newest_label(ctx->previous); - else if (strcmp(name, "exec") == 0 && ctx->onexec) - label = aa_get_newest_label(ctx->onexec); + else if (strcmp(name, "prev") == 0 && tctx->previous) + label = aa_get_newest_label(tctx->previous); + else if (strcmp(name, "exec") == 0 && tctx->onexec) + label = aa_get_newest_label(tctx->onexec); else error = -EINVAL; @@ -699,7 +721,9 @@ static void apparmor_bprm_committing_creds(struct linux_binprm *bprm) */ static void apparmor_bprm_committed_creds(struct linux_binprm *bprm) { - /* TODO: cleanup signals - ipc mediation */ + /* clear out temporary/transitional state from the context */ + aa_clear_task_ctx_trans(current_task_ctx()); + return; } @@ -779,6 +803,8 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(bprm_committing_creds, apparmor_bprm_committing_creds), LSM_HOOK_INIT(bprm_committed_creds, apparmor_bprm_committed_creds), + LSM_HOOK_INIT(task_free, apparmor_task_free), + LSM_HOOK_INIT(task_alloc, apparmor_task_alloc), LSM_HOOK_INIT(task_setrlimit, apparmor_task_setrlimit), LSM_HOOK_INIT(task_kill, apparmor_task_kill), }; @@ -1025,15 +1051,25 @@ static int __init set_init_ctx(void) { struct cred *cred = (struct cred *)current->real_cred; struct aa_cred_ctx *ctx; + struct aa_task_ctx *tctx; ctx = aa_alloc_cred_ctx(GFP_KERNEL); if (!ctx) - return -ENOMEM; + goto fail_cred; + tctx = aa_alloc_task_ctx(GFP_KERNEL); + if (!tctx) + goto fail_task; ctx->label = aa_get_label(ns_unconfined(root_ns)); cred_ctx(cred) = ctx; + task_ctx(current) = tctx; return 0; + +fail_task: + aa_free_cred_ctx(ctx); +fail_cred: + return -ENOMEM; } static void destroy_buffers(void) From d9087c49d4388e3f35f09a5cf7ed6e09c9106604 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 27 Jan 2017 03:53:53 -0800 Subject: [PATCH 15/36] apparmor: drop cred_ctx and reference the label directly With the task domain change information now stored in the task->security context, the cred->security context only stores the label. We can get rid of the cred_ctx and directly reference the label, removing a layer of indirection, and unneeded extra allocations. Signed-off-by: John Johansen --- security/apparmor/context.c | 83 ++++++++--------------------- security/apparmor/domain.c | 14 ++--- security/apparmor/include/context.h | 24 ++------- security/apparmor/lsm.c | 55 +++++-------------- 4 files changed, 47 insertions(+), 129 deletions(-) diff --git a/security/apparmor/context.c b/security/apparmor/context.c index 432672b18945..70e4a094add8 100644 --- a/security/apparmor/context.c +++ b/security/apparmor/context.c @@ -13,11 +13,9 @@ * License. * * - * AppArmor sets confinement on every task, via the the aa_cred_ctx and - * the aa_cred_ctx.label, both of which are required and are not allowed - * to be NULL. The aa_cred_ctx is not reference counted and is unique - * to each cred (which is reference count). The label pointed to by - * the cred_ctx is reference counted. + * AppArmor sets confinement on every task, via the cred_label() which + * is required and are not allowed to be NULL. The cred_label is + * reference counted. * * TODO * If a task uses change_hat it currently does not return to the old @@ -29,40 +27,6 @@ #include "include/context.h" #include "include/policy.h" -/** - * aa_alloc_cred_ctx - allocate a new cred_ctx - * @flags: gfp flags for allocation - * - * Returns: allocated buffer or NULL on failure - */ -struct aa_cred_ctx *aa_alloc_cred_ctx(gfp_t flags) -{ - return kzalloc(sizeof(struct aa_cred_ctx), flags); -} - -/** - * aa_free_cred_ctx - free a cred_ctx - * @ctx: cred_ctx to free (MAYBE NULL) - */ -void aa_free_cred_ctx(struct aa_cred_ctx *ctx) -{ - if (ctx) { - aa_put_label(ctx->label); - - kzfree(ctx); - } -} - -/** - * aa_dup_cred_ctx - duplicate a task context, incrementing reference counts - * @new: a blank task context (NOT NULL) - * @old: the task context to copy (NOT NULL) - */ -void aa_dup_cred_ctx(struct aa_cred_ctx *new, const struct aa_cred_ctx *old) -{ - *new = *old; - aa_get_label(new->label); -} /** * aa_get_task_label - Get another task's label @@ -126,11 +90,12 @@ void aa_dup_task_ctx(struct aa_task_ctx *new, const struct aa_task_ctx *old) */ int aa_replace_current_label(struct aa_label *label) { - struct aa_cred_ctx *ctx = current_cred_ctx(); + struct aa_label *old = aa_current_raw_label(); struct cred *new; + AA_BUG(!label); - if (ctx->label == label) + if (old == label) return 0; if (current_cred() != current_real_cred()) @@ -140,22 +105,22 @@ int aa_replace_current_label(struct aa_label *label) if (!new) return -ENOMEM; - ctx = cred_ctx(new); - if (unconfined(label) || (labels_ns(ctx->label) != labels_ns(label))) - /* if switching to unconfined or a different label namespace + if (unconfined(label) || (labels_ns(old) != labels_ns(label))) + /* + * if switching to unconfined or a different label namespace * clear out context state */ aa_clear_task_ctx_trans(current_task_ctx()); /* - * be careful switching ctx->profile, when racing replacement it - * is possible that ctx->profile->proxy->profile is the reference - * keeping @profile valid, so make sure to get its reference before - * dropping the reference on ctx->profile + * be careful switching cred label, when racing replacement it + * is possible that the cred labels's->proxy->label is the reference + * keeping @label valid, so make sure to get its reference before + * dropping the reference on the cred's label */ aa_get_label(label); - aa_put_label(ctx->label); - ctx->label = label; + aa_put_label(cred_label(new)); + cred_label(new) = label; commit_creds(new); return 0; @@ -193,26 +158,26 @@ int aa_set_current_hat(struct aa_label *label, u64 token) { struct aa_task_ctx *tctx = current_task_ctx(); struct aa_cred_ctx *ctx; - struct cred *new = prepare_creds(); + struct cred *new; + new = prepare_creds(); if (!new) return -ENOMEM; AA_BUG(!label); - ctx = cred_ctx(new); if (!tctx->previous) { /* transfer refcount */ - tctx->previous = ctx->label; + tctx->previous = cred_label(new); tctx->token = token; } else if (tctx->token == token) { - aa_put_label(ctx->label); + aa_put_label(cred_label(new)); } else { /* previous_profile && ctx->token != token */ abort_creds(new); return -EACCES; } - ctx->label = aa_get_newest_label(label); + cred_label(new) = aa_get_newest_label(label); /* clear exec on switching context */ aa_put_label(tctx->onexec); tctx->onexec = NULL; @@ -233,7 +198,6 @@ int aa_set_current_hat(struct aa_label *label, u64 token) int aa_restore_previous_label(u64 token) { struct aa_task_ctx *tctx = current_task_ctx(); - struct aa_cred_ctx *ctx; struct cred *new; if (tctx->token != token) @@ -245,11 +209,10 @@ int aa_restore_previous_label(u64 token) new = prepare_creds(); if (!new) return -ENOMEM; - ctx = cred_ctx(new); - aa_put_label(ctx->label); - ctx->label = aa_get_newest_label(tctx->previous); - AA_BUG(!ctx->label); + aa_put_label(cred_label(new)); + cred_label(new) = aa_get_newest_label(tctx->previous); + AA_BUG(!cred_label(new)); /* clear exec && prev information when restoring to previous context */ aa_clear_task_ctx_trans(tctx); diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index b90759a765b5..5285938680e0 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -779,7 +779,6 @@ static struct aa_label *handle_onexec(struct aa_label *label, */ int apparmor_bprm_set_creds(struct linux_binprm *bprm) { - struct aa_cred_ctx *ctx; struct aa_task_ctx *tctx; struct aa_label *label, *new = NULL; struct aa_profile *profile; @@ -795,12 +794,11 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) if (bprm->called_set_creds) return 0; - ctx = cred_ctx(bprm->cred); tctx = current_task_ctx(); - AA_BUG(!ctx); + AA_BUG(!cred_label(bprm->cred)); AA_BUG(!tctx); - label = aa_get_newest_label(ctx->label); + label = aa_get_newest_label(cred_label(bprm->cred)); /* buffer freed below, name is pointer into buffer */ get_buffers(buffer); @@ -856,9 +854,9 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) } bprm->per_clear |= PER_CLEAR_ON_SETID; } - aa_put_label(ctx->label); - /* transfer reference, released when ctx is freed */ - ctx->label = new; + aa_put_label(cred_label(bprm->cred)); + /* transfer reference, released when cred is freed */ + cred_label(bprm->cred) = new; done: aa_put_label(label); @@ -1049,7 +1047,6 @@ static struct aa_label *change_hat(struct aa_label *label, const char *hats[], int aa_change_hat(const char *hats[], int count, u64 token, int flags) { const struct cred *cred; - struct aa_cred_ctx *ctx; struct aa_task_ctx *tctx; struct aa_label *label, *previous, *new = NULL, *target = NULL; struct aa_profile *profile; @@ -1070,7 +1067,6 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) /* released below */ cred = get_current_cred(); - ctx = cred_ctx(cred); tctx = current_task_ctx(); label = aa_get_newest_cred_label(cred); previous = aa_get_newest_label(tctx->previous); diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h index c3b51d88275b..8d36c14bc76d 100644 --- a/security/apparmor/include/context.h +++ b/security/apparmor/include/context.h @@ -22,21 +22,11 @@ #include "label.h" #include "policy_ns.h" -#define cred_ctx(X) ((X)->security) -#define current_cred_ctx() cred_ctx(current_cred()) - #define task_ctx(X) ((X)->security) #define current_task_ctx() (task_ctx(current)) +#define cred_label(X) ((X)->security) -/** - * struct aa_cred_ctx - primary label for confined tasks - * @label: the current label (NOT NULL) - */ -struct aa_cred_ctx { - struct aa_label *label; -}; - -/** +/* * struct aa_task_ctx - information for current task label change * @onexec: profile to transition to on next exec (MAY BE NULL) * @previous: profile the task may return to (MAY BE NULL) @@ -48,10 +38,6 @@ struct aa_task_ctx { u64 token; }; -struct aa_cred_ctx *aa_alloc_cred_ctx(gfp_t flags); -void aa_free_cred_ctx(struct aa_cred_ctx *ctx); -void aa_dup_cred_ctx(struct aa_cred_ctx *new, const struct aa_cred_ctx *old); - struct aa_task_ctx *aa_alloc_task_ctx(gfp_t flags); void aa_free_task_ctx(struct aa_task_ctx *ctx); void aa_dup_task_ctx(struct aa_task_ctx *new, const struct aa_task_ctx *old); @@ -73,10 +59,10 @@ struct aa_label *aa_get_task_label(struct task_struct *task); */ static inline struct aa_label *aa_cred_raw_label(const struct cred *cred) { - struct aa_cred_ctx *ctx = cred_ctx(cred); + struct aa_label *label = cred_label(cred); - AA_BUG(!ctx || !ctx->label); - return ctx->label; + AA_BUG(!label); + return label; } /** diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index a1d63d93b862..628c6a07df64 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -51,12 +51,12 @@ DEFINE_PER_CPU(struct aa_buffers, aa_buffers); */ /* - * free the associated aa_cred_ctx and put its labels + * put the associated labels */ static void apparmor_cred_free(struct cred *cred) { - aa_free_cred_ctx(cred_ctx(cred)); - cred_ctx(cred) = NULL; + aa_put_label(cred_label(cred)); + cred_label(cred) = NULL; } /* @@ -64,30 +64,17 @@ static void apparmor_cred_free(struct cred *cred) */ static int apparmor_cred_alloc_blank(struct cred *cred, gfp_t gfp) { - /* freed by apparmor_cred_free */ - struct aa_cred_ctx *ctx = aa_alloc_cred_ctx(gfp); - - if (!ctx) - return -ENOMEM; - - cred_ctx(cred) = ctx; + cred_label(cred) = NULL; return 0; } /* - * prepare new aa_cred_ctx for modification by prepare_cred block + * prepare new cred label for modification by prepare_cred block */ static int apparmor_cred_prepare(struct cred *new, const struct cred *old, gfp_t gfp) { - /* freed by apparmor_cred_free */ - struct aa_cred_ctx *ctx = aa_alloc_cred_ctx(gfp); - - if (!ctx) - return -ENOMEM; - - aa_dup_cred_ctx(ctx, cred_ctx(old)); - cred_ctx(new) = ctx; + cred_label(new) = aa_get_newest_label(cred_label(old)); return 0; } @@ -96,10 +83,7 @@ static int apparmor_cred_prepare(struct cred *new, const struct cred *old, */ static void apparmor_cred_transfer(struct cred *new, const struct cred *old) { - const struct aa_cred_ctx *old_ctx = cred_ctx(old); - struct aa_cred_ctx *new_ctx = cred_ctx(new); - - aa_dup_cred_ctx(new_ctx, old_ctx); + cred_label(new) = aa_get_newest_label(cred_label(old)); } static void apparmor_task_free(struct task_struct *task) @@ -599,11 +583,10 @@ static int apparmor_getprocattr(struct task_struct *task, char *name, /* released below */ const struct cred *cred = get_task_cred(task); struct aa_task_ctx *tctx = current_task_ctx(); - struct aa_cred_ctx *ctx = cred_ctx(cred); struct aa_label *label = NULL; if (strcmp(name, "current") == 0) - label = aa_get_newest_label(ctx->label); + label = aa_get_newest_label(cred_label(cred)); else if (strcmp(name, "prev") == 0 && tctx->previous) label = aa_get_newest_label(tctx->previous); else if (strcmp(name, "exec") == 0 && tctx->onexec) @@ -700,11 +683,11 @@ static int apparmor_setprocattr(const char *name, void *value, static void apparmor_bprm_committing_creds(struct linux_binprm *bprm) { struct aa_label *label = aa_current_raw_label(); - struct aa_cred_ctx *new_ctx = cred_ctx(bprm->cred); + struct aa_label *new_label = cred_label(bprm->cred); /* bail out if unconfined or not changing profile */ - if ((new_ctx->label->proxy == label->proxy) || - (unconfined(new_ctx->label))) + if ((new_label->proxy == label->proxy) || + (unconfined(new_label))) return; aa_inherit_files(bprm->cred, current->files); @@ -712,7 +695,7 @@ static void apparmor_bprm_committing_creds(struct linux_binprm *bprm) current->pdeath_signal = 0; /* reset soft limits and set hard limits for the new label */ - __aa_transition_rlimits(label, new_ctx->label); + __aa_transition_rlimits(label, new_label); } /** @@ -1050,26 +1033,16 @@ static int param_set_mode(const char *val, const struct kernel_param *kp) static int __init set_init_ctx(void) { struct cred *cred = (struct cred *)current->real_cred; - struct aa_cred_ctx *ctx; struct aa_task_ctx *tctx; - ctx = aa_alloc_cred_ctx(GFP_KERNEL); - if (!ctx) - goto fail_cred; tctx = aa_alloc_task_ctx(GFP_KERNEL); if (!tctx) - goto fail_task; + return -ENOMEM; - ctx->label = aa_get_label(ns_unconfined(root_ns)); - cred_ctx(cred) = ctx; + cred_label(cred) = aa_get_label(ns_unconfined(root_ns)); task_ctx(current) = tctx; return 0; - -fail_task: - aa_free_cred_ctx(ctx); -fail_cred: - return -ENOMEM; } static void destroy_buffers(void) From f175221af35bedf99b201d861a0fe54e19ef36c2 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 27 Jan 2017 04:09:40 -0800 Subject: [PATCH 16/36] apparmor: rename tctx to ctx now that cred_ctx has been removed we can rename task_ctxs from tctx without causing confusion. Signed-off-by: John Johansen --- security/apparmor/context.c | 25 ++++++++++++------------- security/apparmor/domain.c | 16 ++++++++-------- security/apparmor/lsm.c | 18 +++++++++--------- 3 files changed, 29 insertions(+), 30 deletions(-) diff --git a/security/apparmor/context.c b/security/apparmor/context.c index 70e4a094add8..d95a3d47cb92 100644 --- a/security/apparmor/context.c +++ b/security/apparmor/context.c @@ -156,8 +156,7 @@ int aa_set_current_onexec(struct aa_label *label, bool stack) */ int aa_set_current_hat(struct aa_label *label, u64 token) { - struct aa_task_ctx *tctx = current_task_ctx(); - struct aa_cred_ctx *ctx; + struct aa_task_ctx *ctx = current_task_ctx(); struct cred *new; new = prepare_creds(); @@ -165,11 +164,11 @@ int aa_set_current_hat(struct aa_label *label, u64 token) return -ENOMEM; AA_BUG(!label); - if (!tctx->previous) { + if (!ctx->previous) { /* transfer refcount */ - tctx->previous = cred_label(new); - tctx->token = token; - } else if (tctx->token == token) { + ctx->previous = cred_label(new); + ctx->token = token; + } else if (ctx->token == token) { aa_put_label(cred_label(new)); } else { /* previous_profile && ctx->token != token */ @@ -179,8 +178,8 @@ int aa_set_current_hat(struct aa_label *label, u64 token) cred_label(new) = aa_get_newest_label(label); /* clear exec on switching context */ - aa_put_label(tctx->onexec); - tctx->onexec = NULL; + aa_put_label(ctx->onexec); + ctx->onexec = NULL; commit_creds(new); return 0; @@ -197,13 +196,13 @@ int aa_set_current_hat(struct aa_label *label, u64 token) */ int aa_restore_previous_label(u64 token) { - struct aa_task_ctx *tctx = current_task_ctx(); + struct aa_task_ctx *ctx = current_task_ctx(); struct cred *new; - if (tctx->token != token) + if (ctx->token != token) return -EACCES; /* ignore restores when there is no saved label */ - if (!tctx->previous) + if (!ctx->previous) return 0; new = prepare_creds(); @@ -211,10 +210,10 @@ int aa_restore_previous_label(u64 token) return -ENOMEM; aa_put_label(cred_label(new)); - cred_label(new) = aa_get_newest_label(tctx->previous); + cred_label(new) = aa_get_newest_label(ctx->previous); AA_BUG(!cred_label(new)); /* clear exec && prev information when restoring to previous context */ - aa_clear_task_ctx_trans(tctx); + aa_clear_task_ctx_trans(ctx); commit_creds(new); diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 5285938680e0..b180e10f2b86 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -779,7 +779,7 @@ static struct aa_label *handle_onexec(struct aa_label *label, */ int apparmor_bprm_set_creds(struct linux_binprm *bprm) { - struct aa_task_ctx *tctx; + struct aa_task_ctx *ctx; struct aa_label *label, *new = NULL; struct aa_profile *profile; char *buffer = NULL; @@ -794,17 +794,17 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) if (bprm->called_set_creds) return 0; - tctx = current_task_ctx(); + ctx = current_task_ctx(); AA_BUG(!cred_label(bprm->cred)); - AA_BUG(!tctx); + AA_BUG(!ctx); label = aa_get_newest_label(cred_label(bprm->cred)); /* buffer freed below, name is pointer into buffer */ get_buffers(buffer); /* Test for onexec first as onexec override other x transitions. */ - if (tctx->onexec) - new = handle_onexec(label, tctx->onexec, tctx->token, + if (ctx->onexec) + new = handle_onexec(label, ctx->onexec, ctx->token, bprm, buffer, &cond, &unsafe); else new = fn_label_build(label, profile, GFP_ATOMIC, @@ -1047,7 +1047,7 @@ static struct aa_label *change_hat(struct aa_label *label, const char *hats[], int aa_change_hat(const char *hats[], int count, u64 token, int flags) { const struct cred *cred; - struct aa_task_ctx *tctx; + struct aa_task_ctx *ctx; struct aa_label *label, *previous, *new = NULL, *target = NULL; struct aa_profile *profile; struct aa_perms perms = {}; @@ -1067,9 +1067,9 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) /* released below */ cred = get_current_cred(); - tctx = current_task_ctx(); + ctx = current_task_ctx(); label = aa_get_newest_cred_label(cred); - previous = aa_get_newest_label(tctx->previous); + previous = aa_get_newest_label(ctx->previous); if (unconfined(label)) { info = "unconfined can not change_hat"; diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 628c6a07df64..fda36f3e3820 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -582,15 +582,15 @@ static int apparmor_getprocattr(struct task_struct *task, char *name, int error = -ENOENT; /* released below */ const struct cred *cred = get_task_cred(task); - struct aa_task_ctx *tctx = current_task_ctx(); + struct aa_task_ctx *ctx = current_task_ctx(); struct aa_label *label = NULL; if (strcmp(name, "current") == 0) label = aa_get_newest_label(cred_label(cred)); - else if (strcmp(name, "prev") == 0 && tctx->previous) - label = aa_get_newest_label(tctx->previous); - else if (strcmp(name, "exec") == 0 && tctx->onexec) - label = aa_get_newest_label(tctx->onexec); + else if (strcmp(name, "prev") == 0 && ctx->previous) + label = aa_get_newest_label(ctx->previous); + else if (strcmp(name, "exec") == 0 && ctx->onexec) + label = aa_get_newest_label(ctx->onexec); else error = -EINVAL; @@ -1033,14 +1033,14 @@ static int param_set_mode(const char *val, const struct kernel_param *kp) static int __init set_init_ctx(void) { struct cred *cred = (struct cred *)current->real_cred; - struct aa_task_ctx *tctx; + struct aa_task_ctx *ctx; - tctx = aa_alloc_task_ctx(GFP_KERNEL); - if (!tctx) + ctx = aa_alloc_task_ctx(GFP_KERNEL); + if (!ctx) return -ENOMEM; cred_label(cred) = aa_get_label(ns_unconfined(root_ns)); - task_ctx(current) = tctx; + task_ctx(current) = ctx; return 0; } From e1a03f627b5254fa1ee83bd0761490f31ea2e382 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Fri, 27 Jan 2017 04:36:47 -0800 Subject: [PATCH 17/36] apparmor: cleanup fixup description of aa_replace_profiles Signed-off-by: John Johansen --- security/apparmor/policy.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index c505d517fa3c..a158af1f1b38 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -845,8 +845,9 @@ static struct aa_profile *update_to_newest_parent(struct aa_profile *new) * @udata: serialized data stream (NOT NULL) * * unpack and replace a profile on the profile list and uses of that profile - * by any aa_cred_ctx. If the profile does not exist on the profile list - * it is added. + * by any task creds via invalidating the old version of the profile, which + * tasks will notice to update their own cred. If the profile does not exist + * on the profile list it is added. * * Returns: size of data consumed else error code on failure. */ From d065f2f56522b9240acb8c5ea35e9ee25f1b33e6 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sun, 8 Oct 2017 00:25:27 -0700 Subject: [PATCH 18/36] apparmor: cleanup, drop unused fn __aa_task_is_confined() Signed-off-by: John Johansen --- security/apparmor/include/context.h | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h index 8d36c14bc76d..b2aeb1da7e77 100644 --- a/security/apparmor/include/context.h +++ b/security/apparmor/include/context.h @@ -89,17 +89,6 @@ static inline struct aa_label *__aa_task_raw_label(struct task_struct *task) return aa_cred_raw_label(__task_cred(task)); } -/** - * __aa_task_is_confined - determine if @task has any confinement - * @task: task to check confinement of (NOT NULL) - * - * If @task != current needs to be called in RCU safe critical section - */ -static inline bool __aa_task_is_confined(struct task_struct *task) -{ - return !unconfined(__aa_task_raw_label(task)); -} - /** * aa_current_raw_label - find the current tasks confining label * From de62de59c27881c59c7df2e535cb9e1275cd52cc Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sun, 8 Oct 2017 00:43:02 -0700 Subject: [PATCH 19/36] apparmor: move task related defines and fns to task.X files Signed-off-by: John Johansen --- security/apparmor/Makefile | 2 +- security/apparmor/domain.c | 4 +- security/apparmor/include/context.h | 40 +---------- security/apparmor/include/task.h | 90 +++++++++++++++++++++++++ security/apparmor/lsm.c | 6 +- security/apparmor/{context.c => task.c} | 61 +++-------------- 6 files changed, 105 insertions(+), 98 deletions(-) create mode 100644 security/apparmor/include/task.h rename security/apparmor/{context.c => task.c} (74%) diff --git a/security/apparmor/Makefile b/security/apparmor/Makefile index 9a6b4033d52b..380c8e08174a 100644 --- a/security/apparmor/Makefile +++ b/security/apparmor/Makefile @@ -3,7 +3,7 @@ # obj-$(CONFIG_SECURITY_APPARMOR) += apparmor.o -apparmor-y := apparmorfs.o audit.o capability.o context.o ipc.o lib.o match.o \ +apparmor-y := apparmorfs.o audit.o capability.o task.o ipc.o lib.o match.o \ path.o domain.o policy.o policy_unpack.o procattr.o lsm.o \ resource.o secid.o file.o policy_ns.o label.o mount.o apparmor-$(CONFIG_SECURITY_APPARMOR_HASH) += crypto.o diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index b180e10f2b86..56d080a6d774 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -794,7 +794,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) if (bprm->called_set_creds) return 0; - ctx = current_task_ctx(); + ctx = task_ctx(current); AA_BUG(!cred_label(bprm->cred)); AA_BUG(!ctx); @@ -1067,7 +1067,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) /* released below */ cred = get_current_cred(); - ctx = current_task_ctx(); + ctx = task_ctx(current); label = aa_get_newest_cred_label(cred); previous = aa_get_newest_label(ctx->previous); diff --git a/security/apparmor/include/context.h b/security/apparmor/include/context.h index b2aeb1da7e77..e287b7d0d4be 100644 --- a/security/apparmor/include/context.h +++ b/security/apparmor/include/context.h @@ -21,33 +21,10 @@ #include "label.h" #include "policy_ns.h" +#include "task.h" -#define task_ctx(X) ((X)->security) -#define current_task_ctx() (task_ctx(current)) #define cred_label(X) ((X)->security) -/* - * struct aa_task_ctx - information for current task label change - * @onexec: profile to transition to on next exec (MAY BE NULL) - * @previous: profile the task may return to (MAY BE NULL) - * @token: magic value the task must know for returning to @previous_profile - */ -struct aa_task_ctx { - struct aa_label *onexec; - struct aa_label *previous; - u64 token; -}; - -struct aa_task_ctx *aa_alloc_task_ctx(gfp_t flags); -void aa_free_task_ctx(struct aa_task_ctx *ctx); -void aa_dup_task_ctx(struct aa_task_ctx *new, const struct aa_task_ctx *old); - -int aa_replace_current_label(struct aa_label *label); -int aa_set_current_onexec(struct aa_label *label, bool stack); -int aa_set_current_hat(struct aa_label *label, u64 token); -int aa_restore_previous_label(u64 cookie); -struct aa_label *aa_get_task_label(struct task_struct *task); - /** * aa_cred_raw_label - obtain cred's label @@ -196,19 +173,4 @@ static inline struct aa_ns *aa_get_current_ns(void) return ns; } -/** - * aa_clear_task_ctx_trans - clear transition tracking info from the ctx - * @ctx: task context to clear (NOT NULL) - */ -static inline void aa_clear_task_ctx_trans(struct aa_task_ctx *ctx) -{ - AA_BUG(!ctx); - - aa_put_label(ctx->previous); - aa_put_label(ctx->onexec); - ctx->previous = NULL; - ctx->onexec = NULL; - ctx->token = 0; -} - #endif /* __AA_CONTEXT_H */ diff --git a/security/apparmor/include/task.h b/security/apparmor/include/task.h new file mode 100644 index 000000000000..d222197db299 --- /dev/null +++ b/security/apparmor/include/task.h @@ -0,0 +1,90 @@ +/* + * AppArmor security module + * + * This file contains AppArmor task related definitions and mediation + * + * Copyright 2017 Canonical Ltd. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation, version 2 of the + * License. + */ + +#ifndef __AA_TASK_H +#define __AA_TASK_H + +#define task_ctx(X) ((X)->security) + +/* + * struct aa_task_ctx - information for current task label change + * @onexec: profile to transition to on next exec (MAY BE NULL) + * @previous: profile the task may return to (MAY BE NULL) + * @token: magic value the task must know for returning to @previous_profile + */ +struct aa_task_ctx { + struct aa_label *onexec; + struct aa_label *previous; + u64 token; +}; + +int aa_replace_current_label(struct aa_label *label); +int aa_set_current_onexec(struct aa_label *label, bool stack); +int aa_set_current_hat(struct aa_label *label, u64 token); +int aa_restore_previous_label(u64 cookie); +struct aa_label *aa_get_task_label(struct task_struct *task); + +/** + * aa_alloc_task_ctx - allocate a new task_ctx + * @flags: gfp flags for allocation + * + * Returns: allocated buffer or NULL on failure + */ +static inline struct aa_task_ctx *aa_alloc_task_ctx(gfp_t flags) +{ + return kzalloc(sizeof(struct aa_task_ctx), flags); +} + +/** + * aa_free_task_ctx - free a task_ctx + * @ctx: task_ctx to free (MAYBE NULL) + */ +static inline void aa_free_task_ctx(struct aa_task_ctx *ctx) +{ + if (ctx) { + aa_put_label(ctx->previous); + aa_put_label(ctx->onexec); + + kzfree(ctx); + } +} + +/** + * aa_dup_task_ctx - duplicate a task context, incrementing reference counts + * @new: a blank task context (NOT NULL) + * @old: the task context to copy (NOT NULL) + */ +static inline void aa_dup_task_ctx(struct aa_task_ctx *new, + const struct aa_task_ctx *old) +{ + *new = *old; + aa_get_label(new->previous); + aa_get_label(new->onexec); +} + +/** + * aa_clear_task_ctx_trans - clear transition tracking info from the ctx + * @ctx: task context to clear (NOT NULL) + */ +static inline void aa_clear_task_ctx_trans(struct aa_task_ctx *ctx) +{ + AA_BUG(!ctx); + + aa_put_label(ctx->previous); + aa_put_label(ctx->onexec); + ctx->previous = NULL; + ctx->onexec = NULL; + ctx->token = 0; +} + +#endif /* __AA_TASK_H */ diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index fda36f3e3820..7577cd982230 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -101,7 +101,7 @@ static int apparmor_task_alloc(struct task_struct *task, if (!new) return -ENOMEM; - aa_dup_task_ctx(new, current_task_ctx()); + aa_dup_task_ctx(new, task_ctx(current)); task_ctx(task) = new; return 0; @@ -582,7 +582,7 @@ static int apparmor_getprocattr(struct task_struct *task, char *name, int error = -ENOENT; /* released below */ const struct cred *cred = get_task_cred(task); - struct aa_task_ctx *ctx = current_task_ctx(); + struct aa_task_ctx *ctx = task_ctx(current); struct aa_label *label = NULL; if (strcmp(name, "current") == 0) @@ -705,7 +705,7 @@ static void apparmor_bprm_committing_creds(struct linux_binprm *bprm) static void apparmor_bprm_committed_creds(struct linux_binprm *bprm) { /* clear out temporary/transitional state from the context */ - aa_clear_task_ctx_trans(current_task_ctx()); + aa_clear_task_ctx_trans(task_ctx(current)); return; } diff --git a/security/apparmor/context.c b/security/apparmor/task.c similarity index 74% rename from security/apparmor/context.c rename to security/apparmor/task.c index d95a3d47cb92..36eb8707ad89 100644 --- a/security/apparmor/context.c +++ b/security/apparmor/task.c @@ -1,32 +1,23 @@ /* * AppArmor security module * - * This file contains AppArmor functions used to manipulate object security - * contexts. + * This file contains AppArmor task related definitions and mediation * - * Copyright (C) 1998-2008 Novell/SUSE - * Copyright 2009-2010 Canonical Ltd. + * Copyright 2017 Canonical Ltd. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as * published by the Free Software Foundation, version 2 of the * License. * - * - * AppArmor sets confinement on every task, via the cred_label() which - * is required and are not allowed to be NULL. The cred_label is - * reference counted. - * * TODO * If a task uses change_hat it currently does not return to the old * cred or task context but instead creates a new one. Ideally the task * should return to the previous cred if it has not been modified. - * */ #include "include/context.h" -#include "include/policy.h" - +#include "include/task.h" /** * aa_get_task_label - Get another task's label @@ -45,43 +36,6 @@ struct aa_label *aa_get_task_label(struct task_struct *task) return p; } -/** - * aa_alloc_task_ctx - allocate a new task_ctx - * @flags: gfp flags for allocation - * - * Returns: allocated buffer or NULL on failure - */ -struct aa_task_ctx *aa_alloc_task_ctx(gfp_t flags) -{ - return kzalloc(sizeof(struct aa_task_ctx), flags); -} - -/** - * aa_free_task_ctx - free a task_ctx - * @ctx: task_ctx to free (MAYBE NULL) - */ -void aa_free_task_ctx(struct aa_task_ctx *ctx) -{ - if (ctx) { - aa_put_label(ctx->previous); - aa_put_label(ctx->onexec); - - kzfree(ctx); - } -} - -/** - * aa_dup_task_ctx - duplicate a task context, incrementing reference counts - * @new: a blank task context (NOT NULL) - * @old: the task context to copy (NOT NULL) - */ -void aa_dup_task_ctx(struct aa_task_ctx *new, const struct aa_task_ctx *old) -{ - *new = *old; - aa_get_label(new->previous); - aa_get_label(new->onexec); -} - /** * aa_replace_current_label - replace the current tasks label * @label: new label (NOT NULL) @@ -110,7 +64,7 @@ int aa_replace_current_label(struct aa_label *label) * if switching to unconfined or a different label namespace * clear out context state */ - aa_clear_task_ctx_trans(current_task_ctx()); + aa_clear_task_ctx_trans(task_ctx(current)); /* * be careful switching cred label, when racing replacement it @@ -126,6 +80,7 @@ int aa_replace_current_label(struct aa_label *label) return 0; } + /** * aa_set_current_onexec - set the tasks change_profile to happen onexec * @label: system label to set at exec (MAYBE NULL to clear value) @@ -134,7 +89,7 @@ int aa_replace_current_label(struct aa_label *label) */ int aa_set_current_onexec(struct aa_label *label, bool stack) { - struct aa_task_ctx *ctx = current_task_ctx(); + struct aa_task_ctx *ctx = task_ctx(current); aa_get_label(label); aa_put_label(ctx->onexec); @@ -156,7 +111,7 @@ int aa_set_current_onexec(struct aa_label *label, bool stack) */ int aa_set_current_hat(struct aa_label *label, u64 token) { - struct aa_task_ctx *ctx = current_task_ctx(); + struct aa_task_ctx *ctx = task_ctx(current); struct cred *new; new = prepare_creds(); @@ -196,7 +151,7 @@ int aa_set_current_hat(struct aa_label *label, u64 token) */ int aa_restore_previous_label(u64 token) { - struct aa_task_ctx *ctx = current_task_ctx(); + struct aa_task_ctx *ctx = task_ctx(current); struct cred *new; if (ctx->token != token) From d8889d49e414b371eb235c08c3a759ab3e0cfa51 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Wed, 11 Oct 2017 01:04:48 -0700 Subject: [PATCH 20/36] apparmor: move context.h to cred.h Now that file contexts have been moved into file, and task context fns() and data have been split from the context, only the cred context remains in context.h so rename to cred.h to better reflect what it deals with. Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 2 +- security/apparmor/capability.c | 2 +- security/apparmor/domain.c | 2 +- security/apparmor/file.c | 2 +- security/apparmor/include/{context.h => cred.h} | 0 security/apparmor/ipc.c | 2 +- security/apparmor/label.c | 2 +- security/apparmor/lsm.c | 2 +- security/apparmor/mount.c | 2 +- security/apparmor/policy.c | 2 +- security/apparmor/policy_ns.c | 2 +- security/apparmor/policy_unpack.c | 2 +- security/apparmor/procattr.c | 2 +- security/apparmor/resource.c | 2 +- security/apparmor/task.c | 2 +- 15 files changed, 14 insertions(+), 14 deletions(-) rename security/apparmor/include/{context.h => cred.h} (100%) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 00fc4f9f7f14..874c1bf6b84a 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -30,7 +30,7 @@ #include "include/apparmor.h" #include "include/apparmorfs.h" #include "include/audit.h" -#include "include/context.h" +#include "include/cred.h" #include "include/crypto.h" #include "include/ipc.h" #include "include/label.h" diff --git a/security/apparmor/capability.c b/security/apparmor/capability.c index 67e347192a55..253ef6e9d445 100644 --- a/security/apparmor/capability.c +++ b/security/apparmor/capability.c @@ -19,7 +19,7 @@ #include "include/apparmor.h" #include "include/capability.h" -#include "include/context.h" +#include "include/cred.h" #include "include/policy.h" #include "include/audit.h" diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 56d080a6d774..cd58eef4eb8d 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -22,7 +22,7 @@ #include "include/audit.h" #include "include/apparmorfs.h" -#include "include/context.h" +#include "include/cred.h" #include "include/domain.h" #include "include/file.h" #include "include/ipc.h" diff --git a/security/apparmor/file.c b/security/apparmor/file.c index e79bf44396a3..9a67a33904b3 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -18,7 +18,7 @@ #include "include/apparmor.h" #include "include/audit.h" -#include "include/context.h" +#include "include/cred.h" #include "include/file.h" #include "include/match.h" #include "include/path.h" diff --git a/security/apparmor/include/context.h b/security/apparmor/include/cred.h similarity index 100% rename from security/apparmor/include/context.h rename to security/apparmor/include/cred.h diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c index d7b137d4eb74..527ea1557120 100644 --- a/security/apparmor/ipc.c +++ b/security/apparmor/ipc.c @@ -17,7 +17,7 @@ #include "include/audit.h" #include "include/capability.h" -#include "include/context.h" +#include "include/cred.h" #include "include/policy.h" #include "include/ipc.h" #include "include/sig_names.h" diff --git a/security/apparmor/label.c b/security/apparmor/label.c index 69c7451becef..523250e34837 100644 --- a/security/apparmor/label.c +++ b/security/apparmor/label.c @@ -16,7 +16,7 @@ #include #include "include/apparmor.h" -#include "include/context.h" +#include "include/cred.h" #include "include/label.h" #include "include/policy.h" #include "include/secid.h" diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index 7577cd982230..ef6334e11597 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -30,7 +30,7 @@ #include "include/apparmorfs.h" #include "include/audit.h" #include "include/capability.h" -#include "include/context.h" +#include "include/cred.h" #include "include/file.h" #include "include/ipc.h" #include "include/path.h" diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c index 8c558cbce930..6e8c7ac0b33d 100644 --- a/security/apparmor/mount.c +++ b/security/apparmor/mount.c @@ -18,7 +18,7 @@ #include "include/apparmor.h" #include "include/audit.h" -#include "include/context.h" +#include "include/cred.h" #include "include/domain.h" #include "include/file.h" #include "include/match.h" diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index a158af1f1b38..a8e096a88e62 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -82,7 +82,7 @@ #include "include/apparmor.h" #include "include/capability.h" -#include "include/context.h" +#include "include/cred.h" #include "include/file.h" #include "include/ipc.h" #include "include/match.h" diff --git a/security/apparmor/policy_ns.c b/security/apparmor/policy_ns.c index b1e629cba70b..b0f9dc3f765a 100644 --- a/security/apparmor/policy_ns.c +++ b/security/apparmor/policy_ns.c @@ -21,7 +21,7 @@ #include #include "include/apparmor.h" -#include "include/context.h" +#include "include/cred.h" #include "include/policy_ns.h" #include "include/label.h" #include "include/policy.h" diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index ece0c246cfe6..40c8dc617b13 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -23,7 +23,7 @@ #include "include/apparmor.h" #include "include/audit.h" -#include "include/context.h" +#include "include/cred.h" #include "include/crypto.h" #include "include/match.h" #include "include/path.h" diff --git a/security/apparmor/procattr.c b/security/apparmor/procattr.c index d81617379d63..80c34ed373c3 100644 --- a/security/apparmor/procattr.c +++ b/security/apparmor/procattr.c @@ -13,7 +13,7 @@ */ #include "include/apparmor.h" -#include "include/context.h" +#include "include/cred.h" #include "include/policy.h" #include "include/policy_ns.h" #include "include/domain.h" diff --git a/security/apparmor/resource.c b/security/apparmor/resource.c index cf4d234febe9..d022137143b9 100644 --- a/security/apparmor/resource.c +++ b/security/apparmor/resource.c @@ -16,7 +16,7 @@ #include #include "include/audit.h" -#include "include/context.h" +#include "include/cred.h" #include "include/resource.h" #include "include/policy.h" diff --git a/security/apparmor/task.c b/security/apparmor/task.c index 36eb8707ad89..44b9b938e06d 100644 --- a/security/apparmor/task.c +++ b/security/apparmor/task.c @@ -16,7 +16,7 @@ * should return to the previous cred if it has not been modified. */ -#include "include/context.h" +#include "include/cred.h" #include "include/task.h" /** From 9fcf78cca198600b27c44b4e50f00f8af3927f17 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sun, 8 Oct 2017 18:26:19 -0700 Subject: [PATCH 21/36] apparmor: update domain transitions that are subsets of confinement at nnp Domain transition so far have been largely blocked by no new privs, unless the transition has been provably a subset of the previous confinement. There was a couple problems with the previous implementations, - transitions that weren't explicitly a stack but resulted in a subset of confinement were disallowed - confinement subsets were only calculated from the previous confinement instead of the confinement being enforced at the time of no new privs, so transitions would have to get progressively tighter. Fix this by detecting and storing a reference to the task's confinement at the "time" no new privs is set. This reference is then used to determine whether a transition is a subsystem of the confinement at the time no new privs was set. Unfortunately the implementation is less than ideal in that we have to detect no new privs after the fact when a task attempts a domain transition. This is adequate for the currently but will not work in a stacking situation where no new privs could be conceivably be set in both the "host" and in the container. Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 1 + security/apparmor/domain.c | 163 +++++++++++++++++++------------ security/apparmor/include/task.h | 4 + security/apparmor/task.c | 7 ++ 4 files changed, 110 insertions(+), 65 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 874c1bf6b84a..07623fb41e32 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -2156,6 +2156,7 @@ static struct aa_sfs_entry aa_sfs_entry_domain[] = { AA_SFS_FILE_BOOLEAN("change_profile", 1), AA_SFS_FILE_BOOLEAN("stack", 1), AA_SFS_FILE_BOOLEAN("fix_binfmt_elf_mmap", 1), + AA_SFS_FILE_BOOLEAN("post_nnp_subset", 1), AA_SFS_FILE_STRING("version", "1.2"), { } }; diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index cd58eef4eb8d..9d1936519cfd 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -592,22 +592,6 @@ static struct aa_label *profile_transition(struct aa_profile *profile, if (!new) goto audit; - /* Policy has specified a domain transitions. if no_new_privs and - * confined and not transitioning to the current domain fail. - * - * NOTE: Domain transitions from unconfined and to stritly stacked - * subsets are allowed even when no_new_privs is set because this - * aways results in a further reduction of permissions. - */ - if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) && - !profile_unconfined(profile) && - !aa_label_is_subset(new, &profile->label)) { - error = -EPERM; - info = "no new privs"; - nonewprivs = true; - perms.allow &= ~MAY_EXEC; - goto audit; - } if (!(perms.xindex & AA_X_UNSAFE)) { if (DEBUG_ON) { @@ -684,21 +668,6 @@ static int profile_onexec(struct aa_profile *profile, struct aa_label *onexec, perms.allow &= ~AA_MAY_ONEXEC; goto audit; } - /* Policy has specified a domain transitions. if no_new_privs and - * confined and not transitioning to the current domain fail. - * - * NOTE: Domain transitions from unconfined and to stritly stacked - * subsets are allowed even when no_new_privs is set because this - * aways results in a further reduction of permissions. - */ - if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) && - !profile_unconfined(profile) && - !aa_label_is_subset(onexec, &profile->label)) { - error = -EPERM; - info = "no new privs"; - perms.allow &= ~AA_MAY_ONEXEC; - goto audit; - } if (!(perms.xindex & AA_X_UNSAFE)) { if (DEBUG_ON) { @@ -800,6 +769,17 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) label = aa_get_newest_label(cred_label(bprm->cred)); + /* + * 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 unconfined must be done before the subset test + */ + 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 */ get_buffers(buffer); /* Test for onexec first as onexec override other x transitions. */ @@ -820,7 +800,20 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) goto done; } - /* TODO: Add ns level no_new_privs subset test */ + /* Policy has specified a domain transitions. If no_new_privs and + * confined ensure the transition is to confinement that is subset + * of the confinement when the task entered no new privs. + * + * NOTE: Domain transitions from unconfined and to stacked + * subsets are allowed even when no_new_privs is set because this + * aways results in a further reduction of permissions. + */ + if ((bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) && + !unconfined(label) && !aa_label_is_subset(new, ctx->nnp)) { + error = -EPERM; + info = "no new privs"; + goto audit; + } if (bprm->unsafe & LSM_UNSAFE_SHARE) { /* FIXME: currently don't mediate shared state */ @@ -1047,30 +1040,28 @@ static struct aa_label *change_hat(struct aa_label *label, const char *hats[], int aa_change_hat(const char *hats[], int count, u64 token, int flags) { const struct cred *cred; - struct aa_task_ctx *ctx; + struct aa_task_ctx *ctx = task_ctx(current); struct aa_label *label, *previous, *new = NULL, *target = NULL; struct aa_profile *profile; struct aa_perms perms = {}; const char *info = NULL; int error = 0; - /* - * Fail explicitly requested domain transitions if no_new_privs. - * There is no exception for unconfined as change_hat is not - * available. - */ - if (task_no_new_privs(current)) { - /* not an apparmor denial per se, so don't log it */ - AA_DEBUG("no_new_privs - change_hat denied"); - return -EPERM; - } - /* released below */ cred = get_current_cred(); - ctx = task_ctx(current); label = aa_get_newest_cred_label(cred); previous = aa_get_newest_label(ctx->previous); + /* + * 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 unconfined must be done before the subset test + */ + if (task_no_new_privs(current) && !unconfined(label) && !ctx->nnp) + ctx->nnp = aa_get_label(label); + if (unconfined(label)) { info = "unconfined can not change_hat"; error = -EPERM; @@ -1091,6 +1082,18 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) if (error) goto fail; + /* + * no new privs prevents domain transitions that would + * reduce restrictions. + */ + if (task_no_new_privs(current) && !unconfined(label) && + !aa_label_is_subset(new, ctx->nnp)) { + /* not an apparmor denial per se, so don't log it */ + AA_DEBUG("no_new_privs - change_hat denied"); + error = -EPERM; + goto out; + } + if (flags & AA_CHANGE_TEST) goto out; @@ -1100,6 +1103,18 @@ int aa_change_hat(const char *hats[], int count, u64 token, int flags) /* kill task in case of brute force attacks */ goto kill; } else if (previous && !(flags & AA_CHANGE_TEST)) { + /* + * no new privs prevents domain transitions that would + * reduce restrictions. + */ + if (task_no_new_privs(current) && !unconfined(label) && + !aa_label_is_subset(previous, ctx->nnp)) { + /* not an apparmor denial per se, so don't log it */ + AA_DEBUG("no_new_privs - change_hat denied"); + error = -EPERM; + goto out; + } + /* Return to saved label. Kill task if restore fails * to avoid brute force attacks */ @@ -1142,21 +1157,6 @@ static int change_profile_perms_wrapper(const char *op, const char *name, const char *info = NULL; int error = 0; - /* - * Fail explicitly requested domain transitions when no_new_privs - * and not unconfined OR the transition results in a stack on - * the current label. - * Stacking domain transitions and transitions from unconfined are - * allowed even when no_new_privs is set because this aways results - * in a reduction of permissions. - */ - if (task_no_new_privs(current) && !stack && - !profile_unconfined(profile) && - !aa_label_is_subset(target, &profile->label)) { - info = "no new privs"; - error = -EPERM; - } - if (!error) error = change_profile_perms(profile, target, stack, request, profile->file.start, perms); @@ -1190,10 +1190,23 @@ int aa_change_profile(const char *fqname, int flags) const char *info = NULL; const char *auditname = fqname; /* retain leading & if stack */ bool stack = flags & AA_CHANGE_STACK; + struct aa_task_ctx *ctx = task_ctx(current); int error = 0; char *op; u32 request; + label = aa_get_current_label(); + + /* + * 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 unconfined must be done before the subset test + */ + if (task_no_new_privs(current) && !unconfined(label) && !ctx->nnp) + ctx->nnp = aa_get_label(label); + if (!fqname || !*fqname) { AA_DEBUG("no profile name"); return -EINVAL; @@ -1281,14 +1294,28 @@ int aa_change_profile(const char *fqname, int flags) if (flags & AA_CHANGE_TEST) goto out; + /* stacking is always a subset, so only check the nonstack case */ + if (!stack) { + new = fn_label_build_in_ns(label, profile, GFP_KERNEL, + aa_get_label(target), + aa_get_label(&profile->label)); + /* + * no new privs prevents domain transitions that would + * reduce restrictions. + */ + if (task_no_new_privs(current) && !unconfined(label) && + !aa_label_is_subset(new, ctx->nnp)) { + /* not an apparmor denial per se, so don't log it */ + AA_DEBUG("no_new_privs - change_hat denied"); + error = -EPERM; + goto out; + } + } + if (!(flags & AA_CHANGE_ONEXEC)) { /* only transition profiles in the current ns */ if (stack) new = aa_label_merge(label, target, GFP_KERNEL); - else - new = fn_label_build_in_ns(label, profile, GFP_KERNEL, - aa_get_label(target), - aa_get_label(&profile->label)); if (IS_ERR_OR_NULL(new)) { info = "failed to build target label"; error = PTR_ERR(new); @@ -1297,9 +1324,15 @@ int aa_change_profile(const char *fqname, int flags) goto audit; } error = aa_replace_current_label(new); - } else + } else { + if (new) { + aa_put_label(new); + new = NULL; + } + /* full transition will be built in exec path */ error = aa_set_current_onexec(target, stack); + } audit: error = fn_for_each_in_ns(label, profile, diff --git a/security/apparmor/include/task.h b/security/apparmor/include/task.h index d222197db299..55edaa1d83f8 100644 --- a/security/apparmor/include/task.h +++ b/security/apparmor/include/task.h @@ -18,11 +18,13 @@ /* * struct aa_task_ctx - information for current task label change + * @nnp: snapshot of label at time of no_new_privs * @onexec: profile to transition to on next exec (MAY BE NULL) * @previous: profile the task may return to (MAY BE NULL) * @token: magic value the task must know for returning to @previous_profile */ struct aa_task_ctx { + struct aa_label *nnp; struct aa_label *onexec; struct aa_label *previous; u64 token; @@ -52,6 +54,7 @@ static inline struct aa_task_ctx *aa_alloc_task_ctx(gfp_t flags) static inline void aa_free_task_ctx(struct aa_task_ctx *ctx) { if (ctx) { + aa_put_label(ctx->nnp); aa_put_label(ctx->previous); aa_put_label(ctx->onexec); @@ -68,6 +71,7 @@ static inline void aa_dup_task_ctx(struct aa_task_ctx *new, const struct aa_task_ctx *old) { *new = *old; + aa_get_label(new->nnp); aa_get_label(new->previous); aa_get_label(new->onexec); } diff --git a/security/apparmor/task.c b/security/apparmor/task.c index 44b9b938e06d..c6b78a14da91 100644 --- a/security/apparmor/task.c +++ b/security/apparmor/task.c @@ -45,6 +45,7 @@ struct aa_label *aa_get_task_label(struct task_struct *task) int aa_replace_current_label(struct aa_label *label) { struct aa_label *old = aa_current_raw_label(); + struct aa_task_ctx *ctx = task_ctx(current); struct cred *new; AA_BUG(!label); @@ -59,6 +60,12 @@ int aa_replace_current_label(struct aa_label *label) if (!new) return -ENOMEM; + if (ctx->nnp && label_is_stale(ctx->nnp)) { + struct aa_label *tmp = ctx->nnp; + + ctx->nnp = aa_get_newest_label(tmp); + aa_put_label(tmp); + } if (unconfined(label) || (labels_ns(old) != labels_ns(label))) /* * if switching to unconfined or a different label namespace From 074c1cd798cb0b481d7eaa749b64aa416563c053 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 8 Aug 2017 11:58:33 -0700 Subject: [PATCH 22/36] apparmor: dfa move character match into a macro Signed-off-by: John Johansen --- security/apparmor/match.c | 74 ++++++++++++++------------------------- 1 file changed, 27 insertions(+), 47 deletions(-) diff --git a/security/apparmor/match.c b/security/apparmor/match.c index 5d95caeddebc..aeac68c58689 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -329,6 +329,18 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags) return ERR_PTR(error); } +#define match_char(state, def, base, next, check, C) \ +do { \ + u32 b = (base)[(state)]; \ + unsigned int pos = base_idx(b) + (C); \ + if ((check)[pos] != (state)) { \ + (state) = (def)[(state)]; \ + break; \ + } \ + (state) = (next)[pos]; \ + break; \ +} while (1) + /** * aa_dfa_match_len - traverse @dfa to find state @str stops at * @dfa: the dfa to match @str against (NOT NULL) @@ -352,7 +364,7 @@ unsigned int aa_dfa_match_len(struct aa_dfa *dfa, unsigned int start, u32 *base = BASE_TABLE(dfa); u16 *next = NEXT_TABLE(dfa); u16 *check = CHECK_TABLE(dfa); - unsigned int state = start, pos; + unsigned int state = start; if (state == 0) return 0; @@ -361,23 +373,13 @@ unsigned int aa_dfa_match_len(struct aa_dfa *dfa, unsigned int start, if (dfa->tables[YYTD_ID_EC]) { /* Equivalence class table defined */ u8 *equiv = EQUIV_TABLE(dfa); - /* default is direct to next state */ - for (; len; len--) { - pos = base_idx(base[state]) + equiv[(u8) *str++]; - if (check[pos] == state) - state = next[pos]; - else - state = def[state]; - } + for (; len; len--) + match_char(state, def, base, next, check, + equiv[(u8) *str++]); } else { /* default is direct to next state */ - for (; len; len--) { - pos = base_idx(base[state]) + (u8) *str++; - if (check[pos] == state) - state = next[pos]; - else - state = def[state]; - } + for (; len; len--) + match_char(state, def, base, next, check, (u8) *str++); } return state; @@ -402,7 +404,7 @@ unsigned int aa_dfa_match(struct aa_dfa *dfa, unsigned int start, u32 *base = BASE_TABLE(dfa); u16 *next = NEXT_TABLE(dfa); u16 *check = CHECK_TABLE(dfa); - unsigned int state = start, pos; + unsigned int state = start; if (state == 0) return 0; @@ -412,22 +414,13 @@ unsigned int aa_dfa_match(struct aa_dfa *dfa, unsigned int start, /* Equivalence class table defined */ u8 *equiv = EQUIV_TABLE(dfa); /* default is direct to next state */ - while (*str) { - pos = base_idx(base[state]) + equiv[(u8) *str++]; - if (check[pos] == state) - state = next[pos]; - else - state = def[state]; - } + while (*str) + match_char(state, def, base, next, check, + equiv[(u8) *str++]); } else { /* default is direct to next state */ - while (*str) { - pos = base_idx(base[state]) + (u8) *str++; - if (check[pos] == state) - state = next[pos]; - else - state = def[state]; - } + while (*str) + match_char(state, def, base, next, check, (u8) *str++); } return state; @@ -450,27 +443,14 @@ unsigned int aa_dfa_next(struct aa_dfa *dfa, unsigned int state, u32 *base = BASE_TABLE(dfa); u16 *next = NEXT_TABLE(dfa); u16 *check = CHECK_TABLE(dfa); - unsigned int pos; /* current state is , matching character *str */ if (dfa->tables[YYTD_ID_EC]) { /* Equivalence class table defined */ u8 *equiv = EQUIV_TABLE(dfa); - /* default is direct to next state */ - - pos = base_idx(base[state]) + equiv[(u8) c]; - if (check[pos] == state) - state = next[pos]; - else - state = def[state]; - } else { - /* default is direct to next state */ - pos = base_idx(base[state]) + (u8) c; - if (check[pos] == state) - state = next[pos]; - else - state = def[state]; - } + match_char(state, def, base, next, check, equiv[(u8) c]); + } else + match_char(state, def, base, next, check, (u8) c); return state; } From 031dcc8f4e84fea37dc6f78fdc7288aa7f8386c3 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 8 Aug 2017 12:10:50 -0700 Subject: [PATCH 23/36] apparmor: dfa add support for state differential encoding State differential encoding can provide better compression for apparmor policy, without having significant impact on match time. Signed-off-by: John Johansen --- security/apparmor/include/match.h | 4 ++++ security/apparmor/match.c | 26 +++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h index cd8aeab6ac57..e0de00bd16a8 100644 --- a/security/apparmor/include/match.h +++ b/security/apparmor/include/match.h @@ -40,6 +40,7 @@ */ #define YYTH_MAGIC 0x1B5E783D +#define YYTH_FLAG_DIFF_ENCODE 1 struct table_set_header { u32 th_magic; /* YYTH_MAGIC */ @@ -164,4 +165,7 @@ static inline void aa_put_dfa(struct aa_dfa *dfa) kref_put(&dfa->count, aa_dfa_free_kref); } +#define MATCH_FLAG_DIFF_ENCODE 0x80000000 +#define MARK_DIFF_ENCODE 0x40000000 + #endif /* __AA_MATCH_H */ diff --git a/security/apparmor/match.c b/security/apparmor/match.c index aeac68c58689..70cdcb3c3b25 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -185,7 +185,8 @@ static int verify_dfa(struct aa_dfa *dfa, int flags) if (flags & DFA_FLAG_VERIFY_STATES) { for (i = 0; i < state_count; i++) { - if (DEFAULT_TABLE(dfa)[i] >= state_count) + if (!(BASE_TABLE(dfa)[i] & MATCH_FLAG_DIFF_ENCODE) && + (DEFAULT_TABLE(dfa)[i] >= state_count)) goto out; if (base_idx(BASE_TABLE(dfa)[i]) + 255 >= trans_count) { printk(KERN_ERR "AppArmor DFA next/check upper " @@ -202,6 +203,24 @@ static int verify_dfa(struct aa_dfa *dfa, int flags) } } + /* Now that all the other tables are verified, verify diffencoding */ + if (flags & DFA_FLAG_VERIFY_STATES) { + size_t j, k; + + for (i = 0; i < state_count; i++) { + for (j = i; + (BASE_TABLE(dfa)[j] & MATCH_FLAG_DIFF_ENCODE) && + !(BASE_TABLE(dfa)[j] & MARK_DIFF_ENCODE); + j = k) { + k = DEFAULT_TABLE(dfa)[j]; + if (j == k) + goto out; + if (k < j) + break; /* already verified */ + BASE_TABLE(dfa)[j] |= MARK_DIFF_ENCODE; + } + } + } error = 0; out: return error; @@ -274,6 +293,9 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags) goto fail; dfa->flags = ntohs(*(__be16 *) (data + 12)); + if (dfa->flags != 0 && dfa->flags != YYTH_FLAG_DIFF_ENCODE) + goto fail; + data += hsize; size -= hsize; @@ -335,6 +357,8 @@ do { \ unsigned int pos = base_idx(b) + (C); \ if ((check)[pos] != (state)) { \ (state) = (def)[(state)]; \ + if (b & MATCH_FLAG_DIFF_ENCODE) \ + continue; \ break; \ } \ (state) = (next)[pos]; \ From d901d6a298dc6e9105b9dc091d65b043e9f8c9a6 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 8 Aug 2017 13:01:01 -0700 Subject: [PATCH 24/36] apparmor: dfa split verification of table headers separate the different types of verification so they are logically separate and can be reused separate of each other. Signed-off-by: John Johansen --- security/apparmor/match.c | 116 ++++++++++++++++++++++---------------- 1 file changed, 68 insertions(+), 48 deletions(-) diff --git a/security/apparmor/match.c b/security/apparmor/match.c index 70cdcb3c3b25..7ae6ed9d69dd 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -136,8 +136,8 @@ static struct table_header *unpack_table(char *blob, size_t bsize) } /** - * verify_dfa - verify that transitions and states in the tables are in bounds. - * @dfa: dfa to test (NOT NULL) + * verify_table_headers - verify that the tables headers are as expected + * @tables - array of dfa tables to check (NOT NULL) * @flags: flags controlling what type of accept table are acceptable * * Assumes dfa has gone through the first pass verification done by unpacking @@ -145,83 +145,98 @@ static struct table_header *unpack_table(char *blob, size_t bsize) * * Returns: %0 else error code on failure to verify */ -static int verify_dfa(struct aa_dfa *dfa, int flags) +static int verify_table_headers(struct table_header **tables, int flags) { - size_t i, state_count, trans_count; + size_t state_count, trans_count; int error = -EPROTO; /* check that required tables exist */ - if (!(dfa->tables[YYTD_ID_DEF] && - dfa->tables[YYTD_ID_BASE] && - dfa->tables[YYTD_ID_NXT] && dfa->tables[YYTD_ID_CHK])) + if (!(tables[YYTD_ID_DEF] && tables[YYTD_ID_BASE] && + tables[YYTD_ID_NXT] && tables[YYTD_ID_CHK])) goto out; /* accept.size == default.size == base.size */ - state_count = dfa->tables[YYTD_ID_BASE]->td_lolen; + state_count = tables[YYTD_ID_BASE]->td_lolen; if (ACCEPT1_FLAGS(flags)) { - if (!dfa->tables[YYTD_ID_ACCEPT]) + if (!tables[YYTD_ID_ACCEPT]) goto out; - if (state_count != dfa->tables[YYTD_ID_ACCEPT]->td_lolen) + if (state_count != tables[YYTD_ID_ACCEPT]->td_lolen) goto out; } if (ACCEPT2_FLAGS(flags)) { - if (!dfa->tables[YYTD_ID_ACCEPT2]) + if (!tables[YYTD_ID_ACCEPT2]) goto out; - if (state_count != dfa->tables[YYTD_ID_ACCEPT2]->td_lolen) + if (state_count != tables[YYTD_ID_ACCEPT2]->td_lolen) goto out; } - if (state_count != dfa->tables[YYTD_ID_DEF]->td_lolen) + if (state_count != tables[YYTD_ID_DEF]->td_lolen) goto out; /* next.size == chk.size */ - trans_count = dfa->tables[YYTD_ID_NXT]->td_lolen; - if (trans_count != dfa->tables[YYTD_ID_CHK]->td_lolen) + trans_count = tables[YYTD_ID_NXT]->td_lolen; + if (trans_count != tables[YYTD_ID_CHK]->td_lolen) goto out; /* if equivalence classes then its table size must be 256 */ - if (dfa->tables[YYTD_ID_EC] && - dfa->tables[YYTD_ID_EC]->td_lolen != 256) + if (tables[YYTD_ID_EC] && tables[YYTD_ID_EC]->td_lolen != 256) goto out; - if (flags & DFA_FLAG_VERIFY_STATES) { - for (i = 0; i < state_count; i++) { - if (!(BASE_TABLE(dfa)[i] & MATCH_FLAG_DIFF_ENCODE) && - (DEFAULT_TABLE(dfa)[i] >= state_count)) - goto out; - if (base_idx(BASE_TABLE(dfa)[i]) + 255 >= trans_count) { - printk(KERN_ERR "AppArmor DFA next/check upper " - "bounds error\n"); - goto out; - } - } + error = 0; +out: + return error; +} - for (i = 0; i < trans_count; i++) { - if (NEXT_TABLE(dfa)[i] >= state_count) - goto out; - if (CHECK_TABLE(dfa)[i] >= state_count) - goto out; +/** + * verify_dfa - verify that transitions and states in the tables are in bounds. + * @dfa: dfa to test (NOT NULL) + * + * Assumes dfa has gone through the first pass verification done by unpacking + * NOTE: this does not valid accept table values + * + * Returns: %0 else error code on failure to verify + */ +static int verify_dfa(struct aa_dfa *dfa) +{ + size_t i, state_count, trans_count; + int error = EPROTO; + + state_count = dfa->tables[YYTD_ID_BASE]->td_lolen; + trans_count = dfa->tables[YYTD_ID_NXT]->td_lolen; + for (i = 0; i < state_count; i++) { + if (!(BASE_TABLE(dfa)[i] & MATCH_FLAG_DIFF_ENCODE) && + (DEFAULT_TABLE(dfa)[i] >= state_count)) + goto out; + if (base_idx(BASE_TABLE(dfa)[i]) + 255 >= trans_count) { + pr_err("AppArmor DFA next/check upper bounds error\n"); + goto out; } } + for (i = 0; i < trans_count; i++) { + if (NEXT_TABLE(dfa)[i] >= state_count) + goto out; + if (CHECK_TABLE(dfa)[i] >= state_count) + goto out; + } + /* Now that all the other tables are verified, verify diffencoding */ - if (flags & DFA_FLAG_VERIFY_STATES) { + for (i = 0; i < state_count; i++) { size_t j, k; - for (i = 0; i < state_count; i++) { - for (j = i; - (BASE_TABLE(dfa)[j] & MATCH_FLAG_DIFF_ENCODE) && - !(BASE_TABLE(dfa)[j] & MARK_DIFF_ENCODE); - j = k) { - k = DEFAULT_TABLE(dfa)[j]; - if (j == k) - goto out; - if (k < j) - break; /* already verified */ - BASE_TABLE(dfa)[j] |= MARK_DIFF_ENCODE; - } + for (j = i; + (BASE_TABLE(dfa)[j] & MATCH_FLAG_DIFF_ENCODE) && + !(BASE_TABLE(dfa)[j] & MARK_DIFF_ENCODE); + j = k) { + k = DEFAULT_TABLE(dfa)[j]; + if (j == k) + goto out; + if (k < j) + break; /* already verified */ + BASE_TABLE(dfa)[j] |= MARK_DIFF_ENCODE; } } error = 0; + out: return error; } @@ -338,11 +353,16 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags) size -= table_size(table->td_lolen, table->td_flags); table = NULL; } - - error = verify_dfa(dfa, flags); + error = verify_table_headers(dfa->tables, flags); if (error) goto fail; + if (flags & DFA_FLAG_VERIFY_STATES) { + error = verify_dfa(dfa); + if (error) + goto fail; + } + return dfa; fail: From cf91600071a973c28cebf314e618610a20ec4d6d Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 5 Feb 2018 09:58:29 +0100 Subject: [PATCH 25/36] apparmor: cleanup create_aafs() error path Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 07623fb41e32..8cdab3c5bc63 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -2458,34 +2458,26 @@ static int __init aa_create_aafs(void) dent = securityfs_create_file(".load", 0666, aa_sfs_entry.dentry, NULL, &aa_fs_profile_load); - if (IS_ERR(dent)) { - error = PTR_ERR(dent); - goto error; - } + if (IS_ERR(dent)) + goto dent_error; ns_subload(root_ns) = dent; dent = securityfs_create_file(".replace", 0666, aa_sfs_entry.dentry, NULL, &aa_fs_profile_replace); - if (IS_ERR(dent)) { - error = PTR_ERR(dent); - goto error; - } + if (IS_ERR(dent)) + goto dent_error; ns_subreplace(root_ns) = dent; dent = securityfs_create_file(".remove", 0666, aa_sfs_entry.dentry, NULL, &aa_fs_profile_remove); - if (IS_ERR(dent)) { - error = PTR_ERR(dent); - goto error; - } + if (IS_ERR(dent)) + goto dent_error; ns_subremove(root_ns) = dent; dent = securityfs_create_file("revision", 0444, aa_sfs_entry.dentry, NULL, &aa_fs_ns_revision_fops); - if (IS_ERR(dent)) { - error = PTR_ERR(dent); - goto error; - } + if (IS_ERR(dent)) + goto dent_error; ns_subrevision(root_ns) = dent; /* policy tree referenced by magic policy symlink */ @@ -2499,10 +2491,8 @@ static int __init aa_create_aafs(void) /* magic symlink similar to nsfs redirects based on task policy */ dent = securityfs_create_symlink("policy", aa_sfs_entry.dentry, NULL, &policy_link_iops); - if (IS_ERR(dent)) { - error = PTR_ERR(dent); - goto error; - } + if (IS_ERR(dent)) + goto dent_error; error = aa_mk_null_file(aa_sfs_entry.dentry); if (error) @@ -2514,6 +2504,8 @@ static int __init aa_create_aafs(void) aa_info_message("AppArmor Filesystem Enabled"); return 0; +dent_error: + error = PTR_ERR(dent); error: aa_destroy_aafs(); AA_ERROR("Error creating AppArmor securityfs\n"); From a0781209cb894e5115bb00c269b1d94c4b632d6a Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 5 Feb 2018 18:26:46 +0100 Subject: [PATCH 26/36] apparmor: cleanup: simplify code to get ns symlink name ns_get_name() is called in only one place and can be folded in. Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 8cdab3c5bc63..1e63ff2e5b85 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -119,9 +119,7 @@ static int aafs_count; static int aafs_show_path(struct seq_file *seq, struct dentry *dentry) { - struct inode *inode = d_inode(dentry); - - seq_printf(seq, "%s:[%lu]", AAFS_NAME, inode->i_ino); + seq_printf(seq, "%s:[%lu]", AAFS_NAME, d_inode(dentry)->i_ino); return 0; } @@ -2392,29 +2390,18 @@ static const char *policy_get_link(struct dentry *dentry, return NULL; } -static int ns_get_name(char *buf, size_t size, struct aa_ns *ns, - struct inode *inode) -{ - int res = snprintf(buf, size, "%s:[%lu]", AAFS_NAME, inode->i_ino); - - if (res < 0 || res >= size) - res = -ENOENT; - - return res; -} - static int policy_readlink(struct dentry *dentry, char __user *buffer, int buflen) { - struct aa_ns *ns; char name[32]; int res; - ns = aa_get_current_ns(); - res = ns_get_name(name, sizeof(name), ns, d_inode(dentry)); - if (res >= 0) + res = snprintf(name, sizeof(name), "%s:[%lu]", AAFS_NAME, + d_inode(dentry)->i_ino); + if (res > 0 && res < sizeof(name)) res = readlink_copy(buffer, buflen, name); - aa_put_ns(ns); + else + res = -ENOENT; return res; } From 8e51f9087f4024d20f70f4d9831e1f45d8088331 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Thu, 8 Feb 2018 12:37:19 -0800 Subject: [PATCH 27/36] apparmor: Add support for attaching profiles via xattr, presence and value Make it possible to tie Apparmor profiles to the presence of one or more extended attributes, and optionally their values. An example usecase for this is to automatically transition to a more privileged Apparmor profile if an executable has a valid IMA signature, which can then be appraised by the IMA subsystem. Signed-off-by: Matthew Garrett Signed-off-by: John Johansen --- security/apparmor/domain.c | 150 ++++++++++++++++++++++++----- security/apparmor/include/policy.h | 6 ++ security/apparmor/policy.c | 8 ++ security/apparmor/policy_unpack.c | 85 ++++++++++++++-- 4 files changed, 216 insertions(+), 33 deletions(-) diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 9d1936519cfd..6bcafe8d226d 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -19,6 +19,7 @@ #include #include #include +#include #include "include/audit.h" #include "include/apparmorfs.h" @@ -301,8 +302,57 @@ static int change_profile_perms(struct aa_profile *profile, return label_match(profile, target, stack, start, true, request, perms); } +/** + * aa_xattrs_match - check whether a file matches the xattrs defined in profile + * @bprm: binprm struct for the process to validate + * @profile: profile to match against (NOT NULL) + * + * Returns: number of extended attributes that matched, or < 0 on error + */ +static int aa_xattrs_match(const struct linux_binprm *bprm, + struct aa_profile *profile) +{ + int i; + size_t size; + struct dentry *d; + char *value = NULL; + int value_size = 0, ret = profile->xattr_count; + + if (!bprm || !profile->xattr_count) + return 0; + + d = bprm->file->f_path.dentry; + + for (i = 0; i < profile->xattr_count; i++) { + size = vfs_getxattr_alloc(d, profile->xattrs[i], &value, + value_size, GFP_KERNEL); + if (size < 0) { + ret = -EINVAL; + goto out; + } + + /* Check the xattr value, not just presence */ + if (profile->xattr_lens[i]) { + if (profile->xattr_lens[i] != size) { + ret = -EINVAL; + goto out; + } + + if (memcmp(value, profile->xattr_values[i], size)) { + ret = -EINVAL; + goto out; + } + } + } + +out: + kfree(value); + return ret; +} + /** * __attach_match_ - find an attachment match + * @bprm - binprm structure of transitioning task * @name - to match against (NOT NULL) * @head - profile list to walk (NOT NULL) * @info - info message if there was an error (NOT NULL) @@ -316,11 +366,12 @@ static int change_profile_perms(struct aa_profile *profile, * * Returns: profile or NULL if no match found */ -static struct aa_profile *__attach_match(const char *name, +static struct aa_profile *__attach_match(const struct linux_binprm *bprm, + const char *name, struct list_head *head, const char **info) { - int len = 0; + int len = 0, xattrs = 0; bool conflict = false; struct aa_profile *profile, *candidate = NULL; @@ -329,26 +380,56 @@ static struct aa_profile *__attach_match(const char *name, &profile->label == ns_unconfined(profile->ns)) continue; + /* Find the "best" matching profile. Profiles must + * match the path and extended attributes (if any) + * associated with the file. A more specific path + * match will be preferred over a less specific one, + * and a match with more matching extended attributes + * will be preferred over one with fewer. If the best + * match has both the same level of path specificity + * and the same number of matching extended attributes + * as another profile, signal a conflict and refuse to + * match. + */ if (profile->xmatch) { - if (profile->xmatch_len >= len) { - unsigned int state; - u32 perm; + unsigned int state; + u32 perm; - state = aa_dfa_match(profile->xmatch, - DFA_START, name); - perm = dfa_user_allow(profile->xmatch, state); - /* any accepting state means a valid match. */ - if (perm & MAY_EXEC) { - if (profile->xmatch_len == len) { + if (profile->xmatch_len < len) + continue; + + state = aa_dfa_match(profile->xmatch, + DFA_START, name); + perm = dfa_user_allow(profile->xmatch, state); + /* any accepting state means a valid match. */ + if (perm & MAY_EXEC) { + int ret = aa_xattrs_match(bprm, profile); + + /* Fail matching if the xattrs don't match */ + if (ret < 0) + continue; + + /* The new match isn't more specific + * than the current best match + */ + if (profile->xmatch_len == len && + ret <= xattrs) { + /* Match is equivalent, so conflict */ + if (ret == xattrs) conflict = true; - continue; - } - candidate = profile; - len = profile->xmatch_len; - conflict = false; + continue; } + + /* Either the same length with more matching + * xattrs, or a longer match + */ + candidate = profile; + len = profile->xmatch_len; + xattrs = ret; + conflict = false; } - } else if (!strcmp(profile->base.name, name)) + } else if (!strcmp(profile->base.name, name) && + aa_xattrs_match(bprm, profile) >= 0) /* exact non-re match, no more searching required */ return profile; } @@ -363,6 +444,7 @@ static struct aa_profile *__attach_match(const char *name, /** * find_attach - do attachment search for unconfined processes + * @bprm - binprm structure of transitioning task * @ns: the current namespace (NOT NULL) * @list: list to search (NOT NULL) * @name: the executable name to match against (NOT NULL) @@ -370,13 +452,14 @@ static struct aa_profile *__attach_match(const char *name, * * Returns: label or NULL if no match found */ -static struct aa_label *find_attach(struct aa_ns *ns, struct list_head *list, +static struct aa_label *find_attach(const struct linux_binprm *bprm, + struct aa_ns *ns, struct list_head *list, const char *name, const char **info) { struct aa_profile *profile; rcu_read_lock(); - profile = aa_get_profile(__attach_match(name, list, info)); + profile = aa_get_profile(__attach_match(bprm, name, list, info)); rcu_read_unlock(); return profile ? &profile->label : NULL; @@ -432,6 +515,7 @@ struct aa_label *x_table_lookup(struct aa_profile *profile, u32 xindex, /** * x_to_label - get target label for a given xindex * @profile: current profile (NOT NULL) + * @bprm: binprm structure of transitioning task * @name: name to lookup (NOT NULL) * @xindex: index into x transition table * @lookupname: returns: name used in lookup if one was specified (NOT NULL) @@ -441,6 +525,7 @@ struct aa_label *x_table_lookup(struct aa_profile *profile, u32 xindex, * Returns: refcounted label or NULL if not found available */ static struct aa_label *x_to_label(struct aa_profile *profile, + const struct linux_binprm *bprm, const char *name, u32 xindex, const char **lookupname, const char **info) @@ -468,11 +553,11 @@ static struct aa_label *x_to_label(struct aa_profile *profile, case AA_X_NAME: if (xindex & AA_X_CHILD) /* released by caller */ - new = find_attach(ns, &profile->base.profiles, + new = find_attach(bprm, ns, &profile->base.profiles, name, info); else /* released by caller */ - new = find_attach(ns, &ns->base.profiles, + new = find_attach(bprm, ns, &ns->base.profiles, name, info); *lookupname = name; break; @@ -512,6 +597,8 @@ static struct aa_label *profile_transition(struct aa_profile *profile, bool *secure_exec) { struct aa_label *new = NULL; + struct aa_profile *component; + struct label_it i; const char *info = NULL, *name = NULL, *target = NULL; unsigned int state = profile->file.start; struct aa_perms perms = {}; @@ -536,8 +623,8 @@ static struct aa_label *profile_transition(struct aa_profile *profile, } if (profile_unconfined(profile)) { - new = find_attach(profile->ns, &profile->ns->base.profiles, - name, &info); + new = find_attach(bprm, profile->ns, + &profile->ns->base.profiles, name, &info); if (new) { AA_DEBUG("unconfined attached to new label"); return new; @@ -550,7 +637,8 @@ static struct aa_label *profile_transition(struct aa_profile *profile, state = aa_str_perms(profile->file.dfa, state, name, cond, &perms); if (perms.allow & MAY_EXEC) { /* exec permission determine how to transition */ - new = x_to_label(profile, name, perms.xindex, &target, &info); + new = x_to_label(profile, bprm, name, perms.xindex, &target, + &info); if (new && new->proxy == profile->label.proxy && info) { /* hack ix fallback - improve how this is detected */ goto audit; @@ -559,6 +647,20 @@ static struct aa_label *profile_transition(struct aa_profile *profile, info = "profile transition not found"; /* remove MAY_EXEC to audit as failure */ perms.allow &= ~MAY_EXEC; + } else { + /* verify that each component's xattr requirements are + * met, and fail execution otherwise + */ + label_for_each(i, new, component) { + if (aa_xattrs_match(bprm, component) < 0) { + error = -EACCES; + info = "required xattrs not present"; + perms.allow &= ~MAY_EXEC; + aa_put_label(new); + new = NULL; + goto audit; + } + } } } else if (COMPLAIN_MODE(profile)) { /* no exec permission - learning mode */ diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 17fe41a9cac3..02bde92ebb5c 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -148,6 +148,12 @@ struct aa_profile { struct aa_policydb policy; struct aa_file_rules file; struct aa_caps caps; + + int xattr_count; + char **xattrs; + size_t *xattr_lens; + char **xattr_values; + struct aa_rlimit rlimits; struct aa_loaddata *rawdata; diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index a8e096a88e62..7fee546ba10d 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -210,6 +210,7 @@ static void aa_free_data(void *ptr, void *arg) void aa_free_profile(struct aa_profile *profile) { struct rhashtable *rht; + int i; AA_DEBUG("%s(%p)\n", __func__, profile); @@ -227,6 +228,13 @@ void aa_free_profile(struct aa_profile *profile) aa_free_cap_rules(&profile->caps); aa_free_rlimit_rules(&profile->rlimits); + for (i = 0; i < profile->xattr_count; i++) { + kzfree(profile->xattrs[i]); + kzfree(profile->xattr_values[i]); + } + kzfree(profile->xattrs); + kzfree(profile->xattr_lens); + kzfree(profile->xattr_values); kzfree(profile->dirname); aa_put_dfa(profile->xmatch); aa_put_dfa(profile->policy.dfa); diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 40c8dc617b13..98d019185e57 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -203,6 +203,15 @@ static bool inbounds(struct aa_ext *e, size_t size) return (size <= e->end - e->pos); } +static void *kvmemdup(const void *src, size_t len) +{ + void *p = kvmalloc(len, GFP_KERNEL); + + if (p) + memcpy(p, src, len); + return p; +} + /** * aa_u16_chunck - test and do bounds checking for a u16 size based chunk * @e: serialized data read head (NOT NULL) @@ -522,6 +531,68 @@ static bool unpack_trans_table(struct aa_ext *e, struct aa_profile *profile) return 0; } +static bool unpack_xattrs(struct aa_ext *e, struct aa_profile *profile) +{ + void *pos = e->pos; + + if (unpack_nameX(e, AA_STRUCT, "xattrs")) { + int i, size; + + size = unpack_array(e, NULL); + profile->xattr_count = size; + profile->xattrs = kcalloc(size, sizeof(char *), + GFP_KERNEL); + if (!profile->xattrs) + goto fail; + for (i = 0; i < size; i++) { + if (!unpack_strdup(e, &profile->xattrs[i], NULL)) + goto fail; + } + if (!unpack_nameX(e, AA_ARRAYEND, NULL)) + goto fail; + if (!unpack_nameX(e, AA_STRUCTEND, NULL)) + goto fail; + } + + if (unpack_nameX(e, AA_STRUCT, "xattr_values")) { + int i, size; + + size = unpack_array(e, NULL); + + /* Must be the same number of xattr values as xattrs */ + if (size != profile->xattr_count) + goto fail; + + profile->xattr_lens = kcalloc(size, sizeof(size_t), + GFP_KERNEL); + if (!profile->xattr_lens) + goto fail; + + profile->xattr_values = kcalloc(size, sizeof(char *), + GFP_KERNEL); + if (!profile->xattr_values) + goto fail; + + for (i = 0; i < size; i++) { + profile->xattr_lens[i] = unpack_blob(e, + &profile->xattr_values[i], NULL); + profile->xattr_values[i] = + kvmemdup(profile->xattr_values[i], + profile->xattr_lens[i]); + } + + if (!unpack_nameX(e, AA_ARRAYEND, NULL)) + goto fail; + if (!unpack_nameX(e, AA_STRUCTEND, NULL)) + goto fail; + } + return 1; + +fail: + e->pos = pos; + return 0; +} + static bool unpack_rlimits(struct aa_ext *e, struct aa_profile *profile) { void *pos = e->pos; @@ -556,15 +627,6 @@ static bool unpack_rlimits(struct aa_ext *e, struct aa_profile *profile) return 0; } -static void *kvmemdup(const void *src, size_t len) -{ - void *p = kvmalloc(len, GFP_KERNEL); - - if (p) - memcpy(p, src, len); - return p; -} - static u32 strhash(const void *data, u32 len, u32 seed) { const char * const *key = data; @@ -719,6 +781,11 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name) goto fail; } + if (!unpack_xattrs(e, profile)) { + info = "failed to unpack profile xattrs"; + goto fail; + } + if (!unpack_rlimits(e, profile)) { info = "failed to unpack profile rlimits"; goto fail; From 73f488cd903938e78979d50e081a0314ad142351 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 12 Dec 2017 15:28:05 -0800 Subject: [PATCH 28/36] apparmor: convert attaching profiles via xattrs to use dfa matching This converts profile attachment based on xattrs to a fixed extended conditional using dfa matching. This has a couple of advantages - pattern matching can be used for the xattr match - xattrs can be optional for an attachment or marked as required - the xattr attachment conditional will be able to be combined with other extended conditionals when the flexible extended conditional work lands. The xattr fixed extended conditional is appended to the xmatch conditional. If an xattr attachment is specified the profile xmatch will be generated regardless of whether there is a pattern match on the executable name. Signed-off-by: John Johansen Acked-by: Seth Arnold --- security/apparmor/apparmorfs.c | 5 +++ security/apparmor/domain.c | 58 ++++++++++++++++++++---------- security/apparmor/include/policy.h | 2 -- security/apparmor/policy.c | 6 +--- security/apparmor/policy_unpack.c | 35 +----------------- 5 files changed, 46 insertions(+), 60 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 1e63ff2e5b85..35e6b240fb14 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -2147,6 +2147,10 @@ static struct aa_sfs_entry aa_sfs_entry_signal[] = { { } }; +static struct aa_sfs_entry aa_sfs_entry_attach[] = { + AA_SFS_FILE_BOOLEAN("xattr", 1), + { } +}; static struct aa_sfs_entry aa_sfs_entry_domain[] = { AA_SFS_FILE_BOOLEAN("change_hat", 1), AA_SFS_FILE_BOOLEAN("change_hatv", 1), @@ -2155,6 +2159,7 @@ static struct aa_sfs_entry aa_sfs_entry_domain[] = { AA_SFS_FILE_BOOLEAN("stack", 1), AA_SFS_FILE_BOOLEAN("fix_binfmt_elf_mmap", 1), AA_SFS_FILE_BOOLEAN("post_nnp_subset", 1), + AA_SFS_DIR("attach_conditions", aa_sfs_entry_attach), AA_SFS_FILE_STRING("version", "1.2"), { } }; diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 6bcafe8d226d..6a1279f11fcc 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -306,11 +306,12 @@ static int change_profile_perms(struct aa_profile *profile, * aa_xattrs_match - check whether a file matches the xattrs defined in profile * @bprm: binprm struct for the process to validate * @profile: profile to match against (NOT NULL) + * @state: state to start match in * * Returns: number of extended attributes that matched, or < 0 on error */ static int aa_xattrs_match(const struct linux_binprm *bprm, - struct aa_profile *profile) + struct aa_profile *profile, unsigned int state) { int i; size_t size; @@ -321,27 +322,40 @@ static int aa_xattrs_match(const struct linux_binprm *bprm, if (!bprm || !profile->xattr_count) return 0; + /* transition from exec match to xattr set */ + state = aa_dfa_null_transition(profile->xmatch, state); + d = bprm->file->f_path.dentry; for (i = 0; i < profile->xattr_count; i++) { size = vfs_getxattr_alloc(d, profile->xattrs[i], &value, value_size, GFP_KERNEL); - if (size < 0) { - ret = -EINVAL; - goto out; + if (size >= 0) { + u32 perm; + + /* Check the xattr value, not just presence */ + state = aa_dfa_match_len(profile->xmatch, state, value, + size); + perm = dfa_user_allow(profile->xmatch, state); + if (!(perm & MAY_EXEC)) { + ret = -EINVAL; + goto out; + } } - - /* Check the xattr value, not just presence */ - if (profile->xattr_lens[i]) { - if (profile->xattr_lens[i] != size) { - ret = -EINVAL; - goto out; - } - - if (memcmp(value, profile->xattr_values[i], size)) { + /* transition to next element */ + state = aa_dfa_null_transition(profile->xmatch, state); + if (size < 0) { + /* + * No xattr match, so verify if transition to + * next element was valid. IFF so the xattr + * was optional. + */ + if (!state) { ret = -EINVAL; goto out; } + /* don't count missing optional xattr as matched */ + ret--; } } @@ -403,13 +417,16 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm, perm = dfa_user_allow(profile->xmatch, state); /* any accepting state means a valid match. */ if (perm & MAY_EXEC) { - int ret = aa_xattrs_match(bprm, profile); + int ret = aa_xattrs_match(bprm, profile, state); /* Fail matching if the xattrs don't match */ if (ret < 0) continue; - /* The new match isn't more specific + /* + * TODO: allow for more flexible best match + * + * The new match isn't more specific * than the current best match */ if (profile->xmatch_len == len && @@ -428,9 +445,11 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm, xattrs = ret; conflict = false; } - } else if (!strcmp(profile->base.name, name) && - aa_xattrs_match(bprm, profile) >= 0) - /* exact non-re match, no more searching required */ + } else if (!strcmp(profile->base.name, name)) + /* + * old exact non-re match, without conditionals such + * as xattrs. no more searching required + */ return profile; } @@ -652,7 +671,8 @@ static struct aa_label *profile_transition(struct aa_profile *profile, * met, and fail execution otherwise */ label_for_each(i, new, component) { - if (aa_xattrs_match(bprm, component) < 0) { + if (aa_xattrs_match(bprm, component, state) < + 0) { error = -EACCES; info = "required xattrs not present"; perms.allow &= ~MAY_EXEC; diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index 02bde92ebb5c..c93b9ed55490 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -151,8 +151,6 @@ struct aa_profile { int xattr_count; char **xattrs; - size_t *xattr_lens; - char **xattr_values; struct aa_rlimit rlimits; diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index 7fee546ba10d..c07493ce2376 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -228,13 +228,9 @@ void aa_free_profile(struct aa_profile *profile) aa_free_cap_rules(&profile->caps); aa_free_rlimit_rules(&profile->rlimits); - for (i = 0; i < profile->xattr_count; i++) { + for (i = 0; i < profile->xattr_count; i++) kzfree(profile->xattrs[i]); - kzfree(profile->xattr_values[i]); - } kzfree(profile->xattrs); - kzfree(profile->xattr_lens); - kzfree(profile->xattr_values); kzfree(profile->dirname); aa_put_dfa(profile->xmatch); aa_put_dfa(profile->policy.dfa); diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 98d019185e57..8a31ddd474d7 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -540,8 +540,7 @@ static bool unpack_xattrs(struct aa_ext *e, struct aa_profile *profile) size = unpack_array(e, NULL); profile->xattr_count = size; - profile->xattrs = kcalloc(size, sizeof(char *), - GFP_KERNEL); + profile->xattrs = kcalloc(size, sizeof(char *), GFP_KERNEL); if (!profile->xattrs) goto fail; for (i = 0; i < size; i++) { @@ -554,38 +553,6 @@ static bool unpack_xattrs(struct aa_ext *e, struct aa_profile *profile) goto fail; } - if (unpack_nameX(e, AA_STRUCT, "xattr_values")) { - int i, size; - - size = unpack_array(e, NULL); - - /* Must be the same number of xattr values as xattrs */ - if (size != profile->xattr_count) - goto fail; - - profile->xattr_lens = kcalloc(size, sizeof(size_t), - GFP_KERNEL); - if (!profile->xattr_lens) - goto fail; - - profile->xattr_values = kcalloc(size, sizeof(char *), - GFP_KERNEL); - if (!profile->xattr_values) - goto fail; - - for (i = 0; i < size; i++) { - profile->xattr_lens[i] = unpack_blob(e, - &profile->xattr_values[i], NULL); - profile->xattr_values[i] = - kvmemdup(profile->xattr_values[i], - profile->xattr_lens[i]); - } - - if (!unpack_nameX(e, AA_ARRAYEND, NULL)) - goto fail; - if (!unpack_nameX(e, AA_STRUCTEND, NULL)) - goto fail; - } return 1; fail: From 21f606610502ef56f9180b1529fc7e02957564c8 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sat, 18 Nov 2017 19:43:13 -0800 Subject: [PATCH 29/36] apparmor: improve overlapping domain attachment resolution Overlapping domain attachments using the current longest left exact match fail in some simple cases, and with the fix to ensure consistent behavior by failing unresolvable attachments it becomes important to do a better job. eg. under the current match the following are unresolvable where the alternation is clearly a better match under the most specific left match rule. /** /{bin/,}usr/ Use a counting match that detects when a loop in the state machine is enter, and return the match count to provide a better specific left match resolution. Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 1 + security/apparmor/domain.c | 30 ++++---- security/apparmor/include/match.h | 19 +++++ security/apparmor/match.c | 122 +++++++++++++++++++++++++++++- 4 files changed, 158 insertions(+), 14 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 35e6b240fb14..3dcc122234c8 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -2159,6 +2159,7 @@ static struct aa_sfs_entry aa_sfs_entry_domain[] = { AA_SFS_FILE_BOOLEAN("stack", 1), AA_SFS_FILE_BOOLEAN("fix_binfmt_elf_mmap", 1), AA_SFS_FILE_BOOLEAN("post_nnp_subset", 1), + AA_SFS_FILE_BOOLEAN("computed_longest_left", 1), AA_SFS_DIR("attach_conditions", aa_sfs_entry_attach), AA_SFS_FILE_STRING("version", "1.2"), { } diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 6a1279f11fcc..57cc892e05a2 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -385,10 +385,13 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm, struct list_head *head, const char **info) { - int len = 0, xattrs = 0; + int candidate_len = 0, candidate_xattrs = 0; bool conflict = false; struct aa_profile *profile, *candidate = NULL; + AA_BUG(!name); + AA_BUG(!head); + list_for_each_entry_rcu(profile, head, base.list) { if (profile->label.flags & FLAG_NULL && &profile->label == ns_unconfined(profile->ns)) @@ -406,19 +409,20 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm, * match. */ if (profile->xmatch) { - unsigned int state; + unsigned int state, count; u32 perm; - if (profile->xmatch_len < len) - continue; - - state = aa_dfa_match(profile->xmatch, - DFA_START, name); + state = aa_dfa_leftmatch(profile->xmatch, DFA_START, + name, &count); perm = dfa_user_allow(profile->xmatch, state); /* any accepting state means a valid match. */ if (perm & MAY_EXEC) { - int ret = aa_xattrs_match(bprm, profile, state); + int ret; + if (count < candidate_len) + continue; + + ret = aa_xattrs_match(bprm, profile, state); /* Fail matching if the xattrs don't match */ if (ret < 0) continue; @@ -429,10 +433,10 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm, * The new match isn't more specific * than the current best match */ - if (profile->xmatch_len == len && - ret <= xattrs) { + if (count == candidate_len && + ret <= candidate_xattrs) { /* Match is equivalent, so conflict */ - if (ret == xattrs) + if (ret == candidate_xattrs) conflict = true; continue; } @@ -441,8 +445,8 @@ static struct aa_profile *__attach_match(const struct linux_binprm *bprm, * xattrs, or a longer match */ candidate = profile; - len = profile->xmatch_len; - xattrs = ret; + candidate_len = profile->xmatch_len; + candidate_xattrs = ret; conflict = false; } } else if (!strcmp(profile->base.name, name)) diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h index e0de00bd16a8..958d2b52a7b7 100644 --- a/security/apparmor/include/match.h +++ b/security/apparmor/include/match.h @@ -138,6 +138,25 @@ unsigned int aa_dfa_matchn_until(struct aa_dfa *dfa, unsigned int start, void aa_dfa_free_kref(struct kref *kref); +#define WB_HISTORY_SIZE 8 +struct match_workbuf { + unsigned int count; + unsigned int pos; + unsigned int len; + unsigned int size; /* power of 2, same as history size */ + unsigned int history[WB_HISTORY_SIZE]; +}; +#define DEFINE_MATCH_WB(N) \ +struct match_workbuf N = { \ + .count = 0, \ + .pos = 0, \ + .len = 0, \ + .size = WB_HISTORY_SIZE, \ +} + +unsigned int aa_dfa_leftmatch(struct aa_dfa *dfa, unsigned int start, + const char *str, unsigned int *count); + /** * aa_get_dfa - increment refcount on dfa @p * @dfa: dfa (MAYBE NULL) diff --git a/security/apparmor/match.c b/security/apparmor/match.c index 7ae6ed9d69dd..dd4c995c5e25 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -556,7 +556,6 @@ unsigned int aa_dfa_match_until(struct aa_dfa *dfa, unsigned int start, return state; } - /** * aa_dfa_matchn_until - traverse @dfa until accept or @n bytes consumed * @dfa: the dfa to match @str against (NOT NULL) @@ -618,3 +617,124 @@ unsigned int aa_dfa_matchn_until(struct aa_dfa *dfa, unsigned int start, *retpos = str; return state; } + +#define inc_wb_pos(wb) \ +do { \ + wb->pos = (wb->pos + 1) & (wb->size - 1); \ + wb->len = (wb->len + 1) & (wb->size - 1); \ +} while (0) + +/* For DFAs that don't support extended tagging of states */ +static bool is_loop(struct match_workbuf *wb, unsigned int state, + unsigned int *adjust) +{ + unsigned int pos = wb->pos; + unsigned int i; + + if (wb->history[pos] < state) + return false; + + for (i = 0; i <= wb->len; i++) { + if (wb->history[pos] == state) { + *adjust = i; + return true; + } + if (pos == 0) + pos = wb->size; + pos--; + } + + *adjust = i; + return true; +} + +static unsigned int leftmatch_fb(struct aa_dfa *dfa, unsigned int start, + const char *str, struct match_workbuf *wb, + unsigned int *count) +{ + u16 *def = DEFAULT_TABLE(dfa); + u32 *base = BASE_TABLE(dfa); + u16 *next = NEXT_TABLE(dfa); + u16 *check = CHECK_TABLE(dfa); + unsigned int state = start, pos; + + AA_BUG(!dfa); + AA_BUG(!str); + AA_BUG(!wb); + AA_BUG(!count); + + *count = 0; + if (state == 0) + return 0; + + /* current state is , matching character *str */ + if (dfa->tables[YYTD_ID_EC]) { + /* Equivalence class table defined */ + u8 *equiv = EQUIV_TABLE(dfa); + /* default is direct to next state */ + while (*str) { + unsigned int adjust; + + wb->history[wb->pos] = state; + pos = base_idx(base[state]) + equiv[(u8) *str++]; + if (check[pos] == state) + state = next[pos]; + else + state = def[state]; + if (is_loop(wb, state, &adjust)) { + state = aa_dfa_match(dfa, state, str); + *count -= adjust; + goto out; + } + inc_wb_pos(wb); + (*count)++; + } + } else { + /* default is direct to next state */ + while (*str) { + unsigned int adjust; + + wb->history[wb->pos] = state; + pos = base_idx(base[state]) + (u8) *str++; + if (check[pos] == state) + state = next[pos]; + else + state = def[state]; + if (is_loop(wb, state, &adjust)) { + state = aa_dfa_match(dfa, state, str); + *count -= adjust; + goto out; + } + inc_wb_pos(wb); + (*count)++; + } + } + +out: + if (!state) + *count = 0; + return state; +} + +/** + * aa_dfa_leftmatch - traverse @dfa to find state @str stops at + * @dfa: the dfa to match @str against (NOT NULL) + * @start: the state of the dfa to start matching in + * @str: the null terminated string of bytes to match against the dfa (NOT NULL) + * @count: current count of longest left. + * + * aa_dfa_match will match @str against the dfa and return the state it + * finished matching in. The final state can be used to look up the accepting + * label, or as the start state of a continuing match. + * + * Returns: final state reached after input is consumed + */ +unsigned int aa_dfa_leftmatch(struct aa_dfa *dfa, unsigned int start, + const char *str, unsigned int *count) +{ + DEFINE_MATCH_WB(wb); + + /* TODO: match for extended state dfas */ + + return leftmatch_fb(dfa, start, str, &wb, count); +} From 56974a6fcfef69ee0825bd66ed13e92070ac5224 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 18 Jul 2017 23:18:33 -0700 Subject: [PATCH 30/36] apparmor: add base infastructure for socket mediation version 2 - Force an abi break. Network mediation will only be available in v8 abi complaint policy. Provide a basic mediation of sockets. This is not a full net mediation but just whether a spcific family of socket can be used by an application, along with setting up some basic infrastructure for network mediation to follow. the user space rule hav the basic form of NETWORK RULE = [ QUALIFIERS ] 'network' [ DOMAIN ] [ TYPE | PROTOCOL ] DOMAIN = ( 'inet' | 'ax25' | 'ipx' | 'appletalk' | 'netrom' | 'bridge' | 'atmpvc' | 'x25' | 'inet6' | 'rose' | 'netbeui' | 'security' | 'key' | 'packet' | 'ash' | 'econet' | 'atmsvc' | 'sna' | 'irda' | 'pppox' | 'wanpipe' | 'bluetooth' | 'netlink' | 'unix' | 'rds' | 'llc' | 'can' | 'tipc' | 'iucv' | 'rxrpc' | 'isdn' | 'phonet' | 'ieee802154' | 'caif' | 'alg' | 'nfc' | 'vsock' | 'mpls' | 'ib' | 'kcm' ) ',' TYPE = ( 'stream' | 'dgram' | 'seqpacket' | 'rdm' | 'raw' | 'packet' ) PROTOCOL = ( 'tcp' | 'udp' | 'icmp' ) eg. network, network inet, Signed-off-by: John Johansen Acked-by: Seth Arnold --- security/apparmor/.gitignore | 1 + security/apparmor/Makefile | 43 ++- security/apparmor/apparmorfs.c | 2 + security/apparmor/file.c | 30 ++ security/apparmor/include/apparmor.h | 3 +- security/apparmor/include/audit.h | 6 + security/apparmor/include/net.h | 106 ++++++++ security/apparmor/include/perms.h | 5 +- security/apparmor/include/policy.h | 11 + security/apparmor/lib.c | 5 +- security/apparmor/lsm.c | 392 +++++++++++++++++++++++++++ security/apparmor/net.c | 187 +++++++++++++ security/apparmor/policy_unpack.c | 3 +- 13 files changed, 786 insertions(+), 8 deletions(-) create mode 100644 security/apparmor/include/net.h create mode 100644 security/apparmor/net.c diff --git a/security/apparmor/.gitignore b/security/apparmor/.gitignore index 9cdec70d72b8..d5b291e94264 100644 --- a/security/apparmor/.gitignore +++ b/security/apparmor/.gitignore @@ -1,5 +1,6 @@ # # Generated include files # +net_names.h capability_names.h rlim_names.h diff --git a/security/apparmor/Makefile b/security/apparmor/Makefile index 380c8e08174a..ff23fcfefe19 100644 --- a/security/apparmor/Makefile +++ b/security/apparmor/Makefile @@ -5,11 +5,44 @@ obj-$(CONFIG_SECURITY_APPARMOR) += apparmor.o apparmor-y := apparmorfs.o audit.o capability.o task.o ipc.o lib.o match.o \ path.o domain.o policy.o policy_unpack.o procattr.o lsm.o \ - resource.o secid.o file.o policy_ns.o label.o mount.o + resource.o secid.o file.o policy_ns.o label.o mount.o net.o apparmor-$(CONFIG_SECURITY_APPARMOR_HASH) += crypto.o -clean-files := capability_names.h rlim_names.h +clean-files := capability_names.h rlim_names.h net_names.h +# Build a lower case string table of address family names +# Transform lines from +# #define AF_LOCAL 1 /* POSIX name for AF_UNIX */ +# #define AF_INET 2 /* Internet IP Protocol */ +# to +# [1] = "local", +# [2] = "inet", +# +# and build the securityfs entries for the mapping. +# Transforms lines from +# #define AF_INET 2 /* Internet IP Protocol */ +# to +# #define AA_SFS_AF_MASK "local inet" +quiet_cmd_make-af = GEN $@ +cmd_make-af = echo "static const char *address_family_names[] = {" > $@ ;\ + sed $< >>$@ -r -n -e "/AF_MAX/d" -e "/AF_LOCAL/d" -e "/AF_ROUTE/d" -e \ + 's/^\#define[ \t]+AF_([A-Z0-9_]+)[ \t]+([0-9]+)(.*)/[\2] = "\L\1",/p';\ + echo "};" >> $@ ;\ + printf '%s' '\#define AA_SFS_AF_MASK "' >> $@ ;\ + sed -r -n -e "/AF_MAX/d" -e "/AF_LOCAL/d" -e "/AF_ROUTE/d" -e \ + 's/^\#define[ \t]+AF_([A-Z0-9_]+)[ \t]+([0-9]+)(.*)/\L\1/p'\ + $< | tr '\n' ' ' | sed -e 's/ $$/"\n/' >> $@ + +# Build a lower case string table of sock type names +# Transform lines from +# SOCK_STREAM = 1, +# to +# [1] = "stream", +quiet_cmd_make-sock = GEN $@ +cmd_make-sock = echo "static const char *sock_type_names[] = {" >> $@ ;\ + sed $^ >>$@ -r -n \ + -e 's/^\tSOCK_([A-Z0-9_]+)[\t]+=[ \t]+([0-9]+)(.*)/[\2] = "\L\1",/p';\ + echo "};" >> $@ # Build a lower case string table of capability names # Transforms lines from @@ -62,6 +95,7 @@ cmd_make-rlim = echo "static const char *const rlim_names[RLIM_NLIMITS] = {" \ tr '\n' ' ' | sed -e 's/ $$/"\n/' >> $@ $(obj)/capability.o : $(obj)/capability_names.h +$(obj)/net.o : $(obj)/net_names.h $(obj)/resource.o : $(obj)/rlim_names.h $(obj)/capability_names.h : $(srctree)/include/uapi/linux/capability.h \ $(src)/Makefile @@ -69,3 +103,8 @@ $(obj)/capability_names.h : $(srctree)/include/uapi/linux/capability.h \ $(obj)/rlim_names.h : $(srctree)/include/uapi/asm-generic/resource.h \ $(src)/Makefile $(call cmd,make-rlim) +$(obj)/net_names.h : $(srctree)/include/linux/socket.h \ + $(srctree)/include/linux/net.h \ + $(src)/Makefile + $(call cmd,make-af) + $(call cmd,make-sock) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 3dcc122234c8..10d16e3abed9 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -2169,6 +2169,7 @@ static struct aa_sfs_entry aa_sfs_entry_versions[] = { AA_SFS_FILE_BOOLEAN("v5", 1), AA_SFS_FILE_BOOLEAN("v6", 1), AA_SFS_FILE_BOOLEAN("v7", 1), + AA_SFS_FILE_BOOLEAN("v8", 1), { } }; @@ -2204,6 +2205,7 @@ static struct aa_sfs_entry aa_sfs_entry_features[] = { AA_SFS_DIR("policy", aa_sfs_entry_policy), AA_SFS_DIR("domain", aa_sfs_entry_domain), AA_SFS_DIR("file", aa_sfs_entry_file), + AA_SFS_DIR("network_v8", aa_sfs_entry_network), AA_SFS_DIR("mount", aa_sfs_entry_mount), AA_SFS_DIR("namespaces", aa_sfs_entry_ns), AA_SFS_FILE_U64("capability", VFS_CAP_FLAGS_MASK), diff --git a/security/apparmor/file.c b/security/apparmor/file.c index 9a67a33904b3..224b2fef93ca 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -21,6 +21,7 @@ #include "include/cred.h" #include "include/file.h" #include "include/match.h" +#include "include/net.h" #include "include/path.h" #include "include/policy.h" #include "include/label.h" @@ -560,6 +561,32 @@ static int __file_path_perm(const char *op, struct aa_label *label, return error; } +static int __file_sock_perm(const char *op, struct aa_label *label, + struct aa_label *flabel, struct file *file, + u32 request, u32 denied) +{ + struct socket *sock = (struct socket *) file->private_data; + int error; + + AA_BUG(!sock); + + /* revalidation due to label out of date. No revocation at this time */ + if (!denied && aa_label_is_subset(flabel, label)) + return 0; + + /* TODO: improve to skip profiles cached in flabel */ + error = aa_sock_file_perm(label, op, request, sock); + if (denied) { + /* TODO: improve to skip profiles checked above */ + /* check every profile in file label to is cached */ + last_error(error, aa_sock_file_perm(flabel, op, request, sock)); + } + if (!error) + update_file_ctx(file_ctx(file), label, request); + + return error; +} + /** * aa_file_perm - do permission revalidation check & audit for @file * @op: operation being checked @@ -604,6 +631,9 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file, error = __file_path_perm(op, label, flabel, file, request, denied); + else if (S_ISSOCK(file_inode(file)->i_mode)) + error = __file_sock_perm(op, label, flabel, file, request, + denied); done: rcu_read_unlock(); diff --git a/security/apparmor/include/apparmor.h b/security/apparmor/include/apparmor.h index 829082c35faa..73d63b58d875 100644 --- a/security/apparmor/include/apparmor.h +++ b/security/apparmor/include/apparmor.h @@ -24,12 +24,13 @@ #define AA_CLASS_UNKNOWN 1 #define AA_CLASS_FILE 2 #define AA_CLASS_CAP 3 -#define AA_CLASS_NET 4 +#define AA_CLASS_DEPRECATED 4 #define AA_CLASS_RLIMITS 5 #define AA_CLASS_DOMAIN 6 #define AA_CLASS_MOUNT 7 #define AA_CLASS_PTRACE 9 #define AA_CLASS_SIGNAL 10 +#define AA_CLASS_NET 14 #define AA_CLASS_LABEL 16 #define AA_CLASS_LAST AA_CLASS_LABEL diff --git a/security/apparmor/include/audit.h b/security/apparmor/include/audit.h index 41ad2c947bf4..9c9be9c98c15 100644 --- a/security/apparmor/include/audit.h +++ b/security/apparmor/include/audit.h @@ -134,6 +134,12 @@ struct apparmor_audit_data { int signal; int unmappedsig; }; + struct { + int type, protocol; + struct sock *peer_sk; + void *addr; + int addrlen; + } net; }; }; struct { diff --git a/security/apparmor/include/net.h b/security/apparmor/include/net.h new file mode 100644 index 000000000000..ec7228e857a9 --- /dev/null +++ b/security/apparmor/include/net.h @@ -0,0 +1,106 @@ +/* + * AppArmor security module + * + * This file contains AppArmor network mediation definitions. + * + * Copyright (C) 1998-2008 Novell/SUSE + * Copyright 2009-2017 Canonical Ltd. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation, version 2 of the + * License. + */ + +#ifndef __AA_NET_H +#define __AA_NET_H + +#include +#include + +#include "apparmorfs.h" +#include "label.h" +#include "perms.h" +#include "policy.h" + +#define AA_MAY_SEND AA_MAY_WRITE +#define AA_MAY_RECEIVE AA_MAY_READ + +#define AA_MAY_SHUTDOWN AA_MAY_DELETE + +#define AA_MAY_CONNECT AA_MAY_OPEN +#define AA_MAY_ACCEPT 0x00100000 + +#define AA_MAY_BIND 0x00200000 +#define AA_MAY_LISTEN 0x00400000 + +#define AA_MAY_SETOPT 0x01000000 +#define AA_MAY_GETOPT 0x02000000 + +#define NET_PERMS_MASK (AA_MAY_SEND | AA_MAY_RECEIVE | AA_MAY_CREATE | \ + AA_MAY_SHUTDOWN | AA_MAY_BIND | AA_MAY_LISTEN | \ + AA_MAY_CONNECT | AA_MAY_ACCEPT | AA_MAY_SETATTR | \ + AA_MAY_GETATTR | AA_MAY_SETOPT | AA_MAY_GETOPT) + +#define NET_FS_PERMS (AA_MAY_SEND | AA_MAY_RECEIVE | AA_MAY_CREATE | \ + AA_MAY_SHUTDOWN | AA_MAY_CONNECT | AA_MAY_RENAME |\ + AA_MAY_SETATTR | AA_MAY_GETATTR | AA_MAY_CHMOD | \ + AA_MAY_CHOWN | AA_MAY_CHGRP | AA_MAY_LOCK | \ + AA_MAY_MPROT) + +#define NET_PEER_MASK (AA_MAY_SEND | AA_MAY_RECEIVE | AA_MAY_CONNECT | \ + AA_MAY_ACCEPT) +struct aa_sk_ctx { + struct aa_label *label; + struct aa_label *peer; +}; + +#define SK_CTX(X) ((X)->sk_security) +#define SOCK_ctx(X) SOCK_INODE(X)->i_security +#define DEFINE_AUDIT_NET(NAME, OP, SK, F, T, P) \ + struct lsm_network_audit NAME ## _net = { .sk = (SK), \ + .family = (F)}; \ + DEFINE_AUDIT_DATA(NAME, \ + ((SK) && (F) != AF_UNIX) ? LSM_AUDIT_DATA_NET : \ + LSM_AUDIT_DATA_NONE, \ + OP); \ + NAME.u.net = &(NAME ## _net); \ + aad(&NAME)->net.type = (T); \ + aad(&NAME)->net.protocol = (P) + +#define DEFINE_AUDIT_SK(NAME, OP, SK) \ + DEFINE_AUDIT_NET(NAME, OP, SK, (SK)->sk_family, (SK)->sk_type, \ + (SK)->sk_protocol) + + +#define af_select(FAMILY, FN, DEF_FN) \ +({ \ + int __e; \ + switch ((FAMILY)) { \ + default: \ + __e = DEF_FN; \ + } \ + __e; \ +}) + +extern struct aa_sfs_entry aa_sfs_entry_network[]; + +void audit_net_cb(struct audit_buffer *ab, void *va); +int aa_profile_af_perm(struct aa_profile *profile, struct common_audit_data *sa, + u32 request, u16 family, int type); +int aa_af_perm(struct aa_label *label, const char *op, u32 request, u16 family, + int type, int protocol); +static inline int aa_profile_af_sk_perm(struct aa_profile *profile, + struct common_audit_data *sa, + u32 request, + struct sock *sk) +{ + return aa_profile_af_perm(profile, sa, request, sk->sk_family, + sk->sk_type); +} +int aa_sk_perm(const char *op, u32 request, struct sock *sk); + +int aa_sock_file_perm(struct aa_label *label, const char *op, u32 request, + struct socket *sock); + +#endif /* __AA_NET_H */ diff --git a/security/apparmor/include/perms.h b/security/apparmor/include/perms.h index d7b7e7115160..38aa6247d00f 100644 --- a/security/apparmor/include/perms.h +++ b/security/apparmor/include/perms.h @@ -138,9 +138,10 @@ extern struct aa_perms allperms; void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask); -void aa_audit_perm_names(struct audit_buffer *ab, const char **names, u32 mask); +void aa_audit_perm_names(struct audit_buffer *ab, const char * const *names, + u32 mask); void aa_audit_perm_mask(struct audit_buffer *ab, u32 mask, const char *chrs, - u32 chrsmask, const char **names, u32 namesmask); + u32 chrsmask, const char * const *names, u32 namesmask); void aa_apply_modes_to_perms(struct aa_profile *profile, struct aa_perms *perms); void aa_compute_perms(struct aa_dfa *dfa, unsigned int state, diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index c93b9ed55490..ffe12a2366e0 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -30,6 +30,7 @@ #include "file.h" #include "lib.h" #include "label.h" +#include "net.h" #include "perms.h" #include "resource.h" @@ -224,6 +225,16 @@ static inline unsigned int PROFILE_MEDIATES_SAFE(struct aa_profile *profile, return 0; } +static inline unsigned int PROFILE_MEDIATES_AF(struct aa_profile *profile, + u16 AF) { + unsigned int state = PROFILE_MEDIATES(profile, AA_CLASS_NET); + __be16 be_af = cpu_to_be16(AF); + + if (!state) + return 0; + return aa_dfa_match_len(profile->policy.dfa, state, (char *) &be_af, 2); +} + /** * aa_get_profile - increment refcount on profile @p * @p: profile (MAYBE NULL) diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c index 4d5e98e49d5e..068a9f471f77 100644 --- a/security/apparmor/lib.c +++ b/security/apparmor/lib.c @@ -211,7 +211,8 @@ void aa_perm_mask_to_str(char *str, const char *chrs, u32 mask) *str = '\0'; } -void aa_audit_perm_names(struct audit_buffer *ab, const char **names, u32 mask) +void aa_audit_perm_names(struct audit_buffer *ab, const char * const *names, + u32 mask) { const char *fmt = "%s"; unsigned int i, perm = 1; @@ -229,7 +230,7 @@ void aa_audit_perm_names(struct audit_buffer *ab, const char **names, u32 mask) } void aa_audit_perm_mask(struct audit_buffer *ab, u32 mask, const char *chrs, - u32 chrsmask, const char **names, u32 namesmask) + u32 chrsmask, const char * const *names, u32 namesmask) { char str[33]; diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index ef6334e11597..956edebf83eb 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -33,6 +33,7 @@ #include "include/cred.h" #include "include/file.h" #include "include/ipc.h" +#include "include/net.h" #include "include/path.h" #include "include/label.h" #include "include/policy.h" @@ -743,6 +744,373 @@ static int apparmor_task_kill(struct task_struct *target, struct siginfo *info, return error; } +/** + * apparmor_sk_alloc_security - allocate and attach the sk_security field + */ +static int apparmor_sk_alloc_security(struct sock *sk, int family, gfp_t flags) +{ + struct aa_sk_ctx *ctx; + + ctx = kzalloc(sizeof(*ctx), flags); + if (!ctx) + return -ENOMEM; + + SK_CTX(sk) = ctx; + + return 0; +} + +/** + * apparmor_sk_free_security - free the sk_security field + */ +static void apparmor_sk_free_security(struct sock *sk) +{ + struct aa_sk_ctx *ctx = SK_CTX(sk); + + SK_CTX(sk) = NULL; + aa_put_label(ctx->label); + aa_put_label(ctx->peer); + kfree(ctx); +} + +/** + * apparmor_clone_security - clone the sk_security field + */ +static void apparmor_sk_clone_security(const struct sock *sk, + struct sock *newsk) +{ + struct aa_sk_ctx *ctx = SK_CTX(sk); + struct aa_sk_ctx *new = SK_CTX(newsk); + + new->label = aa_get_label(ctx->label); + new->peer = aa_get_label(ctx->peer); +} + +/** + * apparmor_socket_create - check perms before creating a new socket + */ +static int apparmor_socket_create(int family, int type, int protocol, int kern) +{ + struct aa_label *label; + int error = 0; + + AA_BUG(in_interrupt()); + + label = begin_current_label_crit_section(); + if (!(kern || unconfined(label))) + error = af_select(family, + create_perm(label, family, type, protocol), + aa_af_perm(label, OP_CREATE, AA_MAY_CREATE, + family, type, protocol)); + end_current_label_crit_section(label); + + return error; +} + +/** + * apparmor_socket_post_create - setup the per-socket security struct + * + * Note: + * - kernel sockets currently labeled unconfined but we may want to + * move to a special kernel label + * - socket may not have sk here if created with sock_create_lite or + * sock_alloc. These should be accept cases which will be handled in + * sock_graft. + */ +static int apparmor_socket_post_create(struct socket *sock, int family, + int type, int protocol, int kern) +{ + struct aa_label *label; + + if (kern) { + struct aa_ns *ns = aa_get_current_ns(); + + label = aa_get_label(ns_unconfined(ns)); + aa_put_ns(ns); + } else + label = aa_get_current_label(); + + if (sock->sk) { + struct aa_sk_ctx *ctx = SK_CTX(sock->sk); + + aa_put_label(ctx->label); + ctx->label = aa_get_label(label); + } + aa_put_label(label); + + return 0; +} + +/** + * apparmor_socket_bind - check perms before bind addr to socket + */ +static int apparmor_socket_bind(struct socket *sock, + struct sockaddr *address, int addrlen) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(!address); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + bind_perm(sock, address, addrlen), + aa_sk_perm(OP_BIND, AA_MAY_BIND, sock->sk)); +} + +/** + * apparmor_socket_connect - check perms before connecting @sock to @address + */ +static int apparmor_socket_connect(struct socket *sock, + struct sockaddr *address, int addrlen) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(!address); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + connect_perm(sock, address, addrlen), + aa_sk_perm(OP_CONNECT, AA_MAY_CONNECT, sock->sk)); +} + +/** + * apparmor_socket_list - check perms before allowing listen + */ +static int apparmor_socket_listen(struct socket *sock, int backlog) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + listen_perm(sock, backlog), + aa_sk_perm(OP_LISTEN, AA_MAY_LISTEN, sock->sk)); +} + +/** + * apparmor_socket_accept - check perms before accepting a new connection. + * + * Note: while @newsock is created and has some information, the accept + * has not been done. + */ +static int apparmor_socket_accept(struct socket *sock, struct socket *newsock) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(!newsock); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + accept_perm(sock, newsock), + aa_sk_perm(OP_ACCEPT, AA_MAY_ACCEPT, sock->sk)); +} + +static int aa_sock_msg_perm(const char *op, u32 request, struct socket *sock, + struct msghdr *msg, int size) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(!msg); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + msg_perm(op, request, sock, msg, size), + aa_sk_perm(op, request, sock->sk)); +} + +/** + * apparmor_socket_sendmsg - check perms before sending msg to another socket + */ +static int apparmor_socket_sendmsg(struct socket *sock, + struct msghdr *msg, int size) +{ + return aa_sock_msg_perm(OP_SENDMSG, AA_MAY_SEND, sock, msg, size); +} + +/** + * apparmor_socket_recvmsg - check perms before receiving a message + */ +static int apparmor_socket_recvmsg(struct socket *sock, + struct msghdr *msg, int size, int flags) +{ + return aa_sock_msg_perm(OP_RECVMSG, AA_MAY_RECEIVE, sock, msg, size); +} + +/* revaliation, get/set attr, shutdown */ +static int aa_sock_perm(const char *op, u32 request, struct socket *sock) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + sock_perm(op, request, sock), + aa_sk_perm(op, request, sock->sk)); +} + +/** + * apparmor_socket_getsockname - check perms before getting the local address + */ +static int apparmor_socket_getsockname(struct socket *sock) +{ + return aa_sock_perm(OP_GETSOCKNAME, AA_MAY_GETATTR, sock); +} + +/** + * apparmor_socket_getpeername - check perms before getting remote address + */ +static int apparmor_socket_getpeername(struct socket *sock) +{ + return aa_sock_perm(OP_GETPEERNAME, AA_MAY_GETATTR, sock); +} + +/* revaliation, get/set attr, opt */ +static int aa_sock_opt_perm(const char *op, u32 request, struct socket *sock, + int level, int optname) +{ + AA_BUG(!sock); + AA_BUG(!sock->sk); + AA_BUG(in_interrupt()); + + return af_select(sock->sk->sk_family, + opt_perm(op, request, sock, level, optname), + aa_sk_perm(op, request, sock->sk)); +} + +/** + * apparmor_getsockopt - check perms before getting socket options + */ +static int apparmor_socket_getsockopt(struct socket *sock, int level, + int optname) +{ + return aa_sock_opt_perm(OP_GETSOCKOPT, AA_MAY_GETOPT, sock, + level, optname); +} + +/** + * apparmor_setsockopt - check perms before setting socket options + */ +static int apparmor_socket_setsockopt(struct socket *sock, int level, + int optname) +{ + return aa_sock_opt_perm(OP_SETSOCKOPT, AA_MAY_SETOPT, sock, + level, optname); +} + +/** + * apparmor_socket_shutdown - check perms before shutting down @sock conn + */ +static int apparmor_socket_shutdown(struct socket *sock, int how) +{ + return aa_sock_perm(OP_SHUTDOWN, AA_MAY_SHUTDOWN, sock); +} + +/** + * apparmor_socket_sock_recv_skb - check perms before associating skb to sk + * + * Note: can not sleep may be called with locks held + * + * dont want protocol specific in __skb_recv_datagram() + * to deny an incoming connection socket_sock_rcv_skb() + */ +static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) +{ + return 0; +} + + +static struct aa_label *sk_peer_label(struct sock *sk) +{ + struct aa_sk_ctx *ctx = SK_CTX(sk); + + if (ctx->peer) + return ctx->peer; + + return ERR_PTR(-ENOPROTOOPT); +} + +/** + * apparmor_socket_getpeersec_stream - get security context of peer + * + * Note: for tcp only valid if using ipsec or cipso on lan + */ +static int apparmor_socket_getpeersec_stream(struct socket *sock, + char __user *optval, + int __user *optlen, + unsigned int len) +{ + char *name; + int slen, error = 0; + struct aa_label *label; + struct aa_label *peer; + + label = begin_current_label_crit_section(); + peer = sk_peer_label(sock->sk); + if (IS_ERR(peer)) { + error = PTR_ERR(peer); + goto done; + } + slen = aa_label_asxprint(&name, labels_ns(label), peer, + FLAG_SHOW_MODE | FLAG_VIEW_SUBNS | + FLAG_HIDDEN_UNCONFINED, GFP_KERNEL); + /* don't include terminating \0 in slen, it breaks some apps */ + if (slen < 0) { + error = -ENOMEM; + } else { + if (slen > len) { + error = -ERANGE; + } else if (copy_to_user(optval, name, slen)) { + error = -EFAULT; + goto out; + } + if (put_user(slen, optlen)) + error = -EFAULT; +out: + kfree(name); + + } + +done: + end_current_label_crit_section(label); + + return error; +} + +/** + * apparmor_socket_getpeersec_dgram - get security label of packet + * @sock: the peer socket + * @skb: packet data + * @secid: pointer to where to put the secid of the packet + * + * Sets the netlabel socket state on sk from parent + */ +static int apparmor_socket_getpeersec_dgram(struct socket *sock, + struct sk_buff *skb, u32 *secid) + +{ + /* TODO: requires secid support */ + return -ENOPROTOOPT; +} + +/** + * apparmor_sock_graft - Initialize newly created socket + * @sk: child sock + * @parent: parent socket + * + * Note: could set off of SOCK_CTX(parent) but need to track inode and we can + * just set sk security information off of current creating process label + * Labeling of sk for accept case - probably should be sock based + * instead of task, because of the case where an implicitly labeled + * socket is shared by different tasks. + */ +static void apparmor_sock_graft(struct sock *sk, struct socket *parent) +{ + struct aa_sk_ctx *ctx = SK_CTX(sk); + + if (!ctx->label) + ctx->label = aa_get_current_label(); +} + static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check), LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme), @@ -777,6 +1145,30 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(getprocattr, apparmor_getprocattr), LSM_HOOK_INIT(setprocattr, apparmor_setprocattr), + LSM_HOOK_INIT(sk_alloc_security, apparmor_sk_alloc_security), + LSM_HOOK_INIT(sk_free_security, apparmor_sk_free_security), + LSM_HOOK_INIT(sk_clone_security, apparmor_sk_clone_security), + + LSM_HOOK_INIT(socket_create, apparmor_socket_create), + LSM_HOOK_INIT(socket_post_create, apparmor_socket_post_create), + LSM_HOOK_INIT(socket_bind, apparmor_socket_bind), + LSM_HOOK_INIT(socket_connect, apparmor_socket_connect), + LSM_HOOK_INIT(socket_listen, apparmor_socket_listen), + LSM_HOOK_INIT(socket_accept, apparmor_socket_accept), + LSM_HOOK_INIT(socket_sendmsg, apparmor_socket_sendmsg), + LSM_HOOK_INIT(socket_recvmsg, apparmor_socket_recvmsg), + LSM_HOOK_INIT(socket_getsockname, apparmor_socket_getsockname), + LSM_HOOK_INIT(socket_getpeername, apparmor_socket_getpeername), + LSM_HOOK_INIT(socket_getsockopt, apparmor_socket_getsockopt), + LSM_HOOK_INIT(socket_setsockopt, apparmor_socket_setsockopt), + LSM_HOOK_INIT(socket_shutdown, apparmor_socket_shutdown), + LSM_HOOK_INIT(socket_sock_rcv_skb, apparmor_socket_sock_rcv_skb), + LSM_HOOK_INIT(socket_getpeersec_stream, + apparmor_socket_getpeersec_stream), + LSM_HOOK_INIT(socket_getpeersec_dgram, + apparmor_socket_getpeersec_dgram), + LSM_HOOK_INIT(sock_graft, apparmor_sock_graft), + LSM_HOOK_INIT(cred_alloc_blank, apparmor_cred_alloc_blank), LSM_HOOK_INIT(cred_free, apparmor_cred_free), LSM_HOOK_INIT(cred_prepare, apparmor_cred_prepare), diff --git a/security/apparmor/net.c b/security/apparmor/net.c new file mode 100644 index 000000000000..bb24cfa0a164 --- /dev/null +++ b/security/apparmor/net.c @@ -0,0 +1,187 @@ +/* + * AppArmor security module + * + * This file contains AppArmor network mediation + * + * Copyright (C) 1998-2008 Novell/SUSE + * Copyright 2009-2017 Canonical Ltd. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation, version 2 of the + * License. + */ + +#include "include/apparmor.h" +#include "include/audit.h" +#include "include/cred.h" +#include "include/label.h" +#include "include/net.h" +#include "include/policy.h" + +#include "net_names.h" + + +struct aa_sfs_entry aa_sfs_entry_network[] = { + AA_SFS_FILE_STRING("af_mask", AA_SFS_AF_MASK), + { } +}; + +static const char * const net_mask_names[] = { + "unknown", + "send", + "receive", + "unknown", + + "create", + "shutdown", + "connect", + "unknown", + + "setattr", + "getattr", + "setcred", + "getcred", + + "chmod", + "chown", + "chgrp", + "lock", + + "mmap", + "mprot", + "unknown", + "unknown", + + "accept", + "bind", + "listen", + "unknown", + + "setopt", + "getopt", + "unknown", + "unknown", + + "unknown", + "unknown", + "unknown", + "unknown", +}; + + +/* audit callback for net specific fields */ +void audit_net_cb(struct audit_buffer *ab, void *va) +{ + struct common_audit_data *sa = va; + + audit_log_format(ab, " family="); + if (address_family_names[sa->u.net->family]) + audit_log_string(ab, address_family_names[sa->u.net->family]); + else + audit_log_format(ab, "\"unknown(%d)\"", sa->u.net->family); + audit_log_format(ab, " sock_type="); + if (sock_type_names[aad(sa)->net.type]) + audit_log_string(ab, sock_type_names[aad(sa)->net.type]); + else + audit_log_format(ab, "\"unknown(%d)\"", aad(sa)->net.type); + audit_log_format(ab, " protocol=%d", aad(sa)->net.protocol); + + if (aad(sa)->request & NET_PERMS_MASK) { + audit_log_format(ab, " requested_mask="); + aa_audit_perm_mask(ab, aad(sa)->request, NULL, 0, + net_mask_names, NET_PERMS_MASK); + + if (aad(sa)->denied & NET_PERMS_MASK) { + audit_log_format(ab, " denied_mask="); + aa_audit_perm_mask(ab, aad(sa)->denied, NULL, 0, + net_mask_names, NET_PERMS_MASK); + } + } + if (aad(sa)->peer) { + audit_log_format(ab, " peer="); + aa_label_xaudit(ab, labels_ns(aad(sa)->label), aad(sa)->peer, + FLAGS_NONE, GFP_ATOMIC); + } +} + +/* Generic af perm */ +int aa_profile_af_perm(struct aa_profile *profile, struct common_audit_data *sa, + u32 request, u16 family, int type) +{ + struct aa_perms perms = { }; + unsigned int state; + __be16 buffer[2]; + + AA_BUG(family >= AF_MAX); + AA_BUG(type < 0 || type >= SOCK_MAX); + + if (profile_unconfined(profile)) + return 0; + state = PROFILE_MEDIATES(profile, AA_CLASS_NET); + if (!state) + return 0; + + buffer[0] = cpu_to_be16(family); + buffer[1] = cpu_to_be16((u16) type); + state = aa_dfa_match_len(profile->policy.dfa, state, (char *) &buffer, + 4); + aa_compute_perms(profile->policy.dfa, state, &perms); + aa_apply_modes_to_perms(profile, &perms); + + return aa_check_perms(profile, &perms, request, sa, audit_net_cb); +} + +int aa_af_perm(struct aa_label *label, const char *op, u32 request, u16 family, + int type, int protocol) +{ + struct aa_profile *profile; + DEFINE_AUDIT_NET(sa, op, NULL, family, type, protocol); + + return fn_for_each_confined(label, profile, + aa_profile_af_perm(profile, &sa, request, family, + type)); +} + +static int aa_label_sk_perm(struct aa_label *label, const char *op, u32 request, + struct sock *sk) +{ + struct aa_profile *profile; + DEFINE_AUDIT_SK(sa, op, sk); + + AA_BUG(!label); + AA_BUG(!sk); + + if (unconfined(label)) + return 0; + + return fn_for_each_confined(label, profile, + aa_profile_af_sk_perm(profile, &sa, request, sk)); +} + +int aa_sk_perm(const char *op, u32 request, struct sock *sk) +{ + struct aa_label *label; + int error; + + AA_BUG(!sk); + AA_BUG(in_interrupt()); + + /* TODO: switch to begin_current_label ???? */ + label = begin_current_label_crit_section(); + error = aa_label_sk_perm(label, op, request, sk); + end_current_label_crit_section(label); + + return error; +} + + +int aa_sock_file_perm(struct aa_label *label, const char *op, u32 request, + struct socket *sock) +{ + AA_BUG(!label); + AA_BUG(!sock); + AA_BUG(!sock->sk); + + return aa_label_sk_perm(label, op, request, sock->sk); +} diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c index 8a31ddd474d7..b9e6b2cafa69 100644 --- a/security/apparmor/policy_unpack.c +++ b/security/apparmor/policy_unpack.c @@ -37,7 +37,8 @@ #define v5 5 /* base version */ #define v6 6 /* per entry policydb mediation check */ -#define v7 7 /* full network masking */ +#define v7 7 +#define v8 8 /* full network masking */ /* * The AppArmor interface treats data as a type byte followed by the From b9590ad4c4f2fedc364016613f2af74ea7758bea Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sat, 3 Mar 2018 01:59:02 -0800 Subject: [PATCH 31/36] apparmor: remove POLICY_MEDIATES_SAFE The unpack code now makes sure every profile has a dfa so the safe version of POLICY_MEDIATES is no longer needed. Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 2 +- security/apparmor/include/policy.h | 12 +----------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 10d16e3abed9..701cb3e5ec3b 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -619,7 +619,7 @@ static void profile_query_cb(struct aa_profile *profile, struct aa_perms *perms, tmp = aa_compute_fperms(dfa, state, &cond); } } else if (profile->policy.dfa) { - if (!PROFILE_MEDIATES_SAFE(profile, *match_str)) + if (!PROFILE_MEDIATES(profile, *match_str)) return; /* no change to current perms */ dfa = profile->policy.dfa; state = aa_dfa_match_len(dfa, profile->policy.start[0], diff --git a/security/apparmor/include/policy.h b/security/apparmor/include/policy.h index ffe12a2366e0..ab64c6b5db5a 100644 --- a/security/apparmor/include/policy.h +++ b/security/apparmor/include/policy.h @@ -214,17 +214,7 @@ static inline struct aa_profile *aa_get_newest_profile(struct aa_profile *p) return labels_profile(aa_get_newest_label(&p->label)); } -#define PROFILE_MEDIATES(P, T) ((P)->policy.start[(T)]) -/* safe version of POLICY_MEDIATES for full range input */ -static inline unsigned int PROFILE_MEDIATES_SAFE(struct aa_profile *profile, - unsigned char class) -{ - if (profile->policy.dfa) - return aa_dfa_match_len(profile->policy.dfa, - profile->policy.start[0], &class, 1); - return 0; -} - +#define PROFILE_MEDIATES(P, T) ((P)->policy.start[(unsigned char) (T)]) static inline unsigned int PROFILE_MEDIATES_AF(struct aa_profile *profile, u16 AF) { unsigned int state = PROFILE_MEDIATES(profile, AA_CLASS_NET); From e540c3c901934e23e5eaa9adec8aba1af317220c Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 13 Mar 2018 20:53:08 -0700 Subject: [PATCH 32/36] apparmor: update MAINTAINERS file git and wiki locations The apparmor information in the MAINTAINERS file is out of date update it to the correct git reference for the master apparmor tree. And update the wiki location to use apparmor.net which forwards to the current wiki location on gitlab.com. Signed-off-by: John Johansen --- MAINTAINERS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 845fc25812f1..2c0eceb2ac34 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -915,8 +915,8 @@ F: drivers/char/apm-emulation.c APPARMOR SECURITY MODULE M: John Johansen L: apparmor@lists.ubuntu.com (subscribers-only, general discussion) -W: apparmor.wiki.kernel.org -T: git git://git.kernel.org/pub/scm/linux/kernel/git/jj/apparmor-dev.git +W: wiki.apparmor.net +T: git git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor S: Supported F: security/apparmor/ F: Documentation/admin-guide/LSM/apparmor.rst From a61ecd329cfa951b7d36c13e9e2a07e7761c0e89 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Fri, 23 Mar 2018 23:34:22 +0000 Subject: [PATCH 33/36] apparmor: fix error returns checks by making size a ssize_t Currently variable size is a unsigned size_t, hence comparisons to see if it is less than zero (for error checking) will always be false. Fix this by making size a ssize_t Detected by CoverityScan, CID#1466080 ("Unsigned compared against 0") Fixes: 8e51f9087f40 ("apparmor: Add support for attaching profiles via xattr, presence and value") Signed-off-by: Colin Ian King Signed-off-by: John Johansen --- security/apparmor/domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 57cc892e05a2..590b7e8cd21c 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -314,7 +314,7 @@ static int aa_xattrs_match(const struct linux_binprm *bprm, struct aa_profile *profile, unsigned int state) { int i; - size_t size; + ssize_t size; struct dentry *d; char *value = NULL; int value_size = 0, ret = profile->xattr_count; From d53c9f4d212c25b09670a71e2a993071d1e637a2 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Mon, 19 Mar 2018 12:12:31 +0300 Subject: [PATCH 34/36] apparmor: Fix an error code in verify_table_headers() We accidentally return a positive EPROTO instead of a negative -EPROTO. Since 71 is not an error pointer, that means it eventually results in an Oops in the caller. Fixes: d901d6a298dc ("apparmor: dfa split verification of table headers") Signed-off-by: Dan Carpenter Signed-off-by: John Johansen --- security/apparmor/match.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/apparmor/match.c b/security/apparmor/match.c index dd4c995c5e25..280eba082c7b 100644 --- a/security/apparmor/match.c +++ b/security/apparmor/match.c @@ -198,7 +198,7 @@ static int verify_table_headers(struct table_header **tables, int flags) static int verify_dfa(struct aa_dfa *dfa) { size_t i, state_count, trans_count; - int error = EPROTO; + int error = -EPROTO; state_count = dfa->tables[YYTD_ID_BASE]->td_lolen; trans_count = dfa->tables[YYTD_ID_NXT]->td_lolen; From 1180b4c757aab5506f1be367000364dd5cf5cd02 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Thu, 15 Mar 2018 22:31:38 -0700 Subject: [PATCH 35/36] apparmor: fix dangling symlinks to policy rawdata after replacement When policy replacement occurs the symlinks in the profile directory need to be updated to point to the new rawdata, otherwise once the old rawdata is removed the symlink becomes broken. Fix this by dynamically generating the symlink everytime it is read. These links are used enough that their value needs to be cached and this way we can avoid needing locking to read and update the link value. Fixes: a481f4d917835 ("apparmor: add custom apparmorfs that will be used by policy namespace files") BugLink: http://bugs.launchpad.net/bugs/1755563 Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 126 +++++++++++++++++++++++++-------- 1 file changed, 95 insertions(+), 31 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 701cb3e5ec3b..62301ddbbe5e 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -310,6 +310,7 @@ static struct dentry *aafs_create_dir(const char *name, struct dentry *parent) * @name: name of dentry to create * @parent: parent directory for this dentry * @target: if symlink, symlink target string + * @private: private data * @iops: struct of inode_operations that should be used * * If @target parameter is %NULL, then the @iops parameter needs to be @@ -318,17 +319,17 @@ static struct dentry *aafs_create_dir(const char *name, struct dentry *parent) static struct dentry *aafs_create_symlink(const char *name, struct dentry *parent, const char *target, + void *private, const struct inode_operations *iops) { struct dentry *dent; char *link = NULL; if (target) { - link = kstrdup(target, GFP_KERNEL); if (!link) return ERR_PTR(-ENOMEM); } - dent = aafs_create(name, S_IFLNK | 0444, parent, NULL, link, NULL, + dent = aafs_create(name, S_IFLNK | 0444, parent, private, link, NULL, iops); if (IS_ERR(dent)) kfree(link); @@ -1479,26 +1480,95 @@ static int profile_depth(struct aa_profile *profile) return depth; } -static int gen_symlink_name(char *buffer, size_t bsize, int depth, - const char *dirname, const char *fname) +static char *gen_symlink_name(int depth, const char *dirname, const char *fname) { + char *buffer, *s; int error; + int size = depth * 6 + strlen(dirname) + strlen(fname) + 11; + + s = buffer = kmalloc(size, GFP_KERNEL); + if (!buffer) + return ERR_PTR(-ENOMEM); for (; depth > 0; depth--) { - if (bsize < 7) - return -ENAMETOOLONG; - strcpy(buffer, "../../"); - buffer += 6; - bsize -= 6; + strcpy(s, "../../"); + s += 6; + size -= 6; } - error = snprintf(buffer, bsize, "raw_data/%s/%s", dirname, fname); - if (error >= bsize || error < 0) - return -ENAMETOOLONG; + error = snprintf(s, size, "raw_data/%s/%s", dirname, fname); + if (error >= size || error < 0) + return ERR_PTR(-ENAMETOOLONG); - return 0; + return buffer; } +static void rawdata_link_cb(void *arg) +{ + kfree(arg); +} + +static const char *rawdata_get_link_base(struct dentry *dentry, + struct inode *inode, + struct delayed_call *done, + const char *name) +{ + struct aa_proxy *proxy = inode->i_private; + struct aa_label *label; + struct aa_profile *profile; + char *target; + int depth; + + if (!dentry) + return ERR_PTR(-ECHILD); + + label = aa_get_label_rcu(&proxy->label); + profile = labels_profile(label); + depth = profile_depth(profile); + target = gen_symlink_name(depth, profile->rawdata->name, name); + aa_put_label(label); + + if (IS_ERR(target)) + return target; + + set_delayed_call(done, rawdata_link_cb, target); + + return target; +} + +static const char *rawdata_get_link_sha1(struct dentry *dentry, + struct inode *inode, + struct delayed_call *done) +{ + return rawdata_get_link_base(dentry, inode, done, "sha1"); +} + +static const char *rawdata_get_link_abi(struct dentry *dentry, + struct inode *inode, + struct delayed_call *done) +{ + return rawdata_get_link_base(dentry, inode, done, "abi"); +} + +static const char *rawdata_get_link_data(struct dentry *dentry, + struct inode *inode, + struct delayed_call *done) +{ + return rawdata_get_link_base(dentry, inode, done, "raw_data"); +} + +static const struct inode_operations rawdata_link_sha1_iops = { + .get_link = rawdata_get_link_sha1, +}; + +static const struct inode_operations rawdata_link_abi_iops = { + .get_link = rawdata_get_link_abi, +}; +static const struct inode_operations rawdata_link_data_iops = { + .get_link = rawdata_get_link_data, +}; + + /* * Requires: @profile->ns->lock held */ @@ -1569,34 +1639,28 @@ int __aafs_profile_mkdir(struct aa_profile *profile, struct dentry *parent) } if (profile->rawdata) { - char target[64]; - int depth = profile_depth(profile); - - error = gen_symlink_name(target, sizeof(target), depth, - profile->rawdata->name, "sha1"); - if (error < 0) - goto fail2; - dent = aafs_create_symlink("raw_sha1", dir, target, NULL); + dent = aafs_create_symlink("raw_sha1", dir, NULL, + profile->label.proxy, + &rawdata_link_sha1_iops); if (IS_ERR(dent)) goto fail; + aa_get_proxy(profile->label.proxy); profile->dents[AAFS_PROF_RAW_HASH] = dent; - error = gen_symlink_name(target, sizeof(target), depth, - profile->rawdata->name, "abi"); - if (error < 0) - goto fail2; - dent = aafs_create_symlink("raw_abi", dir, target, NULL); + dent = aafs_create_symlink("raw_abi", dir, NULL, + profile->label.proxy, + &rawdata_link_abi_iops); if (IS_ERR(dent)) goto fail; + aa_get_proxy(profile->label.proxy); profile->dents[AAFS_PROF_RAW_ABI] = dent; - error = gen_symlink_name(target, sizeof(target), depth, - profile->rawdata->name, "raw_data"); - if (error < 0) - goto fail2; - dent = aafs_create_symlink("raw_data", dir, target, NULL); + dent = aafs_create_symlink("raw_data", dir, NULL, + profile->label.proxy, + &rawdata_link_data_iops); if (IS_ERR(dent)) goto fail; + aa_get_proxy(profile->label.proxy); profile->dents[AAFS_PROF_RAW_DATA] = dent; } From 588558eb6d0e0b6edfa65a67e906c2ffeba63ff1 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Tue, 27 Mar 2018 14:35:58 +0100 Subject: [PATCH 36/36] apparmor: fix memory leak on buffer on error exit path Currently on the error exit path the allocated buffer is not free'd causing a memory leak. Fix this by kfree'ing it. Detected by CoverityScan, CID#1466876 ("Resource leaks") Fixes: 1180b4c757aa ("apparmor: fix dangling symlinks to policy rawdata after replacement") Signed-off-by: Colin Ian King Signed-off-by: John Johansen --- security/apparmor/apparmorfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 62301ddbbe5e..f4308683c0af 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -1497,8 +1497,10 @@ static char *gen_symlink_name(int depth, const char *dirname, const char *fname) } error = snprintf(s, size, "raw_data/%s/%s", dirname, fname); - if (error >= size || error < 0) + if (error >= size || error < 0) { + kfree(buffer); return ERR_PTR(-ENAMETOOLONG); + } return buffer; }