Enable spectre mitigations when supported by compiler. #12412

1. 
   jameshilliard commented at 3:38 PM on February 11, 2018: contributor

   Potential option for resolving #12091 using compiler flags recently added in OpenSSH. Audit of assembly code in libsecp256k1 may still be required.

2. fanquake added the label Build system on Feb 11, 2018
3. fanquake requested review from theuni on Feb 13, 2018
4. Enable spectre mitigations when supported by compiler. 20879d31ef
5. jameshilliard force-pushed on Feb 18, 2018
6. 
   fanquake commented at 6:12 AM on February 19, 2018: member

   @jameshilliard What's the best way to test this? I assume it will only work with very recent Clang/GCC?

   Did ./configure with master + https://github.com/bitcoin/bitcoin/pull/12412/commits/20879d31ef9dc7f54cd7f4ea52a417d3b337fba5 on OS X:

   checking whether C++ compiler accepts -Wstack-protector... yes
   checking whether C++ compiler accepts -fstack-protector-all... yes
   checking whether C++ compiler accepts -mfunction-return=thunk... no
   checking whether C++ compiler accepts -mindirect-branch=thunk... no
   checking whether C++ compiler accepts -mretpoline... no
   checking whether C++ preprocessor accepts -D_FORTIFY_SOURCE=2... yes
   

7. 
   jameshilliard commented at 8:53 AM on February 19, 2018: contributor

   @fanquake This is pretty much untested other than checking that the flags don't get set when not available on OS X. Yeah I think you just need a really new Clang/GCC.

8. 
   theuni commented at 5:26 PM on February 19, 2018: member

   I'm not convinced that this is the right thing to do.

   Not only does Bitcoin core need to be compiled this way in order to be safe, the libc and c++ stdlib need to be rebuilt as well. I'd hate to take a performance it, and only be providing a false sense of security. For example, this would not be enough to protect our current gitian builds.

   Seems like this should really be done at the distro level. Are any distros forcing these on by default?

9. 
   jameshilliard commented at 5:36 PM on February 19, 2018: contributor

   Seems like this should really be done at the distro level.

   Tricky part is mitigations are needed at both levels and we intentionally avoid letting bitcoin core get included in distro package managers.

   Are any distros forcing these on by default?

   Sounds like some plan to. Maybe we should just detect if the distro has libs with mitigations and then enable only when they do at compile time.

10. 
    eklitzke commented at 6:56 AM on March 22, 2018: contributor

    AIUI Spectre style attacks let an attacker read memory already mapped in their own address space (contrast to meltdown, which is much more severe). That's why the focus w/ spectre has been on JS engines: it allows "sandboxed" javascript code to read in-process browser data outside of the sandbox.

    That's a serious, but very particular (and unusual) use case. What kind of attack specifically do you think this would insulate Bitcoin from?

11. 
    sipa commented at 11:24 PM on March 22, 2018: member

    @eklitzke Indeed, I think that style of attacks only applies to programs that run JIT compiled untrusted code. I can't see anything in Bitcoin Core that would risk acting like that.

12. 
    fanquake commented at 4:22 AM on March 24, 2018: member

    Going to close this for now. After discussing with @theuni, this doesn't seem to be how we'd want to tackle this, if we even do overall. Discussing can continue in #12091.

13. fanquake closed this on Mar 24, 2018

    ---

14. MarcoFalke locked this on Sep 8, 2021

Review Requested
theuni