Skip to contentSkip to conversation

Don't print OCC warnings about 'key-method', 'keydir' and 'tls-auth'

PATCH In review Steffan Karger 3 participants 4 messages Aug 8, 2018 – Oct 7, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Don't print OCC warnings about 'key-method', 'keydir' and 'tls-auth'

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.

---
 src/openvpn/options.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
src/openvpn/options.c +7 -4
index 61fa983..60f4b6f 100644
@@ -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;
     }
Signature · 1 line
2.7.4
#1 lore raw

Hi,

Quoted 10 lines · Steffan Karger

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.

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

Quoted 7 lines
src/openvpn/options.c +0 -3
@@ -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

src/openvpn/options.c options_warning_safe_scan2(const int msglevel, · line 3801

Quoted 10 lines
src/openvpn/options.c +7 -1
+    /* 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

Signature · 6 lines
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany                             [email protected]...
#2 lore raw

PATCH v2 1/1 Don't print OCC warnings about 'key-method', 'keydir' and 'tls-auth'

From: Steffan Karger <[email protected]...>

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.

---
v2: use strprefix instead of strcmp, and add tun-ipv6.

 src/openvpn/options.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
src/openvpn/options.c +8 -4
index 2199af53..45c5ea64 100644
@@ -3788,11 +3788,15 @@ 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 (strprefix(p1, "key-method ")
+        || strprefix(p1, "keydir ")
+        || strprefix(p1, "proto ")
+        || strprefix(p1, "tls-auth ")
+        || strprefix(p1, "tun-ipv6 "))
     {
         return;
     }
Signature · 1 line
2.17.1
Acked-by on v2 1/1
#3 lore raw

I have not actually bothered to find a constellation that *would* trigger an OCC warning (how?) but it compiles and passes tests, and the code looks reasonable. With added msg() calls I verified that it actually ignores the options this time.

I have taken the liberty to remove the space after "tun-ipv6" to make it actually match (no arguments, so no space) and learned something new about the treasures in our sources (strprefix())... :-)

Your patch has been applied to the master and release/2.4 branch (long-term compatibility and not very intrusive).

commit 3baae9ba52187166b7d0b05901732666477a2acb (master) commit 266178b7b6280d5f32403ba8780ddbb517e3ac6a (release/2.4)

Author: Steffan Karger
Date:   Sun Oct 7 19:52:15 2018 +0200
     Don't print OCC warnings about 'key-method', 'keydir' and 'tls-auth'

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg17618.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering