Skip to content

zpcprovider for sign/verify (ecdsa/eddsa)#41

Open
holger-dengler wants to merge 31 commits into
opencryptoki:mainfrom
holger-dengler:provider-sign
Open

zpcprovider for sign/verify (ecdsa/eddsa)#41
holger-dengler wants to merge 31 commits into
opencryptoki:mainfrom
holger-dengler:provider-sign

Conversation

@holger-dengler
Copy link
Copy Markdown
Contributor

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
Copy link
Copy Markdown
Contributor

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
Copy link
Copy Markdown
Contributor Author

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.

Comment thread src/uri.c Outdated
Comment thread src/provider.c Outdated
Comment thread src/object.h Outdated
Comment thread src/signature.c Outdated
Comment thread src/signature.c
@holger-dengler holger-dengler force-pushed the provider-sign branch 2 times, most recently from 7c72889 to 1659e06 Compare May 2, 2026 15:53
@holger-dengler
Copy link
Copy Markdown
Contributor Author

holger-dengler commented May 2, 2026

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
Copy link
Copy Markdown
Contributor Author

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

@ifranzki
Copy link
Copy Markdown
Contributor

ifranzki commented May 4, 2026

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.

Comment thread CMakeLists.txt Outdated
@holger-dengler
Copy link
Copy Markdown
Contributor Author

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.

Comment thread src/provider.c Outdated
Comment thread src/provider.c Outdated
Comment thread src/provider.c Outdated
Comment thread src/provider.c Outdated
Comment thread src/keymgmt.c
Comment thread src/keymgmt.c
Comment thread src/keymgmt.c
@ifranzki
Copy link
Copy Markdown
Contributor

ifranzki commented May 5, 2026

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
Copy link
Copy Markdown
Contributor Author

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.

Comment thread openssl.cnf.in
The mapping helpers provide mappings between e.g. algorithm strings
and algorithm-related values.

Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Introduce a store-loader for hbkzpc-URI based keys. The store-loader
creates a provider-specific key object and adds relevant information
from the URI.

Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Introduce a asymmetric key management to map the provider-specific key
object to a intern zpc-key.

Not supported:
- key generation
- key import

Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Add helpers to generate DER-encoded algorithm-ids based on key and
digest information.

Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Add signature algorithms for sign/verify with ECDSA and EDDSA keys.

Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Add the supported TLS properties for the zpcprovider.

Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
The ASN.1 module provides DER en-/decoding for hbkzpc-URIs. These
functions are required for the decoder/encoder support.

Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Add internal object build target for ASN.1 module. The internal object
can be shared between targets.

Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Add decoders for PEM and DER to support hbkzpc-URI files.

Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
To use the zpc functionality via the OpenSSL API, the zpcprovider has
to be defined in the OpenSSL configuration.

The build configures the template and creates a `openssl.cnf` file,
which can be used for test purposes. The configuration file will be
created in the build output folder.

The build also configures a second template and creates a
configuration drop-in file `zpcprovider.cnf`. This file can be
included in existing OpenSSL configuration files.

Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
The scripts set breakpoints for to all zpcprovider functions, which
are called by the OpenSSL provider API (dispatch functions). Each
zpcprovider component has its own gdb-script. Sourcing multiple
scripts is possible.

Signed-off-by: Holger Dengler <dengler@linux.ibm.com>
@holger-dengler
Copy link
Copy Markdown
Contributor Author

The current PR version contains a lot of fixes. The tests with the hard-coded keys (t_provider) has been removed completely. For testing purposes, I would suggest to checkout PR #43, as it provides test scripts which uses openssl cli to trigger the zpcprovider.

Further improvements:

  • man-page generation from markdown sources
  • reduced OpenSSL dependency (>v3.0.7)
  • new zpcprovider configuration drop-in

@holger-dengler holger-dengler requested a review from ifranzki May 17, 2026 16:23
Comment thread .travis.yml
env: |
CFLAGS="-O3 -Wextra -Wextra -Werror"
CXXFLAGS="-O3 -Wextra -Wextra -Werror"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you are at it:

Travis repost one build configuration warning:

Build config validation 
root: deprecated key sudo (The key `sudo` has no effect anymore.

Comment thread .travis.yml
- cmake -DBUILD_TEST=ON .. 2> >(tee)
- make 2> >(tee)
- cmake -B build -S . -DCMAKE_INCLUDE_PATH=${OPENSSL_DIR} -DCMAKE_LIBRARY_PATH=${OPENSSL_DIR} 2> >(tee)
- cmake --build build --target zpc 2> >(tee)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to also run some tests here? Probably not .....

Comment thread man/zpcprovider.7.md

IBM Z systems offer different types of cryptographic hardware with different
features, including the CP Assist for Cryptographic Functions (CPACF) and
the IBM® Crypto Express features.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about the ® character... should it be used in man pages?

Comment thread man/zpcprovider.cnf.5.md

A provider section in the OpenSSL configuration define generic parameters,
as well as provider-specific parameters. Each provider section can be
references in a providers sections. The zpcprovider requires at least the
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be references -> referenced ?

Comment thread man/zpcprovider.cnf.5.md
zpcprovider, use the absolute or relative path to the installation
location of `zpcprovider.so`.

identity (optional)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you anywhere describing the URI format?

I always wondered why the URI has hbkzpc: but we otherwise always talk about the zpc provider (no hbk here).

In the example OpenSSL conf file you have identity = hbkzpc. One could wonder why?

Also you register the provider operations with a hard coded provider=hbpzpc property string. What happens if one uses a different identity in the config ? Shouldn't the operations registration follow the identity string?

Or maybe just define that it must be identity = hbkzpc.

Comment thread src/provider.c

static void prov_teardown(void *vpctx)
{
OPENSSL_free(vpctx);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should pctx->libctx be freed here? It was newly allocated in prov_ctx_init() via OSSL_LIB_CTX_new_from_dispatch().

Comment thread src/store.c

if (OPENSSL_hexstr2buf_ex(data->p, data->plen, &data->plen,
attr->value, '\0') != 1) {
OPENSSL_free(data->p);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set data->p = NULL here to avoid possinble double free?

Comment thread src/keymgmt.c
} alg_param_map[] = {
{
.alg = SN_X9_62_prime256v1,
.bits = 256, .secbits = 128, .sigsz = 64 + 8,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a later commit you add ASN1_SIG_HDR. Would it make sense to use this here as well, instead of the hard coded + 8

Comment thread src/keymgmt.c
} else {
if (obj->mkvp && (rc = zpc_ec_key_set_mkvp(key, obj->mkvp)))
goto err;
/* TODO: apqns */
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this TODO is intentionally left for future versions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants