robrichards / xmlseclibs

A PHP library for XML Security
BSD 3-Clause "New" or "Revised" License
386 stars 180 forks source link

loadKey should check return value for openssl_get_privatekey #249

Closed sammarshallou closed 1 year ago

sammarshallou commented 1 year ago

The function openssl_get_privatekey may return false, for example if the passphrase is incorrect. In the other 2 branches of the switch statement, this is checked and an exception thrown, but it was not checked in the private key branch. This commit adds a check.

This makes it easier to detect problems with invalid keys or pass phrases. Without this change, you are likely to get a confusing exception later on when it tries to use the key.

sammarshallou commented 1 year ago

@tvdijen Thanks for review, I've updated as requested.

skaylink-AndreasUlm commented 1 year ago

Hi, would it make sense to inject the output of openssl_error_string() into exception message to allow understanding why it failed?

sammarshallou commented 1 year ago

@skaylink-AndreasUlm (Sorry for the slow response.) Yes I agree that's a good suggestion - I've altered the code. According to the documentation, you should call openssl_error_string repeatedly in case there are multiple errors, but none of the other similar lines in the file do that, so I guess it's OK.

tvdijen commented 1 year ago

I can tell you from experience that it is really useful for debugging if you loop through the errors. It's usually the first one you need, not the last one in the chain.