Added helper to initialize EC key using affine coordinates by igus68 · Pull Request #30597 · openssl/openssl

igus68 · 2026-03-27T08:40:55Z

This PR implements an alternative approach to providing initialisation of an EC public key from its affine coordinates:
it adds the function which converts the affine coordinates of an EC point to an octet string conforming to Sec. 2.3.4 of the SECG SEC 1 ("Elliptic Curve Cryptography") standard. This octet string can further be passed to the OSSL_PARAM_BLD_push_octet_string() function.

Fixes #16270

Additionally, it fixes the null pointer dereference in the EVP_EC_gen macro and adds explicit handling of the situation when NULL is passed to EVP_PKEY_Q_keygen() instead of the curve name.

Checklist

documentation is added or updated
tests are added or updated

t8m · 2026-03-27T09:17:44Z

+int EVP_EC_affine2octet_string(const BIGNUM *X, const BIGNUM *Y,
+    int field_len, unsigned char **pbuf, size_t *pbsize);
+


Please move this to evp.h

And I would move the EVP_EC_gen() there as well.

t8m · 2026-03-27T09:18:32Z

    return ret;
 }

+int EVP_EC_affine2octet_string(const BIGNUM *X, const BIGNUM *Y,


Nit: please use lowercase for x and y.

t8m · 2026-03-27T09:23:15Z

+ int EVP_EC_affine2octet_string(const BIGNUM *X, const BIGNUM *Y, int field_len,
+                                unsigned char **pbuf, size_t *pbsize);


I would move this and EVP_EC_gen() to a separate manual page.

t8m · 2026-04-03T13:52:21Z

    return len;
 }
+
+int EVP_EC_affine2oct(const BIGNUM *x, const BIGNUM *y, int field_len,


You will need to move this into another file (a new one?) in crypto/evp

crypto/evp/ec_support.c looks to be a proper one. Moved.

slontis · 2026-04-09T22:20:12Z

@aisle-analyzer

aisle-research-bot · 2026-04-09T22:20:20Z

🔒 Aisle Security Analysis

We found 2 potential security issue(s) in this PR:

#	Severity	Title
1	🟠 High	Integer overflow and potential buffer overflow in EVP_EC_affine2oct() via unchecked field_len
2	🔵 Low	NULL pointer dereference in public EVP_EC_gen() macro via strstr(curve, "")

Each finding is detailed in a separate comment below.

Analyzed PR: #30597 at commit 685c1cc

_{Last updated on: 2026-04-09T22:21:46Z}

aisle-research-bot · 2026-04-09T22:21:48Z

+#define EVP_EC_gen(curve) \
+    EVP_PKEY_Q_keygen(NULL, NULL, "EC", (char *)(strstr(curve, "")))


2. 🔵 NULL pointer dereference in public EVP_EC_gen() macro via strstr(curve, "")

Property Value

Severity Low

CWE CWE-476

Description

The public macro EVP_EC_gen(curve) expands to a call that unconditionally evaluates strstr(curve, ""):

If a caller passes curve == NULL (e.g., from language bindings or application code that propagates optional/unchecked inputs), strstr(NULL, "") dereferences a NULL pointer and will crash the process.

This is reachable before OpenSSL can raise an error, because the crash happens during macro argument evaluation.

The macro also casts away const ((char *)), which can encourage UB if EVP_PKEY_Q_keygen or downstream code ever attempted to modify the string.

Vulnerable code:

#define EVP_EC_gen(curve) \ EVP_PKEY_Q_keygen(NULL, NULL, "EC", (char *)(strstr(curve, "")))

While passing NULL may be considered API misuse, OpenSSL APIs commonly handle NULL parameters by returning errors. Here it results in an immediate application-level denial of service.

Recommendation

Replace the macro with a real function (or an inline function) that validates inputs before calling the variadic API.

Example (header):

static ossl_inline EVP_PKEY *EVP_EC_gen(const char *curve) { if (curve == NULL) { ERR_raise(ERR_LIB_EVP, ERR_R_PASSED_NULL_PARAMETER); return NULL; } return EVP_PKEY_Q_keygen(NULL, NULL, "EC", (char *)curve); }

If you must keep the macro for ABI/API reasons, avoid evaluating curve inside strstr() and avoid discarding const; at minimum guard against NULL:

#define EVP_EC_gen(curve) \ EVP_PKEY_Q_keygen(NULL, NULL, "EC", (char *)((curve) != NULL ? (curve) : ""))

(Prefer the function form so NULL can produce a proper OpenSSL error rather than silently using an empty string.)

_{Last updated on: 2026-04-09T22:21:48Z}

slontis · 2026-04-09T22:26:33Z

+
+=head1 COPYRIGHT
+
+Copyright 2013-2026 The OpenSSL Project Authors. All Rights Reserved.


Just 2026 maybe?

Description of EVP_EC_gen() is not new, it was moved from EC_KEY_new.pod
I'm not sure what the copyright dates should be in this case.

igus68 · 2026-04-13T14:07:43Z

In practice, overflow is impossible here because field_len is a signed int and the result is converted to size_t, so the result will always be correct. But I agree that from the perspective of the formal specification it is undefined behaviour. So I added an explicit cast of field_len to size_t.

igus68 · 2026-04-13T14:14:34Z

I spent a considerable amount of time finding out why this strange construction "(char *)(strstr(curve, ""))" is necessary in the EVP_EC_gen macro, so I described it in a comment for future generations. And I fixed this NULL dereference

mattcaswell · 2026-04-24T12:47:02Z

    return NID_undef;
 }
+
+int EVP_EC_affine2oct(const BIGNUM *x, const BIGNUM *y, int field_len,


Should field_len really be a size_t? I'm not sure why we should be accepting a signed type here.

We use int for lengths in other functions:
int BN_num_bytes(const BIGNUM *a);
int EC_GROUP_get_degree(const EC_GROUP *group);
BIGNUM *BN_bin2bn(const unsigned char *s, int len, BIGNUM *ret);
etc

Yes, we do. I think those were a bad idea too. No need to make the hole bigger since this is a completely new API.

If we are starting a new life, what type is best here? Although field_len is technically used to determine the size of allocated memory, logically it's a mathematical group parameter that has nothing to do with memory or addressing. Therefore, I'd rather use an unsigned int.

Personally I still think a size_t is most appropriate for lengths and sizes of things. It is ultimately used to determine the amount of memory to allocate.

As agreed, changed to size_t.

mattcaswell · 2026-04-24T12:53:12Z

+ * 0 = uncompressed format
+ * 1 = compressed format
+ * 2 = affine coordinates format via OSSL_PARAM_BLD_push_EC_affine_point()
+ * 3 = affine coordinates format via OSSL_PARAM_BLD_push_EC_affine_point_ex()


What are OSSL_PARAM_BLD_push_EC_affine_point are OSSL_PARAM_BLD_push_EC_affine_point_ex? From an earlier iteration of this PR?

Yes, you are right. Fixed.

mattcaswell · 2026-04-24T12:53:59Z


   *Tomáš Mráz*

+ * Added `EVP_EC_affine2oct()` converts affine coordinates of an EC point


Suggested change

* Added `EVP_EC_affine2oct()` converts affine coordinates of an EC point

* Added `EVP_EC_affine2oct()` that converts the affine coordinates of an EC point

t8m · 2026-04-28T12:37:31Z

@igus68 this needs a rebase

when NULL is passed instead of the curve name.

coordinates of an EC point to an octet string conforming to Sec. 2.3.4 of the SECG SEC 1 ("Elliptic Curve Cryptography") standard.

in this macro.

igus68 · 2026-04-30T08:52:34Z

@igus68 this needs a rebase
Done

t8m · 2026-05-04T15:16:03Z

ping @slontis @mattcaswell for second review

mattcaswell

LGTM. One minor suggested improvement below.

mattcaswell · 2026-05-05T14:34:56Z

+
+"qx" (B<OSSL_PKEY_PARAM_EC_PUB_X>) and "qy" (B<OSSL_PKEY_PARAM_EC_PUB_Y>)
+can be used for getting the EC public key affine coordinates. To set
+them call EVP_EC_affine2oct() to convert affine coordinates to


Suggested change

them call EVP_EC_affine2oct() to convert affine coordinates to

them call L<EVP_EC_affine2oct(3)> to convert affine coordinates to

openssl-machine · 2026-05-06T16:00:41Z

This pull request is ready to merge

t8m · 2026-05-06T16:48:44Z

Merged to the master branch with tweaked commit messages.

Thank you.

It errors out with ERR_R_PASSED_NULL_PARAMETER in such case. Reviewed-by: Matt Caswell <matt@openssl.foundation> Reviewed-by: Simo Sorce <simo@redhat.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Wed May 6 16:47:55 2026 (Merged from #30597)

This function converts affine coordinates of an EC point to an octet string conforming to Sec. 2.3.4 of the SECG SEC 1 ("Elliptic Curve Cryptography") standard. Reviewed-by: Matt Caswell <matt@openssl.foundation> Reviewed-by: Simo Sorce <simo@redhat.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Wed May 6 16:47:57 2026 (Merged from #30597)

Also fixed the potential NULL pointer dereference in this macro. Reviewed-by: Matt Caswell <matt@openssl.foundation> Reviewed-by: Simo Sorce <simo@redhat.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Wed May 6 16:47:58 2026 (Merged from #30597)

It errors out with ERR_R_PASSED_NULL_PARAMETER in such case. Reviewed-by: Matt Caswell <matt@openssl.foundation> Reviewed-by: Simo Sorce <simo@redhat.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Wed May 6 16:47:55 2026 (Merged from openssl#30597)

This function converts affine coordinates of an EC point to an octet string conforming to Sec. 2.3.4 of the SECG SEC 1 ("Elliptic Curve Cryptography") standard. Reviewed-by: Matt Caswell <matt@openssl.foundation> Reviewed-by: Simo Sorce <simo@redhat.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Wed May 6 16:47:57 2026 (Merged from openssl#30597)

Also fixed the potential NULL pointer dereference in this macro. Reviewed-by: Matt Caswell <matt@openssl.foundation> Reviewed-by: Simo Sorce <simo@redhat.com> Reviewed-by: Tomas Mraz <tomas@openssl.foundation> MergeDate: Wed May 6 16:47:58 2026 (Merged from openssl#30597)

igus68 added branch: master Applies to master branch triaged: feature The issue/pr requests/adds a feature backlog fix The issue was closed as part of the backlog reduction initiative. labels Mar 27, 2026

igus68 force-pushed the fix-16270-alt branch from 28ae550 to dea5745 Compare March 27, 2026 08:57

openssl-machine added the approval: review pending This pull request needs review by a committer label Mar 27, 2026

github-actions Bot added severity: fips change The pull request changes FIPS provider sources severity: ABI change This pull request contains ABI changes labels Mar 27, 2026

igus68 force-pushed the fix-16270-alt branch from dea5745 to 71668a5 Compare March 27, 2026 10:20

igus68 mentioned this pull request Mar 27, 2026

Initialize EC key using EC_PUB_X and EC_PUB_Y #16270

Closed

t8m requested changes Mar 27, 2026

View reviewed changes

t8m added the tests: present The PR has suitable tests present label Mar 27, 2026

igus68 force-pushed the fix-16270-alt branch 2 times, most recently from 33a8823 to 7b6f669 Compare March 30, 2026 09:55

t8m requested changes Apr 3, 2026

View reviewed changes

igus68 force-pushed the fix-16270-alt branch from 7b6f669 to 2d4200a Compare April 3, 2026 14:44

t8m mentioned this pull request Apr 7, 2026

Initialize EC key using affine coordinates #30538

Closed

2 tasks

igus68 force-pushed the fix-16270-alt branch from 2d4200a to 685c1cc Compare April 8, 2026 07:39

t8m requested review from slontis and t8m April 9, 2026 07:33

aisle-research-bot Bot reviewed Apr 9, 2026

View reviewed changes

Comment thread crypto/evp/ec_support.c

aisle-research-bot Bot reviewed Apr 9, 2026

View reviewed changes

slontis reviewed Apr 9, 2026

View reviewed changes

Comment thread crypto/evp/ec_support.c

slontis reviewed Apr 9, 2026

View reviewed changes

igus68 force-pushed the fix-16270-alt branch from 685c1cc to 152a4c6 Compare April 13, 2026 13:56

igus68 requested a review from slontis April 13, 2026 14:38

igus68 force-pushed the fix-16270-alt branch from 152a4c6 to cc99533 Compare April 14, 2026 14:55

mattcaswell requested changes Apr 24, 2026

View reviewed changes

igus68 dismissed slontis’s stale review via 401c416 April 24, 2026 14:06

igus68 force-pushed the fix-16270-alt branch from 5e91427 to 401c416 Compare April 24, 2026 14:06

The EVP_PKEY_Q_keygen function now explicitly handles the situation

19815f2

when NULL is passed instead of the curve name.

igus68 force-pushed the fix-16270-alt branch from 401c416 to 38d9222 Compare April 30, 2026 08:40

igus68 added 2 commits April 30, 2026 10:49

Added the EVP_EC_affine2oct() function, which converts affine

4fa126b

coordinates of an EC point to an octet string conforming to Sec. 2.3.4 of the SECG SEC 1 ("Elliptic Curve Cryptography") standard.

Moved the EVP_EC_gen macro to evp.h, fixed the null pointer dereference

929c7c5

in this macro.

igus68 force-pushed the fix-16270-alt branch from 38d9222 to 929c7c5 Compare April 30, 2026 08:50

igus68 requested review from mattcaswell and slontis April 30, 2026 09:46

t8m approved these changes May 4, 2026

View reviewed changes

t8m requested a review from a team May 5, 2026 12:34

mattcaswell approved these changes May 5, 2026

View reviewed changes

openssl-machine added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels May 5, 2026

simo5 approved these changes May 5, 2026

View reviewed changes

openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels May 6, 2026

t8m closed this May 6, 2026

		int EVP_EC_affine2octet_string(const BIGNUM X, const BIGNUM Y,
		int field_len, unsigned char *pbuf, size_t pbsize);

		int EVP_EC_affine2octet_string(const BIGNUM X, const BIGNUM Y, int field_len,
		unsigned char *pbuf, size_t pbsize);

		#define EVP_EC_gen(curve) \
		EVP_PKEY_Q_keygen(NULL, NULL, "EC", (char *)(strstr(curve, "")))


		=head1 COPYRIGHT

		Copyright 2013-2026 The OpenSSL Project Authors. All Rights Reserved.


		Tomáš Mráz

		* Added `EVP_EC_affine2oct()` converts affine coordinates of an EC point

	* Added `EVP_EC_affine2oct()` converts affine coordinates of an EC point
	* Added `EVP_EC_affine2oct()` that converts the affine coordinates of an EC point

	them call EVP_EC_affine2oct() to convert affine coordinates to
	them call L<EVP_EC_affine2oct(3)> to convert affine coordinates to

Uh oh!

Conversation

igus68 commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

slontis commented Apr 9, 2026

Uh oh!

aisle-research-bot Bot commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔒 Aisle Security Analysis

Uh oh!

Uh oh!

aisle-research-bot Bot Apr 9, 2026

Choose a reason for hiding this comment

2. 🔵 NULL pointer dereference in public EVP_EC_gen() macro via strstr(curve, "")

Description

Recommendation

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igus68 commented Apr 13, 2026

Uh oh!

igus68 commented Apr 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t8m commented Apr 28, 2026

Uh oh!

igus68 commented Apr 30, 2026

Uh oh!

t8m commented May 4, 2026

Uh oh!

mattcaswell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openssl-machine commented May 6, 2026

Uh oh!

igus68 commented Mar 27, 2026 •

edited

Loading

aisle-research-bot Bot commented Apr 9, 2026 •

edited

Loading