Memory zeroization improvements #185

issue gmaxwell openend this issue on January 11, 2015
  1. gmaxwell commented at 5:00 am on January 11, 2015: contributor

    Existing ‘best effort’ zeriozation for private data is hardly even best effort. At a minimum we should consider doing this via an extern-ed function and memset_s if available. No guarantees can still be provided, of course.

    We might also consider wrapping the API entrance of private data handling functions like:

    handle_data(){ ret=handle_data_impl(); handle_data_zero_stack(); return ret; }

    Where _zero_stack uses slightly more stack than the whole callgraph for _impl and zeros it, in order to catch private data spilled onto the stack during execution before returning outside of our control.

    I’m not sure where exactly where the line between best effort and security theatre is… there is only so much that can really be done (esp in portable code) here.

  2. gmaxwell added the label enhancement on Jan 11, 2015
  3. briansmith commented at 11:01 am on April 28, 2015: contributor
    I’m not sure what you mean by “no guarantees can be provided.” C11 memset_s and SecureZeroMemory both guarantee zeroization when they are used, when they are available.
  4. sipa commented at 11:03 am on April 28, 2015: contributor
    They may guarantee wiping those bytes of memory. I don’t think they can guarantee that the compiler didn’t copy parts of that data to other places (like the stack).
  5. dcousens commented at 11:39 pm on April 28, 2015: contributor

    guarantee that the compiler didn’t copy parts of that data to other places (like the stack). @sipa can you ever make that guarantee? At least, not without checking the generated assembly every time?

  6. sipa commented at 11:42 pm on April 28, 2015: contributor
    Not without checking the binary, you mean; no, that’s why @gmaxwell is saying that it is best effort.
  7. gmaxwell commented at 0:59 am on April 30, 2015: contributor

    @briansmith As Pieter noted, the compiler can and will store arbitrary secret data in random places on the stack (in fact, it can even stash them in random globals and such). This isn’t just hypothetical either.

    It can also leave them sitting around in registers, and from there other code can save them onto the stack or other locations where they might escape.

    I’m dubious about the value of having memset_s() in the standard: I’ll gladly use it for better best effort; but I think the standards committee made an error in judgement– since it leads to incorrect reasoning about what can actually be accomplished.

    So the best we can do is best-effort; maybe best effort with some non-runtime tests (e.g. load secrets with known patterns and search the stack for them after execution). I’m all for best effort where better can’t be done. Approaches like I suggested of flushing out the stack have a fighting chance at being reliable at least for now… though even that isn’t guaranteed to work; e.g. you can’t guarantee the compiler will give the zero_stack function accesses to the redzone on architectures that have them, if zero_stack is just plain old C.

  8. gmaxwell added this to the milestone initial release on Aug 27, 2015
  9. briansmith referenced this in commit b76f52c03a on Feb 5, 2016
  10. real-or-random commented at 6:29 pm on May 30, 2019: contributor

    Useful resources: https://www.usenix.org/conference/usenixsecurity17/technical-sessions/presentation/yang https://media.ccc.de/v/35c3-9788-memsad

    So we all know that there no single right way of doing this. For building on #448 I think I’ll switch to memset + memory barrier where this is available (GCC, clang) because it does not require external libraries incl. glibc, and the compiler can still optimize the memset (just not optimize it out). The USENIX paper confirms that this is good performance-wise.

    As a fallback, we can try to use platform-specific functions or a volatile store, I need to see how ugly it gets.

    By the way, Bitcoin Core currently uses memset + memory barrier or Windows’s SecureZeroMemory: https://github.com/bitcoin/bitcoin/blob/master/src/support/cleanse.cpp#L31 (However, note the weird logic: On Windows, the memory is cleared twice.) This restricts Core to GCG, clang or MSVC. I think we want to be more portable in secp256k1.

  11. gmaxwell commented at 5:44 pm on June 5, 2019: contributor
    ACK
  12. laanwj referenced this in commit 7f985d6c81 on Jul 3, 2019
  13. monstrobishi referenced this in commit 9f02fb8536 on Sep 6, 2020
  14. PastaPastaPasta referenced this in commit 7efdda4676 on Jun 27, 2021
  15. PastaPastaPasta referenced this in commit 5c0bf5a9fb on Jun 28, 2021
  16. PastaPastaPasta referenced this in commit f95eee6bf5 on Jun 29, 2021
  17. PastaPastaPasta referenced this in commit 38567ca4d7 on Jul 1, 2021
  18. PastaPastaPasta referenced this in commit c2143f62fc on Jul 1, 2021
  19. PastaPastaPasta referenced this in commit 99ec02f43c on Jul 12, 2021
  20. PastaPastaPasta referenced this in commit 6fd162e296 on Jul 13, 2021
  21. gades referenced this in commit 0b0bc91cbe on Apr 29, 2022
  22. real-or-random removed the label enhancement on Jan 5, 2023
  23. real-or-random closed this on Nov 4, 2024

  24. fanquake referenced this in commit b161bffb8b on Nov 4, 2024


gmaxwell briansmith sipa dcousens real-or-random

Milestone
stable release (1.0.0-rc.1)


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-21 16:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me