audit: force seccomp event logging to honor the audit_enabled flag

RFC PATCH In review Paul Moore 3 participants 4 messages Nov 23, 2015 – Nov 24, 2015 Audit

Share & tools

Paul Moore

author

2015-11-23 22:20 UTC #0 lore raw

RFC PATCH 1/1 audit: force seccomp event logging to honor the audit_enabled flag

Previously we were emitting seccomp audit records regardless of the audit_enabled setting, a deparature from the rest of audit. This patch makes seccomp auditing consistent with the rest of the audit record generation code in that when audit_enabled=0 nothing is logged by the audit subsystem.

The bulk of this patch is moving the CONFIG_AUDIT block ahead of the CONFIG_AUDITSYSCALL block in include/linux/audit.h; the only real code change was in the audit_seccomp() definition.

Reported-by Tony Jones <[email protected]>

Signed-off-by Paul Moore <[email protected]>

---
 include/linux/audit.h |  204 +++++++++++++++++++++++++------------------------
 1 file changed, 104 insertions(+), 100 deletions(-)

include/linux/audit.h +104 ~~-100~~

index 20eba1e..476bc12 100644
@@ -113,6 +113,107 @@ struct filename;
 
 extern void audit_log_session_info(struct audit_buffer *ab);
 
+#ifdef CONFIG_AUDIT
+/* These are defined in audit.c */
+				/* Public API */
+extern __printf(4, 5)
+void audit_log(struct audit_context *ctx, gfp_t gfp_mask, int type,
+	       const char *fmt, ...);
+
+extern struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, int type);
+extern __printf(2, 3)
+void audit_log_format(struct audit_buffer *ab, const char *fmt, ...);
+extern void		    audit_log_end(struct audit_buffer *ab);
+extern bool		    audit_string_contains_control(const char *string,
+							  size_t len);
+extern void		    audit_log_n_hex(struct audit_buffer *ab,
+					  const unsigned char *buf,
+					  size_t len);
+extern void		    audit_log_n_string(struct audit_buffer *ab,
+					       const char *buf,
+					       size_t n);
+extern void		    audit_log_n_untrustedstring(struct audit_buffer *ab,
+							const char *string,
+							size_t n);
+extern void		    audit_log_untrustedstring(struct audit_buffer *ab,
+						      const char *string);
+extern void		    audit_log_d_path(struct audit_buffer *ab,
+					     const char *prefix,
+					     const struct path *path);
+extern void		    audit_log_key(struct audit_buffer *ab,
+					  char *key);
+extern void		    audit_log_link_denied(const char *operation,
+						  struct path *link);
+extern void		    audit_log_lost(const char *message);
+#ifdef CONFIG_SECURITY
+extern void 		    audit_log_secctx(struct audit_buffer *ab, u32 secid);
+#else
+static inline void	    audit_log_secctx(struct audit_buffer *ab, u32 secid)
+{ }
+#endif
+
+extern int audit_log_task_context(struct audit_buffer *ab);
+extern void audit_log_task_info(struct audit_buffer *ab,
+				struct task_struct *tsk);
+
+extern int		    audit_update_lsm_rules(void);
+
+				/* Private API (for audit.c only) */
+extern int audit_filter_user(int type);
+extern int audit_filter_type(int type);
+extern int audit_rule_change(int type, __u32 portid, int seq,
+				void *data, size_t datasz);
+extern int audit_list_rules_send(struct sk_buff *request_skb, int seq);
+
+extern u32 audit_enabled;
+#else /* CONFIG_AUDIT */
+static inline __printf(4, 5)
+void audit_log(struct audit_context *ctx, gfp_t gfp_mask, int type,
+	       const char *fmt, ...)
+{ }
+static inline struct audit_buffer *audit_log_start(struct audit_context *ctx,
+						   gfp_t gfp_mask, int type)
+{
+	return NULL;
+}
+static inline __printf(2, 3)
+void audit_log_format(struct audit_buffer *ab, const char *fmt, ...)
+{ }
+static inline void audit_log_end(struct audit_buffer *ab)
+{ }
+static inline void audit_log_n_hex(struct audit_buffer *ab,
+				   const unsigned char *buf, size_t len)
+{ }
+static inline void audit_log_n_string(struct audit_buffer *ab,
+				      const char *buf, size_t n)
+{ }
+static inline void  audit_log_n_untrustedstring(struct audit_buffer *ab,
+						const char *string, size_t n)
+{ }
+static inline void audit_log_untrustedstring(struct audit_buffer *ab,
+					     const char *string)
+{ }
+static inline void audit_log_d_path(struct audit_buffer *ab,
+				    const char *prefix,
+				    const struct path *path)
+{ }
+static inline void audit_log_key(struct audit_buffer *ab, char *key)
+{ }
+static inline void audit_log_link_denied(const char *string,
+					 const struct path *link)
+{ }
+static inline void audit_log_secctx(struct audit_buffer *ab, u32 secid)
+{ }
+static inline int audit_log_task_context(struct audit_buffer *ab)
+{
+	return 0;
+}
+static inline void audit_log_task_info(struct audit_buffer *ab,
+				       struct task_struct *tsk)
+{ }
+#define audit_enabled 0
+#endif /* CONFIG_AUDIT */
+
 #ifdef CONFIG_AUDIT_COMPAT_GENERIC
 #define audit_is_compat(arch)  (!((arch) & __AUDIT_ARCH_64BIT))
 #else

@@ -212,6 +313,9 @@ void audit_core_dumps(long signr);
 
 static inline void audit_seccomp(unsigned long syscall, long signr, int code)
 {
+	if (!audit_enabled)
+		return;
+
 	/* Force a record to be reported if a signal was delivered. */
 	if (signr || unlikely(!audit_dummy_context()))
 		__audit_seccomp(syscall, signr, code);

@@ -446,106 +550,6 @@ static inline bool audit_loginuid_set(struct task_struct *tsk)
 	return uid_valid(audit_get_loginuid(tsk));
 }
 
-#ifdef CONFIG_AUDIT
-/* These are defined in audit.c */
-				/* Public API */
-extern __printf(4, 5)
-void audit_log(struct audit_context *ctx, gfp_t gfp_mask, int type,
-	       const char *fmt, ...);
-
-extern struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, int type);
-extern __printf(2, 3)
-void audit_log_format(struct audit_buffer *ab, const char *fmt, ...);
-extern void		    audit_log_end(struct audit_buffer *ab);
-extern bool		    audit_string_contains_control(const char *string,
-							  size_t len);
-extern void		    audit_log_n_hex(struct audit_buffer *ab,
-					  const unsigned char *buf,
-					  size_t len);
-extern void		    audit_log_n_string(struct audit_buffer *ab,
-					       const char *buf,
-					       size_t n);
-extern void		    audit_log_n_untrustedstring(struct audit_buffer *ab,
-							const char *string,
-							size_t n);
-extern void		    audit_log_untrustedstring(struct audit_buffer *ab,
-						      const char *string);
-extern void		    audit_log_d_path(struct audit_buffer *ab,
-					     const char *prefix,
-					     const struct path *path);
-extern void		    audit_log_key(struct audit_buffer *ab,
-					  char *key);
-extern void		    audit_log_link_denied(const char *operation,
-						  struct path *link);
-extern void		    audit_log_lost(const char *message);
-#ifdef CONFIG_SECURITY
-extern void 		    audit_log_secctx(struct audit_buffer *ab, u32 secid);
-#else
-static inline void	    audit_log_secctx(struct audit_buffer *ab, u32 secid)
-{ }
-#endif
-
-extern int audit_log_task_context(struct audit_buffer *ab);
-extern void audit_log_task_info(struct audit_buffer *ab,
-				struct task_struct *tsk);
-
-extern int		    audit_update_lsm_rules(void);
-
-				/* Private API (for audit.c only) */
-extern int audit_filter_user(int type);
-extern int audit_filter_type(int type);
-extern int audit_rule_change(int type, __u32 portid, int seq,
-				void *data, size_t datasz);
-extern int audit_list_rules_send(struct sk_buff *request_skb, int seq);
-
-extern u32 audit_enabled;
-#else /* CONFIG_AUDIT */
-static inline __printf(4, 5)
-void audit_log(struct audit_context *ctx, gfp_t gfp_mask, int type,
-	       const char *fmt, ...)
-{ }
-static inline struct audit_buffer *audit_log_start(struct audit_context *ctx,
-						   gfp_t gfp_mask, int type)
-{
-	return NULL;
-}
-static inline __printf(2, 3)
-void audit_log_format(struct audit_buffer *ab, const char *fmt, ...)
-{ }
-static inline void audit_log_end(struct audit_buffer *ab)
-{ }
-static inline void audit_log_n_hex(struct audit_buffer *ab,
-				   const unsigned char *buf, size_t len)
-{ }
-static inline void audit_log_n_string(struct audit_buffer *ab,
-				      const char *buf, size_t n)
-{ }
-static inline void  audit_log_n_untrustedstring(struct audit_buffer *ab,
-						const char *string, size_t n)
-{ }
-static inline void audit_log_untrustedstring(struct audit_buffer *ab,
-					     const char *string)
-{ }
-static inline void audit_log_d_path(struct audit_buffer *ab,
-				    const char *prefix,
-				    const struct path *path)
-{ }
-static inline void audit_log_key(struct audit_buffer *ab, char *key)
-{ }
-static inline void audit_log_link_denied(const char *string,
-					 const struct path *link)
-{ }
-static inline void audit_log_secctx(struct audit_buffer *ab, u32 secid)
-{ }
-static inline int audit_log_task_context(struct audit_buffer *ab)
-{
-	return 0;
-}
-static inline void audit_log_task_info(struct audit_buffer *ab,
-				       struct task_struct *tsk)
-{ }
-#define audit_enabled 0
-#endif /* CONFIG_AUDIT */
 static inline void audit_log_string(struct audit_buffer *ab, const char *buf)
 {
 	audit_log_n_string(ab, buf, strlen(buf));

Tony Jones

2015-11-23 22:20 UTC #1 lore raw

Quoted 6 lines · Paul Moore

Previously we were emitting seccomp audit records regardless of the audit_enabled setting, a deparature from the rest of audit. This patch makes seccomp auditing consistent with the rest of the audit record generation code in that when audit_enabled=0 nothing is logged by the audit subsystem.

Quoted 6 lines

The bulk of this patch is moving the CONFIG_AUDIT block ahead of the CONFIG_AUDITSYSCALL block in include/linux/audit.h; the only real code change was in the audit_seccomp() definition.
Reported-by Tony Jones <[email protected]>
Signed-off-by Paul Moore <[email protected]>

Seems pretty much the same (functionally) as the patch I posted to audit list on 10/12/2015 except that didn't hoist the entire block.

Signed-off-by Tony Jones <[email protected]>

Paul Moore

maintainer

2015-11-23 22:35 UTC #2 lore raw

Quoted 11 lines · Tony Jones

Quoted 10 lines · Paul Moore
Previously we were emitting seccomp audit records regardless of the audit_enabled setting, a deparature from the rest of audit. This patch makes seccomp auditing consistent with the rest of the audit record generation code in that when audit_enabled=0 nothing is logged by the audit subsystem.
The bulk of this patch is moving the CONFIG_AUDIT block ahead of the CONFIG_AUDITSYSCALL block in include/linux/audit.h; the only real code change was in the audit_seccomp() definition.

Quoted 5 lines

Quoted 2 lines
Reported-by Tony Jones <[email protected]>
Signed-off-by Paul Moore <[email protected]>
Seems pretty much the same (functionally) as the patch I posted to audit list on 10/12/2015 except that didn't hoist the entire block.

Yep, I prefered to move the block as I think it should have been that way anyway from the start. IMHO we got to many audit Kconfig knobs as-is and splitting that block for just the audit_enabled flag made things worse.

Quoted 1 line

Signed-off-by Tony Jones <[email protected]>

Signature · 2 lines

paul moore
www.paul-moore.com

Paul Moore

author

2015-11-24 18:57 UTC #3 lore raw

Quoted 15 lines · Paul Moore

Quoted 14 lines · Tony Jones
Quoted 12 lines · Paul Moore
Previously we were emitting seccomp audit records regardless of the audit_enabled setting, a deparature from the rest of audit. This patch makes seccomp auditing consistent with the rest of the audit record generation code in that when audit_enabled=0 nothing is logged by the audit subsystem.
The bulk of this patch is moving the CONFIG_AUDIT block ahead of the CONFIG_AUDITSYSCALL block in include/linux/audit.h; the only real code change was in the audit_seccomp() definition.
Reported-by Tony Jones <[email protected]>
Signed-off-by Paul Moore <[email protected]>

Quoted 9 lines

Quoted 2 lines
Seems pretty much the same (functionally) as the patch I posted to audit list on 10/12/2015 except that didn't hoist the entire block.
Yep, I prefered to move the block as I think it should have been that way anyway from the start. IMHO we got to many audit Kconfig knobs as-is and splitting that block for just the audit_enabled flag made things worse.
Quoted 1 line
Signed-off-by Tony Jones <[email protected]>

I just merged this patch into audit#next, the only change is I replaced the "Reported-by" for Tony with his sign-off.

Signature · 2 lines

paul moore
security @ redhat