Re: [PATCH 3/3] crypto: hwrng add sysfs attribute to show user selected rng

[PrasannaKumar Muralidharan]

Hi Harald,

> Here is an updated version with just showing 0 or 1 in the new sysfs
> attribute file:
> == cut ==
> From: Harald Freudenberger 
> Date: Mon, 3 Jul 2017 10:19:22 +0200
> Subject: [PATCH 3/3] crypto: hwrng add sysfs attribute to show user selected
>  rng
>
> This patch introduces a new sysfs attribute file 'rng_selected'
> which shows if the current rng has been chosen by userspace.
>
> If a rng source is chosen by user via echo some valid string
> to rng_current there should be a way to signal this choice to
> userspace. The new attribute file 'rng_selected' shows '1' for
> user selecte rng and '0' otherwise.
>
> Signed-off-by: Harald Freudenberger 
> ---
>  drivers/char/hw_random/core.c | 21 -
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index ffd4e36..2b4c7f6 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -28,8 +28,8 @@
>  #define RNG_MODULE_NAME"hw_random"
>
>  static struct hwrng *current_rng;
> -/* the current rng has been explicitly chosen by user via sysfs */
> -static int cur_rng_set_by_user;
> +/* the rng explicitly selected by user via sysfs */
> +static struct hwrng *selected_rng;
>  static struct task_struct *hwrng_fill;
>  /* list of registered rngs, sorted decending by quality */
>  static LIST_HEAD(rng_list);
> @@ -306,7 +306,7 @@ static ssize_t hwrng_attr_current_store(struct device 
> *dev,
>  list_for_each_entry(rng, _list, list) {
>  if (sysfs_streq(rng->name, buf)) {
>  err = 0;
> -cur_rng_set_by_user = 1;
> +selected_rng = rng;
>  if (rng != current_rng)
>  err = set_current_rng(rng);
>  break;
> @@ -355,16 +355,27 @@ static ssize_t hwrng_attr_available_show(struct device 
> *dev,
>  return strlen(buf);
>  }
>
> +static ssize_t hwrng_attr_selected_show(struct device *dev,
> +struct device_attribute *attr,
> +char *buf)
> +{
> +return snprintf(buf, PAGE_SIZE, "%d\n", selected_rng ? 1 : 0);
> +}
> +
>  static DEVICE_ATTR(rng_current, S_IRUGO | S_IWUSR,
> hwrng_attr_current_show,
> hwrng_attr_current_store);
>  static DEVICE_ATTR(rng_available, S_IRUGO,
> hwrng_attr_available_show,
> NULL);
> +static DEVICE_ATTR(rng_selected, S_IRUGO,
> +   hwrng_attr_selected_show,
> +   NULL);
>
>  static struct attribute *rng_dev_attrs[] = {
>  _attr_rng_current.attr,
>  _attr_rng_available.attr,
> +_attr_rng_selected.attr,
>  NULL
>  };
>
> @@ -448,7 +459,7 @@ int hwrng_register(struct hwrng *rng)
>  old_rng = current_rng;
>  err = 0;
>  if (!old_rng ||
> -(!cur_rng_set_by_user && rng->quality > old_rng->quality)) {
> +(!selected_rng && rng->quality > old_rng->quality)) {
>  /*
>   * Set new rng as current as the new rng source
>   * provides better entropy quality and was not
> @@ -484,7 +495,7 @@ void hwrng_unregister(struct hwrng *rng)
>  list_del(>list);
>  if (current_rng == rng) {
>  drop_current_rng();
> -cur_rng_set_by_user = 0;
> +selected_rng = NULL;
>  /* rng_list is sorted by quality, use the best (=first) one */
>  if (!list_empty(_list)) {
>  struct hwrng *new_rng;
> --
> 2.7.4
> == cut ==
>

Nice to see quick turn around time. Why not just use the
cur_rng_set_by_user instead of selected_rng? This way patch 3 won't
change things that patch 2 introduced and patch 3 will become much
smaller. Also I feel folding patch 3 and 2 will make sense as both are
related to user selected rng.

Regards,
PrasannaKumar

Re: Crypto Update for 4.13

[Linus Torvalds]

On Wed, Jul 5, 2017 at 6:01 AM, Herbert Xu  wrote:
>
> Drivers:
>
> - Add support for CNN55XX adapters in cavium.

Grr. I noticed this too late to fix it in the merge.

That stupid CNN55XX driver was added with a default of "m"?

WTF? Hell no. We don't add random new drivers and default them on -
and we do so even less when they are for very unusual hardware.

  Linus

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

[Theodore Ts'o]

On Wed, Jul 05, 2017 at 09:16:09AM -0400, Paul Koning wrote:
> 
> In the implementations I know, /dev/random and /dev/urandom are the
> same driver, the only difference is that when you read from
> /dev/random there's a check for the current entropy level.

It's in the same driver but /dev/random and /dev/urandom are different
beasts.  Pre-4.9 we use the same SHA-1 based generator, but we use
different state pools, and we periodically "catastrophically reseed"
(ala Yarrow) from the random pool to the urandom pool.  Getrandom(2)
uses the urandom pool.

In 4.9 and later kernels, /dev/urandom and getrandom(2) use a ChaCha20
based generator which provides for more speed.  We still use the SHA-1
pool for the random pool because it allows for a much larger "state
pool" to store entropy.

> If you have a properly constructed RNG, as soon as it's been fed
> enough entropy it is secure (at least for the next 2^64 bits or so).
> The notion of "using up entropy" is not meaningful for a good
> generator.  See Bruce Schneier's "Yarrow" paper for the details.

A lot of this depends on whether or not you trust your crypto
primitives or not.  The /dev/random driver was the very first OS-based
random generator, and back then, export restrictions were still a
thing (which is why we only used MD5 and SHA-1 as crypto primitives),
and our cryptoanalytic understanding of what makes for a good crypto
hash or encryption algorithm was quite limited.  So trying to
accumulate a large amount of entropy pool of entropy is a good thing,
because even if there was a minor weakness in the crypto hash (and for
a while we were using MD5), relying on the adversary not being able to
guess all of the environmental noise harvested by the kernel would
cover for a lot of sins.

Remember, in the kernel we have access to a large amount of
environmental noise, so it makes a lot of sense to take advantage of
that as much as possible.  So by having a huge state pool for the
/dev/random entropy pool, we can harvest and store as much of that
environmental noise as possible.  This buys us a large amount of
safety margin, which is good thing because somewhere there might be
some Linux 2.0 or Linux 2.2 based router sitting in someone's home
where /dev/random is using MD5.  Those ancient kernels are probably
riddled with zero-day security holes, but the safety margin of using a
large entropy pool is such that even though there are many known
attacks against MD5, I'm actually pretty confident that the large
state pool mitigates the weakness of MD5 as used by /dev/random and
/dev/urandom.  At the very least, it will be much easier for the NSA
to use some other zero-day to attack the said router with the ancient
kernel, well before they try to reverse engineer its /dev/urandom
output.  :-)

- Ted

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

[Paul Koning]

> On Jul 5, 2017, at 3:08 AM, Ulrich Windl  
> wrote:
> 
 Jeffrey Walton  schrieb am 17.06.2017 um 16:23 in 
 Nachricht
> :
> 
> [...]
>> But its not clear to me how to ensure uniqueness when its based on
>> randomness from the generators.
> 
> Even with a perfect random generator non-unique values are possible (that's 
> why it's random). It's unlikely, but it can happen. The question is whether 
> the probability of non-unique values from /dev/urandom is any higher than 
> that for values read from /dev/random. One _might_ be able to predict the 
> values from /dev/urandom.

In the implementations I know, /dev/random and /dev/urandom are the same 
driver, the only difference is that when you read from /dev/random there's a 
check for the current entropy level.

If you haven't fed enough entropy yet to the driver since startup, and you read 
/dev/urandom, you get a value that isn't sufficiently secure.  

If you have a properly constructed RNG, as soon as it's been fed enough entropy 
it is secure (at least for the next 2^64 bits or so).  The notion of "using up 
entropy" is not meaningful for a good generator.   See Bruce Schneier's 
"Yarrow" paper for the details.

paul

Crypto Update for 4.13

[Herbert Xu]

Hi Linus: 

Here is the crypto update for 4.13:

Algorithms:

- Add private key generation to ecdh.

Drivers:

- Add generic gcm(aes) to aesni-intel.
- Add SafeXcel EIP197 crypto engine driver.
- Add ecb(aes), cfb(aes) and ecb(des3_ede) to cavium.
- Add support for CNN55XX adapters in cavium.
- Add ctr mode to chcr.
- Add support for gcm(aes) to omap.


Please pull from

git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git linus


Antoine Ténart (24):
  crypto: inside-secure - add SafeXcel EIP197 crypto engine driver
  MAINTAINERS: add a maintainer for the Inside Secure crypto driver
  crypto: sun4i-ss - group variable definitions in sun4i_hash()
  crypto: sun4i-ss - remove conditional checks against 0
  crypto: sun4i-ss - use lower/upper_32_bits helpers
  crypto: sun4i-ss - cannot use DMA is the request is 0 length
  crypto: sun4i-ss - do not dynamically set parts of the last buffer to 0
  crypto: sun4i-ss - simplify the pad length calculation
  crypto: sun4i-ss - simplify the appended bit assignment
  crypto: sun4i-ss - use GENMASK to generate masks
  crypto: sun4i-ss - move from ablkcipher to skcipher API
  crypto: sun4i-ss - add the CRYPTO_ALG_KERN_DRIVER_ONLY flag
  crypto: sun4i-ss - fix large block size support
  crypto: inside-secure - use hmac ipad/opad constants
  crypto: inside-secure - fix the ring wr_cache offset
  crypto: inside-secure - fix incorrect DSE data cache setting
  crypto: inside-secure - update the context and request later
  crypto: inside-secure - use one queue per hw ring
  crypto: inside-secure - stop requeueing failed requests
  crypto: inside-secure - get the backlog before dequeueing the request
  crypto: inside-secure - only dequeue when needed
  crypto: inside-secure - increase the batch size
  crypto: inside-secure - use the base_end pointer in ring rollback
  Documentation/bindings: Document the SafeXel cryptographic engine driver

Ard Biesheuvel (6):
  crypto: arm64/sha - avoid non-standard inline asm tricks
  crypto: arm/aes-ce - enable module autoloading based on CPU feature bits
  crypto: arm/ghash-ce - enable module autoloading based on CPU feature bits
  crypto: arm/sha1-ce - enable module autoloading based on CPU feature bits
  crypto: arm/sha2-ce - enable module autoloading based on CPU feature bits
  crypto: arm/crc32 - enable module autoloading based on CPU feature bits

Arvind Yadav (4):
  hwrng: omap3-rom - Handle return value of clk_prepare_enable
  crypto: img-hash - Handle return value of clk_prepare_enable
  crypto: n2 - make of_device_ids const
  crypto: caam - make of_device_ids const.

Benjamin Peterson (1):
  crypto: doc - fix typo in docs

Christoph Hellwig (1):
  crypto: qat - use pcie_flr instead of duplicating it

Christophe Jaillet (1):
  crypto: crypto4xx - fix an error code

Colin Ian King (3):
  crypto: brcm - fix spelling mistake: "fallbck" -> "fallback"
  crypto: omap-aes - fix spelling mistake "Encryptio" -> "Encryption"
  crypto: cavium - fix spelling mistake "Revsion" -> "Revision"

Corentin LABBE (10):
  crypto: hmac - add hmac IPAD/OPAD constant
  crypto: brcm - Use IPAD/OPAD constant
  crypto: ixp4xx - Use IPAD/OPAD constant
  crypto: marvell - Use IPAD/OPAD constant
  crypto: mv_cesa - Use IPAD/OPAD constant
  crypto: omap-sham - Use IPAD/OPAD constant
  crypto: qat - Use IPAD/OPAD constant
  crypto: mediatek - Use IPAD/OPAD constant
  crypto: ccp - Use IPAD/OPAD constant
  crypto: engine - replace pr_xxx by dev_xxx

Dan Carpenter (4):
  crypto: sha512-mb - add some missing unlock on error
  X.509: Fix error code in x509_cert_parse()
  crypto: glue_helper - Delete some dead code
  crypto: cavium/nitrox - dma_mapping_error() returns bool

Eric Biggers (2):
  crypto: x86/aes - Don't use %rbp as temporary register
  crypto: aes_ti - fix comment for MixColumns step

Gary R Hook (3):
  crypto: ccp - Add a module author
  crypto: ccp - Add debugfs entries for CCP information
  crypto: ccp - Release locks before returning

Geliang Tang (1):
  crypto: mediatek - drop .owner field in mtk_crypto_driver

George Cherian (3):
  crypto: cavium - Downgrade the annoying misc interrupt print from dev_err 
to dev_dbg
  crypto: cavium - Remove the individual encrypt/decrypt function for each 
algorithm
  crypto: cavium - Add more algorithms

Gilad Ben-Yossef (2):
  crypto: tcrypt - don't disable irqs and wait
  crypto: testmgr - use consistent format for errors

Harsh Jain (9):
  crypto: chcr - Pass lcb bit setting to firmware
  crypto: chcr - Fix fallback key setting
  crypto: chcr - Return correct error code
  crypto: chcr - Avoid changing request structure
  crypto: chcr - Add ctr mode and process large sg entries for cipher
  chcr - Add debug counters
  

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

[Theodore Ts'o]

On Wed, Jul 05, 2017 at 09:03:43AM +0200, Ulrich Windl wrote:
> > Note, during the development of my /dev/random implementation, I added the 
> > getrandom-like blocking behavior to /dev/urandom (which is the equivalent to
> > Jason's patch except that it applies to user space). The boot process locked
> 
> I thought reads from urandom never block by definition. An older manual page
> (man urandom) also says: "A  read  from  the  /dev/urandom device will not
> block waiting for more entropy."

As I said in my original message, I *tried* this as an experiment.
Because lots of security-obsessed people were disputing my
intelligence, my judgement, and in some cases, my paternity becuase I
wouldn't change /dev/urandom not to block.

So I did the experiment so I could give them hard data about why we
couldn't go down that path.

> > up since systemd wanted data from /dev/urandom while it processed the 
> > initramfs. As it did not get any, the boot process did not commence that 
> > could 
> > deliver new events to be picked up by the RNG.

And indeed, making this change brick'ed at least one version of Ubuntu
and one version of CeroWRT, as reported by the kernel's 0-day testing
system.  As a result, this patch (which was always a proof of concept,
not anything I thought had any chance of going upstream), was dropped.

Since in the kernel, We Do Not Break Backwards Compatibility, this is
why we have a new interface --- getrandom(2) --- instead of changing
an existing interface.  (Well, there were multiple good reasons for
getrandom, but this was definitely one of them.)

- Ted

Re: [PATCH 0/3] crypto: introduce Microchip / Atmel ECC driver

[Marcel Holtmann]

Hi Tudor,

>>> This patch set introduces Microchip / Atmel ECC driver.
>>> 
>>> The first patch adds some helpers that will be used by fallbacks to
>>> kpp software implementations.
>>> 
>>> The second patch adds ECDH support for the ATECC508A (I2C)
>>> cryptographic engine. The I2C interface is designed to operate
>>> at a maximum clock speed of 1MHz.
>>> 
>>> The device features hardware acceleration for the NIST standard
>>> P256 prime curve and supports the complete key life cycle from
>>> private key generation to ECDH key agreement.
>>> 
>>> Random private key generation is supported internally within
>>> the device to ensure that the private key can never be known
>>> outside of the device. If the user wants to use its own private
>>> keys, the driver will fallback to the ecdh software implementation.
>> can we get this testing with the Bluetooth SMP code? I would really like to 
>> see this being offloaded to hardware. For Bluetooth SMP we never really need 
>> the private key either. The end result is an symmetric 128-bit key for AES. 
>> And we throw the generated key pairs away.
>> With the limitation of private is not available to Linux directly, we should 
>> make sure that KPP users that don’t require the private key are working 
>> properly and can utilize the offload.
> 
> The driver was tested with testmgr, the offload worked.
> 
> I've extended recently the ecdh software implementation with
> ecc privkey generation support. I also added a kpp test in
> testmgr to prove that it works correctly (see [1]).
> 
> I will take a look at Bluetooth SMP code.

you can test this without Bluetooth hardware, just need to make sure you have 
hci_vhci module available. And then from the BlueZ user space source code, you 
run ./tools/smp-tester (no need to install) to exercise the pairing with ECDH 
P-256. If you run ./monitor/btmon in a separate terminal, then it will show you 
the public keys exchanged.

Regards

Marcel

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

[Mimi Zohar]

On Tue, 2017-07-04 at 23:22 -0300, Thiago Jung Bauermann wrote:
> Mimi Zohar  writes:
> 
> > On Wed, 2017-06-21 at 14:45 -0300, Thiago Jung Bauermann wrote:
> >> Mimi Zohar  writes:
> >> > On Wed, 2017-06-07 at 22:49 -0300, Thiago Jung Bauermann wrote:
> >> >> @@ -267,11 +276,18 @@ int ima_appraise_measurement(enum ima_hooks func,
> >> >> status = INTEGRITY_PASS;
> >> >> break;
> >> >> case EVM_IMA_XATTR_DIGSIG:
> >> >> +   case IMA_MODSIG:
> >> >> iint->flags |= IMA_DIGSIG;
> >> >> -   rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
> >> >> -(const char *)xattr_value, 
> >> >> rc,
> >> >> -iint->ima_hash->digest,
> >> >> -iint->ima_hash->length);
> >> >> +
> >> >> +   if (xattr_value->type == EVM_IMA_XATTR_DIGSIG)
> >> >> +   rc = 
> >> >> integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
> >> >> +(const char 
> >> >> *)xattr_value,
> >> >> +rc, 
> >> >> iint->ima_hash->digest,
> >> >> +
> >> >> iint->ima_hash->length);
> >> >> +   else
> >> >> +   rc = ima_modsig_verify(INTEGRITY_KEYRING_IMA,
> >> >> +  xattr_value);
> >> >> +
> >> >
> >> > Perhaps allowing IMA_MODSIG to flow into EVM_IMA_XATTR_DIGSIG on
> >> > failure, would help restore process_measurements() to the way it was.
> >> > Further explanation below.
> >> 
> >> It's not possible to simply flow into EVM_IMA_XATTR_DIGSIG on failure
> >> because after calling ima_read_xattr we need to run again all the logic
> >> before the switch statement. Instead, I'm currently testing the
> >> following change for v3, what do you think?
> >
> > I don't think we can assume that the same algorithm will be used for
> > signing the kernel image. Different entities would be signing the
> > kernel image with different requirements.
> >
> > Suppose for example a stock distro image comes signed using one
> > algorithm (appended signature), but the same kernel image is locally
> > signed using a different algorithm (xattr). Signature verification is
> > dependent on either the distro or local public key being loaded onto
> > the IMA keyring.
> 
> This example is good, but it raises one question: should the xattr
> signature cover the entire contents of the stock distro image (i.e.,
> also cover the appended signature), or should it ignore the appended
> signature and thus cover the same contents that the appended signature
> covers?
> 
> If the former, then we can't reuse the iint->ima_hash that was collected
> when trying to verify the appended signature because it doesn't cover
> the appended signature itself and won't match the hash expected by the
> xattr signature.
> 
> If the latter, then evmctl ima_sign needs to be modified to check
> whether there's an appended signature in the given file and ignore it
> when calculating the xattr signature.
> 
> Which is better?

I realize that having the same file hash for both the appended
signature and extended attribute would make things a lot easier, but
security.ima is a signature of the file as written to disk, meaning it
would include any appended signature

> 
> >> >> @@ -226,30 +282,23 @@ static int process_measurement(struct file *file, 
> >> >> char *buf, loff_t size,
> >> >> goto out_digsig;
> >> >> }
> >> >> 
> >> >> -   template_desc = ima_template_desc_current();
> >> >> -   if ((action & IMA_APPRAISE_SUBMASK) ||
> >> >> -   strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) 
> >> >> != 0)
> >> >> -   /* read 'security.ima' */
> >> >> -   xattr_len = ima_read_xattr(file_dentry(file), 
> >> >> _value);
> >> >> -
> >> >> -   hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
> >> >> -
> >> >> -   rc = ima_collect_measurement(iint, file, buf, size, hash_algo);
> >> >> -   if (rc != 0) {
> >> >> -   if (file->f_flags & O_DIRECT)
> >> >> -   rc = (iint->flags & IMA_PERMIT_DIRECTIO) ? 0 : 
> >> >> -EACCES;
> >> >> -   goto out_digsig;
> >> >> -   }
> >> >> -
> >> >
> >> > There are four stages: collect measurement, store measurement,
> >> > appraise measurement and audit measurement. "Collect" needs to be
> >> > done if any one of the other stages is needed.
> >> >
> >> >> if (!pathbuf)   /* ima_rdwr_violation possibly pre-fetched */
> >> >> pathname = ima_d_path(>f_path, , 
> >> >> filename);
> >> >> 
> >> >> +   if (iint->flags & IMA_MODSIG_ALLOWED)
> >> >> +   rc = measure_and_appraise(file, buf, size, func, 
> >> >> opened, 

Re: [PATCH 3/3] crypto: hwrng add sysfs attribute to show user selected rng

[Harald Freudenberger]

On 07/04/2017 03:15 PM, PrasannaKumar Muralidharan wrote:
> On 3 July 2017 at 15:33, Harald Freudenberger  
> wrote:
>> This patch introduces a new sysfs attribute file 'rng_selected'
>> which shows the the rng chosen by userspace.
>>
>> If a rng source is chosen by user via echo some valid string
>> to rng_current there should be a way to signal this choice to
>> userspace. The new attribute file 'rng_selected' shows either
>> the name of the rng chosen or 'none'.
>>
>> Signed-off-by: Harald Freudenberger 
>> ---
>>  drivers/char/hw_random/core.c | 22 +-
>>  1 file changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
>> index ffd4e36..6a6276a 100644
>> --- a/drivers/char/hw_random/core.c
>> +++ b/drivers/char/hw_random/core.c
>> @@ -28,8 +28,8 @@
>>  #define RNG_MODULE_NAME"hw_random"
>>
>>  static struct hwrng *current_rng;
>> -/* the current rng has been explicitly chosen by user via sysfs */
>> -static int cur_rng_set_by_user;
>> +/* the rng explicitly selected by user via sysfs */
>> +static struct hwrng *selected_rng;
>>  static struct task_struct *hwrng_fill;
>>  /* list of registered rngs, sorted decending by quality */
>>  static LIST_HEAD(rng_list);
>> @@ -306,7 +306,7 @@ static ssize_t hwrng_attr_current_store(struct device 
>> *dev,
>> list_for_each_entry(rng, _list, list) {
>> if (sysfs_streq(rng->name, buf)) {
>> err = 0;
>> -   cur_rng_set_by_user = 1;
>> +   selected_rng = rng;
>> if (rng != current_rng)
>> err = set_current_rng(rng);
>> break;
>> @@ -355,16 +355,28 @@ static ssize_t hwrng_attr_available_show(struct device 
>> *dev,
>> return strlen(buf);
>>  }
>>
>> +static ssize_t hwrng_attr_selected_show(struct device *dev,
>> +   struct device_attribute *attr,
>> +   char *buf)
>> +{
>> +   return snprintf(buf, PAGE_SIZE, "%s\n",
>> +   selected_rng ? selected_rng->name : "none");
>> +}
>> +
>>  static DEVICE_ATTR(rng_current, S_IRUGO | S_IWUSR,
>>hwrng_attr_current_show,
>>hwrng_attr_current_store);
>>  static DEVICE_ATTR(rng_available, S_IRUGO,
>>hwrng_attr_available_show,
>>NULL);
>> +static DEVICE_ATTR(rng_selected, S_IRUGO,
>> +  hwrng_attr_selected_show,
>> +  NULL);
>>
>>  static struct attribute *rng_dev_attrs[] = {
>> _attr_rng_current.attr,
>> _attr_rng_available.attr,
>> +   _attr_rng_selected.attr,
>> NULL
>>  };
>>
>> @@ -448,7 +460,7 @@ int hwrng_register(struct hwrng *rng)
>> old_rng = current_rng;
>> err = 0;
>> if (!old_rng ||
>> -   (!cur_rng_set_by_user && rng->quality > old_rng->quality)) {
>> +   (!selected_rng && rng->quality > old_rng->quality)) {
>> /*
>>  * Set new rng as current as the new rng source
>>  * provides better entropy quality and was not
>> @@ -484,7 +496,7 @@ void hwrng_unregister(struct hwrng *rng)
>> list_del(>list);
>> if (current_rng == rng) {
>> drop_current_rng();
>> -   cur_rng_set_by_user = 0;
>> +   selected_rng = NULL;
>> /* rng_list is sorted by quality, use the best (=first) one 
>> */
>> if (!list_empty(_list)) {
>> struct hwrng *new_rng;
>> --
>> 2.7.4
>>
> The current_rng sysfs attribute shows currently selected rng. So this
> new attribute can contain 1 to indicate user's choice, 0 otherwise.
>
> Regards,
> PrasannaKumar
>
Here is an updated version with just showing 0 or 1 in the new sysfs
attribute file:
== cut ==
From: Harald Freudenberger 
Date: Mon, 3 Jul 2017 10:19:22 +0200
Subject: [PATCH 3/3] crypto: hwrng add sysfs attribute to show user selected
 rng

This patch introduces a new sysfs attribute file 'rng_selected'
which shows if the current rng has been chosen by userspace.

If a rng source is chosen by user via echo some valid string
to rng_current there should be a way to signal this choice to
userspace. The new attribute file 'rng_selected' shows '1' for
user selecte rng and '0' otherwise.

Signed-off-by: Harald Freudenberger 
---
 drivers/char/hw_random/core.c | 21 -
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
index ffd4e36..2b4c7f6 100644
--- a/drivers/char/hw_random/core.c
+++ b/drivers/char/hw_random/core.c
@@ -28,8 +28,8 @@
 #define RNG_MODULE_NAME"hw_random"
 
 static struct 

Re: [PATCH 0/3] crypto: introduce Microchip / Atmel ECC driver

[Tudor Ambarus]

Hi, Marcel,

On 05.07.2017 13:54, Marcel Holtmann wrote:

Hi Tudor,


This patch set introduces Microchip / Atmel ECC driver.

The first patch adds some helpers that will be used by fallbacks to
kpp software implementations.

The second patch adds ECDH support for the ATECC508A (I2C)
cryptographic engine. The I2C interface is designed to operate
at a maximum clock speed of 1MHz.

The device features hardware acceleration for the NIST standard
P256 prime curve and supports the complete key life cycle from
private key generation to ECDH key agreement.

Random private key generation is supported internally within
the device to ensure that the private key can never be known
outside of the device. If the user wants to use its own private
keys, the driver will fallback to the ecdh software implementation.


can we get this testing with the Bluetooth SMP code? I would really like to see 
this being offloaded to hardware. For Bluetooth SMP we never really need the 
private key either. The end result is an symmetric 128-bit key for AES. And we 
throw the generated key pairs away.

With the limitation of private is not available to Linux directly, we should 
make sure that KPP users that don’t require the private key are working 
properly and can utilize the offload.


The driver was tested with testmgr, the offload worked.

I've extended recently the ecdh software implementation with
ecc privkey generation support. I also added a kpp test in
testmgr to prove that it works correctly (see [1]).

I will take a look at Bluetooth SMP code.

Thanks,
ta

[1] http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg25835.html

Re: [PATCH 0/3] crypto: introduce Microchip / Atmel ECC driver

[Marcel Holtmann]

Hi Tudor,

> This patch set introduces Microchip / Atmel ECC driver.
> 
> The first patch adds some helpers that will be used by fallbacks to
> kpp software implementations.
> 
> The second patch adds ECDH support for the ATECC508A (I2C)
> cryptographic engine. The I2C interface is designed to operate
> at a maximum clock speed of 1MHz.
> 
> The device features hardware acceleration for the NIST standard
> P256 prime curve and supports the complete key life cycle from
> private key generation to ECDH key agreement.
> 
> Random private key generation is supported internally within
> the device to ensure that the private key can never be known
> outside of the device. If the user wants to use its own private
> keys, the driver will fallback to the ecdh software implementation.

can we get this testing with the Bluetooth SMP code? I would really like to see 
this being offloaded to hardware. For Bluetooth SMP we never really need the 
private key either. The end result is an symmetric 128-bit key for AES. And we 
throw the generated key pairs away.

With the limitation of private is not available to Linux directly, we should 
make sure that KPP users that don’t require the private key are working 
properly and can utilize the offload.

Regards

Marcel

[PATCH 0/3] crypto: introduce Microchip / Atmel ECC driver

[Tudor Ambarus]

Hi,

This patch set introduces Microchip / Atmel ECC driver.

The first patch adds some helpers that will be used by fallbacks to
kpp software implementations.

The second patch adds ECDH support for the ATECC508A (I2C)
cryptographic engine. The I2C interface is designed to operate
at a maximum clock speed of 1MHz.

The device features hardware acceleration for the NIST standard
P256 prime curve and supports the complete key life cycle from
private key generation to ECDH key agreement.

Random private key generation is supported internally within
the device to ensure that the private key can never be known
outside of the device. If the user wants to use its own private
keys, the driver will fallback to the ecdh software implementation.

Tudor Ambarus (3):
  crypto: kpp: add get/set_flags helpers
  crypto: introduce Microchip / Atmel ECC driver
  MAINTAINERS: add a maintainer for Microchip / Atmel ECC driver

 .../devicetree/bindings/crypto/atmel-crypto.txt|  13 +
 MAINTAINERS|   6 +
 drivers/crypto/Kconfig |  14 +
 drivers/crypto/Makefile|   1 +
 drivers/crypto/atmel-ecc.c | 781 +
 drivers/crypto/atmel-ecc.h | 128 
 include/crypto/kpp.h   |  10 +
 7 files changed, 953 insertions(+)
 create mode 100644 drivers/crypto/atmel-ecc.c
 create mode 100644 drivers/crypto/atmel-ecc.h

-- 
2.7.4

[PATCH 3/3] MAINTAINERS: add a maintainer for Microchip / Atmel ECC driver

[Tudor Ambarus]

A new cryptographic engine driver was added in
drivers/crypto/atmel-ecc.*.
Add myself as a maintainer for this driver.

Signed-off-by: Tudor Ambarus 
---
 MAINTAINERS | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a24eb8c..d55b983 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8464,6 +8464,12 @@ F:   drivers/dma/at_hdmac.c
 F: drivers/dma/at_hdmac_regs.h
 F: include/linux/platform_data/dma-atmel.h
 
+MICROCHIP / ATMEL ECC DRIVER
+M: Tudor Ambarus 
+L: linux-crypto@vger.kernel.org
+S: Maintained
+F: drivers/crypto/atmel-ecc.*
+
 MICROCHIP / ATMEL ISC DRIVER
 M: Songjun Wu 
 L: linux-me...@vger.kernel.org
-- 
2.7.4

[PATCH 1/3] crypto: kpp: add get/set_flags helpers

[Tudor Ambarus]

These helpers will be used for fallbacks to kpp software
implementations.

Signed-off-by: Tudor Ambarus 
---
 include/crypto/kpp.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/include/crypto/kpp.h b/include/crypto/kpp.h
index 2133d17..1bde0a6 100644
--- a/include/crypto/kpp.h
+++ b/include/crypto/kpp.h
@@ -145,6 +145,16 @@ static inline struct crypto_kpp *crypto_kpp_reqtfm(struct 
kpp_request *req)
return __crypto_kpp_tfm(req->base.tfm);
 }
 
+static inline u32 crypto_kpp_get_flags(struct crypto_kpp *tfm)
+{
+   return crypto_tfm_get_flags(crypto_kpp_tfm(tfm));
+}
+
+static inline void crypto_kpp_set_flags(struct crypto_kpp *tfm, u32 flags)
+{
+   crypto_tfm_set_flags(crypto_kpp_tfm(tfm), flags);
+}
+
 /**
  * crypto_free_kpp() - free KPP tfm handle
  *
-- 
2.7.4

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

[Ulrich Windl]

>>> Jeffrey Walton  schrieb am 17.06.2017 um 16:23 in 
>>> Nachricht
:

[...]
> But its not clear to me how to ensure uniqueness when its based on
> randomness from the generators.

Even with a perfect random generator non-unique values are possible (that's why 
it's random). It's unlikely, but it can happen. The question is whether the 
probability of non-unique values from /dev/urandom is any higher than that for 
values read from /dev/random. One _might_ be able to predict the values from 
/dev/urandom.

Regards,
Ulrich

> 
> Jeff
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "open-iscsi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to open-iscsi+unsubscr...@googlegroups.com.
> To post to this group, send email to open-is...@googlegroups.com.
> Visit this group at https://groups.google.com/group/open-iscsi.
> For more options, visit https://groups.google.com/d/optout.

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

[Ulrich Windl]

>>> Stephan Müller  schrieb am 26.06.2017 um 19:38 in
Nachricht <1678474.gnybdsl...@tauon.chronox.de>:
> Am Montag, 26. Juni 2017, 03:23:09 CEST schrieb Nicholas A. Bellinger:
> 
> Hi Nicholas,
> 
>> Hi Stephan, Lee & Jason,
>> 
>> (Adding target-devel CC')
>> 
>> Apologies for coming late to the discussion.  Comments below.
>> 
>> On Sun, 2017-06-18 at 10:04 +0200, Stephan Müller wrote:
>> > Am Samstag, 17. Juni 2017, 05:45:57 CEST schrieb Lee Duncan:
>> > 
>> > Hi Lee,
>> > 
>> > > In your testing, how long might a process have to wait? Are we talking
>> > > seconds? Longer? What about timeouts?
>> > 
>> > In current kernels (starting with 4.8) this timeout should clear within
a
>> > few seconds after boot.
>> > 
>> > In older kernels (pre 4.8), my KVM takes up to 90 seconds to reach that
>> > seeding point. I have heard that on IBM System Z this trigger point
>> > requires minutes to be reached.
>> 
>> I share the same concern as Lee wrt to introducing latency into the
>> existing iscsi-target login sequence.
>> 
>> Namely in the use-cases where a single node is supporting ~1K unique
>> iscsi-target IQNs, and fail-over + re-balancing of IQNs where 100s of
>> login attempts are expected to occur in parallel.
>> 
>> If environments exist that require non trivial amounts of time for RNG
>> seeding to be ready for iscsi-target usage, then enforcing this
>> requirement at iscsi login time can open up problems, especially when
>> iscsi host environments may be sensitive to login timeouts, I/O
>> timeouts, et al.
>> 
>> That said, I'd prefer to simply wait for RNG to be seeded at modprobe
>> iscsi_target_mod time, instead of trying to enforce randomness during
>> login.
>> 
>> This way, those of use who have distributed storage platforms can know
>> to avoid putting a node into a state to accept iscsi-target IQN export
>> migration, before modprobe iscsi_target_mod has successfully loaded and
>> RNG seeding has been confirmed.
>> 
>> WDYT..?
> 
> We may have a chicken and egg problem when the wait is at the modprobe time.

> 
> Assume the modprobe happens during initramfs time to get access to the root

> file system. In this case, you entire boot process will lock up for an 
> indefinite amount of time. The reason is that in order to obtain events 
> detected by the RNG, devices need to be initialized and working. Such 
> devices 
> commonly start working after the the root partition is mounted as it 
> contains 
> all drivers, all configuration information etc.
> 
> Note, during the development of my /dev/random implementation, I added the 
> getrandom-like blocking behavior to /dev/urandom (which is the equivalent to

> 
> Jason's patch except that it applies to user space). The boot process locked


I thought reads from urandom never block by definition. An older manual page
(man urandom) also says: "A  read  from  the  /dev/urandom device will not
block waiting for more entropy."

Regards,
Ulrich

> 
> up since systemd wanted data from /dev/urandom while it processed the 
> initramfs. As it did not get any, the boot process did not commence that 
> could 
> deliver new events to be picked up by the RNG.
> 
> As I do not have such an iscsi system, I cannot test Jason's patch. But 
> maybe 
> the mentioned chicken-and-egg problem I mentioned above is already visible 
> with the current patch as it will lead to a blocking of the mounting of the

> root partition in case the root partition is on an iscsi target.
> 
> Ciao
> Stephan
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "open-iscsi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to open-iscsi+unsubscr...@googlegroups.com.
> To post to this group, send email to open-is...@googlegroups.com.
> Visit this group at https://groups.google.com/group/open-iscsi.
> For more options, visit https://groups.google.com/d/optout.