Opened 4 years ago

Closed 4 years ago

#751 closed Bug / Defect (fixed)

When erasing secrets, use a memset() that's not optimized away

Reported by: Steffan Karger Owned by:
Priority: minor Milestone: beta 2.4
Component: Crypto Version: OpenVPN 2.3.12 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords: hardening constant-time
Cc:

Description

We have a number of memset calls like this in the code:

  ret = true;

exit:
  CLEAR (key2);

  return ret;

Where CLEAR(x) does a memset(x, 0, sizeof(x)). Modern compilers will detect that x is never read afterwards, and optimize away the memset(x) call. In the cases where we want to erase secrets, replace the memset() calls with something that won't be optimized away (which is not sufficient, I know, but at least makes bugs like hardbleed a little harder to exploit).

Hints: use memset_s(), and provide our own implementation if it is not available.

Change History (2)

comment:1 Changed 4 years ago by David Sommerseth

commit 009521ac8ae613084b23b9e3e5dc4ebeccd4c6c8
Author: Steffan Karger <steffan@karger.me>
Date:   Mon Nov 28 23:14:12 2016 +0100

    Introduce and use secure_memzero() to erase secrets
    
    As described in trac #751, and shortly after reported by Zhaomo Yang, of
    the University of California, San Diego, we use memset() (often through
    the CLEAR() macro) to erase secrets after use.  In some cases however, the
    compiler might optimize these calls away.
    
    This patch replaces these memset() calls on secrets by calls to a new
    secure_memzero() function, that will not be optimized away.
    
    Since we use CLEAR() a LOT of times, I'm not changing that to use
    secure_memzero() to prevent performance impact.  I did annotate the macro
    to point people at secure_memzero().
    
    This patch also replaces some CLEAR() or memset() calls with a zero-
    initialization using "= { 0 }" if that has the same effect, because that
    better captures the intend of that code.
    
    Trac: #751
    
    Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
    Acked-by: Selva Nair <selva.nair@gmail.com>
    Acked-by: Gert Doering <gert@greenie.muc.de>
    Message-Id: <1480371252-3880-1-git-send-email-steffan@karger.me>
    URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13278.html
    Signed-off-by: Gert Doering <gert@greenie.muc.de>

comment:2 Changed 4 years ago by Steffan Karger

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.