Skip to content

Conversation

@mhagstrand
Copy link
Contributor

…ivate key pair for DH when the private key is provided

Also DSA_generate_key cannot generate the public key from the private in php_openssl_pkey_init_dsa

@mhagstrand mhagstrand force-pushed the BUG73478 branch 2 times, most recently from dca25ca to 69f46f8 Compare November 11, 2016 18:53
@KalleZ
Copy link
Member

KalleZ commented Nov 11, 2016

cc @bukka

…ivate key pair for DH when the private key is provided

Also DSA_generate_key cannot generate the public key from the private in php_openssl_pkey_init_dsa
@bukka
Copy link
Member

bukka commented Nov 13, 2016

Unfortunately this is no longer supported in OpenSSL 1.1 so I'm not too keen to add just for 1.0 as it will create difference that we can't fix. Please see

https://github.com/openssl/openssl/blob/e2cefab06a9e1b8d9a21d030754f62dfbb199950/crypto/dh/dh_lib.c#L251

(you can't supply an empty public key).

The DH part won't set the key too in OpenSSL 1.1 for the the similar reason. So it would again lead to a different result.

@mhagstrand
Copy link
Contributor Author

Yeah, that is really good point. I didn't realize it would not work in OpenSSL 1.1. I'm going to close this PR. Thanks

@mhagstrand mhagstrand closed this Nov 13, 2016
@ezimuel
Copy link
Contributor

ezimuel commented Nov 15, 2016

@bukka this will introduce a BC break for people using PHP 7.1 and OpenSSL 1.0.x. For instance, Ubuntu 16 offers OpenSSL 1.0.2g as default. I think we should trigger a warning or a deprecation notice if someone will pass a priv_key in dh parameter for openssl_pkey_new(). Basically, the Diffie Hellman keys can be generated only with g (generator) and p (prime) parameters, starting from PHP 7.1.

@bukka
Copy link
Member

bukka commented Nov 18, 2016

@ezimuel I put together #2208 which seems to work fine with OpenSSL 1.1 but needs more testing on lower versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants