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

[Nicholas A. Bellinger]

On Mon, 2017-06-26 at 19:38 +0200, Stephan Müller wrote:
> 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 
> 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.

AFAIK, there are no distro initramfs dependencies for iscsi_target_mod,
and every environment I've ever seen loads iscsi_target_mod after
switching to the real rootfs.

For an iscsi initiator that might not been the case, especially if the
rootfs is running atop a iscsi LUN.

But at least for iscsi-target mode, any blocking during modprobe waiting
for RNG seeding would happen outside of initramfs.

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

[Nicholas A. Bellinger]

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

[PATCH 1/2] crypto: Add struct crypto_alg-cra_check_optimized

[Nicholas A. Bellinger]

From: Nicholas Bellinger n...@linux-iscsi.org

This patch adds an optional struct crypto_alg-cra_check_optimized() caller
that can be used by libcrypto algorithms in order to load an architecture
dependent optimized / HW offload module.

This includes adding the call to alg-cra_check_optimized() inside of
crypto_larval_lookup() once the initial generic algorithm has been
loaded via request_module().

Signed-off-by: Nicholas A. Bellinger n...@linux-iscsi.org
Cc: Herbert Xu herb...@gondor.apana.org.au
---
 crypto/api.c   |6 +-
 include/linux/crypto.h |1 +
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/crypto/api.c b/crypto/api.c
index 033a714..4dcbef6 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -226,8 +226,12 @@ struct crypto_alg *crypto_larval_lookup(const char *name, 
u32 type, u32 mask)
alg = crypto_alg_lookup(name, type, mask);
}
 
-   if (alg)
+   if (alg) {
+   if (alg-cra_check_optimized)
+   alg-cra_check_optimized(alg);
+
return crypto_is_larval(alg) ? crypto_larval_wait(alg) : alg;
+   }
 
return crypto_larval_add(name, type, mask);
 }
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index a6a7a1c..ea7c426 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -295,6 +295,7 @@ struct crypto_alg {
 
int (*cra_init)(struct crypto_tfm *tfm);
void (*cra_exit)(struct crypto_tfm *tfm);
+   void (*cra_check_optimized)(struct crypto_alg *alg);
void (*cra_destroy)(struct crypto_alg *alg);

struct module *cra_module;
-- 
1.5.6.5

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

Re: [PATCH 0/2] Add struct crypto_alg-cra_check_optimized for crc32c_intel

[Nicholas A. Bellinger]

On Thu, 2011-03-10 at 16:43 +0800, Herbert Xu wrote:
 On Thu, Mar 10, 2011 at 12:21:10AM -0800, Nicholas A. Bellinger wrote:
  
  Nicholas Bellinger (2):
crypto: Add struct crypto_alg-cra_check_optimized
crypto/crc32c: Add crc32c_cra_check_optimized for crc32c_intel
 
 I think you misunderstood my suggestion.  The reason I asked
 for this to be done in the crypto API is because it is not specific
 to crc32c_intel.
 
 So sending me a patch that modifies crc32c clearly misses the
 point :)
 
 This should be done in the core API, i.e., api.c/algapi.c.
 

OK, so you mean each struct crypto_alg should define something like a
'cra_optimized_name' for which request_module(alg-cra_optimized_name)
is called somewhere in libcrypto code..?

Would you mind being a bit more specific where this should be happening
in api.c/algapi.c..?

Thanks,

--nab

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

Re: [PATCH 0/2] Add struct crypto_alg-cra_check_optimized for crc32c_intel

[Nicholas A. Bellinger]

On Thu, 2011-03-10 at 17:09 +0800, Herbert Xu wrote:
 On Thu, Mar 10, 2011 at 12:54:24AM -0800, Nicholas A. Bellinger wrote:
 
  OK, so you mean each struct crypto_alg should define something like a
  'cra_optimized_name' for which request_module(alg-cra_optimized_name)
  is called somewhere in libcrypto code..?
 
 No, what I mean is that whenever we look up an algorithm through
 crypto_alg_mod_lookup, we should conditionally call modprobe if
 we havn't done so already.
 
 So you just need to record one bit of info in each crypto_alg
 object to indicate whether we have invoked modprobe.  I suggest
 adding a CRYPTO_ALG_* bit.
 

Mmmm, now I am really confused, and please let me apologize in advance
for my lack of experience with libcrypto internals..  ;)

I thought the problem was that CONFIG_CRYPTO_ALGFOO=y and
CONFIG_CRYPTO_ALGFOO_ARCH_HW_OFFLOAD=m would cause the latter to not
explictly call request_module() for this HW offload case..

So what I don't understand how adding a request_module() call to a list
of known modules works when crc32c_intel.ko has not been loaded yet..?

Am I missing something obvious wrt to how crc32c.ko can tell libcrypto
about which architecture dependent optimized modules it should load..?

Best Regards,

--nab

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

Re: [PATCH 0/2] Add struct crypto_alg-cra_check_optimized for crc32c_intel

[Nicholas A. Bellinger]

On Thu, 2011-03-10 at 01:13 -0800, Nicholas A. Bellinger wrote:
 On Thu, 2011-03-10 at 17:09 +0800, Herbert Xu wrote:
  On Thu, Mar 10, 2011 at 12:54:24AM -0800, Nicholas A. Bellinger wrote:
  
   OK, so you mean each struct crypto_alg should define something like a
   'cra_optimized_name' for which request_module(alg-cra_optimized_name)
   is called somewhere in libcrypto code..?
  
  No, what I mean is that whenever we look up an algorithm through
  crypto_alg_mod_lookup, we should conditionally call modprobe if
  we havn't done so already.
  
  So you just need to record one bit of info in each crypto_alg
  object to indicate whether we have invoked modprobe.  I suggest
  adding a CRYPTO_ALG_* bit.
  
 
 Mmmm, now I am really confused, and please let me apologize in advance
 for my lack of experience with libcrypto internals..  ;)
 
 I thought the problem was that CONFIG_CRYPTO_ALGFOO=y and
 CONFIG_CRYPTO_ALGFOO_ARCH_HW_OFFLOAD=m would cause the latter to not
 explictly call request_module() for this HW offload case..
 
 So what I don't understand how adding a request_module() call to a list
 of known modules works when crc32c_intel.ko has not been loaded yet..?
 
 Am I missing something obvious wrt to how crc32c.ko can tell libcrypto
 about which architecture dependent optimized modules it should load..?
 

Just to clarify a bit on my previous comments..

We are still expecting the libcrypto consumer (iscsi_target_mod.ko) to
call the arch independent crypto_alloc_hash(crc32c, ...) in order to
have libcrypto backend logic perform a request_module() upon
architecture dependent offload modules (like crc32c_intel.ko) that
libcrypto consumers are not (and should not) be calling directly via
crypto_alloc_host(crc32c_intel, ...), correct..?

Where I am getting confused is wrt to a new crypto_alg_mod_lookup() -
request_module() call for a struct shash_alg that has not yet be loaded
via arch/x86/crypto/crc32c-intel.c:crc32c_intel_mod_init() -
crypto_register_shash().

Thanks,

--nab


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

Re: [RFC 12/12] iscsi-target: Add Makefile/Kconfig and update TCM top level

[Nicholas A. Bellinger]

On Fri, 2011-03-04 at 11:00 -0600, James Bottomley wrote:
 On Thu, 2011-03-03 at 12:58 -0800, Nicholas A. Bellinger wrote:
  On Thu, 2011-03-03 at 09:19 -0500, Christoph Hellwig wrote:
   On Wed, Mar 02, 2011 at 01:32:11PM -0800, Nicholas A. Bellinger wrote:
The kernel code itself that is specific to using the SSE v4.2
instruction for CRC32C offload are using #ifdef CONFIG_X86 stubs in
iscsi_target_login.c:iscsi_login_setup_crypto(), and !CONFIG_X86 will
default to using the unoptimized 1x8 slicing soft CRC32C code.  This
particular piece of logic has been tested on powerpc and arm and is
funcitoning as expected from the kernel level using the arch independent
soft code.
   
   I don't think you need that code at all.  The crypto code is structured
   to prefer the optimized implementation if it is present.  Just stripping
   the x86-specific code out and always requesting the plain crc32c
   algorithm should give you the optimized one if it is present on your
   system.
   
   Please give it a try.
   
  
  This is what I originally thought as well, but this ended up not being
  the case when the logic was originally coded up.   I just tried again
  with .38-rc7 on a 5500 series machine and simply stubbing out the
  CONFIG_X86 part from iscsi_login_setup_crypto() and calling:
  
 crypto_alloc_hash(crc32c, 0, CRYPTO_ALG_ASYNC)
  
  does not automatically load and use crc32c_intel.ko when only requesting
  plain crc32c.
 
 It sounds like there might be a bug in the crypto layer, so the Linux
 way is to make it work as intended.
 
 It's absolutely not acceptable just to pull other layer workarounds into
 drivers.
 
  The reason for the extra crypto_alloc_hash(crc32c-intel, ...) call in
  iscsi_login_setup_crypto() is to load crc32c_intel.ko on-demand for
  cpu_has_xmm4_2 capable machines.
  
  I should mention this is with the following .config:
  
  CONFIG_CRYPTO_CRC32C=y
  CONFIG_CRYPTO_CRC32C_INTEL=m
  
  This would seem to indicate that CRC32C_INTEL needs to be compiled in
  (or at least manually loaded) for libcypto to use the optimized
  instructions when the plain crc32c is called, correct..?
 
 That sounds right.  There's probably not an autoload for this on
 recognising sse instructions.
 

I have been thinking about this some more, and modifying libcrypto to be
aware of optimized offload methods for hardware specific modules that it
should load does sound useful, but it seem like overkill to me for only
this particular case.

What about the following to simply call request_module(crc32c_intel)
at module_init() time and top the extra iscsi_login_setup_crypto()
code..?

Thanks,

--nab

[PATCH] iscsi-target: Call request_module(crc32c_intel) during module_init

This patch adds a call during module_init() - iscsi_target_register_configfs()
to request the loading of crc32c_intel.ko to allow libcrypto to properly use
the optimized offload where available.

It also removes the extra crypto_alloc_hash(crc32c-intel, ...) calls
from iscsi_login_setup_crypto() and removes the unnecessary TPG attribute
crc32c_x86_offload for control this offload from configfs.

Signed-off-by: Nicholas A. Bellinger n...@linux-iscsi.org
---
 drivers/target/lio-target/iscsi_target_configfs.c |   18 +++
 drivers/target/lio-target/iscsi_target_core.h |4 --
 drivers/target/lio-target/iscsi_target_login.c|   34 ++---
 drivers/target/lio-target/iscsi_target_tpg.c  |   19 ---
 drivers/target/lio-target/iscsi_target_tpg.h  |1 -
 5 files changed, 15 insertions(+), 61 deletions(-)

diff --git a/drivers/target/lio-target/iscsi_target_configfs.c 
b/drivers/target/lio-target/iscsi_target_configfs.c
index 76ee4fc..7ba169a 100644
--- a/drivers/target/lio-target/iscsi_target_configfs.c
+++ b/drivers/target/lio-target/iscsi_target_configfs.c
@@ -927,11 +927,6 @@ TPG_ATTR(demo_mode_write_protect, S_IRUGO | S_IWUSR);
  */
 DEF_TPG_ATTRIB(prod_mode_write_protect);
 TPG_ATTR(prod_mode_write_protect, S_IRUGO | S_IWUSR);
-/*
- * Define iscsi_tpg_attrib_s_crc32c_x86_offload
- */
-DEF_TPG_ATTRIB(crc32c_x86_offload);
-TPG_ATTR(crc32c_x86_offload, S_IRUGO | S_IWUSR);

 static struct configfs_attribute *lio_target_tpg_attrib_attrs[] = {
iscsi_tpg_attrib_authentication.attr,
@@ -942,7 +937,6 @@ static struct configfs_attribute 
*lio_target_tpg_attrib_attrs[] = {
iscsi_tpg_attrib_cache_dynamic_acls.attr,
iscsi_tpg_attrib_demo_mode_write_protect.attr,
iscsi_tpg_attrib_prod_mode_write_protect.attr,
-   iscsi_tpg_attrib_crc32c_x86_offload.attr,
NULL,
 };

@@ -1525,6 +1519,18 @@ int iscsi_target_register_configfs(void)
lio_target_fabric_configfs = fabric;
printk(KERN_INFO LIO_TARGET[0] - Set fabric -
 lio_target_fabric_configfs\n);
+#ifdef CONFIG_X86
+   /*
+* For cpu_has_xmm4_2 go ahead and load crc32c_intel.ko in order for
+* iscsi_login_setup_crypto

Re: [RFC 12/12] iscsi-target: Add Makefile/Kconfig and update TCM top level

[Nicholas A. Bellinger]

On Thu, 2011-03-03 at 09:19 -0500, Christoph Hellwig wrote:
 On Wed, Mar 02, 2011 at 01:32:11PM -0800, Nicholas A. Bellinger wrote:
  The kernel code itself that is specific to using the SSE v4.2
  instruction for CRC32C offload are using #ifdef CONFIG_X86 stubs in
  iscsi_target_login.c:iscsi_login_setup_crypto(), and !CONFIG_X86 will
  default to using the unoptimized 1x8 slicing soft CRC32C code.  This
  particular piece of logic has been tested on powerpc and arm and is
  funcitoning as expected from the kernel level using the arch independent
  soft code.
 
 I don't think you need that code at all.  The crypto code is structured
 to prefer the optimized implementation if it is present.  Just stripping
 the x86-specific code out and always requesting the plain crc32c
 algorithm should give you the optimized one if it is present on your
 system.
 
 Please give it a try.
 

This is what I originally thought as well, but this ended up not being
the case when the logic was originally coded up.   I just tried again
with .38-rc7 on a 5500 series machine and simply stubbing out the
CONFIG_X86 part from iscsi_login_setup_crypto() and calling:

   crypto_alloc_hash(crc32c, 0, CRYPTO_ALG_ASYNC)

does not automatically load and use crc32c_intel.ko when only requesting
plain crc32c.

The reason for the extra crypto_alloc_hash(crc32c-intel, ...) call in
iscsi_login_setup_crypto() is to load crc32c_intel.ko on-demand for
cpu_has_xmm4_2 capable machines.

I should mention this is with the following .config:

CONFIG_CRYPTO_CRC32C=y
CONFIG_CRYPTO_CRC32C_INTEL=m

This would seem to indicate that CRC32C_INTEL needs to be compiled in
(or at least manually loaded) for libcypto to use the optimized
instructions when the plain crc32c is called, correct..?

--nab

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