From a34197c3758cfdc146708885a2207a5808df3ac4 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Fri, 5 Aug 2022 10:44:30 -0400 Subject: [PATCH 1/2] Change code to load all keys from the token Also remove the dual key storeage in the sotre object and instead deal with a single key, either public, or private in each object store. Signed-off-by: Simo Sorce --- src/asymmetric_cipher.c | 7 +- src/exchange.c | 6 +- src/keymgmt.c | 4 +- src/keys.c | 175 +++++++++++++++------------------ src/provider.h | 13 +-- src/signature.c | 55 ++++------- src/store.c | 210 +++++++++++++++++++++------------------- tests/test.sh | 16 ++- 8 files changed, 224 insertions(+), 262 deletions(-) diff --git a/src/asymmetric_cipher.c b/src/asymmetric_cipher.c index 46d8b2c0..1e874f1c 100644 --- a/src/asymmetric_cipher.c +++ b/src/asymmetric_cipher.c @@ -115,10 +115,7 @@ static int p11prov_rsaenc_encrypt_init(void *ctx, void *provkey, p11prov_debug("encrypt init (ctx=%p, key=%p, params=%p)\n", ctx, provkey, params); - encctx->key = p11prov_object_get_key(obj, false); - if (encctx->key == NULL) { - encctx->key = p11prov_object_get_key(obj, true); - } + encctx->key = p11prov_object_get_key(obj, 0); if (encctx->key == NULL) { return RET_OSSL_ERR; } @@ -219,7 +216,7 @@ static int p11prov_rsaenc_decrypt_init(void *ctx, void *provkey, p11prov_debug("encrypt init (ctx=%p, key=%p, params=%p)\n", ctx, provkey, params); - encctx->key = p11prov_object_get_key(obj, true); + encctx->key = p11prov_object_get_key(obj, CKO_PRIVATE_KEY); if (encctx->key == NULL) { return RET_OSSL_ERR; } diff --git a/src/exchange.c b/src/exchange.c index df2c9582..f00f26b4 100644 --- a/src/exchange.c +++ b/src/exchange.c @@ -186,7 +186,7 @@ static int p11prov_ecdh_init(void *ctx, void *provkey, } p11prov_key_free(ecdhctx->key); - ecdhctx->key = p11prov_object_get_key(obj, true); + ecdhctx->key = p11prov_object_get_key(obj, CKO_PRIVATE_KEY); if (ecdhctx->key == NULL) { return RET_OSSL_ERR; } @@ -204,7 +204,7 @@ static int p11prov_ecdh_set_peer(void *ctx, void *provkey) } p11prov_key_free(ecdhctx->peer_key); - ecdhctx->peer_key = p11prov_object_get_key(obj, false); + ecdhctx->peer_key = p11prov_object_get_key(obj, CKO_PUBLIC_KEY); if (ecdhctx->peer_key == NULL) { return RET_OSSL_ERR; } @@ -616,7 +616,7 @@ static int p11prov_exch_hkdf_init(void *ctx, void *provobj, if (provobj != &p11prov_hkdfkm_static_ctx) { p11prov_key_free(hkdfctx->key); - hkdfctx->key = p11prov_object_get_key(obj, true); + hkdfctx->key = p11prov_object_get_key(obj, CKO_PRIVATE_KEY); if (hkdfctx->key == NULL) { return RET_OSSL_ERR; } diff --git a/src/keymgmt.c b/src/keymgmt.c index 130f8a72..bf162f24 100644 --- a/src/keymgmt.c +++ b/src/keymgmt.c @@ -190,7 +190,7 @@ static int p11prov_rsakm_get_params(void *keydata, OSSL_PARAM params[]) return 0; } - key = p11prov_object_get_key(obj, false); + key = p11prov_object_get_key(obj, 0); if (key == NULL) { ret = RET_OSSL_ERR; goto done; @@ -435,7 +435,7 @@ static int p11prov_eckm_get_params(void *keydata, OSSL_PARAM params[]) return 0; } - key = p11prov_object_get_key(obj, false); + key = p11prov_object_get_key(obj, 0); if (key == NULL) { ret = RET_OSSL_ERR; goto done; diff --git a/src/keys.c b/src/keys.c index 1ece9b98..676f6fac 100644 --- a/src/keys.c +++ b/src/keys.c @@ -117,45 +117,38 @@ CK_ULONG p11prov_key_size(P11PROV_KEY *key) return key->key_size; } -static int fetch_rsa_key(CK_FUNCTION_LIST *f, CK_OBJECT_CLASS class, - CK_SESSION_HANDLE session, CK_OBJECT_HANDLE object, - P11PROV_KEY *key) +static int fetch_rsa_key(CK_FUNCTION_LIST *f, CK_SESSION_HANDLE session, + CK_OBJECT_HANDLE object, P11PROV_KEY *key) { struct fetch_attrs attrs[2]; unsigned long n_len = 0, e_len = 0; CK_BYTE *n = NULL, *e = NULL; int ret; - switch (class) { - case CKO_PRIVATE_KEY: - /* fallthrough */ - case CKO_PUBLIC_KEY: - key->attrs = OPENSSL_zalloc(2 * sizeof(CK_ATTRIBUTE)); - if (key->attrs == NULL) { - return CKR_HOST_MEMORY; - } - FA_ASSIGN_ALL(attrs[0], CKA_MODULUS, &n, &n_len, true, true); - FA_ASSIGN_ALL(attrs[1], CKA_PUBLIC_EXPONENT, &e, &e_len, true, true); - ret = p11prov_fetch_attributes(f, session, object, attrs, 2); - if (ret != CKR_OK) { - /* free any allocated memory */ - OPENSSL_free(n); - OPENSSL_free(e); + key->attrs = OPENSSL_zalloc(2 * sizeof(CK_ATTRIBUTE)); + if (key->attrs == NULL) { + return CKR_HOST_MEMORY; + } + FA_ASSIGN_ALL(attrs[0], CKA_MODULUS, &n, &n_len, true, true); + FA_ASSIGN_ALL(attrs[1], CKA_PUBLIC_EXPONENT, &e, &e_len, true, true); + ret = p11prov_fetch_attributes(f, session, object, attrs, 2); + if (ret != CKR_OK) { + /* free any allocated memory */ + OPENSSL_free(n); + OPENSSL_free(e); - if (class == CKO_PRIVATE_KEY) { - /* A private key may not always return these */ - return CKR_OK; - } - return ret; + if (key->class == CKO_PRIVATE_KEY) { + /* A private key may not always return these */ + return CKR_OK; } - - key->key_size = n_len; - CKATTR_ASSIGN_ALL(key->attrs[0], CKA_MODULUS, n, n_len); - CKATTR_ASSIGN_ALL(key->attrs[1], CKA_PUBLIC_EXPONENT, e, e_len); - key->numattrs = 2; - return CKR_OK; + return ret; } - return CKR_ARGUMENTS_BAD; + + key->key_size = n_len; + CKATTR_ASSIGN_ALL(key->attrs[0], CKA_MODULUS, n, n_len); + CKATTR_ASSIGN_ALL(key->attrs[1], CKA_PUBLIC_EXPONENT, e, e_len); + key->numattrs = 2; + return CKR_OK; } static int fetch_ec_public_key(CK_FUNCTION_LIST *f, CK_SESSION_HANDLE session, @@ -220,15 +213,16 @@ static int fetch_ec_public_key(CK_FUNCTION_LIST *f, CK_SESSION_HANDLE session, /* TODO: may want to have a hashmap with cached keys */ static P11PROV_KEY *object_handle_to_key(CK_FUNCTION_LIST *f, CK_SLOT_ID slotid, - CK_OBJECT_CLASS class, CK_SESSION_HANDLE session, CK_OBJECT_HANDLE object) { P11PROV_KEY *key; - unsigned long *key_type; - unsigned long key_type_len = sizeof(CKA_KEY_TYPE); - unsigned long label_len; - struct fetch_attrs attrs[3]; + CK_OBJECT_CLASS *key_class; + CK_ULONG key_class_len = sizeof(CK_OBJECT_CLASS); + CK_KEY_TYPE *key_type; + CK_ULONG key_type_len = sizeof(CK_KEY_TYPE); + CK_ULONG label_len; + struct fetch_attrs attrs[4]; int ret; key = p11prov_key_new(); @@ -236,15 +230,17 @@ static P11PROV_KEY *object_handle_to_key(CK_FUNCTION_LIST *f, CK_SLOT_ID slotid, return NULL; } + key_class = &key->class; + FA_ASSIGN_ALL(attrs[0], CKA_CLASS, &key_class, &key_class_len, false, true); key_type = &key->type; - FA_ASSIGN_ALL(attrs[0], CKA_KEY_TYPE, &key_type, &key_type_len, false, + FA_ASSIGN_ALL(attrs[1], CKA_KEY_TYPE, &key_type, &key_type_len, false, true); - FA_ASSIGN_ALL(attrs[1], CKA_ID, &key->id, &key->id_len, true, false); - FA_ASSIGN_ALL(attrs[2], CKA_LABEL, &key->label, &label_len, true, false); + FA_ASSIGN_ALL(attrs[2], CKA_ID, &key->id, &key->id_len, true, false); + FA_ASSIGN_ALL(attrs[3], CKA_LABEL, &key->label, &label_len, true, false); /* TODO: fetch also other attributes as specified in * Spev v3 - 4.9 Private key objects ?? */ - ret = p11prov_fetch_attributes(f, session, object, attrs, 3); + ret = p11prov_fetch_attributes(f, session, object, attrs, 4); if (ret != CKR_OK) { p11prov_debug("Failed to query object attributes (%d)\n", ret); p11prov_key_free(key); @@ -253,18 +249,17 @@ static P11PROV_KEY *object_handle_to_key(CK_FUNCTION_LIST *f, CK_SLOT_ID slotid, key->slotid = slotid; key->handle = object; - key->class = class; switch (key->type) { case CKK_RSA: - ret = fetch_rsa_key(f, class, session, object, key); + ret = fetch_rsa_key(f, session, object, key); if (ret != CKR_OK) { p11prov_key_free(key); return NULL; } break; case CKK_EC: - if (class == CKO_PRIVATE_KEY) { + if (key->class == CKO_PRIVATE_KEY) { /* no params to fetch */ break; } @@ -284,23 +279,22 @@ static P11PROV_KEY *object_handle_to_key(CK_FUNCTION_LIST *f, CK_SLOT_ID slotid, return key; } -int find_keys(P11PROV_CTX *provctx, P11PROV_KEY **priv, P11PROV_KEY **pub, - CK_SESSION_HANDLE session, CK_SLOT_ID slotid, - CK_OBJECT_CLASS class, P11PROV_URI *uri) +#define MAX_OBJS_IN_STORE 1024 + +CK_RV find_keys(P11PROV_CTX *provctx, CK_SESSION_HANDLE session, + CK_SLOT_ID slotid, P11PROV_URI *uri, store_key_callback cb, + void *cb_ctx) { CK_FUNCTION_LIST *f = p11prov_ctx_fns(provctx); + CK_OBJECT_CLASS class = p11prov_uri_get_class(uri); CK_ATTRIBUTE id = p11prov_uri_get_id(uri); char *label = p11prov_uri_get_object(uri); - CK_ATTRIBUTE template[3] = { - { CKA_CLASS, &class, sizeof(class) }, - }; - CK_ULONG tsize = 1; - CK_ULONG objcount; - P11PROV_KEY *pubkey = NULL; - P11PROV_KEY *privkey = NULL; + CK_ATTRIBUTE template[3] = { 0 }; + CK_ULONG tsize = 0; + CK_ULONG objcount = 0; P11PROV_KEY *key = NULL; - int result = CKR_GENERAL_ERROR; - int ret; + CK_RV result = CKR_GENERAL_ERROR; + CK_RV ret; p11prov_debug("Find keys\n"); @@ -308,6 +302,14 @@ int find_keys(P11PROV_CTX *provctx, P11PROV_KEY **priv, P11PROV_KEY **pub, return result; } + if (class != CK_UNAVAILABLE_INFORMATION) { + if (class != CKO_PUBLIC_KEY && class != CKO_PRIVATE_KEY) { + /* nothing to find for us */ + return CKR_OK; + } + CKATTR_ASSIGN_ALL(template[tsize], CKA_CLASS, &class, sizeof(class)); + tsize++; + } if (id.type == CKA_ID) { template[tsize] = id; tsize++; @@ -317,58 +319,35 @@ int find_keys(P11PROV_CTX *provctx, P11PROV_KEY **priv, P11PROV_KEY **pub, tsize++; } -again: ret = f->C_FindObjectsInit(session, template, tsize); - if (ret == CKR_OK) { - do { - CK_OBJECT_HANDLE object; - /* TODO: pull multiple objects at once to reduce roundtrips */ - ret = f->C_FindObjects(session, &object, 1, &objcount); - if (ret != CKR_OK || objcount == 0) { - break; - } - - key = object_handle_to_key(f, slotid, class, session, object); + if (ret != CKR_OK) { + P11PROV_raise(provctx, ret, "Error returned by C_FindObjectsInit"); + return ret; + } + for (int idx = 0; idx < MAX_OBJS_IN_STORE; idx += objcount) { + CK_OBJECT_HANDLE object[64]; + ret = f->C_FindObjects(session, object, 64, &objcount); + if (ret != CKR_OK || objcount == 0) { + result = ret; + break; + } - /* we'll get the first that parses fine */ + for (CK_ULONG k = 0; k < objcount; k++) { + key = object_handle_to_key(f, slotid, session, object[k]); if (key) { - result = CKR_OK; - if (class == CKO_PRIVATE_KEY) { - privkey = key; - ret = f->C_FindObjectsFinal(session); - if (ret != CKR_OK) { - P11PROV_raise(provctx, ret, - "Failed to terminate object search"); - } - class = CKO_PUBLIC_KEY; - goto again; + ret = cb(cb_ctx, key->class, key); + if (ret != CKR_OK) { + result = ret; + break; } - if (key) { - /* it is not always possible to pull all (or any) - * attributes from private keys, so fixup key_size - * based on the public key if it is missing. */ - if (privkey && privkey->key_size == 0) { - privkey->key_size = key->key_size; - } - pubkey = key; - } - break; } - - } while (objcount > 0); - - ret = f->C_FindObjectsFinal(session); - if (ret != CKR_OK) { - P11PROV_raise(provctx, ret, "Failed to terminate object search"); } - } else { - P11PROV_raise(provctx, ret, "Error returned by C_FindObjectsInit"); - result = ret; } - if (result == CKR_OK) { - *pub = pubkey; - *priv = privkey; + ret = f->C_FindObjectsFinal(session); + if (ret != CKR_OK) { + /* this is not fatal */ + P11PROV_raise(provctx, ret, "Failed to terminate object search"); } return result; diff --git a/src/provider.h b/src/provider.h index d2376096..dc1b8a89 100644 --- a/src/provider.h +++ b/src/provider.h @@ -91,9 +91,10 @@ CK_SLOT_ID p11prov_key_slotid(P11PROV_KEY *key); CK_OBJECT_HANDLE p11prov_key_handle(P11PROV_KEY *key); CK_ULONG p11prov_key_size(P11PROV_KEY *key); -int find_keys(P11PROV_CTX *provctx, P11PROV_KEY **priv, P11PROV_KEY **pub, - CK_SESSION_HANDLE session, CK_SLOT_ID slotid, - CK_OBJECT_CLASS class, P11PROV_URI *uri); +typedef CK_RV (*store_key_callback)(void *, CK_OBJECT_CLASS, P11PROV_KEY *); +CK_RV find_keys(P11PROV_CTX *provctx, CK_SESSION_HANDLE session, + CK_SLOT_ID slotid, P11PROV_URI *uri, store_key_callback cb, + void *cb_ctx); P11PROV_KEY *p11prov_create_secret_key(P11PROV_CTX *provctx, CK_SESSION_HANDLE session, bool session_key, unsigned char *secret, @@ -106,7 +107,7 @@ P11PROV_OBJ *p11prov_obj_from_reference(const void *reference, size_t reference_sz); int p11prov_object_export_public_rsa_key(P11PROV_OBJ *obj, OSSL_CALLBACK *cb_fn, void *cb_arg); -P11PROV_KEY *p11prov_object_get_key(P11PROV_OBJ *obj, bool priv); +P11PROV_KEY *p11prov_object_get_key(P11PROV_OBJ *obj, CK_OBJECT_CLASS class); /* dispatching */ #define DECL_DISPATCH_FUNC(type, prefix, name) \ @@ -198,8 +199,8 @@ extern const OSSL_DISPATCH p11prov_hkdf_kdf_functions[]; struct fetch_attrs { CK_ATTRIBUTE_TYPE type; - unsigned char **value; - unsigned long *value_len; + CK_BYTE **value; + CK_ULONG *value_len; bool allocate; bool required; }; diff --git a/src/signature.c b/src/signature.c index 4028783b..05313d70 100644 --- a/src/signature.c +++ b/src/signature.c @@ -10,8 +10,7 @@ struct p11prov_sig_ctx { P11PROV_CTX *provctx; char *properties; - P11PROV_KEY *priv_key; - P11PROV_KEY *pub_key; + P11PROV_KEY *key; CK_MECHANISM_TYPE mechtype; CK_MECHANISM_TYPE digest; @@ -79,8 +78,7 @@ static void *p11prov_sig_dupctx(void *ctx) return NULL; } - newctx->priv_key = p11prov_key_ref(sigctx->priv_key); - newctx->pub_key = p11prov_key_ref(sigctx->pub_key); + newctx->key = p11prov_key_ref(sigctx->key); newctx->mechtype = sigctx->mechtype; newctx->digest = sigctx->digest; newctx->pss_params = sigctx->pss_params; @@ -100,12 +98,9 @@ static void *p11prov_sig_dupctx(void *ctx) switch (sigctx->operation) { case CKF_SIGN: - slotid = p11prov_key_slotid(sigctx->priv_key); - handle = p11prov_key_handle(newctx->priv_key); - break; case CKF_VERIFY: - slotid = p11prov_key_slotid(sigctx->pub_key); - handle = p11prov_key_handle(newctx->pub_key); + slotid = p11prov_key_slotid(sigctx->key); + handle = p11prov_key_handle(newctx->key); break; default: p11prov_sig_freectx(newctx); @@ -174,8 +169,7 @@ static void p11prov_sig_freectx(void *ctx) } } - p11prov_key_free(sigctx->priv_key); - p11prov_key_free(sigctx->pub_key); + p11prov_key_free(sigctx->key); OPENSSL_free(sigctx->properties); OPENSSL_clear_free(sigctx, sizeof(P11PROV_SIG_CTX)); } @@ -319,19 +313,17 @@ static int p11prov_sig_set_mechanism(void *ctx, bool digest_sign, if (result == CKR_OK) { p11prov_debug_mechanism(sigctx->provctx, - p11prov_key_slotid(sigctx->pub_key), + p11prov_key_slotid(sigctx->key), mechanism->mechanism); } return result; } -#define ANYKEY(ctx) (ctx->pub_key ? ctx->pub_key : ctx->priv_key) - static int p11prov_sig_get_sig_size(void *ctx, size_t *siglen) { P11PROV_SIG_CTX *sigctx = (P11PROV_SIG_CTX *)ctx; - CK_KEY_TYPE type = p11prov_key_type(ANYKEY(sigctx)); - CK_ULONG size = p11prov_key_size(ANYKEY(sigctx)); + CK_KEY_TYPE type = p11prov_key_type(sigctx->key); + CK_ULONG size = p11prov_key_size(sigctx->key); if (type == CK_UNAVAILABLE_INFORMATION) { return RET_OSSL_ERR; @@ -377,12 +369,14 @@ static int p11prov_sig_op_init(void *ctx, void *provkey, CK_FLAGS operation, P11PROV_SIG_CTX *sigctx = (P11PROV_SIG_CTX *)ctx; P11PROV_OBJ *obj = (P11PROV_OBJ *)provkey; - sigctx->priv_key = p11prov_object_get_key(obj, true); - if (sigctx->priv_key == NULL) { + if (operation == CKF_SIGN) { + sigctx->key = p11prov_object_get_key(obj, CKO_PRIVATE_KEY); + } else { + sigctx->key = p11prov_object_get_key(obj, CKO_PUBLIC_KEY); + } + if (sigctx->key == NULL) { return RET_OSSL_ERR; } - sigctx->pub_key = p11prov_object_get_key(obj, false); - sigctx->operation = operation; if (digest) { @@ -404,7 +398,6 @@ static int p11prov_sig_operate_init(P11PROV_SIG_CTX *sigctx, bool digest_op, CK_SESSION_HANDLE session; CK_OBJECT_HANDLE handle; CK_SLOT_ID slotid; - P11PROV_KEY *key; int ret; f = p11prov_ctx_fns(sigctx->provctx); @@ -412,24 +405,13 @@ static int p11prov_sig_operate_init(P11PROV_SIG_CTX *sigctx, bool digest_op, return CKR_GENERAL_ERROR; } - switch (sigctx->operation) { - case CKF_SIGN: - key = sigctx->priv_key; - break; - case CKF_VERIFY: - key = sigctx->pub_key; - break; - default: - return CKR_FUNCTION_REJECTED; - } - - handle = p11prov_key_handle(key); + handle = p11prov_key_handle(sigctx->key); if (handle == CK_INVALID_HANDLE) { P11PROV_raise(sigctx->provctx, CKR_KEY_HANDLE_INVALID, "Provided key has invalid handle"); return CKR_KEY_HANDLE_INVALID; } - slotid = p11prov_key_slotid(key); + slotid = p11prov_key_slotid(sigctx->key); if (slotid == CK_UNAVAILABLE_INFORMATION) { P11PROV_raise(sigctx->provctx, CKR_SLOT_ID_INVALID, "Provided key has invalid slot"); @@ -969,9 +951,8 @@ static int p11prov_rsasig_set_ctx_params(void *ctx, const OSSL_PARAM params[]) } sigctx->mechtype = mechtype; - p11prov_debug_mechanism(sigctx->provctx, - p11prov_key_slotid(sigctx->pub_key), - sigctx->mechtype); + p11prov_debug_mechanism( + sigctx->provctx, p11prov_key_slotid(sigctx->key), sigctx->mechtype); } p = OSSL_PARAM_locate_const(params, OSSL_SIGNATURE_PARAM_PSS_SALTLEN); diff --git a/src/store.c b/src/store.c index 0045100b..2c57046f 100644 --- a/src/store.c +++ b/src/store.c @@ -6,28 +6,39 @@ #include "platform/endian.h" struct p11prov_obj { - struct p11prov_key *priv_key; - struct p11prov_key *pub_key; + CK_OBJECT_CLASS class; + union { + struct p11prov_key *key; + } data; int refcnt; }; -static P11PROV_OBJ *p11prov_object_new(P11PROV_KEY *priv_key, - P11PROV_KEY *pub_key) +static CK_RV p11prov_object_new(P11PROV_CTX *ctx, CK_OBJECT_CLASS class, + P11PROV_KEY *key, P11PROV_OBJ **object) { P11PROV_OBJ *obj; obj = OPENSSL_zalloc(sizeof(P11PROV_OBJ)); if (obj == NULL) { - return NULL; + return CKR_HOST_MEMORY; + } + obj->class = class; + switch (class) { + case CKO_PUBLIC_KEY: + case CKO_PRIVATE_KEY: + obj->data.key = p11prov_key_ref(key); + break; + default: + P11PROV_raise(ctx, CKR_ARGUMENTS_BAD, "Unsupported object type"); + OPENSSL_free(obj); + return CKR_ARGUMENTS_BAD; } - - obj->priv_key = p11prov_key_ref(priv_key); - obj->pub_key = p11prov_key_ref(pub_key); obj->refcnt = 1; - return obj; + *object = obj; + return CKR_OK; } static P11PROV_OBJ *p11prov_object_ref(P11PROV_OBJ *obj) @@ -51,8 +62,14 @@ void p11prov_object_free(P11PROV_OBJ *obj) return; } - p11prov_key_free(obj->priv_key); - p11prov_key_free(obj->pub_key); + switch (obj->class) { + case CKO_PUBLIC_KEY: + case CKO_PRIVATE_KEY: + p11prov_key_free(obj->data.key); + break; + default: + p11prov_debug("object_free: invalid class: %lu", obj->class); + } OPENSSL_clear_free(obj, sizeof(P11PROV_OBJ)); } @@ -60,17 +77,20 @@ void p11prov_object_free(P11PROV_OBJ *obj) bool p11prov_object_check_key(P11PROV_OBJ *obj, bool priv) { if (priv) { - return obj->priv_key != NULL; + return obj->class == CKO_PRIVATE_KEY; } - return obj->pub_key != NULL; + return obj->class == CKO_PRIVATE_KEY; } -P11PROV_KEY *p11prov_object_get_key(P11PROV_OBJ *obj, bool priv) +P11PROV_KEY *p11prov_object_get_key(P11PROV_OBJ *obj, CK_OBJECT_CLASS class) { - if (priv) { - return p11prov_key_ref(obj->priv_key); + if (class == CKO_PRIVATE_KEY && obj->class != CKO_PRIVATE_KEY) { + return NULL; + } + if (class == CKO_PUBLIC_KEY && obj->class != CKO_PUBLIC_KEY) { + return NULL; } - return p11prov_key_ref(obj->pub_key); + return p11prov_key_ref(obj->data.key); } /* Tokens return data in bigendian order, while openssl @@ -110,11 +130,19 @@ int p11prov_object_export_public_rsa_key(P11PROV_OBJ *obj, OSSL_CALLBACK *cb_fn, CK_ATTRIBUTE *n, *e; unsigned char *val; - if (p11prov_key_type(obj->pub_key) != CKK_RSA) { + switch (obj->class) { + case CKO_PUBLIC_KEY: + case CKO_PRIVATE_KEY: + break; + default: + return RET_OSSL_ERR; + } + + if (p11prov_key_type(obj->data.key) != CKK_RSA) { return RET_OSSL_ERR; } - n = p11prov_key_attr(obj->pub_key, CKA_MODULUS); + n = p11prov_key_attr(obj->data.key, CKA_MODULUS); if (n == NULL) { return RET_OSSL_ERR; } @@ -123,7 +151,7 @@ int p11prov_object_export_public_rsa_key(P11PROV_OBJ *obj, OSSL_CALLBACK *cb_fn, params[0] = OSSL_PARAM_construct_BN(OSSL_PKEY_PARAM_RSA_N, val, n->ulValueLen); - e = p11prov_key_attr(obj->pub_key, CKA_PUBLIC_EXPONENT); + e = p11prov_key_attr(obj->data.key, CKA_PUBLIC_EXPONENT); if (e == NULL) { return RET_OSSL_ERR; } @@ -195,15 +223,25 @@ static void p11prov_store_ctx_free(struct p11prov_store_ctx *ctx) } #define OBJS_ALLOC_SIZE 8 -static CK_RV p11prov_store_ctx_add_obj(struct p11prov_store_ctx *ctx, - P11PROV_OBJ *obj) +static CK_RV p11prov_store_ctx_add_key(void *pctx, CK_OBJECT_CLASS class, + P11PROV_KEY *key) { + struct p11prov_store_ctx *ctx = (struct p11prov_store_ctx *)pctx; + P11PROV_OBJ *obj; + CK_RV ret; + + ret = p11prov_object_new(ctx->provctx, class, key, &obj); + if (ret != CKR_OK) { + return ret; + } + if ((ctx->num_objs % OBJS_ALLOC_SIZE) == 0) { P11PROV_OBJ **tmp = OPENSSL_realloc(ctx->objects, ctx->num_objs + OBJS_ALLOC_SIZE); if (tmp == NULL) { P11PROV_raise(ctx->provctx, CKR_HOST_MEMORY, "Failed to allocate store objects"); + p11prov_object_free(obj); return CKR_HOST_MEMORY; } ctx->objects = tmp; @@ -214,22 +252,11 @@ static CK_RV p11prov_store_ctx_add_obj(struct p11prov_store_ctx *ctx, return CKR_OK; } -DISPATCH_STORE_FN(open); -DISPATCH_STORE_FN(attach); -DISPATCH_STORE_FN(load); -DISPATCH_STORE_FN(eof); -DISPATCH_STORE_FN(close); -DISPATCH_STORE_FN(export_object); -DISPATCH_STORE_FN(set_ctx_params); -DISPATCH_STORE_FN(settable_ctx_params); - static void store_load(struct p11prov_store_ctx *ctx, OSSL_PASSPHRASE_CALLBACK *pw_cb, void *pw_cbarg) { CK_SLOT_ID slotid = CK_UNAVAILABLE_INFORMATION; CK_SLOT_ID nextid = CK_UNAVAILABLE_INFORMATION; - /* 0 is CKO_DATA but we do not use it */ - CK_OBJECT_CLASS class = 0; CK_RV ret; do { @@ -262,37 +289,11 @@ static void store_load(struct p11prov_store_ctx *ctx, return; } - /* FIXME: we should probbaly just load everything, filter later */ - class = CKO_PRIVATE_KEY; - if (p11prov_uri_get_class(ctx->parsed_uri) == CKO_PUBLIC_KEY) { - class = CKO_PUBLIC_KEY; - } - if (class == CKO_PUBLIC_KEY || class == CKO_PRIVATE_KEY) { - P11PROV_KEY *priv_key = NULL; - P11PROV_KEY *pub_key = NULL; - - ret = find_keys(ctx->provctx, &priv_key, &pub_key, ctx->session, - slotid, class, ctx->parsed_uri); - if (ret == CKR_OK) { - /* new object */ - P11PROV_OBJ *obj; - - obj = p11prov_object_new(priv_key, pub_key); - if (obj == NULL) { - p11prov_key_free(priv_key); - p11prov_key_free(pub_key); - return; - } - ret = p11prov_store_ctx_add_obj(ctx, obj); - if (ret != CKR_OK) { - p11prov_key_free(priv_key); - p11prov_key_free(pub_key); - p11prov_object_free(obj); - return; - } - } - p11prov_key_free(priv_key); - p11prov_key_free(pub_key); + ret = find_keys(ctx->provctx, ctx->session, slotid, ctx->parsed_uri, + p11prov_store_ctx_add_key, ctx); + if (ret != CKR_OK) { + ctx->loaded = -1; + return; } slotid = nextid; @@ -301,6 +302,15 @@ static void store_load(struct p11prov_store_ctx *ctx, ctx->loaded = 1; } +DISPATCH_STORE_FN(open); +DISPATCH_STORE_FN(attach); +DISPATCH_STORE_FN(load); +DISPATCH_STORE_FN(eof); +DISPATCH_STORE_FN(close); +DISPATCH_STORE_FN(export_object); +DISPATCH_STORE_FN(set_ctx_params); +DISPATCH_STORE_FN(settable_ctx_params); + static void *p11prov_store_open(void *pctx, const char *uri) { struct p11prov_store_ctx *ctx = NULL; @@ -365,43 +375,45 @@ static int p11prov_store_load(void *pctx, OSSL_CALLBACK *object_cb, obj = ctx->objects[ctx->fetched]; ctx->fetched++; - /* FIXME: always a key for now */ - object_type = OSSL_OBJECT_PKEY; - params[0] = OSSL_PARAM_construct_int(OSSL_OBJECT_PARAM_TYPE, &object_type); - - if (obj->pub_key) { - type = p11prov_key_type(obj->pub_key); - } else { - type = p11prov_key_type(obj->priv_key); - } - switch (type) { - case CKK_RSA: - /* REMOVE once we have encoders to export pub keys. - * we have to handle private keys as our own type, - * while we can let openssl import public keys and - * deal with them in the default provider */ - switch (ctx->expect) { - case OSSL_STORE_INFO_PKEY: - data_type = (char *)P11PROV_NAMES_RSA; - break; - case OSSL_STORE_INFO_PUBKEY: - data_type = (char *)"RSA"; - break; - default: - if (obj->priv_key) { + switch (obj->class) { + case CKO_PUBLIC_KEY: + case CKO_PRIVATE_KEY: + object_type = OSSL_OBJECT_PKEY; + type = p11prov_key_type(obj->data.key); + switch (type) { + case CKK_RSA: + /* REMOVE once we have encoders to export pub keys. + * we have to handle private keys as our own type, + * while we can let openssl import public keys and + * deal with them in the default provider */ + switch (ctx->expect) { + case OSSL_STORE_INFO_PKEY: data_type = (char *)P11PROV_NAMES_RSA; - } else { + break; + case OSSL_STORE_INFO_PUBKEY: data_type = (char *)"RSA"; + break; + default: + if (obj->class == CKO_PRIVATE_KEY) { + data_type = (char *)P11PROV_NAMES_RSA; + } else { + data_type = (char *)"RSA"; + } + break; } break; + case CKK_EC: + data_type = (char *)P11PROV_NAMES_ECDSA; + break; + default: + return RET_OSSL_ERR; } break; - case CKK_EC: - data_type = (char *)P11PROV_NAMES_ECDSA; - break; default: return RET_OSSL_ERR; } + + params[0] = OSSL_PARAM_construct_int(OSSL_OBJECT_PARAM_TYPE, &object_type); params[1] = OSSL_PARAM_construct_utf8_string(OSSL_OBJECT_PARAM_DATA_TYPE, data_type, 0); @@ -420,7 +432,10 @@ static int p11prov_store_eof(void *pctx) p11prov_debug("store eof (%p)\n", ctx); - if (ctx->loaded && (ctx->fetched >= ctx->num_objs)) { + if (ctx->loaded == -1) { + /* error condition nothing more to return */ + return 1; + } else if (ctx->loaded && ctx->fetched >= ctx->num_objs) { return 1; } return 0; @@ -466,14 +481,7 @@ static int p11prov_store_export_object(void *loaderctx, const void *reference, } /* we can only export public bits, so that's all we do */ - switch (p11prov_key_type(obj->pub_key)) { - case CKK_RSA: - return p11prov_object_export_public_rsa_key(obj, cb_fn, cb_arg); - case CKK_EC: - default: - break; - } - return RET_OSSL_ERR; + return p11prov_object_export_public_rsa_key(obj, cb_fn, cb_arg); } static const OSSL_PARAM *p11prov_store_settable_ctx_params(void *provctx) diff --git a/tests/test.sh b/tests/test.sh index 3bd28a82..edff19b2 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -243,15 +243,11 @@ fi title PARA "Sign and Verify with provided Hash and RSA" ossl 'dgst -sha256 -binary -out ${TMPDIR}/sha256.bin ${SEEDFILE}' ossl ' -pkeyutl -sign -inkey "${BASEURI}" +pkeyutl -sign -inkey "${PRIURI}" -in ${TMPDIR}/sha256.bin -out ${TMPDIR}/sha256-sig.bin' ossl ' -pkeyutl -verify -inkey "${PRIURI}" - -in ${TMPDIR}/sha256.bin - -sigfile ${TMPDIR}/sha256-sig.bin' -ossl ' pkeyutl -verify -inkey "${PUBURI}" -pubin -in ${TMPDIR}/sha256.bin @@ -264,7 +260,7 @@ pkeyutl -sign -inkey "${ECBASEURI}" -out ${TMPDIR}/sha256-ecsig.bin' ossl ' -pkeyutl -verify -inkey "${ECBASEURI}" +pkeyutl -verify -inkey "${ECBASEURI}" -pubin -in ${TMPDIR}/sha256.bin -sigfile ${TMPDIR}/sha256-ecsig.bin' @@ -278,7 +274,7 @@ pkeyutl -sign -inkey "${BASEURI}" -rawin -out ${TMPDIR}/sha256-dgstsig.bin' ossl ' -pkeyutl -verify -inkey "${BASEURI}" +pkeyutl -verify -inkey "${BASEURI}" -pubin -digest sha256 -in ${TMPDIR}/64krandom.bin -rawin @@ -302,7 +298,7 @@ pkeyutl -sign -inkey "${BASEURI}" -rawin -out ${TMPDIR}/sha256-dgstsig.bin' ossl ' -pkeyutl -verify -inkey "${BASEURI}" +pkeyutl -verify -inkey "${BASEURI}" -pubin -digest sha256 -pkeyopt pad-mode:pss -pkeyopt mgf1-digest:sha256 @@ -331,7 +327,7 @@ pkeyutl -sign -inkey "${ECBASEURI}" -rawin -out ${TMPDIR}/sha256-ecdgstsig.bin' ossl ' -pkeyutl -verify -inkey "${ECBASEURI}" +pkeyutl -verify -inkey "${ECBASEURI}" -pubin -digest sha256 -in ${TMPDIR}/64krandom.bin -rawin @@ -359,7 +355,7 @@ diff ${TMPDIR}/secret.txt ${TMPDIR}/secret.txt.dec title LINE "Now again all in the token" ossl ' -pkeyutl -encrypt -inkey "${PRIURI}" +pkeyutl -encrypt -inkey "${PUBURI}" -pubin -in ${TMPDIR}/secret.txt -out ${TMPDIR}/secret.txt.enc2' ossl ' From b8f600aaff1aa25acad0814805f0f976d4ef91f5 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Fri, 5 Aug 2022 18:37:01 -0400 Subject: [PATCH 2/2] Fix private key usage The EC private key was lacking EC_PARAMS parsing therefore key size was always reported as 0, additionally we were always return private key type when asking the keymgmt functions. Signed-off-by: Simo Sorce --- src/keys.c | 28 ++++++++++++++++------------ src/store.c | 2 +- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/keys.c b/src/keys.c index 676f6fac..a871c7d1 100644 --- a/src/keys.c +++ b/src/keys.c @@ -151,13 +151,14 @@ static int fetch_rsa_key(CK_FUNCTION_LIST *f, CK_SESSION_HANDLE session, return CKR_OK; } -static int fetch_ec_public_key(CK_FUNCTION_LIST *f, CK_SESSION_HANDLE session, - CK_OBJECT_HANDLE object, P11PROV_KEY *key) +static int fetch_ec_key(CK_FUNCTION_LIST *f, CK_SESSION_HANDLE session, + CK_OBJECT_HANDLE object, P11PROV_KEY *key) { struct fetch_attrs attrs[2]; unsigned long params_len = 0, point_len = 0; CK_BYTE *params = NULL, *point = NULL; size_t n_bytes = 0; + int n; int ret; key->attrs = OPENSSL_zalloc(2 * sizeof(CK_ATTRIBUTE)); @@ -165,9 +166,14 @@ static int fetch_ec_public_key(CK_FUNCTION_LIST *f, CK_SESSION_HANDLE session, return CKR_HOST_MEMORY; } - FA_ASSIGN_ALL(attrs[0], CKA_EC_PARAMS, ¶ms, ¶ms_len, true, true); - FA_ASSIGN_ALL(attrs[1], CKA_EC_POINT, &point, &point_len, true, true); - ret = p11prov_fetch_attributes(f, session, object, attrs, 2); + n = 0; + FA_ASSIGN_ALL(attrs[n], CKA_EC_PARAMS, ¶ms, ¶ms_len, true, true); + n++; + if (key->class == CKO_PUBLIC_KEY) { + FA_ASSIGN_ALL(attrs[n], CKA_EC_POINT, &point, &point_len, true, true); + n++; + } + ret = p11prov_fetch_attributes(f, session, object, attrs, n); if (ret != CKR_OK) { /* free any allocated memory */ OPENSSL_free(params); @@ -206,8 +212,10 @@ static int fetch_ec_public_key(CK_FUNCTION_LIST *f, CK_SESSION_HANDLE session, key->key_size = n_bytes; CKATTR_ASSIGN_ALL(key->attrs[0], CKA_EC_PARAMS, params, params_len); - CKATTR_ASSIGN_ALL(key->attrs[1], CKA_EC_POINT, point, point_len); - key->numattrs = 2; + if (n > 1) { + CKATTR_ASSIGN_ALL(key->attrs[1], CKA_EC_POINT, point, point_len); + } + key->numattrs = n; return CKR_OK; } @@ -259,11 +267,7 @@ static P11PROV_KEY *object_handle_to_key(CK_FUNCTION_LIST *f, CK_SLOT_ID slotid, } break; case CKK_EC: - if (key->class == CKO_PRIVATE_KEY) { - /* no params to fetch */ - break; - } - ret = fetch_ec_public_key(f, session, object, key); + ret = fetch_ec_key(f, session, object, key); if (ret != CKR_OK) { p11prov_key_free(key); return NULL; diff --git a/src/store.c b/src/store.c index 2c57046f..39037cd8 100644 --- a/src/store.c +++ b/src/store.c @@ -79,7 +79,7 @@ bool p11prov_object_check_key(P11PROV_OBJ *obj, bool priv) if (priv) { return obj->class == CKO_PRIVATE_KEY; } - return obj->class == CKO_PRIVATE_KEY; + return obj->class == CKO_PUBLIC_KEY; } P11PROV_KEY *p11prov_object_get_key(P11PROV_OBJ *obj, CK_OBJECT_CLASS class)