Re: [PATCH V3 1/2] evm: Don't deadlock if a crypto algorithm is unavailable

2018-06-12 Thread Matthew Garrett 
On Fri, Jun 8, 2018 at 2:57 PM Matthew Garrett  wrote:
>
> When EVM attempts to appraise a file signed with a crypto algorithm the
> kernel doesn't have support for, it will cause the kernel to trigger a
> module load. If the EVM policy includes appraisal of kernel modules this
> will in turn call back into EVM - since EVM is holding a lock until the
> crypto initialisation is complete, this triggers a deadlock. Add a
> CRYPTO_NOLOAD flag and skip module loading if it's set, and add that flag
> in the EVM case in order to fail gracefully with an error message
> instead of deadlocking.

Hi Herbert,

Does this explain the problem sufficiently?

[PATCH V3 2/2] evm: Allow non-SHA1 digital signatures

2018-06-08 Thread Matthew Garrett 
SHA1 is reasonable in HMAC constructs, but it's desirable to be able to
use stronger hashes in digital signatures. Modify the EVM crypto code so
the hash type is imported from the digital signature and passed down to
the hash calculation code, and return the digest size to higher layers
for validation.

Signed-off-by: Matthew Garrett 
---
 security/integrity/evm/evm.h| 10 --
 security/integrity/evm/evm_crypto.c | 47 +++--
 security/integrity/evm/evm_main.c   | 19 +++-
 3 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 1257c3c24723..c3f437f5db10 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -47,6 +47,11 @@ extern struct crypto_shash *hash_tfm;
 /* List of EVM protected security xattrs */
 extern struct list_head evm_config_xattrnames;
 
+struct evm_digest {
+   struct ima_digest_data hdr;
+   char digest[IMA_MAX_DIGEST_SIZE];
+} __packed;
+
 int evm_init_key(void);
 int evm_update_evmxattr(struct dentry *dentry,
const char *req_xattr_name,
@@ -54,10 +59,11 @@ int evm_update_evmxattr(struct dentry *dentry,
size_t req_xattr_value_len);
 int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
  const char *req_xattr_value,
- size_t req_xattr_value_len, char *digest);
+ size_t req_xattr_value_len, struct evm_digest *data);
 int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
  const char *req_xattr_value,
- size_t req_xattr_value_len, char type, char *digest);
+ size_t req_xattr_value_len, char type,
+ struct evm_digest *data);
 int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
  char *hmac_val);
 int evm_init_secfs(void);
diff --git a/security/integrity/evm/evm_crypto.c 
b/security/integrity/evm/evm_crypto.c
index bea847dc0919..4a4414eb6bba 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "evm.h"
 
 #define EVMKEY "evm-key"
@@ -29,7 +30,7 @@ static unsigned char evmkey[MAX_KEY_SIZE];
 static int evmkey_len = MAX_KEY_SIZE;
 
 struct crypto_shash *hmac_tfm;
-struct crypto_shash *hash_tfm;
+static struct crypto_shash *evm_tfm[HASH_ALGO__LAST];
 
 static DEFINE_MUTEX(mutex);
 
@@ -38,7 +39,6 @@ static DEFINE_MUTEX(mutex);
 static unsigned long evm_set_key_flags;
 
 static char * const evm_hmac = "hmac(sha1)";
-static char * const evm_hash = "sha1";
 
 /**
  * evm_set_key() - set EVM HMAC key from the kernel
@@ -74,10 +74,10 @@ int evm_set_key(void *key, size_t keylen)
 }
 EXPORT_SYMBOL_GPL(evm_set_key);
 
-static struct shash_desc *init_desc(char type)
+static struct shash_desc *init_desc(char type, uint8_t hash_algo)
 {
long rc;
-   char *algo;
+   const char *algo;
struct crypto_shash **tfm;
struct shash_desc *desc;
 
@@ -89,8 +89,8 @@ static struct shash_desc *init_desc(char type)
tfm = _tfm;
algo = evm_hmac;
} else {
-   tfm = _tfm;
-   algo = evm_hash;
+   tfm = _tfm[hash_algo];
+   algo = hash_algo_name[hash_algo];
}
 
if (*tfm == NULL) {
@@ -187,10 +187,10 @@ static void hmac_add_misc(struct shash_desc *desc, struct 
inode *inode,
  * each xattr, but attempt to re-use the previously allocated memory.
  */
 static int evm_calc_hmac_or_hash(struct dentry *dentry,
-   const char *req_xattr_name,
-   const char *req_xattr_value,
-   size_t req_xattr_value_len,
-   char type, char *digest)
+const char *req_xattr_name,
+const char *req_xattr_value,
+size_t req_xattr_value_len,
+uint8_t type, struct evm_digest *data)
 {
struct inode *inode = d_backing_inode(dentry);
struct xattr_list *xattr;
@@ -204,10 +204,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
if (!(inode->i_opflags & IOP_XATTR))
return -EOPNOTSUPP;
 
-   desc = init_desc(type);
+   desc = init_desc(type, data->hdr.algo);
if (IS_ERR(desc))
return PTR_ERR(desc);
 
+   data->hdr.length = crypto_shash_digestsize(desc->tfm);
+
error = -ENODATA;
list_for_each_entry_rcu(xattr, _config_xattrnames, list) {
bool is_ima = false;
@@ -239,7 +241,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
if (is_ima)
ima_present = true;
}
-   hmac_add_misc(desc, inode, type

[PATCH V3 1/2] evm: Don't deadlock if a crypto algorithm is unavailable

2018-06-08 Thread Matthew Garrett 
When EVM attempts to appraise a file signed with a crypto algorithm the
kernel doesn't have support for, it will cause the kernel to trigger a
module load. If the EVM policy includes appraisal of kernel modules this
will in turn call back into EVM - since EVM is holding a lock until the
crypto initialisation is complete, this triggers a deadlock. Add a
CRYPTO_NOLOAD flag and skip module loading if it's set, and add that flag
in the EVM case in order to fail gracefully with an error message
instead of deadlocking.

Signed-off-by: Matthew Garrett 
---
 crypto/api.c| 2 +-
 include/linux/crypto.h  | 5 +
 security/integrity/evm/evm_crypto.c | 3 ++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/crypto/api.c b/crypto/api.c
index 0ee632bba064..7aca9f86c5f3 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -229,7 +229,7 @@ static struct crypto_alg *crypto_larval_lookup(const char 
*name, u32 type,
mask &= ~(CRYPTO_ALG_LARVAL | CRYPTO_ALG_DEAD);
 
alg = crypto_alg_lookup(name, type, mask);
-   if (!alg) {
+   if (!alg && !(mask & CRYPTO_NOLOAD)) {
request_module("crypto-%s", name);
 
if (!((type ^ CRYPTO_ALG_NEED_FALLBACK) & mask &
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 6eb06101089f..e8839d3a7559 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -112,6 +112,11 @@
  */
 #define CRYPTO_ALG_OPTIONAL_KEY0x4000
 
+/*
+ * Don't trigger module loading
+ */
+#define CRYPTO_NOLOAD  0x8000
+
 /*
  * Transform masks and values (for crt_flags).
  */
diff --git a/security/integrity/evm/evm_crypto.c 
b/security/integrity/evm/evm_crypto.c
index 494da5fcc092..bea847dc0919 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -97,7 +97,8 @@ static struct shash_desc *init_desc(char type)
mutex_lock();
if (*tfm)
goto out;
-   *tfm = crypto_alloc_shash(algo, 0, CRYPTO_ALG_ASYNC);
+   *tfm = crypto_alloc_shash(algo, 0,
+ CRYPTO_ALG_ASYNC | CRYPTO_NOLOAD);
if (IS_ERR(*tfm)) {
rc = PTR_ERR(*tfm);
pr_err("Can not allocate %s (reason: %ld)\n", algo, rc);
-- 
2.18.0.rc1.242.g61856ae69a-goog

Support additional signature types in EVM

2018-06-08 Thread Matthew Garrett 
Same as V2, but rebased on next-integrity

[PATCH V2 2/2] evm: Allow non-SHA1 digital signatures

2018-06-06 Thread Matthew Garrett 
SHA1 is reasonable in HMAC constructs, but it's desirable to be able to
use stronger hashes in digital signatures. Modify the EVM crypto code so
the hash type is imported from the digital signature and passed down to
the hash calculation code, and return the digest size to higher layers
for validation.

Signed-off-by: Matthew Garrett 
---
 security/integrity/evm/evm.h| 10 --
 security/integrity/evm/evm_crypto.c | 47 +++--
 security/integrity/evm/evm_main.c   | 19 +++-
 3 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 45c4a89c02ff..a8289fbf2f0c 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -42,6 +42,11 @@ extern struct crypto_shash *hash_tfm;
 /* List of EVM protected security xattrs */
 extern char *evm_config_xattrnames[];
 
+struct evm_digest {
+   struct ima_digest_data hdr;
+   char digest[IMA_MAX_DIGEST_SIZE];
+} __packed;
+
 int evm_init_key(void);
 int evm_update_evmxattr(struct dentry *dentry,
const char *req_xattr_name,
@@ -49,10 +54,11 @@ int evm_update_evmxattr(struct dentry *dentry,
size_t req_xattr_value_len);
 int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
  const char *req_xattr_value,
- size_t req_xattr_value_len, char *digest);
+ size_t req_xattr_value_len, struct evm_digest *data);
 int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
  const char *req_xattr_value,
- size_t req_xattr_value_len, char type, char *digest);
+ size_t req_xattr_value_len, char type,
+ struct evm_digest *data);
 int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
  char *hmac_val);
 int evm_init_secfs(void);
diff --git a/security/integrity/evm/evm_crypto.c 
b/security/integrity/evm/evm_crypto.c
index ccafc9468611..ff8687232a1a 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "evm.h"
 
 #define EVMKEY "evm-key"
@@ -29,7 +30,7 @@ static unsigned char evmkey[MAX_KEY_SIZE];
 static int evmkey_len = MAX_KEY_SIZE;
 
 struct crypto_shash *hmac_tfm;
-struct crypto_shash *hash_tfm;
+static struct crypto_shash *evm_tfm[HASH_ALGO__LAST];
 
 static DEFINE_MUTEX(mutex);
 
@@ -38,7 +39,6 @@ static DEFINE_MUTEX(mutex);
 static unsigned long evm_set_key_flags;
 
 static char * const evm_hmac = "hmac(sha1)";
-static char * const evm_hash = "sha1";
 
 /**
  * evm_set_key() - set EVM HMAC key from the kernel
@@ -74,10 +74,10 @@ int evm_set_key(void *key, size_t keylen)
 }
 EXPORT_SYMBOL_GPL(evm_set_key);
 
-static struct shash_desc *init_desc(char type)
+static struct shash_desc *init_desc(char type, uint8_t hash_algo)
 {
long rc;
-   char *algo;
+   const char *algo;
struct crypto_shash **tfm;
struct shash_desc *desc;
 
@@ -89,8 +89,8 @@ static struct shash_desc *init_desc(char type)
tfm = _tfm;
algo = evm_hmac;
} else {
-   tfm = _tfm;
-   algo = evm_hash;
+   tfm = _tfm[hash_algo];
+   algo = hash_algo_name[hash_algo];
}
 
if (*tfm == NULL) {
@@ -187,10 +187,10 @@ static void hmac_add_misc(struct shash_desc *desc, struct 
inode *inode,
  * each xattr, but attempt to re-use the previously allocated memory.
  */
 static int evm_calc_hmac_or_hash(struct dentry *dentry,
-   const char *req_xattr_name,
-   const char *req_xattr_value,
-   size_t req_xattr_value_len,
-   char type, char *digest)
+const char *req_xattr_name,
+const char *req_xattr_value,
+size_t req_xattr_value_len,
+uint8_t type, struct evm_digest *data)
 {
struct inode *inode = d_backing_inode(dentry);
struct shash_desc *desc;
@@ -204,10 +204,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
if (!(inode->i_opflags & IOP_XATTR))
return -EOPNOTSUPP;
 
-   desc = init_desc(type);
+   desc = init_desc(type, data->hdr.algo);
if (IS_ERR(desc))
return PTR_ERR(desc);
 
+   data->hdr.length = crypto_shash_digestsize(desc->tfm);
+
error = -ENODATA;
for (xattrname = evm_config_xattrnames; *xattrname != NULL; 
xattrname++) {
bool is_ima = false;
@@ -239,7 +241,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
if (is_ima)
ima_present = true;
}
-   hmac_add_misc(desc, inode

[PATCH V2 1/2] evm: Don't deadlock if a crypto algorithm is unavailable

2018-06-06 Thread Matthew Garrett 
When EVM attempts to appraise a file signed with a crypto algorithm the
kernel doesn't have support for, it will cause the kernel to trigger a
module load. If the EVM policy includes appraisal of kernel modules this
will in turn call back into EVM - since EVM is holding a lock until the
crypto initialisation is complete, this triggers a deadlock. Add a
CRYPTO_NOLOAD flag and skip module loading if it's set, and add that flag
in the EVM case in order to fail gracefully with an error message
instead of deadlocking.

Signed-off-by: Matthew Garrett 
---
 crypto/api.c| 2 +-
 include/linux/crypto.h  | 5 +
 security/integrity/evm/evm_crypto.c | 3 ++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/crypto/api.c b/crypto/api.c
index 0ee632bba064..7aca9f86c5f3 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -229,7 +229,7 @@ static struct crypto_alg *crypto_larval_lookup(const char 
*name, u32 type,
mask &= ~(CRYPTO_ALG_LARVAL | CRYPTO_ALG_DEAD);
 
alg = crypto_alg_lookup(name, type, mask);
-   if (!alg) {
+   if (!alg && !(mask & CRYPTO_NOLOAD)) {
request_module("crypto-%s", name);
 
if (!((type ^ CRYPTO_ALG_NEED_FALLBACK) & mask &
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 6eb06101089f..e8839d3a7559 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -112,6 +112,11 @@
  */
 #define CRYPTO_ALG_OPTIONAL_KEY0x4000
 
+/*
+ * Don't trigger module loading
+ */
+#define CRYPTO_NOLOAD  0x8000
+
 /*
  * Transform masks and values (for crt_flags).
  */
diff --git a/security/integrity/evm/evm_crypto.c 
b/security/integrity/evm/evm_crypto.c
index a46fba322340..ccafc9468611 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -97,7 +97,8 @@ static struct shash_desc *init_desc(char type)
mutex_lock();
if (*tfm)
goto out;
-   *tfm = crypto_alloc_shash(algo, 0, CRYPTO_ALG_ASYNC);
+   *tfm = crypto_alloc_shash(algo, 0,
+ CRYPTO_ALG_ASYNC | CRYPTO_NOLOAD);
if (IS_ERR(*tfm)) {
rc = PTR_ERR(*tfm);
pr_err("Can not allocate %s (reason: %ld)\n", algo, rc);
-- 
2.17.1.1185.g55be947832-goog

Re: [PATCH 1/2] evm: Don't deadlock if a crypto algorithm is unavailable

2018-06-04 Thread Matthew Garrett 
On Sat, Jun 2, 2018 at 8:54 AM Herbert Xu  wrote:
>
> On Fri, Jun 01, 2018 at 04:02:43PM -0700, Matthew Garrett wrote:
> > Trying to instantiate a non-existent crypto algorithm will cause the
> > kernel to trigger a module load. If EVM appraisal is enabled, this will
> > in turn trigger appraisal of the module, which will fail because the
> > crypto algorithm isn't available. Add a CRYPTO_NOLOAD flag and skip
> > module loading if it's set, and add that flag in the EVM case.
> >
> > Signed-off-by: Matthew Garrett 
>
> I don't get it.  Without your patch it will fail because the
> EVM appraisal fails.  With your patch it will fail because there
> is no algorithm registered.  So what's the difference?

Without my patch it will deadlock as it recursively calls back into
EVM to perform the module appraisal. Sorry, the description was
unclear on that point.

[PATCH 2/2] evm: Allow non-SHA1 digital signatures

2018-06-01 Thread Matthew Garrett 
SHA1 is reasonable in HMAC constructs, but it's desirable to be able to
use stronger hashes in digital signatures. Modify the EVM crypto code so
the hash type is imported from the digital signature and passed down to
the hash calculation code, and return the digest size to higher layers
for validation.

Signed-off-by: Matthew Garrett 
---
 security/integrity/evm/evm.h| 10 --
 security/integrity/evm/evm_crypto.c | 47 +++--
 security/integrity/evm/evm_main.c   | 19 +++-
 3 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 45c4a89c02ff..a8289fbf2f0c 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -42,6 +42,11 @@ extern struct crypto_shash *hash_tfm;
 /* List of EVM protected security xattrs */
 extern char *evm_config_xattrnames[];
 
+struct evm_digest {
+   struct ima_digest_data hdr;
+   char digest[IMA_MAX_DIGEST_SIZE];
+} __packed;
+
 int evm_init_key(void);
 int evm_update_evmxattr(struct dentry *dentry,
const char *req_xattr_name,
@@ -49,10 +54,11 @@ int evm_update_evmxattr(struct dentry *dentry,
size_t req_xattr_value_len);
 int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
  const char *req_xattr_value,
- size_t req_xattr_value_len, char *digest);
+ size_t req_xattr_value_len, struct evm_digest *data);
 int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
  const char *req_xattr_value,
- size_t req_xattr_value_len, char type, char *digest);
+ size_t req_xattr_value_len, char type,
+ struct evm_digest *data);
 int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
  char *hmac_val);
 int evm_init_secfs(void);
diff --git a/security/integrity/evm/evm_crypto.c 
b/security/integrity/evm/evm_crypto.c
index ccafc9468611..ff8687232a1a 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "evm.h"
 
 #define EVMKEY "evm-key"
@@ -29,7 +30,7 @@ static unsigned char evmkey[MAX_KEY_SIZE];
 static int evmkey_len = MAX_KEY_SIZE;
 
 struct crypto_shash *hmac_tfm;
-struct crypto_shash *hash_tfm;
+static struct crypto_shash *evm_tfm[HASH_ALGO__LAST];
 
 static DEFINE_MUTEX(mutex);
 
@@ -38,7 +39,6 @@ static DEFINE_MUTEX(mutex);
 static unsigned long evm_set_key_flags;
 
 static char * const evm_hmac = "hmac(sha1)";
-static char * const evm_hash = "sha1";
 
 /**
  * evm_set_key() - set EVM HMAC key from the kernel
@@ -74,10 +74,10 @@ int evm_set_key(void *key, size_t keylen)
 }
 EXPORT_SYMBOL_GPL(evm_set_key);
 
-static struct shash_desc *init_desc(char type)
+static struct shash_desc *init_desc(char type, uint8_t hash_algo)
 {
long rc;
-   char *algo;
+   const char *algo;
struct crypto_shash **tfm;
struct shash_desc *desc;
 
@@ -89,8 +89,8 @@ static struct shash_desc *init_desc(char type)
tfm = _tfm;
algo = evm_hmac;
} else {
-   tfm = _tfm;
-   algo = evm_hash;
+   tfm = _tfm[hash_algo];
+   algo = hash_algo_name[hash_algo];
}
 
if (*tfm == NULL) {
@@ -187,10 +187,10 @@ static void hmac_add_misc(struct shash_desc *desc, struct 
inode *inode,
  * each xattr, but attempt to re-use the previously allocated memory.
  */
 static int evm_calc_hmac_or_hash(struct dentry *dentry,
-   const char *req_xattr_name,
-   const char *req_xattr_value,
-   size_t req_xattr_value_len,
-   char type, char *digest)
+const char *req_xattr_name,
+const char *req_xattr_value,
+size_t req_xattr_value_len,
+uint8_t type, struct evm_digest *data)
 {
struct inode *inode = d_backing_inode(dentry);
struct shash_desc *desc;
@@ -204,10 +204,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
if (!(inode->i_opflags & IOP_XATTR))
return -EOPNOTSUPP;
 
-   desc = init_desc(type);
+   desc = init_desc(type, data->hdr.algo);
if (IS_ERR(desc))
return PTR_ERR(desc);
 
+   data->hdr.length = crypto_shash_digestsize(desc->tfm);
+
error = -ENODATA;
for (xattrname = evm_config_xattrnames; *xattrname != NULL; 
xattrname++) {
bool is_ima = false;
@@ -239,7 +241,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
if (is_ima)
ima_present = true;
}
-   hmac_add_misc(desc, inode

[PATCH 1/2] evm: Don't deadlock if a crypto algorithm is unavailable

2018-06-01 Thread Matthew Garrett 
Trying to instantiate a non-existent crypto algorithm will cause the
kernel to trigger a module load. If EVM appraisal is enabled, this will
in turn trigger appraisal of the module, which will fail because the
crypto algorithm isn't available. Add a CRYPTO_NOLOAD flag and skip
module loading if it's set, and add that flag in the EVM case.

Signed-off-by: Matthew Garrett 
---
 crypto/api.c| 2 +-
 include/linux/crypto.h  | 5 +
 security/integrity/evm/evm_crypto.c | 3 ++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/crypto/api.c b/crypto/api.c
index 0ee632bba064..7aca9f86c5f3 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -229,7 +229,7 @@ static struct crypto_alg *crypto_larval_lookup(const char 
*name, u32 type,
mask &= ~(CRYPTO_ALG_LARVAL | CRYPTO_ALG_DEAD);
 
alg = crypto_alg_lookup(name, type, mask);
-   if (!alg) {
+   if (!alg && !(mask & CRYPTO_NOLOAD)) {
request_module("crypto-%s", name);
 
if (!((type ^ CRYPTO_ALG_NEED_FALLBACK) & mask &
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 6eb06101089f..e8839d3a7559 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -112,6 +112,11 @@
  */
 #define CRYPTO_ALG_OPTIONAL_KEY0x4000
 
+/*
+ * Don't trigger module loading
+ */
+#define CRYPTO_NOLOAD  0x8000
+
 /*
  * Transform masks and values (for crt_flags).
  */
diff --git a/security/integrity/evm/evm_crypto.c 
b/security/integrity/evm/evm_crypto.c
index a46fba322340..ccafc9468611 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -97,7 +97,8 @@ static struct shash_desc *init_desc(char type)
mutex_lock();
if (*tfm)
goto out;
-   *tfm = crypto_alloc_shash(algo, 0, CRYPTO_ALG_ASYNC);
+   *tfm = crypto_alloc_shash(algo, 0,
+ CRYPTO_ALG_ASYNC | CRYPTO_NOLOAD);
if (IS_ERR(*tfm)) {
rc = PTR_ERR(*tfm);
pr_err("Can not allocate %s (reason: %ld)\n", algo, rc);
-- 
2.17.1.1185.g55be947832-goog

Re: [RFC PATCH 00/18 v3] Signature verification of hibernate snapshot

2013-09-01 Thread Matthew Garrett 
On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:

 But if you don't generate fresh keys on every boot, the persistent
 keys are mor exposed to other UEFI applications.  Correct me if I'm
 wrong, but I don't think UEFI variables are segregated between
 different UEFI applications, so if anyone gets a generic UEFI variable
 dumper (or setter) signed by the trusted key, this cryptographic
 validation of hibernate snapshots is bypassable.

If anyone can execute arbitrary code in your UEFI environment then 
you've already lost.

-- 
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line unsubscribe linux-crypto 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/18 v3] Signature verification of hibernate snapshot

2013-09-01 Thread Matthew Garrett 
On Sun, Sep 01, 2013 at 06:40:41PM +0200, Florian Weimer wrote:
 * Matthew Garrett:
 
  On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
 
  But if you don't generate fresh keys on every boot, the persistent
  keys are mor exposed to other UEFI applications.  Correct me if I'm
  wrong, but I don't think UEFI variables are segregated between
  different UEFI applications, so if anyone gets a generic UEFI variable
  dumper (or setter) signed by the trusted key, this cryptographic
  validation of hibernate snapshots is bypassable.
 
  If anyone can execute arbitrary code in your UEFI environment then 
  you've already lost.
 
 This is not about arbitrary code execution.  The problematic
 applications which conflict with this proposed functionality are not
 necessarily malicious by themselves and even potentially useful.
 
A signed application that permits the modification of arbitrary boot 
services variables *is* malicious. No implementation is designed to be 
safe in that scenario. Why bother with modifying encryption keys when 
you can just modify MOK instead?

-- 
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 10/18] efi: Enable secure boot lockdown automatically when enabled in firmware

2013-08-25 Thread Matthew Garrett 
On Sun, Aug 25, 2013 at 06:22:43PM +0200, Pavel Machek wrote:
 On Thu 2013-08-22 19:01:49, Lee, Chun-Yi wrote:
  From: Matthew Garrett m...@redhat.com
  
  The firmware has a set of flags that indicate whether secure boot is enabled
  and enforcing. Use them to indicate whether the kernel should lock itself
  down.  We also indicate the machine is in secure boot mode by adding the
  EFI_SECURE_BOOT bit for use with efi_enabled.
 
  +   status = efi_call_phys5(sys_table-runtime-get_variable,
  +   LSecureBoot, var_guid, NULL, datasize, sb);
 
 What is this L... thing?

http://en.wikipedia.org/wiki/C_syntax#Wide_character_strings
-- 
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/1] ima: digital signature verification using asymmetric keys

2013-01-29 Thread Matthew Garrett 
On Tue, Jan 29, 2013 at 11:58:53AM -0500, Vivek Goyal wrote:
 On Mon, Jan 28, 2013 at 08:48:55PM -0500, Mimi Zohar wrote:
  The assumption has always been that the initramfs would be measured, for
  trusted boot, and appraised, for secure boot, before being executed.
 
 Hi Mimi,
 
 Ok. So for trusted boot, if initramfs is changed it will be detected. For
 secureboot, atleast right now initramfs is not signed and appraised. But
 I guess it could be added. 
 
 But initramfs is generated by installer and installer does not have
 private keys to sign it. So distributions could not sign initramfs.

Right, there's a whole range of problems here. The first is that the 
initramfs has to contain the full set of drivers required to boot a 
given piece of hardware, and the precise set required varies between 
machines. Building a truly generic initramfs would result in 
significantly larger images.

There's also an existing expectation that it be possible to break into 
initramfs execution for debugging purposes. Even ignoring that, most 
initramfs implementations aren't expected to be hardened against a user 
inserting shell control characters into the kernel command line. It 
would require significant engineering work to ensure that there's no way 
for an attacker to cause code execution before the key store has been 
locked.

Shipping prebuilt initramfses is also difficult from a release 
engineering perspective. You'd need to keep track of the software 
versions that were included in the initramfs and ensure that the 
initramfs is rebuilt if any of those pieces of software are updated 
between the initramfs being generated and the software being shipped. 
Version skew could cause subtle bugs and also makes license compliance 
difficult.

Finally, portions of the userspace in initramfs may be under licenses 
that require it to be possible for the end user to replace components. 
This isn't a problem as long as the keys in MOK can be used.

There's additional small problems, like the initramfs containing the 
bootsplash theme and users expecting to be able to change that without 
having to generate crypto keys, but that's probably not a showstopper. 
But realistically, the first three problems make it unlikely that most 
distributions will be willing to depend on or ship pre-built initramfs 
images.

-- 
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html