Skip to content

Commit f5e51fa

Browse files
bauermannMimi Zohar
authored andcommitted
ima: Improvements in ima_appraise_measurement()
Replace nested ifs in the EVM xattr verification logic with a switch statement, making the code easier to understand. Also, add comments to the if statements in the out section and constify the cause variable. Signed-off-by: Mimi Zohar <[email protected]> Signed-off-by: Thiago Jung Bauermann <[email protected]> Acked-by: Serge Hallyn <[email protected]>
1 parent 1775cb8 commit f5e51fa

File tree

1 file changed

+22
-13
lines changed

1 file changed

+22
-13
lines changed

security/integrity/ima/ima_appraise.c

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ int ima_appraise_measurement(enum ima_hooks func,
215215
int xattr_len, int opened)
216216
{
217217
static const char op[] = "appraise_data";
218-
char *cause = "unknown";
218+
const char *cause = "unknown";
219219
struct dentry *dentry = file_dentry(file);
220220
struct inode *inode = d_backing_inode(dentry);
221221
enum integrity_status status = INTEGRITY_UNKNOWN;
@@ -241,16 +241,22 @@ int ima_appraise_measurement(enum ima_hooks func,
241241
}
242242

243243
status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
244-
if ((status != INTEGRITY_PASS) &&
245-
(status != INTEGRITY_PASS_IMMUTABLE) &&
246-
(status != INTEGRITY_UNKNOWN)) {
247-
if ((status == INTEGRITY_NOLABEL)
248-
|| (status == INTEGRITY_NOXATTRS))
249-
cause = "missing-HMAC";
250-
else if (status == INTEGRITY_FAIL)
251-
cause = "invalid-HMAC";
244+
switch (status) {
245+
case INTEGRITY_PASS:
246+
case INTEGRITY_PASS_IMMUTABLE:
247+
case INTEGRITY_UNKNOWN:
248+
break;
249+
case INTEGRITY_NOXATTRS: /* No EVM protected xattrs. */
250+
case INTEGRITY_NOLABEL: /* No security.evm xattr. */
251+
cause = "missing-HMAC";
252+
goto out;
253+
case INTEGRITY_FAIL: /* Invalid HMAC/signature. */
254+
cause = "invalid-HMAC";
252255
goto out;
256+
default:
257+
WARN_ONCE(true, "Unexpected integrity status %d\n", status);
253258
}
259+
254260
switch (xattr_value->type) {
255261
case IMA_XATTR_DIGEST_NG:
256262
/* first byte contains algorithm id */
@@ -316,17 +322,20 @@ int ima_appraise_measurement(enum ima_hooks func,
316322
integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,
317323
op, cause, rc, 0);
318324
} else if (status != INTEGRITY_PASS) {
325+
/* Fix mode, but don't replace file signatures. */
319326
if ((ima_appraise & IMA_APPRAISE_FIX) &&
320327
(!xattr_value ||
321328
xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
322329
if (!ima_fix_xattr(dentry, iint))
323330
status = INTEGRITY_PASS;
324-
} else if ((inode->i_size == 0) &&
325-
(iint->flags & IMA_NEW_FILE) &&
326-
(xattr_value &&
327-
xattr_value->type == EVM_IMA_XATTR_DIGSIG)) {
331+
}
332+
333+
/* Permit new files with file signatures, but without data. */
334+
if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE &&
335+
xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG) {
328336
status = INTEGRITY_PASS;
329337
}
338+
330339
integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,
331340
op, cause, rc, 0);
332341
} else {

0 commit comments

Comments
 (0)