diff for duplicates of <1497443972.4287.38.camel@linux.vnet.ibm.com> diff --git a/a/1.txt b/N1/1.txt index 3c0bd61..15e692f 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -155,7 +155,8 @@ be a separate patch. The patch description would explain the need for a new function. > if (!keyring[id]) { -> keyring[id] > @@ -61,18 +60,29 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, +> keyring[id] = +> @@ -61,18 +60,29 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, > int err = PTR_ERR(keyring[id]); > pr_err("no %s keyring: %d\n", keyring_name[id], err); > keyring[id] = NULL; @@ -321,7 +322,7 @@ a new function. > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -172,6 +172,11 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, -> } else if (xattr_len = 17) +> } else if (xattr_len == 17) > return HASH_ALGO_MD5; > break; > + case IMA_MODSIG: @@ -338,23 +339,23 @@ a new function. > > - status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint); > - if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) { -> - if ((status = INTEGRITY_NOLABEL) -> - || (status = INTEGRITY_NOXATTRS)) +> - if ((status == INTEGRITY_NOLABEL) +> - || (status == INTEGRITY_NOXATTRS)) > + /* Appended signatures aren't protected by EVM. */ > + status = evm_verifyxattr(dentry, XATTR_NAME_IMA, -> + xattr_value->type = IMA_MODSIG ? +> + xattr_value->type == IMA_MODSIG ? > + NULL : xattr_value, rc, iint); > + if (status != INTEGRITY_PASS && status != INTEGRITY_UNKNOWN && -> + !(xattr_value->type = IMA_MODSIG && -> + (status = INTEGRITY_NOLABEL || status = INTEGRITY_NOXATTRS))) { +> + !(xattr_value->type == IMA_MODSIG && +> + (status == INTEGRITY_NOLABEL || status == INTEGRITY_NOXATTRS))) { This was messy to begin with, and now it is even more messy. For appended signatures, we're only interested in INTEGRITY_FAIL. Maybe leave the existing "if" clause alone and define a new "if" clause. -> + if (status = INTEGRITY_NOLABEL || status = INTEGRITY_NOXATTRS) +> + if (status == INTEGRITY_NOLABEL || status == INTEGRITY_NOXATTRS) > cause = "missing-HMAC"; -> else if (status = INTEGRITY_FAIL) +> else if (status == INTEGRITY_FAIL) > cause = "invalid-HMAC"; > @@ -267,11 +276,18 @@ int ima_appraise_measurement(enum ima_hooks func, > status = INTEGRITY_PASS; @@ -367,7 +368,7 @@ leave the existing "if" clause alone and define a new "if" clause. > - iint->ima_hash->digest, > - iint->ima_hash->length); > + -> + if (xattr_value->type = EVM_IMA_XATTR_DIGSIG) +> + if (xattr_value->type == EVM_IMA_XATTR_DIGSIG) > + rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, > + (const char *)xattr_value, > + rc, iint->ima_hash->digest, @@ -381,7 +382,7 @@ Perhaps allowing IMA_MODSIG to flow into EVM_IMA_XATTR_DIGSIG on failure, would help restore process_measurements() to the way it was. Further explanation below. -> if (rc = -EOPNOTSUPP) { +> if (rc == -EOPNOTSUPP) { > status = INTEGRITY_UNKNOWN; > } else if (rc) { > @@ -291,13 +307,15 @@ int ima_appraise_measurement(enum ima_hooks func, @@ -393,12 +394,12 @@ failure, would help restore process_measurements() to the way it was. > + xattr_value->type != IMA_MODSIG))) { > if (!ima_fix_xattr(dentry, iint)) > status = INTEGRITY_PASS; -> } else if ((inode->i_size = 0) && +> } else if ((inode->i_size == 0) && > (iint->flags & IMA_NEW_FILE) && > (xattr_value && -> - xattr_value->type = EVM_IMA_XATTR_DIGSIG)) { -> + (xattr_value->type = EVM_IMA_XATTR_DIGSIG || -> + xattr_value->type = IMA_MODSIG))) { +> - xattr_value->type == EVM_IMA_XATTR_DIGSIG)) { +> + (xattr_value->type == EVM_IMA_XATTR_DIGSIG || +> + xattr_value->type == IMA_MODSIG))) { > status = INTEGRITY_PASS; > } > integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename, @@ -406,9 +407,9 @@ failure, would help restore process_measurements() to the way it was. > if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST)) > return -EINVAL; > ima_reset_appraise_flags(d_backing_inode(dentry), -> - (xvalue->type = EVM_IMA_XATTR_DIGSIG) ? 1 : 0); -> + xvalue->type = EVM_IMA_XATTR_DIGSIG || -> + xvalue->type = IMA_MODSIG); +> - (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0); +> + xvalue->type == EVM_IMA_XATTR_DIGSIG || +> + xvalue->type == IMA_MODSIG); Probably easier to read if we set a variable, before calling ima_reset_appraise_flags. @@ -737,7 +738,7 @@ Mimi > + if (!hdr) > + return; > + -> + if (hdr->type = IMA_MODSIG) { +> + if (hdr->type == IMA_MODSIG) { > + struct signature_modsig_hdr *modsig = (typeof(modsig)) hdr; > + > + pkcs7_free_message(modsig->pkcs7_msg); @@ -753,11 +754,11 @@ Mimi > } > > ima_log_string(ab, "appraise_type", args[0].from); -> - if ((strcmp(args[0].from, "imasig")) = 0) -> + if (strcmp(args[0].from, "imasig") = 0) +> - if ((strcmp(args[0].from, "imasig")) == 0) +> + if (strcmp(args[0].from, "imasig") == 0) > entry->flags |= IMA_DIGSIG_REQUIRED; > + else if (ima_hook_supports_modsig(entry->func) && -> + strcmp(args[0].from, "modsig|imasig") = 0) +> + strcmp(args[0].from, "modsig|imasig") == 0) > + entry->flags |= IMA_DIGSIG_REQUIRED > + | IMA_MODSIG_ALLOWED; > else @@ -821,7 +822,7 @@ Mimi > + * The xattr_value for IMA_MODSIG is a runtime structure containing > + * pointers. Get its raw data instead. > + */ -> + if (xattr_value->type = IMA_MODSIG) { +> + if (xattr_value->type == IMA_MODSIG) { > + rc = ima_modsig_serialize_data(xattr_value, &xattr_value, > + &xattr_len); > + if (rc) diff --git a/a/content_digest b/N1/content_digest index fdcf183..9b82833 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -2,7 +2,7 @@ "ref\01496886555-10082-7-git-send-email-bauerman@linux.vnet.ibm.com\0" "From\0Mimi Zohar <zohar@linux.vnet.ibm.com>\0" "Subject\0Re: [PATCH v2 6/6] ima: Support module-style appended signatures for appraisal\0" - "Date\0Wed, 14 Jun 2017 12:39:32 +0000\0" + "Date\0Wed, 14 Jun 2017 08:39:32 -0400\0" "To\0Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>" " linux-security-module@vger.kernel.org\0" "Cc\0linux-ima-devel@lists.sourceforge.net" @@ -180,7 +180,8 @@ "a new function.\n" "\n" "> \tif (!keyring[id]) {\n" - "> \t\tkeyring[id] > @@ -61,18 +60,29 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,\n" + "> \t\tkeyring[id] =\n" + "> @@ -61,18 +60,29 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,\n" "> \t\t\tint err = PTR_ERR(keyring[id]);\n" "> \t\t\tpr_err(\"no %s keyring: %d\\n\", keyring_name[id], err);\n" "> \t\t\tkeyring[id] = NULL;\n" @@ -346,7 +347,7 @@ "> --- a/security/integrity/ima/ima_appraise.c\n" "> +++ b/security/integrity/ima/ima_appraise.c\n" "> @@ -172,6 +172,11 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,\n" - "> \t\t} else if (xattr_len = 17)\n" + "> \t\t} else if (xattr_len == 17)\n" "> \t\t\treturn HASH_ALGO_MD5;\n" "> \t\tbreak;\n" "> +\tcase IMA_MODSIG:\n" @@ -363,23 +364,23 @@ "> \n" "> -\tstatus = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);\n" "> -\tif ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {\n" - "> -\t\tif ((status = INTEGRITY_NOLABEL)\n" - "> -\t\t || (status = INTEGRITY_NOXATTRS))\n" + "> -\t\tif ((status == INTEGRITY_NOLABEL)\n" + "> -\t\t || (status == INTEGRITY_NOXATTRS))\n" "> +\t/* Appended signatures aren't protected by EVM. */\n" "> +\tstatus = evm_verifyxattr(dentry, XATTR_NAME_IMA,\n" - "> +\t\t\t\t xattr_value->type = IMA_MODSIG ?\n" + "> +\t\t\t\t xattr_value->type == IMA_MODSIG ?\n" "> +\t\t\t\t NULL : xattr_value, rc, iint);\n" "> +\tif (status != INTEGRITY_PASS && status != INTEGRITY_UNKNOWN &&\n" - "> +\t !(xattr_value->type = IMA_MODSIG &&\n" - "> +\t (status = INTEGRITY_NOLABEL || status = INTEGRITY_NOXATTRS))) {\n" + "> +\t !(xattr_value->type == IMA_MODSIG &&\n" + "> +\t (status == INTEGRITY_NOLABEL || status == INTEGRITY_NOXATTRS))) {\n" "\302\240\n" "This was messy to begin with, and now it is even more messy. \302\240For\n" "appended signatures, we're only interested in INTEGRITY_FAIL. \302\240Maybe\n" "leave the existing \"if\" clause alone and define a new \"if\" clause.\n" "\n" - "> +\t\tif (status = INTEGRITY_NOLABEL || status = INTEGRITY_NOXATTRS)\n" + "> +\t\tif (status == INTEGRITY_NOLABEL || status == INTEGRITY_NOXATTRS)\n" "> \t\t\tcause = \"missing-HMAC\";\n" - "> \t\telse if (status = INTEGRITY_FAIL)\n" + "> \t\telse if (status == INTEGRITY_FAIL)\n" "> \t\t\tcause = \"invalid-HMAC\";\n" "> @@ -267,11 +276,18 @@ int ima_appraise_measurement(enum ima_hooks func,\n" "> \t\tstatus = INTEGRITY_PASS;\n" @@ -392,7 +393,7 @@ "> -\t\t\t\t\t iint->ima_hash->digest,\n" "> -\t\t\t\t\t iint->ima_hash->length);\n" "> +\n" - "> +\t\tif (xattr_value->type = EVM_IMA_XATTR_DIGSIG)\n" + "> +\t\tif (xattr_value->type == EVM_IMA_XATTR_DIGSIG)\n" "> +\t\t\trc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,\n" "> +\t\t\t\t\t\t (const char *)xattr_value,\n" "> +\t\t\t\t\t\t rc, iint->ima_hash->digest,\n" @@ -406,7 +407,7 @@ "failure, would help restore process_measurements() to the way it was.\n" "\302\240Further explanation below.\302\240\n" "\n" - "> \t\tif (rc = -EOPNOTSUPP) {\n" + "> \t\tif (rc == -EOPNOTSUPP) {\n" "> \t\t\tstatus = INTEGRITY_UNKNOWN;\n" "> \t\t} else if (rc) {\n" "> @@ -291,13 +307,15 @@ int ima_appraise_measurement(enum ima_hooks func,\n" @@ -418,12 +419,12 @@ "> +\t\t xattr_value->type != IMA_MODSIG))) {\n" "> \t\t\tif (!ima_fix_xattr(dentry, iint))\n" "> \t\t\t\tstatus = INTEGRITY_PASS;\n" - "> \t\t} else if ((inode->i_size = 0) &&\n" + "> \t\t} else if ((inode->i_size == 0) &&\n" "> \t\t\t (iint->flags & IMA_NEW_FILE) &&\n" "> \t\t\t (xattr_value &&\n" - "> -\t\t\t xattr_value->type = EVM_IMA_XATTR_DIGSIG)) {\n" - "> +\t\t\t (xattr_value->type = EVM_IMA_XATTR_DIGSIG ||\n" - "> +\t\t\t xattr_value->type = IMA_MODSIG))) {\n" + "> -\t\t\t xattr_value->type == EVM_IMA_XATTR_DIGSIG)) {\n" + "> +\t\t\t (xattr_value->type == EVM_IMA_XATTR_DIGSIG ||\n" + "> +\t\t\t xattr_value->type == IMA_MODSIG))) {\n" "> \t\t\tstatus = INTEGRITY_PASS;\n" "> \t\t}\n" "> \t\tintegrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,\n" @@ -431,9 +432,9 @@ "> \t\tif (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))\n" "> \t\t\treturn -EINVAL;\n" "> \t\tima_reset_appraise_flags(d_backing_inode(dentry),\n" - "> -\t\t\t (xvalue->type = EVM_IMA_XATTR_DIGSIG) ? 1 : 0);\n" - "> +\t\t\t\t\t xvalue->type = EVM_IMA_XATTR_DIGSIG ||\n" - "> +\t\t\t\t\t xvalue->type = IMA_MODSIG);\n" + "> -\t\t\t (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0);\n" + "> +\t\t\t\t\t xvalue->type == EVM_IMA_XATTR_DIGSIG ||\n" + "> +\t\t\t\t\t xvalue->type == IMA_MODSIG);\n" "\n" "Probably easier to read if we set a variable, before calling\n" "ima_reset_appraise_flags.\n" @@ -762,7 +763,7 @@ "> +\tif (!hdr)\n" "> +\t\treturn;\n" "> +\n" - "> +\tif (hdr->type = IMA_MODSIG) {\n" + "> +\tif (hdr->type == IMA_MODSIG) {\n" "> +\t\tstruct signature_modsig_hdr *modsig = (typeof(modsig)) hdr;\n" "> +\n" "> +\t\tpkcs7_free_message(modsig->pkcs7_msg);\n" @@ -778,11 +779,11 @@ "> \t\t\t}\n" "> \n" "> \t\t\tima_log_string(ab, \"appraise_type\", args[0].from);\n" - "> -\t\t\tif ((strcmp(args[0].from, \"imasig\")) = 0)\n" - "> +\t\t\tif (strcmp(args[0].from, \"imasig\") = 0)\n" + "> -\t\t\tif ((strcmp(args[0].from, \"imasig\")) == 0)\n" + "> +\t\t\tif (strcmp(args[0].from, \"imasig\") == 0)\n" "> \t\t\t\tentry->flags |= IMA_DIGSIG_REQUIRED;\n" "> +\t\t\telse if (ima_hook_supports_modsig(entry->func) &&\n" - "> +\t\t\t\t strcmp(args[0].from, \"modsig|imasig\") = 0)\n" + "> +\t\t\t\t strcmp(args[0].from, \"modsig|imasig\") == 0)\n" "> +\t\t\t\tentry->flags |= IMA_DIGSIG_REQUIRED\n" "> +\t\t\t\t\t\t| IMA_MODSIG_ALLOWED;\n" "> \t\t\telse\n" @@ -846,7 +847,7 @@ "> +\t * The xattr_value for IMA_MODSIG is a runtime structure containing\n" "> +\t * pointers. Get its raw data instead.\n" "> +\t */\n" - "> +\tif (xattr_value->type = IMA_MODSIG) {\n" + "> +\tif (xattr_value->type == IMA_MODSIG) {\n" "> +\t\trc = ima_modsig_serialize_data(xattr_value, &xattr_value,\n" "> +\t\t\t\t\t &xattr_len);\n" "> +\t\tif (rc)\n" @@ -891,4 +892,4 @@ "> \t\t\t const char *digest, int digestlen);\n" > -68eb04278ba47f64e5ebbcbb8a73c0b1ca0030c26c555e885541794dddda259c +c0689aa728bcf2b47f8af03101a8c274b63945b6cac14e91f8e82762ac786f26
diff --git a/a/1.txt b/N2/1.txt index 3c0bd61..fc7cdef 100644 --- a/a/1.txt +++ b/N2/1.txt @@ -60,11 +60,11 @@ On Wed, 2017-06-07 at 22:49 -0300, Thiago Jung Bauermann wrote: > > Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> -Thank you, Thiago. Appended signatures seem to be working proper now +Thank you, Thiago. ?Appended signatures seem to be working proper now with multiple keys on the IMA keyring. The length of this patch description is a good indication that this -patch needs to be broken up for easier review. A few +patch needs to be broken up for easier review. ?A few comments/suggestions inline below. > --- @@ -151,11 +151,12 @@ comments/suggestions inline below. > When splitting up this patch, the addition of this new function could -be a separate patch. The patch description would explain the need for +be a separate patch. ?The patch description would explain the need for a new function. > if (!keyring[id]) { -> keyring[id] > @@ -61,18 +60,29 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, +> keyring[id] = +> @@ -61,18 +60,29 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, > int err = PTR_ERR(keyring[id]); > pr_err("no %s keyring: %d\n", keyring_name[id], err); > keyring[id] = NULL; @@ -321,7 +322,7 @@ a new function. > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -172,6 +172,11 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, -> } else if (xattr_len = 17) +> } else if (xattr_len == 17) > return HASH_ALGO_MD5; > break; > + case IMA_MODSIG: @@ -338,23 +339,23 @@ a new function. > > - status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint); > - if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) { -> - if ((status = INTEGRITY_NOLABEL) -> - || (status = INTEGRITY_NOXATTRS)) +> - if ((status == INTEGRITY_NOLABEL) +> - || (status == INTEGRITY_NOXATTRS)) > + /* Appended signatures aren't protected by EVM. */ > + status = evm_verifyxattr(dentry, XATTR_NAME_IMA, -> + xattr_value->type = IMA_MODSIG ? +> + xattr_value->type == IMA_MODSIG ? > + NULL : xattr_value, rc, iint); > + if (status != INTEGRITY_PASS && status != INTEGRITY_UNKNOWN && -> + !(xattr_value->type = IMA_MODSIG && -> + (status = INTEGRITY_NOLABEL || status = INTEGRITY_NOXATTRS))) { - -This was messy to begin with, and now it is even more messy. For -appended signatures, we're only interested in INTEGRITY_FAIL. Maybe +> + !(xattr_value->type == IMA_MODSIG && +> + (status == INTEGRITY_NOLABEL || status == INTEGRITY_NOXATTRS))) { +? +This was messy to begin with, and now it is even more messy. ?For +appended signatures, we're only interested in INTEGRITY_FAIL. ?Maybe leave the existing "if" clause alone and define a new "if" clause. -> + if (status = INTEGRITY_NOLABEL || status = INTEGRITY_NOXATTRS) +> + if (status == INTEGRITY_NOLABEL || status == INTEGRITY_NOXATTRS) > cause = "missing-HMAC"; -> else if (status = INTEGRITY_FAIL) +> else if (status == INTEGRITY_FAIL) > cause = "invalid-HMAC"; > @@ -267,11 +276,18 @@ int ima_appraise_measurement(enum ima_hooks func, > status = INTEGRITY_PASS; @@ -367,7 +368,7 @@ leave the existing "if" clause alone and define a new "if" clause. > - iint->ima_hash->digest, > - iint->ima_hash->length); > + -> + if (xattr_value->type = EVM_IMA_XATTR_DIGSIG) +> + if (xattr_value->type == EVM_IMA_XATTR_DIGSIG) > + rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, > + (const char *)xattr_value, > + rc, iint->ima_hash->digest, @@ -379,9 +380,9 @@ leave the existing "if" clause alone and define a new "if" clause. Perhaps allowing IMA_MODSIG to flow into EVM_IMA_XATTR_DIGSIG on failure, would help restore process_measurements() to the way it was. - Further explanation below. +?Further explanation below.? -> if (rc = -EOPNOTSUPP) { +> if (rc == -EOPNOTSUPP) { > status = INTEGRITY_UNKNOWN; > } else if (rc) { > @@ -291,13 +307,15 @@ int ima_appraise_measurement(enum ima_hooks func, @@ -393,12 +394,12 @@ failure, would help restore process_measurements() to the way it was. > + xattr_value->type != IMA_MODSIG))) { > if (!ima_fix_xattr(dentry, iint)) > status = INTEGRITY_PASS; -> } else if ((inode->i_size = 0) && +> } else if ((inode->i_size == 0) && > (iint->flags & IMA_NEW_FILE) && > (xattr_value && -> - xattr_value->type = EVM_IMA_XATTR_DIGSIG)) { -> + (xattr_value->type = EVM_IMA_XATTR_DIGSIG || -> + xattr_value->type = IMA_MODSIG))) { +> - xattr_value->type == EVM_IMA_XATTR_DIGSIG)) { +> + (xattr_value->type == EVM_IMA_XATTR_DIGSIG || +> + xattr_value->type == IMA_MODSIG))) { > status = INTEGRITY_PASS; > } > integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename, @@ -406,9 +407,9 @@ failure, would help restore process_measurements() to the way it was. > if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST)) > return -EINVAL; > ima_reset_appraise_flags(d_backing_inode(dentry), -> - (xvalue->type = EVM_IMA_XATTR_DIGSIG) ? 1 : 0); -> + xvalue->type = EVM_IMA_XATTR_DIGSIG || -> + xvalue->type = IMA_MODSIG); +> - (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0); +> + xvalue->type == EVM_IMA_XATTR_DIGSIG || +> + xvalue->type == IMA_MODSIG); Probably easier to read if we set a variable, before calling ima_reset_appraise_flags. @@ -527,7 +528,7 @@ ima_reset_appraise_flags. > - There are four stages: collect measurement, store measurement, -appraise measurement and audit measurement. "Collect" needs to be +appraise measurement and audit measurement. ?"Collect" needs to be done if any one of the other stages is needed. > if (!pathbuf) /* ima_rdwr_violation possibly pre-fetched */ @@ -737,7 +738,7 @@ Mimi > + if (!hdr) > + return; > + -> + if (hdr->type = IMA_MODSIG) { +> + if (hdr->type == IMA_MODSIG) { > + struct signature_modsig_hdr *modsig = (typeof(modsig)) hdr; > + > + pkcs7_free_message(modsig->pkcs7_msg); @@ -753,11 +754,11 @@ Mimi > } > > ima_log_string(ab, "appraise_type", args[0].from); -> - if ((strcmp(args[0].from, "imasig")) = 0) -> + if (strcmp(args[0].from, "imasig") = 0) +> - if ((strcmp(args[0].from, "imasig")) == 0) +> + if (strcmp(args[0].from, "imasig") == 0) > entry->flags |= IMA_DIGSIG_REQUIRED; > + else if (ima_hook_supports_modsig(entry->func) && -> + strcmp(args[0].from, "modsig|imasig") = 0) +> + strcmp(args[0].from, "modsig|imasig") == 0) > + entry->flags |= IMA_DIGSIG_REQUIRED > + | IMA_MODSIG_ALLOWED; > else @@ -821,7 +822,7 @@ Mimi > + * The xattr_value for IMA_MODSIG is a runtime structure containing > + * pointers. Get its raw data instead. > + */ -> + if (xattr_value->type = IMA_MODSIG) { +> + if (xattr_value->type == IMA_MODSIG) { > + rc = ima_modsig_serialize_data(xattr_value, &xattr_value, > + &xattr_len); > + if (rc) @@ -864,4 +865,9 @@ Mimi > +struct key *integrity_keyring_from_id(const unsigned int id); > int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, > const char *digest, int digestlen); -> +> + +-- +To unsubscribe from this list: send the line "unsubscribe linux-security-module" in +the body of a message to majordomo at vger.kernel.org +More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/a/content_digest b/N2/content_digest index fdcf183..031feed 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -1,26 +1,9 @@ "ref\01496886555-10082-1-git-send-email-bauerman@linux.vnet.ibm.com\0" "ref\01496886555-10082-7-git-send-email-bauerman@linux.vnet.ibm.com\0" - "From\0Mimi Zohar <zohar@linux.vnet.ibm.com>\0" - "Subject\0Re: [PATCH v2 6/6] ima: Support module-style appended signatures for appraisal\0" - "Date\0Wed, 14 Jun 2017 12:39:32 +0000\0" - "To\0Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>" - " linux-security-module@vger.kernel.org\0" - "Cc\0linux-ima-devel@lists.sourceforge.net" - keyrings@vger.kernel.org - linux-crypto@vger.kernel.org - linuxppc-dev@lists.ozlabs.org - linux-kernel@vger.kernel.org - Dmitry Kasatkin <dmitry.kasatkin@gmail.com> - James Morris <james.l.morris@oracle.com> - Serge E. Hallyn <serge@hallyn.com> - David Howells <dhowells@redhat.com> - David Woodhouse <dwmw2@infradead.org> - Jessica Yu <jeyu@redhat.com> - Rusty Russell <rusty@rustcorp.com.au> - Herbert Xu <herbert@gondor.apana.org.au> - David S. Miller <davem@davemloft.net> - AKASHI - " Takahiro <takahiro.akashi@linaro.org>\0" + "From\0zohar@linux.vnet.ibm.com (Mimi Zohar)\0" + "Subject\0[PATCH v2 6/6] ima: Support module-style appended signatures for appraisal\0" + "Date\0Wed, 14 Jun 2017 08:39:32 -0400\0" + "To\0linux-security-module@vger.kernel.org\0" "\00:1\0" "b\0" "Hi Thiago,\n" @@ -85,11 +68,11 @@ "> \n" "> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>\n" "\n" - "Thank you, Thiago. \302\240Appended signatures seem to be working proper now\n" + "Thank you, Thiago. ?Appended signatures seem to be working proper now\n" "with multiple keys on the IMA keyring.\n" "\n" "The length of this patch description is a good indication that this\n" - "patch needs to be broken up for easier review. \302\240A few\n" + "patch needs to be broken up for easier review. ?A few\n" "comments/suggestions inline below.\n" "\n" "> ---\n" @@ -176,11 +159,12 @@ "> \n" "\n" "When splitting up this patch, the addition of this new function could\n" - "be a separate patch. \302\240The patch description would explain the need for\n" + "be a separate patch. ?The patch description would explain the need for\n" "a new function.\n" "\n" "> \tif (!keyring[id]) {\n" - "> \t\tkeyring[id] > @@ -61,18 +60,29 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,\n" + "> \t\tkeyring[id] =\n" + "> @@ -61,18 +60,29 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,\n" "> \t\t\tint err = PTR_ERR(keyring[id]);\n" "> \t\t\tpr_err(\"no %s keyring: %d\\n\", keyring_name[id], err);\n" "> \t\t\tkeyring[id] = NULL;\n" @@ -346,7 +330,7 @@ "> --- a/security/integrity/ima/ima_appraise.c\n" "> +++ b/security/integrity/ima/ima_appraise.c\n" "> @@ -172,6 +172,11 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,\n" - "> \t\t} else if (xattr_len = 17)\n" + "> \t\t} else if (xattr_len == 17)\n" "> \t\t\treturn HASH_ALGO_MD5;\n" "> \t\tbreak;\n" "> +\tcase IMA_MODSIG:\n" @@ -363,23 +347,23 @@ "> \n" "> -\tstatus = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);\n" "> -\tif ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {\n" - "> -\t\tif ((status = INTEGRITY_NOLABEL)\n" - "> -\t\t || (status = INTEGRITY_NOXATTRS))\n" + "> -\t\tif ((status == INTEGRITY_NOLABEL)\n" + "> -\t\t || (status == INTEGRITY_NOXATTRS))\n" "> +\t/* Appended signatures aren't protected by EVM. */\n" "> +\tstatus = evm_verifyxattr(dentry, XATTR_NAME_IMA,\n" - "> +\t\t\t\t xattr_value->type = IMA_MODSIG ?\n" + "> +\t\t\t\t xattr_value->type == IMA_MODSIG ?\n" "> +\t\t\t\t NULL : xattr_value, rc, iint);\n" "> +\tif (status != INTEGRITY_PASS && status != INTEGRITY_UNKNOWN &&\n" - "> +\t !(xattr_value->type = IMA_MODSIG &&\n" - "> +\t (status = INTEGRITY_NOLABEL || status = INTEGRITY_NOXATTRS))) {\n" - "\302\240\n" - "This was messy to begin with, and now it is even more messy. \302\240For\n" - "appended signatures, we're only interested in INTEGRITY_FAIL. \302\240Maybe\n" + "> +\t !(xattr_value->type == IMA_MODSIG &&\n" + "> +\t (status == INTEGRITY_NOLABEL || status == INTEGRITY_NOXATTRS))) {\n" + "?\n" + "This was messy to begin with, and now it is even more messy. ?For\n" + "appended signatures, we're only interested in INTEGRITY_FAIL. ?Maybe\n" "leave the existing \"if\" clause alone and define a new \"if\" clause.\n" "\n" - "> +\t\tif (status = INTEGRITY_NOLABEL || status = INTEGRITY_NOXATTRS)\n" + "> +\t\tif (status == INTEGRITY_NOLABEL || status == INTEGRITY_NOXATTRS)\n" "> \t\t\tcause = \"missing-HMAC\";\n" - "> \t\telse if (status = INTEGRITY_FAIL)\n" + "> \t\telse if (status == INTEGRITY_FAIL)\n" "> \t\t\tcause = \"invalid-HMAC\";\n" "> @@ -267,11 +276,18 @@ int ima_appraise_measurement(enum ima_hooks func,\n" "> \t\tstatus = INTEGRITY_PASS;\n" @@ -392,7 +376,7 @@ "> -\t\t\t\t\t iint->ima_hash->digest,\n" "> -\t\t\t\t\t iint->ima_hash->length);\n" "> +\n" - "> +\t\tif (xattr_value->type = EVM_IMA_XATTR_DIGSIG)\n" + "> +\t\tif (xattr_value->type == EVM_IMA_XATTR_DIGSIG)\n" "> +\t\t\trc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,\n" "> +\t\t\t\t\t\t (const char *)xattr_value,\n" "> +\t\t\t\t\t\t rc, iint->ima_hash->digest,\n" @@ -404,9 +388,9 @@ "\n" "Perhaps allowing IMA_MODSIG to flow into EVM_IMA_XATTR_DIGSIG on\n" "failure, would help restore process_measurements() to the way it was.\n" - "\302\240Further explanation below.\302\240\n" + "?Further explanation below.?\n" "\n" - "> \t\tif (rc = -EOPNOTSUPP) {\n" + "> \t\tif (rc == -EOPNOTSUPP) {\n" "> \t\t\tstatus = INTEGRITY_UNKNOWN;\n" "> \t\t} else if (rc) {\n" "> @@ -291,13 +307,15 @@ int ima_appraise_measurement(enum ima_hooks func,\n" @@ -418,12 +402,12 @@ "> +\t\t xattr_value->type != IMA_MODSIG))) {\n" "> \t\t\tif (!ima_fix_xattr(dentry, iint))\n" "> \t\t\t\tstatus = INTEGRITY_PASS;\n" - "> \t\t} else if ((inode->i_size = 0) &&\n" + "> \t\t} else if ((inode->i_size == 0) &&\n" "> \t\t\t (iint->flags & IMA_NEW_FILE) &&\n" "> \t\t\t (xattr_value &&\n" - "> -\t\t\t xattr_value->type = EVM_IMA_XATTR_DIGSIG)) {\n" - "> +\t\t\t (xattr_value->type = EVM_IMA_XATTR_DIGSIG ||\n" - "> +\t\t\t xattr_value->type = IMA_MODSIG))) {\n" + "> -\t\t\t xattr_value->type == EVM_IMA_XATTR_DIGSIG)) {\n" + "> +\t\t\t (xattr_value->type == EVM_IMA_XATTR_DIGSIG ||\n" + "> +\t\t\t xattr_value->type == IMA_MODSIG))) {\n" "> \t\t\tstatus = INTEGRITY_PASS;\n" "> \t\t}\n" "> \t\tintegrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,\n" @@ -431,9 +415,9 @@ "> \t\tif (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))\n" "> \t\t\treturn -EINVAL;\n" "> \t\tima_reset_appraise_flags(d_backing_inode(dentry),\n" - "> -\t\t\t (xvalue->type = EVM_IMA_XATTR_DIGSIG) ? 1 : 0);\n" - "> +\t\t\t\t\t xvalue->type = EVM_IMA_XATTR_DIGSIG ||\n" - "> +\t\t\t\t\t xvalue->type = IMA_MODSIG);\n" + "> -\t\t\t (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0);\n" + "> +\t\t\t\t\t xvalue->type == EVM_IMA_XATTR_DIGSIG ||\n" + "> +\t\t\t\t\t xvalue->type == IMA_MODSIG);\n" "\n" "Probably easier to read if we set a variable, before calling\n" "ima_reset_appraise_flags.\n" @@ -552,7 +536,7 @@ "> -\n" "\n" "There are four stages: collect measurement, store measurement,\n" - "appraise measurement and audit measurement. \302\240\"Collect\" needs to be\n" + "appraise measurement and audit measurement. ?\"Collect\" needs to be\n" "done if any one of the other stages is needed.\n" "\n" "> \tif (!pathbuf)\t/* ima_rdwr_violation possibly pre-fetched */\n" @@ -762,7 +746,7 @@ "> +\tif (!hdr)\n" "> +\t\treturn;\n" "> +\n" - "> +\tif (hdr->type = IMA_MODSIG) {\n" + "> +\tif (hdr->type == IMA_MODSIG) {\n" "> +\t\tstruct signature_modsig_hdr *modsig = (typeof(modsig)) hdr;\n" "> +\n" "> +\t\tpkcs7_free_message(modsig->pkcs7_msg);\n" @@ -778,11 +762,11 @@ "> \t\t\t}\n" "> \n" "> \t\t\tima_log_string(ab, \"appraise_type\", args[0].from);\n" - "> -\t\t\tif ((strcmp(args[0].from, \"imasig\")) = 0)\n" - "> +\t\t\tif (strcmp(args[0].from, \"imasig\") = 0)\n" + "> -\t\t\tif ((strcmp(args[0].from, \"imasig\")) == 0)\n" + "> +\t\t\tif (strcmp(args[0].from, \"imasig\") == 0)\n" "> \t\t\t\tentry->flags |= IMA_DIGSIG_REQUIRED;\n" "> +\t\t\telse if (ima_hook_supports_modsig(entry->func) &&\n" - "> +\t\t\t\t strcmp(args[0].from, \"modsig|imasig\") = 0)\n" + "> +\t\t\t\t strcmp(args[0].from, \"modsig|imasig\") == 0)\n" "> +\t\t\t\tentry->flags |= IMA_DIGSIG_REQUIRED\n" "> +\t\t\t\t\t\t| IMA_MODSIG_ALLOWED;\n" "> \t\t\telse\n" @@ -846,7 +830,7 @@ "> +\t * The xattr_value for IMA_MODSIG is a runtime structure containing\n" "> +\t * pointers. Get its raw data instead.\n" "> +\t */\n" - "> +\tif (xattr_value->type = IMA_MODSIG) {\n" + "> +\tif (xattr_value->type == IMA_MODSIG) {\n" "> +\t\trc = ima_modsig_serialize_data(xattr_value, &xattr_value,\n" "> +\t\t\t\t\t &xattr_len);\n" "> +\t\tif (rc)\n" @@ -889,6 +873,11 @@ "> +struct key *integrity_keyring_from_id(const unsigned int id);\n" "> int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,\n" "> \t\t\t const char *digest, int digestlen);\n" - > + "> \n" + "\n" + "--\n" + "To unsubscribe from this list: send the line \"unsubscribe linux-security-module\" in\n" + "the body of a message to majordomo at vger.kernel.org\n" + More majordomo info at http://vger.kernel.org/majordomo-info.html -68eb04278ba47f64e5ebbcbb8a73c0b1ca0030c26c555e885541794dddda259c +75a76f3dca76bff82febeae62b719f64401b98178706a58350a8b5d64e569efd
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.