Skip to content
New issue

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

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

Already on GitHub? # to your account

Use JDK platform APIs for ALPN negotiation #522

Merged
merged 10 commits into from
Apr 24, 2021

Conversation

vasilmkd
Copy link
Contributor

@vasilmkd vasilmkd commented Apr 15, 2021

Before (curl output):

vasil@vasil-mac ~ % curl -v -k https://127.0.0.1:8443/
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/cert.pem
  CApath: none
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-RSA-AES256-GCM-SHA384
* ALPN, server did not agree to a protocol                    ### <----------------------------------------
* Server certificate:
*  subject: C=Unknown; ST=Unknown; L=Unknown; O=Unknown; OU=Unknown; CN=Unknown
*  start date: Dec  7 21:49:01 2014 GMT
*  expire date: Dec  2 21:49:01 2015 GMT
*  issuer: C=Unknown; ST=Unknown; L=Unknown; O=Unknown; OU=Unknown; CN=Unknown
*  SSL certificate verify result: self signed certificate (18), continuing anyway.
> GET / HTTP/1.1
> Host: 127.0.0.1:8443
> User-Agent: curl/7.64.1
> Accept: */*
> 
< HTTP/1.1 200 OK                    ### <----------------------------------------
< content-type: text/plain; charset=utf-8
< date: Fri, 16 Apr 2021 13:07:16 GMT
< content-length: 104
< 
Hello world!
Path: /
Headers
["Host", "127.0.0.1:8443"]
["User-Agent", "curl/7.64.1"]
["Accept", "*/*"]
* Connection #0 to host 127.0.0.1 left intact
* Closing connection 0

After (curl output):

vasil@vasil-mac ~ % curl -v -k https://127.0.0.1:8443/
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/cert.pem
  CApath: none
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-RSA-AES256-GCM-SHA384
* ALPN, server accepted to use h2                    ### <----------------------------------------
* Server certificate:
*  subject: C=Unknown; ST=Unknown; L=Unknown; O=Unknown; OU=Unknown; CN=Unknown
*  start date: Dec  7 21:49:01 2014 GMT
*  expire date: Dec  2 21:49:01 2015 GMT
*  issuer: C=Unknown; ST=Unknown; L=Unknown; O=Unknown; OU=Unknown; CN=Unknown
*  SSL certificate verify result: self signed certificate (18), continuing anyway.
* Using HTTP2, server supports multi-use                    ### <----------------------------------------
* Connection state changed (HTTP/2 confirmed)                    ### <----------------------------------------
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* Using Stream ID: 1 (easy handle 0x7feeaa00c400)
> GET / HTTP/2                    ### <----------------------------------------
> Host: 127.0.0.1:8443
> User-Agent: curl/7.64.1
> Accept: */*
> 
* Connection state changed (MAX_CONCURRENT_STREAMS == 100)!
< HTTP/2 200                     ### <----------------------------------------
< content-length: 77
< content-type: text/plain; charset=utf-8
< 
Hello world!
Path: /
Headers
["user-agent", "curl/7.64.1"]
["accept", "*/*"]
* Connection #0 to host 127.0.0.1 left intact
* Closing connection 0

@vasilmkd
Copy link
Contributor Author

The tests are passing for me reliably when running them locally. They also passed once before in CI, because I got to the mima checks. I can't tell if there's an issue with my changes.

@vasilmkd
Copy link
Contributor Author

It seems the same is happening in #520.

@rossabaker
Copy link
Member

Oh, uh, @hamnis (I think) found something related to a localhost change in http4s main repo.

@rossabaker
Copy link
Member

The failing tests might be http4s/http4s@2836af0.

@rossabaker
Copy link
Member

Nope, those tests run on the wildcard address.

@vasilmkd vasilmkd marked this pull request as draft April 16, 2021 09:53
@vasilmkd
Copy link
Contributor Author

There's a better solution on the server part, I'll keep iterating on this.

@vasilmkd vasilmkd marked this pull request as ready for review April 16, 2021 12:36
@vasilmkd
Copy link
Contributor Author

I'm actually really happy how this turned out. I anticipated it would be much worse to do the switch.

@vasilmkd
Copy link
Contributor Author

vasilmkd commented Apr 16, 2021

Will rebase on #524 when that is merged.

Edit: Rebased.

Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nice.


### Requirements
Blaze requires a modern, supported version of the JVM to build and run, as it relies on server APIs unavailable before
[JDK8u252](https://webtide.com/jetty-alpn-java-8u252/). Any JDK newer than JDK8u252, including 9+ is supported.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is only strictly true for HTTP/2, but it's such good advice anyway, I'm happy not to qualify it.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants