Re: [PATCH 2/2] keys, trusted: seal with a policy

[Mimi Zohar]

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(, " \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

[PATCH 0/2] security: clarify that some code is really non-modular

[Paul Gortmaker]

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

[PATCH 1/2] security/keys: make big_key.c explicitly non-modular

[Paul Gortmaker]

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(_type_big_key);
 }
-
-static void __exit big_key_cleanup(void)
-{
-   unregister_key_type(_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 2/2] security/integrity: make ima/ima_mok.c explicitly non-modular

[Paul Gortmaker]

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, _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 0/2] crypto: KEYS: convert public key to akcipher api

[Tadeusz Struk]

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] integrity: convert digsig to akcipher api

[Tadeusz Struk]

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, );
-
-   mpi_free(pks.rsa.s);
+   pks.s = sig;
+   pks.s_size = siglen;
+   ret = verify_signature(key, );
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 1/2] crypto: KEYS: convert public key to the akcipher api

[Tadeusz Struk]

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