Implement a cURL stream #3183
Implement a cURL stream #3183
Conversation
carlosmn
force-pushed
the
cmn/curl-stream
branch
3 times, most recently
from
77a286e
to
923014a
Compare
|
Extracting certificate information out of the curl handle is rather obtuse. It is only implemented for some crypto libraries, and it's in the form of a list of "key:value" strings so on the libcurl which comes with OSX, we're not able to get any information about the certificate, just whether SecureTransport figured it was valid or not. This is way below the standard which we've set for ourselves with the other crypto backends, but there's no other way we're going to get proxy support. |
|
This should be about ready to go in, though the proxy code is still untested. Testing that is going to be a barrel of laughs. There's also the question of how do we test this stream. We shouldn't replace all the builds with a liburl one, so do we add an extra entry to the matrix asking the build system to install libcurl-dev? |
carlosmn
force-pushed
the
cmn/curl-stream
branch
from
0261abe
to
9c2ae94
Compare
|
As a way to work around libcurl not quite giving us the information from the certificate, I think it'd be worth considering using libcurl only for the proxying capabilities, and send our TLS stream though it. The SecureTransport stream should be ready to take this on, as it sends data over to the socket stream, but having OpenSSL use a user-defined channel is not something on which I've found much information. |
carlosmn
force-pushed
the
cmn/curl-stream
branch
from
e33c623
to
9303876
Compare
|
Have you looked at CURLOPT_HTTPPROXYTUNNEL mode to use libcurl to ONLY provide a tunnel and do all the https negotiation / certificate extraction / etc from the libgit2 side? This is an extremely timely addition as lack of https proxy support was turning into a major headache for me. Thanks. |
|
Yeah, using libcurl solely as a tunnel is the current plan due to the differences in cert reporting across its crypto backends. Maybe I can get away with grabbing its socket descriptor and feeding it into OpenSSL, otherwise I'll have to figure out how to create a BIO. |
carlosmn
force-pushed
the
cmn/curl-stream
branch
2 times, most recently
from
4a44a13
to
00225f9
Compare
|
I'm not sure if there's a reliable[0] way to make CI test the proxy bits, but I have just tested locally and it does work, both by setting The Mac builds automatically have libcurl available, though I think the Ubuntu VMs don't have libcurl-dev installed, which means we won't exercise that path there, and always installing it would make us not use the plain socket stream. Maybe we can install it on the debug builds? Regardless, I'd like to see this get merged soon so it gets some testing in the wild before we make the next release with it. [0] Emphasis on "reliable". We can install tinyproxy and grep its logs, but that's yet another thing that can go wrong. |
|
I tried it yesterday with my corporsate environment and couldn't get it to work at all. |
|
You use this the same way you use it with git, you set I don't know where you expect to specify the tunneling mode, it is an option we pass to libcurl when we set up the handle. |
|
The comment to tunneling mode was in response to what you wrote :-) |
|
The code for tunneling TLS over it was only working early this morning, so whatever you used last night probably didn't have the tunneling on. The "us" in the comment is libgit2. Users of the library are explicitly called out as such. |
|
Duh. The pull request didn't show new commits so I didn't realize you had added more code. Apologies. I just tried with the latest of the cmn/curl-stream branch and it works perfectly. |
cURL has a mode in which it acts a lot like our streams, providing send and recv functions and taking care of the TLS and proxy setup for us. Implement a new stream which uses libcurl instead of raw sockets or the TLS libraries directly. This version does not support reporting certificates or proxies yet.
If the stream claims to support this feature, we can let the transport set the proxy. We also set HTTPPROXYTUNNEL option so curl can create a tunnel through the proxy which lets us create our own TLS session (if needed).
Of the built-in ones, only cURL support it, but there's no reason a user-provided stream wouldn't support it.
The information is exposed by curl for some crypto libraries in the form of name:content strings. We can't do much more than return this information.
When linking against libcurl, use it as the underlying transport instead of straight sockets. We can't quite just give over the file descriptor, as curl puts it into non-blocking mode, so we build a custom BIO so OpenSSL sends the data through our stream, be it the socket or curl streams.
The TLS streams talk over the curl stream themselves, so we don't need to ask for it explicitly. Do so in the case of the non-encrypted one so we can still make use proxies in that case.
We do not want libcurl to perform the TLS negotiation for us, so we don't need to pass this option.
If the libcurl stream is available, use that as the underlying stream instead of the socket stream. This allows us to set a proxy for HTTPS connections.
carlosmn
force-pushed
the
cmn/curl-stream
branch
from
00225f9
to
58ca8c7
Compare
cURL has a mode in which it provides TLS and proxy setup but otherwise allows us to use one of its handles very much like our own streams, so it's fairly simple to make the library use libcurl for talking to the server.