Opened 6 years ago

Closed 6 years ago

#7352 closed defect (fixed)

make defense-in-depth memsets work in spite of compilers

Reported by: nickm_mobile Owned by:
Priority: High Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-relay
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


In many places in our code, we memset things to zero before leaving a function or freeing memory, in an attempt to limit the damage that could be done by a postfacto information leak. But it appears that compilers like to "helpfully" optimize these out.

I've verified that the Llvm-gcc from the latest OSX does this; I haven't yet had time to check vanilla gccs yet.

Obviously, we should fix this. It is not a vulnerability per se, but if there are information leaks from memory, it could make their impact worse than they would be otherwise. It seems like an 024 issue but I could be persuaded otherwise.

Andrey Karpov pointed this out in a recent article, but I can't realistically copy links on this phone.

Next steps: look at actual compiler output from more compilers; identify portable or platform-specific memset replacements that won't get optimized out

Child Tickets

Change History (7)

comment:1 Changed 6 years ago by nickm_mobile

Solar Designer points out OPENSSL_cleanse(), for better or worse. Might be an option for the fallback extra portable version.

comment:2 Changed 6 years ago by arma

Summary: make defensein-depth memsets work in spite of compilersmake defense-in-depth memsets work in spite of compilers

comment:3 Changed 6 years ago by nickm

Status: newneeds_review

Sketch fix in branch "bug7352_023" in my public repository. Please review!

comment:4 Changed 6 years ago by nickm

Because people are probably going to get linked here, here's a description of the problem, its severity, and the solution:

  o Major bugfixes:
    - Tor tries to wipe potentially sensitive data after using it, so
      that if some subsequent security failure exposes Tor's memory,
      the damage will be limited. But we had a bug where the compiler
      was eliminating these wipe operations when it decided that the
      memory was no longer visible to a (correctly running) program,
      hence defeating our attempt at defense in depth. We fix that
      by using OpenSSL's OPENSSL_cleanse() operation, which a compiler
      is unlikely to optimize away. Future versions of Tor may use
      a less ridiculously heavy approach for this. Fixes bug 7352.
      Reported in an article by Andrey Karpov.

comment:5 Changed 6 years ago by nickm

To be specific, when this lands in 0.2.4 we should start looking for faster alternatives to OPENSSL_cleanse(). This are likely to be less portable. We should also grep for all the new memset() instances in 0.2.4.

I used memwipe() here not only on stack-allocated stuff that was about to go out-of-scope, but also on heap-allocated stuff that we were about to get freed, since I bet that somebody out there optimizes those out too, or will eventually.

comment:6 Changed 6 years ago by nickm

Updated even more ; mostly with comment fixes and answers to FAQs in the comments.

comment:7 Changed 6 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged; added a new #7419 to track the "we would like a faster memwipe maybe" ticket; closing this one.

Note: See TracTickets for help on using tickets.