[PATCH 1/2] crypto: KEYS: convert public key 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 | 211 +++-- 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 | 49 ++- 12 files changed, 135 insertions(+), 308 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 def
[PATCH 2/2] integrity: convert digsig to akcipher api
Convert asymmetric_verify to akcipher api. Signed-off-by: Tadeusz Struk --- security/integrity/digsig_asymmetric.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c index 4fec181..55f4830 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 = 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 0/2] 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 --- Tadeusz Struk (2): crypto: KEYS: convert public key to the akcipher api integrity: convert digsig to akcipher api 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 | 211 +++-- 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 | 49 ++- security/integrity/digsig_asymmetric.c| 10 - 13 files changed, 138 insertions(+), 315 deletions(-) delete mode 100644 crypto/asymmetric_keys/public_key.h delete mode 100644 crypto/asymmetric_keys/x509_rsakey.asn1 -- 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 2/2] security/integrity: make ima/ima_mok.c explicitly non-modular
The Kconfig currently controlling compilation of this code is: ima/Kconfig:config IMA_MOK_KEYRING ima/Kconfig: bool "Create IMA machine owner keys (MOK) and blacklist keyrings" ...meaning that it currently is not being built as a module by anyone. Lets remove the couple of traces of modularity so that when reading the driver there is no doubt it really is builtin-only. Since module_init translates to device_initcall in the non-modular case, the init ordering remains unchanged with this commit. Cc: Mimi Zohar Cc: Dmitry Kasatkin Cc: James Morris Cc: "Serge E. Hallyn" Cc: linux-ima-de...@lists.sourceforge.net Cc: linux-ima-u...@lists.sourceforge.net Cc: linux-security-module@vger.kernel.org Signed-off-by: Paul Gortmaker --- security/integrity/ima/ima_mok.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/security/integrity/ima/ima_mok.c b/security/integrity/ima/ima_mok.c index 8dad9a2b8e47..676885e4320e 100644 --- a/security/integrity/ima/ima_mok.c +++ b/security/integrity/ima/ima_mok.c @@ -16,7 +16,7 @@ #include #include #include -#include +#include #include @@ -52,5 +52,4 @@ __init int ima_mok_init(void) set_bit(KEY_FLAG_KEEP, &ima_blacklist_keyring->flags); return 0; } - -module_init(ima_mok_init); +device_initcall(ima_mok_init); -- 2.6.1 -- 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 1/2] security/keys: make big_key.c explicitly non-modular
The Kconfig currently controlling compilation of this code is: config BIG_KEYS bool "Large payload keys" ...meaning that it currently is not being built as a module by anyone. Lets remove the modular code that is essentially orphaned, so that when reading the driver there is no doubt it is builtin-only. Since module_init translates to device_initcall in the non-modular case, the init ordering remains unchanged with this commit. We also delete the MODULE_LICENSE tag since all that information is already contained at the top of the file in the comments. Cc: David Howells Cc: James Morris Cc: "Serge E. Hallyn" Cc: keyri...@linux-nfs.org Cc: linux-security-module@vger.kernel.org Signed-off-by: Paul Gortmaker --- security/keys/big_key.c | 15 +-- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/security/keys/big_key.c b/security/keys/big_key.c index 907c1522ee46..c721e398893a 100644 --- a/security/keys/big_key.c +++ b/security/keys/big_key.c @@ -9,7 +9,6 @@ * 2 of the Licence, or (at your option) any later version. */ -#include #include #include #include @@ -18,8 +17,6 @@ #include #include -MODULE_LICENSE("GPL"); - /* * Layout of key payload words. */ @@ -212,18 +209,8 @@ long big_key_read(const struct key *key, char __user *buffer, size_t buflen) return ret; } -/* - * Module stuff - */ static int __init big_key_init(void) { return register_key_type(&key_type_big_key); } - -static void __exit big_key_cleanup(void) -{ - unregister_key_type(&key_type_big_key); -} - -module_init(big_key_init); -module_exit(big_key_cleanup); +device_initcall(big_key_init); -- 2.6.1 -- 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 0/2] security: clarify that some code is really non-modular
The goal is to ensure that non-modular code doesn't appear modular just by accident. Here we have two more commits to do that and they are of the trivial nature (i.e. no ".remove" functions deleted and no need to block any unbind actions). We just change the registration functions to be the non modular versions and adjust the include headers to match. Paul Gortmaker (2): security/keys: make big_key.c explicitly non-modular security/integrity: make ima/ima_mok.c explicitly non-modular security/integrity/ima/ima_mok.c | 5 ++--- security/keys/big_key.c | 15 +-- 2 files changed, 3 insertions(+), 17 deletions(-) --- Cc: David Howells Cc: James Morris Cc: "Serge E. Hallyn" Cc: keyri...@linux-nfs.org Cc: linux-security-module@vger.kernel.org Cc: Mimi Zohar Cc: Dmitry Kasatkin Cc: James Morris Cc: "Serge E. Hallyn" Cc: linux-ima-de...@lists.sourceforge.net Cc: linux-ima-u...@lists.sourceforge.net Cc: linux-security-module@vger.kernel.org 2.6.1 -- 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 2/2] keys, trusted: seal with a policy
On Wed, 2015-12-09 at 16:24 +0200, Jarkko Sakkinen wrote: > On Tue, Dec 08, 2015 at 06:56:17PM -0500, Mimi Zohar wrote: > > On Tue, 2015-12-08 at 22:24 +0200, Jarkko Sakkinen wrote: > > > On Tue, Dec 08, 2015 at 01:01:02PM +0200, Jarkko Sakkinen wrote: > > > > On Tue, Dec 08, 2015 at 09:35:05AM +1100, James Morris wrote: > > > > > On Mon, 7 Dec 2015, Jarkko Sakkinen wrote: > > > > > > > > > > > On Fri, Nov 20, 2015 at 01:34:35PM +1100, James Morris wrote: > > > > > > > On Wed, 18 Nov 2015, Jarkko Sakkinen wrote: > > > > > > > > > > > > > > > On Wed, Nov 18, 2015 at 11:21:01AM +1100, James Morris wrote: > > > > > > > > > On Tue, 17 Nov 2015, Jarkko Sakkinen wrote: > > > > > > > > > > > > > > > > > > > } > > > > > > > > > > break; > > > > > > > > > > + case Opt_policydigest: > > > > > > > > > > + if (!tpm2 || > > > > > > > > > > + strlen(args[0].from) != (2 * > > > > > > > > > > opt->digest_len)) > > > > > > > > > > + return -EINVAL; > > > > > > > > > > + kfree(opt->policydigest); > > > > > > > > > > + opt->policydigest = > > > > > > > > > > kzalloc(opt->digest_len, > > > > > > > > > > + GFP_KERNEL); > > > > You're allocating the exact amount of storage needed. There's no reason > > to use kzalloc here or elsewhere in the patch. > > Yup. I'll change this. > > > > > > > > > > > > > > > > > > > Is it correct to kfree opt->policydigest here before > > > > > > > > > allocating it? > > > > > > > > > > > > > > > > I think so. The same option might be encountered multiple times. > > > > > > > > > > > > > > This would surely signify an error? > > > > > > > > > > > > I'm following the semantics of other options. That's why I > > > > > > implemented > > > > > > it that way for example: > > > > > > > > > > > > keyctl add trusted kmk "new 32 keyhandle=0x8000 > > > > > > keyhandle=0x8000" > > > > > > > > > > > > is perfectly OK. I just thought that it'd be more odd if this option > > > > > > behaved in a different way... > > > > > > > > > > It seems broken to me -- if you're messing up keyctl commands you > > > > > might > > > > > want to know about it, but we should remain consistent. > > > > > > > > So should I return error if policyhandle/digest appears a second time? I > > > > agree that it'd be better to return -EINVAL. > > > > > > > > The existing behavior is such that any option can appear multiple times > > > > and I chose to be consistent with that. > > > > > > Mimi, David? > > > > I don't have a problem with changing the existing behavior to allow the > > options to be specified only once. > > I don't think this patch is right place to change the behavior as it > should be done for other options too. I think the easiest way of checking if a token has already been seen would be to define a flag and use test_and_set_bit(token, ) after the following code snippet. while ((p = strsep(&c, " \t"))) { if (*p == '\0' || *p == ' ' || *p == '\t') continue; token = match_token(p, key_tokens, args); Having a separate patch is probably a good idea. 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 2/2] keys, trusted: seal with a policy
On Tue, Dec 08, 2015 at 06:56:17PM -0500, Mimi Zohar wrote: > On Tue, 2015-12-08 at 22:24 +0200, Jarkko Sakkinen wrote: > > On Tue, Dec 08, 2015 at 01:01:02PM +0200, Jarkko Sakkinen wrote: > > > On Tue, Dec 08, 2015 at 09:35:05AM +1100, James Morris wrote: > > > > On Mon, 7 Dec 2015, Jarkko Sakkinen wrote: > > > > > > > > > On Fri, Nov 20, 2015 at 01:34:35PM +1100, James Morris wrote: > > > > > > On Wed, 18 Nov 2015, Jarkko Sakkinen wrote: > > > > > > > > > > > > > On Wed, Nov 18, 2015 at 11:21:01AM +1100, James Morris wrote: > > > > > > > > On Tue, 17 Nov 2015, Jarkko Sakkinen wrote: > > > > > > > > > > > > > > > > > } > > > > > > > > > break; > > > > > > > > > + case Opt_policydigest: > > > > > > > > > + if (!tpm2 || > > > > > > > > > + strlen(args[0].from) != (2 * > > > > > > > > > opt->digest_len)) > > > > > > > > > + return -EINVAL; > > > > > > > > > + kfree(opt->policydigest); > > > > > > > > > + opt->policydigest = > > > > > > > > > kzalloc(opt->digest_len, > > > > > > > > > + GFP_KERNEL); > > You're allocating the exact amount of storage needed. There's no reason > to use kzalloc here or elsewhere in the patch. Yup. I'll change this. > > > > > > > > > > > > > > > > Is it correct to kfree opt->policydigest here before allocating > > > > > > > > it? > > > > > > > > > > > > > > I think so. The same option might be encountered multiple times. > > > > > > > > > > > > This would surely signify an error? > > > > > > > > > > I'm following the semantics of other options. That's why I implemented > > > > > it that way for example: > > > > > > > > > > keyctl add trusted kmk "new 32 keyhandle=0x8000 > > > > > keyhandle=0x8000" > > > > > > > > > > is perfectly OK. I just thought that it'd be more odd if this option > > > > > behaved in a different way... > > > > > > > > It seems broken to me -- if you're messing up keyctl commands you might > > > > want to know about it, but we should remain consistent. > > > > > > So should I return error if policyhandle/digest appears a second time? I > > > agree that it'd be better to return -EINVAL. > > > > > > The existing behavior is such that any option can appear multiple times > > > and I chose to be consistent with that. > > > > Mimi, David? > > I don't have a problem with changing the existing behavior to allow the > options to be specified only once. I don't think this patch is right place to change the behavior as it should be done for other options too. > BTW, you might want to fail the getoptions() parsing earlier, rather > than waiting until after the while loop to test "policydigest_len != > opt->digest_len". In both Opt_hash and Opt_policydigest you can check > to see if the other option has already been specified. Agreed. > Mimi /Jarkko -- 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