Implement custom certificate handler option - #42
Conversation
Petezah
left a comment
Petezah
left a comment
There was a problem hiding this comment.
Seems pretty sane to me actually. The changes are fairly minimal, and express intent a lot more clearly than a piecemeal approach we've tended to do. I have a minor suggestion before I approve, but I won't push back if you think it's not beneficial.
| mbedtls_ssl_config */ | ||
| void *userptr); | ||
|
|
||
| /* Unity extension: CURLOPT_UNITY_CERTVERIFY_FUNCTION callback prototype. |
There was a problem hiding this comment.
An idea: What if we wrap all these blocks in #if USE_UNITY_CERTVERIFY? Then we can easily find all the modified blocks for maintenance purposes.
There was a problem hiding this comment.
Good idea! Put all new code behind UNITY_CERTVERIFY.
|
looks good to me, easy to follow and make sense. One trade-off of this unification is that it may become a tid bit more work to upgrade curl in future, but I don't have a good proposal except what peter has suggested above. |
3 participants
Currently in UnityWebRequest we grab the SSL context object from the backend (using
SSL_CTX_FUNCTION) and set a custom certificate handler through that. In our builds of Curl, the backend is always UnityTLS so this works well.However if we wanted to switch backends on some platforms, it could be problematic. The Schannel backend doesn't offer a SSL context object, and while the OpenSSL one does, it's obviously its own object type and would require custom code in UnityWebRequest to hook up correctly (code that would need to be aware of the OpenSSL API).
Instead I'd like to propose a different approach: introduce a new Unity-only Curl option to set a custom certificate validation callback. And then hook up that validation callback in every backend that we might use. This addresses the current limitation of the Schannel backend, while also making the UnityWebRequest code agnostic to the backend being used.
Of course, it also means more invasive changes to upstream Curl code, which might represent a maintenance cost when updating to new versions. I've tried to keep the changes so that they're unlikely to conflict when these files churn. (Also in terms of maintenance cost, it could be argued that maintaining our own entire VTLS backend is more work than dealing with a few patches to existing ones.)