[pkg-opensc-commit] [libp11] 53/86: Refactored attribute retrieval
Eric Dorland
eric at moszumanska.debian.org
Sun Jul 24 21:40:22 UTC 2016
This is an automated email from the git hooks/post-receive script.
eric pushed a commit to branch master
in repository libp11.
commit a86eed9d4c8a39fc563327dacd65aa8c5783327e
Author: Michał Trojnara <Michal.Trojnara at stunnel.org>
Date: Tue Feb 16 15:22:41 2016 +0100
Refactored attribute retrieval
Dynamic memory allocation for retrieved attributes was centralized and simplified.
A few hardcoded size limits were removed.
---
src/libp11-int.h | 18 +++---
src/p11_attr.c | 75 ++++++++++-------------
src/p11_cert.c | 31 +++-------
src/p11_ec.c | 183 +++++++++++++++++++++----------------------------------
src/p11_err.c | 2 +-
src/p11_key.c | 14 ++---
6 files changed, 125 insertions(+), 198 deletions(-)
diff --git a/src/libp11-int.h b/src/libp11-int.h
index fe1163a..f8bfa49 100644
--- a/src/libp11-int.h
+++ b/src/libp11-int.h
@@ -154,26 +154,24 @@ extern void pkcs11_destroy_keys(PKCS11_TOKEN *, unsigned int);
extern void pkcs11_destroy_certs(PKCS11_TOKEN *);
extern char *pkcs11_strdup(char *, size_t);
-extern int pkcs11_getattr(PKCS11_TOKEN *, CK_OBJECT_HANDLE,
- unsigned int, void *, size_t);
-extern int pkcs11_getattr_s(PKCS11_TOKEN *, CK_OBJECT_HANDLE,
- unsigned int, void *, size_t);
extern int pkcs11_getattr_var(PKCS11_TOKEN *, CK_OBJECT_HANDLE,
- unsigned int, void *, size_t *);
+ unsigned int, CK_BYTE *, size_t *);
+extern int pkcs11_getattr_alloc(PKCS11_TOKEN *, CK_OBJECT_HANDLE,
+ unsigned int, CK_BYTE **, size_t *);
extern int pkcs11_getattr_bn(PKCS11_TOKEN *, CK_OBJECT_HANDLE,
unsigned int, BIGNUM **);
extern int pkcs11_reload_key(PKCS11_KEY *);
-#define key_getattr(key, t, p, s) \
- pkcs11_getattr(KEY2TOKEN((key)), PRIVKEY((key))->object, (t), (p), (s))
+#define key_getattr_var(key, t, p, s) \
+ pkcs11_getattr_var(KEY2TOKEN((key)), PRIVKEY((key))->object, (t), (p), (s))
+
+#define key_getattr_alloc(key, t, p, s) \
+ pkcs11_getattr_alloc(KEY2TOKEN((key)), PRIVKEY((key))->object, (t), (p), (s))
#define key_getattr_bn(key, t, bn) \
pkcs11_getattr_bn(KEY2TOKEN((key)), PRIVKEY((key))->object, (t), (bn))
-#define key_getattr_var(key, t, p, s) \
- pkcs11_getattr_var(KEY2TOKEN((key)), PRIVKEY((key))->object, (t), (p), (s))
-
typedef int (*pkcs11_i2d_fn) (void *, unsigned char **);
extern void pkcs11_addattr(CK_ATTRIBUTE_PTR, int, const void *, size_t);
extern void pkcs11_addattr_int(CK_ATTRIBUTE_PTR, int, unsigned long);
diff --git a/src/p11_attr.c b/src/p11_attr.c
index d46bc02..74750a7 100644
--- a/src/p11_attr.c
+++ b/src/p11_attr.c
@@ -1,5 +1,6 @@
/* libp11, a simple layer on to of PKCS#11 API
* Copyright (C) 2005 Olaf Kirch <okir at lst.de>
+ * Copyright (C) 2016 Michał Trojnara <Michal.Trojnara at stunnel.org>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -32,10 +33,9 @@
/*
* Query pkcs11 attributes
*/
-static int
-pkcs11_getattr_int(PKCS11_CTX * ctx, CK_SESSION_HANDLE session,
- CK_OBJECT_HANDLE o, CK_ATTRIBUTE_TYPE type, void *value,
- size_t * size)
+static int pkcs11_getattr_int(PKCS11_CTX *ctx, CK_SESSION_HANDLE session,
+ CK_OBJECT_HANDLE o, CK_ATTRIBUTE_TYPE type, CK_BYTE *value,
+ size_t *size)
{
CK_ATTRIBUTE templ;
int rv;
@@ -51,52 +51,46 @@ pkcs11_getattr_int(PKCS11_CTX * ctx, CK_SESSION_HANDLE session,
return 0;
}
-int
-pkcs11_getattr_var(PKCS11_TOKEN * token, CK_OBJECT_HANDLE object,
- unsigned int type, void *value, size_t * size)
+int pkcs11_getattr_var(PKCS11_TOKEN *token, CK_OBJECT_HANDLE object,
+ unsigned int type, CK_BYTE *value, size_t *size)
{
return pkcs11_getattr_int(TOKEN2CTX(token),
PRIVSLOT(TOKEN2SLOT(token))->session,
object, type, value, size);
}
-int
-pkcs11_getattr(PKCS11_TOKEN * token, CK_OBJECT_HANDLE object,
- unsigned int type, void *value, size_t size)
+int pkcs11_getattr_alloc(PKCS11_TOKEN *token, CK_OBJECT_HANDLE object,
+ unsigned int type, CK_BYTE **value, size_t *size)
{
- return pkcs11_getattr_var(token, object, type, value, &size);
-}
+ CK_BYTE *data;
+ size_t len = 0;
-int
-pkcs11_getattr_s(PKCS11_TOKEN * token, CK_OBJECT_HANDLE object,
- unsigned int type, void *value, size_t size)
-{
- memset(value, 0, size);
- return pkcs11_getattr_var(token, object, type, value, &size);
+ if (pkcs11_getattr_var(token, object, type, NULL, &len))
+ return -1;
+ data = OPENSSL_malloc(len+1);
+ if (data == NULL) {
+ PKCS11err(PKCS11_F_PKCS11_GETATTR, pkcs11_map_err(CKR_HOST_MEMORY));
+ return -1;
+ }
+ memset(data, 0, len+1); /* also null-terminate the allocated data */
+ if (pkcs11_getattr_var(token, object, type, data, &len))
+ return -1;
+ if (value)
+ *value = data;
+ if (size)
+ *size = len;
+ return 0;
}
-int
-pkcs11_getattr_bn(PKCS11_TOKEN * token, CK_OBJECT_HANDLE object,
- unsigned int type, BIGNUM ** bn)
+int pkcs11_getattr_bn(PKCS11_TOKEN *token, CK_OBJECT_HANDLE object,
+ unsigned int type, BIGNUM **bn)
{
CK_BYTE *binary;
size_t size;
- int ret;
size = 0;
- if (pkcs11_getattr_var(token, object, type, NULL, &size) || size == 0)
+ if (pkcs11_getattr_alloc(token, object, type, &binary, &size))
return -1;
-
- binary = OPENSSL_malloc(size);
- if (binary == NULL)
- return -1;
- memset(binary, 0, size);
-
- if (pkcs11_getattr_var(token, object, type, binary, &size)) {
- ret = -1;
- goto cleanup;
- }
-
/*
* @ALON: invalid object,
* not sure it will survive the ulValueLen->size_t and keep sign at all platforms
@@ -104,15 +98,12 @@ pkcs11_getattr_bn(PKCS11_TOKEN * token, CK_OBJECT_HANDLE object,
if (size == (size_t)-1) {
PKCS11err(PKCS11_F_PKCS11_GETATTR,
pkcs11_map_err(CKR_ATTRIBUTE_TYPE_INVALID));
- ret = -1;
- goto cleanup;
+ OPENSSL_free(binary);
+ return -1;
}
- *bn = BN_bin2bn(binary, (int) size, *bn);
- ret = *bn ? 0 : -1;
-
- cleanup:
+ *bn = BN_bin2bn(binary, (int)size, *bn);
OPENSSL_free(binary);
- return ret;
+ return *bn ? 0 : -1;
}
/*
@@ -146,7 +137,7 @@ void pkcs11_addattr_s(CK_ATTRIBUTE_PTR ap, int type, const char *s)
pkcs11_addattr(ap, type, s, s ? strlen(s) : 0); /* RFC2279 string an unpadded string of CK_UTF8CHARs with no null-termination */
}
-void pkcs11_addattr_bn(CK_ATTRIBUTE_PTR ap, int type, const BIGNUM * bn)
+void pkcs11_addattr_bn(CK_ATTRIBUTE_PTR ap, int type, const BIGNUM *bn)
{
unsigned char temp[1024];
unsigned int n;
diff --git a/src/p11_cert.c b/src/p11_cert.c
index a63fde3..b2336b7 100644
--- a/src/p11_cert.c
+++ b/src/p11_cert.c
@@ -140,9 +140,7 @@ static int pkcs11_init_cert(PKCS11_CTX * ctx, PKCS11_TOKEN * token,
PKCS11_TOKEN_private *tpriv;
PKCS11_CERT_private *cpriv;
PKCS11_CERT *cert, *tmp;
- char label[256];
unsigned char *data;
- unsigned char id[256];
CK_CERTIFICATE_TYPE cert_type;
size_t size;
@@ -150,7 +148,7 @@ static int pkcs11_init_cert(PKCS11_CTX * ctx, PKCS11_TOKEN * token,
(void)session;
size = sizeof(cert_type);
- if (pkcs11_getattr_var(token, obj, CKA_CERTIFICATE_TYPE, &cert_type, &size))
+ if (pkcs11_getattr_var(token, obj, CKA_CERTIFICATE_TYPE, (CK_BYTE *)&cert_type, &size))
return -1;
/* Ignore any certs we don't understand */
@@ -177,27 +175,16 @@ static int pkcs11_init_cert(PKCS11_CTX * ctx, PKCS11_TOKEN * token,
cpriv->object = obj;
cpriv->parent = token;
- if (!pkcs11_getattr_s(token, obj, CKA_LABEL, label, sizeof(label)))
- cert->label = OPENSSL_strdup(label);
+ pkcs11_getattr_alloc(token, obj, CKA_LABEL, (CK_BYTE **)&cert->label, NULL);
size = 0;
- if (!pkcs11_getattr_var(token, obj, CKA_VALUE, NULL, &size) && size > 0) {
- data = OPENSSL_malloc(size);
- if (data) {
- if (!pkcs11_getattr_var(token, obj, CKA_VALUE, data, &size)) {
- const unsigned char *p = data;
-
- cert->x509 = d2i_X509(NULL, &p, (long) size);
- }
- OPENSSL_free(data);
- }
- }
- cert->id_len = sizeof(id);
- if (!pkcs11_getattr_var(token, obj, CKA_ID, id, &cert->id_len)) {
- cert->id = OPENSSL_malloc(cert->id_len);
- if (cert->id == NULL)
- return -1;
- memcpy(cert->id, id, cert->id_len);
+ if (!pkcs11_getattr_alloc(token, obj, CKA_VALUE, &data, &size)) {
+ const unsigned char *p = data;
+
+ cert->x509 = d2i_X509(NULL, &p, (long)size);
+ OPENSSL_free(data);
}
+ cert->id_len = 0;
+ pkcs11_getattr_alloc(token, obj, CKA_ID, &cert->id, &cert->id_len);
/* Initialize internal information */
cpriv->id_len = sizeof(cpriv->id);
diff --git a/src/p11_ec.c b/src/p11_ec.c
index 29d1376..6a6baf7 100644
--- a/src/p11_ec.c
+++ b/src/p11_ec.c
@@ -76,6 +76,54 @@ static void free_ec_ex_index()
/********** EVP_PKEY retrieval */
+static EC_KEY *pkcs11_get_ec(PKCS11_KEY *key)
+{
+ EC_KEY *ec;
+ size_t params_len = 0;
+ CK_BYTE *params;
+ size_t point_len = 0;
+ CK_BYTE *point;
+ PKCS11_KEY *pubkey;
+
+ ec = EC_KEY_new();
+ if (ec == NULL)
+ return NULL;
+
+ /* For OpenSSL req we need at least the
+ * EC_KEY_get0_group(ec_key)) to return the group.
+ * Continue even if it fails, as the sign operation does not need
+ * it if the PKCS#11 module or the hardware can figure this out
+ */
+ if (!key_getattr_alloc(key, CKA_EC_PARAMS, ¶ms, ¶ms_len)) {
+ const unsigned char *a = params;
+
+ /* Convert to OpenSSL parmas */
+ d2i_ECParameters(&ec, &a, (long)params_len);
+ OPENSSL_free(params);
+ }
+
+ /* Now retrieve the point */
+ pubkey = key->isPrivate ? pkcs11_find_key_from_key(key) : key;
+ if (pubkey == NULL)
+ return ec;
+ if (!key_getattr_alloc(pubkey, CKA_EC_POINT, &point, &point_len)) {
+ const unsigned char *a;
+ ASN1_OCTET_STRING *os;
+
+ /* PKCS#11 returns ASN1_OCTET_STRING */
+ a = point;
+ os = d2i_ASN1_OCTET_STRING(NULL, &a, (long)point_len);
+ if (os) {
+ a = os->data;
+ o2i_ECPublicKey(&ec, &a, os->length);
+ ASN1_STRING_free(os);
+ }
+ OPENSSL_free(point);
+ }
+
+ return ec;
+}
+
/*
* Get EC key material and stash pointer in ex_data
* Note we get called twice, once for private key, and once for public
@@ -85,83 +133,21 @@ static void free_ec_ex_index()
* is not in the private key, and the params may or may not be.
*
*/
-static EVP_PKEY *pkcs11_get_evp_key_ec(PKCS11_KEY * key)
+static EVP_PKEY *pkcs11_get_evp_key_ec(PKCS11_KEY *key)
{
EVP_PKEY *pk;
- EC_KEY * ec = NULL;
- CK_RV ckrv;
- size_t ec_paramslen = 0;
- CK_BYTE * ec_params = NULL;
- size_t ec_pointlen = 0;
- CK_BYTE * ec_point = NULL;
- PKCS11_KEY * pubkey;
- ASN1_OCTET_STRING *os=NULL;
+ EC_KEY *ec;
- pk = EVP_PKEY_new();
- if (pk == NULL)
+ ec = pkcs11_get_ec(key);
+ if (ec == NULL)
return NULL;
-
- ec = EC_KEY_new();
- if (ec == NULL) {
- EVP_PKEY_free(pk);
+ pk = EVP_PKEY_new();
+ if (pk == NULL) {
+ EC_KEY_free(ec);
return NULL;
}
EVP_PKEY_set1_EC_KEY(pk, ec); /* Also increments the ec ref count */
- /* For Openssl req we need at least the
- * EC_KEY_get0_group(ec_key)) to return the group.
- * Even if it fails will continue as a sign only does not need
- * need this if the pkcs11 or card can figure this out.
- */
-
- if (key_getattr_var(key, CKA_EC_PARAMS, NULL, &ec_paramslen) == CKR_OK &&
- ec_paramslen > 0) {
- ec_params = OPENSSL_malloc(ec_paramslen);
- if (ec_params) {
- ckrv = key_getattr_var(key, CKA_EC_PARAMS, ec_params, &ec_paramslen);
- if (ckrv == CKR_OK) {
- const unsigned char * a = ec_params;
- /* convert to OpenSSL parmas */
- d2i_ECParameters(&ec, &a, (long) ec_paramslen);
- }
- }
- }
-
- /* Now get the ec_point */
- pubkey = key->isPrivate ? pkcs11_find_key_from_key(key) : key;
- if (pubkey) {
- ckrv = key_getattr_var(pubkey, CKA_EC_POINT, NULL, &ec_pointlen);
- if (ckrv == CKR_OK && ec_pointlen > 0) {
- ec_point = OPENSSL_malloc(ec_pointlen);
- if (ec_point) {
- ckrv = key_getattr_var(pubkey, CKA_EC_POINT, ec_point, &ec_pointlen);
- if (ckrv == CKR_OK) {
- /* PKCS#11 returns ASN1 octstring*/
- const unsigned char * a;
- /* we have asn1 octet string, need to strip off 04 len */
-
- a = ec_point;
- os = d2i_ASN1_OCTET_STRING(NULL, &a, (long) ec_pointlen);
- if (os) {
- a = os->data;
- o2i_ECPublicKey(&ec, &a, os->length);
- }
-/* EC_KEY_print_fp(stderr, ec, 5); */
- }
- }
- }
- }
-
- /* If the key is not extractable, create a key object
- * that will use the card's functions to sign & decrypt
- */
- if (os)
- ASN1_STRING_free(os);
- if (ec_point)
- OPENSSL_free(ec_point);
- if (ec_params)
- OPENSSL_free(ec_params);
-
if (key->isPrivate) {
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
EC_KEY_set_method(ec, PKCS11_get_ec_key_method());
@@ -331,9 +317,6 @@ static int pkcs11_ecdh_derive(unsigned char **out, size_t *outlen,
PKCS11_SLOT_private *spriv = PRIVSLOT(slot);
CK_MECHANISM mechanism;
int rv;
- int ret = -1;
- unsigned char *buf = NULL;
- size_t buflen;
CK_BBOOL true = TRUE;
CK_BBOOL false = FALSE;
@@ -364,56 +347,28 @@ static int pkcs11_ecdh_derive(unsigned char **out, size_t *outlen,
break;
#endif
default:
- PKCS11err(PKCS11_F_PKCS11_EC_KEY_COMPUTE_KEY, PKCS11_NOT_SUPPORTED);
- goto err;
+ PKCS11err(PKCS11_F_PKCS11_EC_KEY_COMPUTE_KEY, pkcs11_map_err(PKCS11_NOT_SUPPORTED));
+ return -1;
}
rv = CRYPTOKI_call(ctx, C_DeriveKey(spriv->session, &mechanism, kpriv->object, newkey_template, 5, &newkey));
- if (rv) {
- PKCS11err(PKCS11_F_PKCS11_EC_KEY_COMPUTE_KEY, pkcs11_map_err(rv));
- goto err;
- }
+ CRYPTOKI_checkerr(PKCS11_F_PKCS11_EC_KEY_COMPUTE_KEY, rv);
/* Return the value of the secret key and/or the object handle of the secret key */
-
- /* pkcs11_ec_ckey only asks for the value */
-
- if (out && outlen) {
- /* get size of secret key value */
- if (!pkcs11_getattr_var(token, newkey, CKA_VALUE, NULL, &buflen)
- && buflen > 0) {
- buf = OPENSSL_malloc(buflen);
- if (buf == NULL) {
- PKCS11err(PKCS11_F_PKCS11_EC_KEY_COMPUTE_KEY,
- pkcs11_map_err(CKR_HOST_MEMORY));
- goto err;
- }
- } else {
+ if (out && outlen) { /* pkcs11_ec_ckey only asks for the value */
+ if (pkcs11_getattr_alloc(token, newkey, CKA_VALUE, out, outlen)) {
PKCS11err(PKCS11_F_PKCS11_EC_KEY_COMPUTE_KEY,
pkcs11_map_err(CKR_ATTRIBUTE_VALUE_INVALID));
- goto err;
+ CRYPTOKI_call(ctx, C_DestroyObject(spriv->session, newkey));
+ return -1;
}
-
- pkcs11_getattr_var(token, newkey, CKA_VALUE, buf, &buflen);
- *out = buf;
- *outlen = buflen;
- buf = NULL;
}
-
- /* not used by pkcs11_ec_ckey for future use */
- if (tmpnewkey) {
+ if (tmpnewkey) /* For future use (not used by pkcs11_ec_ckey) */
*tmpnewkey = newkey;
- newkey = CK_INVALID_HANDLE;
- }
-
- ret = 1;
-err:
- if (buf)
- OPENSSL_free(buf);
- if (newkey != CK_INVALID_HANDLE && spriv->session != CK_INVALID_HANDLE)
+ else /* Destroy the temporary key */
CRYPTOKI_call(ctx, C_DestroyObject(spriv->session, newkey));
- return ret;
+ return 0;
}
/**
@@ -436,6 +391,7 @@ static int pkcs11_ec_ckey(void *out,
CK_ECDH1_DERIVE_PARAMS *parms;
unsigned char *buf = NULL;
size_t buflen;
+ int rv;
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
key = (PKCS11_KEY *)EC_KEY_get_ex_data(ecdh, ec_ex_index);
@@ -449,10 +405,11 @@ static int pkcs11_ec_ckey(void *out,
parms = pkcs11_ecdh_params_alloc(EC_KEY_get0_group(ecdh), peer_point);
if (parms == NULL)
return -1;
- if (pkcs11_ecdh_derive(&buf, &buflen, CKM_ECDH1_DERIVE,
- parms, NULL, key) < 0)
- return -1;
+ rv = pkcs11_ecdh_derive(&buf, &buflen, CKM_ECDH1_DERIVE,
+ parms, NULL, key);
pkcs11_ecdh_params_free(parms);
+ if (rv < 0)
+ return -1;
if (KDF) {
if (KDF(buf, buflen, out, &outlen) == NULL) {
diff --git a/src/p11_err.c b/src/p11_err.c
index 46a9f80..9e7a783 100644
--- a/src/p11_err.c
+++ b/src/p11_err.c
@@ -40,7 +40,7 @@ static ERR_STRING_DATA PKCS11_str_functs[] = {
{ERR_PACK(0, PKCS11_F_PKCS11_ENUM_CERTS, 0), "PKCS11_enum_certs"},
{ERR_PACK(0, PKCS11_F_PKCS11_INIT_TOKEN, 0), "PKCS11_init_token"},
{ERR_PACK(0, PKCS11_F_PKCS11_INIT_PIN, 0), "PKCS11_init_pin"},
- {ERR_PACK(0, PKCS11_F_PKCS11_GETATTR, 0), "PKCS11_get_attribute"},
+ {ERR_PACK(0, PKCS11_F_PKCS11_GETATTR, 0), "pkcs11_getattr"},
{ERR_PACK(0, PKCS11_F_PKCS11_LOGOUT, 0), "PKCS11_logout"},
{ERR_PACK(0, PKCS11_F_PKCS11_STORE_PRIVATE_KEY, 0), "PKCS11_store_private_key"},
{ERR_PACK(0, PKCS11_F_PKCS11_GENERATE_KEY, 0), "PKCS11_generate_key"},
diff --git a/src/p11_key.c b/src/p11_key.c
index d931ee6..610204e 100644
--- a/src/p11_key.c
+++ b/src/p11_key.c
@@ -394,8 +394,6 @@ static int pkcs11_init_key(PKCS11_CTX * ctx, PKCS11_TOKEN * token,
PKCS11_keys *keys = (type == CKO_PRIVATE_KEY) ? &tpriv->prv : &tpriv->pub;
PKCS11_KEY_private *kpriv;
PKCS11_KEY *key, *tmp;
- char label[256];
- unsigned char id[256];
CK_KEY_TYPE key_type;
PKCS11_KEY_ops *ops;
size_t size;
@@ -404,7 +402,7 @@ static int pkcs11_init_key(PKCS11_CTX * ctx, PKCS11_TOKEN * token,
(void)session;
size = sizeof(key_type);
- if (pkcs11_getattr_var(token, obj, CKA_KEY_TYPE, &key_type, &size))
+ if (pkcs11_getattr_var(token, obj, CKA_KEY_TYPE, (CK_BYTE *)&key_type, &size))
return -1;
switch (key_type) {
@@ -438,13 +436,9 @@ static int pkcs11_init_key(PKCS11_CTX * ctx, PKCS11_TOKEN * token,
kpriv->object = obj;
kpriv->parent = token;
- if (!pkcs11_getattr_s(token, obj, CKA_LABEL, label, sizeof(label)))
- key->label = OPENSSL_strdup(label);
- key->id_len = sizeof(id);
- if (!pkcs11_getattr_var(token, obj, CKA_ID, id, &key->id_len)) {
- key->id = OPENSSL_malloc(key->id_len);
- memcpy(key->id, id, key->id_len);
- }
+ pkcs11_getattr_alloc(token, obj, CKA_LABEL, (CK_BYTE **)&key->label, NULL);
+ key->id_len = 0;
+ pkcs11_getattr_alloc(token, obj, CKA_ID, &key->id, &key->id_len);
key->isPrivate = (type == CKO_PRIVATE_KEY);
/* Initialize internal information */
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-opensc/libp11.git
More information about the pkg-opensc-commit
mailing list