New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] quic #3314

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@bagder
Copy link
Member

bagder commented Nov 27, 2018

This is early (and incomplete) work on quic support. Pushed as a PR only to make sure the work so far hasn't broken the build in unrelated parts (by running it through the CI tests).

The first steps of the plan;

  • - make all QUIC code conditional on the detection of ngtcp2 at build time
  • - support ngtcp2 in configure
  • - support ngtcp2 in cmake
  • - show "HTTP3" as a feature in --version output
  • - show ngtcp2 in the version string for third party libs
  • - figure out how to show the correct ngtcp2 version number in that string
  • - support CURLOPT_H3 and the CURLH3_DIRECT flag
  • - support --http3-direct in the command line tool
  • - actually connect using QUIC when CURLH3_DIRECT is set
  • - disconnect QUIC

Alt-Svc support (for HTTP/3 and other versions) is being worked on separately by @xquery.

There's no general HTTP/3 support for ngtcp2 available yet.

Wiki page for QUIC implementation thoughts

@bagder bagder force-pushed the ngtcp2 branch 3 times, most recently from abe5201 to 7228506 Dec 3, 2018

@gvanem

This comment has been minimized.

Copy link
Member

gvanem commented Dec 8, 2018

Ops, sorry for the edit (undid it). But I'll try to build with this.

Show resolved Hide resolved lib/quic.c
return NULL;
}

SSL_CTX_set_mode(ssl_ctx, SSL_MODE_QUIC_HACK);

This comment has been minimized.

@gvanem

gvanem Dec 11, 2018

Member

I get this error:

quic.c(269,29):  error: use of undeclared identifier 'SSL_MODE_QUIC_HACK'
@gvanem

This comment has been minimized.

Copy link
Member

gvanem commented Dec 12, 2018

Finally got a version to try. But a curl --http3-direct -v https://www.google.com just creates a lot of SOCK_DGRAM sockets and then gives up with curl: (2) Failed initialization. But I know next to nothing about QUIC.

But an issue when building with -DUSE_SCHANNEL and this Tatsuhiro OpenSSL is the clash of <wincrypt.h> names (just like in BoringSSL). I did this to fix:

--- a/lib/vtls/schannel.h 2018-11-02 23:47:24
+++ b/vtls/schannel.h 2018-12-12 16:13:40
@@ -45,10 +45,12 @@
  * BoringSSL's <openssl/x509.h>: So just undefine those defines here
  * (and only here).
  */
-#if defined(HAVE_BORINGSSL) || defined(OPENSSL_IS_BORINGSSL)
+#if defined(HAVE_BORINGSSL) || defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_Tatsuhiro)
 # undef X509_NAME
 # undef X509_CERT_PAIR
 # undef X509_EXTENSIONS
+# undef OCSP_REQUEST
+# undef OCSP_RESPONSE
 #endif

Anyway, this was the OpenSSL I had to use and build with:

 git clone --recursive --branch quic-draft-15 https://github.com/tatsuhiro-t/openssl.git .

Any hope this will crawl it's way into the official OpenSSL? I hate to have several OpenSSL beasts on my disk.

Show resolved Hide resolved lib/quic.c Outdated
@bagder

This comment has been minimized.

Copy link
Member Author

bagder commented Dec 13, 2018

Finally got a version to try. But a curl --http3-direct -v https://www.google.com just creates a lot of SOCK_DGRAM sockets and then gives up with curl: (2) Failed initialization. But I know next to nothing about QUIC.

Right, I've written a lot of glue code but I've not yet made it work. It's really complicated for me to work out where any problem might lie since I too lack the in-depth knowledge of QUIC, ngtcp2 and TLS that's required. My work so far has been to see what the client.cc code does and then converting that into C code for curl.

It also just fires off a single function call to ngtcp2 so it doesn't even try to do the right thing yet.

But an issue when building with -DUSE_SCHANNEL and this

I've decided to start off with building with OpenSSL only and then only the Tatsuhiro OpenSSL version. I figure we'll deal with multi-SSL and other-SSL issues later.

Any hope this will crawl it's way into the official OpenSSL?

Let's hope so. Otherwise we'll have a really painful situation.

@bagder bagder force-pushed the ngtcp2 branch from 08aa21e to 7e72dd7 Dec 13, 2018

@tatsuhiro-t

This comment has been minimized.

Copy link
Contributor

tatsuhiro-t commented Dec 13, 2018

I don't think google is deploying IETF QUIC which is still in draft stage, and changing rapidly and drastically.
Here is the existing implementations and its endpoint: https://github.com/quicwg/base-drafts/wiki/Implementations
Note that the information might not be up to date.

For OpenSSL stuff, someone has to ask OpenSSL dev to implement required features perhaps after QUIC draft is sent to IESG. I heard that BoringSSL has an implementation for these features, let's hope that they can be added to OpenSSL. I don't know about LibreSSL. Supporting similar but slightly different TLS backends is just a pain.

@gvanem

This comment has been minimized.

Copy link
Member

gvanem commented Dec 13, 2018

It also just fires off a single function call to ngtcp2 so it doesn't even try to do the right thing yet.

Same here. Using Tatsuhiro's test server:

curl.exe -v4 --http3-direct https://nghttp2.org:4433

I get an error in setup_initial_crypto_context() and Curl_qc_derive_initial_secret() (line 69 of quic.c).
And nothing gets sent.

No idea what the hkdf_extract() is all about.

Show resolved Hide resolved docs/HTTP3.md Outdated
Show resolved Hide resolved docs/cmdline-opts/http3-direct.d Outdated

@bagder bagder force-pushed the ngtcp2 branch from 7e72dd7 to 93afbbc Dec 14, 2018

#endif
/* ngtcp2 master branch uses version NGTCP2_PROTO_VER_D14 */
rc = ngtcp2_conn_client_new(&qs->conn, &qs->dcid, &qs->scid,
QUICVER, &qs->callbacks, &qs->settings, conn);

This comment has been minimized.

@tatsuhiro-t

tatsuhiro-t Dec 15, 2018

Contributor

It looks like qs->dcid and qs->scid are not initialized.
They must be some random bytes.

result = Curl_quic_connect(conn, sockfd, &addr.sa_addr, addr.addrlen);
if(result)
return result;
}

This comment has been minimized.

@tatsuhiro-t

tatsuhiro-t Dec 15, 2018

Contributor

It looks like rc is still -1, which makes this function fail.

@bagder bagder force-pushed the ngtcp2 branch from f5d87b1 to 06de3bc Dec 17, 2018

@bagder bagder force-pushed the ngtcp2 branch from 06de3bc to 0d113ba Jan 4, 2019

@bagder bagder force-pushed the ngtcp2 branch 2 times, most recently from 3379309 to 4c7e831 Jan 13, 2019

bagder added some commits Nov 24, 2018

configure: add support for --with-ngtcp2
The output currently says this means http3 support, which strictly isn't
true since ngtcp2 is QUIC only, but going forward I don't think we need
to separate QUIC and HTTP3 for configure users. Even if we might need to
require two separate libs to consider h3 enabled eventually. HTTP/3 will
simply imply QUIC.

@bagder bagder force-pushed the ngtcp2 branch from 4c7e831 to bfc3d65 Jan 18, 2019

@ghedo

This comment has been minimized.

Copy link
Member

ghedo commented Jan 19, 2019

Shameless plug, but I hacked together QUIC support for curl using my own quiche library (keyword is "hacked"). I based my changes on the branch from this PR and replaced the ngtcp2-specific bits and a few other things.

The end result (but still incompete and WIP) can be seen at https://github.com/ghedo/curl/tree/quiche (build instructions in the commit message)

I've got to the point where I can do a single HTTP/1.1 request and get a response from a QUIC server:

 % src/curl --http3-direct https://cloudflare-quic.com/   
       ____    
      / __ \   
     / / / /   
    / /_/ /    
    \___\_\    
       __  __  
      / / / /  
     / / / /   
    / /_/ /    
    \____/     
        ____   
       /  _/   
       / /     
     _/ /      
    /___/      
      ______   
     / ____/   
    / /        
   / /___      
   \____/

Compared to ngtcp2, quiche requires a lot less glue code, but on the other hand it depends on BoringSSL (you can actually use latest BoringSSL master, so no additional patches are required, but still), so building is a bit more complex since right now curl also needs to be built against bssl to avoid conflicts with OpenSSL.

@bagder

This comment has been minimized.

Copy link
Member Author

bagder commented Jan 19, 2019

Awesome @ghedo, I will jump in and try this out soonish.

I would like to make sure that we unify our efforts and make sure that we can both move forward and still have the same take on quic in general within curl and make sure that we can build/use either library according to configure options.

@ghedo

This comment has been minimized.

Copy link
Member

ghedo commented Jan 20, 2019

@bagder Yes, I agree. This was mostly an exercise to check if the quiche API can be integrated into existing code, and it takes some short-cuts so I'm not convinced it's the correct way to go. Though I hope that having something that "works" (even if hacky) can make iterating over the curl internals changes easier.

@bagder

This comment has been minimized.

Copy link
Member Author

bagder commented Jan 20, 2019

I'll clean up my work a little bit and make sure we can build and use it with either ngtcp2 or quiche so that we can experiment better going forward. Gimme a day or two and then I'll present something that should be a little better foundation to build on. I think we'll benefit from that and I really like that we can already do HTTP requests with your branch.

@bagder

This comment has been minimized.

Copy link
Member Author

bagder commented Jan 21, 2019

@ghedo can you give my branch at https://github.com/curl/curl/tree/QUIC a glance and see if you can figure out how I botched quiche from working there as nicely as in your repo?

It is my attempt in making QUIC supported by functions that can be implemented by either of the backends with both ngtcp2 and (your) quiche code in the repo.

@ghedo

This comment has been minimized.

Copy link
Member

ghedo commented Jan 21, 2019

The problem seems to be in singleipconnect(). In my changes I made the connect() syscall happen on the UDP socket as well, because otherwise I couldn't figure out where to get the peer address to do sendto() instead of just send()

Basically you are missing the following change from my branch:

diff --git a/lib/connect.c b/lib/connect.c
index b11df23cb..8db5a0489 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -1076,7 +1076,7 @@ static CURLcode singleipconnect(struct connectdata *conn,
     Curl_expire(data, conn->timeoutms_per_addr, EXPIRE_DNS_PER_NAME);
 
   /* Connect TCP sockets */
-  if(!isconnected && (conn->transport == TRNSPRT_TCP)) {
+  if(!isconnected) {
     if(conn->bits.tcp_fastopen) {
 #if defined(CONNECT_DATA_IDEMPOTENT) /* Darwin */
 #  if defined(HAVE_BUILTIN_AVAILABLE)
@@ -1123,18 +1123,19 @@ static CURLcode singleipconnect(struct connectdata *conn,
     if(-1 == rc)
       error = SOCKERRNO;
   }
+  else {
+    *sockp = sockfd;
+    return CURLE_OK;
+  }
+
 #ifdef ENABLE_QUIC
-  else if(!isconnected && (conn->transport == TRNSPRT_QUIC)) {
+  if(!isconnected && (conn->transport == TRNSPRT_QUIC)) {
     result = Curl_quic_connect(conn, sockfd, &addr.sa_addr, addr.addrlen);
     if(result)
       return result;
     rc = 0; /* connect success */
   }
 #endif
-  else {
-    *sockp = sockfd;
-    return CURLE_OK;
-  }
 
   if(-1 == rc) {
     switch(error) {

This is one of the hacks I had to do to get this going. IIRC the ip_addr and tempaddr fields of struct connectdata weren't fully initialized for some reason, so I just made the socket be "connected".

@lilianmoraru

This comment has been minimized.

Copy link

lilianmoraru commented Jan 22, 2019

@bagder Note that the instructions for compiling both BoringSSL and quiche suggest compiling without optimizations(if it was intentional, please ignore the next lines 😄 ).
For BoringSSL:

cmake -DCMAKE_POSITION_INDEPENDENT_CODE=on -DCMAKE_BUILD_TYPE=RelWithDebInfo .. # for an optimized build with debug info
cmake -DCMAKE_POSITION_INDEPENDENT_CODE=on -DCMAKE_BUILD_TYPE=Release .. # for an optimized build _without_ debug info

For quiche:

QUICHE_BSSL_PATH=$PWD/deps/boringssl cargo build --release # for an optimized build with debug info(`quiche` sets debug = true in `Cargo.toml`)
QUICHE_BSSL_PATH=$PWD/deps/boringssl RUSTFLAGS="-C debuginfo=0" cargo build --release # optimized + forced removal of debug info
@bagder

This comment has been minimized.

Copy link
Member Author

bagder commented Jan 25, 2019

@lilianmoraru Thanks! But I don't think we need to provide the optimal build instructions in here, just something that can get you going to start develop with this. For optimizing their respective builds I trust people will check the docs for each specific library. It might even be more clever to use debug-versions of them while taking the first steps into QUIC development...

@bagder

This comment has been minimized.

Copy link
Member Author

bagder commented Jan 25, 2019

Thanks a lot @ghedo, I adjusted that patch a little, amended my commits and forced-pushed my QUIC branch just now. Since PRs are tied so much to a specific branch, I'll close this and create a new one for that branch which now is a more suitable way forward.

@bagder bagder closed this Jan 25, 2019

@bagder bagder referenced this pull request Jan 25, 2019

Open

[WIP] QUIC and HTTP/3 #3500

10 of 14 tasks complete

@bagder bagder deleted the ngtcp2 branch Jan 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment