[Cryptech-Commits] [sw/pkcs11] 03/05: Bugfixes to new error handling code, refactor some unreadable nested logic in handle lookup code.
git at cryptech.is
git at cryptech.is
Thu May 19 03:17:48 UTC 2016
This is an automated email from the git hooks/post-receive script.
sra at hactrn.net pushed a commit to branch rpc
in repository sw/pkcs11.
commit 00b2adefccab211bb853c79ac84315dbd40ee05d
Author: Rob Austein <sra at hactrn.net>
AuthorDate: Tue May 17 23:07:20 2016 -0400
Bugfixes to new error handling code, refactor some unreadable nested logic in handle lookup code.
The mapping between PKCS #11 objects and libhal handles isn't quite
right yet. This is a snapshot of bugfixes accumulated along the way,
before refactoring mapping code to deal with the underlying problem.
---
GNUmakefile | 13 ++++-----
pkcs11.c | 85 ++++++++++++++++++++++++-----------------------------------
unit_tests.py | 46 +++++++++++++++++++++++++-------
3 files changed, 78 insertions(+), 66 deletions(-)
diff --git a/GNUmakefile b/GNUmakefile
index 4a7df7c..dc41be5 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -140,16 +140,17 @@ ifneq "${HSMBULLY}" ""
bully: all
set -x; \
sudo rm -f ${HSMBULLY_DATABASE} ${HSMBULLY_DATABASE}-journal ${HSMBULLY_KS_CLIENT} ${HSMBULLY_KS_SERVER}; \
- if test -x ${HSMBULLY_SERVER_BIN}; \
+ if test -x '${HSMBULLY_SERVER_BIN}'; \
then \
sudo CRYPTECH_KEYSTORE=${HSMBULLY_KS_SERVER} ${HSMBULLY_SERVER_BIN} & \
pid=$$!; \
- fi; \
- (echo fnord; echo fnord) | sudo ./p11util --set-so-pin --set-user-pin --pin-from-stdin; \
- sudo PKCS11_DATABASE=${HSMBULLY_DATABASE} CRYPTECH_KEYSTORE=${HSMBULLY_KS_CLIENT} ${HSMBULLY} ${HSMBULLY_OPTIONS}; \
- if test -x ${HSMBULLY_SERVER_BIN}; \
- then \
+ sleep 5; \
+ (echo fnord; echo fnord) | CRYPTECH_KEYSTORE=${HSMBULLY_KS_CLIENT} ./p11util --set-so-pin --set-user-pin --pin-from-stdin; \
+ PKCS11_DATABASE=${HSMBULLY_DATABASE} CRYPTECH_KEYSTORE=${HSMBULLY_KS_CLIENT} ${HSMBULLY} ${HSMBULLY_OPTIONS}; \
sudo kill $$pid; \
+ else \
+ (echo fnord; echo fnord) | sudo CRYPTECH_KEYSTORE=${HSMBULLY_KS_CLIENT} ./p11util --set-so-pin --set-user-pin --pin-from-stdin; \
+ sudo PKCS11_DATABASE=${HSMBULLY_DATABASE} CRYPTECH_KEYSTORE=${HSMBULLY_KS_CLIENT} ${HSMBULLY} ${HSMBULLY_OPTIONS}; \
fi; \
sudo rm -f ${HSMBULLY_DATABASE} ${HSMBULLY_DATABASE}-journal ${HSMBULLY_KS_CLIENT} ${HSMBULLY_KS_SERVER}
diff --git a/pkcs11.c b/pkcs11.c
index 3e47c67..4fde44f 100644
--- a/pkcs11.c
+++ b/pkcs11.c
@@ -317,11 +317,11 @@ static inline hal_error_t _hal_whine(const hal_error_t err,
int ok = 0;
hal_error_t code;
- va_start(ap, line)
- do {
- code = va_arg(ap, hal_error_t);
- ok |= (err == code);
- } while (code != HAL_OK);
+ va_start(ap, line);
+ do {
+ code = va_arg(ap, hal_error_t);
+ ok |= (err == code);
+ } while (code != HAL_OK);
va_end(ap);
if (!ok)
@@ -368,6 +368,9 @@ static CK_RV _p11_error_from_hal(const hal_error_t err, const char * const file,
* More here later, first see if this compiles.
*/
+ case HAL_OK:
+ return CKR_OK;
+
default:
#if DEBUG_PKCS11 || DEBUG_HAL
fprintf(stderr, "\n%s:%u: Mapping unhandled HAL error to CKR_FUNCTION_FAILED\n", file, line);
@@ -1630,39 +1633,25 @@ static int p11_object_get_pkey_handle(const p11_session_t * const session,
!sql_check_row(sqlite3_step(q)))
goto fail;
- switch (sqlite3_column_type(q, 0)) {
+ const int column_0_type = sqlite3_column_type(q, 0);
+ const int column_1_type = sqlite3_column_type(q, 1);
- case SQLITE_INTEGER:
+ if (column_0_type == SQLITE_INTEGER)
pkey_type = (hal_key_type_t) sqlite3_column_int64(q, 0);
- break;
-
- case SQLITE_NULL:
- if (!p11_object_pkey_type(object_handle, &pkey_type))
- goto fail;
- break;
- default:
+ else if (column_0_type != SQLITE_NULL || !p11_object_pkey_type(object_handle, &pkey_type))
goto fail;
- }
- switch (sqlite3_column_type(q, 1)) {
-
- case SQLITE_BLOB:
- err = hal_whine_allow(hal_rpc_pkey_find(p11_session_hal_client(session),
- p11_session_hal_session(session), pkey_handle,
- pkey_type, sqlite3_column_blob(q, 1),
- sqlite3_column_bytes(q, 1),
- flags),
+ if (column_1_type == SQLITE_BLOB)
+ err = hal_whine_allow(hal_rpc_pkey_find(p11_session_hal_client(session), p11_session_hal_session(session), pkey_handle,
+ pkey_type, sqlite3_column_blob(q, 1), sqlite3_column_bytes(q, 1), flags),
HAL_ERROR_KEY_NOT_FOUND);
- break;
- case SQLITE_NULL:
- err = HAL_ERROR_KEY_NOT_FOUND;
- break;
+ else if (column_1_type == SQLITE_NULL)
+ err = hal_whine(HAL_ERROR_KEY_NOT_FOUND);
- default:
+ else
goto fail;
- }
if (err == HAL_OK)
ok = 1;
@@ -2285,26 +2274,28 @@ static CK_RV generate_keypair_ec(p11_session_t *session,
static CK_RV generate_keypair(p11_session_t *session,
const CK_MECHANISM_PTR pMechanism,
- const CK_ATTRIBUTE_PTR pPublicKeyTemplate,
- const CK_ULONG ulPublicKeyAttributeCount,
- const CK_ATTRIBUTE_PTR pPrivateKeyTemplate,
- const CK_ULONG ulPrivateKeyAttributeCount,
- CK_OBJECT_HANDLE_PTR phPublicKey,
- CK_OBJECT_HANDLE_PTR phPrivateKey,
CK_RV (*mechanism_handler)(p11_session_t *session,
+
const CK_ATTRIBUTE_PTR pPublicKeyTemplate,
const CK_ULONG ulPublicKeyAttributeCount,
const CK_OBJECT_HANDLE public_handle,
const hal_key_flags_t public_flags,
+
const CK_ATTRIBUTE_PTR pPrivateKeyTemplate,
const CK_ULONG ulPrivateKeyAttributeCount,
const CK_OBJECT_HANDLE private_handle,
const hal_key_flags_t private_flags),
+ const CK_ATTRIBUTE_PTR pPublicKeyTemplate,
+ const CK_ULONG ulPublicKeyAttributeCount,
const p11_descriptor_t * const public_descriptor,
- const p11_descriptor_t * const private_descriptor)
+ CK_OBJECT_HANDLE_PTR phPublicKey,
+ const CK_ATTRIBUTE_PTR pPrivateKeyTemplate,
+ const CK_ULONG ulPrivateKeyAttributeCount,
+ const p11_descriptor_t * const private_descriptor,
+ CK_OBJECT_HANDLE_PTR phPrivateKey)
{
- CK_OBJECT_HANDLE private_handle = CK_INVALID_HANDLE;
CK_OBJECT_HANDLE public_handle = CK_INVALID_HANDLE;
+ CK_OBJECT_HANDLE private_handle = CK_INVALID_HANDLE;
handle_flavor_t public_handle_flavor = handle_flavor_session_object;
handle_flavor_t private_handle_flavor = handle_flavor_session_object;
hal_key_flags_t public_flags = 0;
@@ -4295,23 +4286,15 @@ CK_RV C_GenerateKeyPair(CK_SESSION_HANDLE hSession,
switch (pMechanism->mechanism) {
case CKM_RSA_PKCS_KEY_PAIR_GEN:
- rv = generate_keypair(session, pMechanism,
- pPublicKeyTemplate, ulPublicKeyAttributeCount,
- pPrivateKeyTemplate, ulPrivateKeyAttributeCount,
- phPublicKey, phPrivateKey,
- generate_keypair_rsa_pkcs,
- &p11_descriptor_rsa_public_key,
- &p11_descriptor_rsa_private_key);
+ rv = generate_keypair(session, pMechanism, generate_keypair_rsa_pkcs,
+ pPublicKeyTemplate, ulPublicKeyAttributeCount, &p11_descriptor_rsa_public_key, phPublicKey,
+ pPrivateKeyTemplate, ulPrivateKeyAttributeCount, &p11_descriptor_rsa_private_key, phPrivateKey);
break;
case CKM_EC_KEY_PAIR_GEN:
- rv = generate_keypair(session, pMechanism,
- pPublicKeyTemplate, ulPublicKeyAttributeCount,
- pPrivateKeyTemplate, ulPrivateKeyAttributeCount,
- phPublicKey, phPrivateKey,
- generate_keypair_ec,
- &p11_descriptor_ec_public_key,
- &p11_descriptor_ec_private_key);
+ rv = generate_keypair(session, pMechanism, generate_keypair_ec,
+ pPublicKeyTemplate, ulPublicKeyAttributeCount, &p11_descriptor_ec_public_key, phPublicKey,
+ pPrivateKeyTemplate, ulPrivateKeyAttributeCount, &p11_descriptor_ec_private_key, phPrivateKey);
break;
default:
diff --git a/unit_tests.py b/unit_tests.py
index f5553d4..6866a87 100644
--- a/unit_tests.py
+++ b/unit_tests.py
@@ -272,7 +272,6 @@ class TestKeys(unittest.TestCase):
p11.C_VerifyInit(self.session, CKM_ECDSA_SHA384, public_key)
p11.C_Verify(self.session, hamster, sig)
-
def test_gen_sign_verify_ecdsa_p521_sha512(self):
#if not args.all_tests: self.skipTest("SHA-512 not available in current build")
public_key, private_key = p11.C_GenerateKeyPair(self.session, CKM_EC_KEY_PAIR_GEN,
@@ -286,16 +285,45 @@ class TestKeys(unittest.TestCase):
p11.C_VerifyInit(self.session, CKM_ECDSA_SHA512, public_key)
p11.C_Verify(self.session, hamster, sig)
- def test_gen_rsa_1024(self):
- self.assertIsKeypair(
- p11.C_GenerateKeyPair(self.session, CKM_RSA_PKCS_KEY_PAIR_GEN, CKA_MODULUS_BITS = 1024,
- CKA_ID = "RSA-1024", CKA_SIGN = True, CKA_VERIFY = True))
+ def test_gen_sign_verify_rsa_1024(self):
+ public_key, private_key = p11.C_GenerateKeyPair(
+ self.session, CKM_RSA_PKCS_KEY_PAIR_GEN, CKA_MODULUS_BITS = 1024,
+ CKA_ID = "RSA-1024", CKA_SIGN = True, CKA_VERIFY = True)
+ self.assertIsKeypair(public_key, private_key)
+ hamster = "Your mother was a hamster"
+ p11.C_SignInit(self.session, CKM_SHA512_RSA_PKCS, private_key)
+ sig = p11.C_Sign(self.session, hamster)
+ self.assertIsInstance(sig, str)
+ p11.C_VerifyInit(self.session, CKM_SHA512_RSA_PKCS, public_key)
+ p11.C_Verify(self.session, hamster, sig)
- def test_gen_rsa_2048(self):
+ if False:
+ a = p11.C_GetAttributeValue(self.session, public_key,
+ CKA_CLASS, CKA_KEY_TYPE, CKA_VERIFY, CKA_TOKEN,
+ CKA_PUBLIC_EXPONENT, CKA_MODULUS)
+ a[CKA_TOKEN] = not a[CKA_TOKEN]
+ o = p11.C_CreateObject(self.session, a)
+ p11.C_VerifyInit(self.session, CKM_SHA512_RSA_PKCS, o)
+ p11.C_Verify(self.session, hamster, sig)
+
+ self.tearDown()
+ self.setUp()
+ o = p11.C_CreateObject(self.session, a)
+ p11.C_VerifyInit(self.session, CKM_SHA512_RSA_PKCS, o)
+ p11.C_Verify(self.session, hamster, sig)
+
+ def test_gen_sign_verify_rsa_2048(self):
if not args.all_tests: self.skipTest("RSA key generation is still painfully slow")
- self.assertIsKeypair(
- p11.C_GenerateKeyPair(self.session, CKM_RSA_PKCS_KEY_PAIR_GEN, CKA_MODULUS_BITS = 2048,
- CKA_ID = "RSA-1024", CKA_SIGN = True, CKA_VERIFY = True))
+ public_key, private_key = p11.C_GenerateKeyPair(
+ self.session, CKM_RSA_PKCS_KEY_PAIR_GEN, CKA_MODULUS_BITS = 2048,
+ CKA_ID = "RSA-2048", CKA_SIGN = True, CKA_VERIFY = True)
+ self.assertIsKeypair(public_key, private_key)
+ hamster = "Your mother was a hamster"
+ p11.C_SignInit(self.session, CKM_SHA512_RSA_PKCS, private_key)
+ sig = p11.C_Sign(self.session, hamster)
+ self.assertIsInstance(sig, str)
+ p11.C_VerifyInit(self.session, CKM_SHA512_RSA_PKCS, public_key)
+ p11.C_Verify(self.session, hamster, sig)
@staticmethod
def _build_ecpoint(x, y):
More information about the Commits
mailing list