Browse code

Always clear username/password from memory on error

This issue was found by Quarkslab during the OSTIF-founded security audit
(issue 5.4), we are with their analysis:

"There’s a special case where the client username and password are not
erased when the server is launched without an external script or
authentication plugin. While being invalid, this configuration does not
raise any error. If the client transmits its credentials and the session
is not established (for instance if the certificates chain has not been
verified), these credentials are not erased from memory by the server.

The likelihood of an occurrence of this issue in real life is
exceptionally low since an attacker needs elevated privileges on the
server to exploit this kind of information leak. The severity of this
issue is rated as very low."

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: David Sommerseth <davids@openvpn.net>
Message-Id: <1494354764-19354-1-git-send-email-steffan.karger@fox-it.com>
URL: http://www.mail-archive.com/search?l=mid&q=1494354764-19354-1-git-send-email-steffan.karger@fox-it.com
Signed-off-by: David Sommerseth <davids@openvpn.net>

Steffan Karger authored on 2017/05/10 03:32:44
Showing 1 changed files
... ...
@@ -2492,7 +2492,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_sessio
2492 2492
 
2493 2493
     struct gc_arena gc = gc_new();
2494 2494
     char *options;
2495
-    struct user_pass *up;
2495
+    struct user_pass *up = NULL;
2496 2496
 
2497 2497
     /* allocate temporary objects */
2498 2498
     ALLOC_ARRAY_CLEAR_GC(options, char, TLS_OPTIONS_LEN, &gc);
... ...
@@ -2654,6 +2654,10 @@ key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_sessio
2654 2654
 
2655 2655
 error:
2656 2656
     secure_memzero(ks->key_src, sizeof(*ks->key_src));
2657
+    if (up)
2658
+    {
2659
+        secure_memzero(up, sizeof(*up));
2660
+    }
2657 2661
     buf_clear(buf);
2658 2662
     gc_free(&gc);
2659 2663
     return false;