[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 06/10] X.509: Retain the key verification data

[David Howells]

Retain the key verification data (ie. the struct public_key_signature)
including the digest and the key identifiers.

Note that this means that we need to take a separate copy of the digest in
x509_get_sig_params() rather than lumping it in with the crypto layer data.

Signed-off-by: David Howells 
---

 crypto/asymmetric_keys/pkcs7_trust.c  |8 ++-
 crypto/asymmetric_keys/pkcs7_verify.c |   20 +
 crypto/asymmetric_keys/x509_cert_parser.c |   43 +-
 crypto/asymmetric_keys/x509_parser.h  |4 --
 crypto/asymmetric_keys/x509_public_key.c  |   68 +++--
 5 files changed, 72 insertions(+), 71 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_trust.c 
b/crypto/asymmetric_keys/pkcs7_trust.c
index 388007fed3b2..7bb9389fd644 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -77,16 +77,16 @@ static int pkcs7_validate_trust_one(struct pkcs7_message 
*pkcs7,
 
might_sleep();
last = x509;
-   sig = >sig;
+   sig = last->sig;
}
 
/* No match - see if the root certificate has a signer amongst the
 * trusted keys.
 */
-   if (last && (last->akid_id || last->akid_skid)) {
+   if (last && (last->sig->auth_ids[0] || last->sig->auth_ids[1])) {
key = x509_request_asymmetric_key(trust_keyring,
- last->akid_id,
- last->akid_skid,
+ last->sig->auth_ids[0],
+ last->sig->auth_ids[1],
  false);
if (!IS_ERR(key)) {
x509 = last;
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c 
b/crypto/asymmetric_keys/pkcs7_verify.c
index d20c0b4b880e..e225dccdf559 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -175,6 +175,7 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
 static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
  struct pkcs7_signed_info *sinfo)
 {
+   struct public_key_signature *sig;
struct x509_certificate *x509 = sinfo->signer, *p;
struct asymmetric_key_id *auth;
int ret;
@@ -194,14 +195,15 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message 
*pkcs7,
goto maybe_missing_crypto_in_x509;
 
pr_debug("- issuer %s\n", x509->issuer);
-   if (x509->akid_id)
+   sig = x509->sig;
+   if (sig->auth_ids[0])
pr_debug("- authkeyid.id %*phN\n",
-x509->akid_id->len, x509->akid_id->data);
-   if (x509->akid_skid)
+sig->auth_ids[0]->len, sig->auth_ids[0]->data);
+   if (sig->auth_ids[1])
pr_debug("- authkeyid.skid %*phN\n",
-x509->akid_skid->len, x509->akid_skid->data);
+sig->auth_ids[1]->len, sig->auth_ids[1]->data);
 
-   if ((!x509->akid_id && !x509->akid_skid) ||
+   if ((!x509->sig->auth_ids[0] && !x509->sig->auth_ids[1]) ||
strcmp(x509->subject, x509->issuer) == 0) {
/* If there's no authority certificate specified, then
 * the certificate must be self-signed and is the root
@@ -225,7 +227,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message 
*pkcs7,
/* Look through the X.509 certificates in the PKCS#7 message's
 * list to see if the next one is there.
 */
-   auth = x509->akid_id;
+   auth = sig->auth_ids[0];
if (auth) {
pr_debug("- want %*phN\n", auth->len, auth->data);
for (p = pkcs7->certs; p; p = p->next) {
@@ -235,7 +237,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message 
*pkcs7,
goto found_issuer_check_skid;
}
} else {
-   auth = x509->akid_skid;
+   auth = sig->auth_ids[1];
pr_debug("- want %*phN\n", auth->len, auth->data);
for (p = pkcs7->certs; p; p = p->next) {
if (!p->skid)
@@ -255,8 +257,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message 
*pkcs7,
/* We matched issuer + serialNumber, but if there's an
 * authKeyId.keyId, that must match the CA subjKeyId also.
 */
-   if (x509->akid_skid &&
-   !asymmetric_key_id_same(p->skid, x509->akid_skid)) {
+   if (sig->auth_ids[1] &&
+ 

[PATCH 05/10] KEYS: Add identifier pointers to public_key_signature struct

[David Howells]

Add key identifier pointers to public_key_signature struct so that they can
be used to retain the identifier of the key to be used to verify the
signature in both PKCS#7 and X.509.

Signed-off-by: David Howells 
---

 crypto/asymmetric_keys/public_key.c |2 ++
 include/crypto/public_key.h |1 +
 2 files changed, 3 insertions(+)

diff --git a/crypto/asymmetric_keys/public_key.c 
b/crypto/asymmetric_keys/public_key.c
index e537aaeafdbf..f5b4824b7c77 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -72,6 +72,8 @@ void public_key_free(struct public_key *key,
}
 
if (sig) {
+   for (i = 0; i < ARRAY_SIZE(sig->auth_ids); i++)
+   kfree(sig->auth_ids[i]);
for (i = 0; i < ARRAY_SIZE(sig->mpi); i++)
mpi_free(sig->mpi[i]);
kfree(sig->digest);
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index a3f8f8268e23..ed86bfb23e89 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -76,6 +76,7 @@ struct public_key {
  * Public key cryptography signature data
  */
 struct public_key_signature {
+   struct asymmetric_key_id *auth_ids[2];
u8 *digest;
u8 digest_size; /* Number of bytes in digest */
u8 nr_mpi;  /* Occupancy of mpi[] */

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

[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-crypto" 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 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 04/10] KEYS: Allow authentication data to be stored in an asymmetric key

[David Howells]

Allow authentication data to be stored in an asymmetric key in the 4th
element of the key payload and provide a way for it to be destroyed.

For the public key subtype, this will be a public_key_signature struct.

Signed-off-by: David Howells 
---

 crypto/asymmetric_keys/asymmetric_type.c  |7 +--
 crypto/asymmetric_keys/public_key.c   |   22 +++---
 crypto/asymmetric_keys/x509_cert_parser.c |2 +-
 include/crypto/public_key.h   |5 +++--
 include/keys/asymmetric-subtype.h |2 +-
 include/keys/asymmetric-type.h|7 ---
 6 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c 
b/crypto/asymmetric_keys/asymmetric_type.c
index 9f2165b27d52..a79d30128821 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -331,7 +331,8 @@ static void asymmetric_key_free_preparse(struct 
key_preparsed_payload *prep)
pr_devel("==>%s()\n", __func__);
 
if (subtype) {
-   subtype->destroy(prep->payload.data[asym_crypto]);
+   subtype->destroy(prep->payload.data[asym_crypto],
+prep->payload.data[asym_auth]);
module_put(subtype->owner);
}
asymmetric_key_free_kids(kids);
@@ -346,13 +347,15 @@ static void asymmetric_key_destroy(struct key *key)
struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
struct asymmetric_key_ids *kids = key->payload.data[asym_key_ids];
void *data = key->payload.data[asym_crypto];
+   void *auth = key->payload.data[asym_auth];
 
key->payload.data[asym_crypto] = NULL;
key->payload.data[asym_subtype] = NULL;
key->payload.data[asym_key_ids] = NULL;
+   key->payload.data[asym_auth] = NULL;
 
if (subtype) {
-   subtype->destroy(data);
+   subtype->destroy(data, auth);
module_put(subtype->owner);
}
 
diff --git a/crypto/asymmetric_keys/public_key.c 
b/crypto/asymmetric_keys/public_key.c
index 6db4c01c6503..e537aaeafdbf 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -59,18 +59,34 @@ static void public_key_describe(const struct key 
*asymmetric_key,
 /*
  * Destroy a public key algorithm key.
  */
-void public_key_destroy(void *payload)
+void public_key_free(struct public_key *key,
+struct public_key_signature *sig)
 {
-   struct public_key *key = payload;
int i;
 
if (key) {
for (i = 0; i < ARRAY_SIZE(key->mpi); i++)
mpi_free(key->mpi[i]);
kfree(key);
+   key = NULL;
}
+
+   if (sig) {
+   for (i = 0; i < ARRAY_SIZE(sig->mpi); i++)
+   mpi_free(sig->mpi[i]);
+   kfree(sig->digest);
+   kfree(sig);
+   }
+}
+EXPORT_SYMBOL_GPL(public_key_free);
+
+/*
+ * Destroy a public key algorithm key.
+ */
+static void public_key_destroy(void *payload0, void *payload3)
+{
+   public_key_free(payload0, payload3);
 }
-EXPORT_SYMBOL_GPL(public_key_destroy);
 
 /*
  * Verify a signature using a public key.
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c 
b/crypto/asymmetric_keys/x509_cert_parser.c
index af71878dc15b..430848445dd9 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -48,7 +48,7 @@ struct x509_parse_context {
 void x509_free_certificate(struct x509_certificate *cert)
 {
if (cert) {
-   public_key_destroy(cert->pub);
+   public_key_free(cert->pub, NULL);
kfree(cert->issuer);
kfree(cert->subject);
kfree(cert->id);
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index de50d026576d..a3f8f8268e23 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -72,8 +72,6 @@ struct public_key {
};
 };
 
-extern void public_key_destroy(void *payload);
-
 /*
  * Public key cryptography signature data
  */
@@ -95,6 +93,9 @@ struct public_key_signature {
};
 };
 
+extern void public_key_free(struct public_key *key,
+   struct public_key_signature *sig);
+
 struct key;
 extern int verify_signature(const struct key *key,
const struct public_key_signature *sig);
diff --git a/include/keys/asymmetric-subtype.h 
b/include/keys/asymmetric-subtype.h
index 4915d40d3c3c..2480469ce8fb 100644
--- a/include/keys/asymmetric-subtype.h
+++ b/include/keys/asymmetric-subtype.h
@@ -32,7 +32,7 @@ struct asymmetric_key_subtype {
void (*describe)(const struct key *key, struct seq_file *m);
 
/* Destroy a key of this subtype */
-   void (*destroy)(void *payload);
+   void (*destroy)(void *payload_crypto, void *payload_auth);
 
/* 

Randomness for crypto, github repositories

[Sandy Harris]

I've just created github repositories for two projects:

https://github.com/sandy-harris/random.test

Test program for things I want to add to the Linux random(4) driver. I
am proposing a fairly radical rewrite. This gives an executable test
program for my new code, not a driver.

https://github.com/sandy-harris/maxwell

A demon to feed random(4) with entropy derived from the timer.
Intended mainly for use on limited systems which may lack other good
sources.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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;
+ 

[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.
 */
-   

Re: [Cryptography] Randomness for crypto, github repositories

[dj]

I've only looked at it briefly, but I have a question.. Are you trying to
use the GCM Galois multiply as an entropy extractor?

I don't know of any proof that it is a good extractor for any class of
entropic data. That doesn't mean there isn't one, but I've not heard of
one.

> I've just created github repositories for two projects:
>
> https://github.com/sandy-harris/random.test
>
> Test program for things I want to add to the Linux random(4) driver. I
> am proposing a fairly radical rewrite. This gives an executable test
> program for my new code, not a driver.
>
> https://github.com/sandy-harris/maxwell
>
> A demon to feed random(4) with entropy derived from the timer.
> Intended mainly for use on limited systems which may lack other good
> sources.

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

Re: [Cryptography] Randomness for crypto, github repositories

[Sandy Harris]

On Wed, Oct 21, 2015 at 1:06 PM,   wrote:

> I've only looked at it briefly, but I have a question.. Are you trying to
> use the GCM Galois multiply as an entropy extractor?

Yes, the basic idea is to use a series of GCM multiplies over the pool
data to replace the hashing of that data in the current driver. There
are complications; each hash uses two quasi-constants -- initialiser
and GCM multiplier -- and hashes a counter along with the pool data.
The counter changes on every iteration and is sometimes changed more
drastically, and the constants are sometimes updated

> I don't know of any proof that it is a good extractor for any class of
> entropic data. That doesn't mean there isn't one, but I've not heard of
> one.

Good question. It seems to me th at if it is secure for its
authentication usage, where it replaces an HMAC, then it should be
safe in this application. But no, I don't have a proof & the question
is worth some analysis.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] crypto: qat - don't use userspace pointer

[Tadeusz Struk]

Bugfix - don't dereference userspace pointer.

Signed-off-by: Tadeusz Struk 
---
 drivers/crypto/qat/qat_common/adf_ctl_drv.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c 
b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
index ebc1c63..fcb790e 100644
--- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
+++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
@@ -198,7 +198,7 @@ static int adf_copy_key_value_data(struct adf_accel_dev 
*accel_dev,
goto out_err;
}
 
-   params_head = section_head->params;
+   params_head = section.params;
 
while (params_head) {
if (copy_from_user(_val, (void __user *)params_head,

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

Re: [PATCH 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-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] crypto: qat - don't use userspace pointer

[Greg KH]

On Wed, Oct 21, 2015 at 12:30:45PM -0700, Tadeusz Struk wrote:
> Bugfix - don't dereference userspace pointer.
> 
> Signed-off-by: Tadeusz Struk 
> ---
>  drivers/crypto/qat/qat_common/adf_ctl_drv.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.


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

[PATCH] [RESEND] crypto: qat - don't use userspace pointer

[Tadeusz Struk]

Bugfix - don't dereference userspace pointer.

Resend with the correct way for inclusion in the stable kernel.

Cc: sta...@vger.kernel.org
Signed-off-by: Tadeusz Struk 
---
 drivers/crypto/qat/qat_common/adf_ctl_drv.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c 
b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
index ebc1c63..fcb790e 100644
--- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
+++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
@@ -198,7 +198,7 @@ static int adf_copy_key_value_data(struct adf_accel_dev 
*accel_dev,
goto out_err;
}
 
-   params_head = section_head->params;
+   params_head = section.params;
 
while (params_head) {
if (copy_from_user(_val, (void __user *)params_head,

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

Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

[David Miller]

From: Steffen Klassert 
Date: Wed, 21 Oct 2015 08:57:04 +0200

> On Mon, Oct 19, 2015 at 05:23:29PM -0400, Sowmini Varadhan wrote:
>> On sparc, deleting established SAs (e.g., by restarting ipsec
>> at the peer) results in unaligned access messages via
>> xfrm_del_sa -> km_state_notify -> xfrm_send_state_notify().
>> Use an aligned pointer to xfrm_usersa_info for this case.
>> 
>> Signed-off-by: Sowmini Varadhan 
>> ---
>>  net/xfrm/xfrm_user.c |2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
>> index a8de9e3..158ef4a 100644
>> --- a/net/xfrm/xfrm_user.c
>> +++ b/net/xfrm/xfrm_user.c
>> @@ -2659,7 +2659,7 @@ static int xfrm_notify_sa(struct xfrm_state *x, const 
>> struct km_event *c)
>>  if (attr == NULL)
>>  goto out_free_skb;
>>  
>> -p = nla_data(attr);
>> +p = PTR_ALIGN(nla_data(attr), __alignof__(*p));
> 
> Hm, this breaks userspace notifications on 64-bit systems.
> Userspace expects this to be aligned to 4, with your patch
> it is aligned to 8 on 64-bit.

That's correct, netlink attributes are fundamentally only 4 byte
aligned and this cannot be changed.  nla_data() is exactly
where the attribute must be placed, aligned or not.

The only workaround is, when designing netlink attributes.  Various
netlink libraries have workarounds for accessing, for example, 64-bit
stats which are going to be unaligned in netlink messages.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

[David Miller]

From: Sowmini Varadhan 
Date: Wed, 21 Oct 2015 06:54:42 -0400

> On (10/21/15 08:57), Steffen Klassert wrote:
>> > --- a/net/xfrm/xfrm_user.c
>> > +++ b/net/xfrm/xfrm_user.c
>> > @@ -2659,7 +2659,7 @@ static int xfrm_notify_sa(struct xfrm_state *x, 
>> > const struct km_event *c)
>> >if (attr == NULL)
>> >goto out_free_skb;
>> >  
>> > -  p = nla_data(attr);
>> > +  p = PTR_ALIGN(nla_data(attr), __alignof__(*p));
>> 
>> Hm, this breaks userspace notifications on 64-bit systems.
>> Userspace expects this to be aligned to 4, with your patch
>> it is aligned to 8 on 64-bit.
>> 
>> Without your patch I get the correct notification when deleting a SA:
>> 
> 
> But __alignof__(*p) is 8 on sparc, and without the patch I get
> all types of unaligned access. So what do you suggest as the fix?

The accesses have to be done using something like get_unaligned() and
put_unaligned().

Sorry, but the protocol is set in stone and this is unfortunately how
it is.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

[Sowmini Varadhan]

On (10/21/15 06:22), David Miller wrote:
> memcpy() _never_ works for avoiding unaligned accessed.
> 
> I repeat, no matter what you do, no matter what kinds of casts or
> fancy typing you use, memcpy() _never_ works for this purpose.
  :
> There is one and only one portable way to access unaligned data,
> and that is with the get_unaligned() and put_unaligned() helpers.

ok. I'll fix it up to use the *_unaligned functions and resend this 
out later today.

--Sowmini

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

Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

[Steffen Klassert]

On Mon, Oct 19, 2015 at 05:23:29PM -0400, Sowmini Varadhan wrote:
> On sparc, deleting established SAs (e.g., by restarting ipsec
> at the peer) results in unaligned access messages via
> xfrm_del_sa -> km_state_notify -> xfrm_send_state_notify().
> Use an aligned pointer to xfrm_usersa_info for this case.
> 
> Signed-off-by: Sowmini Varadhan 
> ---
>  net/xfrm/xfrm_user.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index a8de9e3..158ef4a 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -2659,7 +2659,7 @@ static int xfrm_notify_sa(struct xfrm_state *x, const 
> struct km_event *c)
>   if (attr == NULL)
>   goto out_free_skb;
>  
> - p = nla_data(attr);
> + p = PTR_ALIGN(nla_data(attr), __alignof__(*p));

Hm, this breaks userspace notifications on 64-bit systems.
Userspace expects this to be aligned to 4, with your patch
it is aligned to 8 on 64-bit.

Without your patch I get the correct notification when deleting a SA:

ip x m

Deleted src 172.16.0.2 dst 172.16.0.1
proto esp spi 0x0002 reqid 2 mode tunnel
replay-window 32
auth-trunc hmac(sha1) 0x31323334353637383930 96
enc cbc(aes) 0x31323334353637383930313233343536
sel src 10.0.0.0/24 dst 192.168.0.0/24

With your patch I get for the same SA:

ip x m

Deleted src 50.0.0.0 dst 0.0.0.0
proto 0 reqid 0 mode transport
replay-window 0 flag decap-dscp
auth-trunc hmac(sha1) 0x31323334353637383930 96
enc cbc(aes) 0x31323334353637383930313233343536
sel src 0.0.0.0/0 dst 0.234.255.255/0 proto igmp

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

Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

[Sowmini Varadhan]

On (10/21/15 06:54), Sowmini Varadhan wrote:
> But __alignof__(*p) is 8 on sparc, and without the patch I get
> all types of unaligned access. So what do you suggest as the fix?

Even though the alignment is, in fact, 8 (and that comes from
struct xfrm_lifetime_cfg), if uspace is firmly attached to the 4 byte
alignment, I think we can retain that behavior and still avoid
unaligned access in the kernel with the following (admittedly ugly hack).
Can you please take a look? I tested it with 'ip x m' and a transport 
mode tunnel on my sparc.


diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 158ef4a..ca4e7f0 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2620,7 +2620,7 @@ static inline size_t xfrm_sa_len(struct xfrm_state *x)
 static int xfrm_notify_sa(struct xfrm_state *x, const struct km_event *c)
 {
struct net *net = xs_net(x);
-   struct xfrm_usersa_info *p;
+   struct xfrm_usersa_info *p, tmp;
struct xfrm_usersa_id *id;
struct nlmsghdr *nlh;
struct sk_buff *skb;
@@ -2659,11 +2659,16 @@ static int xfrm_notify_sa(struct xfrm_state *x, const 
struct km_event *c)
if (attr == NULL)
goto out_free_skb;
 
-   p = PTR_ALIGN(nla_data(attr), __alignof__(*p));
+   p = nla_data(attr);
+   err = copy_to_user_state_extra(x, , skb);
+   if (err)
+   goto out_free_skb;
+   memcpy((u8 *)p, , sizeof(tmp));
+   } else {
+   err = copy_to_user_state_extra(x, p, skb);
+   if (err)
+   goto out_free_skb;
}
-   err = copy_to_user_state_extra(x, p, skb);
-   if (err)
-   goto out_free_skb;
 
nlmsg_end(skb, nlh);


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

Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

[Sowmini Varadhan]

On (10/21/15 08:57), Steffen Klassert wrote:
> > --- a/net/xfrm/xfrm_user.c
> > +++ b/net/xfrm/xfrm_user.c
> > @@ -2659,7 +2659,7 @@ static int xfrm_notify_sa(struct xfrm_state *x, const 
> > struct km_event *c)
> > if (attr == NULL)
> > goto out_free_skb;
> >  
> > -   p = nla_data(attr);
> > +   p = PTR_ALIGN(nla_data(attr), __alignof__(*p));
> 
> Hm, this breaks userspace notifications on 64-bit systems.
> Userspace expects this to be aligned to 4, with your patch
> it is aligned to 8 on 64-bit.
> 
> Without your patch I get the correct notification when deleting a SA:
> 

But __alignof__(*p) is 8 on sparc, and without the patch I get
all types of unaligned access. So what do you suggest as the fix?

(and openswan/pluto dont flag any errors with the patch, which is
a separate bug).

--Sowmini

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

RE: [PATCH] crypto: add asynchronous compression support

[Li, Weigang]

> -Original Message-
> From: Herbert Xu [mailto:herb...@gondor.apana.org.au]
> Sent: Wednesday, October 21, 2015 3:34 PM
> To: Sergey Senozhatsky
> Cc: Minchan Kim; Joonsoo Kim; Dan Streetman; Seth Jennings; Li, Weigang;
> Struk, Tadeusz
> Subject: Re: [PATCH] crypto: add asynchronous compression support
> 
> On Wed, Oct 21, 2015 at 04:33:22PM +0900, Sergey Senozhatsky wrote:
> >
> > the thing is -- I still want to have/use SW compressors; and they [the
> > existing S/W algorithms] want virtual addresses. so we need to
> > kmap_atomic pages in every SW algorithm? isn't it simpler to keep
> > kmap_atomic_to_page around?
> 
> The Crypto API will do it for you.  Have a look at how we handle it for aead
> and ahash for example.
> 
> Cheers,
> --
> Email: Herbert Xu  Home Page:
> http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Adding the "linux-crypto" list back to the latest discussion.
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html