diff mbox series

[Openvpn-devel,v2] Move pre decrypt lite check to its own function

Message ID 20220425122709.4148015-1-arne@rfc2549.org
State Accepted
Headers
Series [Openvpn-devel,v2] Move pre decrypt lite check to its own function |

Commit Message

Arne Schwabe April 25, 2022, 2:27 a.m. UTC 
  This prepares for extending this function with the HMAC based session ID
check.

Replace the check for m->top.c2.tls_auth_standalone with an ASSERT as this
code path is only used in multi udp server and OpenVPN initialises the
tls_auth_standalone always for the TOP context (CF_INIT_TLS_AUTH_STANDALONE),
even for the tcp m2mp server that does not use it).

Patch v2: replace if with ASSERT

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

Comments

Frank Lichtenheld April 25, 2022, 3:08 a.m. UTC | #1 
> Arne Schwabe <arne@rfc2549.org> hat am 25.04.2022 14:27 geschrieben:
[...]
> diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
> index 4fbe3c1a3..780ca171d 100644
> --- a/src/openvpn/mudp.c
> +++ b/src/openvpn/mudp.c
> @@ -39,6 +39,17 @@
>  #include <sys/inotify.h>
>  #endif
>  
> +static bool
> +do_pre_decrypt_check(struct multi_context *m)
> +{
> +    ASSERT(m->top.c2.tls_auth_standalone);
> +    if (!tls_pre_decrypt_lite(m->top.c2.tls_auth_standalone, &m->top.c2.from, &m->top.c2.buf))
> +    {
> +        return false;
> +    }
> +    return true;

You could replace four lines with one:

return tls_pre_decrypt_lite(m->top.c2.tls_auth_standalone, &m->top.c2.from, &m->top.c2.buf));

But that might not make sense due to changes in later patches? Haven't checked.

Regards,
--
Frank Lichtenheld
Arne Schwabe April 25, 2022, 3:12 a.m. UTC | #2 
Am 25.04.22 um 15:08 schrieb Frank Lichtenheld:
>> Arne Schwabe <arne@rfc2549.org> hat am 25.04.2022 14:27 geschrieben:
> [...]
>> diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
>> index 4fbe3c1a3..780ca171d 100644
>> --- a/src/openvpn/mudp.c
>> +++ b/src/openvpn/mudp.c
>> @@ -39,6 +39,17 @@
>>   #include <sys/inotify.h>
>>   #endif
>>   
>> +static bool
>> +do_pre_decrypt_check(struct multi_context *m)
>> +{
>> +    ASSERT(m->top.c2.tls_auth_standalone);
>> +    if (!tls_pre_decrypt_lite(m->top.c2.tls_auth_standalone, &m->top.c2.from, &m->top.c2.buf))
>> +    {
>> +        return false;
>> +    }
>> +    return true;
> 
> You could replace four lines with one:
> 
> return tls_pre_decrypt_lite(m->top.c2.tls_auth_standalone, &m->top.c2.from, &m->top.c2.buf));
> 
> But that might not make sense due to changes in later patches? Haven't checked.
> 

The function grows to a 70 line function over the course of the patches. :)

Arne
Frank Lichtenheld April 25, 2022, 3:19 a.m. UTC | #3 
Acked-By: Frank Lichtenheld <frank@lichtenheld.com>

> Arne Schwabe <arne@rfc2549.org> hat am 25.04.2022 15:12 geschrieben:
> Am 25.04.22 um 15:08 schrieb Frank Lichtenheld:
[...]
> > You could replace four lines with one:
> > 
> > return tls_pre_decrypt_lite(m->top.c2.tls_auth_standalone, &m->top.c2.from, &m->top.c2.buf));
> > 
> > But that might not make sense due to changes in later patches? Haven't checked.
> > 
> 
> The function grows to a 70 line function over the course of the patches. :)

Fair enough. Acked.

Regards,
--
Frank Lichtenheld
Gert Doering April 25, 2022, 3:51 a.m. UTC | #4 
Your patch has been applied to the master branch.

commit 73713debf56c06ed54a378f9b3d1d742c5f1ed45
Author: Arne Schwabe
Date:   Mon Apr 25 14:27:09 2022 +0200

     Move pre decrypt lite check to its own function

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Message-Id: <20220425122709.4148015-1-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24193.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 4fbe3c1a3..780ca171d 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -39,6 +39,17 @@ 
 #include <sys/inotify.h>
 #endif
 
+static bool
+do_pre_decrypt_check(struct multi_context *m)
+{
+    ASSERT(m->top.c2.tls_auth_standalone);
+    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 +106,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))
                 {