Skip to contentSkip to conversation

show the right string for key-direction

PATCH Superseded Arne Schwabe 3 participants 6 messages Jun 28, 2017 – Feb 20, 2018

lore
Share & tools
lore

Arne Schwabe

author
#0 lore raw

PATCH 1/1 show the right string for key-direction

---
 src/openvpn/options.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
src/openvpn/options.c +1 -1
index 61af6e3a..d8049e76 100644
@@ -1697,7 +1697,7 @@ show_settings(const struct options *o)
 
 #ifdef ENABLE_CRYPTO
     SHOW_STR(shared_secret_file);
-    SHOW_INT(key_direction);
+    SHOW_PARM(key_direction, keydirection2ascii(o->key_direction, false), "%s");
     SHOW_STR(ciphername);
     SHOW_BOOL(ncp_enabled);
     SHOW_STR(ncp_ciphers);
Signature · 1 line
2.11.0 (Apple Git-81)
#1 lore raw
Quoted 10 lines · Arne Schwabe
---
 src/openvpn/options.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
src/openvpn/options.c +0 -0
index 61af6e3a..d8049e76 100644

src/openvpn/options.c show_settings(const struct options *o) · line 1703

Quoted 8 lines
src/openvpn/options.c +1 -1
@@ -1697,7 +1697,7 @@ show_settings(const struct options *o)
 
 #ifdef ENABLE_CRYPTO
     SHOW_STR(shared_secret_file);
-    SHOW_INT(key_direction);
+    SHOW_PARM(key_direction, keydirection2ascii(o->key_direction, false), "%s");
     SHOW_STR(ciphername);
     SHOW_BOOL(ncp_enabled);
     SHOW_STR(ncp_ciphers);

Makes sense, but keydirection2acrii() can return NULL, which technically is undefined behaviour as input for a %s format string specifier...

I know it's slightly lame, but still think either keydirection2ascii should always return a valid string, or this needs to wrapped in a np() call.

-Steffan

author
#2 lore raw

PATCH v2 1/1 show the right string for key-direction

V2: print also a nice string if direction is not set

---
 src/openvpn/options.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
src/openvpn/options.c +1 -1
index 66e1f9d1..f405d8a2 100644
@@ -1706,7 +1706,7 @@ show_settings(const struct options *o)
 #endif
 
     SHOW_STR(shared_secret_file);
-    SHOW_INT(key_direction);
+    SHOW_PARM(key_direction, keydirection2ascii(o->key_direction, false), "%s");
     SHOW_STR(ciphername);
     SHOW_BOOL(ncp_enabled);
     SHOW_STR(ncp_ciphers);
Signature · 1 line
2.15.1
author
#3 lore raw

PATCH v3 1/1 show the right string for key-direction

In reply to Steffan Karger

V2: print also a nice string if direction is not set
V3: really include V2 changes

---
 src/openvpn/crypto.c  | 11 +++++++++--
 src/openvpn/crypto.h  |  2 +-
 src/openvpn/options.c |  4 ++--
 3 files changed, 12 insertions(+), 5 deletions(-)
src/openvpn/crypto.c +9 -2
index 3096f3b0..b3021d4f 100644
@@ -1545,11 +1545,18 @@ ascii2keydirection(int msglevel, const char *str)
 }
 
 const char *
-keydirection2ascii(int kd, bool remote)
+keydirection2ascii(int kd, bool remote, bool humanreadable)
 {
     if (kd == KEY_DIRECTION_BIDIRECTIONAL)
     {
-        return NULL;
+        if (humanreadable)
+        {
+            return "not set";
+        }
+        else
+        {
+            return NULL;
+        }
     }
     else if (kd == KEY_DIRECTION_NORMAL)
     {
src/openvpn/crypto.h +1 -1
index 8e8ee8f5..e1e318eb 100644
@@ -470,7 +470,7 @@ void must_have_n_keys(const char *filename, const char *option, const struct key
 
 int ascii2keydirection(int msglevel, const char *str);
 
-const char *keydirection2ascii(int kd, bool remote);
+const char *keydirection2ascii(int kd, bool remote, bool humanreadable);
 
 /* print keys */
 void key2_print(const struct key2 *k,
src/openvpn/options.c +2 -2
index 66e1f9d1..9f5cd6ed 100644
@@ -1706,7 +1706,7 @@ show_settings(const struct options *o)
 #endif
 
     SHOW_STR(shared_secret_file);
-    SHOW_INT(key_direction);
+    SHOW_PARM(key_direction, keydirection2ascii(o->key_direction, false, true), "%s");
     SHOW_STR(ciphername);
     SHOW_BOOL(ncp_enabled);
     SHOW_STR(ncp_ciphers);

@@ -3583,7 +3583,7 @@ options_string(const struct options *o,
      * Key direction
      */
     {
-        const char *kd = keydirection2ascii(o->key_direction, remote);
+        const char *kd = keydirection2ascii(o->key_direction, remote, false);
         if (kd)
         {
             buf_printf(&out, ",keydir %s", kd);
Signature · 1 line
2.15.1
Acked-by Tested-by on v3 1/1
#4 lore raw

Hi,

Quoted 61 lines · Arne Schwabe

V2: print also a nice string if direction is not set
V3: really include V2 changes

---
 src/openvpn/crypto.c  | 11 +++++++++--
 src/openvpn/crypto.h  |  2 +-
 src/openvpn/options.c |  4 ++--
 3 files changed, 12 insertions(+), 5 deletions(-)
src/openvpn/crypto.c +9 -2
index 3096f3b0..b3021d4f 100644
@@ -1545,11 +1545,18 @@ ascii2keydirection(int msglevel, const char *str)
 }
 
 const char *
-keydirection2ascii(int kd, bool remote)
+keydirection2ascii(int kd, bool remote, bool humanreadable)
 {
     if (kd == KEY_DIRECTION_BIDIRECTIONAL)
     {
-        return NULL;
+        if (humanreadable)
+        {
+            return "not set";
+        }
+        else
+        {
+            return NULL;
+        }
     }
     else if (kd == KEY_DIRECTION_NORMAL)
     {
src/openvpn/crypto.h +1 -1
index 8e8ee8f5..e1e318eb 100644
@@ -470,7 +470,7 @@ void must_have_n_keys(const char *filename, const char *option, const struct key
 
 int ascii2keydirection(int msglevel, const char *str);
 
-const char *keydirection2ascii(int kd, bool remote);
+const char *keydirection2ascii(int kd, bool remote, bool humanreadable);
 
 /* print keys */
 void key2_print(const struct key2 *k,
src/openvpn/options.c +1 -1
index 66e1f9d1..9f5cd6ed 100644
@@ -1706,7 +1706,7 @@ show_settings(const struct options *o)
 #endif
 
     SHOW_STR(shared_secret_file);
-    SHOW_INT(key_direction);
+    SHOW_PARM(key_direction, keydirection2ascii(o->key_direction, false, true), "%s");
     SHOW_STR(ciphername);
     SHOW_BOOL(ncp_enabled);
     SHOW_STR(ncp_ciphers);

src/openvpn/options.c options_string(const struct options *o, · line 3589

Quoted 8 lines
src/openvpn/options.c +1 -1
@@ -3583,7 +3583,7 @@ options_string(const struct options *o,
      * Key direction
      */
     {
-        const char *kd = keydirection2ascii(o->key_direction, remote);
+        const char *kd = keydirection2ascii(o->key_direction, remote, false);
         if (kd)
         {
             buf_printf(&out, ",keydir %s", kd);

'The right' here means 'what was in the config' instead of 'what we use internally'. But I agree that's better UX, so:

Acked-by Steffan Karger <[email protected]...>
Tested-by Steffan Karger <[email protected]...>

-Steffan

#5 lore raw
In reply to Arne Schwabe

Your patch has been applied to the master and release/2.4 branch.

commit 7f7f00da88eeea847da57f4f34c66c1f4a935a73 (master) commit 6725074ba0af914f85e6dbbef5eff08c2539c5b8 (release/2.4)

Author: Arne Schwabe
Date:   Wed Jan 31 10:53:00 2018 +0100
     show the right string for key-direction

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

--
kind regards,

Gert Doering