| Message ID | 22067042.Wh8NKMhEMa@positron.chronox.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
On Tue, Jul 18, 2017 at 09:57:55AM +0200, Stephan Müller wrote: > --- /dev/null > +++ b/include/crypto/jitterentropy.h > @@ -0,0 +1,80 @@ > +/* > + * Copyright (C) 2017, Stephan Mueller <smueller@chronox.de> > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + * notice, and the entire permission notice in its entirety, > + * including the disclaimer of warranties. > + * 2. Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in the > + * documentation and/or other materials provided with the distribution. > + * 3. The name of the author may not be used to endorse or promote > + * products derived from this software without specific prior > + * written permission. > + * > + * ALTERNATIVELY, this product may be distributed under the terms of > + * the GNU General Public License, in which case the provisions of the GPL2 are > + * required INSTEAD OF the above restrictions. (This clause is > + * necessary due to a potential bad interaction between the GPL and > + * the restrictions contained in a BSD-style copyright.) > + * > + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED > + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES > + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF > + * WHICH ARE HEREBY DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT > + * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR > + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF > + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE > + * USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH > + * DAMAGE. > + */ > + > +#ifndef _JITTERENTROPY_H > +#define _JITTERENTROPY_H > + > +typedef unsigned long long __u64; > +typedef long long __s64; types.h already has these defines, don't re-typedef them again...
Am Dienstag, 18. Juli 2017, 10:30:14 CEST schrieb Greg Kroah-Hartman: Hi Greg, > > +typedef unsigned long long __u64; > > +typedef long long __s64; > > types.h already has these defines, don't re-typedef them again... The issue is that the C code is compiled without optimizations. Thus, the C code shall not depend on any other header file. This issue was discussed during the inclusion of the Jitter RNG C code into the kernel. Ciao Stephan
On Tue, Jul 18, 2017 at 10:40:07AM +0200, Stephan Müller wrote: > Am Dienstag, 18. Juli 2017, 10:30:14 CEST schrieb Greg Kroah-Hartman: > > Hi Greg, > > > > +typedef unsigned long long __u64; > > > +typedef long long __s64; > > > > types.h already has these defines, don't re-typedef them again... > > The issue is that the C code is compiled without optimizations. Thus, the C > code shall not depend on any other header file. That is very strange for a kernel file, I don't know what to say... > This issue was discussed during the inclusion of the Jitter RNG C code into > the kernel. Ok, that was then, this is now, why not change it now? How does including types.h change anything? thanks, greg k-h
Am Dienstag, 18. Juli 2017, 10:49:59 CEST schrieb Greg Kroah-Hartman: Hi Greg, > > > This issue was discussed during the inclusion of the Jitter RNG C code > > into > > the kernel. > > Ok, that was then, this is now, why not change it now? How does > including types.h change anything? At the time of discussion, I had no issue compiling it with types.h, but on other architectures, there were some issues. Allow me to check back with the developer who notified me about this issue to see whether types.h can be included. Ciao Stephan
On Tue, Jul 18, 2017 at 10:49 AM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Tue, Jul 18, 2017 at 10:40:07AM +0200, Stephan Müller wrote: >> Am Dienstag, 18. Juli 2017, 10:30:14 CEST schrieb Greg Kroah-Hartman: >> >> Hi Greg, >> >> > > +typedef unsigned long long __u64; >> > > +typedef long long __s64; >> > >> > types.h already has these defines, don't re-typedef them again... >> >> The issue is that the C code is compiled without optimizations. Thus, the C >> code shall not depend on any other header file. > > That is very strange for a kernel file, I don't know what to say... > >> This issue was discussed during the inclusion of the Jitter RNG C code into >> the kernel. > > Ok, that was then, this is now, why not change it now? How does > including types.h change anything? I can see why the jitterentropy implementation avoids using kernel headers, the problem now is that part of it gets moved into a new header, and that already violates the original principle. From my reading of the code, we could probably leave the structure definition in the crypto/jitterentropy.c, and have the statically allocated instance in the same file when CONFIG_LRNG is set, or provide a way to allocate an instance early (I assume you can't call jent_entropy_collector_alloc() here since you need the RNG long before kzalloc() works). Arnd
Am Dienstag, 18. Juli 2017, 11:02:02 CEST schrieb Arnd Bergmann: Hi Arnd, > > I can see why the jitterentropy implementation avoids using kernel headers, > the problem now is that part of it gets moved into a new header, and that > already violates the original principle. > > From my reading of the code, we could probably leave the structure > definition in the crypto/jitterentropy.c, and have the statically > allocated instance in the same file when CONFIG_LRNG is > set, That is a very good idea -- I will implement this approach. > or provide a way to allocate an instance early (I assume you > can't call jent_entropy_collector_alloc() here since you need > the RNG long before kzalloc() works). Correct. I cannot assume that any of the memory allocation routines are available. Thank you. Ciao Stephan
On Tue, Jul 18, 2017 at 11:10 AM, Stephan Müller <smueller@chronox.de> wrote: > Am Dienstag, 18. Juli 2017, 11:02:02 CEST schrieb Arnd Bergmann: > > Hi Arnd, >> >> I can see why the jitterentropy implementation avoids using kernel headers, >> the problem now is that part of it gets moved into a new header, and that >> already violates the original principle. >> >> From my reading of the code, we could probably leave the structure >> definition in the crypto/jitterentropy.c, and have the statically >> allocated instance in the same file when CONFIG_LRNG is >> set, > > That is a very good idea -- I will implement this approach. I guess ideally you just move the inner half of lrng_get_jent(), i.e. everything inside of the spinlock, plus the buffer, into that file. That should keep the low-level side separate from the caller. Arnd
Am Dienstag, 18. Juli 2017, 11:16:10 CEST schrieb Arnd Bergmann: Hi Arnd, > I guess ideally you just move the inner half of lrng_get_jent(), > i.e. everything inside of the spinlock, plus the buffer, into that file. > That should keep the low-level side separate from the caller. Yes, I concur. Thanks. Ciao Stephan
diff --git a/crypto/jitterentropy.c b/crypto/jitterentropy.c index acf44b2..90184a1 100644 --- a/crypto/jitterentropy.c +++ b/crypto/jitterentropy.c @@ -54,40 +54,11 @@ #error "The CPU Jitter random number generator must not be compiled with optimizations. See documentation. Use the compiler switch -O0 for compiling jitterentropy.c." #endif -typedef unsigned long long __u64; -typedef long long __s64; +#include <crypto/jitterentropy.h> + typedef unsigned int __u32; #define NULL ((void *) 0) -/* The entropy pool */ -struct rand_data { - /* all data values that are vital to maintain the security - * of the RNG are marked as SENSITIVE. A user must not - * access that information while the RNG executes its loops to - * calculate the next random value. */ - __u64 data; /* SENSITIVE Actual random number */ - __u64 old_data; /* SENSITIVE Previous random number */ - __u64 prev_time; /* SENSITIVE Previous time stamp */ -#define DATA_SIZE_BITS ((sizeof(__u64)) * 8) - __u64 last_delta; /* SENSITIVE stuck test */ - __s64 last_delta2; /* SENSITIVE stuck test */ - unsigned int stuck:1; /* Time measurement stuck */ - unsigned int osr; /* Oversample rate */ - unsigned int stir:1; /* Post-processing stirring */ - unsigned int disable_unbias:1; /* Deactivate Von-Neuman unbias */ -#define JENT_MEMORY_BLOCKS 64 -#define JENT_MEMORY_BLOCKSIZE 32 -#define JENT_MEMORY_ACCESSLOOPS 128 -#define JENT_MEMORY_SIZE (JENT_MEMORY_BLOCKS*JENT_MEMORY_BLOCKSIZE) - unsigned char *mem; /* Memory access location with size of - * memblocks * memblocksize */ - unsigned int memlocation; /* Pointer to byte in *mem */ - unsigned int memblocks; /* Number of memory blocks in *mem */ - unsigned int memblocksize; /* Size of one memory block in bytes */ - unsigned int memaccessloops; /* Number of memory accesses per random - * bit generation */ -}; - /* Flags that can be used to initialize the RNG */ #define JENT_DISABLE_STIR (1<<0) /* Disable stirring the entropy pool */ #define JENT_DISABLE_UNBIAS (1<<1) /* Disable the Von-Neuman Unbiaser */ diff --git a/include/crypto/jitterentropy.h b/include/crypto/jitterentropy.h new file mode 100644 index 0000000..7ed8f20 --- /dev/null +++ b/include/crypto/jitterentropy.h @@ -0,0 +1,80 @@ +/* + * Copyright (C) 2017, Stephan Mueller <smueller@chronox.de> + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, and the entire permission notice in its entirety, + * including the disclaimer of warranties. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * ALTERNATIVELY, this product may be distributed under the terms of + * the GNU General Public License, in which case the provisions of the GPL2 are + * required INSTEAD OF the above restrictions. (This clause is + * necessary due to a potential bad interaction between the GPL and + * the restrictions contained in a BSD-style copyright.) + * + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF + * WHICH ARE HEREBY DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT + * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE + * USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH + * DAMAGE. + */ + +#ifndef _JITTERENTROPY_H +#define _JITTERENTROPY_H + +typedef unsigned long long __u64; +typedef long long __s64; + +/* The entropy pool */ +struct rand_data { + /* + * All data values that are vital to maintain the security + * of the RNG are marked as SENSITIVE. A user must not + * access that information while the RNG executes its loops to + * calculate the next random value. + */ + __u64 data; /* SENSITIVE Actual random number */ + __u64 old_data; /* SENSITIVE Previous random number */ + __u64 prev_time; /* SENSITIVE Previous time stamp */ +#define DATA_SIZE_BITS ((sizeof(__u64)) * 8) + __u64 last_delta; /* SENSITIVE stuck test */ + __s64 last_delta2; /* SENSITIVE stuck test */ + unsigned int stuck:1; /* Time measurement stuck */ + unsigned int osr; /* Oversample rate */ + unsigned int stir:1; /* Post-processing stirring */ + unsigned int disable_unbias:1; /* Deactivate Von-Neuman unbias */ +#define JENT_MEMORY_BLOCKS 64 +#define JENT_MEMORY_BLOCKSIZE 32 +#define JENT_MEMORY_ACCESSLOOPS 128 +#define JENT_MEMORY_SIZE (JENT_MEMORY_BLOCKS*JENT_MEMORY_BLOCKSIZE) + unsigned char *mem; /* + * Memory access location with size of + * memblocks * memblocksize + */ + unsigned int memlocation; /* Pointer to byte in *mem */ + unsigned int memblocks; /* Number of memory blocks in *mem */ + unsigned int memblocksize; /* Size of one memory block in bytes */ + unsigned int memaccessloops; /* Number of memory accesses per random + * bit generation */ +}; + +int jent_entropy_init(void); +int jent_read_entropy(struct rand_data *ec, unsigned char *data, + unsigned int len); + +#endif /* _JITTERENTROPY_H */
To support the LRNG operation which allocates the Jitter RNG separately from the kernel crypto API, extract the relevant information into a separate header file. CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org> CC: Arnd Bergmann <arnd@arndb.de> CC: Jason A. Donenfeld <Jason@zx2c4.com> Signed-off-by: Stephan Mueller <smueller@chronox.de> --- crypto/jitterentropy.c | 33 ++--------------- include/crypto/jitterentropy.h | 80 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 31 deletions(-) create mode 100644 include/crypto/jitterentropy.h