nhorman / rng-tools

The rng-tools official repository (formerly part of the gkernel project on sourceforge)
GNU General Public License v2.0
156 stars 61 forks source link

rdrand-rng as a Linux kernel module #25

Closed abelbeck closed 6 years ago

abelbeck commented 6 years ago

Hi, Our project cross-compiles rng-tools (much thanks, BTW), and the gcc 4.8.3 toolchain assembler does not recognize the rdseed instruction ...

/home/dev/astlinux/trunk/output/host/usr/bin/x86_64-unknown-linux-gnu-gcc -DHAVE_CONFIG_H -I.   -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -pipe -Os  -MT rdrand_asm.o -MD -MP -MF .deps/rdrand_asm.Tpo -c -o rdrand_asm.o rdrand_asm.S
rdrand_asm.S: Assembler messages:
rdrand_asm.S:61: Error: no such instruction: `rdseed %rax'
make[3]: *** [rdrand_asm.o] Error 1

We worked around this by patching configure.ac https://github.com/astlinux-project/astlinux/blob/master/package/rng-tools/rng-tools-0001-disable-RDRAND.patch

REQUEST: please add a --without-rdrand configure option so we don't have to maintain the patch.

Followup, more interestingly, we looked for alternatives for a rdrand-rng and discovered an elegant kernel patch by the VMware "photon" folks: https://github.com/vmware/photon/commit/d694d7b3f4fb83816a6c29c8af9e4c3e29dbd53b

The referenced patch for Linux 4.4 and 4.9 was trivial to back-port to our 3.16 kernel, and works perfectly with rng-tools 6.3.1. At least for our use case, it is more desirable to have rdrand as a 0: Hardware RNG Device. Also no need for libgcrypt for non-aes-ni CPU's.

Possibly others will find the referenced rdrand-rng kernel module useful wrt rng-tools, as we have.

nhorman commented 6 years ago

Thank you for the report. I really don't want to make the selection of rdrand (or any of the cpu instruction based entropy sources) a manual process. I'd like to keep it inclusive if the arch is appropriate without having to make the choice by hand. That said, I think we can do that. Currently, the selection is made based on the host cpu, which, based on your report, I think is a bug. We should be making the configure selection based on the target cpu rather than the host cpu, which is the problem you are running into it seems. I've made a branch that does that here: https://github.com/nhorman/rng-tools/tree/rdrand-fix

If you would please, give it a try. It works for me on x86, but I don't have a cross environment set up at the moment, so if you could try it in yours and confirm that HAVE_RDRAND gets unset properly there, that would be a big help. If it works, I'll merge it.

As for the vmware rdrand driver, I don't think thats going to get merged into the kernel. theres honestly no need for it. The kernel is capable of accessing cpu based entropy instructions without the need for additional code, and userspace can access it in the same way. The only thing a driver adds is additional overhead shuttling the entropy between user and kernel

abelbeck commented 6 years ago

Thanks, I tried your commit, but the exact same result...

rdrand_asm.S:61: Error: no such instruction: `rdseed %rax'

We cross-compile to a generic class of x86_64, some might have rdrand and some not, we don't compile to a specific rdrand target CPU.

nhorman commented 6 years ago

instead of asking to disable the rdrand feature, why not just add -mrdrand to your compiler flags? That will enable the emitting of rdrand instructions. The rdrand entropy source preforms a runtime check to ensure the cpu in question supports rdrand, and simply doesn't initialize that entropy source if it finds it can't use the instruction. I'd much rather see the instruction used opportunistically at run time, than never used at all because of a decision made at build time

abelbeck commented 6 years ago

Hmmm, I have tried...

CFLAGS="$(TARGET_CFLAGS) -mrdseed -mrdrnd"

yields the same no such instruction error. I'm puzzled.

We are quite happy using the rdrand-rng kernel module, as such /dev/hwrng must be present before we start rngd. For us virtio-rng is another common source via /dev/hwrng.

Also, without adding libgcrypt, as I understand it, rnd-tool's native rdrand support will not work on non-aes-ni CPU's, whereas the rdrand-rng kernel module does.

@nhorman Please set me straight why the rdrand-rng kernel module may not be ideal. Thanks for any info.

nhorman commented 6 years ago

that...is puzzling. By all rights, that should have worked. That said, I do see some posts indicating that those gcc flags didn't work in earlier versions of gcc, near the time rdrand/rdseed support was added (around the 4.6 time frame). You might try enabling -mavx2 (which clang tied to rdrand/rdseed). Or alternatively upgrading your compiler (4.6 is pretty old, the most recent version is 8.2)

virtio-rng is a highly recommended source of entropy, as it allows a modicum of centralization in entropy gathering (that is to say you can do all your entropy collection in the host from physical sources and dole it out to the guests without having to incur the management of physical devices in the guests via sriov/vfio. I was under the impression though, that vmware didn't support virtio-rng from their hypervisor..

your comment regarding the availability of libgcrypt is incorrect. libgcrypt is used opportunistically to mix the output of the rdrand instruction. If libgcrypt is not present at build time (or if --without-libgcrypt is specified), then AES mixing will not be preformed and the raw data from rdseed/rdrand will be used as the output of that entropy source. You can see this if you look at the code, specifically the xread_drng_with_aes and gcrypt_mange functions. You can also disable the use of libgcrypt AES mixing at run time in the most recent rng-tools code by specifying the -O 2:use_aes:0 option

Regarding your question about the use of a rdrand-rng kernel module, the answer lies in the accesability of the hardware entropy source. Unlike normal hardware random number generators, which reside on the pci (or platform specific) bus, and require a kernel module to physically access from user space, the RDRAND/RDSEED instructions are unprivileged and can be accessed directly from user space (as rng-tools is currently demonstrating). As such, because linux/bsd by and large require entropy be pushed into the kernel from user space, adding a kernel driver module to export entropy from these cpu instructions up to user space, only to push them back into the kernel entropy pool really has no benefit. It just adds code to the kernel, and slows the entropy gathering process down by adding additional traps that rngd would have to preform.

you may be thinking that adding a kernel driver would give the kernel immediate, direct access to the entropy from the rdrand/rdseed instructions. Thats not the case. The kernel doesn't need a driver to do that. We could easily add 20 lines of code in the kernel to detect the availability of these instructions and use them to fill the entropy pool directly, but we don't do that as a matter of policy. Not everyone trusts the entropy generated by these instructions, as so linux (and bsd, and most unixes), require that entropy generation be preformed by user space and fed into the kernel, so that entropy sources can come under the control of administrative policy). As such, its more expedient to just use the instructions in rngd, and feed that into the kernel, rather than load a kernel driver module that does for us what we can already do for ourselves.

Hope that answers your question

abelbeck commented 6 years ago

@nhorman Thanks so much for your comments, a couple of reply comments.

your comment regarding the availability of libgcrypt is incorrect

Thanks for explaining, I guess I got off track from this configure.ac note:

[Disable libgcrypt support. Systems that support RDRAND but not AES-NI will require libgcrypt in order to use RDRAND as an entropy source. (Default: --with-libgcrypt)]

I was under the impression though, that vmware didn't support virtio-rng from their hypervisor.

VMware does not support virtio-rng (AFAIK), but KVM/QEMU host environments do, in practice it seems virtio-rng forwards the host /dev/hwrng or /dev/urandom to the guest's /dev/hwrng, I was not aware that additional elements of entropy were added as well, thanks.

you may be thinking that adding a kernel driver would give the kernel immediate, direct access to the entropy from the rdrand/rdseed instructions. Thats not the case. The kernel doesn't need a driver to do that. We could easily add 20 lines of code in the kernel to detect the availability of these instructions and use them to fill the entropy pool directly

Yes, using rng-tools native RDRAND would be more efficient than calling a kernel driver, but rng-tools native RDRAND is not an option for us (as described). Though having a rdrand-rng module outputted to /dev/hwrng makes it easy to compare with other RNG sources using rngtool.

FYI, Here is an example using rngd and our project's custom image on Linode using the rdrand-rng kernel module...

linode ~ # ls -l /dev/hwrng
ls: /dev/hwrng: No such file or directory
linode ~ # modprobe rdrand-rng
linode ~ # ls -l /dev/hwrng
crw-------    1 root     root       10, 183 Aug 27 14:27 /dev/hwrng
linode ~ # cat /sys/devices/virtual/misc/hw_random/rng_available
rdrand_rng 
linode ~ # cat /sys/devices/virtual/misc/hw_random/rng_current  
rdrand_rng
linode ~ # cat /proc/sys/kernel/random/entropy_avail 
147
linode ~ # service rngd init
Starting rngd...
linode ~ # cat /proc/sys/kernel/random/entropy_avail 
3072

Note: virtio-rng is not supported on Linode.

nhorman commented 6 years ago

Hey, In reply

Regarding that comment, yes, its old and I should update it. I'll take care of that shortly.

Regarding the use of virtio-rng, thats almost correct. virtio-rng on the host side doesn't actually forward any devices into the guest (you can do that using vfio if you like). Instead virtio-rng presents a virtualized piece of hardware to a guests as a hardware rng (usually named /dev/hwrng in the guest), and on the host side it is attached simply to the kernel entropy pool. Hypercalls from the guest to the host transfer random data to each guest. Population of the host side entropy pool is handled in the traditional way, by running a host side copy of rngd pulling from other entropy sources.

If you have any influence with vmware, I would strongly suggest that they add support for virtio-rng devices to their hypervisor. Its standard practice for all linux based systems (of which ESX and vsphere are part), and it would solve a great deal of problems for them.

Regarding your comments about rng-tools and its rdrand entropy source not being an option for you...I'm not sure what you mean. I understand its not building for you, but thats not a reason to not use it, its a reason to fix the problem. I had mentioned in a previous post here that I had done some reading indicating that -mrdrand didn't seem to work for a bit shortly after its introduction, and that updating your compiler should be a solution there, as might be using -avx2 (which may include support for rdrand instructions). don't worry about accidentally enabling the emission of avx2 instructions on unsupported hardware, gcc shouldn't emit those instructions unless you use the intrinsic macros or explicitly code them in with asm() statements.

also I'm not sure I follow you when you say "compare with other rng sources". You can do that fairly easily with rngd by enabling entropy sources one at a time and reading the entropy avail value (though thats a pretty bad comparison, as the entropy availability is a function of both how fast a given source can generate entropy, and how fast a consumer can drain it).

abelbeck commented 6 years ago

Hi @nhorman I tried a lot of CFLAGS options, -mavx2 -masm=intel -mrdseed -mrdrnd and permutations thereof with the same error.

Further investigation, the assembler handles rdrand just fine, the problem is the rdseed instruction.

Just as a test I replaced the two occurrences of rdseed with rdrand:

sed -i 's/rdseed\s/rdrand\t/' output/build/rng-tools-6.3.1/rdrand_asm.S

That built just fine.

Given this, can you think of a way to work around this rdseed issue ?

nhorman commented 6 years ago

well, I suppose you could delete the two definitions of x86_rdseed_or_rdrand_bytes in rdrand_asm.S and then comment out the call to that function in rngd_rdrand.c. You will also need to set have_rdseed to 0 in init_drng_entropy_source after the runtime cpuid support check. That will ensure at run time that you never use the rdseed instruction, only rdrand.

I can't stress enough though, thats absolutely the wrong approach to take here. You have a broken compiler, which seems to be incapable of emitting an instruction that it needs to be able to emit (and which subsequent versions of the compiler clearly can). The right answer here is to update your compiler to a version that doesn't have this bug, and then just use rng-tools as-is (with the patch that I wrote you to enable rdrand based on build target rather than build host)

abelbeck commented 6 years ago

OK, I got it to work by using .byte for the rdseed instructions, per this patch (as rng-tools v5 does)

--- rng-tools-6.3.1/rdrand_asm.S.orig   2018-08-29 09:52:46.773547700 -0500
+++ rng-tools-6.3.1/rdrand_asm.S    2018-08-29 09:55:17.549915865 -0500
@@ -58,7 +58,7 @@
 1:
    mov $RDRAND_RETRY_LIMIT, %r10d
 2:
-   rdseed  %rax
+   .byte   0x48,0x0f,0xc7,0xf8 /* rdseed %rax */
    jnc 3f
    mov %rax, (%rdi)
    add $8, %rdi
@@ -140,7 +140,7 @@
 1:
    mov $RDRAND_RETRY_LIMIT, %ecx
 2:
-   rdseed  %eax
+   .byte   0x0f,0xc7,0xf8      /* rdseed %eax */
    jnc 3f
    mov %eax, (%edi)
    add $4, %edi

But... RDRAND works fine for an aes CPU, but a non-aes CPU RDRAND does not enable when built with --without-libgcrypt and rng-tools-6.3.1 does not have the -O runtime option.

It would be nice if RDRAND would work with --without-libgcrypt on non-aes CPU's by default.

Again, many thanks for your time here.

nhorman commented 6 years ago

can you clarify what you mean by "does not enable"? I'm building it here just fine: [nhorman@hmswarspite rng-tools]$ ./autogen.sh configure.ac:45: installing './compile' configure.ac:23: installing './config.guess' configure.ac:23: installing './config.sub' configure.ac:24: installing './install-sh' configure.ac:24: installing './missing' Makefile.am: installing './INSTALL' Makefile.am: installing './COPYING' using GNU General Public License v3 file Makefile.am: Consider adding the COPYING file to the version control system Makefile.am: for your code, to avoid questions about which license your project uses Makefile.am: installing './depcomp' parallel-tests: installing './test-driver' [nhorman@hmswarspite rng-tools]$ [nhorman@hmswarspite rng-tools]$ [nhorman@hmswarspite rng-tools]$ ./configure --without-libgcrypt checking build system type... x86_64-pc-linux-gnu checking host system type... x86_64-pc-linux-gnu checking target system type... x86_64-pc-linux-gnu checking for a BSD-compatible install... /usr/bin/install -c checking whether build environment is sane... yes checking for a thread-safe mkdir -p... /usr/bin/mkdir -p checking for gawk... gawk checking whether make sets $(MAKE)... yes checking whether make supports nested variables... yes checking whether to enable maintainer-specific portions of Makefiles... no checking for style of include used by make... GNU checking for gcc... gcc checking whether the C compiler works... yes checking for C compiler default output file name... a.out checking for suffix of executables... checking whether we are cross compiling... no checking for suffix of object files... o checking whether we are using the GNU C compiler... yes checking whether gcc accepts -g... yes checking for gcc option to accept ISO C89... none needed checking whether gcc understands -c and -o together... yes checking dependency style of gcc... gcc3 checking dependency style of gcc... gcc3 checking for gcc... (cached) gcc checking whether we are using the GNU C compiler... (cached) yes checking whether gcc accepts -g... (cached) yes checking for gcc option to accept ISO C89... (cached) none needed checking whether gcc understands -c and -o together... (cached) yes checking dependency style of gcc... (cached) gcc3 checking for ranlib... ranlib checking how to run the C preprocessor... gcc -E checking for grep that handles long lines and -e... /usr/bin/grep checking for egrep... /usr/bin/grep -E checking whether gcc needs -traditional... no checking for a sed that does not truncate output... /usr/bin/sed checking whether gcc is Clang... no checking whether pthreads work with -pthread... yes checking for joinable pthread attribute... PTHREAD_CREATE_JOINABLE checking whether more special flags are required for pthreads... no checking for PTHREAD_PRIO_INHERIT... yes checking for pkg-config... /usr/bin/pkg-config checking pkg-config is at least version 0.9.0... yes checking for libcurl... yes checking for libxml2... yes checking for openssl... yes checking for library containing sysfs_get_mnt_path... -lsysfs checking for ANSI C header files... yes checking for sys/types.h... yes checking for sys/stat.h... yes checking for stdlib.h... yes checking for string.h... yes checking for memory.h... yes checking for strings.h... yes checking for inttypes.h... yes checking for stdint.h... yes checking for unistd.h... yes checking dependency style of gcc... (cached) gcc3 checking that generated files are newer than configure... done configure: creating ./config.status config.status: creating Makefile config.status: creating contrib/Makefile config.status: creating tests/Makefile config.status: creating rngd.8 config.status: creating rngtest.1 config.status: creating rng-tools-config.h config.status: executing depfiles commands [nhorman@hmswarspite rng-tools]$ [nhorman@hmswarspite rng-tools]$ [nhorman@hmswarspite rng-tools]$ grep GCRYPT rng-tools-config.h / #undef HAVE_LIBGCRYPT / [nhorman@hmswarspite rng-tools]$ make ... [nhorman@hmswarspite rng-tools]$ ./rngd --list Entropy sources that are available but disabled

4: NIST Network Entropy Beacon

Available and enabled entropy sources:

2: Intel RDRAND Instruction RNG

5: JITTER Entropy generator

root@hmswarspite rng-tools]# ./rngd -f -d -x5 Disabling 5: JITTER Entropy generator

Initalizing available sources

read error

hwrng: no available rng Failed to init entropy source 0: Hardware RNG Device

Unable to open file: /dev/tpm0 Failed to init entropy source 1: TPM RNG Device

Enabling RDSEED rng support

Reading entropy from Intel RDRAND Instruction RNG

Its working exactly as it should be for me. Are you having trouble building without libgcrypt? Or is it failing at run time? What is the precise error you are seeing?

abelbeck commented 6 years ago

Builds perfectly, it is a runtime error not finding RDRAND

pbx3 ~ # rngd -f -d -x5
Disabling 5: JITTER Entropy generator

Initalizing available sources
Unable to open file: /dev/hwrng
Failed to init entropy source 0: Hardware RNG Device

Unable to open file: /dev/tpm0
Failed to init entropy source 1: TPM RNG Device

Failed to init entropy source 2: Intel RDRAND Instruction RNG

can't open any entropy source
Maybe RNG device modules are not loaded

Works fine on an aes-ni CPU.

The above lscpu flags:

Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 ssse3 cx16 xtpr pdcm sse4_1 sse4_2 movbe popcnt tsc_deadline_timer rdrand lahf_lm 3dnowprefetch ida arat epb dtherm tpr_shadow vnmi flexpriority ept vpid tsc_adjust smep erms
nhorman commented 6 years ago

ah, crud, thats right I'm running on a cpu with built in AES support, and I forgot to disable aes use if both it and gcrypt aren't present. I'll fix that.

nhorman commented 6 years ago

https://github.com/nhorman/rng-tools/tree/gcrypt-runtime-fix

There, that should fix the problem. It will note that you are running on a system without any AES crypto facilities in the log, and simply disable the use of AES. Please give that a shot and confirm that it solves the issue for you (note that that branch also includes my previous fix for selecting RDRAND entropy based on target arch rather than host arch)

abelbeck commented 6 years ago

Thanks, I'll give it a try.

Quick question, why use AES after RDRAND anyway ? As I understand it the RDRAND instruction already performs AES "so the numbers have multiplicative prediction resistance up to 128 bits and additive beyond 128"

https://stackoverflow.com/a/18004959

Or is this a don't-trust-Intel thing ?

Is there a worry that rng-tool's AES might make things less random ?

nhorman commented 6 years ago

Its a bit of the latter reason. In the stackoverflow post you reference, the author notes that RDRAND instructions draw entropy from an AES based PRNG (pseudo rng). That means that, hardwired into each intel cpu is a (hopefully) unique key, that iteratively encrypts a seed value using that key. the output data then is effectively random for any attacker that doesn't know the secret key or seed (the latter of which IIRC on chips without rdseed instructions, is a fixed number). So if an attacker is able to somehow figure out what your cpus AES key is, they can predict your output entropy stream. The use of AES in rngd is meant to provide an extra layer of obfuscation, so that if your CPU key is compromised, you also have an additional layer of mixing via the key that is generated from whatever garbage is on the stack when you start the rngd process.

abelbeck commented 6 years ago

OK, the gcrypt-runtime-fix works on a non-aes CPU

pbx3 ~ # rngd -f -d -x5               
Disabling 5: JITTER Entropy generator

Initalizing available sources

Unable to open file: /dev/hwrng
Failed to init entropy source 0: Hardware RNG Device

Unable to open file: /dev/tpm0
Failed to init entropy source 1: TPM RNG Device

No AES method available for RDRAND

Enabling RDRAND rng support

Reading entropy from Intel RDRAND Instruction RNG

Though it appears 6.3.1 does not allow non-aes RDRAND with --without-libgcrypt

From above...

Its a bit of the latter reason. In the stackoverflow post you reference, the author notes that RDRAND instructions draw entropy from an AES based PRNG (pseudo rng).

But from the stackoverflow post (assuming accurate)

The seeds come from a true random number generator. This involves a 2.5Gbps entropy source that is fed into a 3:1 compression ratio entropy extractor using AES-CBC-MAC.

I'm no cryptographer, but I would worry performing additional AES after RDRAND could make it less random.

nhorman commented 6 years ago

Ok, good to hear it, I'll merge that fix shortly.

Regarding your comment about 6.3.1 not working with non-AES enabled cpus and without gcrypt...I'm not sure what to tell you other than....yes. thats why I fixed it here. It will work in 6.3.2.

Regarding the seed value generation coming from a TRNG is good to know, though I'm not sure I'd trust that unilaterally (consider running this on an AMD processor, theres no x86 implementation requirement that seeds must come from an rng source).

Regarding your comment regarding using a second AES pass, thats not an argument I'm going to get into, as its, at best anectodal. All we can do when measuring the quality of a randomness source, is attempt to quantify the likelyhood that the next value extracted from the source is predictable (i.e. can be guessed to within a certain probability based on previously extracted values). To that end we regularly run rngd through the fips 140-2 test suite, which preforms several different statistical analyses on our provided entropy sources in rngd, and it consistently passes with two AES passes in the RDRAND source.

That said, its valid to want your random data to come straight from the hardware, if thats something your use case needs. Thats why I added the -O flag, which lets you turn off the second AES pass, even if your cpu support aesni. That will be availabe in 6.3.2 as well

abelbeck commented 6 years ago

@nhorman Thanks for all your attention and work. Much appreciated.

Ok, good to hear it, I'll merge that fix shortly.

BTW, It would be nice to also have a message with -O 2:use_aes:0 on aes-ni hardware that no AES is being used.

Is the state of jitterentropy-library in rnd-tools still experimental ? We have requests to add haveged for cases that rngd currently does not handle, possibly jitterentropy would be a reasonable alternative ?

Thanks again.

nhorman commented 6 years ago

the branch that you tested should be logging a message to the system journal regarding the fact that AES use is being disabled already.

Regarding jitterentropy, it is not experimental, and was added to rngd specifically in response to requests for haveged, as haveged is a single entropy source and therefore considered largely insecure as a source of random data. Given what we've fixed here, I'll release a 6.3.2 rngd soon, and you can build against that.

abelbeck commented 6 years ago

I see v6.4, excellent.

Great to hear about jitterentropy, though the linked source is not in the v6.4 Github release assets.

Am I missing a rng-tools-6.4.tar.gz somewhere that contains the jitterentropy-library and has ./autogen.sh pre-run ? Such a rng-tools-6.4.tar.gz would make it much easier for projects to to use rng-tools.

nhorman commented 6 years ago

no, the git tree populates the jitterentropy library itself using git-submodules. For a release you need to go get the jitterentropy release itself and populate the source in the subdirectory. Ideally we will base jitterentropy inclusion using a standard method like pkg-config, so that you can just install the shared library, but the jitterentropy maintainer hasn't added that feature yet.

abelbeck commented 6 years ago

Possibly you could generate such a release tarball (including jitterentropy and generate ./configure) and upload it to the Github release assets.

I think more people will use the jitterentropy feature if it was available in a standard tarball.

Again, a big thanks for your work.

nhorman commented 6 years ago

No, Im sorry, I'm specifically avoiding packaging that with rng-tools releases, as I don't want to consider the jitterentropy library to be supported by this release. If you take a look at the rng-tools package in Fedora, you'll see its pretty easy to add the library yourself, though the best solution (which I'm currently working on), is to just dynamically link against the jitterentropy library if its installed. Look for that in 6.5

abelbeck commented 6 years ago

I got jitterentropy working, seems to work well, though takes a few seconds to initialize.

A few comments: 1) rngd -O 5 show a use_aes key, seems you left out:

{
    .key = NULL,
}

Here: https://github.com/nhorman/rng-tools/blob/master/rngd.c#L179

2) With jitterentropy threads running, a simple TERM of /var/run/rngd.pid does not work anymore. What seems to work fine is:

start-stop-daemon -S -x /usr/sbin/rngd -p /var/run/rngd.pid -m -b -- -f -q

As such it looks like pid_fd is not initialized for the foreground case. Here: https://github.com/nhorman/rng-tools/blob/master/rngd.c#L513

3) If a CPU has RDRAND support, is there any significant advantage to also running jitterentropy ?

Our init.d script could do some test and add -x5 for such cases, but it would be a nice feature if there was a --best-only or such flag that would only start the best source and no others. Does that make sense ?

nhorman commented 6 years ago

theres no way to determine what the 'best' source is, since the 'best' entropy is really a subjective term based on your use case (i.e the best for some is a hwrng, a cprng for others where shared key predictability is desireable, and multiple sources for others who want ot defend against an attack on any one source).

the TERM issue is an odd one, I'll look into that. I'll also fix the array terminator

abelbeck commented 6 years ago

@nhorman FYI I got a build error linking with libjitterentropy.a on a 32-bit build.

/home/dev/astlinux/x-tools-1.20.0-3.16/i586-unknown-linux-gnu/lib/gcc/i586-unknown-linux-gnu/4.8.3/../../../../i586-unknown-linux-gnu/bin/ld: i386:x86-64 architecture of input file `./jitterentropy-library/libjitterentropy.a(jitterentropy-base.o)' is incompatible with i386 output
collect2: error: ld returned 1 exit status
make[3]: *** [rngd] Error 1
abelbeck commented 6 years ago

Ahhh, when cross-compiling the target GCC and such are not being passed, so it is using the host ... not good ...

Making all in jitterentropy-library
make[3]: Entering directory `/home/dev/astlinux/trunk/output/build/rng-tools-6.4/jitterentropy-library'
gcc -Wextra -Wall -pedantic -fPIC -O0 -fstack-protector-strong -fwrapv --param ssp-buffer-size=4    -c -o jitterentropy-base.o jitterentropy-base.c
gcc -shared -Wl,-soname,libjitterentropy.so.2 -o libjitterentropy.so.2.1.1 jitterentropy-base.o -Wl,-z,relro,-z,now  -lrt
ar rcs libjitterentropy.a jitterentropy-base.o

My host is 64-bit but the target is 32-bit, hence the link error.

nhorman commented 6 years ago

This is why I want to move to a dynamically lnked environment. The build system for jitterentropy is maintained by smuller, you'll have to take it up with him on his github project

abelbeck commented 6 years ago

Hi @nhorman , yes but jitterentropy currently does not use autoconf so I suspect it will be hitched to your wagon until that changes. :-)

The following patch fixes the cross-compile build failure:

--- rng-tools-6.4/configure.ac.orig 2018-08-31 08:17:44.776831614 -0500
+++ rng-tools-6.4/configure.ac  2018-08-31 08:18:56.329353120 -0500
@@ -49,6 +49,8 @@
 AC_PROG_RANLIB
 AC_PROG_GCC_TRADITIONAL

+AC_CHECK_TOOLS([AR], [ar gar], :)
+
 AX_PTHREAD

 AM_CONDITIONAL([RDRAND], [test $target_cpu = x86_64 -o $target_cpu = i686])
--- rng-tools-6.4/Makefile.am.orig  2018-08-31 03:59:22.194083147 -0500
+++ rng-tools-6.4/Makefile.am   2018-08-31 04:03:38.430448774 -0500
@@ -5,6 +5,7 @@
  JSUBDIR = jitterentropy-library
  JSUBLIB = ./jitterentropy-library/libjitterentropy.a
  AM_CPPFLAGS = -I./jitterentropy-library
+ export CC AR CFLAGS LDFLAGS
 endif

 SUBDIRS        = contrib tests $(JSUBDIR) 
--- rng-tools-6.4/jitterentropy-library/Makefile.orig   2018-08-31 04:00:22.085845162 -0500
+++ rng-tools-6.4/jitterentropy-library/Makefile    2018-08-31 04:02:20.103797007 -0500
@@ -1,9 +1,9 @@
 # Compile Noise Source as user space application

-CC=gcc
+CC ?= gcc
 CFLAGS +=-Wextra -Wall -pedantic -fPIC -O0
 #Hardening
-CFLAGS +=-fstack-protector-strong -fwrapv --param ssp-buffer-size=4
+CFLAGS +=-fstack-protector-all -fwrapv --param ssp-buffer-size=4
 LDFLAGS +=-Wl,-z,relro,-z,now

 # Change as necessary

The main Makefile also uses ar so you should check for it.

Tested and works. In jitterentropy, our toolchain compiler did not like -fstack-protector-strong so I changed it to -fstack-protector-all.

nhorman commented 6 years ago

No, thats my point, because I use submodules to pull in jitterentropy, I can't take any changes that modify the the jitterentropy-library code, which yours above does. Adding @smuellerDD . @smuellerDD would you be will to take the appropriate pieces of the above patch until such time as I start doing exclusively run time bindings to your library?

smuellerDD commented 6 years ago

Sure, will do.

nhorman commented 6 years ago

thank you, I'll take my bits then when I pull your latest library

nhorman commented 6 years ago

I've fixed the exit hang in the latest head for the master branch

abelbeck commented 6 years ago

@nhorman Just a reminder as it probably got lost in this dialog...

pid_fd in main() is not initialized here: https://github.com/nhorman/rng-tools/blob/master/rngd.c#L524 I suggest:

-  pid_t pid_fd;
+  pid_t pid_fd = -1;

for the !(arguments->daemon) case, when run in the foreground pid_fd is uninitialized in a test on exit.

We prefer to start rngd using start-stop-daemon ... -b -- -f ... so boot time is not delayed while rngd starts-up.

nhorman commented 6 years ago

yup, fixed in commit b772ac9d368eeed2bac1e3611098ffbe1a4a30e7

smuellerDD commented 6 years ago

jitterentropy-library Makefile fixed as suggested by Lonnie.