giomba
/
opensc
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Code cleanup and minor corrections according to review. pkcs15-lib: Extractable keys are now marked as native. Check return value of check_key_compatibility in more explicit way to avoid misunderstandings.

This commit is contained in:
Hannu Honkanen 2018-10-18 16:56:06 +03:00
parent 90ec7123ba
commit 7bb53423a1
8 changed files with 31 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/libopensc/card-myeid.c
View File

@ -1358,7 +1358,7 @@ static int myeid_unwrap_key(struct sc_card *card, const u8 *crgram, size_t crgra
}
else
{
if (crgram_len > MYEID_MAX_APDU_DATA_LEN)
if (crgram_len >= MYEID_MAX_APDU_DATA_LEN)
{
/* padding indicator byte, 0x81 = first half of 2048 bit cryptogram */
sbuf[0] = 0x81;
@ -1380,7 +1380,7 @@ static int myeid_unwrap_key(struct sc_card *card, const u8 *crgram, size_t crgra
LOG_TEST_RET(card->ctx, r, "APDU transmit failed");
if (apdu.sw1 == 0x90 && apdu.sw2 == 0x00)
{
if (crgram_len > MYEID_MAX_APDU_DATA_LEN)
if (crgram_len >= MYEID_MAX_APDU_DATA_LEN)
{
sc_format_apdu(card, &apdu, SC_APDU_CASE_3_SHORT,
0x2A, 0x00, p2);

2
src/libopensc/padding.c
View File

@ -505,7 +505,7 @@ int sc_get_encoding_flags(sc_context_t *ctx,
*sflags = SC_ALGORITHM_RSA_PAD_PKCS1 | SC_ALGORITHM_RSA_HASH_NONE;
*pflags = iflags & SC_ALGORITHM_RSA_HASHES;
} else if ((iflags & SC_ALGORITHM_AES) == SC_ALGORITHM_AES) { /* TODO: seems like this constant does not belong to the same set of flags used form asymmetric algos. Fix this! */
} else if ((iflags & SC_ALGORITHM_AES) == SC_ALGORITHM_AES) { /* TODO: seems like this constant does not belong to the same set of flags used form asymmetric algos. Fix this! */
*sflags = 0;
*pflags = 0;

19
src/libopensc/pkcs15-sec.c
View File

@ -534,8 +534,8 @@ int sc_pkcs15_wrap(struct sc_pkcs15_card *p15card,
senv.algorithm_flags = sec_flags;
if ((sec_flags & (SC_ALGORITHM_AES_CBC | SC_ALGORITHM_AES_CBC_PAD)) > 0) {
senv_param = (sc_sec_env_param_t) { SC_SEC_ENV_PARAM_IV, (void*) param, paramlen };
LOG_TEST_RET(ctx, sec_env_add_param(&senv, &senv_param), "failed to add IV to security environment");
senv_param = (sc_sec_env_param_t) { SC_SEC_ENV_PARAM_IV, (void*) param, paramlen };
LOG_TEST_RET(ctx, sec_env_add_param(&senv, &senv_param), "failed to add IV to security environment");
}
out = cryptogram;
@ -544,14 +544,13 @@ int sc_pkcs15_wrap(struct sc_pkcs15_card *p15card,
*poutlen);
if (r > -1) {
if (*crgram_len < (unsigned) r) {
*poutlen = r;
if (out != NULL) /* if NULL, return success and required buffer length by PKCS#11 convention */
LOG_TEST_RET(ctx, SC_ERROR_BUFFER_TOO_SMALL, "Buffer too small to hold the wrapped key.");
}
*poutlen = r;
}
if (*crgram_len < (unsigned) r) {
*poutlen = r;
if (out != NULL) /* if NULL, return success and required buffer length by PKCS#11 convention */
LOG_TEST_RET(ctx, SC_ERROR_BUFFER_TOO_SMALL, "Buffer too small to hold the wrapped key.");
}
*poutlen = r;
}
LOG_FUNC_RETURN(ctx, r);
}

6
src/libopensc/pkcs15-skey.c
View File

@ -152,10 +152,8 @@ sc_pkcs15_decode_skdf_entry(struct sc_pkcs15_card *p15card, struct sc_pkcs15_obj
if (asn1_skey_choice[0].flags & SC_ASN1_PRESENT) {
obj->type = SC_PKCS15_TYPE_SKEY_GENERIC;
/* info.key_type = CKK_AES;*/ /* defaults to CKK_AES. TODO: implement choice 15 and GenericKeyAttributes like defined in ISO7816-15 and set key type accordingly. */
/* Check key type. framework-pkcs15 recognizes one type per key, and AES is the only algorithm supported for
* SKEY_GENERIC type keys, so just check if this key is AES compatible. */
/* Check key type. framework-pkcs15 recognizes one type per key, and AES is the only algorithm supported for
* SKEY_GENERIC type keys, so just check if this key is AES compatible. */
for (i = 0; i < SC_MAX_SUPPORTED_ALGORITHMS && info.algo_refs[i] != 0; i++) {
for (ii = 0; ii < SC_MAX_SUPPORTED_ALGORITHMS && p15card->tokeninfo != 0; ii++) {

15
src/pkcs11/framework-pkcs15.c
View File

@ -131,11 +131,10 @@ struct pkcs15_skey_object {
struct sc_pkcs15_skey *valueXXXX;
};
#define is_secret_key(obj) ((__p15_type(obj) & SC_PKCS15_TYPE_CLASS_MASK) == SC_PKCS15_TYPE_SKEY)
#define is_skey(obj) ((__p15_type(obj) & SC_PKCS15_TYPE_CLASS_MASK) == SC_PKCS15_TYPE_SKEY)
#define skey_flags base.base.flags
#define skey_p15obj base.p15_object
#define is_skey(obj) ((__p15_type(obj) & SC_PKCS15_TYPE_CLASS_MASK) == SC_PKCS15_TYPE_SKEY_OBJECT)
extern struct sc_pkcs11_object_ops pkcs15_cert_ops;
extern struct sc_pkcs11_object_ops pkcs15_prkey_ops;
@ -1346,7 +1345,7 @@ _add_pin_related_objects(struct sc_pkcs11_slot *slot, struct sc_pkcs15_object *p
sc_log(context, "Slot:%p Adding cert object %d to PIN '%.*s'", slot, i, (int) sizeof pin_obj->label, pin_obj->label);
pkcs15_add_object(slot, obj, NULL);
}
else if (is_secret_key(obj)) {
else if (is_skey(obj)) {
sc_log(context, "Slot:%p Adding secret key object %d to PIN '%.*s'", slot, i, (int) sizeof pin_obj->label, pin_obj->label);
pkcs15_add_object(slot, obj, NULL);
}
@ -2361,7 +2360,7 @@ pkcs15_create_secret_key(struct sc_pkcs11_slot *slot, struct sc_profile *profile
if(_token == FALSE)
args.session_object = 1; /* store the object on card for duration of the session. */
args.value_len = args.value_len * 8; /* CKA_VALUE_LEN is number of bytes, PKCS#15 needs key length in bits */
args.value_len = args.value_len * 8; /* CKA_VALUE_LEN is number of bytes, PKCS#15 needs key length in bits */
rc = sc_pkcs15init_store_secret_key(fw_data->p15_card, profile, &args, &key_obj);
if (rc < 0) {
rv = sc_to_cryptoki_error(rc, "C_CreateObject");
@ -4768,15 +4767,15 @@ pkcs15_skey_get_attribute(struct sc_pkcs11_session *session,
break;
case CKA_TOKEN:
check_attribute_buffer(attr, sizeof(CK_BBOOL));
*(CK_BBOOL*)attr->pValue = skey->base.p15_object->session_object == 0;
*(CK_BBOOL*)attr->pValue = skey->base.p15_object->session_object == 0 ? CK_TRUE : CK_FALSE;
break;
case CKA_PRIVATE:
check_attribute_buffer(attr, sizeof(CK_BBOOL));
*(CK_BBOOL*)attr->pValue = (skey->base.p15_object->flags & SC_PKCS15_CO_FLAG_PRIVATE) != 0;
*(CK_BBOOL*)attr->pValue = (skey->base.p15_object->flags & SC_PKCS15_CO_FLAG_PRIVATE) != 0 ? CK_TRUE : CK_FALSE;
break;
case CKA_MODIFIABLE:
check_attribute_buffer(attr, sizeof(CK_BBOOL));
*(CK_BBOOL*)attr->pValue = (skey->base.p15_object->flags & 0x02) != 0;
*(CK_BBOOL*)attr->pValue = (skey->base.p15_object->flags & 0x02) != 0 ? CK_TRUE : CK_FALSE;
/*TODO Why no definition of the flag */
break;
case CKA_LABEL:
@ -4814,7 +4813,7 @@ pkcs15_skey_get_attribute(struct sc_pkcs11_session *session,
check_attribute_buffer(attr, sizeof(CK_BBOOL));
*(CK_BBOOL*)attr->pValue = (((skey->base.p15_object->flags & SC_PKCS15_PRKEY_ACCESS_EXTRACTABLE) == SC_PKCS15_PRKEY_ACCESS_EXTRACTABLE)
&& (skey->base.p15_object->flags & SC_PKCS15_PRKEY_ACCESS_NEVEREXTRACTABLE) == 0
&& (skey->base.p15_object->flags & SC_PKCS15_PRKEY_ACCESS_ALWAYSSENSITIVE) == 0);
&& (skey->base.p15_object->flags & SC_PKCS15_PRKEY_ACCESS_ALWAYSSENSITIVE) == 0) ? CK_TRUE : CK_FALSE;
break;
case CKA_VALUE_LEN:
check_attribute_buffer(attr, sizeof(CK_ULONG));

6
src/pkcs11/mechanism.c
View File

@ -844,8 +844,7 @@ sc_pkcs11_wrap(struct sc_pkcs11_session *session,
sc_pkcs11_mechanism_type_t *mt;
CK_RV rv;
if (!session || !session->slot
|| !(p11card = session->slot->p11card))
if (!session || !session->slot || !(p11card = session->slot->p11card))
return CKR_ARGUMENTS_BAD;
/* See if we support this mechanism type */
@ -891,8 +890,7 @@ sc_pkcs11_unwrap(struct sc_pkcs11_session *session,
CK_RV rv;
if (!session || !session->slot
|| !(p11card = session->slot->p11card))
if (!session || !session->slot || !(p11card = session->slot->p11card))
return CKR_ARGUMENTS_BAD;
/* See if we support this mechanism type */

2
src/pkcs11/pkcs11-object.c
View File

@ -1041,7 +1041,7 @@ CK_RV C_WrapKey(CK_SESSION_HANDLE hSession, /* the session's handle */
{ /* receives byte size of wrapped key */
CK_RV rv;
CK_BBOOL can_wrap,
can_be_wrapped;
can_be_wrapped;
CK_KEY_TYPE key_type;
CK_ATTRIBUTE wrap_attribute = { CKA_WRAP, &can_wrap, sizeof(can_wrap) };
CK_ATTRIBUTE extractable_attribute = { CKA_EXTRACTABLE, &can_be_wrapped, sizeof(can_be_wrapped) };

20
src/pkcs15init/pkcs15-lib.c
View File

@ -1215,7 +1215,6 @@ sc_pkcs15init_init_prkdf(struct sc_pkcs15_card *p15card, struct sc_profile *prof
if (keyargs->access_flags & SC_PKCS15_PRKEY_ACCESS_EXTRACTABLE) {
key_info->access_flags &= ~SC_PKCS15_PRKEY_ACCESS_NEVEREXTRACTABLE;
key_info->native = 0;
}
/* Select a Key ID if the user didn't specify one,
@ -1348,10 +1347,6 @@ sc_pkcs15init_init_skdf(struct sc_pkcs15_card *p15card, struct sc_profile *profi
if (keyargs->access_flags & SC_PKCS15_PRKEY_ACCESS_EXTRACTABLE) {
key_info->access_flags &= ~SC_PKCS15_PRKEY_ACCESS_NEVEREXTRACTABLE;
/* The following commented out. I interpret PKCS#15 so that native only means that a key
* can be used in on card crypto. Such key can be extractable (can be wrapped), so IMO this
logic is not correct. */
/* key_info->native = 0; */
}
if (keyargs->session_object > 0)
@ -1461,7 +1456,7 @@ sc_pkcs15init_generate_key(struct sc_pkcs15_card *p15card, struct sc_profile *pr
if (check_key_compatibility(p15card, keygen_args->prkey_args.key.algorithm,
&keygen_args->prkey_args.key, keygen_args->prkey_args.x509_usage,
keybits, SC_ALGORITHM_ONBOARD_KEY_GEN))
keybits, SC_ALGORITHM_ONBOARD_KEY_GEN) != SC_SUCCESS)
LOG_TEST_RET(ctx, SC_ERROR_NOT_SUPPORTED, "Cannot generate key with the given parameters");
if (profile->ops->generate_key == NULL)
@ -1585,7 +1580,7 @@ sc_pkcs15init_generate_secret_key(struct sc_pkcs15_card *p15card, struct sc_prof
LOG_TEST_RET(ctx, r, "Invalid key size");
if (check_key_compatibility(p15card, skey_args->algorithm, NULL, 0,
keybits, SC_ALGORITHM_ONBOARD_KEY_GEN))
keybits, SC_ALGORITHM_ONBOARD_KEY_GEN) != SC_SUCCESS)
LOG_TEST_RET(ctx, SC_ERROR_NOT_SUPPORTED, "Cannot generate key with the given parameters");
if (profile->ops->generate_key == NULL)
@ -1654,11 +1649,11 @@ sc_pkcs15init_store_private_key(struct sc_pkcs15_card *p15card, struct sc_profil
LOG_TEST_RET(ctx, keybits, "Invalid private key size");
/* Now check whether the card is able to handle this key */
if (check_key_compatibility(p15card, key.algorithm, &key, keyargs->x509_usage, keybits, 0)) {
if (check_key_compatibility(p15card, key.algorithm, &key, keyargs->x509_usage, keybits, 0) != SC_SUCCESS) {
/* Make sure the caller explicitly tells us to store
* the key as extractable. */
if (!(keyargs->access_flags & SC_PKCS15_PRKEY_ACCESS_EXTRACTABLE))
LOG_TEST_RET(ctx, SC_ERROR_INCOMPATIBLE_KEY, "Card does not support this key.");
LOG_TEST_RET(ctx, SC_ERROR_INCOMPATIBLE_KEY, "Card does not support this key for crypto. Cannot store it as non extractable.");
}
/* Select a intrinsic Key ID if user didn't specify one */
@ -1887,12 +1882,11 @@ sc_pkcs15init_store_secret_key(struct sc_pkcs15_card *p15card, struct sc_profile
LOG_FUNC_CALLED(ctx);
/* Now check whether the card is able to handle this key */
if (check_key_compatibility(p15card, keyargs->algorithm, NULL, 0, keyargs->key.data_len * 8, 0)) {
if (check_key_compatibility(p15card, keyargs->algorithm, NULL, 0, keyargs->key.data_len * 8, 0) != SC_SUCCESS) {
/* Make sure the caller explicitly tells us to store
* the key as extractable. */
/* Commented out. I don't understand why one couldn't store a key as non extractable. -HH */
/* if (!(keyargs->access_flags & SC_PKCS15_PRKEY_ACCESS_EXTRACTABLE))
LOG_TEST_RET(ctx, SC_ERROR_INCOMPATIBLE_KEY, "Card does not support this key.");*/
if (!(keyargs->access_flags & SC_PKCS15_PRKEY_ACCESS_EXTRACTABLE))
LOG_TEST_RET(ctx, SC_ERROR_INCOMPATIBLE_KEY, "Card does not support this key for crypto. Cannot store it as non extractable.");
}
#ifdef ENABLE_OPENSSL