On Mon, Nov 28, 2016 at 7:13 AM, Steffan Karger <steffan.kar...@fox-it.com>
wrote:
> 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.
>
> Signed-off-by: Steffan Karger <steffan.kar...@fox-it.com>
>
All look good except that it might have missed some memset calls that could
benefit from a SecureZeroMemory() or equivalent:
With no pretense of completeness, here is a list with some context: calls
marked with <-- SecureZero
--- a/src/openvpn/console_builtin.c
+++ b/src/openvpn/console_builtin.c
@@ -218,7 +218,7 @@ static bool get_console_input (const char *prompt,
const bool echo, char *input,
if (gp)
{
strncpynt (input, gp, capacity);
- memset (gp, 0, strlen (gp));
+ memset (gp, 0, strlen (gp)); <-- SecureZero
ret = true;
}
}
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -501,7 +501,7 @@ remove_env_item (const char *str, const bool do_free,
struct env_item **list)
*list = current->next;
if (do_free)
{
- memset (current->string, 0, strlen (current->string));
+ memset (current->string, 0, strlen (current->string)); <--
SecureZero
free (current->string);
free (current);
}
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1212,7 +1212,7 @@ tls_multi_free (struct tls_multi *multi, bool clear)
if (multi->auth_token)
{
- memset (multi->auth_token, 0, AUTH_TOKEN_SIZE);
+ memset (multi->auth_token, 0, AUTH_TOKEN_SIZE); <-- SecureZero
free (multi->auth_token);
}
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1176,7 +1176,7 @@ verify_user_pass(struct user_pass *up, struct
tls_multi *multi,
if (memcmp_constant_time(multi->auth_token, up->password,
strlen(multi->auth_token)) != 0)
{
- memset (multi->auth_token, 0, AUTH_TOKEN_SIZE);
+ memset (multi->auth_token, 0, AUTH_TOKEN_SIZE); <-- SecureZero
free (multi->auth_token);
Selva
------------------------------------------------------------------------------
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
