diff mbox series

[Openvpn-devel] Warn when pkcs11-id or pkcs11-id-management options are ignored

Message ID 20230120021841.2048791-1-selva.nair@gmail.com
State Accepted
Headers show
Series [Openvpn-devel] Warn when pkcs11-id or pkcs11-id-management options are ignored | expand

Commit Message

Selva Nair Jan. 20, 2023, 2:18 a.m. UTC 
From: Selva Nair <selva.nair@gmail.com>

- If there are no pkcs11-providers either directly specified or
  through p11-kit-proxy made available through a build-time detection,
  these options are ignored. Log a warning in such cases.

  Especially important on Windows where automatic loading of p11-kit
  is not enabled in our release builds.

- Document this behaviour.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 doc/man-sections/pkcs11-options.rst | 23 ++++++++++++++---------
 src/openvpn/options.c               |  9 +++++++++
 2 files changed, 23 insertions(+), 9 deletions(-)

Comments

Frank Lichtenheld Jan. 20, 2023, 11:33 a.m. UTC | #1 
On Thu, Jan 19, 2023 at 09:18:41PM -0500, selva.nair@gmail.com wrote:
> From: Selva Nair <selva.nair@gmail.com>
> 
> - If there are no pkcs11-providers either directly specified or
>   through p11-kit-proxy made available through a build-time detection,
>   these options are ignored. Log a warning in such cases.
> 
>   Especially important on Windows where automatic loading of p11-kit
>   is not enabled in our release builds.
> 
> - Document this behaviour.
> 

Acked-By: Frank Lichtenheld <frank@lichtenheld.com>

LGTM.

I was looking at the warning messages whether they are consistent with
other similar messages. But since there is no consistency at all, that
is impossible.

Regards,
Gert Doering Jan. 20, 2023, 5:28 p.m. UTC | #2 
As I do not have a working pkcs11-anything environment (still :( ), I have
not tested this beyond "looks reasonable" and "compiles on github".

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

commit abad04fc8ef6c1da7dc4e976bacee9f34931adea (master)
commit f42b38a68e7a9b44153021a7c3864618e3864cae (release/2.6)
Author: Selva Nair
Date:   Thu Jan 19 21:18:41 2023 -0500

     Warn when pkcs11-id or pkcs11-id-management options are ignored

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Message-Id: <20230120021841.2048791-1-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26056.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/doc/man-sections/pkcs11-options.rst b/doc/man-sections/pkcs11-options.rst
index c064aca5..de1662b7 100644
--- a/doc/man-sections/pkcs11-options.rst
+++ b/doc/man-sections/pkcs11-options.rst
@@ -13,7 +13,8 @@  PKCS#11 / SmartCard options
 
 --pkcs11-id name
   Specify the serialized certificate id to be used. The id can be gotten
-  by the standalone ``--show-pkcs11-ids`` option.
+  by the standalone ``--show-pkcs11-ids`` option. See also the description
+  of ``--pkcs11-providers`` option.
 
 --pkcs11-id-management
   Acquire PKCS#11 id from management interface. In this case a
@@ -21,6 +22,7 @@  PKCS#11 / SmartCard options
   application may use pkcs11-id-count command to retrieve available number of
   certificates, and pkcs11-id-get command to retrieve certificate id and
   certificate body.
+  See also the description of ``--pkcs11-providers`` option.
 
 --pkcs11-pin-cache seconds
   Specify how many seconds the PIN can be cached, the default is until the
@@ -51,15 +53,18 @@  PKCS#11 / SmartCard options
      pkcs11-protected-authentication 0
      pkcs11-protected-authentication 1
 
---pkcs11-providers provider
+--pkcs11-providers providers
   Specify an RSA Security Inc. PKCS #11 Cryptographic Token Interface
-  (Cryptoki) providers to load. This option can be used instead of
-  ``--cert``, ``--key`` and ``--pkcs12``.
-
-  If p11-kit is present on the system, its :code:`p11-kit-proxy.so` module
-  will be loaded by default if either the ``--pkcs11-id`` or
-  ``--pkcs11-id-management`` options are specified without
-  ``--pkcs11-provider`` being given.
+  (Cryptoki) providers to load. A space-separated list of one or more
+  provider library names may be specified. This option along with ``--pkcs11-id``
+  or ``pkcs11-id-management`` can be used instead of
+  ``--cert`` and ``--key`` or ``--pkcs12``.
+
+  If p11-kit is present on the system and was enabled during build, its
+  :code:`p11-kit-proxy.so` module will be loaded by default if either
+  the ``--pkcs11-id`` or ``--pkcs11-id-management`` options is present without
+  ``--pkcs11-providers``. If default loading is not enabled in the build and
+  no providers are specified, the former options will be ignored.
 
 --show-pkcs11-ids args
   (Standalone) Show PKCS#11 token object list.
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 4932a869..f24af3d7 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2855,6 +2855,15 @@  options_postprocess_verify_ce(const struct options *options,
     {
         check_ca_required(options);
 #ifdef ENABLE_PKCS11
+        if (!options->pkcs11_providers[0] && options->pkcs11_id)
+        {
+            msg(M_WARN, "Option pkcs11-id is ignored as no pkcs11-providers are specified");
+        }
+        else if (!options->pkcs11_providers[0] && options->pkcs11_id_management)
+        {
+            msg(M_WARN, "Option pkcs11-id-management is ignored as no pkcs11-providers are specified");
+        }
+
         if (options->pkcs11_providers[0])
         {
             if (options->pkcs11_id_management && options->pkcs11_id != NULL)