Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#10402 closed defect (fixed)

Force disable use of RDRAND in OpenSSL when HardwareAccel is enabled

Reported by: anon Owned by:
Priority: High Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: 023-backport, tor-relay, security, 2016-bug-retrospective
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

FreeBSD announced direct use of RDRAND as sole entropy source is not recommended.[1][2][3]

In Tor crypto_global_init() there is a call to ENGINE_load_builtin_engines() which lets OpenSSL take advantage of
AES-NI acceleration. This is almost always A Good Thing.

From Sandy Bridge onward, however, this also results in the application using RDRAND directly for all entropy.

Since Tor cannot build the OpenSSL linked against (to set OPENSSL_NO_RDRAND), the workaround is to call RAND_set_rand_engine(NULL) after ENGINE_load_builtin_engines().

  1. "FreeBSD Developer Summit: Security Working Group, /dev/random" https://wiki.freebsd.org/201309DevSummit/Security
  1. "Surreptitiously Tampering with Computer Chips" https://www.schneier.com/blog/archives/2013/09/surreptitiously.html
  1. "How does the NSA break SSL? ... Weak random number generators" http://blog.cryptographyengineering.com/2013/12/how-does-nsa-break-ssl.html

Child Tickets

Attachments (3)

0001-Never-allow-OpenSSL-engines-to-replace-the-RAND_SSLe.patch (3.2 KB) - added by nickm 6 years ago.
patch candidate
patch-for-025.patch (3.1 KB) - added by nickm 6 years ago.
openssl-1.0.1e-badrand-test.patch (9.5 KB) - added by anon 6 years ago.
For Testing Only BAD RAND engine to observe RDRAND behavior in application

Download all attachments as: .zip

Change History (26)

comment:1 Changed 6 years ago by anon

Patches here:
https://peertech.org/dist/tor-0.2.4.19-rdrand-disable.patch
https://peertech.org/dist/tor-0.2.5.1-rdrand-disable.patch
https://peertech.org/dist/tor-latest-rdrand-disable.patch

If you have a Sandy Bridge, Ivy Bridge, or other recent Intel CPU with RDRAND, can you try building Tor against openssl-1.0.1e and HardwareAccel 1 set in config?

If you see a log message:

 ... "OpenSSL is using RDRAND by default. Attempting to force disable."

please post the results of the following "Using OpenSSL engine ... for RAND" statement to confirm.

comment:2 Changed 6 years ago by rl1987

I have applied your patch (tor-latest-rdrand-disable.patch) to Tor on Mac OS X Mavericks system with Intel Core i5 CPU that, according to sysctl hw.optional.rdrand has support for RDRAND instruction. Tor source code was cloned from Git master and was compiled with OpenSSL 1.0.1e. When trying to run the patched Tor, it crashed with the following error message:

Dec 14 18:30:52.000 [warn] OpenSSL is using RDRAND by default. Attempting to force disable.

============================================================ T= 1387038652
Tor 0.2.5.1-alpha-dev (git-e6590efaa77c8cf1) died: Caught signal 11
0   tor                                 0x0000000109b7f2ce crash_handler + 46
1   libsystem_platform.dylib            0x00007fff9897d5aa _sigtramp + 26
2   libcrypto.1.0.0.dylib               0x0000000109d928ae ENGINE_set_default_RAND + 7
3   tor                                 0x0000000109b95594 crypto_global_init + 932
4   tor                                 0x0000000109b2b655 tor_init + 933
5   tor                                 0x0000000109b2bab7 tor_main + 87
6   tor                                 0x0000000109aa3b99 main + 25
7   libdyld.dylib                       0x00007fff989c55fd start + 1
8   ???                                 0x0000000000000001 0x0 + 1
Abort trap: 6
Last edited 6 years ago by rl1987 (previous) (diff)

comment:3 Changed 6 years ago by rl1987

In collaboration with coderman from #tor-dev, it was found that changing ENGINE_set_default_RAND(NULL); to ENGINE_unregister_RAND(t); prevents the crash.

comment:4 Changed 6 years ago by anon

Patches updated:
https://peertech.org/dist/tor-0.2.4.19-rdrand-disable.patch
https://peertech.org/dist/tor-0.2.5.1-rdrand-disable.patch
https://peertech.org/dist/tor-latest-rdrand-disable.patch

The diff should look like:

      /* If we are using a version of OpenSSL that supports native RDRAND
         make sure that we force disable its use as sole entropy source.
         See https://trac.torproject.org/projects/tor/ticket/10402 */
      if (SSLeay() > OPENSSL_V_SERIES(1,0,0)) {
        t = ENGINE_get_default_RAND();
        if (t &&
            (strcmp(ENGINE_get_id(t), "rdrand") == 0)) {
          log_warn(LD_CRYPTO, "OpenSSL is using RDRAND by default."
                   " Attempting to force disable.");
          ENGINE_unregister_RAND(t);
          ENGINE_register_all_complete();
        }
      }
      /* Log, if available, the intersection of the set of algorithms
         used by Tor and the set of algorithms available in the engine */
      log_engine("RSA", ENGINE_get_default_RSA());
 .
 .

And should result in a log like:

Dec ... [warn] OpenSSL is using RDRAND by default. Attempting to force disable.
Dec ... [notice] Using OpenSSL engine RSAX engine support [rsax] for RSA
Dec ... [notice] Using default implementation for RAND
 .
 .

What you should NOT see is this line: "[notice] Using OpenSSL engine Intel RDRAND engine [rdrand] for RAND" which is synonymous with EPICFAIL. *grin*

comment:5 Changed 6 years ago by anon

One last note: the OS kernel will likely use RDRAND to keep /dev/random populated. This is a Good Thing (TM) as long as you make sure you're using a recent kernel that integrates RDRAND properly, e.g.:

  1. Mix RDRAND into a hash across the pool, not XOR'd against output
  2. Mix the mix back into pool (prevent backtrack attacks)
  3. Atomically extract portion of pool and mix
  4. Fold final extracted output in half for conservative operation

See https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/char/random.c#n1038

comment:6 Changed 6 years ago by rl1987

Status: newneeds_review

comment:7 Changed 6 years ago by arma

Milestone: Tor: 0.2.5.x-final

comment:8 Changed 6 years ago by nickm

Keywords: 024-backport tor-relay security added

The engine table code is a bit tricky to follow, and the API is a bit odd. Has anybody used a debugger or other tool to verify that after unregistering the engine, we really get the original PRNG ? (That is, to check that that the md_rand.c code is called, and the eng_rdrand.c code is not.)

comment:9 Changed 6 years ago by nickm

That patch didn't work for me (Tor 0.2.4, OSX 10.9.1, openssl 1.0.1e from homebrew). I get:

Dec 18 10:55:56.000 [warn] OpenSSL is using RDRAND by default. Attempting to force disable.
Dec 18 10:55:56.000 [notice] Using OpenSSL engine RSAX engine support [rsax] for RSA
Dec 18 10:55:56.000 [notice] Using OpenSSL engine Intel RDRAND engine [rdrand] for RAND

comment:10 Changed 6 years ago by nickm

On investigation, I think that the log message is wrong: It looks at ENGINE_get_default_RAND(), which should have just gotten pulled off the engine table.

There's also something else screwy going in here, though. It appears that we aren't actually using the RDRAND backend, even when I enable HardwareAccel.

AHA.

We are frequently not using any RAND engine at all. (That's probably a good thing!)

That's because, if any RAND_* function is invoked before setting a RAND engine, the call in rand_lib.c to RAND_get_rand_method() will set the default_RAND_method pointer. That RAND_METHOD pointer will stay that way until it's changed, and nothing will change it unless we somehow clear and re-initialize it.

And when do we first call any random function? During ordinary operation, if we start up with a state file that has any circuit build time values, we'll shuffle them in circuit_build_times_shuffle_store_array.

If we have no state file, then the rand method is not set when the crypto_init() code is first called.

Of course, this isn't really a fix, because when we first start Tor, we'll have no state file, and we'll use the default rand method after all. But wow, this junk sure made debugging hard.

It also suggested a much, much easier fix.

comment:11 Changed 6 years ago by nickm

Keywords: 023-backport added; 024-backport removed
Milestone: Tor: 0.2.5.x-finalTor: 0.2.4.x-final

Branch bug10402_redux_024 in my public repository takes the simpler approach. Please review. Does this work for you?

(I am fairly sure that the other one did not work; when I added logging code to dump RAND_get_rand_method() and RAND_SSLeay(), and ran Tor with an empty state file, those methods were different before.)

Changed 6 years ago by nickm

patch candidate

Changed 6 years ago by nickm

Attachment: patch-for-025.patch added

comment:12 Changed 6 years ago by anon

Hi Nick,

Regarding "Of course, this isn't really a fix, because when we first start Tor, we'll have no state file, and we'll use the default rand method after all. But wow, this junk sure made debugging hard."

I created a BADENGINE that behaves like RdRand except uses libc rand() for testing purposes. I ran the current git Tor on a clean run (no state file) and on subsequent runs.

This confirmed that on the first run without state file, RdRand (or BADRAND) will be used by default, but every subsequent run it will not. Unfortunately first run might also generate long lived secrets depending on Tor config.

Test setup used:
http://www.openssl.org/source/openssl-1.0.1e.tar.gz
with the BADENGINE patch:
https://peertech.org/dist/openssl-1.0.1e-badrand-test.patch

OpenSSL was built with:

./Configure --prefix=/usr/local/test threads shared linux-x86_64

And Tor was built with:

./configure --prefix=/usr/local/test \
 --with-openssl-dir=/usr/local/test \
 --disable-asciidoc \
 --with-tcmalloc

Tor was using config:

Log info stderr
RunAsDaemon 0
DataDirectory /usr/local/test/var/lib/tor
HardwareAccel 1
SocksPort 9060

The first run log is here:

https://peertech.org/dist/tor-badrand-test-first-run.txt

And the second:

https://peertech.org/dist/tor-badrand-test-second-run.txt

With the first showing use of "BADRAND invocation# ..." while the second and subsequent runs with a state file do NOT use the defaults engines for RAND.

I am confirming your fix now...

comment:13 Changed 6 years ago by anon

First patch on your patch: "... to an alleged new alleged entropy source is never to throw away ..."

typo correct to:
"... to a new entropy source is never to throw away" ?

comment:14 Changed 6 years ago by anon

Confirmed!
"Dec 18 14:25:20.000 [notice] It appears that one of our engines has provided a replacement the OpenSSL RNG. Resetting it to the default implementation."

First run with fix:

https://peertech.org/dist/tor-fixrand-verify-first-run.txt

Second run with fix:

https://peertech.org/dist/tor-fixrand-verify-second-run.txt

Changed 6 years ago by anon

For Testing Only BAD RAND engine to observe RDRAND behavior in application

comment:15 Changed 6 years ago by rl1987

Nick's patch seems to work for me as well. I am getting the "... Resetting to default implementation" line provided that I have deleted state file from Tor data directory.

comment:16 Changed 6 years ago by nickm

Milestone: Tor: 0.2.4.x-finalTor: 0.2.3.x-final

Okay, merged. (I missed fixing the typo in the commit message; it will serve as a monument to my fallibility.)

leaving open for possible 0.2.3 backport.

comment:17 Changed 6 years ago by arma

Just to be clear, is this an issue (on first Tor start) even when hardwareaccel is off? Or is it a problem for all Tor users on first start?

Should we recommend people generate new relay keys? New hidden service keys?

comment:18 Changed 6 years ago by anon

arma: only an issue on first-start with no state file AND HardwareAccel 1 (enable) set in config. This is not the default.

Subsequent runs with a state file, or systems using default without hardware acceleration, will not use RDRAND to generate keys.

If there are any relay operators who deployed new relays on Sandy Bridge or Ivy Bridge hardware with HardwareAccel enabled on first run, then they should re-generate keys.

comment:19 Changed 5 years ago by arma

Recommend closing, since this is a non-standard config, it's been 14 months, and also 0.2.3.x is soon to be gone.

comment:20 Changed 5 years ago by nickm

Milestone: Tor: 0.2.3.x-finalTor: 0.2.4.x-final
Resolution: fixed
Status: needs_reviewclosed

Ack; closing.

comment:21 Changed 4 years ago by nickm

Keywords: 2016-bug-retrospective added

Mark bugs for 2016 bug retrospective based on hand-examination of changelogs for 0.2.5 onwards.

comment:22 Changed 4 years ago by nickm

Mark bugs for 2016 bug retrospective based on hand-examination of changelogs for 0.2.5 onwards.

comment:23 Changed 4 years ago by nickm

Mark bugs for 2016 bug retrospective based on hand-examination of changelogs for 0.2.5 onwards.

Note: See TracTickets for help on using tickets.