Re: [PATCH v4 12/13] bluetooth/smp: ensure RNG is properly seeded before ECDH use

[Marcel Holtmann]

Hi Ted,

>> This protocol uses lots of complex cryptography that relies on securely
>> generated random numbers. Thus, it's important that the RNG is actually
>> seeded before use. Fortuantely, it appears we're always operating in
>> process context (there are many GFP_KERNEL allocations and other
>> sleeping operations), and so we can simply demand that the RNG is seeded
>> before we use it.
>> 
>> We take two strategies in this commit. The first is for the library code
>> that's called from other modules like hci or mgmt: here we just change
>> the call to get_random_bytes_wait, and return the result of the wait to
>> the caller, along with the other error codes of those functions like
>> usual. Then there's the SMP protocol handler itself, which makes many
>> many many calls to get_random_bytes during different phases. For this,
>> rather than have to change all the calls to get_random_bytes_wait and
>> propagate the error result, it's actually enough to just put a single
>> call to wait_for_random_bytes() at the beginning of the handler, to
>> ensure that all the subsequent invocations are safe, without having to
>> actually change them. Likewise, for the random address changing
>> function, we'd rather know early on in the function whether the RNG
>> initialization has been interrupted, rather than later, so we call
>> wait_for_random_bytes() at the top, so that later on the call to
>> get_random_bytes() is acceptable.
> 
> Do we need to do all of this?  Bluetooth folks, is it fair to assume
> that hci_power_on() has to be called before any bluetooth functions
> can be done?  If so, adding a wait_for_random_bytes() in
> hci_power_on() might be all that is necessary.

yes, there are plenty of commands needed before a controller becomes usable. 
When plugging in new Bluetooth hardware, we have to power it up and read the 
initial settings and configuration out of.

Also all the cryptographic features only apply to LE enabled controllers. The 
classic BR/EDR controllers have this all in hardware. So if you are not LE 
enabled, then there is not even a point in waiting for any seeding. However 
that said, also all LE controllers have an extra random number function we 
could call if we need extra seeding. We never bothered to hook this up since we 
thought that the kernel has enough sources.

Regards

Marcel

Re: [kernel-hardening] Re: [PATCH v3 04/13] crypto/rng: ensure that the RNG is ready before using

[Daniel Micay]

On Wed, 2017-06-07 at 18:26 +0100, Mark Rutland wrote:
> On Wed, Jun 07, 2017 at 01:00:25PM -0400, Daniel Micay wrote:
> > > On the better bootloaders, an initramfs segment can be loaded
> > > independently (and you can have as many as required), which makes
> > > an
> > > early_initramfs a more palatable vector to inject large amounts of
> > > entropy into the next boot than, say, modifying the kernel image
> > > directly at every boot/shutdown to stash entropy in there
> > > somewhere.
> 
> [...]
>  
> > I didn't really understand the device tree approach and mentioned a
> > few times before. Passing via the kernel cmdline is a lot simpler
> > than
> > modifying the device tree in-memory and persistent modification
> > isn't
> > an option unless verified boot is missing anyway.
> 
> I might be missing something here, but the command line is inside of
> the
> device tree, at /chosen/bootargs, so modifying the kernel command line
> *is* modifying the device tree in-memory.
> 
> For arm64, we have a /chosen/kaslr-seed property that we hope
> FW/bootloaders fill in, and similar could be done for some initial
> entropy, provided appropriate HW/FW support.
> 
> Thanks,
> Mark.

I was assuming it was simpler since bootloaders are already setting it,
but it seems I'm wrong about that.

Re: [PATCH v4 12/13] bluetooth/smp: ensure RNG is properly seeded before ECDH use

[Theodore Ts'o]

On Tue, Jun 06, 2017 at 07:48:03PM +0200, Jason A. Donenfeld wrote:
> This protocol uses lots of complex cryptography that relies on securely
> generated random numbers. Thus, it's important that the RNG is actually
> seeded before use. Fortuantely, it appears we're always operating in
> process context (there are many GFP_KERNEL allocations and other
> sleeping operations), and so we can simply demand that the RNG is seeded
> before we use it.
> 
> We take two strategies in this commit. The first is for the library code
> that's called from other modules like hci or mgmt: here we just change
> the call to get_random_bytes_wait, and return the result of the wait to
> the caller, along with the other error codes of those functions like
> usual. Then there's the SMP protocol handler itself, which makes many
> many many calls to get_random_bytes during different phases. For this,
> rather than have to change all the calls to get_random_bytes_wait and
> propagate the error result, it's actually enough to just put a single
> call to wait_for_random_bytes() at the beginning of the handler, to
> ensure that all the subsequent invocations are safe, without having to
> actually change them. Likewise, for the random address changing
> function, we'd rather know early on in the function whether the RNG
> initialization has been interrupted, rather than later, so we call
> wait_for_random_bytes() at the top, so that later on the call to
> get_random_bytes() is acceptable.

Do we need to do all of this?  Bluetooth folks, is it fair to assume
that hci_power_on() has to be called before any bluetooth functions
can be done?  If so, adding a wait_for_random_bytes() in
hci_power_on() might be all that is necessary.

- Ted

Re: [PATCH v4 11/13] net/route: use get_random_int for random counter

[Theodore Ts'o]

On Tue, Jun 06, 2017 at 07:48:02PM +0200, Jason A. Donenfeld wrote:
> Using get_random_int here is faster, more fitting of the use case, and
> just as cryptographically secure. It also has the benefit of providing
> better randomness at early boot, which is when many of these structures
> are assigned.
> 
> Also, semantically, it's not really proper to have been assigning an
> atomic_t in this way before, even if in practice it works fine.
> 
> Signed-off-by: Jason A. Donenfeld 
> Cc: David Miller 

Applied to the dev branch of the random.git branch, thanks.

- Ted

Re: [PATCH v4 10/13] net/neighbor: use get_random_u32 for 32-bit hash random

[Theodore Ts'o]

On Tue, Jun 06, 2017 at 07:48:01PM +0200, Jason A. Donenfeld wrote:
> Using get_random_u32 here is faster, more fitting of the use case, and
> just as cryptographically secure. It also has the benefit of providing
> better randomness at early boot, which is when many of these structures
> are assigned.
> 
> Signed-off-by: Jason A. Donenfeld 
> Cc: David Miller 

Applied to the random.git dev branch, thanks.

  - Ted

Re: [PATCH v4 09/13] rhashtable: use get_random_u32 for hash_rnd

[Theodore Ts'o]

On Tue, Jun 06, 2017 at 07:48:00PM +0200, Jason A. Donenfeld wrote:
> This is much faster and just as secure. It also has the added benefit of
> probably returning better randomness at early-boot on systems with
> architectural RNGs.
> 
> Signed-off-by: Jason A. Donenfeld 
> Cc: Thomas Graf 
> Cc: Herbert Xu 

Thanks, applied to the random.git dev branch.

  - Ted

Re: [kernel-hardening] [PATCH v4 07/13] ceph: ensure RNG is seeded before using

[Theodore Ts'o]

On Tue, Jun 06, 2017 at 07:47:58PM +0200, Jason A. Donenfeld wrote:
> Ceph uses the RNG for various nonce generations, and it shouldn't accept
> using bad randomness. So, we wait for the RNG to be properly seeded. We
> do this by calling wait_for_random_bytes() in a function that is
> certainly called in process context, early on, so that all subsequent
> calls to get_random_bytes are necessarily acceptable.
> 
> Signed-off-by: Jason A. Donenfeld 
> Cc: Ilya Dryomov 
> Cc: "Yan, Zheng" 
> Cc: Sage Weil 

Thanks, applied to the dev branch.

- Ted

Re: [PATCH v4 06/13] iscsi: ensure RNG is seeded before use

[Theodore Ts'o]

On Tue, Jun 06, 2017 at 07:47:57PM +0200, Jason A. Donenfeld wrote:
> It's not safe to use weak random data here, especially for the challenge
> response randomness. Since we're always in process context, it's safe to
> simply wait until we have enough randomness to carry out the
> authentication correctly.
> 
> While we're at it, we clean up a small memleak during an error
> condition.

What was the testing that was done for commit?  It looks safe, but I'm
unfamiliar enough with how the iSCSI authentication works that I'd
prefer getting an ack'ed by from the iSCSI maintainers or
alternativel, information about how to kick off some kind of automated
test suite ala xfstests for file systems.

Thanks,

- Ted

[PATCH v2 3/6] ima: Log the same audit cause whenever a file has no signature

[Thiago Jung Bauermann]

If the file doesn't have an xattr, ima_appraise_measurement sets cause to
"missing-hash" while if there's an xattr but it's a digest instead of a
signature it sets cause to "IMA-signature-required".

Fix it by setting cause to "IMA-signature-required" in both cases.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima_appraise.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_appraise.c 
b/security/integrity/ima/ima_appraise.c
index ea36a4f134f4..809ba70fbbbf 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -217,7 +217,8 @@ int ima_appraise_measurement(enum ima_hooks func,
if (rc && rc != -ENODATA)
goto out;
 
-   cause = "missing-hash";
+   cause = iint->flags & IMA_DIGSIG_REQUIRED ?
+   "IMA-signature-required" : "missing-hash";
status = INTEGRITY_NOLABEL;
if (opened & FILE_CREATED)
iint->flags |= IMA_NEW_FILE;
-- 
2.7.4

[PATCH v2 4/6] integrity: Introduce struct evm_hmac_xattr

[Thiago Jung Bauermann]

Even though struct evm_ima_xattr_data includes a fixed-size array to hold a
SHA1 digest, most of the code ignores the array and uses the struct to mean
"type indicator followed by data of unspecified size" and tracks the real
size of what the struct represents in a separate length variable.

The only exception to that is the EVM code, which correctly uses the
definition of struct evm_ima_xattr_data.

This patch makes this explicit in the code by removing the length
specification from the array in struct evm_ima_xattr_data. It also changes
the name of the element from digest to data, since in most places the array
doesn't hold a digest.

A separate struct evm_hmac_xattr is introduced, with the original
definition of evm_ima_xattr_data to be used in the places that actually
expect that definition.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/evm/evm.h  | 5 +
 security/integrity/evm/evm_crypto.c   | 2 +-
 security/integrity/evm/evm_main.c | 8 
 security/integrity/ima/ima_appraise.c | 7 ---
 security/integrity/integrity.h| 2 +-
 5 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index f5f12727771a..e1081cf2f9c5 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -24,6 +24,11 @@
 #define EVM_INIT_HMAC  0x0001
 #define EVM_INIT_X509  0x0002
 
+struct evm_hmac_xattr {
+   u8 type;/* Should be EVM_XATTR_HMAC. */
+   u8 digest[SHA1_DIGEST_SIZE];
+} __packed;
+
 extern int evm_initialized;
 extern char *evm_hmac;
 extern char *evm_hash;
diff --git a/security/integrity/evm/evm_crypto.c 
b/security/integrity/evm/evm_crypto.c
index d7f282d75cc1..08dde59f3128 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -252,7 +252,7 @@ int evm_update_evmxattr(struct dentry *dentry, const char 
*xattr_name,
const char *xattr_value, size_t xattr_value_len)
 {
struct inode *inode = d_backing_inode(dentry);
-   struct evm_ima_xattr_data xattr_data;
+   struct evm_hmac_xattr xattr_data;
int rc = 0;
 
rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
diff --git a/security/integrity/evm/evm_main.c 
b/security/integrity/evm/evm_main.c
index 063d38aef64e..b7c1e11a915e 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -116,7 +116,7 @@ static enum integrity_status evm_verify_hmac(struct dentry 
*dentry,
 struct integrity_iint_cache *iint)
 {
struct evm_ima_xattr_data *xattr_data = NULL;
-   struct evm_ima_xattr_data calc;
+   struct evm_hmac_xattr calc;
enum integrity_status evm_status = INTEGRITY_PASS;
int rc, xattr_len;
 
@@ -147,7 +147,7 @@ static enum integrity_status evm_verify_hmac(struct dentry 
*dentry,
/* check value type */
switch (xattr_data->type) {
case EVM_XATTR_HMAC:
-   if (xattr_len != sizeof(struct evm_ima_xattr_data)) {
+   if (xattr_len != sizeof(struct evm_hmac_xattr)) {
evm_status = INTEGRITY_FAIL;
goto out;
}
@@ -155,7 +155,7 @@ static enum integrity_status evm_verify_hmac(struct dentry 
*dentry,
   xattr_value_len, calc.digest);
if (rc)
break;
-   rc = crypto_memneq(xattr_data->digest, calc.digest,
+   rc = crypto_memneq(xattr_data->data, calc.digest,
sizeof(calc.digest));
if (rc)
rc = -EINVAL;
@@ -467,7 +467,7 @@ int evm_inode_init_security(struct inode *inode,
 const struct xattr *lsm_xattr,
 struct xattr *evm_xattr)
 {
-   struct evm_ima_xattr_data *xattr_data;
+   struct evm_hmac_xattr *xattr_data;
int rc;
 
if (!evm_initialized || !evm_protected_xattr(lsm_xattr->name))
diff --git a/security/integrity/ima/ima_appraise.c 
b/security/integrity/ima/ima_appraise.c
index 809ba70fbbbf..87d2b601cf8e 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -156,7 +156,8 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data 
*xattr_value,
return sig->hash_algo;
break;
case IMA_XATTR_DIGEST_NG:
-   ret = xattr_value->digest[0];
+   /* first byte contains algorithm id */
+   ret = xattr_value->data[0];
if (ret < HASH_ALGO__LAST)
return ret;
break;
@@ -164,7 +165,7 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data 
*xattr_value,
/* this is for backward compatibility */
if (xattr_len == 21) {
unsigned int zero = 0;

[PATCH v2 6/6] ima: Support module-style appended signatures for appraisal

[Thiago Jung Bauermann]

This patch introduces the modsig keyword to the IMA policy syntax to
specify that a given hook should expect the file to have the IMA signature
appended to it. Here is how it can be used in a rule:

appraise func=KEXEC_KERNEL_CHECK appraise_type=modsig|imasig

With this rule, IMA will accept either an appended signature or a signature
stored in the extended attribute. In that case, it will first check whether
there is an appended signature, and if not it will read it from the
extended attribute.

The format of the appended signature is the same used for signed kernel
modules. This means that the file can be signed with the scripts/sign-file
tool, with a command line such as this:

$ sign-file sha256 privkey_ima.pem x509_ima.der vmlinux

This code only works for files that are hashed from a memory buffer, not
for files that are read from disk at the time of hash calculation. In other
words, only hooks that use kernel_read_file can support appended
signatures. This means that only FIRMWARE_CHECK, KEXEC_KERNEL_CHECK,
KEXEC_INITRAMFS_CHECK and POLICY_CHECK can be supported.

This feature warrants a separate config option because it depends on many
other config options:

 ASYMMETRIC_KEY_TYPE n -> y
 CRYPTO_RSA n -> y
 INTEGRITY_SIGNATURE n -> y
 MODULE_SIG_FORMAT n -> y
 SYSTEM_DATA_VERIFICATION n -> y
+ASN1 y
+ASYMMETRIC_PUBLIC_KEY_SUBTYPE y
+CLZ_TAB y
+CRYPTO_AKCIPHER y
+IMA_APPRAISE_MODSIG y
+IMA_TRUSTED_KEYRING n
+INTEGRITY_ASYMMETRIC_KEYS y
+INTEGRITY_TRUSTED_KEYRING n
+MPILIB y
+OID_REGISTRY y
+PKCS7_MESSAGE_PARSER y
+PKCS7_TEST_KEY n
+SECONDARY_TRUSTED_KEYRING n
+SIGNATURE y
+SIGNED_PE_FILE_VERIFICATION n
+SYSTEM_EXTRA_CERTIFICATE n
+SYSTEM_TRUSTED_KEYRING y
+SYSTEM_TRUSTED_KEYS ""
+X509_CERTIFICATE_PARSER y

The change in CONFIG_INTEGRITY_SIGNATURE to select CONFIG_KEYS instead of
depending on it is to avoid a dependency recursion in
CONFIG_IMA_APPRAISE_MODSIG, because CONFIG_MODULE_SIG_FORMAT selects
CONFIG_KEYS and Kconfig complains that CONFIG_INTEGRITY_SIGNATURE depends
on it.

Signed-off-by: Thiago Jung Bauermann 
---
 crypto/asymmetric_keys/pkcs7_parser.c |  12 +++
 include/crypto/pkcs7.h|   3 +
 security/integrity/Kconfig|   2 +-
 security/integrity/digsig.c   |  28 +++--
 security/integrity/ima/Kconfig|  13 +++
 security/integrity/ima/Makefile   |   1 +
 security/integrity/ima/ima.h  |  53 ++
 security/integrity/ima/ima_api.c  |   2 +-
 security/integrity/ima/ima_appraise.c |  41 ++--
 security/integrity/ima/ima_main.c |  91 
 security/integrity/ima/ima_modsig.c   | 167 ++
 security/integrity/ima/ima_policy.c   |  26 +++--
 security/integrity/ima/ima_template_lib.c |  14 ++-
 security/integrity/integrity.h|   5 +-
 14 files changed, 404 insertions(+), 54 deletions(-)

diff --git a/crypto/asymmetric_keys/pkcs7_parser.c 
b/crypto/asymmetric_keys/pkcs7_parser.c
index af4cd8649117..e41beda297a8 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -673,3 +673,15 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
return -ENOMEM;
return 0;
 }
+
+/**
+ * pkcs7_get_message_sig - get signature in @pkcs7
+ *
+ * This function doesn't meaningfully support messages with more than one
+ * signature. It will always return the first signature.
+ */
+const struct public_key_signature *pkcs7_get_message_sig(
+   const struct pkcs7_message *pkcs7)
+{
+   return pkcs7->signed_infos ? pkcs7->signed_infos->sig : NULL;
+}
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 583f199400a3..a05a0d7214e6 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -29,6 +29,9 @@ extern int pkcs7_get_content_data(const struct pkcs7_message 
*pkcs7,
  const void **_data, size_t *_datalen,
  size_t *_headerlen);
 
+extern const struct public_key_signature *pkcs7_get_message_sig(
+   const struct pkcs7_message *pkcs7);
+
 /*
  * pkcs7_trust.c
  */
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index da9565891738..0d642e0317c7 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -17,8 +17,8 @@ if INTEGRITY
 
 config INTEGRITY_SIGNATURE
bool "Digital signature verification using multiple keyrings"
-   depends on KEYS
default n
+   select KEYS
select SIGNATURE
help
  This option enables digital signature verification support
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 06554c448dce..9190c9058f4f 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -48,11 +48,10 @@ static bool init_keyring __initdata;
 #define restrict_link_to_ima 

[PATCH v2 5/6] MODSIGN: Export module signature definitions.

[Thiago Jung Bauermann]

IMA will use the module_signature format for append signatures, so export
the relevant definitions and factor out the code which verifies that the
appended signature trailer is valid.

Also, add function verify_pkcs7_message_signature which takes a struct
pkcs7_message for verification isntead of the raw bytes that
verify_pkcs7_signature takes.

Finally, create a CONFIG_MODULE_SIG_FORMAT option so that IMA can select it
and be able to use validate_module_signature without having to depend on
CONFIG_MODULE_SIG.

Signed-off-by: Thiago Jung Bauermann 
---
 certs/system_keyring.c   | 62 -
 include/linux/module.h   |  3 --
 include/linux/module_signature.h | 48 ++
 include/linux/verification.h | 10 ++
 init/Kconfig |  6 +++-
 kernel/Makefile  |  2 +-
 kernel/module.c  |  1 +
 kernel/module_signing.c  | 74 +---
 8 files changed, 142 insertions(+), 64 deletions(-)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 6251d1b27f0c..0597f87a2375 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -190,33 +190,28 @@ late_initcall(load_system_certificate_list);
 #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
 
 /**
- * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
+ * verify_pkcs7_message_signature - Verify 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.
+ * @pkcs7: The PKCS#7 message that is the signature.
  * @trusted_keys: Trusted keys to use (NULL for builtin trusted keys only,
  * (void *)1UL for all trusted keys).
  * @usage: The use to which the key is being put.
  * @view_content: Callback to gain access to content.
  * @ctx: Context for callback.
  */
-int verify_pkcs7_signature(const void *data, size_t len,
-  const void *raw_pkcs7, size_t pkcs7_len,
-  struct key *trusted_keys,
-  enum key_being_used_for usage,
-  int (*view_content)(void *ctx,
-  const void *data, size_t len,
-  size_t asn1hdrlen),
-  void *ctx)
+int verify_pkcs7_message_signature(const void *data, size_t len,
+  struct pkcs7_message *pkcs7,
+  struct key *trusted_keys,
+  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;
int ret;
 
-   pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
-   if (IS_ERR(pkcs7))
-   return PTR_ERR(pkcs7);
-
/* The data should be detached - so we need to supply it. */
if (data && pkcs7_supply_detached_data(pkcs7, data, len) < 0) {
pr_err("PKCS#7 signature with non-detached data\n");
@@ -258,6 +253,41 @@ int verify_pkcs7_signature(const void *data, size_t len,
}
 
 error:
+   pr_devel("<==%s() = %d\n", __func__, ret);
+   return ret;
+}
+
+/**
+ * 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 builtin trusted keys only,
+ * (void *)1UL for all trusted keys).
+ * @usage: The use to which the key is being put.
+ * @view_content: Callback to gain access to content.
+ * @ctx: Context for callback.
+ */
+int verify_pkcs7_signature(const void *data, size_t len,
+  const void *raw_pkcs7, size_t pkcs7_len,
+  struct key *trusted_keys,
+  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;
+   int ret;
+
+   pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
+   if (IS_ERR(pkcs7))
+   return PTR_ERR(pkcs7);
+
+   ret = verify_pkcs7_message_signature(data, len, pkcs7, trusted_keys,
+

[PATCH v2 2/6] ima: Simplify policy_func_show.

[Thiago Jung Bauermann]

If the func_tokens array uses the same indices as enum ima_hooks,
policy_func_show can be a lot simpler, and the func_* enum becomes
unnecessary.

Also, if we use the same macro trick used by kernel_read_file_id_str we can
use one hooks list for both the enum and the string array, making sure they
are always in sync (suggested by Mimi Zohar).

Finally, by using the printf pattern for the function token directly
instead of using the pt macro we can simplify policy_func_show even further
and avoid needing a temporary buffer.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/ima/ima.h| 25 +---
 security/integrity/ima/ima_policy.c | 58 -
 2 files changed, 21 insertions(+), 62 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 215a93c41b51..d52b487ad259 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -172,17 +172,22 @@ static inline unsigned long ima_hash_key(u8 *digest)
return hash_long(*digest, IMA_HASH_BITS);
 }
 
+#define __ima_hooks(hook)  \
+   hook(NONE)  \
+   hook(FILE_CHECK)\
+   hook(MMAP_CHECK)\
+   hook(BPRM_CHECK)\
+   hook(POST_SETATTR)  \
+   hook(MODULE_CHECK)  \
+   hook(FIRMWARE_CHECK)\
+   hook(KEXEC_KERNEL_CHECK)\
+   hook(KEXEC_INITRAMFS_CHECK) \
+   hook(POLICY_CHECK)  \
+   hook(MAX_CHECK)
+#define __ima_hook_enumify(ENUM)   ENUM,
+
 enum ima_hooks {
-   FILE_CHECK = 1,
-   MMAP_CHECK,
-   BPRM_CHECK,
-   POST_SETATTR,
-   MODULE_CHECK,
-   FIRMWARE_CHECK,
-   KEXEC_KERNEL_CHECK,
-   KEXEC_INITRAMFS_CHECK,
-   POLICY_CHECK,
-   MAX_CHECK
+   __ima_hooks(__ima_hook_enumify)
 };
 
 /* LIM API function definitions */
diff --git a/security/integrity/ima/ima_policy.c 
b/security/integrity/ima/ima_policy.c
index 949ad3858327..f4436626ccb7 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -972,23 +972,10 @@ static const char *const mask_tokens[] = {
"MAY_APPEND"
 };
 
-enum {
-   func_file = 0, func_mmap, func_bprm,
-   func_module, func_firmware, func_post,
-   func_kexec_kernel, func_kexec_initramfs,
-   func_policy
-};
+#define __ima_hook_stringify(str)  (#str),
 
 static const char *const func_tokens[] = {
-   "FILE_CHECK",
-   "MMAP_CHECK",
-   "BPRM_CHECK",
-   "MODULE_CHECK",
-   "FIRMWARE_CHECK",
-   "POST_SETATTR",
-   "KEXEC_KERNEL_CHECK",
-   "KEXEC_INITRAMFS_CHECK",
-   "POLICY_CHECK"
+   __ima_hooks(__ima_hook_stringify)
 };
 
 void *ima_policy_start(struct seq_file *m, loff_t *pos)
@@ -1025,49 +1012,16 @@ void ima_policy_stop(struct seq_file *m, void *v)
 
 #define pt(token)  policy_tokens[token + Opt_err].pattern
 #define mt(token)  mask_tokens[token]
-#define ft(token)  func_tokens[token]
 
 /*
  * policy_func_show - display the ima_hooks policy rule
  */
 static void policy_func_show(struct seq_file *m, enum ima_hooks func)
 {
-   char tbuf[64] = {0,};
-
-   switch (func) {
-   case FILE_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_file));
-   break;
-   case MMAP_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_mmap));
-   break;
-   case BPRM_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_bprm));
-   break;
-   case MODULE_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_module));
-   break;
-   case FIRMWARE_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_firmware));
-   break;
-   case POST_SETATTR:
-   seq_printf(m, pt(Opt_func), ft(func_post));
-   break;
-   case KEXEC_KERNEL_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_kexec_kernel));
-   break;
-   case KEXEC_INITRAMFS_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_kexec_initramfs));
-   break;
-   case POLICY_CHECK:
-   seq_printf(m, pt(Opt_func), ft(func_policy));
-   break;
-   default:
-   snprintf(tbuf, sizeof(tbuf), "%d", func);
-   seq_printf(m, pt(Opt_func), tbuf);
-   break;
-   }
-   seq_puts(m, " ");
+   if (func > 0 && func < MAX_CHECK)
+   seq_printf(m, "func=%s ", func_tokens[func]);
+   else
+   seq_printf(m, "func=%d ", func);
 }
 
 int ima_policy_show(struct seq_file *m, void *v)
-- 
2.7.4

[PATCH v2 0/6] Appended signatures support for IMA appraisal

[Thiago Jung Bauermann]

On the OpenPOWER platform, secure boot and trusted boot are being
implemented using IMA for taking measurements and verifying signatures.
Since the kernel image on Power servers is an ELF binary, kernels are
signed using the scripts/sign-file tool and thus use the same signature
format as signed kernel modules.

This patch series adds support in IMA for verifying those signatures.
It adds flexibility to OpenPOWER secure boot, because it allows it to boot
kernels with the signature appended to them as well as kernels where the
signature is stored in the IMA extended attribute.

The first four patches are cleanups and improvements that can be taken
independently from the others (and from each other as well). The last two
are the ones actually focused on this feature.

Since modsig is only supported on some specific hooks which don't get
called often (cf. ima_hook_supports_modsig), it's possible to always check
for the presence of an appended modsig before looking for the xattr sig. In
that case, the policy doesn't need to be changed to support the modsig
keyword. Is that preferable than requiring the policy to explicitly allow a
modsig like this code does?

I tested these patches with EVM and I believe they don't break it and
things work as expected, but I'm not really familiar with EVM and its use
cases so this should be taken with a grain of salt.

I also verified that the code correctly recalculates the file hash if the
modsig verification fails and the file also has an xattr signature which
uses a different hash algorithm.

These patches apply on top of today's linux-integrity/next.

Changes since v1:
- Patch "integrity: Small code improvements"
  - Add missing #endif comment in ima.h.

- Patch "ima: Tidy up constant strings"
  - Squashed into previous patch.

- Patch "ima: Simplify policy_func_show."
  - Generate ima_hooks enum and func_tokens array from a single macro.
(suggested by Mimi)
  - Further simplify policy_func_show by not using the printf format string
from the policy_tokens table.

- Patch "integrity: Introduce struct evm_hmac_xattr"
  - New patch.

- Patch "MODSIGN: Export module signature definitions."
  - Add function verify_pkcs7_message_signature which takes a
struct pkcs7_message.
  - Move MODULE_SIG_STRING definition from  to
.

- Patch "ima: Support appended signatures for appraisal"
  - Changed name from appended_sig to modsig. (suggested by Mehmet Kayaalp)
  - Don't add key_being_used_for value VERIFYING_KEXEC_CMS_SIGNATURE. Use
existing VERIFYING_MODULE_SIGNATURE. (suggested by Mimi)
  - Moved modsig code to its own file. (suggested by Mimi)
  - Added new xattr "subtype" IMA_MODSIG. (suggested by Mimi)
  - Check whether a hook supports modsig when the policy is being parsed.
(suggested by Mimi)
  - If the modsig verification fails, look for an xattr signature.
(suggested by Mimi)
  - Add integrity_keyring_from_id function.
  - Put modsig to measurement list if the template requires the signature
contents. (suggested by Mimi).

Thiago Jung Bauermann (6):
  integrity: Small code improvements
  ima: Simplify policy_func_show.
  ima: Log the same audit cause whenever a file has no signature
  integrity: Introduce struct evm_hmac_xattr
  MODSIGN: Export module signature definitions.
  ima: Support module-style appended signatures for appraisal

 certs/system_keyring.c|  62 ---
 crypto/asymmetric_keys/pkcs7_parser.c |  12 +++
 include/crypto/pkcs7.h|   3 +
 include/linux/module.h|   3 -
 include/linux/module_signature.h  |  48 +
 include/linux/verification.h  |  10 ++
 init/Kconfig  |   6 +-
 kernel/Makefile   |   2 +-
 kernel/module.c   |   1 +
 kernel/module_signing.c   |  74 ++---
 security/integrity/Kconfig|   2 +-
 security/integrity/digsig.c   |  28 +++--
 security/integrity/digsig_asymmetric.c|   4 +-
 security/integrity/evm/evm.h  |   5 +
 security/integrity/evm/evm_crypto.c   |   2 +-
 security/integrity/evm/evm_main.c |   8 +-
 security/integrity/iint.c |   2 +-
 security/integrity/ima/Kconfig|  13 +++
 security/integrity/ima/Makefile   |   1 +
 security/integrity/ima/ima.h  |  78 --
 security/integrity/ima/ima_api.c  |   2 +-
 security/integrity/ima/ima_appraise.c |  52 +++---
 security/integrity/ima/ima_main.c |  91 
 security/integrity/ima/ima_modsig.c   | 167 ++
 security/integrity/ima/ima_policy.c   |  82 ---
 security/integrity/ima/ima_template_lib.c |  14 ++-
 security/integrity/integrity.h|  14 ++-
 27 files changed, 591 insertions(+), 195 deletions(-)
 create mode 100644 include/linux/module_signature.h
 create mode 100644 

[PATCH v2 1/6] integrity: Small code improvements

[Thiago Jung Bauermann]

These changes are too small to warrant their own patches:

The keyid and sig_size members of struct signature_v2_hdr are in BE format,
so use a type that makes this assumption explicit. Also, use beXX_to_cpu
instead of __beXX_to_cpu to read them.

Change integrity_kernel_read to take a void * buffer instead of char *
buffer, so that callers don't have to use a cast if they provide a buffer
that isn't a char *.

Add missing #endif comment in ima.h pointing out which macro it refers to.

Add missing fall through comment in ima_appraise.c.

Constify mask_tokens and func_tokens arrays.

Signed-off-by: Thiago Jung Bauermann 
---
 security/integrity/digsig_asymmetric.c | 4 ++--
 security/integrity/iint.c  | 2 +-
 security/integrity/ima/ima.h   | 2 +-
 security/integrity/ima/ima_appraise.c  | 1 +
 security/integrity/ima/ima_policy.c| 4 ++--
 security/integrity/integrity.h | 7 ---
 6 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/security/integrity/digsig_asymmetric.c 
b/security/integrity/digsig_asymmetric.c
index 80052ed8d467..ab6a029062a1 100644
--- a/security/integrity/digsig_asymmetric.c
+++ b/security/integrity/digsig_asymmetric.c
@@ -92,13 +92,13 @@ int asymmetric_verify(struct key *keyring, const char *sig,
 
siglen -= sizeof(*hdr);
 
-   if (siglen != __be16_to_cpu(hdr->sig_size))
+   if (siglen != be16_to_cpu(hdr->sig_size))
return -EBADMSG;
 
if (hdr->hash_algo >= HASH_ALGO__LAST)
return -ENOPKG;
 
-   key = request_asymmetric_key(keyring, __be32_to_cpu(hdr->keyid));
+   key = request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
if (IS_ERR(key))
return PTR_ERR(key);
 
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index c710d22042f9..6fc888ca468e 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -182,7 +182,7 @@ security_initcall(integrity_iintcache_init);
  *
  */
 int integrity_kernel_read(struct file *file, loff_t offset,
- char *addr, unsigned long count)
+ void *addr, unsigned long count)
 {
mm_segment_t old_fs;
char __user *buf = (char __user *)addr;
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d26a30e37d13..215a93c41b51 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -284,7 +284,7 @@ static inline int ima_read_xattr(struct dentry *dentry,
return 0;
 }
 
-#endif
+#endif /* CONFIG_IMA_APPRAISE */
 
 /* LSM based policy rules require audit */
 #ifdef CONFIG_IMA_LSM_RULES
diff --git a/security/integrity/ima/ima_appraise.c 
b/security/integrity/ima/ima_appraise.c
index 7fe0566142d8..ea36a4f134f4 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -240,6 +240,7 @@ int ima_appraise_measurement(enum ima_hooks func,
case IMA_XATTR_DIGEST_NG:
/* first byte contains algorithm id */
hash_start = 1;
+   /* fall through */
case IMA_XATTR_DIGEST:
if (iint->flags & IMA_DIGSIG_REQUIRED) {
cause = "IMA-signature-required";
diff --git a/security/integrity/ima/ima_policy.c 
b/security/integrity/ima/ima_policy.c
index 0acd68decb17..949ad3858327 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -965,7 +965,7 @@ enum {
mask_exec = 0, mask_write, mask_read, mask_append
 };
 
-static char *mask_tokens[] = {
+static const char *const mask_tokens[] = {
"MAY_EXEC",
"MAY_WRITE",
"MAY_READ",
@@ -979,7 +979,7 @@ enum {
func_policy
 };
 
-static char *func_tokens[] = {
+static const char *const func_tokens[] = {
"FILE_CHECK",
"MMAP_CHECK",
"BPRM_CHECK",
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 24520b4ef3b0..a53e7e4ab06c 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -92,8 +92,8 @@ struct signature_v2_hdr {
uint8_t type;   /* xattr type */
uint8_t version;/* signature format version */
uint8_t hash_algo;  /* Digest algorithm [enum hash_algo] */
-   uint32_t keyid; /* IMA key identifier - not X509/PGP specific */
-   uint16_t sig_size;  /* signature size */
+   __be32 keyid;   /* IMA key identifier - not X509/PGP specific */
+   __be16 sig_size;/* signature size */
uint8_t sig[0]; /* signature payload */
 } __packed;
 
@@ -118,7 +118,8 @@ struct integrity_iint_cache {
 struct integrity_iint_cache *integrity_iint_find(struct inode *inode);
 
 int integrity_kernel_read(struct file *file, loff_t offset,
- char *addr, unsigned long count);
+ void *addr, unsigned long count);
+
 int __init 

Re: [PATCH v4 04/13] security/keys: ensure RNG is seeded before use

[Jason A. Donenfeld]

On Thu, Jun 8, 2017 at 2:50 AM, Jason A. Donenfeld  wrote:
> On Thu, Jun 8, 2017 at 2:31 AM, Theodore Ts'o  wrote:
>> I'm guessing you changed key_alloc_serial() to return an int back when
>> you were thinking that you might use get_random_bytes_wait(), which
>> could return -ERESTARTSYS.
>>
>> Now that you're not doing this, but using get_random_u32() instead,
>> there's no point to change the function signature of
>> key_alloc_serial() and add an error check in key_alloc() that will
>> never fail, right?  That's just adding a dead code path.  Which the
>> compiler can probably optimize away, but why make the code slightly
>> harder to read than necessasry?
>
> Good catch, and thanks for reading these so thoroughly that you caught
> the churn artifacts. Do you want me to clean this up and resubmit, or
> are you planning on adjusting it in the dev branch?

Fixed it up here if you just want to grab this instead:

https://git.kernel.org/pub/scm/linux/kernel/git/zx2c4/linux.git/patch/?id=a0361e55bce30ace529ed8b28bd452e3ac0ee91f

Re: [PATCH v4 01/13] random: invalidate batched entropy after crng init

[Jason A. Donenfeld]

On Thu, Jun 8, 2017 at 1:58 AM, Theodore Ts'o  wrote:
> Thanks, applied.  This will be on the for_stable that I will be
> sending to Linus sometime during 4.12-rcX.

I think you might have just missed the kbuild test robot complaining
about an incorrect compiler warning, when using an ancient gcc for the
sh platform. I fixed that bug in v5, which I posted about an hour
ago. The only change was a single commit, so you can just cherry-pick
that as you wish:

https://git.kernel.org/pub/scm/linux/kernel/git/zx2c4/linux.git/patch/?id=2f390fd961d1f97738d7af4f4797542e05f4607e

Other than that one commit change, it was the same.

Or, if it's easier, the tag for-random-4.12 in
git://git.kernel.org/pub/scm/linux/kernel/git/tytso/random.git is for you.

Re: [PATCH v4 04/13] security/keys: ensure RNG is seeded before use

[Jason A. Donenfeld]

On Thu, Jun 8, 2017 at 2:31 AM, Theodore Ts'o  wrote:
> I'm guessing you changed key_alloc_serial() to return an int back when
> you were thinking that you might use get_random_bytes_wait(), which
> could return -ERESTARTSYS.
>
> Now that you're not doing this, but using get_random_u32() instead,
> there's no point to change the function signature of
> key_alloc_serial() and add an error check in key_alloc() that will
> never fail, right?  That's just adding a dead code path.  Which the
> compiler can probably optimize away, but why make the code slightly
> harder to read than necessasry?

Good catch, and thanks for reading these so thoroughly that you caught
the churn artifacts. Do you want me to clean this up and resubmit, or
are you planning on adjusting it in the dev branch?

Re: [kernel-hardening] [PATCH v4 05/13] crypto/rng: ensure that the RNG is ready before using

[Jason A. Donenfeld]

On Thu, Jun 8, 2017 at 2:41 AM, Theodore Ts'o  wrote:
> The use in keys/big_key is _being_ removed, so this commit is
> dependent on that commit landing, correct?  (Order matters, because
> otherwise we don't want to potentially screw up doing a kernel bisect
> and causing their kernel to deadlock during the boot while they are
> trying to track down an unreleated problem.)

Yes. It's actually landing with get_random_bytes, to avoid a
dependency problem when merging. After these both lands, I'll submit a
third changing that over to get_random_bytes_wait in the right place.

Re: [kernel-hardening] [PATCH v4 05/13] crypto/rng: ensure that the RNG is ready before using

[Theodore Ts'o]

On Tue, Jun 06, 2017 at 07:47:56PM +0200, Jason A. Donenfeld wrote:
> Otherwise, we might be seeding the RNG using bad randomness, which is
> dangerous. The one use of this function from within the kernel -- not
> from userspace -- is being removed (keys/big_key), so that call site
> isn't relevant in assessing this.

The use in keys/big_key is _being_ removed, so this commit is
dependent on that commit landing, correct?  (Order matters, because
otherwise we don't want to potentially screw up doing a kernel bisect
and causing their kernel to deadlock during the boot while they are
trying to track down an unreleated problem.)

   - Ted

Re: [kernel-hardening] [PATCH v4 08/13] cifs: use get_random_u32 for 32-bit lock random

[Jason A. Donenfeld]

On Thu, Jun 8, 2017 at 2:25 AM, Theodore Ts'o  wrote:
> There's a bigger problem here, which is that cifs_lock_secret is a
> 32-bit value which is being used to obscure flock->fl_owner before it
> is sent across the wire.  But flock->fl_owner is a pointer to the
> struct file *, so 64-bit architecture, the high 64-bits of a kernel
> pointer is being exposed to anyone using tcpdump.  (Oops, I'm showing
> my age; I guess all the cool kids are using Wireshark these days.)
>
> Worse, the obscuring is being done using XOR.  How an active attacker
> might be able to trivially reverse engineer the 32-bit "secret" is
> left as an exercise to the reader.  The bottom line is if the goal is
> to hide the memory location of a struct file from an attacker,
> cifs_lock_secret is about as useful as a TSA agent doing security
> theatre at an airport.  Which is to say, it makes the civilians feel
> good.  :-)

High five for taking the deep dive and actually reading how this all
works. Nice bug!

> Not waiting
> for the CRNG to be fully initialized is the *least* of its problems.

The kernel is vast and filled with tons of bugs of many sorts. On this
reasoning, maybe I should spend my time auditing web apps instead,
which are usually the "front door" of bugs? I like the puzzles of
random.c. I also had a real world need for wait_for_random_bytes() in
a module I'm writing.

But anyway, your general point is a really good one. Tons of callers
of the random functions are doing it wrong in one way or another.
Spending time looking at those is probably a good idea...

> Anyway, I'll include this commit in the dev branch of the random tree,
> since it's not going to make things worse.

Great, thanks.

Re: [kernel-hardening] [PATCH v4 08/13] cifs: use get_random_u32 for 32-bit lock random

[Jason A. Donenfeld]

On Thu, Jun 8, 2017 at 2:25 AM, Theodore Ts'o  wrote:
> There's a bigger problem here, which is that cifs_lock_secret is a
> 32-bit value which is being used to obscure flock->fl_owner before it
> is sent across the wire.  But flock->fl_owner is a pointer to the
> struct file *, so 64-bit architecture, the high 64-bits of a kernel
> pointer is being exposed to anyone using tcpdump.  (Oops, I'm showing
> my age; I guess all the cool kids are using Wireshark these days.)
>
> Worse, the obscuring is being done using XOR.  How an active attacker
> might be able to trivially reverse engineer the 32-bit "secret" is
> left as an exercise to the reader.  The bottom line is if the goal is
> to hide the memory location of a struct file from an attacker,
> cifs_lock_secret is about as useful as a TSA agent doing security
> theatre at an airport.  Which is to say, it makes the civilians feel
> good.  :-)

High five for taking the deep dive and actually reading how this all
works. Nice bug!

> Not waiting
> for the CRNG to be fully initialized is the *least* of its problems.

The kernel is vast and filled with tons of bugs of many sorts. On this
reasoning, maybe I should spend my time auditing web apps instead,
which are usually the "front door" of bugs? I like the puzzles of
random.c. I also had a real world need for wait_for_random_bytes() in
a module I'm writing.

But anyway, your general point is a really good one. Tons of callers
of the random functions are doing it wrong in one way or another.
Spending time looking at those is probably a good idea...

> Anyway, I'll include this commit in the dev branch of the random tree,
> since it's not going to make things worse.

Great, thanks.

Re: [PATCH v4 04/13] security/keys: ensure RNG is seeded before use

[Theodore Ts'o]

On Tue, Jun 06, 2017 at 07:47:55PM +0200, Jason A. Donenfeld wrote:
> -static inline void key_alloc_serial(struct key *key)
> +static inline int key_alloc_serial(struct key *key)

> @@ -170,7 +168,7 @@ static inline void key_alloc_serial(struct key *key)
>   rb_insert_color(>serial_node, _serial_tree);
>  
>   spin_unlock(_serial_lock);
> - return;
> + return 0;
>  
>   /* we found a key with the proposed serial number - walk the tree from
>* that point looking for the next unused serial number */

> @@ -314,7 +312,9 @@ struct key *key_alloc(struct key_type *type, const char 
> *desc,
>  
>   /* publish the key by giving it a serial number */
>   atomic_inc(>nkeys);
> - key_alloc_serial(key);
> + ret = key_alloc_serial(key);
> + if (ret < 0)
> + goto security_error;
>  
>  error:
>   return key;

I'm guessing you changed key_alloc_serial() to return an int back when
you were thinking that you might use get_random_bytes_wait(), which
could return -ERESTARTSYS.

Now that you're not doing this, but using get_random_u32() instead,
there's no point to change the function signature of
key_alloc_serial() and add an error check in key_alloc() that will
never fail, right?  That's just adding a dead code path.  Which the
compiler can probably optimize away, but why make the code slightly
harder to read than necessasry?

- Ted

Re: [kernel-hardening] [PATCH v4 08/13] cifs: use get_random_u32 for 32-bit lock random

[Theodore Ts'o]

On Tue, Jun 06, 2017 at 07:47:59PM +0200, Jason A. Donenfeld wrote:
> Using get_random_u32 here is faster, more fitting of the use case, and
> just as cryptographically secure. It also has the benefit of providing
> better randomness at early boot, which is sometimes when this is used.
> 
> Signed-off-by: Jason A. Donenfeld 
> Cc: Steve French 

There's a bigger problem here, which is that cifs_lock_secret is a
32-bit value which is being used to obscure flock->fl_owner before it
is sent across the wire.  But flock->fl_owner is a pointer to the
struct file *, so 64-bit architecture, the high 64-bits of a kernel
pointer is being exposed to anyone using tcpdump.  (Oops, I'm showing
my age; I guess all the cool kids are using Wireshark these days.)

Worse, the obscuring is being done using XOR.  How an active attacker
might be able to trivially reverse engineer the 32-bit "secret" is
left as an exercise to the reader.  The bottom line is if the goal is
to hide the memory location of a struct file from an attacker,
cifs_lock_secret is about as useful as a TSA agent doing security
theatre at an airport.  Which is to say, it makes the civilians feel
good.  :-)

BTW, Jason, this is why it's *good* to audit all of the uses of
get_random_bytes().  It only took me about 30 seconds in the first
patch in your series that changes a caller of get_random_bytes(), and
look what I was able to find by just taking a quick look.  Not waiting
for the CRNG to be fully initialized is the *least* of its problems.

Anyway, I'll include this commit in the dev branch of the random tree,
since it's not going to make things worse.

- Ted

Re: [kernel-hardening] [PATCH v4 03/13] random: add get_random_{bytes,u32,u64,int,long,once}_wait family

[Theodore Ts'o]

On Tue, Jun 06, 2017 at 07:47:54PM +0200, Jason A. Donenfeld wrote:
> These functions are simple convenience wrappers that call
> wait_for_random_bytes before calling the respective get_random_*
> function.
> 
> Signed-off-by: Jason A. Donenfeld 

Thanks, applied to the dev branch.

- Ted

Re: [PATCH v4 02/13] random: add synchronous API for the urandom pool

[Theodore Ts'o]

On Tue, Jun 06, 2017 at 07:47:53PM +0200, Jason A. Donenfeld wrote:
> This enables users of get_random_{bytes,u32,u64,int,long} to wait until
> the pool is ready before using this function, in case they actually want
> to have reliable randomness.
> 
> Signed-off-by: Jason A. Donenfeld 

Thanks, applied for the dev branch of random.git.  (I changed the
patch summary slightly; it now reads: "random: add
wait_for_random_bytes() API").

- Ted

Re: [PATCH v4 01/13] random: invalidate batched entropy after crng init

[Theodore Ts'o]

On Tue, Jun 06, 2017 at 07:47:52PM +0200, Jason A. Donenfeld wrote:
> It's possible that get_random_{u32,u64} is used before the crng has
> initialized, in which case, its output might not be cryptographically
> secure. For this problem, directly, this patch set is introducing the
> *_wait variety of functions, but even with that, there's a subtle issue:
> what happens to our batched entropy that was generated before
> initialization. Prior to this commit, it'd stick around, supplying bad
> numbers. After this commit, we force the entropy to be re-extracted
> after each phase of the crng has initialized.
> 
> In order to avoid a race condition with the position counter, we
> introduce a simple rwlock for this invalidation. Since it's only during
> this awkward transition period, after things are all set up, we stop
> using it, so that it doesn't have an impact on performance.
> 
> This should probably be backported to 4.11.
> 
> (Also: adding my copyright to the top. With the patch series from
> January, this patch, and then the ones that come after, I think there's
> a relevant amount of code in here to add my name to the top.)
> 
> Signed-off-by: Jason A. Donenfeld 
> Cc: Greg Kroah-Hartman 

Thanks, applied.  This will be on the for_stable that I will be
sending to Linus sometime during 4.12-rcX.

- Ted

[PATCH v5 02/13] random: add synchronous API for the urandom pool

[Jason A. Donenfeld]

This enables users of get_random_{bytes,u32,u64,int,long} to wait until
the pool is ready before using this function, in case they actually want
to have reliable randomness.

Signed-off-by: Jason A. Donenfeld 
---
 drivers/char/random.c  | 41 +++--
 include/linux/random.h |  1 +
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index d35da1603e12..77587ac1ecb2 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -851,11 +851,6 @@ static void crng_reseed(struct crng_state *crng, struct 
entropy_store *r)
spin_unlock_irqrestore(_crng.lock, flags);
 }
 
-static inline void crng_wait_ready(void)
-{
-   wait_event_interruptible(crng_init_wait, crng_ready());
-}
-
 static void _extract_crng(struct crng_state *crng,
  __u8 out[CHACHA20_BLOCK_SIZE])
 {
@@ -1477,7 +1472,10 @@ static ssize_t extract_entropy_user(struct entropy_store 
*r, void __user *buf,
  * number of good random numbers, suitable for key generation, seeding
  * TCP sequence numbers, etc.  It does not rely on the hardware random
  * number generator.  For random bytes direct from the hardware RNG
- * (when available), use get_random_bytes_arch().
+ * (when available), use get_random_bytes_arch(). In order to ensure
+ * that the randomness provided by this function is okay, the function
+ * wait_for_random_bytes() should be called and return 0 at least once
+ * at any point prior.
  */
 void get_random_bytes(void *buf, int nbytes)
 {
@@ -1507,6 +1505,24 @@ void get_random_bytes(void *buf, int nbytes)
 EXPORT_SYMBOL(get_random_bytes);
 
 /*
+ * Wait for the urandom pool to be seeded and thus guaranteed to supply
+ * cryptographically secure random numbers. This applies to: the /dev/urandom
+ * device, the get_random_bytes function, and the get_random_{u32,u64,int,long}
+ * family of functions. Using any of these functions without first calling
+ * this function forfeits the guarantee of security.
+ *
+ * Returns: 0 if the urandom pool has been seeded.
+ *  -ERESTARTSYS if the function was interrupted by a signal.
+ */
+int wait_for_random_bytes(void)
+{
+   if (likely(crng_ready()))
+   return 0;
+   return wait_event_interruptible(crng_init_wait, crng_ready());
+}
+EXPORT_SYMBOL(wait_for_random_bytes);
+
+/*
  * Add a callback function that will be invoked when the nonblocking
  * pool is initialised.
  *
@@ -1860,6 +1876,8 @@ const struct file_operations urandom_fops = {
 SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, count,
unsigned int, flags)
 {
+   int ret;
+
if (flags & ~(GRND_NONBLOCK|GRND_RANDOM))
return -EINVAL;
 
@@ -1872,9 +1890,9 @@ SYSCALL_DEFINE3(getrandom, char __user *, buf, size_t, 
count,
if (!crng_ready()) {
if (flags & GRND_NONBLOCK)
return -EAGAIN;
-   crng_wait_ready();
-   if (signal_pending(current))
-   return -ERESTARTSYS;
+   ret = wait_for_random_bytes();
+   if (unlikely(ret))
+   return ret;
}
return urandom_read(NULL, buf, count, NULL);
 }
@@ -2035,7 +2053,10 @@ static rwlock_t batched_entropy_reset_lock = 
__RW_LOCK_UNLOCKED(batched_entropy_
 /*
  * Get a random word for internal kernel use only. The quality of the random
  * number is either as good as RDRAND or as good as /dev/urandom, with the
- * goal of being quite fast and not depleting entropy.
+ * goal of being quite fast and not depleting entropy. In order to ensure
+ * that the randomness provided by this function is okay, the function
+ * wait_for_random_bytes() should be called and return 0 at least once
+ * at any point prior.
  */
 static DEFINE_PER_CPU(struct batched_entropy, batched_entropy_u64);
 u64 get_random_u64(void)
diff --git a/include/linux/random.h b/include/linux/random.h
index ed5c3838780d..e29929347c95 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -34,6 +34,7 @@ extern void add_input_randomness(unsigned int type, unsigned 
int code,
 extern void add_interrupt_randomness(int irq, int irq_flags) __latent_entropy;
 
 extern void get_random_bytes(void *buf, int nbytes);
+extern int wait_for_random_bytes(void);
 extern int add_random_ready_callback(struct random_ready_callback *rdy);
 extern void del_random_ready_callback(struct random_ready_callback *rdy);
 extern void get_random_bytes_arch(void *buf, int nbytes);
-- 
2.13.0

[PATCH v5 04/13] security/keys: ensure RNG is seeded before use

[Jason A. Donenfeld]

Otherwise, we might use bad random numbers which, particularly in the
case of IV generation, could be quite bad. It makes sense to use the
synchronous API here, because we're always in process context (as the
code is littered with GFP_KERNEL and the like). However, we can't change
to using a blocking function in key serial allocation, because this will
block booting in some configurations, so here we use the more
appropriate get_random_u32, which will use RDRAND if available.

Signed-off-by: Jason A. Donenfeld 
Cc: David Howells 
Cc: Mimi Zohar 
Cc: David Safford 
---
 security/keys/encrypted-keys/encrypted.c |  8 +---
 security/keys/key.c  | 16 
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/security/keys/encrypted-keys/encrypted.c 
b/security/keys/encrypted-keys/encrypted.c
index 0010955d7876..d51a28fc5cd5 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -777,10 +777,12 @@ static int encrypted_init(struct encrypted_key_payload 
*epayload,
 
__ekey_init(epayload, format, master_desc, datalen);
if (!hex_encoded_iv) {
-   get_random_bytes(epayload->iv, ivsize);
+   ret = get_random_bytes_wait(epayload->iv, ivsize);
+   if (unlikely(ret))
+   return ret;
 
-   get_random_bytes(epayload->decrypted_data,
-epayload->decrypted_datalen);
+   ret = get_random_bytes_wait(epayload->decrypted_data,
+   epayload->decrypted_datalen);
} else
ret = encrypted_key_decrypt(epayload, format, hex_encoded_iv);
return ret;
diff --git a/security/keys/key.c b/security/keys/key.c
index 455c04d80bbb..b72078e532f2 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -134,17 +134,15 @@ void key_user_put(struct key_user *user)
  * Allocate a serial number for a key.  These are assigned randomly to avoid
  * security issues through covert channel problems.
  */
-static inline void key_alloc_serial(struct key *key)
+static inline int key_alloc_serial(struct key *key)
 {
struct rb_node *parent, **p;
struct key *xkey;
 
-   /* propose a random serial number and look for a hole for it in the
-* serial number tree */
+   /* propose a non-negative random serial number and look for a hole for
+* it in the serial number tree */
do {
-   get_random_bytes(>serial, sizeof(key->serial));
-
-   key->serial >>= 1; /* negative numbers are not permitted */
+   key->serial = get_random_u32() >> 1;
} while (key->serial < 3);
 
spin_lock(_serial_lock);
@@ -170,7 +168,7 @@ static inline void key_alloc_serial(struct key *key)
rb_insert_color(>serial_node, _serial_tree);
 
spin_unlock(_serial_lock);
-   return;
+   return 0;
 
/* we found a key with the proposed serial number - walk the tree from
 * that point looking for the next unused serial number */
@@ -314,7 +312,9 @@ struct key *key_alloc(struct key_type *type, const char 
*desc,
 
/* publish the key by giving it a serial number */
atomic_inc(>nkeys);
-   key_alloc_serial(key);
+   ret = key_alloc_serial(key);
+   if (ret < 0)
+   goto security_error;
 
 error:
return key;
-- 
2.13.0

[PATCH v5 00/13] Unseeded In-Kernel Randomness Fixes

[Jason A. Donenfeld]

It looks like critique of this has come to an end. Could somebody take
this into their tree for 4.12?

As discussed in [1], there is a problem with get_random_bytes being
used before the RNG has actually been seeded. The solution for fixing
this appears to be multi-pronged. One of those prongs involves adding
a simple blocking API so that modules that use the RNG in process
context can just sleep (in an interruptable manner) until the RNG is
ready to be used. This winds up being a very useful API that covers
a few use cases, several of which are included in this patch set.

[1] http://www.openwall.com/lists/kernel-hardening/2017/06/02/2

Changes v4->v5:
  - Old versions of gcc warned on an uninitialized variable, so set
this to silence warning.

Jason A. Donenfeld (13):
  random: invalidate batched entropy after crng init
  random: add synchronous API for the urandom pool
  random: add get_random_{bytes,u32,u64,int,long,once}_wait family
  security/keys: ensure RNG is seeded before use
  crypto/rng: ensure that the RNG is ready before using
  iscsi: ensure RNG is seeded before use
  ceph: ensure RNG is seeded before using
  cifs: use get_random_u32 for 32-bit lock random
  rhashtable: use get_random_u32 for hash_rnd
  net/neighbor: use get_random_u32 for 32-bit hash random
  net/route: use get_random_int for random counter
  bluetooth/smp: ensure RNG is properly seeded before ECDH use
  random: warn when kernel uses unseeded randomness

 crypto/rng.c  |  6 +-
 drivers/char/random.c | 93 +++
 drivers/target/iscsi/iscsi_target_auth.c  | 14 -
 drivers/target/iscsi/iscsi_target_login.c | 22 +---
 fs/cifs/cifsfs.c  |  2 +-
 include/linux/net.h   |  2 +
 include/linux/once.h  |  2 +
 include/linux/random.h| 26 +
 lib/Kconfig.debug | 16 ++
 lib/rhashtable.c  |  2 +-
 net/bluetooth/hci_request.c   |  6 ++
 net/bluetooth/smp.c   | 18 --
 net/ceph/ceph_common.c|  6 +-
 net/core/neighbour.c  |  3 +-
 net/ipv4/route.c  |  3 +-
 security/keys/encrypted-keys/encrypted.c  |  8 ++-
 security/keys/key.c   | 16 +++---
 17 files changed, 198 insertions(+), 47 deletions(-)

-- 
2.13.0

[PATCH v5 07/13] ceph: ensure RNG is seeded before using

[Jason A. Donenfeld]

Ceph uses the RNG for various nonce generations, and it shouldn't accept
using bad randomness. So, we wait for the RNG to be properly seeded. We
do this by calling wait_for_random_bytes() in a function that is
certainly called in process context, early on, so that all subsequent
calls to get_random_bytes are necessarily acceptable.

Signed-off-by: Jason A. Donenfeld 
Cc: Ilya Dryomov 
Cc: "Yan, Zheng" 
Cc: Sage Weil 
---
 net/ceph/ceph_common.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/ceph/ceph_common.c b/net/ceph/ceph_common.c
index 47e94b560ba0..0368a04995b3 100644
--- a/net/ceph/ceph_common.c
+++ b/net/ceph/ceph_common.c
@@ -598,7 +598,11 @@ struct ceph_client *ceph_create_client(struct ceph_options 
*opt, void *private)
 {
struct ceph_client *client;
struct ceph_entity_addr *myaddr = NULL;
-   int err = -ENOMEM;
+   int err;
+
+   err = wait_for_random_bytes();
+   if (err < 0)
+   return ERR_PTR(err);
 
client = kzalloc(sizeof(*client), GFP_KERNEL);
if (client == NULL)
-- 
2.13.0

[PATCH v5 05/13] crypto/rng: ensure that the RNG is ready before using

[Jason A. Donenfeld]

Otherwise, we might be seeding the RNG using bad randomness, which is
dangerous. The one use of this function from within the kernel -- not
from userspace -- is being removed (keys/big_key), so that call site
isn't relevant in assessing this.

Cc: Herbert Xu 
Signed-off-by: Jason A. Donenfeld 
---
 crypto/rng.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/crypto/rng.c b/crypto/rng.c
index f46dac5288b9..e042437e64b4 100644
--- a/crypto/rng.c
+++ b/crypto/rng.c
@@ -48,12 +48,14 @@ int crypto_rng_reset(struct crypto_rng *tfm, const u8 
*seed, unsigned int slen)
if (!buf)
return -ENOMEM;
 
-   get_random_bytes(buf, slen);
+   err = get_random_bytes_wait(buf, slen);
+   if (err)
+   goto out;
seed = buf;
}
 
err = crypto_rng_alg(tfm)->seed(tfm, seed, slen);
-
+out:
kzfree(buf);
return err;
 }
-- 
2.13.0

[PATCH v5 08/13] cifs: use get_random_u32 for 32-bit lock random

[Jason A. Donenfeld]

Using get_random_u32 here is faster, more fitting of the use case, and
just as cryptographically secure. It also has the benefit of providing
better randomness at early boot, which is sometimes when this is used.

Signed-off-by: Jason A. Donenfeld 
Cc: Steve French 
---
 fs/cifs/cifsfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 9a1667e0e8d6..fe0c8dcc7dc7 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -1359,7 +1359,7 @@ init_cifs(void)
spin_lock_init(_tcp_ses_lock);
spin_lock_init(_Lock);
 
-   get_random_bytes(_lock_secret, sizeof(cifs_lock_secret));
+   cifs_lock_secret = get_random_u32();
 
if (cifs_max_pending < 2) {
cifs_max_pending = 2;
-- 
2.13.0

[PATCH v5 06/13] iscsi: ensure RNG is seeded before use

[Jason A. Donenfeld]

It's not safe to use weak random data here, especially for the challenge
response randomness. Since we're always in process context, it's safe to
simply wait until we have enough randomness to carry out the
authentication correctly.

While we're at it, we clean up a small memleak during an error
condition.

Signed-off-by: Jason A. Donenfeld 
Cc: "Nicholas A. Bellinger" 
Cc: Lee Duncan 
Cc: Chris Leech 
---
 drivers/target/iscsi/iscsi_target_auth.c  | 14 +++---
 drivers/target/iscsi/iscsi_target_login.c | 22 ++
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_auth.c 
b/drivers/target/iscsi/iscsi_target_auth.c
index 903b667f8e01..f9bc8ec6fb6b 100644
--- a/drivers/target/iscsi/iscsi_target_auth.c
+++ b/drivers/target/iscsi/iscsi_target_auth.c
@@ -47,18 +47,21 @@ static void chap_binaryhex_to_asciihex(char *dst, char 
*src, int src_len)
}
 }
 
-static void chap_gen_challenge(
+static int chap_gen_challenge(
struct iscsi_conn *conn,
int caller,
char *c_str,
unsigned int *c_len)
 {
+   int ret;
unsigned char challenge_asciihex[CHAP_CHALLENGE_LENGTH * 2 + 1];
struct iscsi_chap *chap = conn->auth_protocol;
 
memset(challenge_asciihex, 0, CHAP_CHALLENGE_LENGTH * 2 + 1);
 
-   get_random_bytes(chap->challenge, CHAP_CHALLENGE_LENGTH);
+   ret = get_random_bytes_wait(chap->challenge, CHAP_CHALLENGE_LENGTH);
+   if (unlikely(ret))
+   return ret;
chap_binaryhex_to_asciihex(challenge_asciihex, chap->challenge,
CHAP_CHALLENGE_LENGTH);
/*
@@ -69,6 +72,7 @@ static void chap_gen_challenge(
 
pr_debug("[%s] Sending CHAP_C=0x%s\n\n", (caller) ? "server" : "client",
challenge_asciihex);
+   return 0;
 }
 
 static int chap_check_algorithm(const char *a_str)
@@ -143,6 +147,7 @@ static struct iscsi_chap *chap_server_open(
case CHAP_DIGEST_UNKNOWN:
default:
pr_err("Unsupported CHAP_A value\n");
+   kfree(conn->auth_protocol);
return NULL;
}
 
@@ -156,7 +161,10 @@ static struct iscsi_chap *chap_server_open(
/*
 * Generate Challenge.
 */
-   chap_gen_challenge(conn, 1, aic_str, aic_len);
+   if (chap_gen_challenge(conn, 1, aic_str, aic_len) < 0) {
+   kfree(conn->auth_protocol);
+   return NULL;
+   }
 
return chap;
 }
diff --git a/drivers/target/iscsi/iscsi_target_login.c 
b/drivers/target/iscsi/iscsi_target_login.c
index 92b96b51d506..e9bdc8b86e7d 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -245,22 +245,26 @@ int iscsi_check_for_session_reinstatement(struct 
iscsi_conn *conn)
return 0;
 }
 
-static void iscsi_login_set_conn_values(
+static int iscsi_login_set_conn_values(
struct iscsi_session *sess,
struct iscsi_conn *conn,
__be16 cid)
 {
+   int ret;
conn->sess  = sess;
conn->cid   = be16_to_cpu(cid);
/*
 * Generate a random Status sequence number (statsn) for the new
 * iSCSI connection.
 */
-   get_random_bytes(>stat_sn, sizeof(u32));
+   ret = get_random_bytes_wait(>stat_sn, sizeof(u32));
+   if (unlikely(ret))
+   return ret;
 
mutex_lock(_id_lock);
conn->auth_id   = iscsit_global->auth_id++;
mutex_unlock(_id_lock);
+   return 0;
 }
 
 __printf(2, 3) int iscsi_change_param_sprintf(
@@ -306,7 +310,11 @@ static int iscsi_login_zero_tsih_s1(
return -ENOMEM;
}
 
-   iscsi_login_set_conn_values(sess, conn, pdu->cid);
+   ret = iscsi_login_set_conn_values(sess, conn, pdu->cid);
+   if (unlikely(ret)) {
+   kfree(sess);
+   return ret;
+   }
sess->init_task_tag = pdu->itt;
memcpy(>isid, pdu->isid, 6);
sess->exp_cmd_sn= be32_to_cpu(pdu->cmdsn);
@@ -497,8 +505,7 @@ static int iscsi_login_non_zero_tsih_s1(
 {
struct iscsi_login_req *pdu = (struct iscsi_login_req *)buf;
 
-   iscsi_login_set_conn_values(NULL, conn, pdu->cid);
-   return 0;
+   return iscsi_login_set_conn_values(NULL, conn, pdu->cid);
 }
 
 /*
@@ -554,9 +561,8 @@ static int iscsi_login_non_zero_tsih_s2(
atomic_set(>session_continuation, 1);
spin_unlock_bh(>conn_lock);
 
-   iscsi_login_set_conn_values(sess, conn, pdu->cid);
-
-   if (iscsi_copy_param_list(>param_list,
+   if (iscsi_login_set_conn_values(sess, conn, pdu->cid) < 0 ||
+   iscsi_copy_param_list(>param_list,
conn->tpg->param_list, 0) < 0) {
iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
   

[PATCH v5 11/13] net/route: use get_random_int for random counter

[Jason A. Donenfeld]

Using get_random_int here is faster, more fitting of the use case, and
just as cryptographically secure. It also has the benefit of providing
better randomness at early boot, which is when many of these structures
are assigned.

Also, semantically, it's not really proper to have been assigning an
atomic_t in this way before, even if in practice it works fine.

Signed-off-by: Jason A. Donenfeld 
Cc: David Miller 
---
 net/ipv4/route.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 6883b3d4ba8f..32a3332ec9cf 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2944,8 +2944,7 @@ static __net_init int rt_genid_init(struct net *net)
 {
atomic_set(>ipv4.rt_genid, 0);
atomic_set(>fnhe_genid, 0);
-   get_random_bytes(>ipv4.dev_addr_genid,
-sizeof(net->ipv4.dev_addr_genid));
+   atomic_set(>ipv4.dev_addr_genid, get_random_int());
return 0;
 }
 
-- 
2.13.0

[PATCH v5 12/13] bluetooth/smp: ensure RNG is properly seeded before ECDH use

[Jason A. Donenfeld]

This protocol uses lots of complex cryptography that relies on securely
generated random numbers. Thus, it's important that the RNG is actually
seeded before use. Fortuantely, it appears we're always operating in
process context (there are many GFP_KERNEL allocations and other
sleeping operations), and so we can simply demand that the RNG is seeded
before we use it.

We take two strategies in this commit. The first is for the library code
that's called from other modules like hci or mgmt: here we just change
the call to get_random_bytes_wait, and return the result of the wait to
the caller, along with the other error codes of those functions like
usual. Then there's the SMP protocol handler itself, which makes many
many many calls to get_random_bytes during different phases. For this,
rather than have to change all the calls to get_random_bytes_wait and
propagate the error result, it's actually enough to just put a single
call to wait_for_random_bytes() at the beginning of the handler, to
ensure that all the subsequent invocations are safe, without having to
actually change them. Likewise, for the random address changing
function, we'd rather know early on in the function whether the RNG
initialization has been interrupted, rather than later, so we call
wait_for_random_bytes() at the top, so that later on the call to
get_random_bytes() is acceptable.

Signed-off-by: Jason A. Donenfeld 
Cc: Marcel Holtmann 
Cc: Gustavo Padovan 
Cc: Johan Hedberg 
---
 net/bluetooth/hci_request.c |  6 ++
 net/bluetooth/smp.c | 18 ++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index b5faff458d8b..4078057c4fd7 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1406,6 +1406,12 @@ int hci_update_random_address(struct hci_request *req, 
bool require_privacy,
struct hci_dev *hdev = req->hdev;
int err;
 
+   if (require_privacy) {
+   err = wait_for_random_bytes();
+   if (unlikely(err))
+   return err;
+   }
+
/* If privacy is enabled use a resolvable private address. If
 * current RPA has expired or there is something else than
 * the current RPA in use, then generate a new one.
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 14585edc9439..5fef1bc96f42 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -537,7 +537,9 @@ int smp_generate_rpa(struct hci_dev *hdev, const u8 
irk[16], bdaddr_t *rpa)
 
smp = chan->data;
 
-   get_random_bytes(>b[3], 3);
+   err = get_random_bytes_wait(>b[3], 3);
+   if (unlikely(err))
+   return err;
 
rpa->b[5] &= 0x3f;  /* Clear two most significant bits */
rpa->b[5] |= 0x40;  /* Set second most significant bit */
@@ -570,7 +572,9 @@ int smp_generate_oob(struct hci_dev *hdev, u8 hash[16], u8 
rand[16])
} else {
while (true) {
/* Seed private key with random number */
-   get_random_bytes(smp->local_sk, 32);
+   err = get_random_bytes_wait(smp->local_sk, 32);
+   if (unlikely(err))
+   return err;
 
/* Generate local key pair for Secure Connections */
if (!generate_ecdh_keys(smp->local_pk, smp->local_sk))
@@ -589,7 +593,9 @@ int smp_generate_oob(struct hci_dev *hdev, u8 hash[16], u8 
rand[16])
SMP_DBG("OOB Public Key Y: %32phN", smp->local_pk + 32);
SMP_DBG("OOB Private Key:  %32phN", smp->local_sk);
 
-   get_random_bytes(smp->local_rand, 16);
+   err = get_random_bytes_wait(smp->local_rand, 16);
+   if (unlikely(err))
+   return err;
 
err = smp_f4(smp->tfm_cmac, smp->local_pk, smp->local_pk,
 smp->local_rand, 0, hash);
@@ -2831,7 +2837,11 @@ static int smp_sig_channel(struct l2cap_chan *chan, 
struct sk_buff *skb)
struct hci_conn *hcon = conn->hcon;
struct smp_chan *smp;
__u8 code, reason;
-   int err = 0;
+   int err;
+
+   err = wait_for_random_bytes();
+   if (unlikely(err))
+   return err;
 
if (skb->len < 1)
return -EILSEQ;
-- 
2.13.0

[PATCH v5 13/13] random: warn when kernel uses unseeded randomness

[Jason A. Donenfeld]

This enables an important dmesg notification about when drivers have
used the crng without it being seeded first. Prior, these errors would
occur silently, and so there hasn't been a great way of diagnosing these
types of bugs for obscure setups. By adding this as a config option, we
can leave it on by default, so that we learn where these issues happen,
in the field, will still allowing some people to turn it off, if they
really know what they're doing and do not want the log entries.

However, we don't leave it _completely_ by default. An earlier version
of this patch simply had `default y`. I'd really love that, but it turns
out, this problem with unseeded randomness being used is really quite
present and is going to take a long time to fix. Thus, as a compromise
between log-messages-for-all and nobody-knows, this is `default y`,
except it is also `depends on DEBUG_KERNEL`. This will ensure that the
curious see the messages while others don't have to.

Signed-off-by: Jason A. Donenfeld 
---
 drivers/char/random.c | 15 +--
 lib/Kconfig.debug | 16 
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 77587ac1ecb2..5252690610ec 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -288,7 +288,6 @@
 #define SEC_XFER_SIZE  512
 #define EXTRACT_SIZE   10
 
-#define DEBUG_RANDOM_BOOT 0
 
 #define LONGS(x) (((x) + sizeof(unsigned long) - 1)/sizeof(unsigned long))
 
@@ -1481,7 +1480,7 @@ void get_random_bytes(void *buf, int nbytes)
 {
__u8 tmp[CHACHA20_BLOCK_SIZE];
 
-#if DEBUG_RANDOM_BOOT > 0
+#ifdef CONFIG_WARN_UNSEEDED_RANDOM
if (!crng_ready())
printk(KERN_NOTICE "random: %pF get_random_bytes called "
   "with crng_init = %d\n", (void *) _RET_IP_, crng_init);
@@ -2075,6 +2074,12 @@ u64 get_random_u64(void)
return ret;
 #endif
 
+#ifdef CONFIG_WARN_UNSEEDED_RANDOM
+   if (!crng_ready())
+   printk(KERN_NOTICE "random: %pF get_random_u64 called "
+  "with crng_init = %d\n", (void *) _RET_IP_, crng_init);
+#endif
+
batch = _cpu_var(batched_entropy_u64);
if (use_lock)
read_lock_irqsave(_entropy_reset_lock, flags);
@@ -2101,6 +2106,12 @@ u32 get_random_u32(void)
if (arch_get_random_int())
return ret;
 
+#ifdef CONFIG_WARN_UNSEEDED_RANDOM
+   if (!crng_ready())
+   printk(KERN_NOTICE "random: %pF get_random_u32 called "
+  "with crng_init = %d\n", (void *) _RET_IP_, crng_init);
+#endif
+
batch = _cpu_var(batched_entropy_u32);
if (use_lock)
read_lock_irqsave(_entropy_reset_lock, flags);
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index e4587ebe52c7..c4159605bfbf 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1209,6 +1209,22 @@ config STACKTRACE
  It is also used by various kernel debugging features that require
  stack trace generation.
 
+config WARN_UNSEEDED_RANDOM
+   bool "Warn when kernel uses unseeded randomness"
+   default y
+   depends on DEBUG_KERNEL
+   help
+ Some parts of the kernel contain bugs relating to their use of
+ cryptographically secure random numbers before it's actually possible
+ to generate those numbers securely. This setting ensures that these
+ flaws don't go unnoticed, by enabling a message, should this ever
+ occur. This will allow people with obscure setups to know when things
+ are going wrong, so that they might contact developers about fixing
+ it.
+
+ Say Y here, unless you simply do not care about using unseeded
+ randomness and do not want a potential warning message in your logs.
+
 config DEBUG_KOBJECT
bool "kobject debugging"
depends on DEBUG_KERNEL
-- 
2.13.0

[PATCH v5 09/13] rhashtable: use get_random_u32 for hash_rnd

[Jason A. Donenfeld]

This is much faster and just as secure. It also has the added benefit of
probably returning better randomness at early-boot on systems with
architectural RNGs.

Signed-off-by: Jason A. Donenfeld 
Cc: Thomas Graf 
Cc: Herbert Xu 
---
 lib/rhashtable.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index d9e7274a04cd..a1eb7c947f46 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -235,7 +235,7 @@ static struct bucket_table *bucket_table_alloc(struct 
rhashtable *ht,
 
INIT_LIST_HEAD(>walkers);
 
-   get_random_bytes(>hash_rnd, sizeof(tbl->hash_rnd));
+   tbl->hash_rnd = get_random_u32();
 
for (i = 0; i < nbuckets; i++)
INIT_RHT_NULLS_HEAD(tbl->buckets[i], ht, i);
-- 
2.13.0

[PATCH v5 10/13] net/neighbor: use get_random_u32 for 32-bit hash random

[Jason A. Donenfeld]

Using get_random_u32 here is faster, more fitting of the use case, and
just as cryptographically secure. It also has the benefit of providing
better randomness at early boot, which is when many of these structures
are assigned.

Signed-off-by: Jason A. Donenfeld 
Cc: David Miller 
---
 net/core/neighbour.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index d274f81fcc2c..9784133b0cdb 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -312,8 +312,7 @@ static struct neighbour *neigh_alloc(struct neigh_table 
*tbl, struct net_device
 
 static void neigh_get_hash_rnd(u32 *x)
 {
-   get_random_bytes(x, sizeof(*x));
-   *x |= 1;
+   *x = get_random_u32() | 1;
 }
 
 static struct neigh_hash_table *neigh_hash_alloc(unsigned int shift)
-- 
2.13.0

[PATCH v5 01/13] random: invalidate batched entropy after crng init

[Jason A. Donenfeld]

It's possible that get_random_{u32,u64} is used before the crng has
initialized, in which case, its output might not be cryptographically
secure. For this problem, directly, this patch set is introducing the
*_wait variety of functions, but even with that, there's a subtle issue:
what happens to our batched entropy that was generated before
initialization. Prior to this commit, it'd stick around, supplying bad
numbers. After this commit, we force the entropy to be re-extracted
after each phase of the crng has initialized.

In order to avoid a race condition with the position counter, we
introduce a simple rwlock for this invalidation. Since it's only during
this awkward transition period, after things are all set up, we stop
using it, so that it doesn't have an impact on performance.

This should probably be backported to 4.11.

(Also: adding my copyright to the top. With the patch series from
January, this patch, and then the ones that come after, I think there's
a relevant amount of code in here to add my name to the top.)

Signed-off-by: Jason A. Donenfeld 
Cc: Greg Kroah-Hartman 
---
 drivers/char/random.c | 37 +
 1 file changed, 37 insertions(+)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index a561f0c2f428..d35da1603e12 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1,6 +1,9 @@
 /*
  * random.c -- A strong random number generator
  *
+ * Copyright (C) 2017 Jason A. Donenfeld . All
+ * Rights Reserved.
+ *
  * Copyright Matt Mackall , 2003, 2004, 2005
  *
  * Copyright Theodore Ts'o, 1994, 1995, 1996, 1997, 1998, 1999.  All
@@ -762,6 +765,8 @@ static DECLARE_WAIT_QUEUE_HEAD(crng_init_wait);
 static struct crng_state **crng_node_pool __read_mostly;
 #endif
 
+static void invalidate_batched_entropy(void);
+
 static void crng_initialize(struct crng_state *crng)
 {
int i;
@@ -799,6 +804,7 @@ static int crng_fast_load(const char *cp, size_t len)
cp++; crng_init_cnt++; len--;
}
if (crng_init_cnt >= CRNG_INIT_CNT_THRESH) {
+   invalidate_batched_entropy();
crng_init = 1;
wake_up_interruptible(_init_wait);
pr_notice("random: fast init done\n");
@@ -836,6 +842,7 @@ static void crng_reseed(struct crng_state *crng, struct 
entropy_store *r)
memzero_explicit(, sizeof(buf));
crng->init_time = jiffies;
if (crng == _crng && crng_init < 2) {
+   invalidate_batched_entropy();
crng_init = 2;
process_random_ready_list();
wake_up_interruptible(_init_wait);
@@ -2023,6 +2030,7 @@ struct batched_entropy {
};
unsigned int position;
 };
+static rwlock_t batched_entropy_reset_lock = 
__RW_LOCK_UNLOCKED(batched_entropy_reset_lock);
 
 /*
  * Get a random word for internal kernel use only. The quality of the random
@@ -2033,6 +2041,8 @@ static DEFINE_PER_CPU(struct batched_entropy, 
batched_entropy_u64);
 u64 get_random_u64(void)
 {
u64 ret;
+   const bool use_lock = READ_ONCE(crng_init) < 2;
+   unsigned long flags = 0;
struct batched_entropy *batch;
 
 #if BITS_PER_LONG == 64
@@ -2045,11 +2055,15 @@ u64 get_random_u64(void)
 #endif
 
batch = _cpu_var(batched_entropy_u64);
+   if (use_lock)
+   read_lock_irqsave(_entropy_reset_lock, flags);
if (batch->position % ARRAY_SIZE(batch->entropy_u64) == 0) {
extract_crng((u8 *)batch->entropy_u64);
batch->position = 0;
}
ret = batch->entropy_u64[batch->position++];
+   if (use_lock)
+   read_unlock_irqrestore(_entropy_reset_lock, flags);
put_cpu_var(batched_entropy_u64);
return ret;
 }
@@ -2059,22 +2073,45 @@ static DEFINE_PER_CPU(struct batched_entropy, 
batched_entropy_u32);
 u32 get_random_u32(void)
 {
u32 ret;
+   const bool use_lock = READ_ONCE(crng_init) < 2;
+   unsigned long flags = 0;
struct batched_entropy *batch;
 
if (arch_get_random_int())
return ret;
 
batch = _cpu_var(batched_entropy_u32);
+   if (use_lock)
+   read_lock_irqsave(_entropy_reset_lock, flags);
if (batch->position % ARRAY_SIZE(batch->entropy_u32) == 0) {
extract_crng((u8 *)batch->entropy_u32);
batch->position = 0;
}
ret = batch->entropy_u32[batch->position++];
+   if (use_lock)
+   read_unlock_irqrestore(_entropy_reset_lock, flags);
put_cpu_var(batched_entropy_u32);
return ret;
 }
 EXPORT_SYMBOL(get_random_u32);
 
+/* It's important to invalidate all potential batched entropy that might
+ * be stored before the crng is initialized, which we can do lazily by
+ * simply resetting the counter to zero so that it's re-extracted on the
+ * next usage. */
+static void 

[PATCH v5 01/13] random: invalidate batched entropy after crng init

[Jason A. Donenfeld]

It's possible that get_random_{u32,u64} is used before the crng has
initialized, in which case, its output might not be cryptographically
secure. For this problem, directly, this patch set is introducing the
*_wait variety of functions, but even with that, there's a subtle issue:
what happens to our batched entropy that was generated before
initialization. Prior to this commit, it'd stick around, supplying bad
numbers. After this commit, we force the entropy to be re-extracted
after each phase of the crng has initialized.

In order to avoid a race condition with the position counter, we
introduce a simple rwlock for this invalidation. Since it's only during
this awkward transition period, after things are all set up, we stop
using it, so that it doesn't have an impact on performance.

This should probably be backported to 4.11.

(Also: adding my copyright to the top. With the patch series from
January, this patch, and then the ones that come after, I think there's
a relevant amount of code in here to add my name to the top.)

Signed-off-by: Jason A. Donenfeld 
Cc: Greg Kroah-Hartman 
---
 drivers/char/random.c | 37 +
 1 file changed, 37 insertions(+)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index a561f0c2f428..d35da1603e12 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1,6 +1,9 @@
 /*
  * random.c -- A strong random number generator
  *
+ * Copyright (C) 2017 Jason A. Donenfeld . All
+ * Rights Reserved.
+ *
  * Copyright Matt Mackall , 2003, 2004, 2005
  *
  * Copyright Theodore Ts'o, 1994, 1995, 1996, 1997, 1998, 1999.  All
@@ -762,6 +765,8 @@ static DECLARE_WAIT_QUEUE_HEAD(crng_init_wait);
 static struct crng_state **crng_node_pool __read_mostly;
 #endif
 
+static void invalidate_batched_entropy(void);
+
 static void crng_initialize(struct crng_state *crng)
 {
int i;
@@ -799,6 +804,7 @@ static int crng_fast_load(const char *cp, size_t len)
cp++; crng_init_cnt++; len--;
}
if (crng_init_cnt >= CRNG_INIT_CNT_THRESH) {
+   invalidate_batched_entropy();
crng_init = 1;
wake_up_interruptible(_init_wait);
pr_notice("random: fast init done\n");
@@ -836,6 +842,7 @@ static void crng_reseed(struct crng_state *crng, struct 
entropy_store *r)
memzero_explicit(, sizeof(buf));
crng->init_time = jiffies;
if (crng == _crng && crng_init < 2) {
+   invalidate_batched_entropy();
crng_init = 2;
process_random_ready_list();
wake_up_interruptible(_init_wait);
@@ -2023,6 +2030,7 @@ struct batched_entropy {
};
unsigned int position;
 };
+static rwlock_t batched_entropy_reset_lock = 
__RW_LOCK_UNLOCKED(batched_entropy_reset_lock);
 
 /*
  * Get a random word for internal kernel use only. The quality of the random
@@ -2033,6 +2041,8 @@ static DEFINE_PER_CPU(struct batched_entropy, 
batched_entropy_u64);
 u64 get_random_u64(void)
 {
u64 ret;
+   const bool use_lock = READ_ONCE(crng_init) < 2;
+   unsigned long flags = 0;
struct batched_entropy *batch;
 
 #if BITS_PER_LONG == 64
@@ -2045,11 +2055,15 @@ u64 get_random_u64(void)
 #endif
 
batch = _cpu_var(batched_entropy_u64);
+   if (use_lock)
+   read_lock_irqsave(_entropy_reset_lock, flags);
if (batch->position % ARRAY_SIZE(batch->entropy_u64) == 0) {
extract_crng((u8 *)batch->entropy_u64);
batch->position = 0;
}
ret = batch->entropy_u64[batch->position++];
+   if (use_lock)
+   read_unlock_irqrestore(_entropy_reset_lock, flags);
put_cpu_var(batched_entropy_u64);
return ret;
 }
@@ -2059,22 +2073,45 @@ static DEFINE_PER_CPU(struct batched_entropy, 
batched_entropy_u32);
 u32 get_random_u32(void)
 {
u32 ret;
+   const bool use_lock = READ_ONCE(crng_init) < 2;
+   unsigned long flags = 0;
struct batched_entropy *batch;
 
if (arch_get_random_int())
return ret;
 
batch = _cpu_var(batched_entropy_u32);
+   if (use_lock)
+   read_lock_irqsave(_entropy_reset_lock, flags);
if (batch->position % ARRAY_SIZE(batch->entropy_u32) == 0) {
extract_crng((u8 *)batch->entropy_u32);
batch->position = 0;
}
ret = batch->entropy_u32[batch->position++];
+   if (use_lock)
+   read_unlock_irqrestore(_entropy_reset_lock, flags);
put_cpu_var(batched_entropy_u32);
return ret;
 }
 EXPORT_SYMBOL(get_random_u32);
 
+/* It's important to invalidate all potential batched entropy that might
+ * be stored before the crng is initialized, which we can do lazily by
+ * simply resetting the counter to zero so that it's re-extracted on the
+ * next usage. */
+static void 

[PATCH v5 00/13] Unseeded In-Kernel Randomness Fixes

[Jason A. Donenfeld]

It looks like critique of this has come to an end. Could somebody take
this into their tree for 4.12?

As discussed in [1], there is a problem with get_random_bytes being
used before the RNG has actually been seeded. The solution for fixing
this appears to be multi-pronged. One of those prongs involves adding
a simple blocking API so that modules that use the RNG in process
context can just sleep (in an interruptable manner) until the RNG is
ready to be used. This winds up being a very useful API that covers
a few use cases, several of which are included in this patch set.

[1] http://www.openwall.com/lists/kernel-hardening/2017/06/02/2

Changes v4->v5:
  - Old versions of gcc warned on an uninitialized variable, so set
this to silence warning.

Jason A. Donenfeld (13):
  random: invalidate batched entropy after crng init
  random: add synchronous API for the urandom pool
  random: add get_random_{bytes,u32,u64,int,long,once}_wait family
  security/keys: ensure RNG is seeded before use
  crypto/rng: ensure that the RNG is ready before using
  iscsi: ensure RNG is seeded before use
  ceph: ensure RNG is seeded before using
  cifs: use get_random_u32 for 32-bit lock random
  rhashtable: use get_random_u32 for hash_rnd
  net/neighbor: use get_random_u32 for 32-bit hash random
  net/route: use get_random_int for random counter
  bluetooth/smp: ensure RNG is properly seeded before ECDH use
  random: warn when kernel uses unseeded randomness

 crypto/rng.c  |  6 +-
 drivers/char/random.c | 93 +++
 drivers/target/iscsi/iscsi_target_auth.c  | 14 -
 drivers/target/iscsi/iscsi_target_login.c | 22 +---
 fs/cifs/cifsfs.c  |  2 +-
 include/linux/net.h   |  2 +
 include/linux/once.h  |  2 +
 include/linux/random.h| 26 +
 lib/Kconfig.debug | 16 ++
 lib/rhashtable.c  |  2 +-
 net/bluetooth/hci_request.c   |  6 ++
 net/bluetooth/smp.c   | 18 --
 net/ceph/ceph_common.c|  6 +-
 net/core/neighbour.c  |  3 +-
 net/ipv4/route.c  |  3 +-
 security/keys/encrypted-keys/encrypted.c  |  8 ++-
 security/keys/key.c   | 16 +++---
 17 files changed, 198 insertions(+), 47 deletions(-)

-- 
2.13.0

Re: [PATCH v1 2/2] dt-bindings: crypto: remove mediatek ethif clock

[Rob Herring]

On Thu, Jun 01, 2017 at 10:30:22AM +0800, Ryder Lee wrote:
> This patch removes the parent clock 'ethif' in bindings, since we don't
> need to control the parent of a clock in current clock framework.
> 
> Moreover, the clocks are get by name in the driver, thus this change
> does not break backwards compatibility.
> 
> Signed-off-by: Ryder Lee 
> Reviewed-by: Matthias Brugger 
> ---
>  Documentation/devicetree/bindings/crypto/mediatek-crypto.txt | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)

Acked-by: Rob Herring 

Re: [kernel-hardening] Re: [PATCH v3 04/13] crypto/rng: ensure that the RNG is ready before using

[Theodore Ts'o]

On Wed, Jun 07, 2017 at 07:00:17AM +0200, Stephan Müller wrote:
> > On that same idea, one could add an early_initramfs handler for entropy
> > data.
> 
> Any data that comes from outside during the boot process, be it some NVRAM 
> location, the /var/lib...seed file for /dev/random or other approaches are 
> viewed by a number of folks to have zero bits of entropy.

The Open BSD folks would disagree with you.  They've designed their
whole system around saving entropy at shutdown, reading it as early as
possible by the bootloader, and then as soon as possible after the
reboot, to overwrite and reinitialize the entropy seed stored on disk
so that on a reboot after a crash.  They consider this good enough to
assume that their CRNG is *always* strongly initialized.

I'll let you have that discussion/argument with Theo de Raadt, though.
Be warned that he has opinions about security that are almost
certainly as strong (and held with the same level of certainty) as a
certain Brad Spengler...

- Ted

Re: [PATCH v2 net-next 1/4] tcp: ULP infrastructure

[David Miller]

From: Dave Watson 
Date: Tue, 6 Jun 2017 10:00:20 -0700

> +EXPORT_SYMBOL(tcp_register_ulp);

EXPORT_SYMBOL_GPL() please.

> +EXPORT_SYMBOL(tcp_unregister_ulp);

Likewise.

Re: [PATCH v3 03/13] random: invalidate batched entropy after crng init

[Jason A. Donenfeld]

Strange, not all compilers do this warning. Fixing with:

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 12758db..5252690 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -2061,8 +2061,8 @@ static DEFINE_PER_CPU(struct batched_entropy,
batched_entropy_u64);
u64 get_random_u64(void)
{
   u64 ret;
-   bool use_lock = crng_init < 2;
-   unsigned long flags;
+   const bool use_lock = READ_ONCE(crng_init) < 2;
+   unsigned long flags = 0;
   struct batched_entropy *batch;

#if BITS_PER_LONG == 64
@@ -2099,8 +2099,8 @@ static DEFINE_PER_CPU(struct batched_entropy,
batched_entropy_u32);
u32 get_random_u32(void)
{
   u32 ret;
-   bool use_lock = crng_init < 2;
-   unsigned long flags;
+   const bool use_lock = READ_ONCE(crng_init) < 2;
+   unsigned long flags = 0;
   struct batched_entropy *batch;

   if (arch_get_random_int())

Const, because it's more correct. READ_ONCE to be certain that the
compiler doesn't try to paste the expression into both uses. And
finally flags=0 to shutup the compiler.

If anybody has a better way of approaching this, feel free to chime in.

Re: [PATCH v3 03/13] random: invalidate batched entropy after crng init

[kbuild test robot]

Hi Jason,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.12-rc4]
[cannot apply to next-20170607]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Jason-A-Donenfeld/Unseeded-In-Kernel-Randomness-Fixes/20170606-171249
config: sh-ul2_defconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   In file included from include/linux/spinlock.h:53:0,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:10,
from include/linux/pid.h:4,
from include/linux/sched.h:13,
from include/linux/utsname.h:5,
from drivers//char/random.c:238:
   drivers//char/random.c: In function 'get_random_u64':
>> include/linux/irqflags.h:69:3: warning: 'flags' may be used uninitialized in 
>> this function [-Wmaybe-uninitialized]
  arch_local_irq_restore(flags);  \
  ^~
   drivers//char/random.c:2063:16: note: 'flags' was declared here
 unsigned long flags;
   ^
   In file included from include/linux/spinlock.h:53:0,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:10,
from include/linux/pid.h:4,
from include/linux/sched.h:13,
from include/linux/utsname.h:5,
from drivers//char/random.c:238:
   drivers//char/random.c: In function 'get_random_u32':
>> include/linux/irqflags.h:69:3: warning: 'flags' may be used uninitialized in 
>> this function [-Wmaybe-uninitialized]
  arch_local_irq_restore(flags);  \
  ^~
   drivers//char/random.c:2095:16: note: 'flags' was declared here
 unsigned long flags;
   ^
--
   In file included from include/linux/spinlock.h:53:0,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:10,
from include/linux/pid.h:4,
from include/linux/sched.h:13,
from include/linux/utsname.h:5,
from drivers/char/random.c:238:
   drivers/char/random.c: In function 'get_random_u64':
>> include/linux/irqflags.h:69:3: warning: 'flags' may be used uninitialized in 
>> this function [-Wmaybe-uninitialized]
  arch_local_irq_restore(flags);  \
  ^~
   drivers/char/random.c:2063:16: note: 'flags' was declared here
 unsigned long flags;
   ^
   In file included from include/linux/spinlock.h:53:0,
from include/linux/rcupdate.h:38,
from include/linux/rculist.h:10,
from include/linux/pid.h:4,
from include/linux/sched.h:13,
from include/linux/utsname.h:5,
from drivers/char/random.c:238:
   drivers/char/random.c: In function 'get_random_u32':
>> include/linux/irqflags.h:69:3: warning: 'flags' may be used uninitialized in 
>> this function [-Wmaybe-uninitialized]
  arch_local_irq_restore(flags);  \
  ^~
   drivers/char/random.c:2095:16: note: 'flags' was declared here
 unsigned long flags;
   ^

vim +/flags +69 include/linux/irqflags.h

81d68a96a Steven Rostedt 2008-05-12  53  # define start_critical_timings() do { 
} while (0)
81d68a96a Steven Rostedt 2008-05-12  54  #endif
81d68a96a Steven Rostedt 2008-05-12  55  
df9ee2927 David Howells  2010-10-07  56  /*
df9ee2927 David Howells  2010-10-07  57   * Wrap the arch provided IRQ routines 
to provide appropriate checks.
df9ee2927 David Howells  2010-10-07  58   */
df9ee2927 David Howells  2010-10-07  59  #define raw_local_irq_disable()
arch_local_irq_disable()
df9ee2927 David Howells  2010-10-07  60  #define raw_local_irq_enable() 
arch_local_irq_enable()
df9ee2927 David Howells  2010-10-07  61  #define raw_local_irq_save(flags)  
\
df9ee2927 David Howells  2010-10-07  62 do {
\
df9ee2927 David Howells  2010-10-07  63 typecheck(unsigned 
long, flags);\
df9ee2927 David Howells  2010-10-07  64 flags = 
arch_local_irq_save();  \
df9ee2927 David Howells  2010-10-07  65 } while (0)
df9ee2927 David Howell

Re: [kernel-hardening] Re: [PATCH v3 04/13] crypto/rng: ensure that the RNG is ready before using

[Mark Rutland]

On Tue, Jun 06, 2017 at 01:03:19PM -0400, Theodore Ts'o wrote:
> The other approach is to find a way to have initialized "seed" entropy
> which we can count on at every boot.  The problem is that this is very
> much dependent on how the bootloader works.  It's easy to say "store
> it in the kernel", but where the kernel is stored varies greatly from
> architecture to architecture.  In some cases, the kernel can stored in
> ROM, where it can't be modified at all.
> 
> It might be possible, for example, to store a cryptographic key in a
> UEFI boot-services variable, where the key becomes inaccessible after
> the boot-time services terminate.  But you also need either a reliable
> time-of-day clock, or a reliable counter which is incremented each
> time the system that boots, and which can't be messed with by an
> attacker, or trivially reset by a clueless user/sysadmin.

FWIW, EFI has an (optional) EFI_RNG_PROTOCOL, that we currently use to
seed the kernel's entropy pool. The EFI stub creates a config table with
the entropy, which the kernel reads.

This is re-seeded prior to kexec() to avoid the entropy being recycled.

See commits:

636259880a7e7d34 ("efi: Add support for seeding the RNG from a UEFI config 
table")
568bc4e87033d232 (" efi/arm*/libstub: Invoke EFI_RNG_PROTOCOL to seed the UEFI 
RNG table")

Unfortunately, I beleive that support for the protocol is currently rare
in practice.

Thanks,
Mark.

Re: [kernel-hardening] Re: [PATCH v3 04/13] crypto/rng: ensure that the RNG is ready before using

[Mark Rutland]

On Wed, Jun 07, 2017 at 01:00:25PM -0400, Daniel Micay wrote:
> > On the better bootloaders, an initramfs segment can be loaded
> > independently (and you can have as many as required), which makes an
> > early_initramfs a more palatable vector to inject large amounts of
> > entropy into the next boot than, say, modifying the kernel image
> > directly at every boot/shutdown to stash entropy in there somewhere.

[...]
 
> I didn't really understand the device tree approach and mentioned a
> few times before. Passing via the kernel cmdline is a lot simpler than
> modifying the device tree in-memory and persistent modification isn't
> an option unless verified boot is missing anyway.

I might be missing something here, but the command line is inside of the
device tree, at /chosen/bootargs, so modifying the kernel command line
*is* modifying the device tree in-memory.

For arm64, we have a /chosen/kaslr-seed property that we hope
FW/bootloaders fill in, and similar could be done for some initial
entropy, provided appropriate HW/FW support.

Thanks,
Mark.

Re: [kernel-hardening] Re: [PATCH v3 04/13] crypto/rng: ensure that the RNG is ready before using

[Daniel Micay]

> On the better bootloaders, an initramfs segment can be loaded
> independently (and you can have as many as required), which makes an
> early_initramfs a more palatable vector to inject large amounts of
> entropy into the next boot than, say, modifying the kernel image
> directly at every boot/shutdown to stash entropy in there somewhere.

Modifying the kernel image on storage isn't compatible with verified
boot so it's not really a solution. The kernel, initrd and rest of the
OS are signed and verified on operating systems like Android, Android
Things, ChromeOS and many embedded devices, etc. putting some basic
effort into security. I didn't really understand the device tree
approach and mentioned a few times before. Passing via the kernel
cmdline is a lot simpler than modifying the device tree in-memory and
persistent modification isn't an option unless verified boot is missing
anyway.

Re: [PATCH 1/4] dt-bindings: rng: add generic bindings for MediaTek SoCs

[Sean Wang]

On Wed, 2017-06-07 at 15:25 +0200, Matthias Brugger wrote:
> 
> On 07/06/17 15:20, Sean Wang wrote:
> > On Tue, 2017-06-06 at 13:07 +0200, Matthias Brugger wrote:
> >>
> >> On 31/05/17 20:44, sean.w...@mediatek.com wrote:
> >>> From: Sean Wang 
> >>>
> >>> Add the generic binding for allowing the support of RNG on MediaTek SoCs
> >>> such as MT7622.
> >>>
> >>> Signed-off-by: Sean Wang 
> >>> ---
> >>>Documentation/devicetree/bindings/rng/mtk-rng.txt | 3 ++-
> >>>1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/rng/mtk-rng.txt 
> >>> b/Documentation/devicetree/bindings/rng/mtk-rng.txt
> >>> index a6d62a2..0772913 100644
> >>> --- a/Documentation/devicetree/bindings/rng/mtk-rng.txt
> >>> +++ b/Documentation/devicetree/bindings/rng/mtk-rng.txt
> >>> @@ -2,7 +2,8 @@ Device-Tree bindings for Mediatek random number generator
> >>>found in Mediatek SoC family
> >>>
> >>>Required properties:
> >>> -- compatible : Should be "mediatek,mt7623-rng"
> >>> +- compatible : Should be "mediatek,generic-rng" or
> >>> + "mediatek,mt7623-rng".
> >>
> >> What does generic-rng mean. Is it for all mt7xxx, or also for mt6xxx and
> >> mt8xxx based SoCs? I think we should stick with SoC specific bindings,
> >> as we don't know if Mediatek won't publish a new IP block next year
> >> which is differnet.
> >>
> > 
> > Yes, what I mean is generic-rng can be applied to all
> > platform MediaTek provides.
> > 
> > 
> >> Just in case we should add a binding for the actual SoC + a fallback.
> >> For example.
> >> - compatible " Should be
> >>"mediatek,mt7622-rng",  "mediatek,mt7623-rng" for SoC mt7622
> >>"mediatek,mt7623-rng" for SoC mt7623
> >>
> >> This will also eliminate the need of adding mt6722-rng to the driver, as
> >> it will use mt7623-rng as fallback. If in the future we realize that
> >> mt7622-rng has a extra feature/bug, we can still work around it, without
> >> breaking the bindings.
> >>
> > 
> > I knew the fallback rules you said here because I saw them being used in
> > many drivers such as sysirq and uart driver, such kind of basic drivers.
> > 
> > These drivers are basic enough, various following chipsets almost fall
> > back into the oldest one. So the clues let me think the hardware
> > interface shouldn't have too much differences among them.
> > 
> > If there is string used like generic-uart or generic-sysirq, it can
> > stop we blindly add new string into the binding document when a new
> > platform is introduced.
> > 
> > And they easily allows users unfamiliar MediaTek platform (they didn't
> > know what the oldest MediaTek chipset is) pick up the right compatible
> > string to start bring up the new platform.
> > 
> > The specific one can be added after new feature required is added or
> > critical hardware bug is found. Otherwise the generic one can fit
> > all generic needs for those.
> > 
> > Those are only opinions, if you don't like it, I still can accept the
> > original way as you suggest :)
> > 
> 
> I can see your reasoning, but the device tree maintainers prefer to have 
> the bindings updated for a new SoC. As I mentioned before, just imagine 
> next year Mediatek changes the IP block and from now on, it uses the new 
> device in all SoCs. In 5 years we would have a binding which states 
> 'generic' although it is not compatible with any SoC of the last So
> 
> So please keep with the bindings as done up to now.

Understood, I'll follow up. 

And that also reminds me that all missing bindings need to be added for
each driver on the MT7623 which can fall back to other existing SoCs.

Thank you a lot

Sean

> 
> Best regards,
> Matthias
> 
> > 
> >> Makes sense?
> >>
> >> Regards,
> >> Matthias
> >>
> >>>- clocks   : list of clock specifiers, corresponding to
> >>> entries in clock-names property;
> >>>- clock-names  : Should contain "rng" entries;
> >>>
> > 
> > 

Re: [kernel-hardening] Re: [PATCH v3 04/13] crypto/rng: ensure that the RNG is ready before using

[Henrique de Moraes Holschuh]

On Wed, 07 Jun 2017, Stephan Müller wrote:
> Am Mittwoch, 7. Juni 2017, 00:19:10 CEST schrieb Henrique de Moraes Holschuh:
> > On that same idea, one could add an early_initramfs handler for entropy
> > data.
> 
> Any data that comes from outside during the boot process, be it some NVRAM 
> location, the /var/lib...seed file for /dev/random or other approaches are 
> viewed by a number of folks to have zero bits of entropy.
> 
> I.e. this data is nice for stirring the pool, but is not considered to help 
> our entropy problem.

Stirring the pool is actually the objective, not entropy accounting.
Let's not lose sight of what is really important.

But yes, you are quite correct in that it won't help anything that would
like to block due to entropy accouting, or which needs to be certain
about the amount of randomness in the pools.

-- 
  Henrique Holschuh

Re: Can someone check linux kernel 4.4, and 4.9 rfc4309 test vectors?

[Stephan Müller]

Am Mittwoch, 7. Juni 2017, 15:57:31 CEST schrieb Che-Min Hsieh:

Hi Che,

> Rfc4309 test vectors in testmgr.h have gone through major changes from
> linux3  to linux4. In linux 4.4, linux4.9, there are vectors as such

I think you and the kernel implement crypto properly. It is just the 
formatting that you do not get right.

See crypto/ccm.c:

static struct aead_request *crypto_rfc4309_crypt(struct aead_request *req)
{
...
scatterwalk_map_and_copy(iv + 16, req->src, 0, req->assoclen - 8, 0);
...

The key is how to understand the input data format. RFC4309 CCM is no cipher 
implementation, but rather a special formatting of the CCM input data.

In your code, change the following line

>// Add the AAD
> EVP_EncryptUpdate(cryptCtx, 0, , A, sizeof(A));

to

EVP_EncryptUpdate(cryptCtx, 0, , A, sizeof(A) - 8);

and you will see consistent results.

Ciao
Stephan

Can someone check linux kernel 4.4, and 4.9 rfc4309 test vectors?

[Che-Min Hsieh]

Rfc4309 test vectors in testmgr.h have gone through major changes from linux3  
to linux4.
In linux 4.4, linux4.9, there are vectors as such

23194 static struct aead_testvec aes_ccm_rfc4309_enc_tv_template[] = {
23195{ /* Generated using Crypto++ */
23196.key   = zeroed_string,
23197.klen  = 19,
23198.iv   = zeroed_string,
23199.input= zeroed_string,
23200.ilen   = 16,
23201.assoc= zeroed_string,
23202.alen  = 16,
23203.result   = "\x2E\x9A\xCA\x6B\xDA\x54\xFC\x6F"
23204  "\x12\x50\xE8\xDE\x81\x3C\x63\x08"
23205  "\x1A\x22\xBA\x75\xEE\xD4\xD5\xB5"
23206  "\x27\x50\x01\xAC\x03\x33\x39\xFB",
23207.rlen   = 32,


I have a test program using open ssl API (-l crypto), and run on Ubuntu Linux 
PC,   I  get the following  test result:

2e 9a ca 6b da 54 fc 6f 12 50 e8 de 81 3c 63 08
fb 64 91 b4 dd dc bf 5d fd 67 e3 a2 f8 7c 0e 6c
  The first part of encrypted text is correct. But MAC is not the 
same.

My program is as the following:

void ccmTest()
{
/* Initialization */
EVP_CIPHER_CTX ctx;
EVP_CIPHER_CTX *cryptCtx = 
EVP_CIPHER_CTX_init(cryptCtx);
int i;

   unsigned char P[16] = {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0};
int Psize = sizeof(P);
unsigned char K[16] = {0};
unsigned char N[11] = {0};
unsigned char A[16] = {0};
unsigned char CT[128];

int Nsize = 11;
int Tsize = 16;

// Initialize the context with the alg only
EVP_EncryptInit(cryptCtx, EVP_aes_128_ccm(), 0, 0);

// Set nonce and tag sizes
EVP_CIPHER_CTX_ctrl(cryptCtx, EVP_CTRL_CCM_SET_IVLEN, Nsize, 0);
EVP_CIPHER_CTX_ctrl(cryptCtx, EVP_CTRL_CCM_SET_TAG, Tsize, 0);

// Finally set the key and the nonce
EVP_EncryptInit(cryptCtx, 0, K, N);

// Tell the alg we will encrypt Psize bytes
int outl = 0;
EVP_EncryptUpdate(cryptCtx, 0, , 0, sizeof(P));
   // Add the AAD
EVP_EncryptUpdate(cryptCtx, 0, , A, sizeof(A));
   // Now we encrypt the data in P, placing the output in CT
EVP_EncryptUpdate(cryptCtx, CT, , P, Psize);
EVP_EncryptFinal(cryptCtx, [outl], );
// Append the tag to the end of the encrypted output
EVP_CIPHER_CTX_ctrl(cryptCtx, EVP_CTRL_CCM_GET_TAG, Tsize, [Psize]);
hexdump(CT, Tsize+Psize);
}


I run "insmod tcrypt.ko mode=45"  rfc4309 test with Qualcomm crypto hardware on 
Linux4.4. The test fails. The generated output is the same as my openSSL test 
application in 1.  

My test application runs on Ubuntu with linux 3.10 rfc4309 test vector, and 
generated MAC as expected from test vectors.  Qualcomm crypto hardware runs 
"insmod tcrypt.ko mode=45" successfully with linux 3.10.

I am suspicious about the test vectors of 4.4. Can someone verify the Linux 4.4 
rfc4309 test vectors with his/her openSSL application on PC?

Chemin

Re: [PATCH 1/4] dt-bindings: rng: add generic bindings for MediaTek SoCs

[Matthias Brugger]

On 07/06/17 15:20, Sean Wang wrote:

On Tue, 2017-06-06 at 13:07 +0200, Matthias Brugger wrote:


On 31/05/17 20:44, sean.w...@mediatek.com wrote:

From: Sean Wang 

Add the generic binding for allowing the support of RNG on MediaTek SoCs
such as MT7622.

Signed-off-by: Sean Wang 
---
   Documentation/devicetree/bindings/rng/mtk-rng.txt | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/rng/mtk-rng.txt 
b/Documentation/devicetree/bindings/rng/mtk-rng.txt
index a6d62a2..0772913 100644
--- a/Documentation/devicetree/bindings/rng/mtk-rng.txt
+++ b/Documentation/devicetree/bindings/rng/mtk-rng.txt
@@ -2,7 +2,8 @@ Device-Tree bindings for Mediatek random number generator
   found in Mediatek SoC family
   
   Required properties:

-- compatible   : Should be "mediatek,mt7623-rng"
+- compatible   : Should be "mediatek,generic-rng" or
+   "mediatek,mt7623-rng".


What does generic-rng mean. Is it for all mt7xxx, or also for mt6xxx and
mt8xxx based SoCs? I think we should stick with SoC specific bindings,
as we don't know if Mediatek won't publish a new IP block next year
which is differnet.



Yes, what I mean is generic-rng can be applied to all
platform MediaTek provides.



Just in case we should add a binding for the actual SoC + a fallback.
For example.
- compatible " Should be
"mediatek,mt7622-rng","mediatek,mt7623-rng" for SoC mt7622
"mediatek,mt7623-rng" for SoC mt7623

This will also eliminate the need of adding mt6722-rng to the driver, as
it will use mt7623-rng as fallback. If in the future we realize that
mt7622-rng has a extra feature/bug, we can still work around it, without
breaking the bindings.



I knew the fallback rules you said here because I saw them being used in
many drivers such as sysirq and uart driver, such kind of basic drivers.

These drivers are basic enough, various following chipsets almost fall
back into the oldest one. So the clues let me think the hardware
interface shouldn't have too much differences among them.

If there is string used like generic-uart or generic-sysirq, it can
stop we blindly add new string into the binding document when a new
platform is introduced.

And they easily allows users unfamiliar MediaTek platform (they didn't
know what the oldest MediaTek chipset is) pick up the right compatible
string to start bring up the new platform.

The specific one can be added after new feature required is added or
critical hardware bug is found. Otherwise the generic one can fit
all generic needs for those.

Those are only opinions, if you don't like it, I still can accept the
original way as you suggest :)



I can see your reasoning, but the device tree maintainers prefer to have 
the bindings updated for a new SoC. As I mentioned before, just imagine 
next year Mediatek changes the IP block and from now on, it uses the new 
device in all SoCs. In 5 years we would have a binding which states 
'generic' although it is not compatible with any SoC of the last So


So please keep with the bindings as done up to now.

Best regards,
Matthias




Makes sense?

Regards,
Matthias


   - clocks : list of clock specifiers, corresponding to
  entries in clock-names property;
   - clock-names: Should contain "rng" entries;

Re: [PATCH 1/4] dt-bindings: rng: add generic bindings for MediaTek SoCs

[Sean Wang]

On Tue, 2017-06-06 at 13:07 +0200, Matthias Brugger wrote:
> 
> On 31/05/17 20:44, sean.w...@mediatek.com wrote:
> > From: Sean Wang 
> > 
> > Add the generic binding for allowing the support of RNG on MediaTek SoCs
> > such as MT7622.
> > 
> > Signed-off-by: Sean Wang 
> > ---
> >   Documentation/devicetree/bindings/rng/mtk-rng.txt | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/rng/mtk-rng.txt 
> > b/Documentation/devicetree/bindings/rng/mtk-rng.txt
> > index a6d62a2..0772913 100644
> > --- a/Documentation/devicetree/bindings/rng/mtk-rng.txt
> > +++ b/Documentation/devicetree/bindings/rng/mtk-rng.txt
> > @@ -2,7 +2,8 @@ Device-Tree bindings for Mediatek random number generator
> >   found in Mediatek SoC family
> >   
> >   Required properties:
> > -- compatible   : Should be "mediatek,mt7623-rng"
> > +- compatible   : Should be "mediatek,generic-rng" or
> > +   "mediatek,mt7623-rng".
> 
> What does generic-rng mean. Is it for all mt7xxx, or also for mt6xxx and 
> mt8xxx based SoCs? I think we should stick with SoC specific bindings, 
> as we don't know if Mediatek won't publish a new IP block next year 
> which is differnet.
> 

Yes, what I mean is generic-rng can be applied to all
platform MediaTek provides. 


> Just in case we should add a binding for the actual SoC + a fallback. 
> For example.
> - compatible " Should be
>   "mediatek,mt7622-rng",  "mediatek,mt7623-rng" for SoC mt7622
>   "mediatek,mt7623-rng" for SoC mt7623
> 
> This will also eliminate the need of adding mt6722-rng to the driver, as 
> it will use mt7623-rng as fallback. If in the future we realize that 
> mt7622-rng has a extra feature/bug, we can still work around it, without 
> breaking the bindings.
> 

I knew the fallback rules you said here because I saw them being used in
many drivers such as sysirq and uart driver, such kind of basic drivers.

These drivers are basic enough, various following chipsets almost fall
back into the oldest one. So the clues let me think the hardware
interface shouldn't have too much differences among them.

If there is string used like generic-uart or generic-sysirq, it can
stop we blindly add new string into the binding document when a new
platform is introduced. 

And they easily allows users unfamiliar MediaTek platform (they didn't
know what the oldest MediaTek chipset is) pick up the right compatible
string to start bring up the new platform. 

The specific one can be added after new feature required is added or
critical hardware bug is found. Otherwise the generic one can fit 
all generic needs for those.

Those are only opinions, if you don't like it, I still can accept the
original way as you suggest :)


> Makes sense?
> 
> Regards,
> Matthias
> 
> >   - clocks  : list of clock specifiers, corresponding to
> >   entries in clock-names property;
> >   - clock-names : Should contain "rng" entries;
> > 

Re: [PATCH v4 00/13] Unseeded In-Kernel Randomness Fixes

[Jason A. Donenfeld]

Hi Ted,

Could I get your Signed-off-by on this patchset, so that somebody can
add it to their tree?

Thanks,
Jason