[Openvpn-devel,03/28] Move pre decrypt lite check to its own function

Message ID 20220422134038.3801239-4-arne@rfc2549.org
State Superseded
Headers show
Series
  • Stateless three-way handshake and control channel improvements
Related show

Commit Message

Arne Schwabe April 22, 2022, 1:40 p.m.
This prepares for extending this function with the HMAC based session ID
check.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/mudp.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Frank Lichtenheld April 22, 2022, 3:52 p.m. | #1
> Arne Schwabe <arne@rfc2549.org> hat am 22.04.2022 15:40 geschrieben:
> diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
> index 4fbe3c1a3..910268333 100644
> --- a/src/openvpn/mudp.c
> +++ b/src/openvpn/mudp.c
> @@ -39,6 +39,20 @@
>  #include <sys/inotify.h>
>  #endif
>  
> +static bool
> +do_pre_decrypt_check(struct multi_context *m)
> +{
> +    if (!m->top.c2.tls_auth_standalone)
> +    {
> +        return false;
> +    }
> +    if (!tls_pre_decrypt_lite(m->top.c2.tls_auth_standalone, &m->top.c2.from, &m->top.c2.buf))
> +    {
> +        return false;
> +    }
> +    return true;

Shouldn't this be
if (!m->top.c2.tls_auth_standalone)
{
   return true;
}
if (tls_pre_decrypt_lite(m->top.c2.tls_auth_standalone, &m->top.c2.from, &m->top.c2.buf))
{
   return true;
}
return false;

Your patch seems to mangle the logic in several different ways.

> +}
> +
>  /*
>   * Get a client instance based on real address.  If
>   * the instance doesn't exist, create it while
> @@ -95,8 +109,7 @@ multi_get_create_instance_udp(struct multi_context *m, bool *floated)
>          }
>          if (!mi)
>          {
> -            if (!m->top.c2.tls_auth_standalone
> -                || tls_pre_decrypt_lite(m->top.c2.tls_auth_standalone, &m->top.c2.from, &m->top.c2.buf))
> +            if (do_pre_decrypt_check(m))
>              {
>                  if (frequency_limit_event_allowed(m->new_connection_limiter))
>                  {

--
Frank Lichtenheld
Arne Schwabe April 25, 2022, 12:06 p.m. | #2
Am 22.04.22 um 17:52 schrieb Frank Lichtenheld:
> 
> 
>> Arne Schwabe <arne@rfc2549.org> hat am 22.04.2022 15:40 geschrieben:
>> diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
>> index 4fbe3c1a3..910268333 100644
>> --- a/src/openvpn/mudp.c
>> +++ b/src/openvpn/mudp.c
>> @@ -39,6 +39,20 @@
>>   #include <sys/inotify.h>
>>   #endif
>>   
>> +static bool
>> +do_pre_decrypt_check(struct multi_context *m)
>> +{
>> +    if (!m->top.c2.tls_auth_standalone)
>> +    {
>> +        return false;
>> +    }
>> +    if (!tls_pre_decrypt_lite(m->top.c2.tls_auth_standalone, &m->top.c2.from, &m->top.c2.buf))
>> +    {
>> +        return false;
>> +    }
>> +    return true;
> 
> Shouldn't this be
> if (!m->top.c2.tls_auth_standalone)

Yes. But then again the whole line is dead code anyway because 
m->top.c2.tls_auth_standalone is always initialised.

Arne

Patch

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 4fbe3c1a3..910268333 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -39,6 +39,20 @@ 
 #include <sys/inotify.h>
 #endif
 
+static bool
+do_pre_decrypt_check(struct multi_context *m)
+{
+    if (!m->top.c2.tls_auth_standalone)
+    {
+        return false;
+    }
+    if (!tls_pre_decrypt_lite(m->top.c2.tls_auth_standalone, &m->top.c2.from, &m->top.c2.buf))
+    {
+        return false;
+    }
+    return true;
+}
+
 /*
  * Get a client instance based on real address.  If
  * the instance doesn't exist, create it while
@@ -95,8 +109,7 @@  multi_get_create_instance_udp(struct multi_context *m, bool *floated)
         }
         if (!mi)
         {
-            if (!m->top.c2.tls_auth_standalone
-                || tls_pre_decrypt_lite(m->top.c2.tls_auth_standalone, &m->top.c2.from, &m->top.c2.buf))
+            if (do_pre_decrypt_check(m))
             {
                 if (frequency_limit_event_allowed(m->new_connection_limiter))
                 {