[PATCH v5 1/3] crypto: KEYS: convert public key and digsig asym to the akcipher api
This patch converts the module verification code to the new akcipher API. Signed-off-by: Tadeusz Struk --- crypto/asymmetric_keys/Kconfig|2 crypto/asymmetric_keys/Makefile |7 - crypto/asymmetric_keys/pkcs7_parser.c | 12 +- crypto/asymmetric_keys/pkcs7_trust.c |2 crypto/asymmetric_keys/pkcs7_verify.c |2 crypto/asymmetric_keys/public_key.c | 64 +++-- crypto/asymmetric_keys/public_key.h | 36 - crypto/asymmetric_keys/rsa.c | 213 +++-- crypto/asymmetric_keys/x509_cert_parser.c | 37 + crypto/asymmetric_keys/x509_public_key.c | 17 +- crypto/asymmetric_keys/x509_rsakey.asn1 |4 - include/crypto/public_key.h | 34 + 12 files changed, 134 insertions(+), 296 deletions(-) delete mode 100644 crypto/asymmetric_keys/public_key.h delete mode 100644 crypto/asymmetric_keys/x509_rsakey.asn1 diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig index 4870f28..905d745 100644 --- a/crypto/asymmetric_keys/Kconfig +++ b/crypto/asymmetric_keys/Kconfig @@ -22,7 +22,7 @@ config ASYMMETRIC_PUBLIC_KEY_SUBTYPE config PUBLIC_KEY_ALGO_RSA tristate "RSA public-key algorithm" - select MPILIB + select CRYPTO_RSA help This option enables support for the RSA algorithm (PKCS#1, RFC3447). diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile index cd1406f..b78a194 100644 --- a/crypto/asymmetric_keys/Makefile +++ b/crypto/asymmetric_keys/Makefile @@ -16,21 +16,18 @@ obj-$(CONFIG_X509_CERTIFICATE_PARSER) += x509_key_parser.o x509_key_parser-y := \ x509-asn1.o \ x509_akid-asn1.o \ - x509_rsakey-asn1.o \ x509_cert_parser.o \ x509_public_key.o $(obj)/x509_cert_parser.o: \ $(obj)/x509-asn1.h \ - $(obj)/x509_akid-asn1.h \ - $(obj)/x509_rsakey-asn1.h + $(obj)/x509_akid-asn1.h + $(obj)/x509-asn1.o: $(obj)/x509-asn1.c $(obj)/x509-asn1.h $(obj)/x509_akid-asn1.o: $(obj)/x509_akid-asn1.c $(obj)/x509_akid-asn1.h -$(obj)/x509_rsakey-asn1.o: $(obj)/x509_rsakey-asn1.c $(obj)/x509_rsakey-asn1.h clean-files+= x509-asn1.c x509-asn1.h clean-files+= x509_akid-asn1.c x509_akid-asn1.h -clean-files+= x509_rsakey-asn1.c x509_rsakey-asn1.h # # PKCS#7 message handling diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c index 758acab..12912c1 100644 --- a/crypto/asymmetric_keys/pkcs7_parser.c +++ b/crypto/asymmetric_keys/pkcs7_parser.c @@ -15,7 +15,7 @@ #include #include #include -#include "public_key.h" +#include #include "pkcs7_parser.h" #include "pkcs7-asn1.h" @@ -44,7 +44,7 @@ struct pkcs7_parse_context { static void pkcs7_free_signed_info(struct pkcs7_signed_info *sinfo) { if (sinfo) { - mpi_free(sinfo->sig.mpi[0]); + kfree(sinfo->sig.s); kfree(sinfo->sig.digest); kfree(sinfo->signing_cert_id); kfree(sinfo); @@ -616,16 +616,14 @@ int pkcs7_sig_note_signature(void *context, size_t hdrlen, const void *value, size_t vlen) { struct pkcs7_parse_context *ctx = context; - MPI mpi; BUG_ON(ctx->sinfo->sig.pkey_algo != PKEY_ALGO_RSA); - mpi = mpi_read_raw_data(value, vlen); - if (!mpi) + ctx->sinfo->sig.s = kmemdup(value, vlen, GFP_KERNEL); + if (!ctx->sinfo->sig.s) return -ENOMEM; - ctx->sinfo->sig.mpi[0] = mpi; - ctx->sinfo->sig.nr_mpi = 1; + ctx->sinfo->sig.s_size = vlen; return 0; } diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c index 90d6d47..3bbdcc7 100644 --- a/crypto/asymmetric_keys/pkcs7_trust.c +++ b/crypto/asymmetric_keys/pkcs7_trust.c @@ -17,7 +17,7 @@ #include #include #include -#include "public_key.h" +#include #include "pkcs7_parser.h" /** diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c index 325575c..f5db137 100644 --- a/crypto/asymmetric_keys/pkcs7_verify.c +++ b/crypto/asymmetric_keys/pkcs7_verify.c @@ -16,7 +16,7 @@ #include #include #include -#include "public_key.h" +#include #include "pkcs7_parser.h" /* diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 6db4c01..b383629 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -18,24 +18,16 @@ #include #include #include -#include "public_key.h" +#include MODULE_LICENSE("GPL"); const char *const pkey_algo_name[PKEY_ALGO__LAST] = { - [PKEY_ALGO_DSA] = "DSA", - [PKEY_ALGO_RSA] = "RSA", + [PKEY_ALGO_DSA] = "dsa", + [PKEY_ALGO_RSA] = "rsa", }; EXPORT_SYMBOL_GPL(pkey_algo_name); -const struct public_key_algorithm *pkey_algo[PKEY_ALGO__LAST] = { -#if defin
[PATCH v5 2/3] integrity: convert digsig to akcipher api
Convert asymmetric_verify to akcipher api. Signed-off-by: Tadeusz Struk --- security/integrity/Kconfig |1 + security/integrity/digsig_asymmetric.c | 10 +++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig index 73c457b..f0b2463 100644 --- a/security/integrity/Kconfig +++ b/security/integrity/Kconfig @@ -36,6 +36,7 @@ config INTEGRITY_ASYMMETRIC_KEYS select ASYMMETRIC_KEY_TYPE select ASYMMETRIC_PUBLIC_KEY_SUBTYPE select PUBLIC_KEY_ALGO_RSA +select CRYPTO_RSA select X509_CERTIFICATE_PARSER help This option enables digital signature verification using diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c index 4fec181..5629372 100644 --- a/security/integrity/digsig_asymmetric.c +++ b/security/integrity/digsig_asymmetric.c @@ -92,13 +92,9 @@ int asymmetric_verify(struct key *keyring, const char *sig, pks.pkey_hash_algo = hdr->hash_algo; pks.digest = (u8 *)data; pks.digest_size = datalen; - pks.nr_mpi = 1; - pks.rsa.s = mpi_read_raw_data(hdr->sig, siglen); - - if (pks.rsa.s) - ret = verify_signature(key, &pks); - - mpi_free(pks.rsa.s); + pks.s = hdr->sig; + pks.s_size = siglen; + ret = verify_signature(key, &pks); key_put(key); pr_debug("%s() = %d\n", __func__, ret); return ret; -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 3/3] crypto: public_key: remove MPIs from public_key_signature struct
After digsig_asymmetric.c is converted the MPIs can be now safely removed from the public_key_signature structure. Signed-off-by: Tadeusz Struk --- include/crypto/public_key.h | 14 +- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h index 50ac875..a1693ed 100644 --- a/include/crypto/public_key.h +++ b/include/crypto/public_key.h @@ -14,7 +14,6 @@ #ifndef _LINUX_PUBLIC_KEY_H #define _LINUX_PUBLIC_KEY_H -#include #include enum pkey_algo { @@ -73,20 +72,9 @@ struct public_key_signature { u8 *s; /* Signature */ u32 s_size; /* Number of bytes in signature */ u8 *digest; - u8 digest_size; /* Number of bytes in digest */ - u8 nr_mpi; /* Occupancy of mpi[] */ + u8 digest_size; /* Number of bytes in digest */ enum pkey_algo pkey_algo : 8; enum hash_algo pkey_hash_algo : 8; - union { - MPI mpi[2]; - struct { - MPI s; /* m^d mod n */ - } rsa; - struct { - MPI r; - MPI s; - } dsa; - }; }; extern struct asymmetric_key_subtype public_key_subtype; -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 0/3] crypto: KEYS: convert public key to akcipher api
This patch set converts the module verification and digital signature code to the new akcipher API. RSA implementation has been removed from crypto/asymmetric_keys and the new API is used for cryptographic primitives. There is no need for MPI above the akcipher API anymore. Modules can be verified with software as well as HW RSA implementations. Patches generated against cryptodev-2.6 Changes in v5: - Revert back v4 and add a new patch that removes the MPIs from the public_key_signature struct after the asymmetric_verify funtc in digsig is converted as proposed by Herbert. Changes in v4: - Flatten both patches into one to avoid bisect compilation problems. Changes in v3: - Don't include keys/asymmetric-type.h in crypto/public_key.h Changes in v2: - Fix the whey public_key_signature is setup. The pointer s needs to point to the signature instread of the signature_v2_hdr. - Select CRYPTO_RSA when INTEGRITY_ASYMMETRIC_KEYS is selected. --- Tadeusz Struk (3): crypto: KEYS: convert public key and digsig asym to the akcipher api integrity: convert digsig to akcipher api crypto: public_key: remove MPIs from public_key_signature struct crypto/asymmetric_keys/Kconfig|2 crypto/asymmetric_keys/Makefile |7 - crypto/asymmetric_keys/pkcs7_parser.c | 12 +- crypto/asymmetric_keys/pkcs7_trust.c |2 crypto/asymmetric_keys/pkcs7_verify.c |2 crypto/asymmetric_keys/public_key.c | 64 +++-- crypto/asymmetric_keys/public_key.h | 36 - crypto/asymmetric_keys/rsa.c | 213 +++-- crypto/asymmetric_keys/x509_cert_parser.c | 37 + crypto/asymmetric_keys/x509_public_key.c | 17 +- crypto/asymmetric_keys/x509_rsakey.asn1 |4 - include/crypto/public_key.h | 48 +-- security/integrity/Kconfig|1 security/integrity/digsig_asymmetric.c| 10 - 14 files changed, 139 insertions(+), 316 deletions(-) delete mode 100644 crypto/asymmetric_keys/public_key.h delete mode 100644 crypto/asymmetric_keys/x509_rsakey.asn1 -- -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] crypto: KEYS: convert public key and digsig asym to the akcipher api
On 12/23/2015 12:21 PM, Herbert Xu wrote: > Why not just leave the MPIs in the structure and only remove them > by adding a third patch? Right, I think that's a better way of resolving this. v5 on it's way. Thanks, -- TS -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 5/7] ima: measure and appraise firmware (improvement)
Instead of reading the firmware twice, once for measuring/appraising the firmware and again reading the file contents into memory, this patch reads the firmware once. Signed-off-by: Mimi Zohar --- drivers/base/firmware_class.c | 5 + include/linux/ima.h | 1 + security/integrity/ima/ima.h | 1 - security/integrity/ima/ima_appraise.c | 8 security/integrity/ima/ima_main.c | 2 +- security/integrity/ima/ima_policy.c | 24 +++- security/integrity/integrity.h| 11 --- 7 files changed, 22 insertions(+), 30 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 8524450..3ca96a6 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -29,6 +29,7 @@ #include #include #include +#include #include @@ -311,6 +312,10 @@ static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf) rc = -EIO; goto fail; } + rc = ima_hash_and_process_file(file, buf, size, FIRMWARE_CHECK); + if (rc) + goto fail; + rc = security_kernel_fw_from_file(file, buf, size); if (rc) goto fail; diff --git a/include/linux/ima.h b/include/linux/ima.h index 020de0f..c40fb3d 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -16,6 +16,7 @@ struct linux_binprm; enum ima_read_hooks { KEXEC_CHECK = 1, INITRAMFS_CHECK, + FIRMWARE_CHECK, IMA_MAX_READ_CHECK }; diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index fa801fa..7fb0ab7 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -165,7 +165,6 @@ enum ima_hooks { MMAP_CHECK, BPRM_CHECK, MODULE_CHECK, - FIRMWARE_CHECK, POST_SETATTR }; diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 3adf937..57b1ad1 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -76,8 +76,6 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, return iint->ima_bprm_status; case MODULE_CHECK: return iint->ima_module_status; - case FIRMWARE_CHECK: - return iint->ima_firmware_status; case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1: return iint->ima_read_status; case FILE_CHECK: @@ -99,9 +97,6 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint, case MODULE_CHECK: iint->ima_module_status = status; break; - case FIRMWARE_CHECK: - iint->ima_firmware_status = status; - break; case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1: iint->ima_read_status = status; break; @@ -124,9 +119,6 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, int func) case MODULE_CHECK: iint->flags |= (IMA_MODULE_APPRAISED | IMA_APPRAISED); break; - case FIRMWARE_CHECK: - iint->flags |= (IMA_FIRMWARE_APPRAISED | IMA_APPRAISED); - break; case KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1: break; case FILE_CHECK: diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index cd9c576..c55ee06 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -345,7 +345,7 @@ int ima_fw_from_file(struct file *file, char *buf, size_t size) return -EACCES; /* INTEGRITY_UNKNOWN */ return 0; } - return process_measurement(file, NULL, 0, MAY_EXEC, FIRMWARE_CHECK, 0); + return 0; } /** diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 26e24df..6ac25c5 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -102,7 +102,7 @@ static struct ima_rule_entry original_measurement_rules[] = { {.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_MASK | IMA_UID}, {.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC}, - {.action = MEASURE, .func = FIRMWARE_CHECK, .flags = IMA_FUNC}, + {.action = MEASURE, .read_func = FIRMWARE_CHECK, .flags = IMA_FUNC}, }; static struct ima_rule_entry default_measurement_rules[] = { @@ -115,7 +115,7 @@ static struct ima_rule_entry default_measurement_rules[] = { {.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_INMASK | IMA_UID}, {.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC}, - {.action = MEASURE, .func = FIRMWARE_CHECK, .flags = IMA_FUNC}, + {.action = MEASURE, .read_func = FIRMWARE_
[PATCH v2 0/7] ima: measuring/appraising files read by the kernel
This patch set closes a number of measurement/appraisal gaps by defining a generic function named ima_hash_and_process_file() for measuring and appraising files read by the kernel (eg. kexec image and initramfs, firmware, IMA policy). To differentiate between callers of ima_hash_and_process_file() in the IMA policy, a new enumeration is defined named ima_read_hooks, which initially includes KEXEC_CHECK, INITRAMFS_CHECK, FIRMWARE_CHECK, and POLICY_CHECK. Changelog v2: - Calculate the file hash from an in memory buffer (suggested by Dave Young) - Rename ima_read_and_process_file() to ima_hash_and_process_file() to reflect doing a buffer hash. - Changelog v1: - Instead of ima_read_and_process_file() allocating memory, the caller allocates and frees the memory. - Moved the kexec measurement/appraisal call to copy_file_from_fd(). The same call now measures and appraises both the kexec image and initramfs. Mimi Dmitry Kasatkin (3): ima: separate 'security.ima' reading functionality from collect ima: load policy using path ima: provide buffer hash calculation function Mimi Zohar (4): ima: measure and appraise kexec image and initramfs ima: measure and appraise firmware (improvement) ima: measure and appraise the IMA policy itself ima: require signed IMA policy Documentation/ABI/testing/ima_policy | 2 +- drivers/base/firmware_class.c | 5 +++ include/linux/ima.h | 18 kernel/kexec_file.c | 24 ++ security/integrity/digsig.c | 2 +- security/integrity/iint.c | 17 --- security/integrity/ima/ima.h | 36 +-- security/integrity/ima/ima_api.c | 19 +++- security/integrity/ima/ima_appraise.c | 38 security/integrity/ima/ima_crypto.c | 13 -- security/integrity/ima/ima_fs.c | 45 ++- security/integrity/ima/ima_init.c | 2 +- security/integrity/ima/ima_main.c | 50 - security/integrity/ima/ima_policy.c | 73 +++ security/integrity/ima/ima_template.c | 2 - security/integrity/ima/ima_template_lib.c | 1 - security/integrity/integrity.h| 14 +++--- 17 files changed, 255 insertions(+), 106 deletions(-) -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/7] ima: measure and appraise the IMA policy itself
Call ima_hash_and_process_file() to measure and appraise the IMA policy. This patch defines a new policy hook named POLICY_CHECK. Changelog v2: - remove S_ISREG() test Signed-off-by: Mimi Zohar --- include/linux/ima.h | 1 + security/integrity/digsig.c | 2 +- security/integrity/iint.c | 14 ++ security/integrity/ima/ima.h| 1 + security/integrity/ima/ima_fs.c | 12 +--- security/integrity/ima/ima_policy.c | 14 -- security/integrity/integrity.h | 4 +++- 7 files changed, 37 insertions(+), 11 deletions(-) diff --git a/include/linux/ima.h b/include/linux/ima.h index c40fb3d..1080623 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -17,6 +17,7 @@ enum ima_read_hooks { KEXEC_CHECK = 1, INITRAMFS_CHECK, FIRMWARE_CHECK, + POLICY_CHECK, IMA_MAX_READ_CHECK }; diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index 8ef1511..e58a5f6 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -104,7 +104,7 @@ int __init integrity_load_x509(const unsigned int id, const char *path) if (!keyring[id]) return -EINVAL; - rc = integrity_read_file(path, &data); + rc = integrity_read_file(path, &data, 0); if (rc < 0) return rc; diff --git a/security/integrity/iint.c b/security/integrity/iint.c index 8a45576..24776f7 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -205,7 +205,8 @@ int integrity_kernel_read(struct file *file, loff_t offset, * size, read entire file content to the buffer and closes the file * */ -int integrity_read_file(const char *path, char **data) +int integrity_read_file(const char *path, char **data, + enum ima_read_hooks read_func) { struct file *file; loff_t size; @@ -233,12 +234,17 @@ int integrity_read_file(const char *path, char **data) } rc = integrity_kernel_read(file, 0, buf, size); + if (rc > 0 && rc != size) + rc = -EIO; + else if (rc > 0) + rc = ima_hash_and_process_file(file, buf, size, read_func); + if (rc < 0) kfree(buf); - else if (rc != size) - rc = -EIO; - else + else { + rc = size; *data = buf; + } out: fput(file); return rc; diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 7fb0ab7..ef380f7 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -185,6 +185,7 @@ int ima_policy_show(struct seq_file *m, void *v); #define IMA_APPRAISE_LOG 0x04 #define IMA_APPRAISE_MODULES 0x08 #define IMA_APPRAISE_FIRMWARE 0x10 +#define IMA_APPRAISE_POLICY0x20 #ifdef CONFIG_IMA_APPRAISE int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index f902b6b..fdc5326 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -268,13 +268,12 @@ static ssize_t ima_read_policy(char *path) datap = path; strsep(&datap, "\n"); - rc = integrity_read_file(path, &data); + rc = integrity_read_file(path, &data, POLICY_CHECK); if (rc < 0) return rc; size = rc; datap = data; - while (size > 0 && (p = strsep(&datap, "\n"))) { pr_debug("rule: %s\n", p); rc = ima_parse_add_rule(p); @@ -324,7 +323,14 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, if (data[0] == '/') result = ima_read_policy(data); - else + else if (ima_appraise & IMA_APPRAISE_POLICY) { + pr_err("IMA: signed policy required\n"); + integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, + "policy_update", "signed policy required", + 1, 0); + if (ima_appraise & IMA_APPRAISE_ENFORCE) + result = -EACCES; + } else result = ima_parse_add_rule(data); out: if (result < 0) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 6ac25c5..3715c9e 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -116,6 +116,7 @@ static struct ima_rule_entry default_measurement_rules[] = { .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_INMASK | IMA_UID}, {.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC}, {.action = MEASURE, .read_func = FIRMWARE_CHECK, .flags = IMA_FUNC}, + {.action = MEASURE, .read_func = POLICY_CHECK, .flags = IMA_FUNC}, }; static struct ima_rule_entry default_appraise_rules[] = { @@ -610,6 +611,8 @@ static int ima_parse_rule(char *ru
[PATCH v2 1/7] ima: separate 'security.ima' reading functionality from collect
From: Dmitry Kasatkin Instead of passing pointers to pointers to ima_collect_measurent() to read and return the 'security.ima' xattr value, this patch moves the functionality to the calling process_measurement() to directly read the xattr and pass only the hash algo to the ima_collect_measurement(). Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 15 +++ security/integrity/ima/ima_api.c | 15 +++ security/integrity/ima/ima_appraise.c | 25 ++--- security/integrity/ima/ima_crypto.c | 2 +- security/integrity/ima/ima_init.c | 2 +- security/integrity/ima/ima_main.c | 11 +++ security/integrity/ima/ima_template.c | 2 -- security/integrity/ima/ima_template_lib.c | 1 - 8 files changed, 33 insertions(+), 40 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 917407f..9ebfec1 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -23,6 +23,7 @@ #include #include #include +#include #include "../integrity.h" @@ -140,9 +141,7 @@ static inline unsigned long ima_hash_key(u8 *digest) int ima_get_action(struct inode *inode, int mask, int function); int ima_must_measure(struct inode *inode, int mask, int function); int ima_collect_measurement(struct integrity_iint_cache *iint, - struct file *file, - struct evm_ima_xattr_data **xattr_value, - int *xattr_len); + struct file *file, enum hash_algo algo); void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, @@ -187,8 +186,8 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func); void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file); enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, int func); -void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len, - struct ima_digest_data *hash); +enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, +int xattr_len); int ima_read_xattr(struct dentry *dentry, struct evm_ima_xattr_data **xattr_value); @@ -220,10 +219,10 @@ static inline enum integrity_status ima_get_cache_status(struct integrity_iint_c return INTEGRITY_UNKNOWN; } -static inline void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, -int xattr_len, -struct ima_digest_data *hash) +static inline enum hash_algo +ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len) { + return ima_hash_algo; } static inline int ima_read_xattr(struct dentry *dentry, diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 1d950fb..e7c7a5d 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -18,7 +18,7 @@ #include #include #include -#include + #include "ima.h" /* @@ -188,9 +188,7 @@ int ima_get_action(struct inode *inode, int mask, int function) * Return 0 on success, error code otherwise */ int ima_collect_measurement(struct integrity_iint_cache *iint, - struct file *file, - struct evm_ima_xattr_data **xattr_value, - int *xattr_len) + struct file *file, enum hash_algo algo) { const char *audit_cause = "failed"; struct inode *inode = file_inode(file); @@ -201,9 +199,6 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, char digest[IMA_MAX_DIGEST_SIZE]; } hash; - if (xattr_value) - *xattr_len = ima_read_xattr(file->f_path.dentry, xattr_value); - if (!(iint->flags & IMA_COLLECTED)) { u64 i_version = file_inode(file)->i_version; @@ -213,11 +208,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, goto out; } - /* use default hash algorithm */ - hash.hdr.algo = ima_hash_algo; - - if (xattr_value) - ima_get_hash_algo(*xattr_value, *xattr_len, &hash.hdr); + hash.hdr.algo = algo; result = ima_calc_file_hash(file, &hash.hdr); if (!result) { diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 1873b55..9c2b46b 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -15,7 +15,6 @@ #include #include #include -#include #inclu
[PATCH v2 4/7] ima: measure and appraise kexec image and initramfs
This patch defines a new IMA hook ima_hash_and_process_file() for measuring and appraising files read by the kernel. The caller loads the file into memory before calling this function, which calculates the hash followed by the normal IMA policy based processing. Two new IMA policy functions named KEXEC_CHECK and INITRAMFS_CHECK are defined for measuring, appraising or auditing the kexec image and initramfs. Changelog v2: - Calculate the file hash from the in memory buffer (suggested by Dave Young) - Rename ima_read_and_process_file() to ima_hash_and_process_file() - replace individual case statements with range: KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1 v1: - Instead of ima_read_and_process_file() allocating memory, the caller allocates and frees the memory. - Moved the kexec measurement/appraisal call to copy_file_from_fd(). The same call now measures and appraises both the kexec image and initramfs. Signed-off-by: Mimi Zohar --- Documentation/ABI/testing/ima_policy | 2 +- include/linux/ima.h | 16 ++ kernel/kexec_file.c | 24 security/integrity/iint.c | 1 + security/integrity/ima/ima.h | 21 -- security/integrity/ima/ima_api.c | 6 +++-- security/integrity/ima/ima_appraise.c | 11 -- security/integrity/ima/ima_main.c | 41 --- security/integrity/ima/ima_policy.c | 38 security/integrity/integrity.h| 7 -- 10 files changed, 127 insertions(+), 40 deletions(-) diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index 0a378a8..e80f767 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -26,7 +26,7 @@ Description: option: [[appraise_type=]] [permit_directio] base: func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK] - [FIRMWARE_CHECK] + [FIRMWARE_CHECK] [KEXEC_CHECK] [INITRAMFS_CHECK] mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND] [[^]MAY_EXEC] fsmagic:= hex value diff --git a/include/linux/ima.h b/include/linux/ima.h index 120ccc5..020de0f 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -13,6 +13,12 @@ #include struct linux_binprm; +enum ima_read_hooks { + KEXEC_CHECK = 1, + INITRAMFS_CHECK, + IMA_MAX_READ_CHECK +}; + #ifdef CONFIG_IMA extern int ima_bprm_check(struct linux_binprm *bprm); extern int ima_file_check(struct file *file, int mask, int opened); @@ -20,6 +26,9 @@ extern void ima_file_free(struct file *file); extern int ima_file_mmap(struct file *file, unsigned long prot); extern int ima_module_check(struct file *file); extern int ima_fw_from_file(struct file *file, char *buf, size_t size); +extern int ima_hash_and_process_file(struct file *file, +void *buf, size_t size, +enum ima_read_hooks read_func); #else static inline int ima_bprm_check(struct linux_binprm *bprm) @@ -52,6 +61,13 @@ static inline int ima_fw_from_file(struct file *file, char *buf, size_t size) return 0; } +static inline int ima_hash_and_process_file(struct file *file, + void *buf, size_t size, + enum ima_read_hooks read_func) +{ + return 0; +} + #endif /* CONFIG_IMA */ #ifdef CONFIG_IMA_APPRAISE diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index b70ada0..1d0d998 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -33,7 +34,8 @@ size_t __weak kexec_purgatory_size = 0; static int kexec_calculate_store_digests(struct kimage *image); -static int copy_file_from_fd(int fd, void **buf, unsigned long *buf_len) +static int copy_file_from_fd(int fd, void **buf, unsigned long *buf_len, +enum ima_read_hooks read_func) { struct fd f = fdget(fd); int ret; @@ -70,9 +72,8 @@ static int copy_file_from_fd(int fd, void **buf, unsigned long *buf_len) bytes = kernel_read(f.file, pos, (char *)(*buf) + pos, stat.size - pos); if (bytes < 0) { - vfree(*buf); ret = bytes; - goto out; + goto out_free; } if (bytes == 0) @@ -80,13 +81,17 @@ static int copy_file_from_fd(int fd, void **buf, unsigned long *buf_len) pos += bytes; } - if (pos != stat.size) { + if (pos != stat.size) ret = -EBADF; + + ret = ima_hash_and_process_file(f.file, *buf,
[PATCH v2 3/7] ima: provide buffer hash calculation function
From: Dmitry Kasatkin This patch provides convenient buffer hash calculation function. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h| 2 ++ security/integrity/ima/ima_crypto.c | 11 +-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 9ebfec1..e3a5b7b 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -107,6 +107,8 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, const char *op, struct inode *inode, const unsigned char *filename); int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash); +int ima_calc_buffer_hash(const void *buf, int len, +struct ima_digest_data *hash); int ima_calc_field_array_hash(struct ima_field_data *field_data, struct ima_template_desc *desc, int num_fields, struct ima_digest_data *hash); diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index fb30ce4..aad1998 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -478,13 +478,13 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data, u8 *data_to_hash = field_data[i].data; u32 datalen = field_data[i].len; - if (strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) { + if (td && strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) { rc = crypto_shash_update(shash, (const u8 *) &field_data[i].len, sizeof(field_data[i].len)); if (rc) break; - } else if (strcmp(td->fields[i]->field_id, "n") == 0) { + } else if (td && strcmp(td->fields[i]->field_id, "n") == 0) { memcpy(buffer, data_to_hash, datalen); data_to_hash = buffer; datalen = IMA_EVENT_NAME_LEN_MAX + 1; @@ -519,6 +519,13 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data, return rc; } +int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash) +{ + struct ima_field_data fd = { .data = (u8 *)buf, .len = len }; + + return ima_calc_field_array_hash(&fd, NULL, 1, hash); +} + static void __init ima_pcrread(int idx, u8 *pcr) { if (!ima_used_chip) -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 7/7] ima: require signed IMA policy
Require the IMA policy to be signed when additional rules can be added. Changelog v1: - initialize the policy flag - include IMA_APPRAISE_POLICY in the policy flag Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_policy.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 3715c9e..8d5fd0b 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -131,6 +131,10 @@ static struct ima_rule_entry default_appraise_rules[] = { {.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_APPRAISE, .fsmagic = NSFS_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC}, +#ifdef CONFIG_IMA_WRITE_POLICY + {.action = APPRAISE, .read_func = POLICY_CHECK, + .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, +#endif #ifndef CONFIG_IMA_APPRAISE_SIGNED_INIT {.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER}, #else @@ -414,9 +418,12 @@ void __init ima_init_policy(void) for (i = 0; i < appraise_entries; i++) { list_add_tail(&default_appraise_rules[i].list, &ima_default_rules); + if (default_appraise_rules[i].read_func == POLICY_CHECK) + temp_ima_appraise |= IMA_APPRAISE_POLICY; } ima_rules = &ima_default_rules; + ima_update_policy_flag(); } /** -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/7] ima: load policy using path
From: Dmitry Kasatkin We currently cannot do appraisal or signature vetting of IMA policies since we currently can only load IMA policies by writing the contents of the policy directly in, as follows: cat policy-file > /ima/policy If we provide the kernel the path to the IMA policy so it can load the policy itself it'd be able to later appraise or vet the file signature if it has one. This patch adds support to load the IMA policy with a given path as follows: echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy Changelog v2: - Patch description re-written by Luis R. Rodriguez Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/iint.c | 4 +--- security/integrity/ima/ima_fs.c | 39 ++- security/integrity/integrity.h | 2 +- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/security/integrity/iint.c b/security/integrity/iint.c index 2de9c82..54b51a4 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -203,10 +203,8 @@ int integrity_kernel_read(struct file *file, loff_t offset, * This is function opens a file, allocates the buffer of required * size, read entire file content to the buffer and closes the file * - * It is used only by init code. - * */ -int __init integrity_read_file(const char *path, char **data) +int integrity_read_file(const char *path, char **data) { struct file *file; loff_t size; diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index eebb985..f902b6b 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -258,6 +258,40 @@ static const struct file_operations ima_ascii_measurements_ops = { .release = seq_release, }; +static ssize_t ima_read_policy(char *path) +{ + char *data, *datap; + int rc, size, pathlen = strlen(path); + char *p; + + /* remove \n */ + datap = path; + strsep(&datap, "\n"); + + rc = integrity_read_file(path, &data); + if (rc < 0) + return rc; + + size = rc; + datap = data; + + while (size > 0 && (p = strsep(&datap, "\n"))) { + pr_debug("rule: %s\n", p); + rc = ima_parse_add_rule(p); + if (rc < 0) + break; + size -= rc; + } + + kfree(data); + if (rc < 0) + return rc; + else if (size) + return -EINVAL; + else + return pathlen; +} + static ssize_t ima_write_policy(struct file *file, const char __user *buf, size_t datalen, loff_t *ppos) { @@ -288,7 +322,10 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, if (copy_from_user(data, buf, datalen)) goto out; - result = ima_parse_add_rule(data); + if (data[0] == '/') + result = ima_read_policy(data); + else + result = ima_parse_add_rule(data); out: if (result < 0) valid_policy = 0; diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 5efe2ec..5413f22 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -122,7 +122,7 @@ struct integrity_iint_cache *integrity_iint_find(struct inode *inode); int integrity_kernel_read(struct file *file, loff_t offset, char *addr, unsigned long count); -int __init integrity_read_file(const char *path, char **data); +int integrity_read_file(const char *path, char **data); #define INTEGRITY_KEYRING_EVM 0 #define INTEGRITY_KEYRING_IMA 1 -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/5] firmware: generalize "firmware" as "system data" helpers
From: "Luis R. Rodriguez" Historically firmware_class code was added to help get device driver firmware binaries but these days request_firmware*() helpers are being repurposed for general system data needed by the kernel. Annotate this before we extend firmare_class more, as this is expected. We want to generalize the code as much as possible. Cc: Rusty Russell Cc: Andrew Morton Cc: Greg Kroah-Hartman Cc: David Howells Cc: Kees Cook Cc: Casey Schaufler Cc: Ming Lei Cc: Takashi Iwai Cc: Vojtěch Pavlík Cc: Kyle McMartin Cc: Matthew Garrett Cc: linux-ker...@vger.kernel.org Signed-off-by: Luis R. Rodriguez --- drivers/base/firmware_class.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 8524450e75bd..6f5fcda71a60 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -353,15 +353,15 @@ static int fw_get_filesystem_firmware(struct device *device, rc = fw_read_file_contents(file, buf); fput(file); if (rc) - dev_warn(device, "firmware, attempted to load %s, but failed with error %d\n", - path, rc); + dev_warn(device, "system data, attempted to load %s, but failed with error %d\n", +path, rc); else break; } __putname(path); if (!rc) { - dev_dbg(device, "firmware: direct-loading firmware %s\n", + dev_dbg(device, "system data: direct-loading firmware %s\n", buf->fw_id); mutex_lock(&fw_lock); set_bit(FW_STATUS_DONE, &buf->status); @@ -1051,7 +1051,7 @@ _request_firmware_prepare(struct firmware **firmware_p, const char *name, } if (fw_get_builtin_firmware(firmware, name)) { - dev_dbg(device, "firmware: using built-in firmware %s\n", name); + dev_dbg(device, "system data: using built-in system data%s\n", name); return 0; /* assigned */ } -- 2.6.2 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/5] firmware: fold successful fw read early
From: David Howells We'll be folding in some more checks on fw_read_file_contents(), this will make the success case easier to follow. Signed-off-by: David Howells Signed-off-by: Luis R. Rodriguez --- drivers/base/firmware_class.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index d8148aa89b01..e10a5349aa61 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -361,20 +361,18 @@ static int fw_get_filesystem_firmware(struct device *device, continue; rc = fw_read_file_contents(file, buf); fput(file); - if (rc) + if (rc == 0) { + dev_dbg(device, "system data: direct-loading firmware %s\n", + buf->fw_id); + fw_finish_direct_load(device, buf); + goto out; + } else dev_warn(device, "system data, attempted to load %s, but failed with error %d\n", path, rc); - else - break; } +out: __putname(path); - if (!rc) { - dev_dbg(device, "system data: direct-loading firmware %s\n", - buf->fw_id); - fw_finish_direct_load(device, buf); - } - return rc; } -- 2.6.2 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/5] firmware: generalize reading file contents as a helper
From: David Howells We'll want to reuse this same code later in order to read two separate types of file contents. This generalizes fw_read_file_contents() for reading a file and rebrands it as fw_read_file(). This new caller is now generic: the path used can be arbitrary and the caller is also agnostic to the firmware_class code now, this begs the possibility of code re-use with other similar callers in the kernel. For instance in the future we may want to share a solution with: - firmware_class: fw_read_file() - module: kernel_read() - kexec: copy_file_fd() While at it this also cleans up the exit paths on fw_read_file(). Reviewed-by: Josh Boyer Signed-off-by: David Howells Signed-off-by: Luis R. Rodriguez --- drivers/base/firmware_class.c | 62 +++ 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index e10a5349aa61..cd51a90ed012 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -291,34 +291,51 @@ static const char * const fw_path[] = { module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644); MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path"); -static int fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf) +/* + * Read the contents of a file. + */ +static int fw_read_file(const char *path, void **_buf, size_t *_size) { - int size; + struct file *file; + size_t size; char *buf; int rc; + file = filp_open(path, O_RDONLY, 0); + if (IS_ERR(file)) + return PTR_ERR(file); + + rc = -EINVAL; if (!S_ISREG(file_inode(file)->i_mode)) - return -EINVAL; + goto err_file; size = i_size_read(file_inode(file)); if (size <= 0) - return -EINVAL; + goto err_file; + rc = -ENOMEM; buf = vmalloc(size); if (!buf) - return -ENOMEM; + goto err_file; + rc = kernel_read(file, 0, buf, size); + if (rc < 0) + goto err_buf; if (rc != size) { - if (rc > 0) - rc = -EIO; - goto fail; + rc = -EIO; + goto err_buf; } + rc = security_kernel_fw_from_file(file, buf, size); if (rc) - goto fail; - fw_buf->data = buf; - fw_buf->size = size; + goto err_buf; + + *_buf = buf; + *_size = size; return 0; -fail: + +err_buf: vfree(buf); +err_file: + fput(file); return rc; } @@ -332,19 +349,21 @@ static void fw_finish_direct_load(struct device *device, } static int fw_get_filesystem_firmware(struct device *device, - struct firmware_buf *buf) + struct firmware_buf *buf) { int i, len; int rc = -ENOENT; - char *path; + char *path = NULL; path = __getname(); if (!path) return -ENOMEM; + /* +* Try each possible firmware blob in turn till one doesn't produce +* ENOENT. +*/ for (i = 0; i < ARRAY_SIZE(fw_path); i++) { - struct file *file; - /* skip the unset customized path */ if (!fw_path[i][0]) continue; @@ -356,23 +375,20 @@ static int fw_get_filesystem_firmware(struct device *device, break; } - file = filp_open(path, O_RDONLY, 0); - if (IS_ERR(file)) - continue; - rc = fw_read_file_contents(file, buf); - fput(file); + rc = fw_read_file(path, &buf->data, &buf->size); if (rc == 0) { dev_dbg(device, "system data: direct-loading firmware %s\n", buf->fw_id); fw_finish_direct_load(device, buf); goto out; - } else + } else if (rc != -ENOENT) { dev_warn(device, "system data, attempted to load %s, but failed with error %d\n", path, rc); + goto out; + } } out: __putname(path); - return rc; } -- 2.6.2 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 5/5] firmware: add an extensible system data helpers
From: "Luis R. Rodriguez" The firmware API has evolved over the years slowly, as it grows we extend it by adding new routines or at times we extend existing routines with more or less arguments. This doesn't scale well, when new arguments are added to existing routines it means we need to traverse the kernel with a slew of collateral evolutions to adjust old driver users. The firmware API is also now being used for things outside of the scope of what typically would be considered "firmware", an example here is the p54 driver enables users to provide a custom EEPROM through this interface. Another example is optional CPU microcode updates. There are other subsystems which would like to make use of the APIs for similar things but have different requirements and criteria which they'd like to be met for the requested file. If different requirements are needed it would again mean adding more arguments and making a slew of collateral evolutions, or adding yet-another-new-API-call. Instead of extending the existing firmware API even more this provides an new extensible API which can be used to supercede the old firmware API without causing a series of collateral evolutions as requirements grow. This leaves the old firmware API as-is, ignores all consideration for usermode-helpers, labels the new API to reflect its broad use outside of the scope of firmware: system data helpers, and builds on top of the original firmware core code. The new extensible "system data" set of helpers accepts that there really are only two types of requests for accessing system data: a) synchronous requests b) asynchronous requests Both of these requests may have a different set of requirements which must be met. These requirements can simply be passed as a descriptors to each type of request. The descriptor can be extended over time to support different requirements as the kernel evolves. Using the new system data helpers is only necessary if you have requirements outside of what the existing old firmware API accepts. We encourage developers to leave the old API as-is and extend the new descriptors and system data code to provide support for new features. A few simple features added as part of the new set of system data request APIs, other than making the new API easily extensible for the future: - By default the kernel will free the system data file for you after your callbacks are called, you however are allowed to request to that you wish to keep the system data file on the descriptor. This is dealt with by requiring a consumer callback for the system data file. - Allow both asynchronous and synchronous request to specify that system data files are optional. With the old APIs we had added one full API call, request_firmware_direct() just for this purpose -- although it should be noted another of its goal was to also skip the usermode helper. The system data request APIs allow for you to annotate that a system data file is optional for both synchronous or asynchronous requests through the same two basic set of APIs. - Usermode helpers are completely ignored, always - The system data request APIs currently match the old synchronous firmware API calls to refcounted firmware_class module, but it should be easy to add support now to enable also refcounting the caller's module should it be be needed. Likewise the system data request APIs match the old asynchronous firmware API call and refcounts the caller's module. In order to try to help phase out user mode helpers this makes no use of the old user mode helper code *at all*, and if we wish to can easily phase this code out with time then. Signed-off-by: Luis R. Rodriguez --- Documentation/firmware_class/system_data.txt | 79 MAINTAINERS | 4 +- drivers/base/firmware_class.c| 291 +++ include/linux/sysdata.h | 212 +++ 4 files changed, 585 insertions(+), 1 deletion(-) create mode 100644 Documentation/firmware_class/system_data.txt create mode 100644 include/linux/sysdata.h diff --git a/Documentation/firmware_class/system_data.txt b/Documentation/firmware_class/system_data.txt new file mode 100644 index ..ab509243455a --- /dev/null +++ b/Documentation/firmware_class/system_data.txt @@ -0,0 +1,79 @@ +System data requests API + + +As the kernel evolves we keep extending the firmware_class set of APIs +with more or less arguments, this creates a slew of collateral evolutions. +The set of users of firmware request APIs has also grown now to include +users which are not looking for "firmware" per se, but instead general +system data files which for one reason or another has been decided to be +kept oustide of the kernel, and/or to allow dynamic updates. The system data +request set of APIs addresses rebranding of firmware as generic system data +files, and provides a way to enab
[PATCH v3 2/5] firmware: move completing fw into a helper
From: "Luis R. Rodriguez" This will be re-used later through a new extensible interface. Signed-off-by: Luis R. Rodriguez --- drivers/base/firmware_class.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 6f5fcda71a60..d8148aa89b01 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -322,6 +322,15 @@ fail: return rc; } +static void fw_finish_direct_load(struct device *device, + struct firmware_buf *buf) +{ + mutex_lock(&fw_lock); + set_bit(FW_STATUS_DONE, &buf->status); + complete_all(&buf->completion); + mutex_unlock(&fw_lock); +} + static int fw_get_filesystem_firmware(struct device *device, struct firmware_buf *buf) { @@ -363,10 +372,7 @@ static int fw_get_filesystem_firmware(struct device *device, if (!rc) { dev_dbg(device, "system data: direct-loading firmware %s\n", buf->fw_id); - mutex_lock(&fw_lock); - set_bit(FW_STATUS_DONE, &buf->status); - complete_all(&buf->completion); - mutex_unlock(&fw_lock); + fw_finish_direct_load(device, buf); } return rc; -- 2.6.2 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/5] firmware_class: extensible firmware API
From: "Luis R. Rodriguez" This v3 builds up on the last v2 series from October [0], I had not send out any updates after that as we had the kernel summit and figured it'd be best to hash out any kinks there. This patch set *only* provides a new set of extensible firmware helpers with strong semantics and which avoids the usermode helper completely. Support for firmware signing was discussed and we seem to have come to an agreement that it is needed, and how we'd handle certificates. This patch set does not address that -- that will be handled in another patch set. If you're curious to take a peak at how that will be handled though please have a look at the firmware enhancements wiki page [1], it provides Andy's latest recommendations. Changes since last v2 (thanks to Josh Boyer and Johannes Berg for feedback): * remove WARN_ON() and BUG_ON() - josh * make union sysdata_file_cbs const - johannes * refer to wiki on future enhancements page * upon request by gregkh added myself as maintainer Please note, I will be actually leaving on vacation far away from computers tomorrow, so posting this as it was overdue, I just had not had time to get to this yet. I'll be back January 13th. These patches are based on top of linux-next tag next-20151223. [0] http://lkml.kernel.org/r/1443721449-22882-1-git-send-email-mcg...@do-not-panic.com [1] http://kernelnewbies.org/KernelProjects/firmware-class-enhancements David Howells (2): firmware: fold successful fw read early firmware: generalize reading file contents as a helper Luis R. Rodriguez (3): firmware: generalize "firmware" as "system data" helpers firmware: move completing fw into a helper firmware: add an extensible system data helpers Documentation/firmware_class/system_data.txt | 79 ++ MAINTAINERS | 4 +- drivers/base/firmware_class.c| 385 --- include/linux/sysdata.h | 212 +++ 4 files changed, 642 insertions(+), 38 deletions(-) create mode 100644 Documentation/firmware_class/system_data.txt create mode 100644 include/linux/sysdata.h -- 2.6.2 -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] crypto: KEYS: convert public key and digsig asym to the akcipher api
On Wed, Dec 23, 2015 at 06:58:55AM -0800, Tadeusz Struk wrote: > > Because the first patch modifies the struct public_key and removes the MPIs > from it, > which the code modified in the second patch still uses. If bisect only takes > the first > then the build will fail on the security/integrity/digsig_asymmetric.c as > reported by > kbuild test robot: Why not just leave the MPIs in the structure and only remove them by adding a third patch? Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net] sctp: label accepted/peeled off sockets
On Wednesday, December 23, 2015 04:44:09 PM Marcelo Ricardo Leitner wrote: > From: Marcelo Ricardo Leitner > > Accepted or peeled off sockets were missing a security label (e.g. > SELinux) which means that socket was in "unlabeled" state. > > This patch clones the sock's label from the parent sock and resolves the > issue (similar to AF_BLUETOOTH protocol family). > > Cc: Paul Moore > Cc: David Teigland > Signed-off-by: Marcelo Ricardo Leitner > --- > net/sctp/socket.c | 2 ++ > 1 file changed, 2 insertions(+) [NOTE: added the LSM and SELinux lists to the CC line as a FYI] Proper SCTP support is on the SELinux todo list, but in the meantime it looks like the patch below should at least ensure that SCTP sockets inherit their parent's label which is probably the best we can hope for right now. Acked-by: Paul Moore > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index > 400a14d744834c7a503b338bc68f5f8b5b5dae8e..b67162767b7957b3e9f4f7bf52ab51fc1 > a3499c8 100644 --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -7202,6 +7202,8 @@ void sctp_copy_sock(struct sock *newsk, struct sock > *sk, > > if (newsk->sk_flags & SK_FLAGS_TIMESTAMP) > net_enable_timestamp(); > + > + security_sk_clone(sk, newsk); > } > > static inline void sctp_copy_descendant(struct sock *sk_to, -- paul moore www.paul-moore.com -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 00/17] CALIPSO implementation
On Tue, Dec 22, 2015 at 12:28 PM, Casey Schaufler wrote: > On 12/22/2015 3:46 AM, Huw Davies wrote: >> This patch series implements RFC 5570 - Common Architecture Label IPv6 >> Security Option (CALIPSO). Its goal is to set MLS sensitivity labels >> on IPv6 packets using a hop-by-hop option. CALIPSO very similar to >> its IPv4 cousin CIPSO and much of this series is based on that code. > > There's a one line change to the Smack code in 15/17 due to > a change in the api, but I assume that there has been no > attempt to verify that this works with Smack. It's not 100% > clear that this won't break a Smack kernel, but I haven't > tried it. > > You'll need to provide sufficient information (or code!) so > that security modules other than SELinux can use this. If > you look at how Smack uses netlabel for IPv4 you will see > that it differs substantially from the way SELinux uses it. Smack is going to have some difficulties implementing CALIPSO due to some previous design decisions and inconsistencies between how Smack handles both IPv4 and IPv6 packet labeling today. I think we can all agree that asking Huw to resolve these problems isn't quite fair, although asking Huw to make sure he doesn't break existing functionality *is* fair, and a requirement for patch acceptance. Huw, I would suggest you ensure that the NetLabel/CALIPSO changes don't break the existing Smack code, and that everything is commented appropriately. Adding CALIPSO support to the netlbl_cfg_*() functions in netlabel_kapi.c would also be a good idea, and shouldn't be too difficult (I should have commented on this earlier, my mistake). However, I think resolving the Smack IPv6 design issues is something best left to Casey and the rest of the Smack developers. > Thank you for tackling RFC 5570. The lack of something like > this has put IPv6 at a real disadvantage. Agreed, thanks Huw for all the hard work you put into this implementation. I started a similar effort on two separate occasions but never had the time to see it through to the end; I'm happy that someone was finally able to get it finished. -- paul moore www.paul-moore.com -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] crypto: KEYS: convert public key and digsig asym to the akcipher api
On 12/23/2015 06:51 AM, Herbert Xu wrote: >> > Changes in v4: >> > - Flatten both patches into one to avoid bisect compilation problems. > Why is this necessary? Because the first patch modifies the struct public_key and removes the MPIs from it, which the code modified in the second patch still uses. If bisect only takes the first then the build will fail on the security/integrity/digsig_asymmetric.c as reported by kbuild test robot: >> security/integrity/digsig_asymmetric.c:95:5: error: 'struct >> public_key_signature' has no member named 'nr_mpi' pks.nr_mpi = 1; Thanks, -- TS -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] crypto: KEYS: convert public key and digsig asym to the akcipher api
On Wed, Dec 23, 2015 at 06:33:53AM -0800, Tadeusz Struk wrote: > > Changes in v4: > - Flatten both patches into one to avoid bisect compilation problems. Why is this necessary? Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] crypto: KEYS: convert public key and digsig asym to the akcipher api
This patch converts the module verification and digital signature code to the new akcipher API. RSA implementation has been removed from crypto/asymmetric_keys and the new API is used for cryptographic primitives. There is no need for MPI above the akcipher API anymore. Modules can be verified with software as well as HW RSA implementations. Patches generated against cryptodev-2.6 Changes in v4: - Flatten both patches into one to avoid bisect compilation problems. Changes in v3: - Don't include keys/asymmetric-type.h in crypto/public_key.h Changes in v2: - Fix the whey public_key_signature is setup. The pointer s needs to point to the signature instread of the signature_v2_hdr. - Select CRYPTO_RSA when INTEGRITY_ASYMMETRIC_KEYS is selected. Signed-off-by: Tadeusz Struk --- crypto/asymmetric_keys/Kconfig|2 crypto/asymmetric_keys/Makefile |7 - crypto/asymmetric_keys/pkcs7_parser.c | 12 +- crypto/asymmetric_keys/pkcs7_trust.c |2 crypto/asymmetric_keys/pkcs7_verify.c |2 crypto/asymmetric_keys/public_key.c | 64 +++-- crypto/asymmetric_keys/public_key.h | 36 - crypto/asymmetric_keys/rsa.c | 213 +++-- crypto/asymmetric_keys/x509_cert_parser.c | 37 + crypto/asymmetric_keys/x509_public_key.c | 17 +- crypto/asymmetric_keys/x509_rsakey.asn1 |4 - include/crypto/public_key.h | 48 +-- security/integrity/Kconfig|1 security/integrity/digsig_asymmetric.c| 10 - 14 files changed, 139 insertions(+), 316 deletions(-) delete mode 100644 crypto/asymmetric_keys/public_key.h delete mode 100644 crypto/asymmetric_keys/x509_rsakey.asn1 diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig index 4870f28..905d745 100644 --- a/crypto/asymmetric_keys/Kconfig +++ b/crypto/asymmetric_keys/Kconfig @@ -22,7 +22,7 @@ config ASYMMETRIC_PUBLIC_KEY_SUBTYPE config PUBLIC_KEY_ALGO_RSA tristate "RSA public-key algorithm" - select MPILIB + select CRYPTO_RSA help This option enables support for the RSA algorithm (PKCS#1, RFC3447). diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile index cd1406f..b78a194 100644 --- a/crypto/asymmetric_keys/Makefile +++ b/crypto/asymmetric_keys/Makefile @@ -16,21 +16,18 @@ obj-$(CONFIG_X509_CERTIFICATE_PARSER) += x509_key_parser.o x509_key_parser-y := \ x509-asn1.o \ x509_akid-asn1.o \ - x509_rsakey-asn1.o \ x509_cert_parser.o \ x509_public_key.o $(obj)/x509_cert_parser.o: \ $(obj)/x509-asn1.h \ - $(obj)/x509_akid-asn1.h \ - $(obj)/x509_rsakey-asn1.h + $(obj)/x509_akid-asn1.h + $(obj)/x509-asn1.o: $(obj)/x509-asn1.c $(obj)/x509-asn1.h $(obj)/x509_akid-asn1.o: $(obj)/x509_akid-asn1.c $(obj)/x509_akid-asn1.h -$(obj)/x509_rsakey-asn1.o: $(obj)/x509_rsakey-asn1.c $(obj)/x509_rsakey-asn1.h clean-files+= x509-asn1.c x509-asn1.h clean-files+= x509_akid-asn1.c x509_akid-asn1.h -clean-files+= x509_rsakey-asn1.c x509_rsakey-asn1.h # # PKCS#7 message handling diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c index 758acab..12912c1 100644 --- a/crypto/asymmetric_keys/pkcs7_parser.c +++ b/crypto/asymmetric_keys/pkcs7_parser.c @@ -15,7 +15,7 @@ #include #include #include -#include "public_key.h" +#include #include "pkcs7_parser.h" #include "pkcs7-asn1.h" @@ -44,7 +44,7 @@ struct pkcs7_parse_context { static void pkcs7_free_signed_info(struct pkcs7_signed_info *sinfo) { if (sinfo) { - mpi_free(sinfo->sig.mpi[0]); + kfree(sinfo->sig.s); kfree(sinfo->sig.digest); kfree(sinfo->signing_cert_id); kfree(sinfo); @@ -616,16 +616,14 @@ int pkcs7_sig_note_signature(void *context, size_t hdrlen, const void *value, size_t vlen) { struct pkcs7_parse_context *ctx = context; - MPI mpi; BUG_ON(ctx->sinfo->sig.pkey_algo != PKEY_ALGO_RSA); - mpi = mpi_read_raw_data(value, vlen); - if (!mpi) + ctx->sinfo->sig.s = kmemdup(value, vlen, GFP_KERNEL); + if (!ctx->sinfo->sig.s) return -ENOMEM; - ctx->sinfo->sig.mpi[0] = mpi; - ctx->sinfo->sig.nr_mpi = 1; + ctx->sinfo->sig.s_size = vlen; return 0; } diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c index 90d6d47..3bbdcc7 100644 --- a/crypto/asymmetric_keys/pkcs7_trust.c +++ b/crypto/asymmetric_keys/pkcs7_trust.c @@ -17,7 +17,7 @@ #include #include #include -#include "public_key.h" +#include #include "pkcs7_parser.h" /** diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c index 325575c..f5db137 100644 --- a/crypto/asymmetric_keys/pkcs7_verify.c +++ b/crypto/asymmetric_keys/pkcs
Re: [Linux-ima-devel] [PATCH] IMA: policy can be updated zero times
On Wed, 2015-12-23 at 07:24 -0500, Mimi Zohar wrote: > On Wed, 2015-12-23 at 13:47 +0200, Petko Manolov wrote: > > > On 15-12-22 16:50:01, Sasha Levin wrote: > > > On 12/22/2015 04:40 PM, Petko Manolov wrote: > > > >> Thanks, Sasha. By the time ima_update_policy() is called > > > >> >ima_release_policy() has already output the policy update status > > > >> >message. I guess an empty policy could be considered a valid policy. > > > >> >Could you add a msg indicating that the new policy was empty? > > > > > > > > As far as I can say we can't get to ima_update_policy() with empty > > > > ima_temp_rules because ima_write_policy() will set valid_policy to 0 in > > > > case > > > > of an empty rule. I'll double check it tomorrow, but please you do > > > > that > > > > too. > > > > > > This is based on an actual crash rather than code analysis. > > > > I was able to reproduce the crash with: echo "" > > > /sys/kernel/security/ima/policy > > > > It turns out ima_parse_add_rule() returns 1, even though the string is > > empty > > This logic may be part of "empty policy is a valid policy" or something > > else. > > As it is more dangerous to change the behavior at this point i assume your > > patch > > is the right solution for the problem. > > > > Acked-by: Petko Manolov > > > > Mimi, shall we change ima_parse_add_rule's behavior in the future or it's > > too > > much work? > > ima_parse_add_rules() has no way of knowing if the policy as a whole is > valid. I would define a new function in ima_policy.c to return the > number of rules being added and call it at the beginning of > ima_release_policy() before the status message. That way the number of > rules added can be included in the status message. > > For now, the function could just return have rules or no rules, instead > of the number of rules. Sasha, could you make your fix a separate function (above ima_update_policy) and call it from ima_release_policy()? Thanks! Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] IMA: policy can be updated zero times
On Wed, 2015-12-23 at 13:47 +0200, Petko Manolov wrote: > On 15-12-22 16:50:01, Sasha Levin wrote: > > On 12/22/2015 04:40 PM, Petko Manolov wrote: > > >> Thanks, Sasha. By the time ima_update_policy() is called > > >> >ima_release_policy() has already output the policy update status > > >> >message. I guess an empty policy could be considered a valid policy. > > >> >Could you add a msg indicating that the new policy was empty? > > > > > > As far as I can say we can't get to ima_update_policy() with empty > > > ima_temp_rules because ima_write_policy() will set valid_policy to 0 in > > > case > > > of an empty rule. I'll double check it tomorrow, but please you do that > > > too. > > > > This is based on an actual crash rather than code analysis. > > I was able to reproduce the crash with: echo "" > > /sys/kernel/security/ima/policy > > It turns out ima_parse_add_rule() returns 1, even though the string is empty > This logic may be part of "empty policy is a valid policy" or something else. > > As it is more dangerous to change the behavior at this point i assume your > patch > is the right solution for the problem. > > Acked-by: Petko Manolov > > Mimi, shall we change ima_parse_add_rule's behavior in the future or it's too > much work? ima_parse_add_rules() has no way of knowing if the policy as a whole is valid. I would define a new function in ima_policy.c to return the number of rules being added and call it at the beginning of ima_release_policy() before the status message. That way the number of rules added can be included in the status message. For now, the function could just return have rules or no rules, instead of the number of rules. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] IMA: policy can be updated zero times
On 15-12-22 16:50:01, Sasha Levin wrote: > On 12/22/2015 04:40 PM, Petko Manolov wrote: > >> Thanks, Sasha. By the time ima_update_policy() is called > >> >ima_release_policy() has already output the policy update status > >> >message. I guess an empty policy could be considered a valid policy. > >> >Could you add a msg indicating that the new policy was empty? > > > > As far as I can say we can't get to ima_update_policy() with empty > > ima_temp_rules because ima_write_policy() will set valid_policy to 0 in > > case > > of an empty rule. I'll double check it tomorrow, but please you do that > > too. > > This is based on an actual crash rather than code analysis. I was able to reproduce the crash with: echo "" > /sys/kernel/security/ima/policy It turns out ima_parse_add_rule() returns 1, even though the string is empty This logic may be part of "empty policy is a valid policy" or something else. As it is more dangerous to change the behavior at this point i assume your patch is the right solution for the problem. Acked-by: Petko Manolov Mimi, shall we change ima_parse_add_rule's behavior in the future or it's too much work? cheers, Petko -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/2] crypto: KEYS: convert public key to the akcipher api
Hi Tadeusz, [auto build test ERROR on cryptodev/master] [also build test ERROR on v4.4-rc6 next-20151223] url: https://github.com/0day-ci/linux/commits/Tadeusz-Struk/crypto-KEYS-convert-public-key-to-akcipher-api/20151223-132001 base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master config: xtensa-allyesconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=xtensa Note: the linux-review/Tadeusz-Struk/crypto-KEYS-convert-public-key-to-akcipher-api/20151223-132001 HEAD 06fa55a0feec438fdc649cdac682af7ab99f8099 builds fine. It only hurts bisectibility. All errors (new ones prefixed by >>): security/integrity/digsig_asymmetric.c: In function 'asymmetric_verify': >> security/integrity/digsig_asymmetric.c:95:5: error: 'struct >> public_key_signature' has no member named 'nr_mpi' pks.nr_mpi = 1; ^ >> security/integrity/digsig_asymmetric.c:96:5: error: 'struct >> public_key_signature' has no member named 'rsa' pks.rsa.s = mpi_read_raw_data(hdr->sig, siglen); ^ >> security/integrity/digsig_asymmetric.c:96:2: error: implicit declaration of >> function 'mpi_read_raw_data' [-Werror=implicit-function-declaration] pks.rsa.s = mpi_read_raw_data(hdr->sig, siglen); ^ security/integrity/digsig_asymmetric.c:98:9: error: 'struct public_key_signature' has no member named 'rsa' if (pks.rsa.s) ^ >> security/integrity/digsig_asymmetric.c:101:2: error: implicit declaration of >> function 'mpi_free' [-Werror=implicit-function-declaration] mpi_free(pks.rsa.s); ^ security/integrity/digsig_asymmetric.c:101:14: error: 'struct public_key_signature' has no member named 'rsa' mpi_free(pks.rsa.s); ^ cc1: some warnings being treated as errors vim +95 security/integrity/digsig_asymmetric.c e0751257 Dmitry Kasatkin 2013-02-07 89 e0751257 Dmitry Kasatkin 2013-02-07 90memset(&pks, 0, sizeof(pks)); e0751257 Dmitry Kasatkin 2013-02-07 91 e0751257 Dmitry Kasatkin 2013-02-07 92pks.pkey_hash_algo = hdr->hash_algo; e0751257 Dmitry Kasatkin 2013-02-07 93pks.digest = (u8 *)data; e0751257 Dmitry Kasatkin 2013-02-07 94pks.digest_size = datalen; e0751257 Dmitry Kasatkin 2013-02-07 @95pks.nr_mpi = 1; e0751257 Dmitry Kasatkin 2013-02-07 @96pks.rsa.s = mpi_read_raw_data(hdr->sig, siglen); e0751257 Dmitry Kasatkin 2013-02-07 97 e0751257 Dmitry Kasatkin 2013-02-07 98if (pks.rsa.s) e0751257 Dmitry Kasatkin 2013-02-07 99ret = verify_signature(key, &pks); e0751257 Dmitry Kasatkin 2013-02-07 100 e0751257 Dmitry Kasatkin 2013-02-07 @101mpi_free(pks.rsa.s); e0751257 Dmitry Kasatkin 2013-02-07 102key_put(key); e0751257 Dmitry Kasatkin 2013-02-07 103pr_debug("%s() = %d\n", __func__, ret); e0751257 Dmitry Kasatkin 2013-02-07 104return ret; :: The code at line 95 was first introduced by commit :: e0751257a64ea10cca96ccb06522bfb10e36cb5b ima: digital signature verification using asymmetric keys :: TO: Dmitry Kasatkin :: CC: Mimi Zohar --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data