support public key extraction for libraries with non-compliant `CKA_EC_POINT` implementations by keldonin · Pull Request #65 · Mastercard/pkcs11-tools

keldonin · 2025-06-25T09:12:44Z

support public key extraction for libraries with non-compliant CKA_EC_POINT implementations (with no OCTET STRING encapsulation)

There are PKCS#11 libraries that do not encapsulate CKA_EC_POINT properly.

According to the spec, a EC public key is a point encapsulated inside an OCTET STRING. It means the resulting format is, for uncompressed representation, something like this:

04 [LEN] 04 {X-coord in hex} {Y-coord in hex}

Unfortunately, there are PKCS#11 libraries not following the spec, and not encapsulating properly within a OCTET STRING:

04 {X-coord in hex} {Y-coord in hex}

This patch is detecting this condition and tries another way to decode the public key.
This affects p11cat, p11more, p11req and p11mkcert.

Note that if the CKA_EC_POINT can't be decoded, a warning is printed on stderr:

Warning: CKA_EC_POINT format likely not compliant, trying alternate way to decode public key

…C_POINT` implementations (with no OCTET STRING encapsulation)

Copilot

Pull Request Overview

This PR adds support for extracting public keys from libraries that do not encapsulate the CKA_EC_POINT attribute within an OCTET STRING as specified by the standard. It introduces an alternate decoding path and prints a warning if the conventional decoding fails.

Added alternate decoding logic for the CKA_EC_POINT attribute in multiple modules.
Updated warning messages and corresponding error handling.
Recorded the change in the CHANGELOG.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
lib/pkcs11_more.c	Added alternate logic for EC point decoding with a warning message
lib/pkcs11_cert_common.c	Introduced alternate decoding with a warning message and error recovery
lib/pkcs11_cat.c	Implemented alternate EC point decoding and warning on decoding failure
CHANGELOG.md	Documented the new behavior regarding non-compliant CKA_EC_POINT implementations

Copilot · 2025-06-25T12:34:37Z

+				    fprintf(stderr, "Warning: CKA_EC_POINT format likely not compliant, trying alternate way to decode public key\n");
+				    /* d2i_TYPE() will NULLify the destination pointer in case of error (??!) */
+				    /* we need to reset the value */
+				    if( (ec_point_container=ASN1_OCTET_STRING_new()) == NULL ) {
+					P_ERR();
+					goto key_ec_error;
+				    }
+
+				    if(ASN1_OCTET_STRING_set(ec_point_container, oecpoint->pValue, oecpoint->ulValueLen) == 0) {
+					P_ERR();
+					goto key_ec_error;
+				    }


[nitpick] Noticing that similar alternate decoding logic is repeated across multiple files; consider refactoring this logic into a shared helper function to improve maintainability.

Suggested change

fprintf(stderr, "Warning: CKA_EC_POINT format likely not compliant, trying alternate way to decode public key\n");

/* d2i_TYPE() will NULLify the destination pointer in case of error (??!) */

/* we need to reset the value */

if( (ec_point_container=ASN1_OCTET_STRING_new()) == NULL ) {

P_ERR();

goto key_ec_error;

}

if(ASN1_OCTET_STRING_set(ec_point_container, oecpoint->pValue, oecpoint->ulValueLen) == 0) {

P_ERR();

goto key_ec_error;

}

ec_point_container = decode_asn1_octet_string(oecpoint->pValue, oecpoint->ulValueLen);

if (ec_point_container == NULL) {

P_ERR();

goto key_ec_error;

}

Copilot · 2025-06-25T12:34:37Z

-	P_ERR();
-	goto err;
+	/* P_ERR(); */
+	fprintf(stderr, "Warning: CKA_EC_POINT format likely not compliant, trying alternate way to decode public key\n");


[nitpick] The alternate decoding code here is duplicated in other modules; refactoring it into a common function could reduce redundancy and ease future maintenance.

Copilot · 2025-06-25T12:34:37Z

 		    pp = oecpoint->pValue; /* copy the pointer */
 		    if(d2i_ASN1_OCTET_STRING(&ec_point_container, &pp, oecpoint->ulValueLen) == NULL ) {
-			P_ERR();
-			goto key_ec_error;
+			fprintf(stderr, "Warning: CKA_EC_POINT format likely not compliant, trying alternate way to decode public key\n");
+			/* d2i_TYPE() will NULLify the destination pointer in case of error (??!) */
+			/* we need to reset the value */
+			if( (ec_point_container=ASN1_OCTET_STRING_new()) == NULL ) {
+			    P_ERR();
+			    goto key_ec_error;
+			}
+
+			if(ASN1_OCTET_STRING_set(ec_point_container, oecpoint->pValue, oecpoint->ulValueLen) == 0) {
+			    P_ERR();
+			    goto key_ec_error;
+			}


[nitpick] Similar to the other files, this repeated decoding fallback logic might benefit from being refactored into a central helper function for better code reuse.

Suggested change

pp = oecpoint->pValue; /* copy the pointer */

if(d2i_ASN1_OCTET_STRING(&ec_point_container, &pp, oecpoint->ulValueLen) == NULL ) {

P_ERR();

goto key_ec_error;

fprintf(stderr, "Warning: CKA_EC_POINT format likely not compliant, trying alternate way to decode public key\n");

/* d2i_TYPE() will NULLify the destination pointer in case of error (??!) */

/* we need to reset the value */

if( (ec_point_container=ASN1_OCTET_STRING_new()) == NULL ) {

P_ERR();

goto key_ec_error;

}

if(ASN1_OCTET_STRING_set(ec_point_container, oecpoint->pValue, oecpoint->ulValueLen) == 0) {

P_ERR();

goto key_ec_error;

}

ec_point_container = decode_CKA_EC_POINT(oecpoint->pValue, oecpoint->ulValueLen);

if (ec_point_container == NULL) {

goto key_ec_error;

covertmatthew · 2025-06-25T16:42:08Z

@keldonin any interest in abstracting into common code? Maybe something like this...

void fallback_decode_ec_point(ASN1_OCTET_STRING **ec_point_container, const CK_BYTE *data, CK_ULONG len) {
    fprintf(stderr, "Warning: CKA_EC_POINT format likely not compliant, trying alternate way to decode public key\n");

    *ec_point_container = ASN1_OCTET_STRING_new();
    if (*ec_point_container == NULL) {
        P_ERR();
        return;
    }

    if (ASN1_OCTET_STRING_set(*ec_point_container, data, len) == 0) {
        P_ERR();
        ASN1_OCTET_STRING_free(*ec_point_container);
        *ec_point_container = NULL;
    }
}

called via...

fallback_decode_ec_point(&ec_point_container, oecpoint->pValue, oecpoint->ulValueLen);
if (ec_point_container == NULL) goto key_ec_error;

Probably out of scope for this PR, but something to consider.

covertmatthew · 2025-06-25T19:22:13Z

@@ -548,8 +548,19 @@ EVP_PKEY *pkcs11_SPKI_from_EC(pkcs11AttrList *attrlist )
    ptr = attr->pValue; /* copy the pointer, check OpenSSL d2i & i2d API doc for details */

    if(d2i_ASN1_OCTET_STRING(&ec_point_container, &ptr, attr->ulValueLen) == NULL ) {


Same comment as in pkcs11_cat

covertmatthew · 2025-06-25T19:22:34Z

@@ -378,8 +378,18 @@ func_rc pkcs11_more_object_with_label(pkcs11Context *p11Context, char *label)
 				/* openssl pattern: &pp will be incremented beyond size of DER struct */
 				pp = oecpoint->pValue; /* copy the pointer */
 				if(d2i_ASN1_OCTET_STRING(&ec_point_container, &pp, oecpoint->ulValueLen) == NULL ) {


Same comment as in pkcs11_cat

jake32321 · 2025-06-30T15:33:02Z

I don't have anything additional to add other than what @covertmatthew has already mentioned. Once that feedback is addressed, I am good to go. Apologies for the delay!

covertmatthew · 2025-08-01T17:40:38Z

@keldonin @jake32321 I've removed the duplicative creation of ec_point_container. I requested the original changes, so I'll let one of you be the arbiter of approval and hit the merge button.

Compiled and spot tested:

with_luna p11keygen -k rsa -b 2048 -i signing-key CKA_SIGN=true CKA_VERIFY=true

with_luna p11cat pubk/signing-key
-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEApJxxSxc+qbcYfteWDgnX
JYD0XtpOD93G60d4SaNFbNCx7mbT4P54aBc793a49E5CIBu9JDYoR4x3wwQvwiub
Jd5ZnhBpkbHo0MCuFzffMSWdfVoEgUyg1NHp0Xie83T0fMadFnbosqUeiWlQNpux
NHN/f6irGWdcA84mdZu5TlGco52L8wkjliQE2d4twQejcznfHB/CNO3cQbzdZCb/
zrwBFB5tjYCv2XlEhbgdA1SHjLkrqEV+l9QDYmKqtfxFfDzb4PIxirBUKxvd+6KX
utEU/JczOGxvx4qjKB5UaMhqMvYKCqQA0nzi1CDb6U1gDnF60tSh+t0IA5yjogko
bwIDAQAB
-----END PUBLIC KEY-----

with_luna p11importcert -f ./root_ca_cert.pem -i rootca
PEM format detected
p11importcert: importing certificate succeeded.

with_luna p11more pubk/signing-key
RSA Public-Key: (2048 bit)
Modulus:
    00:a4:9c:71:4b:17:3e:a9:b7:18:7e:d7:96:0e:09:
    d7:25:80:f4:5e:da:4e:0f:dd:c6:eb:47:78:49:a3:
    45:6c:d0:b1:ee:66:d3:e0:fe:78:68:17:3b:f7:76:
    b8:f4:4e:42:20:1b:bd:24:36:28:47:8c:77:c3:04:
    2f:c2:2b:9b:25:de:59:9e:10:69:91:b1:e8:d0:c0:
    ae:17:37:df:31:25:9d:7d:5a:04:81:4c:a0:d4:d1:
    e9:d1:78:9e:f3:74:f4:7c:c6:9d:16:76:e8:b2:a5:
    1e:89:69:50:36:9b:b1:34:73:7f:7f:a8:ab:19:67:
    5c:03:ce:26:75:9b:b9:4e:51:9c:a3:9d:8b:f3:09:
    23:96:24:04:d9:de:2d:c1:07:a3:73:39:df:1c:1f:
    c2:34:ed:dc:41:bc:dd:64:26:ff:ce:bc:01:14:1e:
    6d:8d:80:af:d9:79:44:85:b8:1d:03:54:87:8c:b9:
    2b:a8:45:7e:97:d4:03:62:62:aa:b5:fc:45:7c:3c:
    db:e0:f2:31:8a:b0:54:2b:1b:dd:fb:a2:97:ba:d1:
    14:fc:97:33:38:6c:6f:c7:8a:a3:28:1e:54:68:c8:
    6a:32:f6:0a:0a:a4:00:d2:7c:e2:d4:20:db:e9:4d:
    60:0e:71:7a:d2:d4:a1:fa:dd:08:03:9c:a3:a2:09:
    28:6f
Exponent: 65537 (0x10001)

keldonin · 2025-08-04T10:36:22Z

@covertmatthew I checked on the problematic library, it seems to work:

$ with_beid p11more pubk/Authentication                                                                                          127
Warning: CKA_EC_POINT format likely not compliant, trying alternate way to decode public key
Public-Key: (384 bit)
pub:
    04:96:70:20:86:18:59:30:6a:c6:85:d3:92:c4:8f:
    d4:cf:9d:bf:d4:ec:7e:5b:60:dc:f0:4c:62:a8:f7:
    3b:41:63:50:4a:9d:d0:3b:8b:34:b0:cf:f2:1c:40:
    46:4c:ea:e8:13:83:c2:5a:b7:b5:a2:be:fc:78:bc:
    df:2b:4b:72:41:d1:f0:8a:d8:d4:c9:1c:6c:d6:f2:
    81:6e:a0:46:02:e5:9a:d4:34:52:9b:e8:e5:62:93:
    33:f1:ba:63:ae:e2:7e
ASN1 OID: secp384r1
NIST CURVE: P-384

LGTM

support public key extraction for libraries with non-compliant `CKA_E…

a1e4480

…C_POINT` implementations (with no OCTET STRING encapsulation)

keldonin added the enhancement New feature or request label Jun 25, 2025

keldonin assigned keldonin, covertmatthew and jake32321 and unassigned keldonin Jun 25, 2025

keldonin requested a review from Copilot June 25, 2025 12:33

Copilot AI reviewed Jun 25, 2025

View reviewed changes

covertmatthew requested changes Jun 25, 2025

View reviewed changes

Removing duplicative ec_point_container creation

d746425

covertmatthew self-requested a review August 1, 2025 17:40

covertmatthew approved these changes Aug 1, 2025

View reviewed changes

keldonin merged commit 7982b59 into Mastercard:master Aug 4, 2025
2 checks passed

keldonin deleted the support_noncompliant_ec_point_format branch August 4, 2025 10:37

Navigation Menu

Uh oh!

support public key extraction for libraries with non-compliant `CKA_EC_POINT` implementations - #65

support public key extraction for libraries with non-compliant `CKA_EC_POINT` implementations#65
keldonin merged 2 commits into
Mastercard:masterfrom
keldonin:support_noncompliant_ec_point_format

keldonin commented Jun 25, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jun 25, 2025

Uh oh!

Copilot AI Jun 25, 2025

Uh oh!

Copilot AI Jun 25, 2025

Uh oh!

covertmatthew commented Jun 25, 2025 •

edited

Loading

Uh oh!

Uh oh!

covertmatthew Jun 25, 2025

Uh oh!

covertmatthew Jun 25, 2025

Uh oh!

jake32321 commented Jun 30, 2025 •

edited

Loading

Uh oh!

covertmatthew commented Aug 1, 2025 •

edited

Loading

Uh oh!

keldonin commented Aug 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@@ -548,8 +548,19 @@ EVP_PKEY pkcs11_SPKI_from_EC(pkcs11AttrList attrlist )
		ptr = attr->pValue; /* copy the pointer, check OpenSSL d2i & i2d API doc for details */

		if(d2i_ASN1_OCTET_STRING(&ec_point_container, &ptr, attr->ulValueLen) == NULL ) {

Uh oh!

Conversation

keldonin commented Jun 25, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

covertmatthew commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

covertmatthew Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

covertmatthew Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

jake32321 commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

covertmatthew commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keldonin commented Aug 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

covertmatthew commented Jun 25, 2025 •

edited

Loading

jake32321 commented Jun 30, 2025 •

edited

Loading

covertmatthew commented Aug 1, 2025 •

edited

Loading