zpcprovider for sign/verify (ecdsa/eddsa)

[holger-dengler]

This PR requires PR #40.

This PR adds OpenSSL provider support for libzpc. This first version covers the main functionality for store, keymgmt, sign/verify and decoder support.

As the series add a lot of new code, it is split into many commits to make the review (hopefully) a bit easier.

ToDo:

• public key support for store/decoder
• alloc/free needs some debugging

Restriction:

• the tests uses hard-coded public keys, so running tests on other machines require code changes in tprovider.c.
• the provider only supports uv origins, so testing is limited to SEL environments.

[ifranzki]

Are you looking at the Travis build failures? You might need to build OpenSSL from source to have a 3.5.0 version to build against.

[holger-dengler]

Yes, a fix for the travis is in progress. It looks like travis only provides 3.0 out of the box. So I'll build my own. At least 3.5, better 4.0.

[holger-dengler]

The update contains:

• re-order of the commits (move test to the end of the series)
• rework of signature/eddsa
• pick review comments
• minor fixes

[holger-dengler]

Another update (force push) to remove the merge conflicts and add a fix for travis.

[ifranzki]

I would suggest to also add tests that utilize the openssl application to use protected key origins. For example, creating/signing a certificate with a protected key origin specified via URI or PEM using openssl x509 -in cert.csr -out cert.pem -req -signkey <protected key origin> -days 1001.

[holger-dengler]

I would suggest to also add tests that utilize the openssl application to use protected key origins. For example, creating/signing a certificate with a protected key origin specified via URI or PEM using openssl x509 -in cert.csr -out cert.pem -req -signkey <protected key origin> -days 1001.

I plan to rework the test. tprovider will be renamed (t_provider) and changed to do only the lookup for the provider components. All key-related functional test will be moved to a new t_openssl test script, which uses openssl to create keys and call the functions. such a split is required to run the tests in a CI.

[ifranzki]

I would move the provider sources into a subdirectory, e.g. src/provider/. Maybe even move the library sources into a subdirectory e.g. src/lib/.

[holger-dengler]

I would postpone the source code restructuring until the provider transition is done (symmetric cipher, secure-key origins etc). There will be eventually also some removal of no longer used code.

[holger-dengler]

New PR version:

• rework ecc_key_compare() (locking, re-derive)
• fixes for compile warnings
• provider module init/fini

[ifranzki]

Now that the libzpc is no shared library anymore: What if one builds the internal tests (BUILD_INTERNAL_TEST=ON)? I guess it will then statically link to the libzpc. How about calling zpc_init() and zpc_fini() in this case? I would assume the test executables should then call zpc_init() and zpc_fini() explicitly, now that its no longer a constructor/destructor.

[ifranzki]

I run into strange situations where zpc_init is called twice.... It seems that this is due to missing -fPIC for the library.
So adding set(CMAKE_POSITION_INDEPENDENT_CODE ON) at the beginning of CMakeLists.txt (e.g. line 33) fixes the problem.

The problem happens when I load the provider explicitly via OSSL_PROVIDER_load() into a separate OpenSSL library context (as done by my PKCS#11 interface for zpc). It seems that without PIC the provider is loaded twice (once via OpenSSL config, and another time via explicate load), and then the globals in globals.c exist twice....

Bob explains this as follows:

What Happens Without -fPIC:

1. zpcprovider.so links $<TARGET_OBJECTS:zpc> (which includes globals.c)
2. zpcpkcs11.so loads and then loads zpcprovider.so via OSSL_PROVIDER_load()
3. Without -fPIC, the linker may create copy relocations for global variables
4. This results in two separate copies of the global variables (debuglock, ccalock, ep11lock, etc.) in memory:
   • One copy in zpcprovider.so's address space
   • One copy in the main executable's address space (or zpcpkcs11.so's space)

With PIC global variables are accessed through the Global Offset Table (GOT).

So I think we should build the code with PIC enabled.

[ifranzki]

So I think we should build the code with PIC enabled.

You have it set to URI and ASN1, but not for the rest.

According to the CMAC documentation. CMAKE_POSITION_INDEPENDENT_CODE is ON by default for SHARED and MODULE library targets, but not for anything else.

So this is an effect of making libzpc a static library, and no longer a shared library. Before it was using PIC by default, now we need to set PIC explicitly on anything that is linked into a shared library.

[holger-dengler]

Instead of enabling PIC for all targets, I would prefer to do it only for the OBJECT target explicitly. The MODULE target enables it automatically, and for executables it is not necessary. The targets asn1and uri has it already enabled, so I will do it the same way also for zpc.

[ifranzki]

Now that the libzpc is no shared library anymore: What if one builds the internal tests (BUILD_INTERNAL_TEST=ON)? I guess it will then statically link to the libzpc. How about calling zpc_init() and zpc_fini() in this case? I would assume the test executables should then call zpc_init() and zpc_fini() explicitly, now that its no longer a constructor/destructor.

Fix proposal from Bob:

diff --git a/test/testlib.cc b/test/testlib.cc
index d5bc683..ea19b4c 100644
--- a/test/testlib.cc
+++ b/test/testlib.cc
@@ -7,6 +7,7 @@
 
 #include "testlib.h"
 
+#include "zpc/init.h"
 #include "zpc/aes_key.h"
 #include "zpc/aes_xts_key.h"
 #include "zpc/ecc_key.h"
@@ -1370,3 +1371,25 @@ int testlib_set_aes_xts_key_from_pvsecret(struct zpc_aes_xts_key *aes_key, int k
 
 	return rc;
 }
+
+/*
+ * ZpcEnvironment implementation - Global test environment for zpc library.
+ * This ensures zpc_init() is called once before all tests and zpc_fini()
+ * is called once after all tests complete.
+ */
+void ZpcEnvironment::SetUp()
+{
+	zpc_init();
+}
+
+void ZpcEnvironment::TearDown()
+{
+	zpc_fini();
+}
+
+/*
+ * Register the global test environment.
+ * This is called automatically when the test binary starts.
+ */
+::testing::Environment* const zpc_env =
+	::testing::AddGlobalTestEnvironment(new ZpcEnvironment);
diff --git a/test/testlib.h b/test/testlib.h
index 7ea4e79..e9828b5 100644
--- a/test/testlib.h
+++ b/test/testlib.h
@@ -10,6 +10,18 @@
 
 # include "gtest/gtest.h"
 
+/*
+ * Global test environment for zpc initialization/finalization.
+ * This ensures zpc_init() is called once before all tests and
+ * zpc_fini() is called once after all tests complete.
+ */
+class ZpcEnvironment : public ::testing::Environment {
+public:
+	virtual ~ZpcEnvironment() {}
+	virtual void SetUp();
+	virtual void TearDown();
+};
+
 # ifdef __cplusplus
 /* *INDENT-OFF* */
 extern "C" {
-- 

[ifranzki]

Maybe mention the OpenSSL minimum version in README.md ?

[holger-dengler]

The README is under contruction in preparation for the release.

[holger-dengler]

This PR will be closed, as the merged PR #45 is now available.