UPSTREAM PR #17088: HTTP client: simplify and improve HTTP(S) proxy support#674
Open
loci-dev wants to merge 7 commits into
Open
UPSTREAM PR #17088: HTTP client: simplify and improve HTTP(S) proxy support#674loci-dev wants to merge 7 commits into
loci-dev wants to merge 7 commits into
Conversation
…OSSL_HTTP_REQ_CTX* parameter
Reduce the arg type of the bio_update_fn used by APPS to its bare minimum: a SSL_CTX pointer.
This is possible by
1. introducing OSSL_HTTP_REQ_CTX_proxy_connect(), a variant of OSSL_HTTP_proxy_connect()
that uses a OSSL_HTTP_REQ_CTX pointer, and calling this in case a SSL/TLS connection is being opened
already from OSSL_HTTP_open() (where the pointer is available) rather than letting the callback do this.
2. using SSL_CTX_{set,get}_ex_data to convey the server host info via the SSL_CTX to the callback function.
…nd to include the Host: header line first, adding non-default server port
…not show any given user:pass@ part
…hen HTTP(S) proxy use test cases
421b135 to
770bf14
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Source pull request: openssl/openssl#17088
This fixes minor bugs and handles a design flaw with the
bio_update_fnconnect/disconnect callback function of typeOSSL_HTTP_bio_cb_tnot having a parameter of typeOSSL_HTTP_REQ_CTX *, which causes trouble with connecting via an HTTPS proxy during TLS connection setup because this needs access to the proxy and no_proxy information. Moreover, the callback needs access to theSSL_CTXand to the server hostname for setting the Server Name Indication (SNI). All of this has been done so far in a pretty cumbersome and not entirely correct way.Make sure thatOSSL_HTTP_proxy_connect()is called if and only if needed with TLS.So far, this did not correctly take into account the environment variables
https_proxyandno_proxy.Doing this fix unfortunately requires generalizing the callback function such that it can take into account whether a proxy is actually being used during connection setup.
This is done in a binary backward compatible way: by adding to
OSSL_HTTP_bio_cb_tanrtcxparameter that can be used unless thedetailsparameter is1on connect, which is tried first for compatibility with OpenSSL 3.0.The generalization also makes the design of
OSSL_HTTP_bio_cb_tmore future-proof.Also introduceOSSL_HTTP_REQ_CTX_proxy_connect(), an improved variant ofOSSL_HTTP_proxy_connect(), which strongly simplifies the use of the callback function, e.g., inapps/.Update of Feb 9, 2026:
I recently found a way of working around the missing
OSSL_HTTP_REQ_CTXpointer that does not require adding a parameter toOSSL_HTTP_bio_cb_t, which would have incurred an API break.This is possible by introducing
OSSL_HTTP_REQ_CTX_proxy_connect(), a variant ofOSSL_HTTP_proxy_connect()that uses a
OSSL_HTTP_REQ_CTXpointer, and calling this function (in case an SSL/TLS connection is being opened)already from
OSSL_HTTP_open(), where the pointer is available, rather than letting the callback do this.Moreover, using
SSL_CTX_{set,get}_ex_data()to convey the server host name via theSSL_CTXto the callback function avoids having to pass the server name via the generic callback arg.
As a result, the callback arg can be reduced to the bare minimum: a pointer to the
SSL_CTX.Implementing this improvement I noticed that that userinfo (user name and password) that can be provided with the proxy URI was not used, neither for the HTTP nor the HTTPS proxy case. I added this.
This PR also includes several fixes:
added several missing failure checks to
OSSL_HTTP_proxy_connect()fix
OSSL_HTTP_open()to include in theHost:header line the server port, which is needed for non-default portsfix the CMP app to provide the right proxy usage info
Fix cleanup of TLS BIO viabio_update_fncallback function.Make
app_http_tls_cb()tidy up on disconnect the SSL BIO it pushes on connect.Make
OSSL_HTTP_close()respect this.OSSL_HTTP_proxy_connect(): Fix glitch in response HTTP header parsing.It would be good to have CI tests for using an HTTP(S) proxy, but this would require providing within OpenSSL a test proxy that supports HTTP and HTTPS connections.This PR meanwhile contains client-side unit tests for the provided improvements of HTTP(S) proxy use,
which has become possible after lifting some not really needed argument restrictions of
OSSL_HTTP_open().I tested the improved implementation also with external both HTTP and HTTPS proxies, including proxy client credential (user name and password) usage.
This PR also includes a commit that fixes an omission documenting theokparameter ofOSSL_HTTP_close().