-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Improve ext-openssl to generate EC keys with parameters #9991
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This looks like a change that should target the master branch (not any of the stable branches). |
@cmb69 The change have targeted the master branch. Will it be merged into PHP 8.1 and above? |
@cmb69 I added compatibility code for OpenSSL<3.0. It may need to re-review, thank you for much. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just an initial review without checking the logic extensively. The main thing that is missing is cover all the new functionality by tests which is a requirement to get it merged. Preferably try to not modify existing tests but create new ones.
Just to answer
No because it is a new feature which is not exactly self contained so it can go only to master - it means PHP 8.3. |
cd7a2a4
to
eb83e02
Compare
@Eno-CN Looks better now but I will need to find some time for a proper review and testing which might take me around a month or possibly slightly longer just to give some expectation as I have got some other things on my list before this one which has got still plenty of time before the change freeze. If you could rebase the changes on master so there's no conflict, that would be great! |
cd1b0ca
to
3660bc3
Compare
ae368b2
to
3660bc3
Compare
ec51180
to
403ba69
Compare
3088eba
to
6763590
Compare
ChangesAdd Explicit EC Parameters for openssl_pkey_new
Optimize the code for generating EC public/private keysAdjusted the code style to be consistent with the overall code style of
Use EVP_PKEY_generate() to generate keypair instead of EVP_PKEY_keygen()EVP_PKEY_keygen() do exactly the same thing as EVP_PKEY_generate(), after checking that the corresponding EVP_PKEY_keygen_init() was used to initialize ctx. Fix sm2 compatibility bugs on openssl_pkey_get_details#9422 - openssl ext sm2 compatibility ExamplesEC - generate keypair with curve_name<?php
/*
* Custom parameters x, y, and d are not supported with SM2 in OpenSSL 3.x.
* Directly creating EVP_PKEY_CTX using EVP_PKEY_CTX_new_from_name(NULL, "SM2", NULL)
* will result in generating incorrect private keys (which cannot be correctly recognized
* by existing external applications based on the SM2 algorithm).
*/
$curve_name = 'SM2';
$pkey = openssl_pkey_new(array(
'ec'=> array(
'curve_name' => $curve_name,
)
));
$details = openssl_pkey_get_details($pkey);
var_dump($details);
$pubkey = $details['key'];
openssl_pkey_export($pkey, $prikey);
echo 'Private Key:', PHP_EOL, $prikey, PHP_EOL;
echo 'Public Key:', PHP_EOL, $pubkey, PHP_EOL; EC - generate keypair with custom params (OSCCA WAPIP192v1 Elliptic curve)<?php
$d = hex2bin('8D0AC65AAEA0D6B96254C65817D4A143A9E7A03876F1A37D'); // private key binary
$x = hex2bin('98E07AAD50C31F9189EBE6B8B5C70E5DEE59D7A8BC344CC6'); // public key x binary
$y = hex2bin('6109D3D96E52D0867B9D05D72D07BE5876A3D973E0E96792'); // public key y binary
$p = hex2bin('BDB6F4FE3E8B1D9E0DA8C0D46F4C318CEFE4AFE3B6B8551F');
$a = hex2bin('BB8E5E8FBC115E139FE6A814FE48AAA6F0ADA1AA5DF91985');
$b = hex2bin('1854BEBDC31B21B7AEFC80AB0ECD10D5B1B3308E6DBF11C1');
$g_x = hex2bin('4AD5F7048DE709AD51236DE65E4D4B482C836DC6E4106640');
$g_y = hex2bin('02BB3A02D4AAADACAE24817A4CA3A1B014B5270432DB27D2');
$order = hex2bin('BDB6F4FE3E8B1D9E0DA8C0D40FC962195DFAE76F56564677');
$pkey = openssl_pkey_new(array(
'ec'=> array(
'p' => $p,
'a' => $a,
'b' => $b,
'order' => $order,
'g_x' => $g_x,
'g_y' => $g_y,
//'d' => $d, // import the private key to generate keypairs
)
));
$details = openssl_pkey_get_details($pkey);
var_dump($details);
$pubkey = $details['key'];
openssl_pkey_export($pkey, $prikey);
echo 'Private Key:', PHP_EOL, $prikey, PHP_EOL;
echo 'Public Key:', PHP_EOL, $pubkey, PHP_EOL; EC - generate keypair with custom params (SM2 curve)<?php
$p = hex2bin('FFFFFFFEFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00000000FFFFFFFFFFFFFFFF');
$a = hex2bin('FFFFFFFEFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00000000FFFFFFFFFFFFFFFC');
$b = hex2bin('28E9FA9E9D9F5E344D5A9E4BCF6509A7F39789F515AB8F92DDBCBD414D940E93');
$g_x = hex2bin('32C4AE2C1F1981195F9904466A39C9948FE30BBFF2660BE1715A4589334C74C7');
$g_y = hex2bin('BC3736A2F4F6779C59BDCEE36B692153D0A9877CC62A474002DF32E52139F0A0');
$order = hex2bin('FFFFFFFEFFFFFFFFFFFFFFFFFFFFFFFF7203DF6B21C6052B53BBF40939D54123');
/*
* Custom parameters x, y, and d are not supported with SM2 in OpenSSL 3.x.
* Directly creating EVP_PKEY_CTX using EVP_PKEY_CTX_new_from_name(NULL, "SM2", NULL)
* will result in generating incorrect private keys (which cannot be correctly recognized
* by existing external applications based on the SM2 algorithm).
*/
$pkey = openssl_pkey_new(array(
'ec'=> array(
'p' => $p,
'a' => $a,
'b' => $b,
'order' => $order,
'g_x' => $g_x,
'g_y' => $g_y,
)
));
/*
* It is not entirely the same as generating keys through the SM2 curve naming method.
* So the generated key will be in PKCS8 format to store algorithm information.
*/
$details = openssl_pkey_get_details($pkey);
var_dump($details);
$pubkey = $details['key'];
openssl_pkey_export($pkey, $prikey);
echo 'Private Key:', PHP_EOL, $prikey, PHP_EOL;
echo 'Public Key:', PHP_EOL, $pubkey, PHP_EOL; Reference
|
@bukka After testing, it was found that this issue is related to the excessive length of the first comment in the PR. When the length of the first comment is too long, it causes a certain special change in the output format of the phpinfo test script, which leads to erroneous assertions in the test script. |
670fc00
to
96a5314
Compare
@bukka Now all CI tests have passed. If you have time, please review the code changes. |
5246c93
to
bc708e5
Compare
bc708e5
to
ac963e0
Compare
Generate EC keys using the macro OPENSSL_EC_EXPLICIT_CURVE compatible with OpenSSL versions below 1.1.0 Fix SM2 compatibility bugs Separate EC tests Add SM2 compatibility test
e9839f5
to
1174633
Compare
int base_id = 0; | ||
|
||
if (EVP_PKEY_id(pkey) != EVP_PKEY_KEYMGMT) { | ||
base_id = EVP_PKEY_base_id(pkey); | ||
} else { | ||
const char *type_name = EVP_PKEY_get0_type_name(pkey); | ||
if (type_name) { | ||
int nid = OBJ_txt2nid(type_name); | ||
if (nid != NID_undef) { | ||
base_id = EVP_PKEY_type(nid); | ||
} | ||
} | ||
} | ||
|
||
switch (base_id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems actually as a bug fix so adding note for myself to back port it potentially.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Eno-CN I have done another proper review of the code and all looks good. I have also done extensive testing of all the main supported OpenSSL versions (1.0.2, 1.1.1, 3.0, latest 3.1) and all tests are passing for me locally. Pipeline is also good. So everything seems good. Thanks for your work on this!
@bukka Thank you very much for your thorough review and extensive testing! I'm glad to hear that everything is working fine. Your expertise and experience have made a significant contribution to this project. Thank you for your efforts! If there are any further questions or topics you'd like to discuss, please let me know. |
Feat: support ext-openssl Explicit EC Parameters