integrity: Add result field in audit message
Subsystems (4)
PATCH v2 1/2 integrity: Add result field in audit message
Result code is not included in the audit messages logged by the integrity subsystem. Add "result" field in the audit messages logged by the integrity subsystem and set the value to the result code passed to integrity_audit_msg() in the "result" parameter.
Sample audit message:
[ 6.284329] audit: type=1804 audit(1591756723.627:2): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=kernel op=add_boot_aggregate cause=alloc_entry comm="swapper/0" name="boot_aggregate" res=0 result=-12
[ 8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 op=policy_update cause=completed comm="systemd" res=1 result=0
--- security/integrity/integrity_audit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
security/integrity/integrity_audit.c +1 -1
@@ -53,6 +53,6 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,audit_log_untrustedstring (ab, inode->i_sb->s_id);audit_log_format (ab," ino=%lu" , inode->i_ino); } -audit_log_format (ab," res=%d" , !result); +audit_log_format (ab," res=%d result=%d" , !result, result);audit_log_end (ab); }
Signature · 1 line
2.27.0
PATCH v2 2/2 IMA: Add audit log for failure conditions
process_buffer_measurement() and ima_alloc_key_entry() functions need to log an audit message for auditing integrity measurement failures.
Add audit message in these two functions. Remove "pr_devel" log message in process_buffer_measurement().
Sample audit messages:
[ 6.415374] audit: type=1804 audit(1592005945.627:2): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=kernel op=measuring_kexec_cmdline cause=alloc_entry comm="swapper/0" name="kexec-cmdline" res=0 result=-12
[ 8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 op=policy_update cause=completed comm="systemd" res=1 result=0
[ 8.128004] audit: type=1804 audit(1592005947.341:11): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 op=measuring_key cause=hashing_error comm="systemd" name=".builtin_trusted_keys" res=0 result=-22
--- security/integrity/ima/ima.h | 48 ++++++++++++++++--------- security/integrity/ima/ima_main.c | 18 +++++++--- security/integrity/ima/ima_policy.c | 2 +- security/integrity/ima/ima_queue_keys.c | 5 +++ 4 files changed, 51 insertions(+), 22 deletions(-)
security/integrity/ima/ima.h +32 -16
@@ -186,27 +186,43 @@ static inline unsigned int ima_hash_key(u8 *digest)return (digest[0 ] | digest[1 ] <<8 ) %IMA_MEASURE_HTABLE_SIZE ; } -#define __ima_hooks (hook) \ - hook(NONE) \ - hook(FILE_CHECK) \ - hook(MMAP_CHECK) \ - hook(BPRM_CHECK) \ - hook(CREDS_CHECK) \ - hook(POST_SETATTR) \ - hook(MODULE_CHECK) \ - hook(FIRMWARE_CHECK) \ - hook(KEXEC_KERNEL_CHECK) \ - hook(KEXEC_INITRAMFS_CHECK) \ - hook(POLICY_CHECK) \ - hook(KEXEC_CMDLINE) \ - hook(KEY_CHECK) \ - hook(MAX_CHECK) -#define __ima_hook_enumify (ENUM ) ENUM, +#define __ima_hooks (hook) \ + hook(NONE, none) \ + hook(FILE_CHECK, file) \ + hook(MMAP_CHECK, mmap) \ + hook(BPRM_CHECK, bprm) \ + hook(CREDS_CHECK, creds) \ + hook(POST_SETATTR, post_setattr) \ + hook(MODULE_CHECK, module) \ + hook(FIRMWARE_CHECK, firmware) \ + hook(KEXEC_KERNEL_CHECK, kexec_kernel) \ + hook(KEXEC_INITRAMFS_CHECK, kexec_initramfs) \ + hook(POLICY_CHECK, policy) \ + hook(KEXEC_CMDLINE, kexec_cmdline) \ + hook(KEY_CHECK, key) \ + hook(MAX_CHECK, none) + +#define __ima_hook_enumify (ENUM , str) ENUM, +#define __ima_stringify (arg) (#arg) +#define __ima_hook_measuring_stringify (ENUM , str) \ + (__ima_stringify(measuring_ ##str)),enum ima_hooks { __ima_hooks(__ima_hook_enumify) }; +static const char *const ima_hooks_measure_str[] = { +__ima_hooks (__ima_hook_measuring_stringify) +}; + +static inline const char *func_measure_str (enum ima_hooks func) +{ +if (func >=MAX_CHECK ) +return ima_hooks_measure_str[NONE ]; + +return ima_hooks_measure_str[func]; +} +extern const char *const func_tokens[];struct modsig ;
security/integrity/ima/ima_main.c +13 -5
@@ -740,6 +740,7 @@ void process_buffer_measurement(const void *buf, int size, int pcr, const char *keyring) { int ret = 0; +const char *audit_cause ="ENOMEM" ;struct ima_template_entry *entry =NULL ;struct integrity_iint_cache iint = {}; struct ima_event_data event_data = {.iint = &iint,
@@ -794,21 +795,28 @@ void process_buffer_measurement(const void *buf, int size, iint.ima_hash->length = hash_digest_size[ima_hash_algo]; ret =ima_calc_buffer_hash (buf, size, iint.ima_hash); -if (ret <0 ) +if (ret <0 ) { + audit_cause ="hashing_error" ; goto out; + } ret =ima_alloc_init_template (&event_data, &entry, template); -if (ret <0 ) +if (ret <0 ) { + audit_cause ="alloc_entry" ; goto out; + } ret =ima_store_template (entry, violation,NULL , buf, pcr); - -if (ret <0 ) +if (ret <0 ) { + audit_cause ="store_entry" ;ima_free_template_entry (entry); + } out:if (ret <0 ) -pr_devel ("%s: failed, result: %d\n" , __func__, ret); +integrity_audit_msg (AUDIT_INTEGRITY_PCR ,NULL , eventname, +func_measure_str (func), + audit_cause, ret,0 );return ; }
security/integrity/ima/ima_policy.c +1 -1
@@ -1414,7 +1414,7 @@ void ima_delete_rules(void) } } -#define __ima_hook_stringify (str) (#str), +#define __ima_hook_stringify (func, str) (#func),const char *const func_tokens[] = {__ima_hooks (__ima_hook_stringify)
security/integrity/ima/ima_queue_keys.c +5 -0
@@ -68,6 +68,7 @@ static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring, size_t payload_len) {int rc =0 ; +const char *audit_cause ="ENOMEM" ;struct ima_key_entry *entry; entry =kzalloc (sizeof (*entry),GFP_KERNEL );
@@ -88,6 +89,10 @@ static struct ima_key_entry *ima_alloc_key_entry(struct key *keyring, out:if (rc) { +integrity_audit_msg (AUDIT_INTEGRITY_PCR ,NULL , + keyring->description, +func_measure_str (KEY_CHECK ), + audit_cause, rc,0 );ima_free_key_entry (entry); entry =NULL ; }
Signature · 1 line
2.27.0
Quoted 9 lines · Lakshmi Ramasubramanian
Result code is not included in the audit messages logged by the integrity subsystem. Add "result" field in the audit messages logged by the integrity subsystem and set the value to the result code passed to
integrity_audit_msg()in the "result" parameter.Sample audit message:
[ 6.284329] audit: type=1804 audit(1591756723.627:2): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=kernel op=add_boot_aggregate cause=alloc_entry comm="swapper/0" name="boot_aggregate" res=0 result=-12
Quoted 7 lines
[ 8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 op=policy_update cause=completed comm="systemd" res=1 result=0Signed-off-by Lakshmi Ramasubramanian <[email protected]>Suggested-by Steve Grubb <[email protected]>--- security/integrity/integrity_audit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
If we can't use "res=" to carry more than 0/1 then this seems reasonable.
Quoted 14 lines
security/integrity/integrity_audit.c +1
-2@@ -53,6 +53,6 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,audit_log_untrustedstring (ab, inode->i_sb->s_id);audit_log_format (ab," ino=%lu" , inode->i_ino); } -audit_log_format (ab," res=%d" , !result); +audit_log_format (ab," res=%d result=%d" , !result, result);audit_log_end (ab); } --2.27.0
Signature · 2 lines
paul moore www.paul-moore.com
Quoted 19 lines · EDT Paul Moore
On Fri, Jun 12, 2020 at 10:26 PM Lakshmi Ramasubramanian
Quoted 16 lines · <[email protected]> wrote:
Result code is not included in the audit messages logged by the integrity subsystem. Add "result" field in the audit messages logged by the integrity subsystem and set the value to the result code passed to
integrity_audit_msg()in the "result" parameter.Sample audit message:
[ 6.284329] audit: type=1804 audit(1591756723.627:2): pid=1 uid=0auid=4294967295 ses=4294967295 subj=kernel op=add_boot_aggregate cause=alloc_entry comm="swapper/0" name="boot_aggregate" res=0 result=-12
[ 8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0 op=policy_update cause=completed comm="systemd" res=1 result=0
Quoted 8 lines
Quoted 6 lines
Signed-off-by Lakshmi Ramasubramanian <[email protected]>Suggested-by Steve Grubb <[email protected]>---
security/integrity/integrity_audit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)If we can't use "res=" to carry more than 0/1 then this seems reasonable.
Paul,
But we can't do this. The field name dictionary says this is used to convey success/fail. It is hard coded in the field interpretation table to look for 0/1 and interpret that. Interpeting this field will now produce an error message. And "result" is a searchable field.
As I suggested a few emails back, let's just use errno or something not already taken in the dictionary. NACK.
-Steve
Quoted 17 lines
Acked-by Paul Moore <[email protected]>Quoted 15 lines
(quoted diff) +0
-0
b/security/integrity/integrity_audit.cindex5109173839cc..84002d3d5a95100644security/integrity/integrity_audit.c +1
-1@@ -53,6 +53,6 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,>audit_log_untrustedstring (ab, inode->i_sb->s_id);audit_log_format (ab," ino=%lu" , inode->i_ino); } -audit_log_format (ab," res=%d" , !result); +audit_log_format (ab," res=%d result=%d" , !result, result);
Quoted 6 lines
Quoted 6 lines
audit_log_end(ab);}
--
2.27.0
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit