| Message ID | 20221211191403.805-1-marc.becker@astos.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | None | expand |
Hi, On Sun, Dec 11, 2022 at 2:14 PM Marc Becker via Openvpn-devel < openvpn-devel@lists.sourceforge.net> wrote: > The new interface in pkcs11-helper 1.28 allows decoupling of provider > registration and initialization. > This allows modifying more (and future) properties apart from the > 6 fixed ones supported as arguments to pkcs11h_addProvider(). > > With the new interface it is easier to see (from a code perspective) > which option is set to which value. > It's also not necessary to supply values for built-in defaults: > - slot_event_method=PKCS11H_SLOTEVENT_METHOD_AUTO > - slot_poll_interval=0 > > Signed-off-by: Marc Becker <marc.becker@astos.de> > --- > v2: improved code and description, no (essentially) duplicated log output > --- > src/openvpn/pkcs11.c | 33 +++++++++++++++++++++++++++++++++ > 1 file changed, 33 insertions(+) > > diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c > index fbc4c472..b74ac8f4 100644 > --- a/src/openvpn/pkcs11.c > +++ b/src/openvpn/pkcs11.c > @@ -396,6 +396,38 @@ pkcs11_addProvider( > provider > ); > > +#if PKCS11H_VERSION >= ((1<<16) | (28<<8) | (0<<0)) > + if ((rv = pkcs11h_registerProvider(provider)) != CKR_OK) > + { > + msg(M_WARN, "PKCS#11: Cannot register provider '%s' %ld-'%s'", > provider, rv, pkcs11h_getMessage(rv)); > + } > + else > + { > + PKCS11H_BOOL allow_protected_auth = protected_auth; > + PKCS11H_BOOL cert_is_private = cert_private; > + > + rv = pkcs11h_setProviderProperty(provider, > PKCS11H_PROVIDER_PROPERTY_LOCATION, provider, strlen(provider) + 1); > + > + if (rv == CKR_OK) > + { > + rv = pkcs11h_setProviderProperty(provider, > PKCS11H_PROVIDER_PROPERTY_ALLOW_PROTECTED_AUTH, &allow_protected_auth, > sizeof(allow_protected_auth)); > + } > + if (rv == CKR_OK) > + { > + rv = pkcs11h_setProviderProperty(provider, > PKCS11H_PROVIDER_PROPERTY_MASK_PRIVATE_MODE, &private_mode, > sizeof(private_mode)); > + } > + if (rv == CKR_OK) > + { > + rv = pkcs11h_setProviderProperty(provider, > PKCS11H_PROVIDER_PROPERTY_CERT_IS_PRIVATE, &cert_is_private, > sizeof(cert_is_private)); > + } > + > + if (rv != CKR_OK || (rv = pkcs11h_initializeProvider(provider)) > != CKR_OK) > + { > + msg(M_WARN, "PKCS#11: Cannot initialize provider '%s' > %ld-'%s'", provider, rv, pkcs11h_getMessage(rv)); > + pkcs11h_removeProvider(provider); > + } > + } > +#else /* if PKCS11H_VERSION >= ((1<<16) | (28<<8) | (0<<0)) */ > if ( > (rv = pkcs11h_addProvider( > provider, > @@ -410,6 +442,7 @@ pkcs11_addProvider( > { > msg(M_WARN, "PKCS#11: Cannot initialize provider '%s' %ld-'%s'", > provider, rv, pkcs11h_getMessage(rv)); > } > +#endif /* if PKCS11H_VERSION >= ((1<<16) | (28<<8) | (0<<0)) */ > > dmsg( > D_PKCS11_DEBUG, > -- > 2.38.1.windows.1 > I have been delaying acking this until I get time to test 3/3, but as 1/3 is has been acked here goes: Effectively this is the same as the original but splitting addProvider to explicit register/set-properties/intialize calls when using recent versions of pkcs11-helper is a useful refactoring. Acked-by: Selva Nair <selva.nair@gmail.com>
I have test compiled this on Gentoo with pkcs-helper-1.27.0-r1, and
"it compiled". I'm aware that this is not actually excercising any
of the new code paths, but "stare at code" sees nothing explosive,
and I trust Selva's review.
Your patch has been applied to the master and release/2.6 branch.
commit 5954f03bb321a969ea6edbdd353e1dd4f61ac8f0 (master)
commit d0141344b0c21747ba18be64470236066da62f33 (release/2.6)
Author: Marc Becker via Openvpn-devel
Date: Sun Dec 11 20:14:03 2022 +0100
use new pkcs11-helper interface to add providers
Signed-off-by: Marc Becker <marc.becker@astos.de>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20221211191403.805-1-marc.becker@astos.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25643.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
Hi, On Thu, Dec 15, 2022 at 09:04:18AM +0100, Gert Doering wrote: > Your patch has been applied to the master and release/2.6 branch. > > commit 5954f03bb321a969ea6edbdd353e1dd4f61ac8f0 (master) > commit d0141344b0c21747ba18be64470236066da62f33 (release/2.6) Same thing here: fixing DMARC induced nonsense, the new commit IDs are: commit 45d9b0210a22353e587c29c5d3c3990346a4a189 (master) commit 0236518cee65cc3d1da8d57b1d7785ecb2663a23 (release/2.6) Author: Marc Becker <marc.becker@astos.de> Date: Sun Dec 11 20:14:03 2022 +0100 use new pkcs11-helper interface to add providers gert
diff --git a/src/openvpn/pkcs11.c b/src/openvpn/pkcs11.c index fbc4c472..b74ac8f4 100644 --- a/src/openvpn/pkcs11.c +++ b/src/openvpn/pkcs11.c @@ -396,6 +396,38 @@ pkcs11_addProvider( provider ); +#if PKCS11H_VERSION >= ((1<<16) | (28<<8) | (0<<0)) + if ((rv = pkcs11h_registerProvider(provider)) != CKR_OK) + { + msg(M_WARN, "PKCS#11: Cannot register provider '%s' %ld-'%s'", provider, rv, pkcs11h_getMessage(rv)); + } + else + { + PKCS11H_BOOL allow_protected_auth = protected_auth; + PKCS11H_BOOL cert_is_private = cert_private; + + rv = pkcs11h_setProviderProperty(provider, PKCS11H_PROVIDER_PROPERTY_LOCATION, provider, strlen(provider) + 1); + + if (rv == CKR_OK) + { + rv = pkcs11h_setProviderProperty(provider, PKCS11H_PROVIDER_PROPERTY_ALLOW_PROTECTED_AUTH, &allow_protected_auth, sizeof(allow_protected_auth)); + } + if (rv == CKR_OK) + { + rv = pkcs11h_setProviderProperty(provider, PKCS11H_PROVIDER_PROPERTY_MASK_PRIVATE_MODE, &private_mode, sizeof(private_mode)); + } + if (rv == CKR_OK) + { + rv = pkcs11h_setProviderProperty(provider, PKCS11H_PROVIDER_PROPERTY_CERT_IS_PRIVATE, &cert_is_private, sizeof(cert_is_private)); + } + + if (rv != CKR_OK || (rv = pkcs11h_initializeProvider(provider)) != CKR_OK) + { + msg(M_WARN, "PKCS#11: Cannot initialize provider '%s' %ld-'%s'", provider, rv, pkcs11h_getMessage(rv)); + pkcs11h_removeProvider(provider); + } + } +#else /* if PKCS11H_VERSION >= ((1<<16) | (28<<8) | (0<<0)) */ if ( (rv = pkcs11h_addProvider( provider, @@ -410,6 +442,7 @@ pkcs11_addProvider( { msg(M_WARN, "PKCS#11: Cannot initialize provider '%s' %ld-'%s'", provider, rv, pkcs11h_getMessage(rv)); } +#endif /* if PKCS11H_VERSION >= ((1<<16) | (28<<8) | (0<<0)) */ dmsg( D_PKCS11_DEBUG,
The new interface in pkcs11-helper 1.28 allows decoupling of provider registration and initialization. This allows modifying more (and future) properties apart from the 6 fixed ones supported as arguments to pkcs11h_addProvider(). With the new interface it is easier to see (from a code perspective) which option is set to which value. It's also not necessary to supply values for built-in defaults: - slot_event_method=PKCS11H_SLOTEVENT_METHOD_AUTO - slot_poll_interval=0 Signed-off-by: Marc Becker <marc.becker@astos.de> --- v2: improved code and description, no (essentially) duplicated log output --- src/openvpn/pkcs11.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)