ext/openssl: fix OpenSSL 4 build and test compatibility - #22267
Conversation
shivammathur
changed the title
iluuu1994
left a comment
iluuu1994
left a comment
There was a problem hiding this comment.
Thanks @shivammathur! I can't meaningfully review the OpenSSL part (though I did look at it and did not see an obvious mistake), but ack for the GHA part.
6ffd0fa to
46303d2
Compare
bukka
left a comment
bukka
left a comment
There was a problem hiding this comment.
I checked it all and it makes sense. There might be a bit safer version of that common name match but I won't object back porting as it is if you have properly tested even on 1.1.1
| name = X509_get_subject_name(peer); | ||
| name_index = X509_NAME_get_index_by_NID(name, NID_commonName, -1); | ||
| if (name_index == -1) { | ||
| php_stream_warn(stream, NetworkRecvFailed, "Unable to locate peer certificate CN"); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This is good for master but for backport, wouldn't be enough to just const X509_NAME *cert_name; and leave the rest?
There was a problem hiding this comment.
Agreed, I only wanted to target the master branch with this, but if we want to backport, I can do another PR keeping this limited to the const X509_NAME *cert_name change.
4f01ab7 to
97088f2
Compare
|
@bukka I have made a minor fix to keep OpenSSL 1.1.1 compatibility and tested it. |
3 participants
(Ref openssl/openssl@e75bd84ffc73, openssl/openssl@b0f2107b4404)
openssl@4in macOS CI, we can also upgrade it for the master branch on Windows once merged.