ima: minimize open-writers and ToMToU violations

Each time a file in policy, that is already opened for write, is opened for read, an open-writers integrity violation audit message is emitted and a violation record is added to the IMA measurement list.

Similarly each time a file in policy, that is already opened for read, is opened for write, a Time-of-Measure-Time-of-Use (ToMToU) integrity violation audit message is emitted and a violation record is added to the IMA measurement list.

As there is no benefit in having multiple open-writers or ToMToU violations for the same file open in the audit log and IMA measurement list, minimize them.

Minimizing open-writer violations results in a single open-writers violation being emitted until all writers are closed no matter the number of subsequent file open readers (or writers).

Minimizing ToMToU violations results in a single ToMToU violation being emitted for all subsequent file open writers, until another in policy file open reader.

Since the IMA_MUST_MEASURE atomic flag is only used for tracking ToMToU violations, rename the atomic flag to IMA_MAY_EMIT_TOMTOU.

Define a new atomic flag named IMA_EMITTED_OPENWRITERS to minimize open-writer violations.

Mimi Zohar (2):
ima: limit the number of open-writers integrity violations
ima: limit the number of ToMToU integrity violations

 security/integrity/ima/ima.h      |  3 ++-
 security/integrity/ima/ima_main.c | 18 +++++++++++++-----
 2 files changed, 15 insertions(+), 6 deletions(-)

--
2.48.1

Each time a file in policy, that is already opened for write, is opened for read, an open-writers integrity violation audit message is emitted and a violation record is added to the IMA measurement list. This occurs even if an open-writers violation has already been recorded.

Limit the number of open-writers integrity violations for an existing file open for write to one. After the existing file open for write closes (__fput), subsequent open-writers integrity violations may be emitted.

---
Changelog v2:
- Based on Roberto's review, renamed the atomic flag name, updated the patch description.
- Added Petr's and Stefan's tags.

Changelog v1:
- Basesd on Stefan's RFC comments, updated the patch description and code:
removed unnecessary checks, simplified code.

 security/integrity/ima/ima.h      |  1 +
 security/integrity/ima/ima_main.c | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

index a4f284bd846c..3423b3088de5 100644
@@ -182,6 +182,7 @@ struct ima_kexec_hdr {
 #define IMA_CHANGE_ATTR		2
 #define IMA_DIGSIG		3
 #define IMA_MUST_MEASURE	4
+#define IMA_EMITTED_OPENWRITERS	5
 
 /* IMA integrity metadata associated with an inode */
 struct ima_iint_cache {

index 28b8b0db6f9b..95118c1887cb 100644
@@ -137,8 +137,13 @@ static void ima_rdwr_violation_check(struct file *file,
 	} else {
 		if (must_measure)
 			set_bit(IMA_MUST_MEASURE, &iint->atomic_flags);
-		if (inode_is_open_for_write(inode) && must_measure)
-			send_writers = true;
+
+		/* Limit number of open_writers violations */
+		if (inode_is_open_for_write(inode) && must_measure) {
+			if (!test_and_set_bit(IMA_EMITTED_OPENWRITERS,
+					      &iint->atomic_flags))
+				send_writers = true;
+		}
 	}
 
 	if (!send_tomtou && !send_writers)

@@ -167,6 +172,8 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
 	if (atomic_read(&inode->i_writecount) == 1) {
 		struct kstat stat;
 
+		clear_bit(IMA_EMITTED_OPENWRITERS, &iint->atomic_flags);
+
 		update = test_and_clear_bit(IMA_UPDATE_XATTR,
 					    &iint->atomic_flags);
 		if ((iint->flags & IMA_NEW_FILE) ||

--
2.48.1

Each time a file in policy, that is already opened for read, is opened for write, a Time-of-Measure-Time-of-Use (ToMToU) integrity violation audit message is emitted and a violation record is added to the IMA measurement list. This occurs even if a ToMToU violation has already been recorded.

Limit the number of ToMToU integrity violations per file open for read.

Note: The IMA_MAY_EMIT_TOMTOU atomic flag must be set from the reader side based on policy. This may result in a per file open for read ToMToU violation.

Since IMA_MUST_MEASURE is only used for violations, rename the atomic IMA_MUST_MEASURE flag to IMA_MAY_EMIT_TOMTOU.

---
Changelog v2:
- Based on Roberto's review, renamed the atomic flag name, updated the patch description.
- Added Petr's and Stefan's tags.

 security/integrity/ima/ima.h      | 2 +-
 security/integrity/ima/ima_main.c | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

index 3423b3088de5..e0489c6f7f59 100644
@@ -181,7 +181,7 @@ struct ima_kexec_hdr {
 #define IMA_UPDATE_XATTR	1
 #define IMA_CHANGE_ATTR		2
 #define IMA_DIGSIG		3
-#define IMA_MUST_MEASURE	4
+#define IMA_MAY_EMIT_TOMTOU	4
 #define IMA_EMITTED_OPENWRITERS	5
 
 /* IMA integrity metadata associated with an inode */

index 95118c1887cb..f3e7ac513db3 100644
@@ -129,14 +129,15 @@ static void ima_rdwr_violation_check(struct file *file,
 		if (atomic_read(&inode->i_readcount) && IS_IMA(inode)) {
 			if (!iint)
 				iint = ima_iint_find(inode);
+
 			/* IMA_MEASURE is set from reader side */
-			if (iint && test_bit(IMA_MUST_MEASURE,
-						&iint->atomic_flags))
+			if (iint && test_and_clear_bit(IMA_MAY_EMIT_TOMTOU,
+						       &iint->atomic_flags))
 				send_tomtou = true;
 		}
 	} else {
 		if (must_measure)
-			set_bit(IMA_MUST_MEASURE, &iint->atomic_flags);
+			set_bit(IMA_MAY_EMIT_TOMTOU, &iint->atomic_flags);
 
 		/* Limit number of open_writers violations */
 		if (inode_is_open_for_write(inode) && must_measure) {

--
2.48.1

Thanks Mimi. For the whole series:

Roberto