Re: [PATCH v5 0/3] RFC: Secure Memory Allocation Framework

[James Morris]

On Wed, 21 Oct 2015, Benjamin Gaignard wrote:

> 
> The outcome of the previous RFC about how do secure data path was the need
> of a secure memory allocator (https://lkml.org/lkml/2015/5/5/551)
> 

Have you addressed all the questions raised by Alan here:

https://lkml.org/lkml/2015/5/8/629

Also, is there any application of this beyond DRM?


- James
-- 
James Morris


--
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 10/10] KEYS: Move the point of trust determination to __key_link()

[David Howells]

Move the point at which a key is determined to be trustworthy to
__key_link() so that we use the contents of the keyring being linked in to
to determine whether the key being linked in is trusted or not.

What is 'trusted' then becomes a matter of what's in the keyring.

Currently, the test is done when the key is parsed, but given that at that
point we can only sensibly refer to the contents of the system trusted
keyring, we can only use that as the basis for working out the
trustworthiness of a new key.

With this change, a trusted keyring is a set of keys that once the
trusted-only flag is set cannot be added to except by verification through
one of the contained keys.

Further, adding a key into a trusted keyring, whilst it might grant
trustworthiness in the context of that keyring, does not automatically
grant trustworthiness in the context of a second keyring to which it could
be secondarily linked.

To accomplish this, the authentication data associated with the key source
must now be retained.  For an X.509 cert, this means the contents of the
AuthorityKeyIdentifier and the signature data.

Signed-off-by: David Howells 
---

 certs/system_keyring.c|3 +
 crypto/asymmetric_keys/Makefile   |2 -
 crypto/asymmetric_keys/asymmetric_keys.h  |2 +
 crypto/asymmetric_keys/asymmetric_type.c  |   15 +
 crypto/asymmetric_keys/pkcs7_trust.c  |   22 +++
 crypto/asymmetric_keys/public_key.c   |   19 ++
 crypto/asymmetric_keys/public_key.h   |6 ++
 crypto/asymmetric_keys/public_key_trust.c |   94 +
 crypto/asymmetric_keys/x509_parser.h  |6 --
 crypto/asymmetric_keys/x509_public_key.c  |6 --
 include/crypto/public_key.h   |8 +-
 include/keys/asymmetric-subtype.h |4 +
 security/integrity/digsig_asymmetric.c|5 +-
 13 files changed, 108 insertions(+), 84 deletions(-)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index e7f286413276..fbaaaea59f02 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -35,7 +35,8 @@ static __init int system_trusted_keyring_init(void)
keyring_alloc(".system_keyring",
  KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
  ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
- KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH),
+  KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH |
+  KEY_USR_WRITE),
  KEY_ALLOC_NOT_IN_QUOTA, NULL);
if (IS_ERR(system_trusted_keyring))
panic("Can't allocate system trusted keyring\n");
diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
index bd07987c64e7..69bcdc9a2ce6 100644
--- a/crypto/asymmetric_keys/Makefile
+++ b/crypto/asymmetric_keys/Makefile
@@ -6,7 +6,7 @@ obj-$(CONFIG_ASYMMETRIC_KEY_TYPE) += asymmetric_keys.o
 
 asymmetric_keys-y := asymmetric_type.o signature.o
 
-obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o
+obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o public_key_trust.o
 obj-$(CONFIG_PUBLIC_KEY_ALGO_RSA) += rsa.o
 
 #
diff --git a/crypto/asymmetric_keys/asymmetric_keys.h 
b/crypto/asymmetric_keys/asymmetric_keys.h
index 1d450b580245..ca8e9ac34ce6 100644
--- a/crypto/asymmetric_keys/asymmetric_keys.h
+++ b/crypto/asymmetric_keys/asymmetric_keys.h
@@ -9,6 +9,8 @@
  * 2 of the Licence, or (at your option) any later version.
  */
 
+#include 
+
 extern struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id);
 
 extern int __asymmetric_key_hex_to_key_id(const char *id,
diff --git a/crypto/asymmetric_keys/asymmetric_type.c 
b/crypto/asymmetric_keys/asymmetric_type.c
index a79d30128821..e02cbd068151 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -362,10 +362,25 @@ static void asymmetric_key_destroy(struct key *key)
asymmetric_key_free_kids(kids);
 }
 
+/*
+ * Verify the trust on an asymmetric key when added to a trusted-only keyring.
+ * The keyring provides a list of keys to check against.
+ */
+static int asymmetric_key_verify_trust(const union key_payload *payload,
+  struct key *keyring)
+{
+   struct asymmetric_key_subtype *subtype = payload->data[asym_subtype];
+
+   pr_devel("==>%s()\n", __func__);
+
+   return subtype->verify_trust(payload, keyring);
+}
+
 struct key_type key_type_asymmetric = {
.name   = "asymmetric",
.preparse   = asymmetric_key_preparse,
.free_preparse  = asymmetric_key_free_preparse,
+   .verify_trust   = asymmetric_key_verify_trust,
.instantiate= generic_key_instantiate,
.match_preparse = asymmetric_key_match_preparse,
.match_free = asymmetric_key_match_free,
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c 

[PATCH 6/6] KEYS: Merge the type-specific data with the payload data

[David Howells]

Merge the type-specific data with the payload data into one four-word chunk
as it seems pointless to keep them separate.

Use user_key_payload() for accessing the payloads of overloaded
user-defined keys.

Signed-off-by: David Howells 
cc: linux-c...@vger.kernel.org
cc: ecryp...@vger.kernel.org
cc: linux-e...@vger.kernel.org
cc: linux-f2fs-de...@lists.sourceforge.net
cc: linux-...@vger.kernel.org
cc: ceph-de...@vger.kernel.org
cc: linux-ima-de...@lists.sourceforge.net
---

 Documentation/crypto/asymmetric-keys.txt |   27 +++--
 Documentation/security/keys.txt  |   41 ---
 crypto/asymmetric_keys/asymmetric_keys.h |5 --
 crypto/asymmetric_keys/asymmetric_type.c |   44 -
 crypto/asymmetric_keys/public_key.c  |4 +-
 crypto/asymmetric_keys/signature.c   |2 -
 crypto/asymmetric_keys/x509_parser.h |1 
 crypto/asymmetric_keys/x509_public_key.c |9 ++--
 fs/cifs/cifs_spnego.c|6 +--
 fs/cifs/cifsacl.c|   25 ++--
 fs/cifs/connect.c|9 ++--
 fs/cifs/sess.c   |2 -
 fs/cifs/smb2pdu.c|2 -
 fs/ecryptfs/ecryptfs_kernel.h|5 +-
 fs/ext4/crypto_key.c |4 +-
 fs/f2fs/crypto_key.c |4 +-
 fs/fscache/object-list.c |4 +-
 fs/nfs/nfs4idmap.c   |4 +-
 include/crypto/public_key.h  |1 
 include/keys/asymmetric-subtype.h|2 -
 include/keys/asymmetric-type.h   |   15 +++
 include/keys/user-type.h |8 
 include/linux/key-type.h |3 -
 include/linux/key.h  |   33 +++
 kernel/module_signing.c  |1 
 lib/digsig.c |7 ++-
 net/ceph/ceph_common.c   |2 -
 net/ceph/crypto.c|6 +--
 net/dns_resolver/dns_key.c   |   20 +
 net/dns_resolver/dns_query.c |7 +--
 net/dns_resolver/internal.h  |8 
 net/rxrpc/af_rxrpc.c |2 -
 net/rxrpc/ar-key.c   |   32 +++
 net/rxrpc/ar-output.c|2 -
 net/rxrpc/ar-security.c  |4 +-
 net/rxrpc/rxkad.c|   16 ---
 security/integrity/evm/evm_crypto.c  |2 -
 security/keys/big_key.c  |   47 +++---
 security/keys/encrypted-keys/encrypted.c |   18 
 security/keys/encrypted-keys/encrypted.h |4 +-
 security/keys/encrypted-keys/masterkey_trusted.c |4 +-
 security/keys/key.c  |   18 
 security/keys/keyctl.c   |4 +-
 security/keys/keyring.c  |   12 +++---
 security/keys/process_keys.c |4 +-
 security/keys/request_key.c  |4 +-
 security/keys/request_key_auth.c |   12 +++---
 security/keys/trusted.c  |6 +--
 security/keys/user_defined.c |   14 +++
 49 files changed, 286 insertions(+), 230 deletions(-)

diff --git a/Documentation/crypto/asymmetric-keys.txt 
b/Documentation/crypto/asymmetric-keys.txt
index b7675904a747..8c07e0ea6bc0 100644
--- a/Documentation/crypto/asymmetric-keys.txt
+++ b/Documentation/crypto/asymmetric-keys.txt
@@ -186,7 +186,7 @@ and looks like the following:
const struct public_key_signature *sig);
};
 
-Asymmetric keys point to this with their type_data[0] member.
+Asymmetric keys point to this with their payload[asym_subtype] member.
 
 The owner and name fields should be set to the owning module and the name of
 the subtype.  Currently, the name is only used for print statements.
@@ -269,8 +269,7 @@ mandatory:
 
struct key_preparsed_payload {
char*description;
-   void*type_data[2];
-   void*payload;
+   void*payload[4];
const void  *data;
size_t  datalen;
size_t  quotalen;
@@ -283,16 +282,18 @@ mandatory:
  not theirs.
 
  If the parser is happy with the blob, it should propose a description for
- the key and attach it to ->description, ->type_data[0] should be set to
- point to the subtype to be used, ->payload should be set to point to the
- initialised data for that 

Re: [PATCH 1/6] KEYS: use kvfree() in add_key

[David Howells]

These patches can be found here also:


http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-next

And tagged with:

keys-next-20151021

David
--
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 00/10] KEYS: Change how keys are determined to be trusted

[David Howells]

Here's a set of patches that changes how keys are determined to be trusted
- currently, that's a case of whether a key has KEY_FLAG_TRUSTED set upon
it.  A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
indicates that only keys with this flag set may be added to that keyring.

Further, any time an X.509 certificate is instantiated without this flag
set, the certificate is judged against the contents of the system trusted
keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.

With these patches, KEY_FLAG_TRUSTED is removed.  The kernel may add
implicitly trusted keys to a trusted-only keyring by asserting
KEY_ALLOC_TRUSTED when the key is created, but otherwise the key will only
be allowed to be added to the keyring if it can be verified by a key
already in that keyring.  The system trusted keyring is not then special in
this sense and other trusted keyrings can be set up that are wholly
independent of it.

To make this work, we have to retain sufficient data from the X.509
certificate that we can then verify the signature at need.

The patches can be found here also:


http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-trust

and are tagged with:

keys-trust-20151021

David
---
David Howells (10):
  KEYS: Generalise system_verify_data() to provide access to internal 
content
  PKCS#7: Make trust determination dependent on contents of trust keyring
  KEYS: Add facility to check key trustworthiness upon link creation
  KEYS: Allow authentication data to be stored in an asymmetric key
  KEYS: Add identifier pointers to public_key_signature struct
  X.509: Retain the key verification data
  X.509: Extract signature digest and make self-signed cert checks earlier
  PKCS#7: Make the signature a pointer rather than embedding it
  X.509: Move the trust validation code out to its own file
  KEYS: Move the point of trust determination to __key_link()


 Documentation/security/keys.txt   |   17 ++
 arch/x86/kernel/kexec-bzimage64.c |   18 --
 certs/system_keyring.c|   49 +++--
 crypto/asymmetric_keys/Kconfig|1 
 crypto/asymmetric_keys/Makefile   |4 
 crypto/asymmetric_keys/asymmetric_keys.h  |2 
 crypto/asymmetric_keys/asymmetric_type.c  |   22 ++
 crypto/asymmetric_keys/mscode_parser.c|   21 +-
 crypto/asymmetric_keys/pkcs7_key_type.c   |   64 +++---
 crypto/asymmetric_keys/pkcs7_parser.c |   59 +++--
 crypto/asymmetric_keys/pkcs7_parser.h |   11 -
 crypto/asymmetric_keys/pkcs7_trust.c  |   44 ++--
 crypto/asymmetric_keys/pkcs7_verify.c |  108 --
 crypto/asymmetric_keys/public_key.c   |   43 
 crypto/asymmetric_keys/public_key.h   |6 +
 crypto/asymmetric_keys/public_key_trust.c |  180 +
 crypto/asymmetric_keys/verify_pefile.c|   40 +---
 crypto/asymmetric_keys/verify_pefile.h|5 
 crypto/asymmetric_keys/x509_cert_parser.c |   53 +++--
 crypto/asymmetric_keys/x509_parser.h  |   12 -
 crypto/asymmetric_keys/x509_public_key.c  |  312 +
 include/crypto/pkcs7.h|6 -
 include/crypto/public_key.h   |   28 +--
 include/keys/asymmetric-subtype.h |6 -
 include/keys/asymmetric-type.h|8 -
 include/keys/system_keyring.h |7 -
 include/linux/key-type.h  |   10 +
 include/linux/key.h   |   12 +
 include/linux/verification.h  |   49 +
 include/linux/verify_pefile.h |   22 --
 kernel/module_signing.c   |5 
 security/integrity/digsig_asymmetric.c|5 
 security/keys/key.c   |   44 +++-
 security/keys/keyring.c   |   18 +-
 34 files changed, 735 insertions(+), 556 deletions(-)
 create mode 100644 crypto/asymmetric_keys/public_key_trust.c
 create mode 100644 include/linux/verification.h
 delete mode 100644 include/linux/verify_pefile.h

--
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 03/10] KEYS: Add facility to check key trustworthiness upon link creation

[David Howells]

Add a facility whereby if KEY_FLAG_TRUSTED_ONLY is set on the destination
keyring, the creation of a link to a candidate key will cause the
trustworthiness of that key to be evaluated against the already present
contents of that keyring.  This affects operations like add_key(),
KEYCTL_LINK and KEYCTL_INSTANTIATE.

To this end:

 (1) A new key type method is provided:

int (*verify_trust)(const union key_payload *payload,
struct key *keyring);

 This is implemented by key types for which verification of one key by
 another is appropriate.  It is primarily intended for use with the
 asymmetric key type.

 When called, it is given the payload or prospective payload[*] of the
 candidate key to verify and a pointer to the destination keyring.  The
 method is expected to search the keying for an appropriate key with
 which to verify the candidate.

 [*] If called during add_key(), preparse is called before this method,
 but a key isn't actually allocated unless the verification is
 successful.

 (2) KEY_FLAG_TRUSTED is removed.  A key is now trusted by virtue of being
 contained in the trusted-only keyring being searched.

 (3) KEY_ALLOC_TRUSTED now acts as an override.  If this is passed to
 key_create_or_update() then the ->verify_trust() method will be
 ignored and the key will be added anyway.

Signed-off-by: David Howells 
---

 Documentation/security/keys.txt  |   17 
 crypto/asymmetric_keys/x509_public_key.c |6 ++--
 include/linux/key-type.h |   10 ++-
 include/linux/key.h  |   12 +---
 security/keys/key.c  |   44 --
 security/keys/keyring.c  |   18 +++-
 6 files changed, 87 insertions(+), 20 deletions(-)

diff --git a/Documentation/security/keys.txt b/Documentation/security/keys.txt
index 8c183873b2b7..e7f3447ccd1b 100644
--- a/Documentation/security/keys.txt
+++ b/Documentation/security/keys.txt
@@ -1183,6 +1183,23 @@ The structure has a number of fields, some of which are 
mandatory:
  successfully, even if instantiate() or update() succeed.
 
 
+ (*) int (*verify_trust)(const union key_payload *payload, struct key 
*keyring);
+
+ If the keyring to which a candidate key is being added/linked is marked as
+ KEY_FLAG_TRUSTED_ONLY then this function will get called in the candidate
+ key type to verify the key or proposed key based on its payload.  It is
+ expected to use the contents of the supplied destination keyring to
+ determine whether the candidate key is to be trusted and added to the
+ keyring.
+
+ The method should return 0 to allow the addition and an error otherwise,
+ typically ENOKEY if there's no key in the keyring to verify this key and
+ EKEYREJECTED if the selected key fails to verify the candidate.
+
+ This method is optional.  If it is not supplied, keys of this type cannot
+ be added to trusted-only keyrings and EPERM will be returned.
+
+
  (*) int (*instantiate)(struct key *key, struct key_preparsed_payload *prep);
 
  This method is called to attach a payload to a key during construction.
diff --git a/crypto/asymmetric_keys/x509_public_key.c 
b/crypto/asymmetric_keys/x509_public_key.c
index 64d42981a8d7..76c211b31da7 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -318,10 +318,10 @@ static int x509_key_preparse(struct key_preparsed_payload 
*prep)
ret = x509_check_signature(cert->pub, cert); /* self-signed */
if (ret < 0)
goto error_free_cert;
-   } else if (!prep->trusted) {
+   } else {
ret = x509_validate_trust(cert, get_system_trusted_keyring());
-   if (!ret)
-   prep->trusted = 1;
+   if (ret == -EKEYREJECTED)
+   goto error_free_cert;
}
 
/* Propose a description */
diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index 7463355a198b..5d7cf5e7f8c6 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -45,7 +45,6 @@ struct key_preparsed_payload {
size_t  datalen;/* Raw datalen */
size_t  quotalen;   /* Quota length for proposed payload */
time_t  expiry; /* Expiry time of key */
-   booltrusted;/* True if key is trusted */
 };
 
 typedef int (*request_key_actor_t)(struct key_construction *key,
@@ -95,6 +94,15 @@ struct key_type {
 */
void (*free_preparse)(struct key_preparsed_payload *prep);
 
+   /* Verify the trust on a key when added to a trusted-only keyring.
+*
+* If this method isn't provided then it is assumed that the concept of
+* trust is irrelevant to keys of this type and an 

[PATCH 02/10] PKCS#7: Make trust determination dependent on contents of trust keyring

[David Howells]

Make the determination of the trustworthiness of a key dependent on whether
a key that can verify it is present in the ring of trusted keys rather than
whether or not the verifying key has KEY_FLAG_TRUSTED set.

Signed-off-by: David Howells 
---

 certs/system_keyring.c  |   13 -
 crypto/asymmetric_keys/pkcs7_key_type.c |2 +-
 crypto/asymmetric_keys/pkcs7_parser.h   |1 -
 crypto/asymmetric_keys/pkcs7_trust.c|   16 +++-
 crypto/asymmetric_keys/verify_pefile.c  |2 +-
 crypto/asymmetric_keys/x509_parser.h|1 -
 include/crypto/pkcs7.h  |3 +--
 include/linux/verification.h|1 -
 kernel/module_signing.c |2 +-
 9 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index cf55bd3a072a..e7f286413276 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -121,7 +121,6 @@ late_initcall(load_system_certificate_list);
 int verify_pkcs7_signature(const void *data, size_t len,
   const void *raw_pkcs7, size_t pkcs7_len,
   struct key *trusted_keys,
-  int untrusted_error,
   enum key_being_used_for usage,
   int (*view_content)(void *ctx,
   const void *data, size_t len,
@@ -129,7 +128,6 @@ int verify_pkcs7_signature(const void *data, size_t len,
   void *ctx)
 {
struct pkcs7_message *pkcs7;
-   bool trusted;
int ret;
 
pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
@@ -149,13 +147,10 @@ int verify_pkcs7_signature(const void *data, size_t len,
 
if (!trusted_keys)
trusted_keys = system_trusted_keyring;
-   ret = pkcs7_validate_trust(pkcs7, trusted_keys, );
-   if (ret < 0)
-   goto error;
-
-   if (!trusted && untrusted_error) {
-   pr_err("PKCS#7 signature not signed with a trusted key\n");
-   ret = untrusted_error;
+   ret = pkcs7_validate_trust(pkcs7, trusted_keys);
+   if (ret < 0) {
+   if (ret == -ENOKEY)
+   pr_err("PKCS#7 signature not signed with a trusted 
key\n");
goto error;
}
 
diff --git a/crypto/asymmetric_keys/pkcs7_key_type.c 
b/crypto/asymmetric_keys/pkcs7_key_type.c
index 240a5303ebb7..89b75477868d 100644
--- a/crypto/asymmetric_keys/pkcs7_key_type.c
+++ b/crypto/asymmetric_keys/pkcs7_key_type.c
@@ -71,7 +71,7 @@ static int pkcs7_preparse(struct key_preparsed_payload *prep)
 
ret = verify_pkcs7_signature(NULL, 0,
 prep->data, prep->datalen,
-NULL, -ENOKEY, usage,
+NULL, usage,
 pkcs7_view_content, prep);
 
kleave(" = %d", ret);
diff --git a/crypto/asymmetric_keys/pkcs7_parser.h 
b/crypto/asymmetric_keys/pkcs7_parser.h
index a66b19ebcf47..c8159983ed8f 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.h
+++ b/crypto/asymmetric_keys/pkcs7_parser.h
@@ -22,7 +22,6 @@ struct pkcs7_signed_info {
struct pkcs7_signed_info *next;
struct x509_certificate *signer; /* Signing certificate (in msg->certs) 
*/
unsignedindex;
-   booltrusted;
boolunsupported_crypto; /* T if not usable due to 
missing crypto */
 
/* Message digest - the digest of the Content Data (or NULL) */
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c 
b/crypto/asymmetric_keys/pkcs7_trust.c
index 90d6d47965b0..388007fed3b2 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -30,7 +30,6 @@ static int pkcs7_validate_trust_one(struct pkcs7_message 
*pkcs7,
struct public_key_signature *sig = >sig;
struct x509_certificate *x509, *last = NULL, *p;
struct key *key;
-   bool trusted;
int ret;
 
kenter(",%u,", sinfo->index);
@@ -42,10 +41,8 @@ static int pkcs7_validate_trust_one(struct pkcs7_message 
*pkcs7,
 
for (x509 = sinfo->signer; x509; x509 = x509->signer) {
if (x509->seen) {
-   if (x509->verified) {
-   trusted = x509->trusted;
+   if (x509->verified)
goto verified;
-   }
kleave(" = -ENOKEY [cached]");
return -ENOKEY;
}
@@ -122,7 +119,6 @@ static int pkcs7_validate_trust_one(struct pkcs7_message 
*pkcs7,
 
 matched:
ret = verify_signature(key, sig);
-   trusted = test_bit(KEY_FLAG_TRUSTED, >flags);
key_put(key);
if (ret < 0) {
if (ret == -ENOMEM)
@@ -134,12 +130,9 @@ matched:
 verified:
if (x509) {

[PATCH 01/10] KEYS: Generalise system_verify_data() to provide access to internal content

[David Howells]

Generalise system_verify_data() to provide access to internal content
through a callback.  This allows all the PKCS#7 stuff to be hidden inside
this function and removed from the PE file parser and the PKCS#7 test key.

If external content is not required, NULL should be passed as data to the
function.  If the callback is not required, that can be set to NULL.

The function is now called verify_pkcs7_signature() to contrast with
verify_pefile_signature() and the definitions of both have been moved into
linux/verification.h along with the key_being_used_for enum.

Signed-off-by: David Howells 
---

 arch/x86/kernel/kexec-bzimage64.c   |   18 ++---
 certs/system_keyring.c  |   45 +-
 crypto/asymmetric_keys/Kconfig  |1 
 crypto/asymmetric_keys/mscode_parser.c  |   21 +++---
 crypto/asymmetric_keys/pkcs7_key_type.c |   64 +++
 crypto/asymmetric_keys/pkcs7_parser.c   |   21 +-
 crypto/asymmetric_keys/verify_pefile.c  |   40 ---
 crypto/asymmetric_keys/verify_pefile.h  |5 +-
 include/crypto/pkcs7.h  |3 +
 include/crypto/public_key.h |   14 ---
 include/keys/asymmetric-type.h  |1 
 include/keys/system_keyring.h   |7 ---
 include/linux/verification.h|   50 
 include/linux/verify_pefile.h   |   22 ---
 kernel/module_signing.c |5 +-
 15 files changed, 156 insertions(+), 161 deletions(-)
 create mode 100644 include/linux/verification.h
 delete mode 100644 include/linux/verify_pefile.h

diff --git a/arch/x86/kernel/kexec-bzimage64.c 
b/arch/x86/kernel/kexec-bzimage64.c
index 0f8a6bbaaa44..0b5da62eb203 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -19,8 +19,7 @@
 #include 
 #include 
 #include 
-#include 
-#include 
+#include 
 
 #include 
 #include 
@@ -529,18 +528,9 @@ static int bzImage64_cleanup(void *loader_data)
 #ifdef CONFIG_KEXEC_BZIMAGE_VERIFY_SIG
 static int bzImage64_verify_sig(const char *kernel, unsigned long kernel_len)
 {
-   bool trusted;
-   int ret;
-
-   ret = verify_pefile_signature(kernel, kernel_len,
- system_trusted_keyring,
- VERIFYING_KEXEC_PE_SIGNATURE,
- );
-   if (ret < 0)
-   return ret;
-   if (!trusted)
-   return -EKEYREJECTED;
-   return 0;
+   return verify_pefile_signature(kernel, kernel_len,
+  NULL,
+  VERIFYING_KEXEC_PE_SIGNATURE);
 }
 #endif
 
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 2570598b784d..cf55bd3a072a 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -108,16 +108,25 @@ late_initcall(load_system_certificate_list);
 #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
 
 /**
- * Verify a PKCS#7-based signature on system data.
- * @data: The data to be verified.
+ * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
+ * @data: The data to be verified (NULL if expecting internal data).
  * @len: Size of @data.
  * @raw_pkcs7: The PKCS#7 message that is the signature.
  * @pkcs7_len: The size of @raw_pkcs7.
+ * @trusted_keys: Trusted keys to use (NULL for system_trusted_keyring).
  * @usage: The use to which the key is being put.
+ * @view_content: Callback to gain access to content.
+ * @ctx: Context for callback.
  */
-int system_verify_data(const void *data, unsigned long len,
-  const void *raw_pkcs7, size_t pkcs7_len,
-  enum key_being_used_for usage)
+int verify_pkcs7_signature(const void *data, size_t len,
+  const void *raw_pkcs7, size_t pkcs7_len,
+  struct key *trusted_keys,
+  int untrusted_error,
+  enum key_being_used_for usage,
+  int (*view_content)(void *ctx,
+  const void *data, size_t len,
+  size_t asn1hdrlen),
+  void *ctx)
 {
struct pkcs7_message *pkcs7;
bool trusted;
@@ -128,7 +137,7 @@ int system_verify_data(const void *data, unsigned long len,
return PTR_ERR(pkcs7);
 
/* The data should be detached - so we need to supply it. */
-   if (pkcs7_supply_detached_data(pkcs7, data, len) < 0) {
+   if (data && pkcs7_supply_detached_data(pkcs7, data, len) < 0) {
pr_err("PKCS#7 signature with non-detached data\n");
ret = -EBADMSG;
goto error;
@@ -138,13 +147,29 @@ int system_verify_data(const void *data, unsigned long 
len,
if (ret < 0)
goto error;
 
-   ret = pkcs7_validate_trust(pkcs7, 

[PATCH 3/6] keys: Be more consistent in selection of union members used

[David Howells]

From: Insu Yun 

key->description and key->index_key.description are same because
they are unioned. But, for readability, using same name for
duplication and validation seems better.

Signed-off-by: Insu Yun 
Signed-off-by: David Howells 
---

 security/keys/key.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/key.c b/security/keys/key.c
index aee2ec5a18fc..c0478465d1ac 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -278,7 +278,7 @@ struct key *key_alloc(struct key_type *type, const char 
*desc,
 
key->index_key.desc_len = desclen;
key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
-   if (!key->description)
+   if (!key->index_key.description)
goto no_memory_3;
 
atomic_set(>usage, 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 00/10] KEYS: Change how keys are determined to be trusted

[Mimi Zohar]

On Wed, 2015-10-21 at 16:13 +0100, David Howells wrote:
> Here's a set of patches that changes how keys are determined to be trusted
> - currently, that's a case of whether a key has KEY_FLAG_TRUSTED set upon
> it.  A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
> indicates that only keys with this flag set may be added to that keyring.
> 
> Further, any time an X.509 certificate is instantiated without this flag
> set, the certificate is judged against the contents of the system trusted
> keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.
> 
> With these patches, KEY_FLAG_TRUSTED is removed.  The kernel may add
> implicitly trusted keys to a trusted-only keyring by asserting
> KEY_ALLOC_TRUSTED when the key is created, 

Ok, but only the x509 certificates built into the kernel image should be
automatically trusted and can be added to a trusted keyring, because the
kernel itself was signed (and verified).  These certificates extend the
(UEFI) certificate chain of trust that is rooted in hardware to the OS.

Other keys that the kernel reads and loads should not automatically be
trusted (eg. ima_load_x509).  They need to be validated against a
trusted key.

> but otherwise the key will only
> be allowed to be added to the keyring if it can be verified by a key
> already in that keyring.  The system trusted keyring is not then special in
> this sense and other trusted keyrings can be set up that are wholly
> independent of it.

We already went down this path of "transitive trust" back when we first
introduced the concept of trusted keys and keyrings.   Just because a
key is on a trusted keyring, doesn't imply that it should be permitted
to load other keys on the same trusted keyring.  In the case of
IMA-appraisal, the key should only be used to verify the file data
signature, not other keys.

The trusted keys used for verifying other certificates should be stored
on a separate keyring, not the target keyring.   Petko's patches define
a new IMA keyring named .ima_mok for this purpose.

Mimi

> To make this work, we have to retain sufficient data from the X.509
> certificate that we can then verify the signature at need.
> 
> The patches can be found here also:
> 
>   
> http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-trust
> 
> and are tagged with:
> 
>   keys-trust-20151021
> 


--
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 v5 0/3] RFC: Secure Memory Allocation Framework

[Benjamin Gaignard]

2015-10-21 16:34 GMT+02:00 James Morris :
> On Wed, 21 Oct 2015, Benjamin Gaignard wrote:
>
>>
>> The outcome of the previous RFC about how do secure data path was the need
>> of a secure memory allocator (https://lkml.org/lkml/2015/5/5/551)
>>
>
> Have you addressed all the questions raised by Alan here:
>
> https://lkml.org/lkml/2015/5/8/629

SMAF create /dev/smaf where all allocations could be done and is the
owner of the dmabuf.
Secure module is called to check permissions before that the CPU could
access to the memory.

I hope this cover what Alan expected but I can't speak form him.

>
> Also, is there any application of this beyond DRM?
>

If you don't use the secure part you can consider that SMAF is a
central allocator with helpers to select
the best allocator for your hardware devices.
While SMAF doesn't rely on DRM concepts (crypto, CENC, keys etc...) we
can use it outside this context but obviously it that been first
designed for DRM uses cases.

>
> - James
> --
> James Morris
> 
>



-- 
Benjamin Gaignard

Graphic Working Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
--
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 4/6] KEYS: Provide a script to extract the sys cert list from a vmlinux file

[David Howells]

The supplied script takes a vmlinux file - and if necessary a System.map
file - locates the system certificates list and extracts it to the named
file.

Call as:

./scripts/extract-sys-certs vmlinux certs

if vmlinux contains symbols and:

./scripts/extract-sys-certs -s System.map vmlinux certs

if it does not.

It prints something like the following to stdout:

Have 27 sections
No symbols in vmlinux, trying System.map
Have 80088 symbols
Have 1346 bytes of certs at VMA 0x8201c540
Certificate list in section .init.data
Certificate list at file offset 0x141c540

If vmlinux contains symbols then that is used rather than System.map - even
if one is given.

Signed-off-by: David Howells 
---

 scripts/extract-sys-certs.pl |  144 ++
 1 file changed, 144 insertions(+)
 create mode 100755 scripts/extract-sys-certs.pl

diff --git a/scripts/extract-sys-certs.pl b/scripts/extract-sys-certs.pl
new file mode 100755
index ..d476e7d1fd88
--- /dev/null
+++ b/scripts/extract-sys-certs.pl
@@ -0,0 +1,144 @@
+#!/usr/bin/perl -w
+#
+use strict;
+use Math::BigInt;
+use Fcntl "SEEK_SET";
+
+die "Format: $0 [-s ]  \n"
+if ($#ARGV != 1 && $#ARGV != 3 ||
+   $#ARGV == 3 && $ARGV[0] ne "-s");
+
+my $sysmap = "";
+if ($#ARGV == 3) {
+shift;
+$sysmap = $ARGV[0];
+shift;
+}
+
+my $vmlinux = $ARGV[0];
+my $keyring = $ARGV[1];
+
+#
+# Parse the vmlinux section table
+#
+open FD, "objdump -h $vmlinux |" || die $vmlinux;
+my @lines = ;
+close(FD) || die $vmlinux;
+
+my @sections = ();
+
+foreach my $line (@lines) {
+chomp($line);
+if ($line =~ 
/\s*([0-9]+)\s+(\S+)\s+([0-9a-f]+)\s+([0-9a-f]+)\s+([0-9a-f]+)\s+([0-9a-f]+)\s+2[*][*]([0-9]+)/
+   ) {
+   my $seg  = $1;
+   my $name = $2;
+   my $len  = Math::BigInt->new("0x" . $3);
+   my $vma  = Math::BigInt->new("0x" . $4);
+   my $lma  = Math::BigInt->new("0x" . $5);
+   my $foff = Math::BigInt->new("0x" . $6);
+   my $align = 2 ** $7;
+
+   push @sections, { name => $name,
+ vma => $vma,
+ len => $len,
+ foff => $foff };
+}
+}
+
+print "Have $#sections sections\n";
+
+#
+# Try and parse the vmlinux symbol table.  If the vmlinux file has been created
+# from a vmlinuz file with extract-vmlinux then the symbol table will be empty.
+#
+open FD, "nm $vmlinux 2>/dev/null |" || die $vmlinux;
+@lines = ;
+close(FD) || die $vmlinux;
+
+my %symbols = ();
+my $nr_symbols = 0;
+
+sub parse_symbols(@) {
+foreach my $line (@_) {
+   chomp($line);
+   if ($line =~ /([0-9a-f]+)\s([a-zA-Z])\s(\S+)/
+   ) {
+   my $addr = "0x" . $1;
+   my $type = $2;
+   my $name = $3;
+
+   $symbols{$name} = $addr;
+   $nr_symbols++;
+   }
+}
+}
+parse_symbols(@lines);
+
+if ($nr_symbols == 0 && $sysmap ne "") {
+print "No symbols in vmlinux, trying $sysmap\n";
+
+open FD, "<$sysmap" || die $sysmap;
+@lines = ;
+close(FD) || die $sysmap;
+parse_symbols(@lines);
+}
+
+die "No symbols available\n"
+if ($nr_symbols == 0);
+
+print "Have $nr_symbols symbols\n";
+
+die "Can't find system certificate list"
+unless (exists($symbols{"__cert_list_start"}) &&
+   exists($symbols{"__cert_list_end"}));
+
+my $start = Math::BigInt->new($symbols{"__cert_list_start"});
+my $end = Math::BigInt->new($symbols{"__cert_list_end"});
+my $size = $end - $start;
+
+printf "Have %u bytes of certs at VMA 0x%x\n", $size, $start;
+
+my $s = undef;
+foreach my $sec (@sections) {
+my $s_name = $sec->{name};
+my $s_vma = $sec->{vma};
+my $s_len = $sec->{len};
+my $s_foff = $sec->{foff};
+my $s_vend = $s_vma + $s_len;
+
+next unless ($start >= $s_vma);
+next if ($start >= $s_vend);
+
+die "Cert object partially overflows section $s_name\n"
+   if ($end > $s_vend);
+
+die "Cert object in multiple sections: ", $s_name, " and ", $s->{name}, 
"\n"
+   if ($s);
+$s = $sec;
+}
+
+die "Cert object not inside a section\n"
+unless ($s);
+
+print "Certificate list in section ", $s->{name}, "\n";
+
+my $foff = $start - $s->{vma} + $s->{foff};
+
+printf "Certificate list at file offset 0x%x\n", $foff;
+
+open FD, "<$vmlinux" || die $vmlinux;
+binmode(FD);
+die $vmlinux if (!defined(sysseek(FD, $foff, SEEK_SET)));
+my $buf = "";
+my $len = sysread(FD, $buf, $size);
+die "$vmlinux" if (!defined($len));
+die "Short read on $vmlinux\n" if ($len != $size);
+close(FD) || die $vmlinux;
+
+open FD, ">$keyring" || die $keyring;
+binmode(FD);
+$len = syswrite(FD, $buf, $size);
+die "$keyring" if (!defined($len));
+die "Short write on $keyring\n" if ($len != $size);
+close(FD) || die $keyring;

--
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 

[PATCH 09/10] X.509: Move the trust validation code out to its own file

[David Howells]

Move the X.509 trust validation code out to its own file so that it can be
generalised.

Signed-off-by: David Howells 
---

 crypto/asymmetric_keys/Makefile   |2 
 crypto/asymmetric_keys/public_key_trust.c |  192 +
 crypto/asymmetric_keys/x509_parser.h  |6 +
 crypto/asymmetric_keys/x509_public_key.c  |  167 -
 4 files changed, 199 insertions(+), 168 deletions(-)
 create mode 100644 crypto/asymmetric_keys/public_key_trust.c

diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
index cd1406f9b14a..bd07987c64e7 100644
--- a/crypto/asymmetric_keys/Makefile
+++ b/crypto/asymmetric_keys/Makefile
@@ -12,7 +12,7 @@ obj-$(CONFIG_PUBLIC_KEY_ALGO_RSA) += rsa.o
 #
 # X.509 Certificate handling
 #
-obj-$(CONFIG_X509_CERTIFICATE_PARSER) += x509_key_parser.o
+obj-$(CONFIG_X509_CERTIFICATE_PARSER) += x509_key_parser.o public_key_trust.o
 x509_key_parser-y := \
x509-asn1.o \
x509_akid-asn1.o \
diff --git a/crypto/asymmetric_keys/public_key_trust.c 
b/crypto/asymmetric_keys/public_key_trust.c
new file mode 100644
index ..753a413d479b
--- /dev/null
+++ b/crypto/asymmetric_keys/public_key_trust.c
@@ -0,0 +1,192 @@
+/* Instantiate a public key crypto key from an X.509 Certificate
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowe...@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#define pr_fmt(fmt) "X.509: "fmt
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "asymmetric_keys.h"
+#include "public_key.h"
+#include "x509_parser.h"
+
+static bool use_builtin_keys;
+static struct asymmetric_key_id *ca_keyid;
+
+#ifndef MODULE
+static struct {
+   struct asymmetric_key_id id;
+   unsigned char data[10];
+} cakey;
+
+static int __init ca_keys_setup(char *str)
+{
+   if (!str)   /* default system keyring */
+   return 1;
+
+   if (strncmp(str, "id:", 3) == 0) {
+   struct asymmetric_key_id *p = 
+   size_t hexlen = (strlen(str) - 3) / 2;
+   int ret;
+
+   if (hexlen == 0 || hexlen > sizeof(cakey.data)) {
+   pr_err("Missing or invalid ca_keys id\n");
+   return 1;
+   }
+
+   ret = __asymmetric_key_hex_to_key_id(str + 3, p, hexlen);
+   if (ret < 0)
+   pr_err("Unparsable ca_keys id hex string\n");
+   else
+   ca_keyid = p;   /* owner key 'id:xx' */
+   } else if (strcmp(str, "builtin") == 0) {
+   use_builtin_keys = true;
+   }
+
+   return 1;
+}
+__setup("ca_keys=", ca_keys_setup);
+#endif
+
+/**
+ * x509_request_asymmetric_key - Request a key by X.509 certificate params.
+ * @keyring: The keys to search.
+ * @id: The issuer & serialNumber to look for or NULL.
+ * @skid: The subjectKeyIdentifier to look for or NULL.
+ * @partial: Use partial match if true, exact if false.
+ *
+ * Find a key in the given keyring by identifier.  The preferred identifier is
+ * the issuer + serialNumber and the fallback identifier is the
+ * subjectKeyIdentifier.  If both are given, the lookup is by the former, but
+ * the latter must also match.
+ */
+struct key *x509_request_asymmetric_key(struct key *keyring,
+   const struct asymmetric_key_id *id,
+   const struct asymmetric_key_id *skid,
+   bool partial)
+{
+   struct key *key;
+   key_ref_t ref;
+   const char *lookup;
+   char *req, *p;
+   int len;
+
+   if (id) {
+   lookup = id->data;
+   len = id->len;
+   } else {
+   lookup = skid->data;
+   len = skid->len;
+   }
+
+   /* Construct an identifier "id:". */
+   p = req = kmalloc(2 + 1 + len * 2 + 1, GFP_KERNEL);
+   if (!req)
+   return ERR_PTR(-ENOMEM);
+
+   if (partial) {
+   *p++ = 'i';
+   *p++ = 'd';
+   } else {
+   *p++ = 'e';
+   *p++ = 'x';
+   }
+   *p++ = ':';
+   p = bin2hex(p, lookup, len);
+   *p = 0;
+
+   pr_debug("Look up: \"%s\"\n", req);
+
+   ref = keyring_search(make_key_ref(keyring, 1),
+_type_asymmetric, req);
+   if (IS_ERR(ref))
+   pr_debug("Request for key '%s' err %ld\n", req, PTR_ERR(ref));
+   kfree(req);
+
+   if (IS_ERR(ref)) {
+   switch (PTR_ERR(ref)) {
+   /* Hide some search errors */
+   

[PATCH 07/10] X.509: Extract signature digest and make self-signed cert checks earlier

[David Howells]

Extract the signature digest for an X.509 certificate earlier, at the end
of x509_cert_parse() rather than leaving it to the callers thereof.

Further, immediately after that, check the signature on self-signed
certificates, also rather in the callers of x509_cert_parse().

This we need to determine whether or not the X.509 cert requires crypto
that we don't support before we do the above two steps.

We note in the x509_certificate struct the following bits of information:

 (1) Whether the signature is self-signed (even if we can't check the
 signature due to missing crypto).

 (2) Whether the key held in the certificate needs unsupported crypto to be
 used.  We may get a PKCS#7 message with X.509 certs that we can't make
 use of - we just ignore them and give ENOPKG at the end it we couldn't
 verify anything if at least one of these unusable certs are in the
 chain of trust.

 (3) Whether the signature held in the certificate needs unsupported crypto
 to be checked.  We can still use the key held in this certificate,
 even if we can't check the signature on it - if it is held in the
 system trusted keyring, for instance.  We just can't add it to a ring
 of trusted keys or follow it further up the chain of trust.

Making these checks earlier allows x509_check_signature() to be removed and
replaced with direct calls to public_key_verify_signature().

Signed-off-by: David Howells 
---

 crypto/asymmetric_keys/pkcs7_verify.c |   38 ++--
 crypto/asymmetric_keys/x509_cert_parser.c |   10 ++
 crypto/asymmetric_keys/x509_parser.h  |7 +
 crypto/asymmetric_keys/x509_public_key.c  |  139 -
 4 files changed, 121 insertions(+), 73 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_verify.c 
b/crypto/asymmetric_keys/pkcs7_verify.c
index e225dccdf559..1dede0199673 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -190,9 +190,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message 
*pkcs7,
 x509->subject,
 x509->raw_serial_size, x509->raw_serial);
x509->seen = true;
-   ret = x509_get_sig_params(x509);
-   if (ret < 0)
-   goto maybe_missing_crypto_in_x509;
+   if (x509->unsupported_key)
+   goto unsupported_crypto_in_x509;
 
pr_debug("- issuer %s\n", x509->issuer);
sig = x509->sig;
@@ -203,22 +202,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message 
*pkcs7,
pr_debug("- authkeyid.skid %*phN\n",
 sig->auth_ids[1]->len, sig->auth_ids[1]->data);
 
-   if ((!x509->sig->auth_ids[0] && !x509->sig->auth_ids[1]) ||
-   strcmp(x509->subject, x509->issuer) == 0) {
+   if (x509->self_signed) {
/* If there's no authority certificate specified, then
 * the certificate must be self-signed and is the root
 * of the chain.  Likewise if the cert is its own
 * authority.
 */
-   pr_debug("- no auth?\n");
-   if (x509->raw_subject_size != x509->raw_issuer_size ||
-   memcmp(x509->raw_subject, x509->raw_issuer,
-  x509->raw_issuer_size) != 0)
-   return 0;
-
-   ret = x509_check_signature(x509->pub, x509);
-   if (ret < 0)
-   goto maybe_missing_crypto_in_x509;
+   if (x509->unsupported_sig)
+   goto unsupported_crypto_in_x509;
x509->signer = x509;
pr_debug("- self-signed\n");
return 0;
@@ -270,7 +261,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message 
*pkcs7,
sinfo->index);
return 0;
}
-   ret = x509_check_signature(p->pub, x509);
+   ret = public_key_verify_signature(p->pub, p->sig);
if (ret < 0)
return ret;
x509->signer = p;
@@ -282,16 +273,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message 
*pkcs7,
might_sleep();
}
 
-maybe_missing_crypto_in_x509:
+unsupported_crypto_in_x509:
/* Just prune the certificate chain at this point if we lack some
 * crypto module to go further.  Note, however, we don't want to set
-* sinfo->missing_crypto as the signed info block may still be
+* sinfo->unsupported_crypto as the signed info block may still be
 * validatable against an X.509 cert lower in the chain that we have a
 * trusted copy of.
 */
-   

[PATCH 08/10] PKCS#7: Make the signature a pointer rather than embedding it

[David Howells]

Point to the public_key_signature struct from the pkcs7_signed_info struct
rather than embedding it.  This makes it easier to have it take an
arbitrary number of MPIs in future.

We also save a copy of the digest in the signature without sharing the
memory with the crypto layer metadata.  This means we can use
public_key_free() to get rid of the signature record.

Signed-off-by: David Howells 
---

 crypto/asymmetric_keys/pkcs7_parser.c |   38 +++-
 crypto/asymmetric_keys/pkcs7_parser.h |   10 +++---
 crypto/asymmetric_keys/pkcs7_trust.c  |4 +--
 crypto/asymmetric_keys/pkcs7_verify.c |   52 +
 4 files changed, 56 insertions(+), 48 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_parser.c 
b/crypto/asymmetric_keys/pkcs7_parser.c
index 7b69783cff99..8454ae5b5aa8 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -44,9 +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.digest);
-   kfree(sinfo->signing_cert_id);
+   public_key_free(NULL, sinfo->sig);
kfree(sinfo);
}
 }
@@ -125,6 +123,10 @@ struct pkcs7_message *pkcs7_parse_message(const void 
*data, size_t datalen)
ctx->sinfo = kzalloc(sizeof(struct pkcs7_signed_info), GFP_KERNEL);
if (!ctx->sinfo)
goto out_no_sinfo;
+   ctx->sinfo->sig = kzalloc(sizeof(struct public_key_signature),
+ GFP_KERNEL);
+   if (!ctx->sinfo->sig)
+   goto out_no_sig;
 
ctx->data = (unsigned long)data;
ctx->ppcerts = >certs;
@@ -150,6 +152,7 @@ out:
ctx->certs = cert->next;
x509_free_certificate(cert);
}
+out_no_sig:
pkcs7_free_signed_info(ctx->sinfo);
 out_no_sinfo:
pkcs7_free_message(ctx->msg);
@@ -219,25 +222,25 @@ int pkcs7_sig_note_digest_algo(void *context, size_t 
hdrlen,
 
switch (ctx->last_oid) {
case OID_md4:
-   ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_MD4;
+   ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_MD4;
break;
case OID_md5:
-   ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_MD5;
+   ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_MD5;
break;
case OID_sha1:
-   ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA1;
+   ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_SHA1;
break;
case OID_sha256:
-   ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA256;
+   ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_SHA256;
break;
case OID_sha384:
-   ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA384;
+   ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_SHA384;
break;
case OID_sha512:
-   ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA512;
+   ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_SHA512;
break;
case OID_sha224:
-   ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA224;
+   ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_SHA224;
default:
printk("Unsupported digest algo: %u\n", ctx->last_oid);
return -ENOPKG;
@@ -256,7 +259,7 @@ int pkcs7_sig_note_pkey_algo(void *context, size_t hdrlen,
 
switch (ctx->last_oid) {
case OID_rsaEncryption:
-   ctx->sinfo->sig.pkey_algo = PKEY_ALGO_RSA;
+   ctx->sinfo->sig->pkey_algo = PKEY_ALGO_RSA;
break;
default:
printk("Unsupported pkey algo: %u\n", ctx->last_oid);
@@ -617,16 +620,17 @@ int pkcs7_sig_note_signature(void *context, size_t hdrlen,
 const void *value, size_t vlen)
 {
struct pkcs7_parse_context *ctx = context;
+   struct public_key_signature *sig = ctx->sinfo->sig;
MPI mpi;
 
-   BUG_ON(ctx->sinfo->sig.pkey_algo != PKEY_ALGO_RSA);
+   BUG_ON(sig->pkey_algo != PKEY_ALGO_RSA);
 
mpi = mpi_read_raw_data(value, vlen);
if (!mpi)
return -ENOMEM;
 
-   ctx->sinfo->sig.mpi[0] = mpi;
-   ctx->sinfo->sig.nr_mpi = 1;
+   sig->mpi[0] = mpi;
+   sig->nr_mpi = 1;
return 0;
 }
 
@@ -662,12 +666,16 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
 
pr_devel("SINFO KID: %u [%*phN]\n", kid->len, kid->len, kid->data);
 
-   sinfo->signing_cert_id = kid;
+   sinfo->sig->auth_ids[0] = kid;
sinfo->index = ++ctx->sinfo_index;
*ctx->ppsinfo = sinfo;
ctx->ppsinfo = >next;
ctx->sinfo = kzalloc(sizeof(struct pkcs7_signed_info), GFP_KERNEL);
if (!ctx->sinfo)
return -ENOMEM;
+ 

Re: [PATCH 00/10] KEYS: Change how keys are determined to be trusted

[Josh Boyer]

On Wed, Oct 21, 2015 at 1:02 PM, Mimi Zohar  wrote:
> On Wed, 2015-10-21 at 16:13 +0100, David Howells wrote:
>> Here's a set of patches that changes how keys are determined to be trusted
>> - currently, that's a case of whether a key has KEY_FLAG_TRUSTED set upon
>> it.  A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
>> indicates that only keys with this flag set may be added to that keyring.
>>
>> Further, any time an X.509 certificate is instantiated without this flag
>> set, the certificate is judged against the contents of the system trusted
>> keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.
>>
>> With these patches, KEY_FLAG_TRUSTED is removed.  The kernel may add
>> implicitly trusted keys to a trusted-only keyring by asserting
>> KEY_ALLOC_TRUSTED when the key is created,
>
> Ok, but only the x509 certificates built into the kernel image should be
> automatically trusted and can be added to a trusted keyring, because the
> kernel itself was signed (and verified).  These certificates extend the
> (UEFI) certificate chain of trust that is rooted in hardware to the OS.

That doesn't sound accurate to me.  The cert built into the kernel
image doesn't extend the UEFI certificates.  In most cases, it is a
ephemeral cert that is automatically generated at kernel build time
and then discarded.  It is not chained to or derived from any of the
UEFI certs stored in the db (or mok) variables.  The built-in cert is
used for module loading verification.  I agree that it should be
trusted, but not really for the reason you list.  Perhaps you meant
the key that the PE image of the kernel is signed with?  If so, the
kernel doesn't load that.  Only shim (and grub2 via shim) read that
key.

However, that does bring up the UEFI db/mok certs and how to deal with
those.  The out-of-tree patches we have add them to the system keyring
as trusted keys.  We can modify the patches to use KEY_ALLOC_TRUSTED
to preserve that functionality I suppose.

josh
--
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 00/10] KEYS: Change how keys are determined to be trusted

[Josh Boyer]

On Wed, Oct 21, 2015 at 2:11 PM, Mimi Zohar  wrote:
> On Wed, 2015-10-21 at 13:21 -0400, Josh Boyer wrote:
>> On Wed, Oct 21, 2015 at 1:02 PM, Mimi Zohar  wrote:
>> > On Wed, 2015-10-21 at 16:13 +0100, David Howells wrote:
>> >> Here's a set of patches that changes how keys are determined to be trusted
>> >> - currently, that's a case of whether a key has KEY_FLAG_TRUSTED set upon
>> >> it.  A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
>> >> indicates that only keys with this flag set may be added to that keyring.
>> >>
>> >> Further, any time an X.509 certificate is instantiated without this flag
>> >> set, the certificate is judged against the contents of the system trusted
>> >> keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.
>> >>
>> >> With these patches, KEY_FLAG_TRUSTED is removed.  The kernel may add
>> >> implicitly trusted keys to a trusted-only keyring by asserting
>> >> KEY_ALLOC_TRUSTED when the key is created,
>> >
>> > Ok, but only the x509 certificates built into the kernel image should be
>> > automatically trusted and can be added to a trusted keyring, because the
>> > kernel itself was signed (and verified).  These certificates extend the
>> > (UEFI) certificate chain of trust that is rooted in hardware to the OS.
>>
>> That doesn't sound accurate to me.  The cert built into the kernel
>> image doesn't extend the UEFI certificates.  In most cases, it is a
>> ephemeral cert that is automatically generated at kernel build time
>> and then discarded.  It is not chained to or derived from any of the
>> UEFI certs stored in the db (or mok) variables.  The built-in cert is
>> used for module loading verification.  I agree that it should be
>> trusted, but not really for the reason you list.  Perhaps you meant
>> the key that the PE image of the kernel is signed with?  If so, the
>> kernel doesn't load that.  Only shim (and grub2 via shim) read that
>> key.
>
> This is similar to the concept of the MoK DB.  Keys added to the MoK
> aren't signed by a UEFI key, yet they extend the UEFI secure boot
> certificate chain of trust.  Similarly, the certificates built into the

Right, because UEFI is verifying shim, which verifies grub2, which
verifies the kernel.  I get that.  However, it's irrelevant.

> kernel image don't need to be signed by a UEFI/MoK key for it to extend
> the certificate chain of trust.

The certificates built _into_ the kernel need to be trusted in all
cases.  It is how module signing is done.  So a user not using Secure
Boot, or even not using UEFI, still needs those embedded certs trusted
so that they can load modules.  It has nothing to do with UEFI or some
single-root-of-trust.

At any rate, I believe we are both saying the embedded cert needs to
be trusted so there's little point in debating further.  I just wanted
to point out that this need has nothing to do with UEFI.

josh
--
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 00/10] KEYS: Change how keys are determined to be trusted

[Petko Manolov]

On 15-10-21 13:02:48, Mimi Zohar wrote:
> On Wed, 2015-10-21 at 16:13 +0100, David Howells wrote:
> > Here's a set of patches that changes how keys are determined to be trusted
> > - currently, that's a case of whether a key has KEY_FLAG_TRUSTED set upon
> > it.  A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
> > indicates that only keys with this flag set may be added to that keyring.
> > 
> > Further, any time an X.509 certificate is instantiated without this flag
> > set, the certificate is judged against the contents of the system trusted
> > keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.
> > 
> > With these patches, KEY_FLAG_TRUSTED is removed.  The kernel may add
> > implicitly trusted keys to a trusted-only keyring by asserting
> > KEY_ALLOC_TRUSTED when the key is created, 
> 
> Ok, but only the x509 certificates built into the kernel image should be
> automatically trusted and can be added to a trusted keyring, because the
> kernel itself was signed (and verified).  These certificates extend the
> (UEFI) certificate chain of trust that is rooted in hardware to the OS.
> 
> Other keys that the kernel reads and loads should not automatically be
> trusted (eg. ima_load_x509).  They need to be validated against a
> trusted key.
> 
> > but otherwise the key will only
> > be allowed to be added to the keyring if it can be verified by a key
> > already in that keyring.  The system trusted keyring is not then special in
> > this sense and other trusted keyrings can be set up that are wholly
> > independent of it.
> 
> We already went down this path of "transitive trust" back when we first 
> introduced the concept of trusted keys and keyrings.  Just because a key is 
> on 
> a trusted keyring, doesn't imply that it should be permitted to load other 
> keys on the same trusted keyring.  In the case of IMA-appraisal, the key 
> should only be used to verify the file data signature, not other keys.
> 
> The trusted keys used for verifying other certificates should be stored on a 
> separate keyring, not the target keyring.  Petko's patches define a new IMA 
> keyring named .ima_mok for this purpose.

The concept is not new.  Some embedded applications are multi-tenant and 
typically have uptime measured in years.  The current CA hierarchy model of the 
kernel is somewhat limited in terms of dynamically adding trusted certificates 
and trusted keys.

.ima_mok was introduced as an intermediate keyring storing CAs that are 
themselves signed by CAs in the system keyring, which is trusted by default.  
Only keys that have been signed by certificate in .system or .ima_mok may land 
in .ima keyring.  This:

.system ---> .ima_mok ---> .ima ---> actual.key

gives us the ability to extend the chain of trust and also cover the above 
criteria.  That said, .ima_mok may be used for a whole bunch of other cases.

Think of a kernel module that comes from one of the tenants or even the machine 
owner.  They obviously don't have access to the Manufacturer's signing key 
(CA-M), but do have certificate (CA-O) that has been signed by it (CA-M).

This certificate (CA-O) can now go to .ima_mok (or whatever the name) and 
successfully verify the kernel's module signature.  CA-O may even sign another 
certificate, CA-O2, and by the above rules it may also go into .ima_mok.  And 
so 
on...

I think that in general having an intermediate CA keyring adds a lot of 
flexibility to the kernel's key management, although it's typical use does not 
make this mandatory.


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 00/10] KEYS: Change how keys are determined to be trusted

[Mimi Zohar]

On Wed, 2015-10-21 at 13:21 -0400, Josh Boyer wrote:
> On Wed, Oct 21, 2015 at 1:02 PM, Mimi Zohar  wrote:
> > On Wed, 2015-10-21 at 16:13 +0100, David Howells wrote:
> >> Here's a set of patches that changes how keys are determined to be trusted
> >> - currently, that's a case of whether a key has KEY_FLAG_TRUSTED set upon
> >> it.  A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
> >> indicates that only keys with this flag set may be added to that keyring.
> >>
> >> Further, any time an X.509 certificate is instantiated without this flag
> >> set, the certificate is judged against the contents of the system trusted
> >> keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.
> >>
> >> With these patches, KEY_FLAG_TRUSTED is removed.  The kernel may add
> >> implicitly trusted keys to a trusted-only keyring by asserting
> >> KEY_ALLOC_TRUSTED when the key is created,
> >
> > Ok, but only the x509 certificates built into the kernel image should be
> > automatically trusted and can be added to a trusted keyring, because the
> > kernel itself was signed (and verified).  These certificates extend the
> > (UEFI) certificate chain of trust that is rooted in hardware to the OS.
> 
> That doesn't sound accurate to me.  The cert built into the kernel
> image doesn't extend the UEFI certificates.  In most cases, it is a
> ephemeral cert that is automatically generated at kernel build time
> and then discarded.  It is not chained to or derived from any of the
> UEFI certs stored in the db (or mok) variables.  The built-in cert is
> used for module loading verification.  I agree that it should be
> trusted, but not really for the reason you list.  Perhaps you meant
> the key that the PE image of the kernel is signed with?  If so, the
> kernel doesn't load that.  Only shim (and grub2 via shim) read that
> key.

This is similar to the concept of the MoK DB.  Keys added to the MoK
aren't signed by a UEFI key, yet they extend the UEFI secure boot
certificate chain of trust.  Similarly, the certificates built into the
kernel image don't need to be signed by a UEFI/MoK key for it to extend
the certificate chain of trust.

> However, that does bring up the UEFI db/mok certs and how to deal with
> those.  The out-of-tree patches we have add them to the system keyring
> as trusted keys.  We can modify the patches to use KEY_ALLOC_TRUSTED
> to preserve that functionality I suppose.

Certificates are use case specific.  Just because a key was trusted at
the UEFI layer doesn't mean it should be trusted by the kernel (eg.
Microsoft key).  To illustrate this point, David Howells/David Woodhouse
recently posted/upstreamed patches to differentiate how keys loaded onto
the system keyring may be used. (Reference needed.)

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] userns/capability: Add user namespace capability

[Andy Lutomirski]

On Oct 19, 2015 7:25 AM, "Austin S Hemmelgarn"  wrote:
>
> On 2015-10-17 11:58, Tobias Markus wrote:
>>
>> Add capability CAP_SYS_USER_NS.
>> Tasks having CAP_SYS_USER_NS are allowed to create a new user namespace
>> when calling clone or unshare with CLONE_NEWUSER.
>>
>> Rationale:
>>
>> Linux 3.8 saw the introduction of unpriviledged user namespaces,
>> allowing unpriviledged users (without CAP_SYS_ADMIN) to be a "fake" root
>> inside a separate user namespace. Before that, any namespace creation
>> required CAP_SYS_ADMIN (or, in practice, the user had to be root).
>> Unfortunately, there have been some security-relevant bugs in the
>> meantime. Because of the fairly complex nature of user namespaces, it is
>> reasonable to say that future vulnerabilties can not be excluded. Some
>> distributions even wholly disable user namespaces because of this.
>>
>> Both options, user namespaces with and without CAP_SYS_ADMIN, can be
>> said to represent the extreme end of the spectrum. In practice, there is
>> no reason for every process to have the abilitiy to create user
>> namespaces. Indeed, only very few and specialized programs require user
>> namespaces. This seems to be a perfect fit for the (file) capability
>> system: Priviledged users could manually allow only a certain executable
>> to be able to create user namespaces by setting a certain capability,
>> I'd suggest the name CAP_SYS_USER_NS. Executables completely unrelated
>> to user namespaces should and can not create them.
>>
>> The capability should only be required in the "root" user namespace (the
>> user namespace with level 0) though, to allow nested user namespaces to
>> work as intended. If a user namespace has a level greater than 0, the
>> original process must have had CAP_SYS_USER_NS, so it is "trusted" anyway.
>>
>> One question remains though: Does this break userspace executables that
>> expect being able to create user namespaces without priviledge? Since
>> creating user namespaces without CAP_SYS_ADMIN was not possible before
>> Linux 3.8, programs should already expect a potential EPERM upon calling
>> clone. Since creating a user namespace without CAP_SYS_USER_NS would
>> also cause EPERM, we should be on the safe side.
>
>
> Potentially stupid counter proposal:
> Make it CAP_SYS_NS, make it allow access to all namespace types for 
> non-root/CAP_SYS_ADMIN users, and teach the stuff that's using userns just to 
> get to mount/pid/net/ipc namespaces to use those instead when it's something 
> that doesn't really need to think it's running as root.
>
> While this would still add a new capability (which is arguably not a good 
> thing), the resultant capability would be significantly more useful for many 
> of the use cases.

Then you'd have to come up with some argument that it could possibly
be safe.  You'd need *at least* no_new_privs forced on.  You would
also have fun defining the privilege to own such a namespace once
created.

--Andy
--
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 00/10] KEYS: Change how keys are determined to be trusted

[Mimi Zohar]

On Wed, 2015-10-21 at 14:21 -0400, Josh Boyer wrote:
> On Wed, Oct 21, 2015 at 2:11 PM, Mimi Zohar  wrote:
> > On Wed, 2015-10-21 at 13:21 -0400, Josh Boyer wrote:
> >> On Wed, Oct 21, 2015 at 1:02 PM, Mimi Zohar  
> >> wrote:
> >> > On Wed, 2015-10-21 at 16:13 +0100, David Howells wrote:
> >> >> Here's a set of patches that changes how keys are determined to be 
> >> >> trusted
> >> >> - currently, that's a case of whether a key has KEY_FLAG_TRUSTED set 
> >> >> upon
> >> >> it.  A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
> >> >> indicates that only keys with this flag set may be added to that 
> >> >> keyring.
> >> >>
> >> >> Further, any time an X.509 certificate is instantiated without this flag
> >> >> set, the certificate is judged against the contents of the system 
> >> >> trusted
> >> >> keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.
> >> >>
> >> >> With these patches, KEY_FLAG_TRUSTED is removed.  The kernel may add
> >> >> implicitly trusted keys to a trusted-only keyring by asserting
> >> >> KEY_ALLOC_TRUSTED when the key is created,
> >> >
> >> > Ok, but only the x509 certificates built into the kernel image should be
> >> > automatically trusted and can be added to a trusted keyring, because the
> >> > kernel itself was signed (and verified).  These certificates extend the
> >> > (UEFI) certificate chain of trust that is rooted in hardware to the OS.
> >>
> >> That doesn't sound accurate to me.  The cert built into the kernel
> >> image doesn't extend the UEFI certificates.  In most cases, it is a
> >> ephemeral cert that is automatically generated at kernel build time
> >> and then discarded.  It is not chained to or derived from any of the
> >> UEFI certs stored in the db (or mok) variables.  The built-in cert is
> >> used for module loading verification.  I agree that it should be
> >> trusted, but not really for the reason you list.  Perhaps you meant
> >> the key that the PE image of the kernel is signed with?  If so, the
> >> kernel doesn't load that.  Only shim (and grub2 via shim) read that
> >> key.
> >
> > This is similar to the concept of the MoK DB.  Keys added to the MoK
> > aren't signed by a UEFI key, yet they extend the UEFI secure boot
> > certificate chain of trust.  Similarly, the certificates built into the
> 
> Right, because UEFI is verifying shim, which verifies grub2, which
> verifies the kernel.  I get that.  However, it's irrelevant.
> 
> > kernel image don't need to be signed by a UEFI/MoK key for it to extend
> > the certificate chain of trust.
> 
> The certificates built _into_ the kernel need to be trusted in all
> cases.  It is how module signing is done.  So a user not using Secure
> Boot, or even not using UEFI, still needs those embedded certs trusted
> so that they can load modules.  It has nothing to do with UEFI or some
> single-root-of-trust.
> 
> At any rate, I believe we are both saying the embedded cert needs to
> be trusted so there's little point in debating further.  I just wanted
> to point out that this need has nothing to do with UEFI.

Right, the embedded certs need to trusted.  But that trust needs to be
based on something.  One method of establishing that trust is (UEFI)
secure boot, which verifies the kernel image signature, including the
embedded certs.

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

[GIT PULL] SELinux patches for 4.4

[Paul Moore]

Hi James,

Nine SELinux patches in total for v4.4, although six of those patches are 
either trivial, minor cleanups, or both.  The remaining three patches aren't 
too bad: one changes the CHECKREQPROT default to check the actual memory 
protections, one stops us from checking file:open on ftruncate() calls, and 
one converts the file_security_struct over to kmem_cache.

All pass the SELinux testsuite and should apply cleanly on top of your next 
branch.

Enjoy,
-Paul

---
The following changes since commit 09302fd19efbff9569eaad3f78ead8f411defd87:

  Merge branch 'smack-for-4.4' of https://github.com/cschaufler/smack-next 
into next (2015-10-21 10:49:29 +1100)

are available in the git repository at:

  git://git.infradead.org/users/pcmoore/selinux upstream

for you to fetch changes up to 63205654c0e05e5ffa1c6eef2fbef21dcabd2185:

  selinux: Use a kmem_cache for allocation struct file_security_struct
(2015-10-21 17:44:30 -0400)


Geliang Tang (1):
  selinux: ioctl_has_perm should be static

Jeff Vander Stoep (1):
  selinux: do not check open perm on ftruncate call

Paul Moore (1):
  selinux: change CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE default

Rasmus Villemoes (5):
  selinux: introduce security_context_str_to_sid
  selinux: remove pointless cast in selinux_inode_setsecurity()
  selinux: use kmemdup in security_sid_to_context_core()
  selinux: use kstrdup() in security_get_bools()
  selinux: use sprintf return value

Sangwoo (1):
  selinux: Use a kmem_cache for allocation struct file_security_struct

 security/selinux/Kconfig|  4 ++--
 security/selinux/hooks.c| 27 ++-
 security/selinux/include/security.h |  2 ++
 security/selinux/selinuxfs.c| 26 +-
 security/selinux/ss/services.c  | 22 +-
 5 files changed, 36 insertions(+), 45 deletions(-)

-- 
paul moore
security @ redhat

--
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 v5 1/3] create SMAF module

[James Morris]

On Wed, 21 Oct 2015, Benjamin Gaignard wrote:

> Secure Memory Allocation Framework goal is to be able
> to allocate memory that can be securing.
> There is so much ways to allocate and securing memory that SMAF
> doesn't do it by itself but need help of additional modules.
> To be sure to use the correct allocation method SMAF implement
> deferred allocation (i.e. allocate memory when only really needed)
> 
> Allocation modules (smaf-alloctor.h):
> SMAF could manage with multiple allocation modules at same time.
> To select the good one SMAF call match() to be sure that a module
> can allocate memory for a given list of devices. It is to the module
> to check if the devices are compatible or not with it allocation
> method.
> 
> Securing module (smaf-secure.h):
> The way of how securing memory it is done is platform specific.
> Secure module is responsible of grant/revoke memory access.
> 

This documentation is highly inadequate.

What does "allocate memory that can be securing" mean?


-- 
James Morris


--
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: [GIT PULL] SELinux patches for 4.4

[James Morris]

On Wed, 21 Oct 2015, Paul Moore wrote:

> Hi James,
> 
> Nine SELinux patches in total for v4.4, although six of those patches are 
> either trivial, minor cleanups, or both.  The remaining three patches aren't 
> too bad: one changes the CHECKREQPROT default to check the actual memory 
> protections, one stops us from checking file:open on ftruncate() calls, and 
> one converts the file_security_struct over to kmem_cache.
> 
> All pass the SELinux testsuite and should apply cleanly on top of your next 
> branch.
> 

Pulled, thanks.


-- 
James Morris


--
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] apparmor: clarify CRYPTO dependency

[James Morris]

On Wed, 21 Oct 2015, Arnd Bergmann wrote:

> The crypto framework can be built as a loadable module, but the
> apparmor hash code can only be built-in, which then causes a
> link error:
> 
> security/built-in.o: In function `aa_calc_profile_hash':
> integrity_audit.c:(.text+0x21610): undefined reference to 
> `crypto_shash_update'
> security/built-in.o: In function `init_profile_hash':
> integrity_audit.c:(.init.text+0xb4c): undefined reference to 
> `crypto_alloc_shash'
> 
> This changes Apparmor to use 'select CRYPTO' like a lot of other
> subsystems do.
> 
> Signed-off-by: Arnd Bergmann 

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next


-- 
James Morris


--
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 1/3] create SMAF module

[Benjamin Gaignard]

Secure Memory Allocation Framework goal is to be able
to allocate memory that can be securing.
There is so much ways to allocate and securing memory that SMAF
doesn't do it by itself but need help of additional modules.
To be sure to use the correct allocation method SMAF implement
deferred allocation (i.e. allocate memory when only really needed)

Allocation modules (smaf-alloctor.h):
SMAF could manage with multiple allocation modules at same time.
To select the good one SMAF call match() to be sure that a module
can allocate memory for a given list of devices. It is to the module
to check if the devices are compatible or not with it allocation
method.

Securing module (smaf-secure.h):
The way of how securing memory it is done is platform specific.
Secure module is responsible of grant/revoke memory access.

Signed-off-by: Benjamin Gaignard 
---
 drivers/Kconfig|   2 +
 drivers/Makefile   |   1 +
 drivers/smaf/Kconfig   |   5 +
 drivers/smaf/Makefile  |   1 +
 drivers/smaf/smaf-core.c   | 753 +
 include/linux/smaf-allocator.h |  54 +++
 include/linux/smaf-secure.h|  75 
 include/uapi/linux/smaf.h  |  52 +++
 8 files changed, 943 insertions(+)
 create mode 100644 drivers/smaf/Kconfig
 create mode 100644 drivers/smaf/Makefile
 create mode 100644 drivers/smaf/smaf-core.c
 create mode 100644 include/linux/smaf-allocator.h
 create mode 100644 include/linux/smaf-secure.h
 create mode 100644 include/uapi/linux/smaf.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 46b4a8e..a488c20 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -188,4 +188,6 @@ source "drivers/nvdimm/Kconfig"
 
 source "drivers/nvmem/Kconfig"
 
+source "drivers/smaf/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index b250b36..693390b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -167,3 +167,4 @@ obj-$(CONFIG_THUNDERBOLT)   += thunderbolt/
 obj-$(CONFIG_CORESIGHT)+= hwtracing/coresight/
 obj-$(CONFIG_ANDROID)  += android/
 obj-$(CONFIG_NVMEM)+= nvmem/
+obj-$(CONFIG_SMAF) += smaf/
diff --git a/drivers/smaf/Kconfig b/drivers/smaf/Kconfig
new file mode 100644
index 000..d36651a
--- /dev/null
+++ b/drivers/smaf/Kconfig
@@ -0,0 +1,5 @@
+config SMAF
+   tristate "Secure Memory Allocation Framework"
+   depends on DMA_SHARED_BUFFER
+   help
+ Choose this option to enable Secure Memory Allocation Framework
diff --git a/drivers/smaf/Makefile b/drivers/smaf/Makefile
new file mode 100644
index 000..40cd882
--- /dev/null
+++ b/drivers/smaf/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SMAF) += smaf-core.o
diff --git a/drivers/smaf/smaf-core.c b/drivers/smaf/smaf-core.c
new file mode 100644
index 000..1d9a55e
--- /dev/null
+++ b/drivers/smaf/smaf-core.c
@@ -0,0 +1,753 @@
+/*
+ * smaf.c
+ *
+ * Copyright (C) Linaro SA 2015
+ * Author: Benjamin Gaignard  for Linaro.
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct smaf_handle {
+   struct dma_buf *dmabuf;
+   struct smaf_allocator *allocator;
+   struct dma_buf *db_alloc;
+   size_t length;
+   unsigned int flags;
+   int fd;
+   atomic_t is_secure;
+   void *secure_ctx;
+};
+
+/**
+ * struct smaf_device - smaf device node private data
+ * @misc_dev:  the misc device
+ * @head:  list of allocator
+ * @lock:  list and secure pointer mutex
+ * @secure:pointer to secure functions helpers
+ */
+struct smaf_device {
+   struct miscdevice misc_dev;
+   struct list_head head;
+   /* list and secure pointer lock*/
+   struct mutex lock;
+   struct smaf_secure *secure;
+};
+
+static struct smaf_device smaf_dev;
+
+static bool have_secure_module(void)
+{
+   return !!smaf_dev.secure;
+}
+
+/**
+ * smaf_allow_cpu_access return true if CPU can access to memory
+ * if their is no secure module associated to SMAF assume that CPU can get
+ * access to the memory.
+ */
+static bool smaf_allow_cpu_access(struct smaf_handle *handle,
+ unsigned long flags)
+{
+   bool ret = true;
+
+   if (!atomic_read(>is_secure))
+   return true;
+
+   mutex_lock(_dev.lock);
+
+   if (!have_secure_module())
+   goto unlock;
+
+   ret = smaf_dev.secure->allow_cpu_access(handle->secure_ctx, flags);
+
+unlock:
+   mutex_unlock(_dev.lock);
+   return ret;
+}
+
+/**
+ * smaf_grant_access - return true if the specified device can get access
+ * to the memory area
+ *
+ * This function must be called with smaf_dev.lock set
+ */
+static bool smaf_grant_access(struct smaf_handle *handle, struct device *dev,
+dma_addr_t addr, size_t size,
+ 

[PATCH v5 3/3] SMAF: add fake secure module

[Benjamin Gaignard]

This module is allow testing secure calls of SMAF.

Signed-off-by: Benjamin Gaignard 
---
 drivers/smaf/Kconfig   |  6 +++
 drivers/smaf/Makefile  |  1 +
 drivers/smaf/smaf-fakesecure.c | 92 ++
 3 files changed, 99 insertions(+)
 create mode 100644 drivers/smaf/smaf-fakesecure.c

diff --git a/drivers/smaf/Kconfig b/drivers/smaf/Kconfig
index 058ec4c..fd17005 100644
--- a/drivers/smaf/Kconfig
+++ b/drivers/smaf/Kconfig
@@ -9,3 +9,9 @@ config SMAF_CMA
depends on SMAF && HAVE_DMA_ATTRS
help
  Choose this option to enable CMA allocation within SMAF
+
+config SMAF_FAKE_SECURE
+   tristate "SMAF fake secure module"
+   depends on SMAF
+   help
+ Choose this option to enable fake secure module for test purpose
diff --git a/drivers/smaf/Makefile b/drivers/smaf/Makefile
index 05bab01..00d5cd4 100644
--- a/drivers/smaf/Makefile
+++ b/drivers/smaf/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_SMAF) += smaf-core.o
 obj-$(CONFIG_SMAF_CMA) += smaf-cma.o
+obj-$(CONFIG_SMAF_FAKE_SECURE) += smaf-fakesecure.o
diff --git a/drivers/smaf/smaf-fakesecure.c b/drivers/smaf/smaf-fakesecure.c
new file mode 100644
index 000..75e12dd
--- /dev/null
+++ b/drivers/smaf/smaf-fakesecure.c
@@ -0,0 +1,92 @@
+/*
+ * smaf-fakesecure.c
+ *
+ * Copyright (C) Linaro SA 2015
+ * Author: Benjamin Gaignard  for Linaro.
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+#include 
+#include 
+#include 
+
+#define MAGIC 0xDEADBEEF
+
+struct fake_private {
+   int magic;
+};
+
+static void *smaf_fakesecure_create(void)
+{
+   struct fake_private *priv;
+
+   priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+   priv->magic = MAGIC;
+
+   return priv;
+}
+
+static int smaf_fakesecure_destroy(void *ctx)
+{
+   struct fake_private *priv = (struct fake_private *)ctx;
+
+   WARN_ON(!priv || (priv->magic != MAGIC));
+   kfree(priv);
+
+   return 0;
+}
+
+static bool smaf_fakesecure_grant_access(void *ctx,
+struct device *dev,
+size_t addr, size_t size,
+enum dma_data_direction direction)
+{
+   struct fake_private *priv = (struct fake_private *)ctx;
+
+   WARN_ON(!priv || (priv->magic != MAGIC));
+
+   return priv->magic == MAGIC;
+}
+
+static void smaf_fakesecure_revoke_access(void *ctx,
+ struct device *dev,
+ size_t addr, size_t size,
+ enum dma_data_direction direction)
+{
+   struct fake_private *priv = (struct fake_private *)ctx;
+
+   WARN_ON(!priv || (priv->magic != MAGIC));
+}
+
+static bool smaf_fakesecure_allow_cpu_access(void *ctx,
+enum dma_data_direction direction)
+{
+   struct fake_private *priv = (struct fake_private *)ctx;
+
+   WARN_ON(!priv || (priv->magic != MAGIC));
+
+   return priv->magic == MAGIC;
+}
+
+static struct smaf_secure fake = {
+   .create_ctx = smaf_fakesecure_create,
+   .destroy_ctx = smaf_fakesecure_destroy,
+   .grant_access = smaf_fakesecure_grant_access,
+   .revoke_access = smaf_fakesecure_revoke_access,
+   .allow_cpu_access = smaf_fakesecure_allow_cpu_access,
+};
+
+static int __init smaf_fakesecure_init(void)
+{
+   return smaf_register_secure();
+}
+module_init(smaf_fakesecure_init);
+
+static void __exit smaf_fakesecure_deinit(void)
+{
+   smaf_unregister_secure();
+}
+module_exit(smaf_fakesecure_deinit);
+
+MODULE_DESCRIPTION("SMAF fake secure module for test purpose");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Benjamin Gaignard ");
-- 
1.9.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 v5 2/3] SMAF: add CMA allocator

[Benjamin Gaignard]

SMAF CMA allocator implement helpers functions to allow SMAF
to allocate contiguous memory.

match() each if at least one of the attached devices have coherent_dma_mask
set to DMA_BIT_MASK(32).

For allocation it use dma_alloc_attrs() with DMA_ATTR_WRITE_COMBINE and not
dma_alloc_writecombine to be compatible with ARM 64bits architecture

Signed-off-by: Benjamin Gaignard 
---
 drivers/smaf/Kconfig|   6 ++
 drivers/smaf/Makefile   |   1 +
 drivers/smaf/smaf-cma.c | 200 
 3 files changed, 207 insertions(+)
 create mode 100644 drivers/smaf/smaf-cma.c

diff --git a/drivers/smaf/Kconfig b/drivers/smaf/Kconfig
index d36651a..058ec4c 100644
--- a/drivers/smaf/Kconfig
+++ b/drivers/smaf/Kconfig
@@ -3,3 +3,9 @@ config SMAF
depends on DMA_SHARED_BUFFER
help
  Choose this option to enable Secure Memory Allocation Framework
+
+config SMAF_CMA
+   tristate "SMAF CMA allocator"
+   depends on SMAF && HAVE_DMA_ATTRS
+   help
+ Choose this option to enable CMA allocation within SMAF
diff --git a/drivers/smaf/Makefile b/drivers/smaf/Makefile
index 40cd882..05bab01 100644
--- a/drivers/smaf/Makefile
+++ b/drivers/smaf/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_SMAF) += smaf-core.o
+obj-$(CONFIG_SMAF_CMA) += smaf-cma.o
diff --git a/drivers/smaf/smaf-cma.c b/drivers/smaf/smaf-cma.c
new file mode 100644
index 000..012edd3
--- /dev/null
+++ b/drivers/smaf/smaf-cma.c
@@ -0,0 +1,200 @@
+/*
+ * smaf-cma.c
+ *
+ * Copyright (C) Linaro SA 2015
+ * Author: Benjamin Gaignard  for Linaro.
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+struct smaf_cma_buffer_info {
+   struct device *dev;
+   size_t size;
+   void *vaddr;
+   dma_addr_t paddr;
+};
+
+/**
+ * find_matching_device - iterate over the attached devices to find one
+ * with coherent_dma_mask correctly set to DMA_BIT_MASK(32).
+ * Matching device (if any) will be used to aim CMA area.
+ */
+static struct device *find_matching_device(struct dma_buf *dmabuf)
+{
+   struct dma_buf_attachment *attach_obj;
+
+   list_for_each_entry(attach_obj, >attachments, node) {
+   if (attach_obj->dev->coherent_dma_mask == DMA_BIT_MASK(32))
+   return attach_obj->dev;
+   }
+
+   return NULL;
+}
+
+/**
+ * smaf_cma_match - return true if at least one device has been found
+ */
+static bool smaf_cma_match(struct dma_buf *dmabuf)
+{
+   return !!find_matching_device(dmabuf);
+}
+
+static void smaf_cma_release(struct dma_buf *dmabuf)
+{
+   struct smaf_cma_buffer_info *info = dmabuf->priv;
+   DEFINE_DMA_ATTRS(attrs);
+
+   dma_set_attr(DMA_ATTR_WRITE_COMBINE, );
+
+   dma_free_attrs(info->dev, info->size, info->vaddr, info->paddr, );
+
+   kfree(info);
+}
+
+static struct sg_table *smaf_cma_map(struct dma_buf_attachment *attachment,
+enum dma_data_direction direction)
+{
+   struct smaf_cma_buffer_info *info = attachment->dmabuf->priv;
+   struct sg_table *sgt;
+   int ret;
+
+   sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
+   if (!sgt)
+   return NULL;
+
+   ret = dma_get_sgtable(info->dev, sgt, info->vaddr,
+ info->paddr, info->size);
+   if (ret < 0)
+   goto out;
+
+   sg_dma_address(sgt->sgl) = info->paddr;
+   return sgt;
+
+out:
+   kfree(sgt);
+   return NULL;
+}
+
+static void smaf_cma_unmap(struct dma_buf_attachment *attachment,
+  struct sg_table *sgt,
+  enum dma_data_direction direction)
+{
+   /* do nothing */
+}
+
+static int smaf_cma_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
+{
+   struct smaf_cma_buffer_info *info = dmabuf->priv;
+   int ret;
+   DEFINE_DMA_ATTRS(attrs);
+
+   dma_set_attr(DMA_ATTR_WRITE_COMBINE, );
+
+   if (info->size < vma->vm_end - vma->vm_start)
+   return -EINVAL;
+
+   vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
+   ret = dma_mmap_attrs(info->dev, vma, info->vaddr, info->paddr,
+info->size, );
+
+   return ret;
+}
+
+static void *smaf_cma_vmap(struct dma_buf *dmabuf)
+{
+   struct smaf_cma_buffer_info *info = dmabuf->priv;
+
+   return info->vaddr;
+}
+
+static void *smaf_kmap_atomic(struct dma_buf *dmabuf, unsigned long offset)
+{
+   struct smaf_cma_buffer_info *info = dmabuf->priv;
+
+   return (void *)info->vaddr + offset;
+}
+
+static struct dma_buf_ops smaf_cma_ops = {
+   .map_dma_buf = smaf_cma_map,
+   .unmap_dma_buf = smaf_cma_unmap,
+   .mmap = smaf_cma_mmap,
+   .release = smaf_cma_release,
+   .kmap_atomic = smaf_kmap_atomic,
+   .kmap = smaf_kmap_atomic,
+   .vmap = smaf_cma_vmap,
+};
+
+static struct 

Re: [PATCH v4 2/3] Create IMA machine owner keys (MOK) and blacklist keyrings;

[Mimi Zohar]

On Wed, 2015-10-21 at 11:50 +0100, David Howells wrote:
> Mimi Zohar  wrote:
> 
> > Thinking about the blacklist keyring some more...
> 
> Are we talking about a blacklist keyring that userspace can use - or can it be
> only usable by the kernel?

The blacklist is referenced by the kernel before using a key on either
the .ima or the new .ima_mok keyring to validate a file signature.

> > My concern is more that keys can be added and removed at run time from
> > either of the .ima or the ima_mok keyrings.  The need for a blacklist
> > keyring is to prevent the key from being removed and at a later point
> > re-added.  Unfortunately, keys can be added and removed similarly from the
> > blacklist keyring as well.  Unless keys can be added, without the ability of
> > removing them, I'm not sure of the benefit of a blacklist keyring.  I assume
> > adding and removing keys requires the same write privilege.
> 
> The operations that modify the contents of a keyring in some way (link,
> unlink, clear) all operate under Write privilege.  That said, we could add a
> flag that suppresses unlink and clear on a keyring.  This could also suppress
> garbage collection of revoked and invalidated keys.

Adding the semantics at the keyring level would be better than at the
individual key level.   This new flag would prevent keys on the
blacklist from being removed.  I like this solution.

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 v4 2/3] Create IMA machine owner keys (MOK) and blacklist keyrings;

[Petko Manolov]

On 15-10-21 11:55:40, David Howells wrote:
> Mimi Zohar  wrote:
> 
> > > I need to think about this.  Should -EKEYREVOKED be the same as -ENOKEY 
> > > in 
> > > this case?  I guess the end result is pretty much the same from IMA view 
> > > point, but there may be a requirement to list all revoked keys...
> > 
> > When checking the blacklist, getting -EKEYREVOKED is definitely different 
> > than -ENOKEY.
> 
> Actually, I misspoke earlier.  Revoked keys are only skipped by the search if 
> a live key is found.  Should all the keys in the blacklist just be revoked so 
> that the search of the list returns either -ENOKEY (no key there) or 
> -EKEYREVOKED (the key is blacklisted)?  That might be getting too 
> over-complicated though.

>From IMA point of view both errors have the same effect - the requested 
operation is rejected.  I guess searching the blacklist keyring should return 
-EKEYREVOKED, which properly describes it's state.


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 v4 2/3] Create IMA machine owner keys (MOK) and blacklist keyrings;

[Mimi Zohar]

On Wed, 2015-10-21 at 11:52 +0100, David Howells wrote:
> Petko Manolov  wrote:
> 
> > As far as i know there is no concept of write-once to a keyring in the
> > kernel.  David will correct me if i am wrong.  I wonder how hard would it be
> > to add such functionality, in case it is missing?
> 
> Not hard, particularly if it's only an attribute that the kernel can set.

So the new flag would be set at keyring creation, similarly to
KEY_FLAG_TRUSTED_ONLY.

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 v4 2/3] Create IMA machine owner keys (MOK) and blacklist keyrings;

[David Howells]

Petko Manolov  wrote:

> > > As far as i know there is no concept of write-once to a keyring in the
> > > kernel.  David will correct me if i am wrong.  I wonder how hard would
> > > it be to add such functionality, in case it is missing?
> > 
> > Not hard, particularly if it's only an attribute that the kernel can set.
> 
> Definitely kernel-only.  The other way does not appeal to me in terms of 
> security.

Nor me in terms of letting userspace lock keys into the kernel arbitrarily.

David
--
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 2/3] Create IMA machine owner keys (MOK) and blacklist keyrings;

[Petko Manolov]

On 15-10-21 12:49:19, David Howells wrote:
> Petko Manolov  wrote:
> 
> > > > As far as i know there is no concept of write-once to a keyring in the
> > > > kernel.  David will correct me if i am wrong.  I wonder how hard would
> > > > it be to add such functionality, in case it is missing?
> > > 
> > > Not hard, particularly if it's only an attribute that the kernel can set.
> > 
> > Definitely kernel-only.  The other way does not appeal to me in terms of 
> > security.
> 
> Nor me in terms of letting userspace lock keys into the kernel arbitrarily.

+1


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 v4 2/3] Create IMA machine owner keys (MOK) and blacklist keyrings;

[Petko Manolov]

On 15-10-21 07:52:20, Mimi Zohar wrote:
> On Wed, 2015-10-21 at 14:29 +0300, Petko Manolov wrote:
> > On 15-10-21 07:22:58, Mimi Zohar wrote:
> > > On Wed, 2015-10-21 at 11:50 +0100, David Howells wrote:
> > > > Mimi Zohar  wrote:
> 
> > > Adding the semantics at the keyring level would be better than at the 
> > > individual key level.  This new flag would prevent keys on the blacklist 
> > > from 
> > > being removed.  I like this solution.
> > 
> > Err, what if the key's end-of-life is reached?  Revoked or not, it should 
> > go.  
> > This is more of a question rather than a statement.
> 
> Keys that have not expired should not be removed from the blacklist. 
> Otherwise 
> nothing prevents those keys from being re-loaded and used on a trusted 
> keyring. Expired keys would be flagged normally.  Any searches would result 
> in 
> -EKEYEXPIRED.

I guess the above summarizes the issue nicely.  Now let's do it. :)

> I guess there's no harm in removing expired keys from the blacklist.

I say this would be the correct behavior.


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 v4 2/3] Create IMA machine owner keys (MOK) and blacklist keyrings;

[Petko Manolov]

On 15-10-21 11:50:27, David Howells wrote:
> Mimi Zohar  wrote:
> 
> > Thinking about the blacklist keyring some more...
> 
> Are we talking about a blacklist keyring that userspace can use - or can it 
> be 
> only usable by the kernel?

So far the discussion has been entirely in IMA context.  Keys can be shuffled 
around by userspace (by root and non-root users) but their only consumer is the 
IMA code.  That said, nothing prevents these keyrings to be used by the rest of 
the system.  I guess sooner or later we'll end up making them global.  Having 
more flexible CA hierarchy in the kernel is a good thing IMHO.

> > My concern is more that keys can be added and removed at run time from 
> > either of the .ima or the ima_mok keyrings.  The need for a blacklist 
> > keyring is to prevent the key from being removed and at a later point 
> > re-added.  Unfortunately, keys can be added and removed similarly from the 
> > blacklist keyring as well.  Unless keys can be added, without the ability 
> > of 
> > removing them, I'm not sure of the benefit of a blacklist keyring.  I 
> > assume 
> > adding and removing keys requires the same write privilege.
> 
> The operations that modify the contents of a keyring in some way (link, 
> unlink, clear) all operate under Write privilege.  That said, we could add a 
> flag that suppresses unlink and clear on a keyring.  This could also suppress 
> garbage collection of revoked and invalidated keys.

Agreed.  If it is needed to preserve revoked key from being GCed or unlinked, i 
assume we'll need another flag.  Current permissions model can't guarantee it.

> Note, however, that keyring searches also skip revoked and invalidated keys, 
> so that would also need dealing with.

That too.  Some scenarios may require listing all revoked keys.  I am still 
waiting to hear whether we'll need this functionality.

I assume you are not opposed to the idea of introducing such flag(s) if needed?

> > (We previously resolved the problem of keyrings being removed by userspace, 
> > even by a privileged user, by dot prefixing the keyrings.)
> 
> That doesn't stop keys being addressed directly for invalidation and 
> revocation, but you can probably manage that with permissions.

In this particular case we only deal with trusted keyrings so this is of no 
concern.


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 v4 2/3] Create IMA machine owner keys (MOK) and blacklist keyrings;

[David Howells]

Mimi Zohar  wrote:

> > I need to think about this.  Should -EKEYREVOKED be the same as -ENOKEY in
> > this case?  I guess the end result is pretty much the same from IMA view
> > point, but there may be a requirement to list all revoked keys...
> 
> When checking the blacklist, getting -EKEYREVOKED is definitely
> different than -ENOKEY.

Actually, I misspoke earlier.  Revoked keys are only skipped by the search if
a live key is found.  Should all the keys in the blacklist just be revoked so
that the search of the list returns either -ENOKEY (no key there) or
-EKEYREVOKED (the key is blacklisted)?  That might be getting too
over-complicated though.

David
--
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 2/3] Create IMA machine owner keys (MOK) and blacklist keyrings;

[Mimi Zohar]

On Wed, 2015-10-21 at 11:55 +0100, David Howells wrote:
> Mimi Zohar  wrote:
> 
> > > I need to think about this.  Should -EKEYREVOKED be the same as -ENOKEY in
> > > this case?  I guess the end result is pretty much the same from IMA view
> > > point, but there may be a requirement to list all revoked keys...
> > 
> > When checking the blacklist, getting -EKEYREVOKED is definitely
> > different than -ENOKEY.
> 
> Actually, I misspoke earlier.  Revoked keys are only skipped by the search if
> a live key is found.  Should all the keys in the blacklist just be revoked so
> that the search of the list returns either -ENOKEY (no key there) or
> -EKEYREVOKED (the key is blacklisted)?  That might be getting too
> over-complicated though.

Right, your suggestion of adding a new flag on the keyring itself is
definitely preferable.

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 v4 2/3] Create IMA machine owner keys (MOK) and blacklist keyrings;

[Petko Manolov]

On 15-10-21 11:52:04, David Howells wrote:
> Petko Manolov  wrote:
> 
> > As far as i know there is no concept of write-once to a keyring in the 
> > kernel.  David will correct me if i am wrong.  I wonder how hard would it 
> > be 
> > to add such functionality, in case it is missing?
> 
> Not hard, particularly if it's only an attribute that the kernel can set.

Definitely kernel-only.  The other way does not appeal to me in terms of 
security.

> > Ideally a revoked key should stay in .blacklist until it expire or the 
> > system is rebooted.
> 
> That's not quite sufficient.  Search would also need to be modified otherwise 
> the revoked key would be skipped.

OK, let's see if this is going to be a problem or not.


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