diff mbox series

[Openvpn-devel,2/2] options: Restore --tls-crypt-v2 inline file capability

Message ID 20200508114411.15762-1-davids@openvpn.net
State Accepted
Headers
Series [Openvpn-devel,1/2] options: Fix failing inline tls-auth/crypt with persist-key |

Commit Message

David Sommerseth May 8, 2020, 1:44 a.m. UTC 
  Commit cb2e9218f2bc73f re-factored the internal file handling, but
somehow overlooked the --tls-crypt-v2 option processing.  It was no
longer possible to load a configuration file with this key file inlined.

There where two issues here.  First was that the OPT_P_INLINE flag was
not set, so the option parser rejected --tls-crypt-v2 as inline capable.

Second issue was that the 'streq(p[1], INLINE_FILE_TAG)' check makes no
longer sense, as at this point p[1] contains the file contents.  Instead
use the is_inline flag.

Signed-off-by: David Sommerseth <davids@openvpn.net>
---
 src/openvpn/options.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

Comments

Antonio Quartulli May 8, 2020, 10:59 a.m. UTC | #1 
Hi,

On 08/05/2020 13:44, David Sommerseth wrote:
> Commit cb2e9218f2bc73f re-factored the internal file handling, but
> somehow overlooked the --tls-crypt-v2 option processing.  It was no
> longer possible to load a configuration file with this key file inlined.
> 

maybe tls-crypt-v2 was not yet merged when my patch was conceived?

> There where two issues here.  First was that the OPT_P_INLINE flag was
> not set, so the option parser rejected --tls-crypt-v2 as inline capable.
> 
> Second issue was that the 'streq(p[1], INLINE_FILE_TAG)' check makes no
> longer sense, as at this point p[1] contains the file contents.  Instead
> use the is_inline flag.
> 
> Signed-off-by: David Sommerseth <davids@openvpn.net>

Thanks a lot for fixing this too.

The change makes sense and just converts tls-crypt-v2 to the same
pattern as the other "inlineable" options.

I wonder if we should add unit-tests or t_client.sh test cases with
inlined files too.


Acked-by: Antonio Quartulli <a@unstable.cc>
Gert Doering May 8, 2020, 11:53 a.m. UTC | #2 
Your patch has been applied to the master branch.

Not done any testing either, but this is very similar to the other hunks,
so makes sense.  Patch lingered for too long, new inline-capable options
added :-(

commit 7ae8dbb7c4a2ca4a23efae7b08222a8db0efc529
Author: David Sommerseth
Date:   Fri May 8 13:44:11 2020 +0200

     options: Restore --tls-crypt-v2 inline file capability

     Signed-off-by: David Sommerseth <davids@openvpn.net>
     Acked-by: Antonio Quartulli <antonio@openvpn.net>
     Message-Id: <20200508114411.15762-1-davids@openvpn.net>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19859.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index a37106ce..56c9e411 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8324,22 +8324,16 @@  add_option(struct options *options,
     }
     else if (streq(p[0], "tls-crypt-v2") && p[1] && !p[3])
     {
-        VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION);
+        VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION|OPT_P_INLINE);
         if (permission_mask & OPT_P_GENERAL)
         {
-            if (streq(p[1], INLINE_FILE_TAG) && p[2])
-            {
-                options->tls_crypt_v2_file_inline = p[2];
-            }
             options->tls_crypt_v2_file = p[1];
+            options->tls_crypt_v2_file_inline = is_inline;
         }
         else if (permission_mask & OPT_P_CONNECTION)
         {
-            if (streq(p[1], INLINE_FILE_TAG) && p[2])
-            {
-                options->ce.tls_crypt_v2_file_inline = p[2];
-            }
             options->ce.tls_crypt_v2_file = p[1];
+            options->ce.tls_crypt_v2_file_inline = is_inline;
         }
     }
     else if (streq(p[0], "tls-crypt-v2-verify") && p[1] && !p[2])