Skip to content

Commit 37f79c3

Browse files
bob-beckbob-beck
committed
Use accessors for ASN1_STRING values
OpenSSL is going to make ASN1_STRING opaque. The accessors in question have been around in OpenSSL for a very long time. (see openssl/openssl#29117)
1 parent 4e8bf09 commit 37f79c3

File tree

3 files changed

+74
-41
lines changed

3 files changed

+74
-41
lines changed

‎src/encoder.c‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -451,8 +451,8 @@ static int p11prov_print_pkeyinfo(CK_ATTRIBUTE *pkeyinfo, BIO *out)
451451
}
452452
} else if (ptype == V_ASN1_SEQUENCE) {
453453
const ASN1_STRING *pstr = pval;
454-
const unsigned char *pm = pstr->data;
455-
int pmlen = pstr->length;
454+
const unsigned char *pm = ASN1_STRING_get0_data(pstr);
455+
int pmlen = ASN1_STRING_length(pstr);
456456
EC_GROUP *group;
457457

458458
group = d2i_ECPKParameters(NULL, &pm, pmlen);
@@ -1054,6 +1054,7 @@ static int p11prov_ec_set_keypoint_data(const OSSL_PARAM *params, void *key)
10541054
keypoint->curve_type = V_ASN1_OBJECT;
10551055
} else {
10561056
EC_GROUP *group = EC_GROUP_new_from_params(params, NULL, NULL);
1057+
int len = 0;
10571058
if (!group) {
10581059
return RET_OSSL_ERR;
10591060
}
@@ -1063,12 +1064,14 @@ static int p11prov_ec_set_keypoint_data(const OSSL_PARAM *params, void *key)
10631064
EC_GROUP_free(group);
10641065
return RET_OSSL_ERR;
10651066
}
1066-
pstr->length = i2d_ECPKParameters(group, &pstr->data);
1067+
unsigned char *buf = NULL;
1068+
len = i2d_ECPKParameters(group, &buf);
10671069
EC_GROUP_free(group);
1068-
if (pstr->length <= 0) {
1069-
ASN1_STRING_free(pstr);
1070+
if (len <= 0) {
1071+
OPENSSL_free(buf);
10701072
return RET_OSSL_ERR;
10711073
}
1074+
ASN1_STRING_set0(pstr, buf, len);
10721075
keypoint->curve.sequence = pstr;
10731076
keypoint->curve_type = V_ASN1_SEQUENCE;
10741077
}

‎src/obj/keymgmt.c‎

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -332,12 +332,14 @@ CK_RV decode_ec_point(P11PROV_CTX *provctx, CK_KEY_TYPE key_type,
332332
}
333333
}
334334

335-
ec_point->data = octet->data;
336-
ec_point->length = octet->length;
337-
338-
/* moved octet data, do not free it */
339-
octet->data = NULL;
340-
octet->length = 0;
335+
ec_point->data =
336+
OPENSSL_memdup(ASN1_STRING_get0_data(octet), ASN1_STRING_length(octet));
337+
if (!ec_point->data) {
338+
ret = CKR_HOST_MEMORY;
339+
ec_point->length = 0;
340+
goto done;
341+
}
342+
ec_point->length = ASN1_STRING_length(octet);
341343

342344
ret = CKR_OK;
343345
done:
@@ -410,11 +412,11 @@ CK_RV p11prov_obj_set_ec_encoded_public_key(P11PROV_OBJ *key,
410412
const void *pubkey,
411413
size_t pubkey_len)
412414
{
413-
CK_RV rv;
415+
CK_RV rv = CKR_GENERAL_ERROR;
414416
CK_ATTRIBUTE *pub;
415417
CK_ATTRIBUTE *ecpoint;
416418
CK_ATTRIBUTE new_pub;
417-
ASN1_OCTET_STRING oct;
419+
ASN1_OCTET_STRING *oct = NULL;
418420
unsigned char *der = NULL;
419421
int add_attrs = 0;
420422
int len;
@@ -427,7 +429,8 @@ CK_RV p11prov_obj_set_ec_encoded_public_key(P11PROV_OBJ *key,
427429
/* not matching, error out */
428430
P11PROV_raise(key->ctx, CKR_KEY_INDIGESTIBLE,
429431
"Cannot change public key of a token object");
430-
return CKR_KEY_INDIGESTIBLE;
432+
rv = CKR_KEY_INDIGESTIBLE;
433+
goto done;
431434
}
432435

433436
switch (key->data.key.type) {
@@ -440,15 +443,17 @@ CK_RV p11prov_obj_set_ec_encoded_public_key(P11PROV_OBJ *key,
440443
/* check that this is a public key */
441444
P11PROV_raise(key->ctx, CKR_KEY_INDIGESTIBLE,
442445
"Invalid Key type, not a public key");
443-
return CKR_KEY_INDIGESTIBLE;
446+
rv = CKR_KEY_INDIGESTIBLE;
447+
goto done;
444448
}
445449
break;
446450
case CKK_EC_MONTGOMERY:
447451
break;
448452
default:
449453
P11PROV_raise(key->ctx, CKR_KEY_INDIGESTIBLE,
450454
"Invalid Key type, not an EC/ED key");
451-
return CKR_KEY_INDIGESTIBLE;
455+
rv = CKR_KEY_INDIGESTIBLE;
456+
goto done;
452457
}
453458

454459
pub = p11prov_obj_get_attr(key, CKA_P11PROV_PUB_KEY);
@@ -467,7 +472,8 @@ CK_RV p11prov_obj_set_ec_encoded_public_key(P11PROV_OBJ *key,
467472
if (!ptr) {
468473
P11PROV_raise(key->ctx, CKR_HOST_MEMORY,
469474
"Failed to store key public key");
470-
return CKR_HOST_MEMORY;
475+
rv = CKR_HOST_MEMORY;
476+
goto done;
471477
}
472478
key->attrs = ptr;
473479
}
@@ -495,24 +501,37 @@ CK_RV p11prov_obj_set_ec_encoded_public_key(P11PROV_OBJ *key,
495501
new_pub.ulValueLen = (CK_ULONG)pubkey_len;
496502
rv = p11prov_copy_attr(pub, &new_pub);
497503
if (rv != CKR_OK) {
498-
return rv;
504+
goto done;
499505
}
500506

501-
oct.data = (unsigned char *)pubkey;
502-
oct.length = (int)pubkey_len;
503-
oct.flags = 0;
504-
505-
len = i2d_ASN1_OCTET_STRING(&oct, &der);
507+
oct = ASN1_OCTET_STRING_new();
508+
if (!oct) {
509+
rv = CKR_HOST_MEMORY;
510+
goto done;
511+
}
512+
if (!ASN1_STRING_set(oct, pubkey, pubkey_len)) {
513+
rv = CKR_HOST_MEMORY;
514+
goto done;
515+
}
516+
len = i2d_ASN1_OCTET_STRING(oct, &der);
506517
if (len < 0) {
507518
P11PROV_raise(key->ctx, CKR_KEY_INDIGESTIBLE,
508519
"Failure to encode EC point to DER");
509-
return CKR_KEY_INDIGESTIBLE;
520+
rv = CKR_KEY_INDIGESTIBLE;
521+
goto done;
510522
}
511523
ecpoint->type = CKA_EC_POINT;
512524
ecpoint->pValue = der;
525+
der = NULL;
513526
ecpoint->ulValueLen = len;
514527

515-
return CKR_OK;
528+
rv = CKR_OK;
529+
530+
done:
531+
ASN1_OCTET_STRING_free(oct);
532+
OPENSSL_free(der);
533+
534+
return rv;
516535
}
517536

518537
static int cmp_bn_attr(P11PROV_OBJ *key1, P11PROV_OBJ *key2,

‎src/obj/store.c‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -703,43 +703,54 @@ CK_RV p11prov_obj_copy_key_data(P11PROV_OBJ *dst, P11PROV_OBJ *src)
703703

704704
static CK_RV fix_ec_key_import(P11PROV_OBJ *key, int allocattrs)
705705
{
706+
CK_RV ret = CKR_GENERAL_ERROR;
706707
CK_ATTRIBUTE *pub;
707-
ASN1_OCTET_STRING oct;
708+
ASN1_OCTET_STRING *oct = NULL;
708709
unsigned char *der = NULL;
709710
int len;
710711

711712
if (key->numattrs >= allocattrs) {
712-
P11PROV_raise(key->ctx, CKR_GENERAL_ERROR,
713-
"Too many attributes?? %d >= %d", key->numattrs,
714-
allocattrs);
715-
return CKR_GENERAL_ERROR;
713+
P11PROV_raise(key->ctx, ret, "Too many attributes?? %d >= %d",
714+
key->numattrs, allocattrs);
715+
goto done;
716716
}
717717

718718
pub = p11prov_obj_get_attr(key, CKA_P11PROV_PUB_KEY);
719719
if (!pub) {
720-
P11PROV_raise(key->ctx, CKR_KEY_INDIGESTIBLE, "No public key found");
721-
return CKR_KEY_INDIGESTIBLE;
720+
ret = CKR_KEY_INDIGESTIBLE;
721+
P11PROV_raise(key->ctx, ret, "No public key found");
722+
goto done;
722723
}
723724

724-
oct.data = pub->pValue;
725-
oct.length = pub->ulValueLen;
726-
oct.flags = 0;
727-
725+
oct = ASN1_OCTET_STRING_new();
726+
if (!oct) {
727+
goto done;
728+
}
729+
if (!ASN1_STRING_set(oct, pub->pValue, pub->ulValueLen)) {
730+
goto done;
731+
}
728732
/* FIXME: should we set just bytes for Edwards and Montgomery keys? */
729-
len = i2d_ASN1_OCTET_STRING(&oct, &der);
733+
len = i2d_ASN1_OCTET_STRING(oct, &der);
730734
if (len < 0) {
731-
P11PROV_raise(key->ctx, CKR_KEY_INDIGESTIBLE,
732-
"Failure to encode EC point to DER");
733-
return CKR_KEY_INDIGESTIBLE;
735+
ret = CKR_KEY_INDIGESTIBLE;
736+
P11PROV_raise(key->ctx, ret, "Failure to encode EC point to DER");
737+
goto done;
734738
}
735739
key->attrs[key->numattrs].type = CKA_EC_POINT;
736740
key->attrs[key->numattrs].pValue = der;
741+
der = NULL;
737742
key->attrs[key->numattrs].ulValueLen = len;
738743
key->numattrs++;
739744

740745
P11PROV_debug("fixing EC key %p import", key);
741746

742-
return CKR_OK;
747+
ret = CKR_OK;
748+
749+
done:
750+
OPENSSL_free(der);
751+
ASN1_OCTET_STRING_free(oct);
752+
753+
return ret;
743754
}
744755

745756
static CK_RV p11prov_obj_import_public_key(P11PROV_OBJ *key,

0 commit comments

Comments
 (0)