Add openssl_x509_verify() function #3624
Conversation
|
Comment on behalf of petk at php.net: Labelling. Thank you for the pull request @DASPRiD |
|
Comment on behalf of petk at php.net: Labelling |
|
By the way, I take that it's probably too late for this to make it into 7.3, correct? |
ext/openssl/openssl.c
Outdated
| } | ||
| key = php_openssl_evp_from_zval(zkey, 1, NULL, 0, 0, &keyresource); | ||
| if (key == NULL) { | ||
| RETURN_FALSE; |
There was a problem hiding this comment.
Would I have to do RETVAL_FALSE; instead? As I said, I'm not too confident with this stuff :)
There was a problem hiding this comment.
I guess you are missing a X509_free(cert); here
ext/openssl/openssl.c
Outdated
|
|
||
| if (err < 0) { | ||
| php_openssl_store_errors(); | ||
| } |
|
/cc @bukka for review |
ext/openssl/openssl.c
Outdated
| } | ||
| cert = php_openssl_x509_from_zval(zcert, 0, NULL); | ||
| if (cert == NULL) { | ||
| RETURN_FALSE; |
There was a problem hiding this comment.
It seems a bit strange to return false if the return type is int. If you want to follow X509_verify, then it should probably return -1.
There was a problem hiding this comment.
Good point, I think openssl_verify() does it similar.
|
I think it seems reasonable once all the comments are addressed. |
|
@petk What's blocking the merge? :) |
|
Sorry for the delay. Seems like tests have passed and issues mentioned above have been addressed. Nothing from my side so I'll check it out and merge it soon... Thanks for the addition. |
|
Anyone knows or has some suggestions maybe how we should document new functions such as this one? Do we wait and add it to the docs later in the process or is it suitable to be added now already at https://php.net/manual/en/book.openssl.php |
Most important is to note all changes that require documentation in UPGRADING, too (also adding a link to the PR or RFC appears reasonable). This file serves as base for the migration guide, and users certainly want to know about all new functions (etc.) Whether it's better to document the actual change in the manual proper (i.e. not in the migration guide) right away or sometimes later, is of secondary concern, and may be arguable. I prefer the former, mostly because I find it easier to document freshly written or reviewed code than old stuff, and particularly for new functions (etc.) because I can create the basic documentation using docgen right away. |
|
UPGRADING file updated via 35a9ec1 |
> - Core: > Added get_mangled_object_vars($object) function, which returns the mangled > object properties. It returns the same result as (array) $object, with the > exception that it ignores overloaded array casts, such as used by > ArrayObject. > > - OpenSSL: > Added openssl_x509_verify(mixed cert, mixed key) function that verifies the > signature of the certificate using a public key. A wrapper around the > OpenSSL's X509_verify() function. > See <php/php-src#3624>. > > - Pcntl: > Added bool pcntl_unshare(int flags) function which allows to dissociate > parts of the process execution context which are currently being shared with > other processes. Explicitly, it allows you to unshare the mount, IPC, UTS, > network, PID, user and cgroup namespaces. > > - SQLite3: > Added SQLite3Stmt::getSQL() to retrieve the SQL of the statement. If TRUE is > passed as parameter, query parameters will be replaced in the return value > by their currently bound value, if libsqlite ≥ 3.14 is used. > > - Standard > * bool sapi_windows_set_ctrl_handler(callable handler, [, bool add = true]) - > set or remove a handler function upon receiving a CTRL event. The handler > function is expected have a signature "function handler(int $event)". > * bool sapi_windows_generate_ctrl_event(int type, int pid) - send a CTRL event > to another process. Refs: * https://github.com/php/php-src/blob/42cc58ff7b2fee1c17a00dc77a4873552ffb577f/UPGRADING#L348-L379
> - Core: > Added get_mangled_object_vars($object) function, which returns the mangled > object properties. It returns the same result as (array) $object, with the > exception that it ignores overloaded array casts, such as used by > ArrayObject. > > - OpenSSL: > Added openssl_x509_verify(mixed cert, mixed key) function that verifies the > signature of the certificate using a public key. A wrapper around the > OpenSSL's X509_verify() function. > See <php/php-src#3624>. > > - Pcntl: > Added bool pcntl_unshare(int flags) function which allows to dissociate > parts of the process execution context which are currently being shared with > other processes. Explicitly, it allows you to unshare the mount, IPC, UTS, > network, PID, user and cgroup namespaces. > > - Standard > * bool sapi_windows_set_ctrl_handler(callable handler, [, bool add = true]) - > set or remove a handler function upon receiving a CTRL event. The handler > function is expected have a signature "function handler(int $event)". > * bool sapi_windows_generate_ctrl_event(int type, int pid) - send a CTRL event > to another process. Refs: * https://github.com/php/php-src/blob/42cc58ff7b2fee1c17a00dc77a4873552ffb577f/UPGRADING#L348-L379
This PR introduces a wrapper around OpenSSL's X509_verify() function.
The tests all work, but since I'm not neither confident in OpenSSL nor C at all, I'd like to have someone verify that all I've done is correct before anyone merges this.