ext/openssl: Check that loading/writing to RANDFILE succeeds #17758
Conversation
Girgias
force-pushed
the
openssl-follow-up
branch
from
ffde1ab to
0df4f1a
Compare
Maybe. But I also don't know why CI is completely red. Might wait and see if this is related to #19369 |
Hm I doubt it. Try rebasing? |
|
This is not correct because failure to load rand file when rand status is still 1 is not really a problem. We should probably not even try to load such file if it doesn't exist to prevent https://bugs.php.net/bug.php?id=78444 . Also the error messages are already triggered for real errors. So what we should do is to stat the loaded file and return success if it doesn't exist or fail to load but the status is still 1. For write, there is probably not much point to return anything. In reality there should be no reason to even try to load the file but we can leave it for extra paranoid users. But if it fails, it's not really an issue as I mentioned. |
| char *randfile = php_openssl_conf_get_string(req->req_config, req->section_name, "RANDFILE"); | ||
| php_openssl_load_rand_file(randfile, &egdsocket, &seeded); | ||
| if (php_openssl_load_rand_file(randfile, &egdsocket, &seeded) == FAILURE) { | ||
| php_error_docref(NULL, E_WARNING, "Failed to load RANDFILE"); |
There was a problem hiding this comment.
There is no point for extra warning.
| cleanup: | ||
| php_openssl_write_rand_file(randfile, egdsocket, seeded); | ||
| if (php_openssl_write_rand_file(randfile, egdsocket, seeded) == FAILURE) { | ||
| php_error_docref(NULL, E_WARNING, "Failed to write to RANDFILE"); |
|
I would probably target just master as it's not a big deal. |
3 participants
Follow-up to #17721