| Message ID | 20180604205455.2325754-5-stefanb@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jun 4, 2018 at 4:54 PM, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote: > The AUDIT_INTEGRITY_RULE is used for auditing IMA policy rules and > the IMA "audit" policy action. This patch defines > AUDIT_INTEGRITY_POLICY_RULE to reflect the IMA policy rules. > > Since we defined a new message type we can now also pass the > audit_context and get an associated SYSCALL record. This now produces > the following records when parsing IMA policy's rules: Aaand now I see you included the current->audit_context pointer I mentioned in my comments for 3/4 ;) So basically this should be fine, although I should point out that you do not need to define a new message type to associate records together. The fact that we don't associate all connected records is basically a bug. Anyway, patches 3/4 and 4/4 look good to me. Considering this is likely going in during the *next* merge window, I would ask that you convert from "current->audit_context" to "audit_context()" as soon as this merge window closes. Thanks! > type=UNKNOWN[1807] msg=audit(1527888965.738:320): action=audit \ > func=MMAP_CHECK mask=MAY_EXEC res=1 > type=UNKNOWN[1807] msg=audit(1527888965.738:320): action=audit \ > func=FILE_CHECK mask=MAY_READ res=1 > type=SYSCALL msg=audit(1527888965.738:320): arch=c000003e syscall=1 \ > success=yes exit=17 a0=1 a1=55bcfcca9030 a2=11 a3=7fcc1b55fb38 \ > items=0 ppid=1567 pid=1601 auid=0 uid=0 gid=0 euid=0 suid=0 \ > fsuid=0 egid=0 sgid=0 fsgid=0 tty=tty2 ses=2 comm="echo" \ > exe="/usr/bin/echo" \ > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) > > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> > --- > include/uapi/linux/audit.h | 1 + > security/integrity/ima/ima_policy.c | 4 ++-- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h > index 65d9293f1fb8..cb358551376b 100644 > --- a/include/uapi/linux/audit.h > +++ b/include/uapi/linux/audit.h > @@ -148,6 +148,7 @@ > #define AUDIT_INTEGRITY_PCR 1804 /* PCR invalidation msgs */ > #define AUDIT_INTEGRITY_RULE 1805 /* policy rule */ > #define AUDIT_INTEGRITY_EVM_XATTR 1806 /* New EVM-covered xattr */ > +#define AUDIT_INTEGRITY_POLICY_RULE 1807 /* IMA policy rules */ > > #define AUDIT_KERNEL 2000 /* Asynchronous audit record. NOT A REQUEST. */ > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index bc99713dfe57..f7230db217a7 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -652,8 +652,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > bool uid_token; > int result = 0; > > - ab = integrity_audit_log_start(NULL, GFP_KERNEL, > - AUDIT_INTEGRITY_RULE); > + ab = integrity_audit_log_start(current->audit_context, GFP_KERNEL, > + AUDIT_INTEGRITY_POLICY_RULE); > > entry->uid = INVALID_UID; > entry->fowner = INVALID_UID; > -- > 2.13.6 >
Hi Paul, On Mon, 2018-06-04 at 20:21 -0400, Paul Moore wrote: > On Mon, Jun 4, 2018 at 4:54 PM, Stefan Berger > <stefanb@linux.vnet.ibm.com> wrote: > > The AUDIT_INTEGRITY_RULE is used for auditing IMA policy rules and > > the IMA "audit" policy action. This patch defines > > AUDIT_INTEGRITY_POLICY_RULE to reflect the IMA policy rules. > > > > Since we defined a new message type we can now also pass the > > audit_context and get an associated SYSCALL record. This now produces > > the following records when parsing IMA policy's rules: > > Aaand now I see you included the current->audit_context pointer I > mentioned in my comments for 3/4 ;) > > So basically this should be fine, although I should point out that you > do not need to define a new message type to associate records > together. The fact that we don't associate all connected records is > basically a bug. > > Anyway, patches 3/4 and 4/4 look good to me. Considering this is > likely going in during the *next* merge window, I would ask that you > convert from "current->audit_context" to "audit_context()" as soon as > this merge window closes. > > Thanks! Thanks, Paul. I'd like to start queueing patches for the next open window now, instead of scrambling later. Can I add your Ack now, and remember to make this change when rebasing? Mimi > > > type=UNKNOWN[1807] msg=audit(1527888965.738:320): action=audit \ > > func=MMAP_CHECK mask=MAY_EXEC res=1 > > type=UNKNOWN[1807] msg=audit(1527888965.738:320): action=audit \ > > func=FILE_CHECK mask=MAY_READ res=1 > > type=SYSCALL msg=audit(1527888965.738:320): arch=c000003e syscall=1 \ > > success=yes exit=17 a0=1 a1=55bcfcca9030 a2=11 a3=7fcc1b55fb38 \ > > items=0 ppid=1567 pid=1601 auid=0 uid=0 gid=0 euid=0 suid=0 \ > > fsuid=0 egid=0 sgid=0 fsgid=0 tty=tty2 ses=2 comm="echo" \ > > exe="/usr/bin/echo" \ > > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) > > > > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> > > --- > > include/uapi/linux/audit.h | 1 + > > security/integrity/ima/ima_policy.c | 4 ++-- > > 2 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h > > index 65d9293f1fb8..cb358551376b 100644 > > --- a/include/uapi/linux/audit.h > > +++ b/include/uapi/linux/audit.h > > @@ -148,6 +148,7 @@ > > #define AUDIT_INTEGRITY_PCR 1804 /* PCR invalidation msgs */ > > #define AUDIT_INTEGRITY_RULE 1805 /* policy rule */ > > #define AUDIT_INTEGRITY_EVM_XATTR 1806 /* New EVM-covered xattr */ > > +#define AUDIT_INTEGRITY_POLICY_RULE 1807 /* IMA policy rules */ > > > > #define AUDIT_KERNEL 2000 /* Asynchronous audit record. NOT A REQUEST. */ > > > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > > index bc99713dfe57..f7230db217a7 100644 > > --- a/security/integrity/ima/ima_policy.c > > +++ b/security/integrity/ima/ima_policy.c > > @@ -652,8 +652,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > > bool uid_token; > > int result = 0; > > > > - ab = integrity_audit_log_start(NULL, GFP_KERNEL, > > - AUDIT_INTEGRITY_RULE); > > + ab = integrity_audit_log_start(current->audit_context, GFP_KERNEL, > > + AUDIT_INTEGRITY_POLICY_RULE); > > > > entry->uid = INVALID_UID; > > entry->fowner = INVALID_UID; > > -- > > 2.13.6 > > >
On Tue, Jun 5, 2018 at 10:15 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > Hi Paul, > > On Mon, 2018-06-04 at 20:21 -0400, Paul Moore wrote: >> On Mon, Jun 4, 2018 at 4:54 PM, Stefan Berger >> <stefanb@linux.vnet.ibm.com> wrote: >> > The AUDIT_INTEGRITY_RULE is used for auditing IMA policy rules and >> > the IMA "audit" policy action. This patch defines >> > AUDIT_INTEGRITY_POLICY_RULE to reflect the IMA policy rules. >> > >> > Since we defined a new message type we can now also pass the >> > audit_context and get an associated SYSCALL record. This now produces >> > the following records when parsing IMA policy's rules: >> >> Aaand now I see you included the current->audit_context pointer I >> mentioned in my comments for 3/4 ;) >> >> So basically this should be fine, although I should point out that you >> do not need to define a new message type to associate records >> together. The fact that we don't associate all connected records is >> basically a bug. >> >> Anyway, patches 3/4 and 4/4 look good to me. Considering this is >> likely going in during the *next* merge window, I would ask that you >> convert from "current->audit_context" to "audit_context()" as soon as >> this merge window closes. >> >> Thanks! > > Thanks, Paul. I'd like to start queueing patches for the next open > window now, instead of scrambling later. Can I add your Ack now, and > remember to make this change when rebasing? Sure, go ahead and add my ACK to both 3/4 and 4/4 as long as you double pinky swear you'll do the audit_context() fix-up during the merge :) Acked-by: Paul Moore <paul@paul-moore.com>
On Tue, 2018-06-05 at 18:18 -0400, Paul Moore wrote: > On Tue, Jun 5, 2018 at 10:15 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > Hi Paul, > > > > On Mon, 2018-06-04 at 20:21 -0400, Paul Moore wrote: > >> On Mon, Jun 4, 2018 at 4:54 PM, Stefan Berger > >> <stefanb@linux.vnet.ibm.com> wrote: > >> > The AUDIT_INTEGRITY_RULE is used for auditing IMA policy rules and > >> > the IMA "audit" policy action. This patch defines > >> > AUDIT_INTEGRITY_POLICY_RULE to reflect the IMA policy rules. > >> > > >> > Since we defined a new message type we can now also pass the > >> > audit_context and get an associated SYSCALL record. This now produces > >> > the following records when parsing IMA policy's rules: > >> > >> Aaand now I see you included the current->audit_context pointer I > >> mentioned in my comments for 3/4 ;) > >> > >> So basically this should be fine, although I should point out that you > >> do not need to define a new message type to associate records > >> together. The fact that we don't associate all connected records is > >> basically a bug. > >> > >> Anyway, patches 3/4 and 4/4 look good to me. Considering this is > >> likely going in during the *next* merge window, I would ask that you > >> convert from "current->audit_context" to "audit_context()" as soon as > >> this merge window closes. > >> > >> Thanks! > > > > Thanks, Paul. I'd like to start queueing patches for the next open > > window now, instead of scrambling later. Can I add your Ack now, and > > remember to make this change when rebasing? > > Sure, go ahead and add my ACK to both 3/4 and 4/4 as long as you > double pinky swear you'll do the audit_context() fix-up during the > merge :) > > Acked-by: Paul Moore <paul@paul-moore.com> Sure, it will be really hard to miss. The next-integrity-queued branch has: Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> *** Remember replace current->audit_context with call to audit_context() *** Acked-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h index 65d9293f1fb8..cb358551376b 100644 --- a/include/uapi/linux/audit.h +++ b/include/uapi/linux/audit.h @@ -148,6 +148,7 @@ #define AUDIT_INTEGRITY_PCR 1804 /* PCR invalidation msgs */ #define AUDIT_INTEGRITY_RULE 1805 /* policy rule */ #define AUDIT_INTEGRITY_EVM_XATTR 1806 /* New EVM-covered xattr */ +#define AUDIT_INTEGRITY_POLICY_RULE 1807 /* IMA policy rules */ #define AUDIT_KERNEL 2000 /* Asynchronous audit record. NOT A REQUEST. */ diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index bc99713dfe57..f7230db217a7 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -652,8 +652,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) bool uid_token; int result = 0; - ab = integrity_audit_log_start(NULL, GFP_KERNEL, - AUDIT_INTEGRITY_RULE); + ab = integrity_audit_log_start(current->audit_context, GFP_KERNEL, + AUDIT_INTEGRITY_POLICY_RULE); entry->uid = INVALID_UID; entry->fowner = INVALID_UID;
The AUDIT_INTEGRITY_RULE is used for auditing IMA policy rules and the IMA "audit" policy action. This patch defines AUDIT_INTEGRITY_POLICY_RULE to reflect the IMA policy rules. Since we defined a new message type we can now also pass the audit_context and get an associated SYSCALL record. This now produces the following records when parsing IMA policy's rules: type=UNKNOWN[1807] msg=audit(1527888965.738:320): action=audit \ func=MMAP_CHECK mask=MAY_EXEC res=1 type=UNKNOWN[1807] msg=audit(1527888965.738:320): action=audit \ func=FILE_CHECK mask=MAY_READ res=1 type=SYSCALL msg=audit(1527888965.738:320): arch=c000003e syscall=1 \ success=yes exit=17 a0=1 a1=55bcfcca9030 a2=11 a3=7fcc1b55fb38 \ items=0 ppid=1567 pid=1601 auid=0 uid=0 gid=0 euid=0 suid=0 \ fsuid=0 egid=0 sgid=0 fsgid=0 tty=tty2 ses=2 comm="echo" \ exe="/usr/bin/echo" \ subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- include/uapi/linux/audit.h | 1 + security/integrity/ima/ima_policy.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-)