Re: [Openvpn-devel] [PATCH v2] auth-gen-token: Hardening memory cleanup on auth-token failuers

2016-12-15 Thread David Sommerseth 
[lets try unencrypted this time.]

On 15/12/16 22:52, Steffan Karger wrote:
> Hi,
> 
> On 15 December 2016 at 13:22, David Sommerseth  wrote:
>> Further improve the memory management when a clients --auth-token
>> fails the server side token authentication enabled via --auth-gen-token.
>>
>> v2 - Add ASSERT() if base64 encoding of token fails
> 
> This will need rebasing because of the reformatting.
> 
>> @@ -1255,24 +1269,11 @@ verify_user_pass(struct user_pass *up, struct 
>> tls_multi *multi,
>>   /* The token should be longer than the input when
>> * being base64 encoded
>> */
>> - if( openvpn_base64_encode(tok, AUTH_TOKEN_SIZE,
>> ->auth_token) < AUTH_TOKEN_SIZE)
>> -   {
>> - msg(D_TLS_ERRORS, "BASE64 encoding of token failed. "
>> -  "No auth-token will be activated now");
>> [...]
>> +  ASSERT(openvpn_base64_encode(tok, AUTH_TOKEN_SIZE,
>> +   >auth_token) < 
>> AUTH_TOKEN_SIZE);
> 
> Uhm, I think the < should be a > now?
> 

*facepalm*

I was completely sure I had tested it.  I really did run tests.  I just
ran the server and client on the opposite hosts.  So the VM which had
not been updated which should run the client ran the server mode instead
... thus no issues :/

I'll send a v3 patch now.

-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2] auth-gen-token: Hardening memory cleanup on auth-token failuers

2016-12-15 Thread Steffan Karger 
Hi,

On 15 December 2016 at 13:22, David Sommerseth  wrote:
> Further improve the memory management when a clients --auth-token
> fails the server side token authentication enabled via --auth-gen-token.
>
> v2 - Add ASSERT() if base64 encoding of token fails

This will need rebasing because of the reformatting.

> @@ -1255,24 +1269,11 @@ verify_user_pass(struct user_pass *up, struct 
> tls_multi *multi,
>   /* The token should be longer than the input when
> * being base64 encoded
> */
> - if( openvpn_base64_encode(tok, AUTH_TOKEN_SIZE,
> ->auth_token) < AUTH_TOKEN_SIZE)
> -   {
> - msg(D_TLS_ERRORS, "BASE64 encoding of token failed. "
> -  "No auth-token will be activated now");
> [...]
> +  ASSERT(openvpn_base64_encode(tok, AUTH_TOKEN_SIZE,
> +   >auth_token) < 
> AUTH_TOKEN_SIZE);

Uhm, I think the < should be a > now?

-Steffan

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] auth-gen-token: Hardening memory cleanup on auth-token failuers

2016-12-15 Thread David Sommerseth 
Further improve the memory management when a clients --auth-token
fails the server side token authentication enabled via --auth-gen-token.

v2 - Add ASSERT() if base64 encoding of token fails

Signed-off-by: David Sommerseth 
---
 src/openvpn/ssl_verify.c | 45 +++--
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 4328828..955d522 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1108,6 +1108,21 @@ verify_user_pass_management (struct tls_session 
*session, const struct user_pass
 }
 #endif
 
+/**
+ *  Wipes the authentication token out of the memory, frees and cleans up 
related buffers and flags
+ *
+ *  @param multi  Pointer to a multi object holding the auth_token variables
+ */
+static void
+wipe_auth_token(struct tls_multi *multi)
+{
+  secure_memzero (multi->auth_token, AUTH_TOKEN_SIZE);
+  free (multi->auth_token);
+  multi->auth_token = NULL;
+  multi->auth_token_sent = false;
+}
+
+
 /*
  * Main username/password verification entry point
  */
@@ -1157,6 +1172,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi 
*multi,
   /* Ensure that the username has not changed */
   if (!tls_lock_username(multi, up->username))
 {
+  wipe_auth_token(multi);
   ks->authenticated = false;
   goto done;
 }
@@ -1168,6 +1184,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi 
*multi,
   && (multi->auth_token_tstamp + session->opt->auth_token_lifetime) < 
now)
 {
   msg (D_HANDSHAKE, "Auth-token for client expired\n");
+  wipe_auth_token(multi);
   ks->authenticated = false;
   goto done;
 }
@@ -1176,10 +1193,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)
 {
-  secure_memzero (multi->auth_token, AUTH_TOKEN_SIZE);
-  free (multi->auth_token);
-  multi->auth_token = NULL;
-  multi->auth_token_sent = false;
+  wipe_auth_token(multi);
   ks->authenticated = false;
   tls_deauthenticate (multi);
 
@@ -1255,24 +1269,11 @@ verify_user_pass(struct user_pass *up, struct tls_multi 
*multi,
  /* The token should be longer than the input when
* being base64 encoded
*/
- if( openvpn_base64_encode(tok, AUTH_TOKEN_SIZE,
->auth_token) < AUTH_TOKEN_SIZE)
-   {
- msg(D_TLS_ERRORS, "BASE64 encoding of token failed. "
-  "No auth-token will be activated now");
- if (multi->auth_token)
-   {
- secure_memzero (multi->auth_token, AUTH_TOKEN_SIZE);
- free (multi->auth_token);
- multi->auth_token = NULL;
-   }
-   }
- else
-   {
- multi->auth_token_tstamp = now;
- dmsg (D_SHOW_KEYS, "Generated token for client: %s",
-multi->auth_token);
-   }
+  ASSERT(openvpn_base64_encode(tok, AUTH_TOKEN_SIZE,
+   >auth_token) < AUTH_TOKEN_SIZE);
+  multi->auth_token_tstamp = now;
+  dmsg (D_SHOW_KEYS, "Generated token for client: %s",
+multi->auth_token);
}
 
   if ((session->opt->ssl_flags & SSLF_USERNAME_AS_COMMON_NAME))
-- 
1.8.3.1


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel