| Message ID | 20220422134038.3801239-4-arne@rfc2549.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Stateless three-way handshake and control channel improvements | expand |
> 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
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
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)) {
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(-)