[Openvpn-devel] Don't print OCC warnings about 'key-method', 'keydir' and 'tls-auth'

Message ID 1533722327-5228-1-git-send-email-steffan.karger@fox-it.com
State Superseded
Headers show
Series [Openvpn-devel] Don't print OCC warnings about 'key-method', 'keydir' and 'tls-auth' | expand

Commit Message

Steffan Karger Aug. 7, 2018, 11:58 p.m. UTC
Like 'proto', a mismatch in key-method, keydir or tls-auth would fail
before we ever get to the point where we can print this warning.

This prepares for removing these from the occ string later on, but also
prepares for tls-crypt-v2, which allows a server to support tls-auth and
tls-crypt-v2 connections in parallel.  Such a server will send 'keydir'
and 'tls-auth' in the occ string.  This change removes the spurious
warnings about that in the client log.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
---
 src/openvpn/options.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Gert Doering Oct. 7, 2018, 2:01 a.m. UTC | #1
Hi,

On Wed, Aug 08, 2018 at 11:58:47AM +0200, Steffan Karger wrote:
> Like 'proto', a mismatch in key-method, keydir or tls-auth would fail
> before we ever get to the point where we can print this warning.
> 
> This prepares for removing these from the occ string later on, but also
> prepares for tls-crypt-v2, which allows a server to support tls-auth and
> tls-crypt-v2 connections in parallel.  Such a server will send 'keydir'
> and 'tls-auth' in the occ string.  This change removes the spurious
> warnings about that in the client log.
> 
> Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>

ACK on the feature, NAK on the implementation...

> @@ -3790,11 +3790,14 @@ options_warning_safe_scan2(const int msglevel,
>                             const char *b1_name,
>                             const char *b2_name)
>  {
> -    /* we will stop sending 'proto xxx' in OCC in a future version
> -     * (because it's not useful), and to reduce questions when
> -     * interoperating, we start not-printing a warning about it today
> +    /* We will stop sending 'key-method', 'keydir', 'proto' and 'tls-auth' in
> +     * OCC in a future version (because it's not useful).  To reduce questions
> +     * when interoperating, we no longer printing a warning about it.
>       */
> -    if (strncmp(p1, "proto ", 6) == 0)
> +    if (strcmp(p1, "key-method ") == 0
> +        || strcmp(p1, "keydir ") == 0
> +        || strcmp(p1, "proto ") == 0
> +        || strcmp(p1, "tls-auth ") == 0)
>      {

... because this will actually not match if p1 is "proto TCPv4_SERVER" or
"key-method 2" (string in p1 being longer than "proto ", it will only
match with strncmp() ).

I wasn't sure why the original code had a strncmp() here, so I instrumented
a build to print "p1"...

Sun Oct  7 14:57:21 2018 OCC: 'proto TCPv4_SERVER'
Sun Oct  7 14:57:21 2018 OCC: 'key-method 2'
Sun Oct  7 14:57:21 2018 OCC: 'cipher BF-CBC'
Sun Oct  7 14:57:21 2018 OCC: 'auth SHA1'
Sun Oct  7 14:57:21 2018 OCC: 'keysize 128'

... and no match found...

While at it... could you ignore tun-ipv6 in v2, please? :-)

Sun Oct  7 14:57:21 2018 WARNING: 'tun-ipv6' is present in remote config but missing in local config, remote='tun-ipv6'

thanks,

gert

Patch

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 61fa983..60f4b6f 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3790,11 +3790,14 @@  options_warning_safe_scan2(const int msglevel,
                            const char *b1_name,
                            const char *b2_name)
 {
-    /* we will stop sending 'proto xxx' in OCC in a future version
-     * (because it's not useful), and to reduce questions when
-     * interoperating, we start not-printing a warning about it today
+    /* We will stop sending 'key-method', 'keydir', 'proto' and 'tls-auth' in
+     * OCC in a future version (because it's not useful).  To reduce questions
+     * when interoperating, we no longer printing a warning about it.
      */
-    if (strncmp(p1, "proto ", 6) == 0)
+    if (strcmp(p1, "key-method ") == 0
+        || strcmp(p1, "keydir ") == 0
+        || strcmp(p1, "proto ") == 0
+        || strcmp(p1, "tls-auth ") == 0)
     {
         return;
     }