diff mbox series

[Openvpn-devel,1/2] options: Fix failing inline tls-auth/crypt with persist-key

Message ID 20200508114243.15532-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:42 a.m. UTC 
  A configuration file using --persist-key and with inlined --tls-auth or
--tls-crypt files was failing in check_file_access().  The file argument
to check_file_access() contained the key file and not the file name.

This was because check_file_access_inline() which calls
check_file_access() if the file is not inlined was told the file was not
an inline file.

The reason the check_file_access_inline() was misled was due to a prior
option_postprocess_mutate() call puts these key files into a connection
block entry in option_postprocess_mutate_ce().  OpenVPN was modified a
long while ago to always use connection blocks in the option structure
for simplicity.  So the "root" key files would be transferred into a
connection entry in this method.

When --persist-key is used, option_postprocess_mutate_ce() will load the
key file and "convert" the option into an inline option.  But in
commit cb2e9218f2bc73fa2 this logic had lost the "inline indicator".  The
result was that the connection entry had the key file content stored in
the object but was "tagged" as a normal file (name) not an inline file.

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

Comments

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

On 08/05/2020 13:42, David Sommerseth wrote:
> A configuration file using --persist-key and with inlined --tls-auth or
> --tls-crypt files was failing in check_file_access().  The file argument
> to check_file_access() contained the key file and not the file name.
> 
> This was because check_file_access_inline() which calls
> check_file_access() if the file is not inlined was told the file was not
> an inline file.
> 
> The reason the check_file_access_inline() was misled was due to a prior
> option_postprocess_mutate() call puts these key files into a connection
> block entry in option_postprocess_mutate_ce().  OpenVPN was modified a
> long while ago to always use connection blocks in the option structure
> for simplicity.  So the "root" key files would be transferred into a
> connection entry in this method.
> 
> When --persist-key is used, option_postprocess_mutate_ce() will load the
> key file and "convert" the option into an inline option.  But in
> commit cb2e9218f2bc73fa2 this logic had lost the "inline indicator".  The
> result was that the connection entry had the key file content stored in
> the object but was "tagged" as a normal file (name) not an inline file.
> 
> Signed-off-by: David Sommerseth <davids@openvpn.net>

Thanks for fixing my bugs :-)
Unfortunately the code has changed a bit since the issue of v1 until
v11...so these nasty errors sneaked in.


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

Have not done any testing, but the code change looks very reasonable
and exactly like it would fix the bug :-)

commit e8e410fdc6fb91451d8119b129bac7be00ff81df
Author: David Sommerseth
Date:   Fri May 8 13:42:43 2020 +0200

     options: Fix failing inline tls-auth/crypt with persist-key

     Signed-off-by: David Sommerseth <davids@openvpn.net>
     Acked-by: Antonio Quartulli <antonio@openvpn.net>
     Message-Id: <20200508114243.15532-1-davids@openvpn.net>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19858.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 611652fd..a37106ce 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2936,6 +2936,7 @@  options_postprocess_mutate_ce(struct options *o, struct connection_entry *ce)
             }
 
             ce->tls_auth_file = (char *)in.data;
+            ce->tls_auth_file_inline = true;
         }
 
         if (ce->tls_crypt_file && !ce->tls_crypt_file_inline)
@@ -2948,6 +2949,7 @@  options_postprocess_mutate_ce(struct options *o, struct connection_entry *ce)
             }
 
             ce->tls_crypt_file = (char *)in.data;
+            ce->tls_crypt_file_inline = true;
         }
     }
 }