Skip to content

Add a PKCS#11 interface for libzpc - #50

Open
ifranzki wants to merge 17 commits into
opencryptoki:mainfrom
ifranzki:pkcs11
Open

ifranzki wants to merge 17 commits into
opencryptoki:mainfrom
ifranzki:pkcs11

Conversation

@ifranzki

Copy link
Copy Markdown
Contributor

Allow to use hardware backed keys via a PKCS#11 interface. Under the covers the zpc provider is used to perform the crypto operations.

Comment thread src/pkcs11/pkcs11.c Outdated
Add the initial skeleton for a PKCS#11 v3.2 shared library (zpcpkcs11).
Implement C_Initialize, C_Finalize, C_GetInfo, C_GetFunctionList,
C_GetInterfaceList, and C_GetInterface. All other functions return
CKR_FUNCTION_NOT_SUPPORTED.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Report a single slot (ID 0) and a read-only token named "ZPC" as
present and initialized.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Create a dedicated OpenSSL library context and load the default, base,
and zpc provider into it. The context is used for all subsequent key
loading and cryptographic operations.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Add config.c which parses the PKCS#11 configuration file (default:
/etc/zpcpkcs11/zpcpkcs11.conf, overridden via ZPCPKCS11_CONFIG).
Each non-comment line is expected in the form '<label>=<uri>' and
is passed to a caller-supplied callback for further processing.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Use the OpenSSL STORE API to load each key URI from the config file
into an EVP_PKEY. Extract the EC/EdDSA curve parameters, public key
point, and SPKI encoding so they can be stored as PKCS#11 objects.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Add utils.c with a generic dynamic array (dyn_array) implementation
used throughout the PKCS#11 layer to store lists of objects and
sessions without a fixed upper bound.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Add object.c with helpers to allocate, populate, and free PKCS#11
object structures. Provide functions to build default CKA_* attribute
sets for EC and EdDSA public and private key objects.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Add a module-level object list backed by a dyn_array. Provide
object_list_init, object_list_term, object_list_add, object_list_get,
and object_list_find for managing PKCS#11 objects at runtime.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Wire up openssl_load_keys to call config_process and pass each loaded
EVP_PKEY to object_add_ec_ed_private_key or object_add_ec_ed_public_key
so that keys become queryable PKCS#11 objects after C_Initialize.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Add session.c with a module-level session list. Provide helpers to
initialize and terminate the list, open and close sessions, query
session counts, and manage per-session operation state.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Implement C_OpenSession, C_CloseSession, C_CloseAllSessions,
C_GetSessionInfo, C_Login, C_LoginUser, and C_Logout using the
session list. Report live session counts in C_GetTokenInfo.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Implement C_FindObjectsInit, C_FindObjects, and C_FindObjectsFinal.
The init function searches the object list for entries matching the
supplied attribute template and stores the results in the session.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Implement C_GetObjectSize and C_GetAttributeValue to allow callers
to query the size and individual attributes of a PKCS#11 object.
Mark C_CreateObject, C_CopyObject, and C_DestroyObject as
CKR_TOKEN_WRITE_PROTECTED since the token is read-only.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Implement C_GetMechanismList and C_GetMechanismInfo. Advertise
ECDSA with SHA-1/224/256/384/512/SHA3 variants for P-256 to P-521,
and EdDSA for Ed25519/Ed448.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Add signature.c and implement C_SignInit, C_Sign, C_SignUpdate,
C_SignFinal, C_VerifyInit, C_Verify, C_VerifyUpdate, and
C_VerifyFinal. Support ECDSA (with raw hash or prehash variants)
and EdDSA using the OpenSSL EVP signing API.

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Add zpcpkcs11(7) describing the library, its token/session/login
model, and the full function support table. Add zpcpkcs11.conf(5)
describing the key configuration file format.

Assisted-by: IBM Bob:2.0.3
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
Add three bash test scripts using TAP output:
- t_pkcs11_uv_prepconf: generate test keys and a config file
- t_pkcs11_uv_list: list keys via p11tool or pkcs11-tool
- t_pkcs11_uv_sign_verify: sign and verify data for each key type

Assisted-by: IBM Bob:2.0.3
Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
@ifranzki

Copy link
Copy Markdown
Contributor Author

Close + reopen to trigger Travis ....

@ifranzki

Copy link
Copy Markdown
Contributor Author

@holger-dengler friendly reminder :-)

@holger-dengler holger-dengler left a comment

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.

This is the first part of the review. I'll continue tomorrow.

One general note about coding style:
I'm a friend of letting the compiler doing the right thing™. For example, if (!ptr) is working quite well, we not really need to do it explicitly if (ptr == NULL). This project is only targeted for linux with gcc or clang. Both do not have any problems with the short syntax. So lets think about using such short expressions in this module as well. I know, that the pkcs#11 coding style is different, but as long as we're still complient with the shorter syntax, I would prefer it.

Comment thread src/pkcs11/pkcs11.h
Comment on lines +1 to +2
// SPDX-License-Identifier: MIT
// Copyright contributors to the libzpc project

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.

Maybe you can mention in the header comment, from where this file is derived. (I would guess from here: https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.2/csd01/include/pkcs11-v3.2/pkcs11.h)

Comment thread src/pkcs11/pkcs11.c Outdated
Comment on lines +101 to +103
(void)tokenPresent;
(void)pSlotList;
(void)pulCount;

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.

The provider uses attribute for unused parameters. I've no strong meaning about doing it this or that way. But over time, it would make sense to hamonize it in the project.

The core zpc library uses the following macro:

#define UNUSED(x)   (void)(x)

Maybe you can do so as well. It increases teh readability. Maybe I will switch to the same for the provider as well.

Comment thread CMakeLists.txt
)

###########################################################
# zpckcs11

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.

typo: zpcpkcs11

Comment thread CMakeLists.txt
src/pkcs11/pkcs11.c
)

add_library(zpcpkcs11 SHARED ${ZPCPKCS11_SOURCES})

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 my opinion, we have the same situation here as for the provider: we're not generating a classical shared library with the full versioning scheme and soname. The .so, that is generated here is more of a plug-in. If this is correct, I would request to change the target type here from SHARED to MODULE.

Comment thread src/pkcs11/pkcs11.c
interf = &interfaces[i];

if ((pInterfaceName == NULL ||
(pInterfaceName != NULL &&

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.

Isn't this check implicit? If the previous statement is true (pInterfaceName == NULL), we're not getting to the the current line.

You did it already correctly for the pVersion, where you do not check for != NULL after the first check.

Comment thread src/pkcs11/pkcs11.h
Comment on lines +520 to +522
typedef struct CK_IBM_FUNCTION_LIST_1_0 CK_IBM_FUNCTION_LIST_1_0;
typedef struct CK_IBM_FUNCTION_LIST_1_0 CK_PTR CK_IBM_FUNCTION_LIST_1_0_PTR;
typedef CK_IBM_FUNCTION_LIST_1_0_PTR CK_PTR CK_IBM_FUNCTION_LIST_1_0_PTR_PTR;

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.

Do we plan to implement vendor-specific things? If not, can we remove it for the moment?

Comment thread CMakeLists.txt
set(ZPCPKCS11_SOURCES
src/pkcs11/pkcs11.c
src/pkcs11/openssl.c
src/pkcs11/config.c

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.

config.c is part of the next commit. So maybe you can remove it here, so that the history stays bisectable.

Comment thread CMakeLists.txt
Comment on lines +408 to +409
target_include_directories(zpcpkcs11 PRIVATE src/pkcs11 ${OPENSSL_INCLUDE_DIR})
target_link_libraries(zpcpkcs11 PRIVATE OpenSSL::Crypto)

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.

Maybe we can use the same coding style as for the provider? having each include-dir and each lib in a separate line. Or is this too noisy?

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