diff mbox series

[Openvpn-devel,v3,3/3] special handling for PKCS11 providers on win32

Message ID 20221211200108.1402-1-marc.becker@astos.de
State Accepted
Headers
Series None |

Commit Message

Marc Becker Dec. 11, 2022, 8:01 p.m. UTC 
  Change win32 dynamic loader behavior when supplying an absolute path.
The DLL location is considered/preferred to resolve dependencies.
Support in pkcs11-helper for loader flag is detected at compile time.

3rd party DLLs and additional dependencies do no longer need to be moved
to the OpenVPN directory or require changes to %PATH% configuration.

Signed-off-by: Marc Becker <marc.becker@astos.de>
---
 src/openvpn/pkcs11.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Selva Nair Dec. 14, 2022, 3:48 p.m. UTC | #1 
Hi,

On Sun, Dec 11, 2022 at 3:01 PM Marc Becker via Openvpn-devel <
openvpn-devel@lists.sourceforge.net> wrote:

> Change win32 dynamic loader behavior when supplying an absolute path.
> The DLL location is considered/preferred to resolve dependencies.
> Support in pkcs11-helper for loader flag is detected at compile time.
>
> 3rd party DLLs and additional dependencies do no longer need to be moved
> to the OpenVPN directory or require changes to %PATH% configuration.
>
> Signed-off-by: Marc Becker <marc.becker@astos.de>
> ---
>  src/openvpn/pkcs11.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c
> index b74ac8f4..aa027337 100644
> --- a/src/openvpn/pkcs11.c
> +++ b/src/openvpn/pkcs11.c
> @@ -420,6 +420,13 @@ pkcs11_addProvider(
>          {
>              rv = pkcs11h_setProviderProperty(provider,
> PKCS11H_PROVIDER_PROPERTY_CERT_IS_PRIVATE, &cert_is_private,
> sizeof(cert_is_private));
>          }
> +#if defined(WIN32) && defined(PKCS11H_PROVIDER_PROPERTY_LOADER_FLAGS)
> +        if (rv == CKR_OK && platform_absolute_pathname(provider))
> +        {
> +            unsigned loader_flags = LOAD_LIBRARY_SEARCH_DEFAULT_DIRS |
> LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR;
> +            rv = pkcs11h_setProviderProperty(provider,
> PKCS11H_PROVIDER_PROPERTY_LOADER_FLAGS, &loader_flags,
> sizeof(loader_flags));
> +        }
> +#endif
>
>          if (rv != CKR_OK || (rv = pkcs11h_initializeProvider(provider))
> != CKR_OK)
>          {
>

This requires a proposed change to pkcs11-helper which I believe will be
merged upstream -- Alon has closed the PR with a positive remark which
usually means change accepted, but not sure when it will land in the repo.
So, I have only stared at the code and it looks good to me: doesn't break
current builds and should add provider path to dll search path on Windows
when upstream gets updated (or if/when we locally patch pkcs11-helper)

Acked-by: Selva Nair <selva.nair@gmail.com>
Gert Doering Dec. 15, 2022, 8:21 a.m. UTC | #2 
I have not tested this at all, as I do not have the necessary infrastructure.

It does not affect non-WIN32 builds, so what I'll do is to feed this +
the vcpkg patch to my github instance, so it can be tested for windows,
and only then push to the "real" repos.

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

commit e299b8d0d62a4763b20bf9a3bd6aadf414aa89fe (master)
commit 38993a110001b6134a803797a6b4b472ef3546db (release/2.6)
Author: Marc Becker
Date:   Sun Dec 11 21:01:08 2022 +0100

     special handling for PKCS11 providers on win32

     Signed-off-by: Marc Becker <marc.becker@astos.de>
     Acked-by: Selva Nair <selva.nair@gmail.com>
     Message-Id: <20221211200108.1402-1-marc.becker@astos.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25646.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c
index b74ac8f4..aa027337 100644
--- a/src/openvpn/pkcs11.c
+++ b/src/openvpn/pkcs11.c
@@ -420,6 +420,13 @@  pkcs11_addProvider(
         {
             rv = pkcs11h_setProviderProperty(provider, PKCS11H_PROVIDER_PROPERTY_CERT_IS_PRIVATE, &cert_is_private, sizeof(cert_is_private));
         }
+#if defined(WIN32) && defined(PKCS11H_PROVIDER_PROPERTY_LOADER_FLAGS)
+        if (rv == CKR_OK && platform_absolute_pathname(provider))
+        {
+            unsigned loader_flags = LOAD_LIBRARY_SEARCH_DEFAULT_DIRS | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR;
+            rv = pkcs11h_setProviderProperty(provider, PKCS11H_PROVIDER_PROPERTY_LOADER_FLAGS, &loader_flags, sizeof(loader_flags));
+        }
+#endif
 
         if (rv != CKR_OK || (rv = pkcs11h_initializeProvider(provider)) != CKR_OK)
         {