Skip to content

Conversation

Furgas
Copy link
Contributor

@Furgas Furgas commented Jun 24, 2015

The "key values mismatch" error is triggered in openssl_pkcs12_read by PKCS12_parse, because it uses X509_check_private_key to separate main certificate (which corresponds to private key) from extra certificates. Extra certificates usually comes first (p12 contents are reversed as stack) and X509_check_private_key triggers X509_R_KEY_VALUES_MISMATCH error.

The fix pops "key values mismatch" error from OpenSSL error stack for each extra certificate if there are any.

@datibbaw
Copy link
Contributor

The test failure doesn't seem to be related to your PR, not sure why you've closed it ;-)

@@ -2575,7 +2575,13 @@ PHP_FUNCTION(openssl_pkcs12_read)
zval * zextracert;
X509* aCA = sk_X509_pop(ca);
if (!aCA) break;


// fix for bug 69882
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use block-style comments, i.e. /* fix for bug 69882 */

@Furgas
Copy link
Contributor Author

Furgas commented Jun 25, 2015

I've closed the one based on PHP-5.6.7. AFAIK you can't change the base branch in Github PR, you must create another one.
Today I will push changes you mentioned. Thanks.

…read with extra certs

The "key values mismatch" error is triggered in openssl_pkcs12_read by
PKCS12_parse, because it uses X509_check_private_key to separate main
certificate (which corresponds to private key) from extra certificates.
Extra certificates usually comes first (p12 contents are reversed as
stack) and X509_check_private_key triggers X509_R_KEY_VALUES_MISMATCH
error.
The fix pops "key values mismatch" error from OpenSSL error stack for
each extra certificate if there are any.
@php-pulls
Copy link

Comment on behalf of datibbaw at php.net:

Merged via 2ff3daf

Thanks for your contribution!

@php-pulls php-pulls closed this Jun 25, 2015
@Furgas Furgas deleted the fix-69882 branch June 26, 2015 06:27
@dzuelke
Copy link
Contributor

dzuelke commented Feb 15, 2016

Should this be reverted at some point? The issue probably needs proper fixing in the OpenSSL extension instead - see #1223 and all comments in there for info.

/cc @Furgas @datibbaw @bukka @rdlowrey @smalyshev

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