loadKey should check return value for openssl_get_privatekey #249
Conversation
src/XMLSecurityKey.php
Outdated
|
|
||
| case 'private': | ||
| $this->key = openssl_get_privatekey($this->key, $this->passphrase); | ||
| if (! $this->key) { |
There was a problem hiding this comment.
Be as strict & specific as you can and use === false instead
sammarshallou
force-pushed
the
detect-private-key-load-failures
branch
from
8df2e9e to
30f7d6c
Compare
|
@tvdijen Thanks for review, I've updated as requested. |
|
Hi, |
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
force-pushed
the
detect-private-key-load-failures
branch
from
30f7d6c to
35967f8
Compare
|
@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. |
|
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. |
|
This commit was rolled back in If you experience the exception Exception encountered during processing SAML authentication response: Unable to extract private key (invalid key or passphrase): error:0909006C:PEM routines:get_name:no start line Then simply |
5 participants
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.