Trim N entries of IMA event logs
Subsystems (5)
v5 cover Trim N entries of IMA event logs
The Integrity Measurement Architecture (IMA) maintains a measurement list —a record of system events used for integrity verification. The IMA event logs are the entries within this measurement list, each representing a specific event or measurement that contributes to the system's integrity assessment.
This update introduces the ability to trim, or remove, N entries from the current measurement list. Trimming involves deleting N entries from the list. This action atomically truncates the measurement list, ensuring that no new measurements can be added until the operation is complete. Importantly, only one writer can initiate this trimming process at a time, maintaining consistency and preventing race conditions.
A userspace interface, ima_trim_log, has been provided for this purpose. When this interface is read, it returns the total number T of entries trimmed since system boot up. This value T need to be preserved across kexec soft reboots. By writing two number T:N to this interface, userspace can request the kernel to trim N entries from the IMA event logs.
To maintain a complete record, userspace is responsible for concatenating and storing the logs before initiating trimming. Userspace can then send the collected data to remote verifiers for validation. After receiving confirmation from the remote verifiers, userspace may instruct the kernel to proceed with trimming the IMA event logs accordingly.
The primary benefit of this solution is the ability to free valuable kernel memory by delegating the task of reconstructing the full measurement list from log chunks to userspace. Trust is not required in userspace for the integrity of the measurement list, as its integrity is cryptographically protected by the Trusted Platform Module (TPM).
Multiple readers are allowed to access the ima_trim_log interface concurrently, while only one writer can trigger log trimming at any time. During trimming, readers do not see the list and cannot access it while deletion is in progress, ensuring atomicity.
Introduce the new kernel option ima_flush_htable to decide whether or not the digests of measurement entries are flushed from the hash table (from reference [2]).
The ima_measure_users counter (protected by the ima_measure_lock mutex) has been introduced to protect access to the measurement list part. The open method of all the measurement interfaces has been extended to allow only one writer at a time or, in alternative, multiple readers. The write permission is used to stage/delete the measurements, the read permission to read them. Write requires also the CAP_SYS_ADMIN capability (from reference [2]). This ima_measure_users needs to be preserved across kexec soft reboots
The total trimmed number T and the ima_measure_users both need to be preserved across kexec soft reboot and new patch will be added for this purpose in next version.
New IMA log trim event is added when trimming finish.
The time required for trimming is minimal, and IMA event logs are briefly on hold during this process, preventing read or add operations. This short interruption has no impact on the overall functionality of IMA.
A new critical data record "ima_log_trim" is added in this solution. This record logs the trim event with number of entries deleted total T since system start and time when this happened. User space can get the total number T of entries trimmed by checking "ima_log_trim" event in the measurement list.
The following are how user space to use the measurement list and ima_log_trim interface
1. get the total numer trimmed T through "ima_log_trim" interface
2. get the PCR quote
3. read the measurement list file, close the file, send for verification
4. wait for response from verifier, until get the good response from verifier with number N that matched the PCR quote got in step 2
5. get the number N from the above message
6. write the T:N to the ima_log_trim interface when no conflict
when kernel get log trim request T:N
Get the T, compare with the total trimmed number
if equal, then do trim N and change T to T+N
else return error
Using above way to trim the log, the time for user space to hold the list will be trimming T:N operation itself at the step 6. User space agent race condition is solved too in this way.
References:
-----------
[1] [PATCH 0/1] Trim N entries of IMA event logs
https://lore.kernel.org/linux-integrity/[email protected]/T/#t
[2] [RFC][PATCH] ima: Add support for staging measurements for deletion
https://lore.kernel.org/linux-integrity/[email protected]/T/#t
[3] [PATCH v2 0/1] Trim N entries of IMA event logs
https://lore.kernel.org/linux-integrity/[email protected]/T/#t
[4] [PATCH v3 0/3] Trim N entries of IMA event logs
https://lore.kernel.org/linux-integrity/[email protected]/T/#t
[5] [PATCH v4 0/3] Trim N entries of IMA event logs
https://lore.kernel.org/linux-integrity/[email protected]/T/#t
Change Log v5:
- lock time performance improvement
- Keep hash table unchanged because log already use the hash value
- Updated patch descriptions as necessary.
Change Log v4:
- Incorporated feedback from Roberto on v3 series.
- Update "ima_log_trim" interface definition
When read this interface, return total number of records trimmed T need to write T:N to this interface to trim N records
- Update user space use case on how to trim IMA event logs
- Updated patch descriptions as necessary.
Change Log v3:
- Incorporated feedback from Mimi on v2 series.
- split patch into multiple patches
- lock time performance improvement
- Updated patch descriptions as necessary.
Change Log v2:
- Incorporated feedback from the Roberto on v1 series.
- Adapted code from Roberto's RFC [Reference 2]
- Add IMA log trim event log to record trim event
- Updated patch descriptions as necessary
steven chen (3):
ima: make ima event log trimming configurable
ima: trim N IMA event log records
ima: add new critical data record to measure log trim
.../admin-guide/kernel-parameters.txt | 4 + security/integrity/ima/Kconfig | 12 + security/integrity/ima/ima.h | 4 +- security/integrity/ima/ima_fs.c | 218 +++++++++++++++++- security/integrity/ima/ima_kexec.c | 2 +- security/integrity/ima/ima_queue.c | 96 ++++++++ 6 files changed, 328 insertions(+), 8 deletions(-)
Signature · 1 line
2.43.0
PATCH v5 1/3 ima: make ima event log trimming configurable
Make ima event log trimming function configurable.
--- security/integrity/ima/Kconfig | 12 ++++++++++++ 1 file changed, 12 insertions(+)
security/integrity/ima/Kconfig +12 -0
@@ -332,4 +332,16 @@ config IMA_KEXEC_EXTRA_MEMORY_KB If set to the default value of 0, an extra half page of memory for those additional measurements will be allocated. +config IMA_LOG_TRIMMING + bool "IMA Event Log Trimming" + default n + help + Say Y here if you want support for IMA Event Log Trimming. + This creates the file /sys/kernel/security/integrity/ima/ima_trim_log. + Userspace + - writes to this file to trigger IMA event log trimming + - reads this file to get number of entried trimming last time + + If unsure, say N. + endif
Signature · 1 line
2.43.0
PATCH v5 2/3 ima: trim N IMA event log records
In reply to steven chenTrim N entries of the IMA event logs. Do not clean the hash table. The values saved in hash table were already used.
Provide a userspace interface ima_trim_log:
When read this interface, it returns total number T of entries trimmed since system boot up.
When write to this interface need to provide two numbers T:N to let kernel to trim N entries of IMA event logs.
Kernel measurement list lock time performance improvement by not clean the hash table.
when kernel get log trim request T:N
- Get the T, compare with the total trimmed number
- if equal, then do trim N and change T to T+N
- else return error
--- .../admin-guide/kernel-parameters.txt | 4 + security/integrity/ima/ima.h | 4 +- security/integrity/ima/ima_fs.c | 198 +++++++++++++++++- security/integrity/ima/ima_kexec.c | 2 +- security/integrity/ima/ima_queue.c | 96 +++++++++ 5 files changed, 296 insertions(+), 8 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +4 -0
@@ -2197,6 +2197,10 @@ Use the canonical format for the binary runtime measurements, instead of host native format. + ima_flush_htable [IMA] + Flush the measurement list hash table when trim all + or a part of it for deletion. + ima_hash= [IMA] Format: { md5 | sha1 | rmd160 | sha256 | sha384 | sha512 | ... }
security/integrity/ima/ima.h +3 -1
@@ -243,11 +243,13 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key, const void *payload, size_t plen, unsigned long flags, bool create);#endif - +extern atomic_long_t ima_number_entries;#ifdef CONFIG_IMA_KEXEC void ima_measure_kexec_event (const char *event_name); +long ima_delete_event_log (long req_val);#else static inline void ima_measure_kexec_event (const char *event_name) {} +static inline long ima_delete_event_log (long req_val) {return 0 ; }#endif /*
security/integrity/ima/ima_fs.c +192 -6
248 lines not on this page · raw message
security/integrity/ima/ima_kexec.c +1 -1
@@ -41,7 +41,7 @@ void ima_measure_kexec_event(const char *event_name)int n; buf_size =ima_get_binary_runtime_size (); - len =atomic_long_read (&ima_htable.len); + len =atomic_long_read (&ima_number_entries); n =scnprintf (ima_kexec_event,IMA_KEXEC_EVENT_LEN , "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
security/integrity/ima/ima_queue.c +96 -0
114 lines not on this page · raw message
Signature · 1 line
2.43.0
Re: [PATCH v5 2/3] ima: trim N IMA event log records
Quoted 1 line · steven chen
Trim N entries of the IMA event logs. Do not clean the hash table.
The very first change of this patch is the kernel option ima_flush_htable option that I introduced for my use case.
At the bottom of this patch you actually check the ima_flush_htable boolean, and delete the measurements entries without disconnecting them from the hash table, so the digest lookup is done on freed memory.
Next, you duplicated my changes regarding the measurements list counter. But instead of removing the old counter from the hash table, you keep incrementing both, but use the new one.
In ima_log_trim_open(), you use again my duplicated code to manage exclusive write/concurrent read scheme for the measurement interfaces. However, for read, if the process does not have CAP_SYS_ADMIN it falls back calling _ima_measurements_open(). Not sure it was intended.
And, in ima_log_trim_release(), you check again CAP_SYS_ADMIN which is redundant, you would not reach this code if the same requirements were not met at open time. You also return an error on close().
In ima_log_trim_write(), you do manual string to number conversion for your first number and use kstrtoul() for the second.
The measurements lists and the associated counter are atomically updated in ima_add_digest_entry(), but not atomically accessed in ima_delete_event_log(). Also, the measurements list is traversed without _rcu variant or lock.
While this trimming scheme aims at minimizing the kernel space and user space delay, it also introduces the following problem. If two agents perform a TPM quote that include a different number of entries, there is no guarantee that the one willing to trim less entries wins. Which means that, one agent could end up not seeing the most recent entries, as they were already trimmed by the other agent.
My solution is not affected by this problem, since there will be only one process collecting all the measurements in user space and exposing them to the agents.
Also, I didn't understand why T and ima_measure_users have to be preserved on soft reboots. Especially ima_measure_users reflects the state of open files for a particular kernel, but on soft reboot a new kernel is booted.
I personally will not endorse a solution based on the ima_trim_log interface. I could accept trimming N even more efficiently than we currently do with a lockless walk to determine the cutting position in ima_queue_stage(), so that we don't need to splice back entries to the measurement list. This would be a replacement of patch 11 in my patch set, but this would be as far as I would like to go.
Roberto
Quoted 451 lines
The values saved in hash table were already used.
Provide a userspace interface
ima_trim_log:
When read this interface, it returns total number T of entries trimmed since system boot up.
When write to this interface need to provide two numbers T:N to let kernel to trim N entries of IMA event logs.Kernel measurement list lock time performance improvement by not clean the hash table.
when kernel get log trim request T:N
- Get the T, compare with the total trimmed number
- if equal, then do trim N and change T to T+N
- else return errorSigned-off-by steven chen <[email protected]>--- .../admin-guide/kernel-parameters.txt | 4 + security/integrity/ima/ima.h | 4 +- security/integrity/ima/ima_fs.c | 198 +++++++++++++++++- security/integrity/ima/ima_kexec.c | 2 +- security/integrity/ima/ima_queue.c | 96 +++++++++ 5 files changed, 296 insertions(+), 8 deletions(-)Documentation/admin-guide/kernel-parameters.txt +4
-0@@ -2197,6 +2197,10 @@ Use the canonical format for the binary runtime measurements, instead of host native format. + ima_flush_htable [IMA] + Flush the measurement list hash table when trim all + or a part of it for deletion. + ima_hash= [IMA] Format: { md5 | sha1 | rmd160 | sha256 | sha384 | sha512 | ... }security/integrity/ima/ima.h +3
-1@@ -243,11 +243,13 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key, const void *payload, size_t plen, unsigned long flags, bool create);#endif - +extern atomic_long_t ima_number_entries;#ifdef CONFIG_IMA_KEXEC void ima_measure_kexec_event (const char *event_name); +long ima_delete_event_log (long req_val);#else static inline void ima_measure_kexec_event (const char *event_name) {} +static inline long ima_delete_event_log (long req_val) {return 0 ; }#endif /* security/integrity/ima/ima_fs.c +192
-6@@ -21,6 +21,9 @@#include <linux/rcupdate.h> #include <linux/parser.h> #include <linux/vmalloc.h> +#include <linux/ktime.h> +#include <linux/timekeeping.h> +#include <linux/ima.h> #include "ima.h" @@ -38,6 +41,17 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);static int valid_policy =1 ; +#define IMA_LOG_TRIM_REQ_NUM_LENGTH 15 +#define IMA_LOG_TRIM_REQ_TOTAL_LENGTH 32 +atomic_long_t ima_number_entries =ATOMIC_LONG_INIT (0 ); +static long trimcount; +/* mutex protects atomicity of trimming measurement list +* and also protects atomicity the measurement list read +* write operation. +*/ +static DEFINE_MUTEX (ima_measure_lock); +static long ima_measure_users; +static ssize_t ima_show_htable_value (char __user *buf,size_t count,loff_t *ppos,atomic_long_t *val) {@@ -64,8 +78,7 @@ static ssize_t ima_show_measurements_count(struct file *filp,char __user *buf, size_t count, loff_t *ppos) { - return ima_show_htable_value(buf, count, ppos, &ima_htable.len); - + return ima_show_htable_value(buf, count, ppos, &ima_number_entries); } static const struct file_operations ima_measurements_count_ops = {@@ -202,16 +215,77 @@ static const struct seq_operations ima_measurments_seqops = { .show = ima_measurements_show }; +/* +* _ima_measurements_open - open the IMA measurements file +* @inode: inode of the file being opened +* @file: file being opened +* @seq_ops: sequence operations for the file +* +* Returns 0 on success, or negative error code. +* Implements mutual exclusion between readers and writer +* of the measurements file. Multiple readers are allowed, +* but writer get exclusive access only no other readers/writers. +* Readers is not allowed when there is a writer. +*/ +static int _ima_measurements_open (struct inode *inode,struct file *file, +const struct seq_operations *seq_ops) +{ +bool write = !!(file->f_mode &FMODE_WRITE ); +int ret; + +if (write && !capable (CAP_SYS_ADMIN )) +return -EPERM ; + +mutex_lock (&ima_measure_lock); +if ((write && ima_measure_users !=0 ) || + (!write && ima_measure_users <0 )) { +mutex_unlock (&ima_measure_lock); +return -EBUSY ; + } + + ret =seq_open (file, seq_ops); +if (ret <0 ) { +mutex_unlock (&ima_measure_lock); +return ret; + } + +if (write) + ima_measure_users--; +else + ima_measure_users++; + +mutex_unlock (&ima_measure_lock); +return ret; +} +static int ima_measurements_open (struct inode *inode,struct file *file) { -return seq_open (file, &ima_measurments_seqops); +return _ima_measurements_open (inode, file, &ima_measurments_seqops); +} + +static int ima_measurements_release (struct inode *inode,struct file *file) +{ +bool write = !!(file->f_mode &FMODE_WRITE ); +int ret; + +mutex_lock (&ima_measure_lock); + ret =seq_release (inode, file); +if (!ret) { +if (!write) + ima_measure_users--; +else + ima_measure_users++; + } + +mutex_unlock (&ima_measure_lock); +return ret; }static const struct file_operations ima_measurements_ops = { .open = ima_measurements_open, .read = seq_read, .llseek = seq_lseek, - .release = seq_release, + .release = ima_measurements_release, };void ima_print_digest (struct seq_file *m,u8 *digest,u32 size)@@ -279,14 +353,114 @@ static const struct seq_operations ima_ascii_measurements_seqops = {static int ima_ascii_measurements_open (struct inode *inode,struct file *file) { -return seq_open (file, &ima_ascii_measurements_seqops); +return _ima_measurements_open (inode, file, &ima_ascii_measurements_seqops); }static const struct file_operations ima_ascii_measurements_ops = { .open = ima_ascii_measurements_open, .read = seq_read, .llseek = seq_lseek, - .release = seq_release, + .release = ima_measurements_release, +}; + +static int ima_log_trim_open (struct inode *inode,struct file *file) +{ +bool write = !!(file->f_mode &FMODE_WRITE ); + +if (!write &&capable (CAP_SYS_ADMIN )) +return 0 ; +else if (!capable (CAP_SYS_ADMIN )) +return -EPERM ; + +return _ima_measurements_open (inode, file, &ima_measurments_seqops); +} + +static ssize_t ima_log_trim_read (struct file *file,char __user *buf,size_t size,loff_t *ppos) +{ +char tmpbuf[IMA_LOG_TRIM_REQ_NUM_LENGTH ]; +ssize_t len; + + len =scnprintf (tmpbuf,sizeof (tmpbuf),"%li\n" , trimcount); +return simple_read_from_buffer (buf, size, ppos, tmpbuf, len); +} + +static ssize_t ima_log_trim_write (struct file *file, +const char __user *buf,size_t datalen,loff_t *ppos) +{ +char tmpbuf[IMA_LOG_TRIM_REQ_TOTAL_LENGTH ]; +char *p = tmpbuf; +long count, ret, val =0 , max =LONG_MAX ; + +if (*ppos >0 || datalen >IMA_LOG_TRIM_REQ_TOTAL_LENGTH || datalen <2 ) { + ret = -EINVAL ; + goto out; + } + +if (copy_from_user (tmpbuf, buf, datalen) !=0 ) { + ret = -EFAULT ; + goto out; + } + + p = tmpbuf; + +while (*p && *p !=':' ) { +if (!isdigit ((unsigned char )*p)) +return -EINVAL ; + +/* digit value */ +int d = *p -'0' ; + +/* overflow check: val * 10 + d > max -> (val > (max - d) / 10) */ +if (val > (max - d) /10 ) +return -ERANGE ; + + val = val *10 + d; + p++; + } + +if (*p !=':' ) +return -EINVAL ; + +/* verify trim count matches */ +if (val != trimcount) +return -EINVAL ; + + p++;/* skip ':' */ + ret =kstrtoul (p,0 , &count); + +if (ret <0 ) + goto out; + + ret =ima_delete_event_log (count); + +if (ret <0 ) + goto out; + + trimcount += ret; + + ret = datalen; +out: +return ret; +} + +static int ima_log_trim_release (struct inode *inode,struct file *file) +{ +bool write = !!(file->f_mode &FMODE_WRITE ); + +if (!write &&capable (CAP_SYS_ADMIN )) +return 0 ; +else if (!capable (CAP_SYS_ADMIN )) +return -EPERM ; + +return ima_measurements_release (inode, file); +} + +static const struct file_operations ima_log_trim_ops = { + .open = ima_log_trim_open, + .read = ima_log_trim_read, + .write = ima_log_trim_write, + .llseek = generic_file_llseek, + .release = ima_log_trim_release };static ssize_t ima_read_policy (char *path)@@ -528,6 +702,18 @@ int __init ima_fs_init(void) goto out; } +if (IS_ENABLED (CONFIG_IMA_LOG_TRIMMING )) { + dentry =securityfs_create_file ("ima_trim_log" , +S_IRUSR |S_IRGRP |S_IWUSR |S_IWGRP , + ima_dir,NULL , &ima_log_trim_ops); +if (IS_ERR (dentry)) { + ret =PTR_ERR (dentry); + goto out; + } + } + + trimcount =0 ; + dentry =securityfs_create_file ("runtime_measurements_count" ,S_IRUSR |S_IRGRP , ima_dir,NULL , &ima_measurements_count_ops);security/integrity/ima/ima_kexec.c +1
-1@@ -41,7 +41,7 @@ void ima_measure_kexec_event(const char *event_name)int n; buf_size =ima_get_binary_runtime_size (); - len =atomic_long_read (&ima_htable.len); + len =atomic_long_read (&ima_number_entries); n =scnprintf (ima_kexec_event,IMA_KEXEC_EVENT_LEN , "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"security/integrity/ima/ima_queue.c +96
-0@@ -22,6 +22,14 @@#define AUDIT_CAUSE_LEN_MAX 32 +bool ima_flush_htable; +static int __init ima_flush_htable_setup (char *str) +{ + ima_flush_htable = true; +return 1 ; +} +__setup ("ima_flush_htable" , ima_flush_htable_setup); +/* pre-allocated array of tpm_digest structures to extend a PCR */ static struct tpm_digest *digests;@@ -114,6 +122,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,list_add_tail_rcu (&qe->later, &ima_measurements);atomic_long_inc (&ima_htable.len); +atomic_long_inc (&ima_number_entries);if (update_htable) { key =ima_hash_key (entry->digests[ima_hash_algo_idx].digest);hlist_add_head_rcu (&qe->hnext, &ima_htable.queue[key]);@@ -220,6 +229,93 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,return result; } +/** +* ima_delete_event_log - delete IMA event entry +* @num_records: number of records to delete +* +* delete num_records entries off the measurement list. +* Returns num_records, or negative error code. +*/ +long ima_delete_event_log (long num_records) +{ +long len, cur = num_records, tmp_len =0 ; +struct ima_queue_entry *qe, *qe_tmp; +LIST_HEAD (ima_measurements_to_delete); +struct list_head *list_ptr; + +if (!IS_ENABLED (CONFIG_IMA_LOG_TRIMMING )) +return -EOPNOTSUPP ; + +if (num_records <=0 ) +return num_records; + + list_ptr = &ima_measurements; + + len =atomic_long_read (&ima_number_entries); + +if (num_records <= len) { +list_for_each_entry (qe, list_ptr, later) { +if (cur >0 ) { + tmp_len +=get_binary_runtime_size (qe->entry); + --cur; + } +if (cur ==0 ) { + qe_tmp = qe; +break ; + } + } + } +else { +return -ENOENT ; + } + + +mutex_lock (&ima_extend_list_mutex); + len =atomic_long_read (&ima_number_entries); + +if (num_records == len) { +list_replace (&ima_measurements, &ima_measurements_to_delete); +INIT_LIST_HEAD (&ima_measurements); +atomic_long_set (&ima_number_entries,0 ); + list_ptr = &ima_measurements_to_delete; + } +else { +__list_cut_position (&ima_measurements_to_delete, &ima_measurements, + &qe_tmp->later); +atomic_long_sub (num_records, &ima_number_entries); +if (IS_ENABLED (CONFIG_IMA_KEXEC )) + binary_runtime_size -= tmp_len; + } + +mutex_unlock (&ima_extend_list_mutex); + +if (ima_flush_htable) +synchronize_rcu (); + +list_for_each_entry_safe (qe, qe_tmp, &ima_measurements_to_delete, later) { +/* +* Ok because after list delete qe is only accessed by +* ima_lookup_digest_entry(). +*/ +for (int i =0 ; i < qe->entry->template_desc->num_fields; i++) { +kfree (qe->entry->template_data[i].data); + qe->entry->template_data[i].data =NULL ; + qe->entry->template_data[i].len =0 ; + } + +list_del (&qe->later); + +/* No leak if !ima_flush_htable, referenced by ima_htable. */ +if (ima_flush_htable) { +kfree (qe->entry->digests); +kfree (qe->entry); +kfree (qe); + } + } + +return num_records; +} +int ima_restore_measurement_entry (struct ima_template_entry *entry) {int result =0 ;
Re: [PATCH v5 2/3] ima: trim N IMA event log records
Quoted 9 lines · Roberto Sassu
Quoted 1 line · steven chen
Trim N entries of the IMA event logs. Do not clean the hash table.
The very first change of this patch is the kernel option
ima_flush_htableoption that I introduced for my use case.At the bottom of this patch you actually check the
ima_flush_htableboolean, and delete the measurements entries without disconnecting them from the hash table, so the digest lookup is done on freed memory.
Quoted 8 lines
Next, you duplicated my changes regarding the measurements list counter. But instead of removing the old counter from the hash table, you keep incrementing both, but use the new one.
In
ima_log_trim_open(), you use again my duplicated code to manage exclusive write/concurrent read scheme for the measurement interfaces. However, for read, if the process does not haveCAP_SYS_ADMINit falls back calling_ima_measurements_open(). Not sure it was intended.
Hi Roberto,
I acknowledged these are coming from you in my cover letter. Please let me know the best way to show your contribution and I will update in my next version.
All above issues you mentioned, I will update in next version.
Quoted 3 lines
And, in
ima_log_trim_release(), you check againCAP_SYS_ADMINwhich is redundant, you would not reach this code if the same requirements were not met at open time. You also return an error onclose().
Will update in next version.
Thanks,
Quoted 7 lines
In
ima_log_trim_write(), you do manual string to number conversion for your first number and usekstrtoul()for the second.The measurements lists and the associated counter are atomically updated in
ima_add_digest_entry(), but not atomically accessed inima_delete_event_log(). Also, the measurements list is traversed without_rcuvariant or lock.
Will update in next version.
Thanks
Quoted 7 lines
While this trimming scheme aims at minimizing the kernel space and user space delay, it also introduces the following problem. If two agents perform a TPM quote that include a different number of entries, there is no guarantee that the one willing to trim less entries wins. Which means that, one agent could end up not seeing the most recent entries, as they were already trimmed by the other agent.
This should be acceptable: the second trim request will be rejected and the agent can find all logs in user space if all user agents handle the log in the right way.
Also there is other way to do it: the user agent can hold the list by open the ima_trim_log with write permission during reading, attestation, trim period. In this way, the user agent for "Trim N method" will have similar user space hold time as "staged method" but has less kernel list lock time, and user agent requirement for "Trim N method" is much simple than that for "stage method".
Quoted 4 lines
My solution is not affected by this problem, since there will be only one process collecting all the measurements in user space and exposing them to the agents.
Please see above response.
Thanks,
Steven
Quoted 466 lines
Also, I didn't understand why T and
ima_measure_usershave to be preserved on soft reboots. Especiallyima_measure_usersreflects the state of open files for a particular kernel, but on soft reboot a new kernel is booted.I personally will not endorse a solution based on the
ima_trim_loginterface. I could accept trimming N even more efficiently than we currently do with a lockless walk to determine the cutting position inima_queue_stage(), so that we don't need to splice back entries to the measurement list. This would be a replacement of patch 11 in my patch set, but this would be as far as I would like to go.Roberto
Quoted 451 lines
The values saved in hash table were already used.
Provide a userspace interface
ima_trim_log:
When read this interface, it returns total number T of entries trimmed since system boot up.
When write to this interface need to provide two numbers T:N to let kernel to trim N entries of IMA event logs.Kernel measurement list lock time performance improvement by not clean the hash table.
when kernel get log trim request T:N
- Get the T, compare with the total trimmed number
- if equal, then do trim N and change T to T+N
- else return errorSigned-off-by steven chen <[email protected]>--- .../admin-guide/kernel-parameters.txt | 4 + security/integrity/ima/ima.h | 4 +- security/integrity/ima/ima_fs.c | 198 +++++++++++++++++- security/integrity/ima/ima_kexec.c | 2 +- security/integrity/ima/ima_queue.c | 96 +++++++++ 5 files changed, 296 insertions(+), 8 deletions(-)Documentation/admin-guide/kernel-parameters.txt +4
-0@@ -2197,6 +2197,10 @@ Use the canonical format for the binary runtime measurements, instead of host native format. + ima_flush_htable [IMA] + Flush the measurement list hash table when trim all + or a part of it for deletion. + ima_hash= [IMA] Format: { md5 | sha1 | rmd160 | sha256 | sha384 | sha512 | ... }security/integrity/ima/ima.h +3
-1@@ -243,11 +243,13 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key, const void *payload, size_t plen, unsigned long flags, bool create);#endif - +extern atomic_long_t ima_number_entries;#ifdef CONFIG_IMA_KEXEC void ima_measure_kexec_event (const char *event_name); +long ima_delete_event_log (long req_val);#else static inline void ima_measure_kexec_event (const char *event_name) {} +static inline long ima_delete_event_log (long req_val) {return 0 ; }#endif /* security/integrity/ima/ima_fs.c +192
-6@@ -21,6 +21,9 @@#include <linux/rcupdate.h> #include <linux/parser.h> #include <linux/vmalloc.h> +#include <linux/ktime.h> +#include <linux/timekeeping.h> +#include <linux/ima.h> #include "ima.h" @@ -38,6 +41,17 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);static int valid_policy =1 ; +#define IMA_LOG_TRIM_REQ_NUM_LENGTH 15 +#define IMA_LOG_TRIM_REQ_TOTAL_LENGTH 32 +atomic_long_t ima_number_entries =ATOMIC_LONG_INIT (0 ); +static long trimcount; +/* mutex protects atomicity of trimming measurement list +* and also protects atomicity the measurement list read +* write operation. +*/ +static DEFINE_MUTEX (ima_measure_lock); +static long ima_measure_users; +static ssize_t ima_show_htable_value (char __user *buf,size_t count,loff_t *ppos,atomic_long_t *val) {@@ -64,8 +78,7 @@ static ssize_t ima_show_measurements_count(struct file *filp,char __user *buf, size_t count, loff_t *ppos) { - return ima_show_htable_value(buf, count, ppos, &ima_htable.len); - + return ima_show_htable_value(buf, count, ppos, &ima_number_entries); } static const struct file_operations ima_measurements_count_ops = {@@ -202,16 +215,77 @@ static const struct seq_operations ima_measurments_seqops = { .show = ima_measurements_show }; +/* +* _ima_measurements_open - open the IMA measurements file +* @inode: inode of the file being opened +* @file: file being opened +* @seq_ops: sequence operations for the file +* +* Returns 0 on success, or negative error code. +* Implements mutual exclusion between readers and writer +* of the measurements file. Multiple readers are allowed, +* but writer get exclusive access only no other readers/writers. +* Readers is not allowed when there is a writer. +*/ +static int _ima_measurements_open (struct inode *inode,struct file *file, +const struct seq_operations *seq_ops) +{ +bool write = !!(file->f_mode &FMODE_WRITE ); +int ret; + +if (write && !capable (CAP_SYS_ADMIN )) +return -EPERM ; + +mutex_lock (&ima_measure_lock); +if ((write && ima_measure_users !=0 ) || + (!write && ima_measure_users <0 )) { +mutex_unlock (&ima_measure_lock); +return -EBUSY ; + } + + ret =seq_open (file, seq_ops); +if (ret <0 ) { +mutex_unlock (&ima_measure_lock); +return ret; + } + +if (write) + ima_measure_users--; +else + ima_measure_users++; + +mutex_unlock (&ima_measure_lock); +return ret; +} +static int ima_measurements_open (struct inode *inode,struct file *file) { -return seq_open (file, &ima_measurments_seqops); +return _ima_measurements_open (inode, file, &ima_measurments_seqops); +} + +static int ima_measurements_release (struct inode *inode,struct file *file) +{ +bool write = !!(file->f_mode &FMODE_WRITE ); +int ret; + +mutex_lock (&ima_measure_lock); + ret =seq_release (inode, file); +if (!ret) { +if (!write) + ima_measure_users--; +else + ima_measure_users++; + } + +mutex_unlock (&ima_measure_lock); +return ret; }static const struct file_operations ima_measurements_ops = { .open = ima_measurements_open, .read = seq_read, .llseek = seq_lseek, - .release = seq_release, + .release = ima_measurements_release, };void ima_print_digest (struct seq_file *m,u8 *digest,u32 size)@@ -279,14 +353,114 @@ static const struct seq_operations ima_ascii_measurements_seqops = {static int ima_ascii_measurements_open (struct inode *inode,struct file *file) { -return seq_open (file, &ima_ascii_measurements_seqops); +return _ima_measurements_open (inode, file, &ima_ascii_measurements_seqops); }static const struct file_operations ima_ascii_measurements_ops = { .open = ima_ascii_measurements_open, .read = seq_read, .llseek = seq_lseek, - .release = seq_release, + .release = ima_measurements_release, +}; + +static int ima_log_trim_open (struct inode *inode,struct file *file) +{ +bool write = !!(file->f_mode &FMODE_WRITE ); + +if (!write &&capable (CAP_SYS_ADMIN )) +return 0 ; +else if (!capable (CAP_SYS_ADMIN )) +return -EPERM ; + +return _ima_measurements_open (inode, file, &ima_measurments_seqops); +} + +static ssize_t ima_log_trim_read (struct file *file,char __user *buf,size_t size,loff_t *ppos) +{ +char tmpbuf[IMA_LOG_TRIM_REQ_NUM_LENGTH ]; +ssize_t len; + + len =scnprintf (tmpbuf,sizeof (tmpbuf),"%li\n" , trimcount); +return simple_read_from_buffer (buf, size, ppos, tmpbuf, len); +} + +static ssize_t ima_log_trim_write (struct file *file, +const char __user *buf,size_t datalen,loff_t *ppos) +{ +char tmpbuf[IMA_LOG_TRIM_REQ_TOTAL_LENGTH ]; +char *p = tmpbuf; +long count, ret, val =0 , max =LONG_MAX ; + +if (*ppos >0 || datalen >IMA_LOG_TRIM_REQ_TOTAL_LENGTH || datalen <2 ) { + ret = -EINVAL ; + goto out; + } + +if (copy_from_user (tmpbuf, buf, datalen) !=0 ) { + ret = -EFAULT ; + goto out; + } + + p = tmpbuf; + +while (*p && *p !=':' ) { +if (!isdigit ((unsigned char )*p)) +return -EINVAL ; + +/* digit value */ +int d = *p -'0' ; + +/* overflow check: val * 10 + d > max -> (val > (max - d) / 10) */ +if (val > (max - d) /10 ) +return -ERANGE ; + + val = val *10 + d; + p++; + } + +if (*p !=':' ) +return -EINVAL ; + +/* verify trim count matches */ +if (val != trimcount) +return -EINVAL ; + + p++;/* skip ':' */ + ret =kstrtoul (p,0 , &count); + +if (ret <0 ) + goto out; + + ret =ima_delete_event_log (count); + +if (ret <0 ) + goto out; + + trimcount += ret; + + ret = datalen; +out: +return ret; +} + +static int ima_log_trim_release (struct inode *inode,struct file *file) +{ +bool write = !!(file->f_mode &FMODE_WRITE ); + +if (!write &&capable (CAP_SYS_ADMIN )) +return 0 ; +else if (!capable (CAP_SYS_ADMIN )) +return -EPERM ; + +return ima_measurements_release (inode, file); +} + +static const struct file_operations ima_log_trim_ops = { + .open = ima_log_trim_open, + .read = ima_log_trim_read, + .write = ima_log_trim_write, + .llseek = generic_file_llseek, + .release = ima_log_trim_release };static ssize_t ima_read_policy (char *path)@@ -528,6 +702,18 @@ int __init ima_fs_init(void) goto out; } +if (IS_ENABLED (CONFIG_IMA_LOG_TRIMMING )) { + dentry =securityfs_create_file ("ima_trim_log" , +S_IRUSR |S_IRGRP |S_IWUSR |S_IWGRP , + ima_dir,NULL , &ima_log_trim_ops); +if (IS_ERR (dentry)) { + ret =PTR_ERR (dentry); + goto out; + } + } + + trimcount =0 ; + dentry =securityfs_create_file ("runtime_measurements_count" ,S_IRUSR |S_IRGRP , ima_dir,NULL , &ima_measurements_count_ops);security/integrity/ima/ima_kexec.c +1
-1@@ -41,7 +41,7 @@ void ima_measure_kexec_event(const char *event_name)int n; buf_size =ima_get_binary_runtime_size (); - len =atomic_long_read (&ima_htable.len); + len =atomic_long_read (&ima_number_entries); n =scnprintf (ima_kexec_event,IMA_KEXEC_EVENT_LEN , "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"security/integrity/ima/ima_queue.c +96
-0@@ -22,6 +22,14 @@#define AUDIT_CAUSE_LEN_MAX 32 +bool ima_flush_htable; +static int __init ima_flush_htable_setup (char *str) +{ + ima_flush_htable = true; +return 1 ; +} +__setup ("ima_flush_htable" , ima_flush_htable_setup); +/* pre-allocated array of tpm_digest structures to extend a PCR */ static struct tpm_digest *digests;@@ -114,6 +122,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,list_add_tail_rcu (&qe->later, &ima_measurements);atomic_long_inc (&ima_htable.len); +atomic_long_inc (&ima_number_entries);if (update_htable) { key =ima_hash_key (entry->digests[ima_hash_algo_idx].digest);hlist_add_head_rcu (&qe->hnext, &ima_htable.queue[key]);@@ -220,6 +229,93 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,return result; } +/** +* ima_delete_event_log - delete IMA event entry +* @num_records: number of records to delete +* +* delete num_records entries off the measurement list. +* Returns num_records, or negative error code. +*/ +long ima_delete_event_log (long num_records) +{ +long len, cur = num_records, tmp_len =0 ; +struct ima_queue_entry *qe, *qe_tmp; +LIST_HEAD (ima_measurements_to_delete); +struct list_head *list_ptr; + +if (!IS_ENABLED (CONFIG_IMA_LOG_TRIMMING )) +return -EOPNOTSUPP ; + +if (num_records <=0 ) +return num_records; + + list_ptr = &ima_measurements; + + len =atomic_long_read (&ima_number_entries); + +if (num_records <= len) { +list_for_each_entry (qe, list_ptr, later) { +if (cur >0 ) { + tmp_len +=get_binary_runtime_size (qe->entry); + --cur; + } +if (cur ==0 ) { + qe_tmp = qe; +break ; + } + } + } +else { +return -ENOENT ; + } + + +mutex_lock (&ima_extend_list_mutex); + len =atomic_long_read (&ima_number_entries); + +if (num_records == len) { +list_replace (&ima_measurements, &ima_measurements_to_delete); +INIT_LIST_HEAD (&ima_measurements); +atomic_long_set (&ima_number_entries,0 ); + list_ptr = &ima_measurements_to_delete; + } +else { +__list_cut_position (&ima_measurements_to_delete, &ima_measurements, + &qe_tmp->later); +atomic_long_sub (num_records, &ima_number_entries); +if (IS_ENABLED (CONFIG_IMA_KEXEC )) + binary_runtime_size -= tmp_len; + } + +mutex_unlock (&ima_extend_list_mutex); + +if (ima_flush_htable) +synchronize_rcu (); + +list_for_each_entry_safe (qe, qe_tmp, &ima_measurements_to_delete, later) { +/* +* Ok because after list delete qe is only accessed by +* ima_lookup_digest_entry(). +*/ +for (int i =0 ; i < qe->entry->template_desc->num_fields; i++) { +kfree (qe->entry->template_data[i].data); + qe->entry->template_data[i].data =NULL ; + qe->entry->template_data[i].len =0 ; + } + +list_del (&qe->later); + +/* No leak if !ima_flush_htable, referenced by ima_htable. */ +if (ima_flush_htable) { +kfree (qe->entry->digests); +kfree (qe->entry); +kfree (qe); + } + } + +return num_records; +} +int ima_restore_measurement_entry (struct ima_template_entry *entry) {int result =0 ;
PATCH v5 3/3 ima: add new critical data record to measure log trim
In reply to steven chenAdd a new critical data record to measure the trimming event when ima event records are deleted since system boot up.
If all IMA event logs are saved in the userspace, use this log to get total numbers of records deleted since system boot up at that point.
--- security/integrity/ima/ima_fs.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
security/integrity/ima/ima_fs.c +20 -0
@@ -43,6 +43,7 @@ static int valid_policy = 1;#define IMA_LOG_TRIM_REQ_NUM_LENGTH 15#define IMA_LOG_TRIM_REQ_TOTAL_LENGTH 32 +#define IMA_LOG_TRIM_EVENT_LEN 256atomic_long_t ima_number_entries =ATOMIC_LONG_INIT (0 );static long trimcount;/* mutex protects atomicity of trimming measurement list
@@ -52,6 +53,22 @@ static long trimcount;static DEFINE_MUTEX (ima_measure_lock);static long ima_measure_users; +static void ima_measure_trim_event (void ) +{ +char ima_log_trim_event[IMA_LOG_TRIM_EVENT_LEN ]; +struct timespec64 ts; +u64 time_ns; +int n; + +ktime_get_real_ts64 (&ts); + time_ns = (u64 )ts.tv_sec *1000000000ULL + ts.tv_nsec; + n =scnprintf (ima_log_trim_event,IMA_LOG_TRIM_EVENT_LEN , +"time= %llu; number= %lu;" , time_ns, trimcount); + +ima_measure_critical_data ("ima_log_trim" ,"trim ima event logs" , + ima_log_trim_event, n, false,NULL ,0 ); +} +static ssize_t ima_show_htable_value (char __user *buf,size_t count,loff_t *ppos,atomic_long_t *val) {
@@ -436,6 +453,9 @@ static ssize_t ima_log_trim_write(struct file *file,if (ret <0 ) goto out; +if (ret >0 ) +ima_measure_trim_event (); + trimcount += ret; ret = datalen;
Signature · 1 line
2.43.0