diff mbox series

[Openvpn-devel] Fix loading PKCS12 files on Windows

Message ID 20211006090709.200-1-lstipakov@gmail.com
State Accepted
Headers show
Series [Openvpn-devel] Fix loading PKCS12 files on Windows | expand

Commit Message

Lev Stipakov Oct. 5, 2021, 10:07 p.m. UTC 
From: Lev Stipakov <lev@openvpn.net>

Starting from 2.5.4 we have switched to MSVC builds,
including dependencies such as OpenSSL.

When we link with natively-built OpenSSL .DLLs
(not cross compiled with MinGW), we are expected to include
applink.c, which provides glue between OpenSSL BIO layer
and compiler run-time. This doesn't apply to ARM64.

Failure to do that results in "no OPENSSL_Applink" fatal error
when calling, for example, d2i_PKCS12_fp(), which we do when
loading PKCS12 files.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 src/openvpn/ssl_openssl.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Gert Doering Oct. 5, 2021, 10:53 p.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

As discussed on IRC, and checked with the OpenSSL documentation and
"Internet knowledge".  Most important: confirmed that it breaks with
--pkcs12 without that patch, and works with it (IRC, I did not test
myself).

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

commit af5de933a0633436e0fe00c42464f4a7ab5ac509 (master)
commit f397c3bfe728e0163fe6d226f62874e1dd25ec60 (release/2.5)
Author: Lev Stipakov
Date:   Wed Oct 6 12:07:09 2021 +0300

     Fix loading PKCS12 files on Windows

     Signed-off-by: Lev Stipakov <lev@openvpn.net>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20211006090709.200-1-lstipakov@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22920.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Lev Stipakov Oct. 6, 2021, 1:40 a.m. UTC | #2 
Adding openvpn-devel.

> Does that mean that CFG and Spectre protection are already included?

Those are merged into the "master" branch, but not into "released". We
could probably include those into the next 2.5 release? Otherwise
they'll be part of 2.6.

-Lev
Gert Doering Oct. 6, 2021, 6:27 a.m. UTC | #3 
Hi,

On Wed, Oct 06, 2021 at 03:40:55PM +0300, Lev Stipakov wrote:
> > Does that mean that CFG and Spectre protection are already included?
> 
> Those are merged into the "master" branch, but not into "released". We
> could probably include those into the next 2.5 release? Otherwise
> they'll be part of 2.6.

"You tell me".

It was not relevant before as we did not build 2.5 with MSVC (except
for ARM).

gert
Lev Stipakov Oct. 6, 2021, 11:26 p.m. UTC | #4 
Hi,

> > Those are merged into the "master" branch, but not into "released". We
> > could probably include those into the next 2.5 release? Otherwise
> > they'll be part of 2.6.
>
> "You tell me".

The probable performance impact should be a less issue for master,
which in most cases will use dco on Windows.

For 2.5, let's conduct performance tests first. Balancing security
with performance tradeoffs is a difficult task.
diff mbox series

Patch

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 31d94f2b..27fb66aa 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -65,6 +65,10 @@ 
 #include <openssl/ec.h>
 #endif
 
+#if defined(_MSC_VER) && !defined(_M_ARM64)
+#include <openssl/applink.c>
+#endif
+
 /*
  * Allocate space in SSL objects in which to store a struct tls_session
  * pointer back to parent.