use a case-insenstive header hash for upgrade server #67

kenichi · 2016-10-18T18:09:18Z

nghttp sends upgrade: h2c because HTTP/1.x header keys are case insensitive, so this line was failing.

coveralls · 2016-10-18T18:12:33Z

Coverage remained the same at 96.913% when pulling 4f25eb7 on kenichi:header_key_case into bf4b930 on igrigorik:master.

igrigorik · 2016-10-19T00:58:43Z

example/upgrade_server.rb

    log = Logger.new(stream.id)
-    req, buffer = {}, ''
+    request_headers = {}
+    req = request_header_hash(request_headers)


Do we actually need both req and request_headers? Can we simplify it by removing one of them? Also we're passing in an empty value which is also the default on the method..

yes of course! 😊

georgeu2000 · 2016-10-21T01:16:39Z

@kenichi - How does require 'http_parser' work for you? I tried 3 different gems and none works for me. Is it a gem?

Which version of Ruby are you using?

igrigorik · 2016-10-21T03:04:42Z

@georgeu2000 you're looking for: https://github.com/tmm1/http_parser.rb

georgeu2000 · 2016-10-21T13:32:40Z

@igrigorik - OK, thanks. I think we should add it to the Gemfile or .gemspec so other people can find it.

igrigorik · 2016-10-21T15:15:19Z

Adding it to a Gemfile in examples dir sgtm.

georgeu2000 · 2016-10-21T21:36:03Z

@igrigorik - The http_parser.rb gem worked. Thank you.

I am still working on getting the upgrade server example to work with Chrome or Firefox. Do I need to use nghttp? It seems that http-2 is a server, so not sure why nghttp is being used, except as a reverse proxy.
EDIT: I see nghttp can be used as a client, so never mind.

Does the upgrade server example work with Chrome or Firefox? I made some updates (for which I can do a PR), and it works with CURL, on both http and https. But it doesn't work on Chrome or Firefox.

Here is what is happening on Chrome. I have done a lot of research, but the issue is still not clear to me. I tried debugging with Wireshark, and there is a TLS 1.2 connection and 5 data packets.

[Stream 1]: Received GET request
Sent frame: {:type=>:headers, :flags=>[:end_headers], :payload=>[[":status", "200"], ["content-length", "27"], ["content-type", "text/plain"]], :stream=>1}
Sent frame: {:type=>:data, :flags=>[], :payload=>"Hello", :stream=>1}
Sent frame: {:type=>:data, :flags=>[:end_stream], :payload=>" HTTP 2.0! GET request", :stream=>1}
[Stream 1]: stream closed

Chrome hangs waiting for the response to finish. The response is interpreted as an octet stream, not plain text. Here is the downloaded file:

��d��à\�27_áI|•äË�™��Hello�� HTTP 2.0! GET request

So, it seems that the headers (which are correct) {:type=>:headers, :flags=>[:end_headers], :payload=>[[":status", "200"], ["content-length", "27"], ["content-type", "text/plain"]], :stream=>1} are not being interpreted as HTTP/2 compressed headers.

Does anyone have any idea what the issue could be?

EDIT: Here is the downloaded file:

00000000:  0000 0604 0000 0000 0000 0300 0000 6400 000e 0104 0000 0001  :..............d.........
00000018:  885c 0232 375f 8749 7ca5 8ae8 19aa 0000 1b00 0100 0000 0148  :.\.27_.I|..............H
00000030:  656c 6c6f 2048 5454 5020 322e 3021 2047 4554 2072 6571 7565  :ello HTTP 2.0! GET reque
00000048:  7374                                                         :st

igrigorik · 2016-10-21T23:42:49Z

Does the upgrade server example work with Chrome or Firefox? I made some updates (for which I can do a PR), and it works with CURL, on both http and https. But it doesn't work on Chrome or Firefox.

No... Neither Chrome, Firefox, Edge, or Safari support h2c upgrade flow. All are TLS only. This is not a bug, this an intentional decision.

georgeu2000 · 2016-10-22T03:24:04Z

@igrigorik - OK, so I guess (hope) server.rb --secure supports ALPN for HTTP/2 (and TLS1.2) and works with Chrome or Firefox.

CURL works fine:

curl -vk --http2  --cert example/keys/server.crt --key example/keys/server.key https://localhost:8080/
...
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / DHE-RSA-AES256-GCM-SHA384
* ALPN, server accepted to use h2
* Server certificate:
*  subject: C=AU; ST=Some-State; O=Internet Widgits Pty Ltd; CN=localhost
*  start date: Jun  6 19:43:25 2016 GMT
*  expire date: Jun  6 19:43:25 2017 GMT
*  issuer: C=AU; ST=Some-State; O=Internet Widgits Pty Ltd; CN=localhost
*  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 0x7fc7b3000000)
> GET / HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.50.3
> Accept: */*
>
* Connection state changed (MAX_CONCURRENT_STREAMS updated)!
< HTTP/2 200
< content-length: 27
< content-type: text/plain
<
* Curl_http_done: called premature == 0
* Connection #0 to host localhost left intact
Hello HTTP 2.0! GET request

But on Chrome,

https://localhost:8080/
Received frame: {:length=>12, :type=>:settings, :flags=>[], :stream=>0, :payload=>[[:settings_max_concurrent_streams, 1000], [:settings_initial_window_size, 6291456]]}
Sent frame: {:type=>:settings, :stream=>0, :payload=>[], :flags=>[:ack]}
Writing bytes: 000000040100000000
Sent frame: {:type=>:goaway, :last_stream=>0, :error=>:protocol_error, :payload=>nil}
Writing bytes: 0000080700000000000000000000000001
OpenSSL::SSL::SSLError exception: SSL_write: bad write retry - closing socket.

Not sure how to figure out what is going wrong. It seems that the server is sending goaway, but the socket is closed...?

igrigorik · 2016-10-22T03:31:18Z

I'm assuming you're using a self-signed cert.. Did you whitelist it in your keychain or equivalent?

Also, check chrome://net-internals/#http2 for the session, it should be provide some additional context for why Chrome is closing the connection.

georgeu2000 · 2016-10-22T14:50:58Z

@igrigorik - Yes, it is a self-signed cert and added it to Keychain. chrome://net-internals/#events shows ERR_SPDY_INADEQUATE_TRANSPORT_SECURITY. Googling suggests that it is an issue with the ciphers, so I changed them, but it did not help. I'll keep digging.

It works OK on Safari, so that's a good start. Thank you for all your help.

kenichi · 2016-10-22T18:35:43Z

@georgeu2000 i am having the same problem with chrome using a valid letsencrypt cert, and i'm looking at the cipher suite now.

georgeu2000 · 2016-10-22T22:11:19Z

@kenichi - OK, cool. It *seems* to be an issue with the cipher suite, but I'm not sure exactly what the problem is because the error from Chrome does not expose it.

I upgraded Firefox to v49, and it now shows the same error. It may be that if the server offers a suite that is on the blacklist, then it raises this error.

Here are the 97 default ciphers:

ECDHE-RSA-AES256-GCM-SHA384 TLSv1/SSLv3 256 256
ECDHE-ECDSA-AES256-GCM-SHA384 TLSv1/SSLv3 256 256
ECDHE-RSA-AES256-SHA384 TLSv1/SSLv3 256 256
ECDHE-ECDSA-AES256-SHA384 TLSv1/SSLv3 256 256
ECDHE-RSA-AES256-SHA TLSv1/SSLv3 256 256
ECDHE-ECDSA-AES256-SHA TLSv1/SSLv3 256 256
SRP-DSS-AES-256-CBC-SHA TLSv1/SSLv3 256 256
SRP-RSA-AES-256-CBC-SHA TLSv1/SSLv3 256 256
SRP-AES-256-CBC-SHA TLSv1/SSLv3 256 256
DH-DSS-AES256-GCM-SHA384 TLSv1/SSLv3 256 256
DHE-DSS-AES256-GCM-SHA384 TLSv1/SSLv3 256 256
DH-RSA-AES256-GCM-SHA384 TLSv1/SSLv3 256 256
DHE-RSA-AES256-GCM-SHA384 TLSv1/SSLv3 256 256
DHE-RSA-AES256-SHA256 TLSv1/SSLv3 256 256
DHE-DSS-AES256-SHA256 TLSv1/SSLv3 256 256
DH-RSA-AES256-SHA256 TLSv1/SSLv3 256 256
DH-DSS-AES256-SHA256 TLSv1/SSLv3 256 256
DHE-RSA-AES256-SHA TLSv1/SSLv3 256 256
DHE-DSS-AES256-SHA TLSv1/SSLv3 256 256
DH-RSA-AES256-SHA TLSv1/SSLv3 256 256
DH-DSS-AES256-SHA TLSv1/SSLv3 256 256
DHE-RSA-CAMELLIA256-SHA TLSv1/SSLv3 256 256
DHE-DSS-CAMELLIA256-SHA TLSv1/SSLv3 256 256
DH-RSA-CAMELLIA256-SHA TLSv1/SSLv3 256 256
DH-DSS-CAMELLIA256-SHA TLSv1/SSLv3 256 256
ECDH-RSA-AES256-GCM-SHA384 TLSv1/SSLv3 256 256
ECDH-ECDSA-AES256-GCM-SHA384 TLSv1/SSLv3 256 256
ECDH-RSA-AES256-SHA384 TLSv1/SSLv3 256 256
ECDH-ECDSA-AES256-SHA384 TLSv1/SSLv3 256 256
ECDH-RSA-AES256-SHA TLSv1/SSLv3 256 256
ECDH-ECDSA-AES256-SHA TLSv1/SSLv3 256 256
AES256-GCM-SHA384 TLSv1/SSLv3 256 256
AES256-SHA256 TLSv1/SSLv3 256 256
AES256-SHA TLSv1/SSLv3 256 256
CAMELLIA256-SHA TLSv1/SSLv3 256 256
PSK-AES256-CBC-SHA TLSv1/SSLv3 256 256
ECDHE-RSA-AES128-GCM-SHA256 TLSv1/SSLv3 128 128
ECDHE-ECDSA-AES128-GCM-SHA256 TLSv1/SSLv3 128 128
ECDHE-RSA-AES128-SHA256 TLSv1/SSLv3 128 128
ECDHE-ECDSA-AES128-SHA256 TLSv1/SSLv3 128 128
ECDHE-RSA-AES128-SHA TLSv1/SSLv3 128 128
ECDHE-ECDSA-AES128-SHA TLSv1/SSLv3 128 128
SRP-DSS-AES-128-CBC-SHA TLSv1/SSLv3 128 128
SRP-RSA-AES-128-CBC-SHA TLSv1/SSLv3 128 128
SRP-AES-128-CBC-SHA TLSv1/SSLv3 128 128
DH-DSS-AES128-GCM-SHA256 TLSv1/SSLv3 128 128
DHE-DSS-AES128-GCM-SHA256 TLSv1/SSLv3 128 128
DH-RSA-AES128-GCM-SHA256 TLSv1/SSLv3 128 128
DHE-RSA-AES128-GCM-SHA256 TLSv1/SSLv3 128 128
DHE-RSA-AES128-SHA256 TLSv1/SSLv3 128 128
DHE-DSS-AES128-SHA256 TLSv1/SSLv3 128 128
DH-RSA-AES128-SHA256 TLSv1/SSLv3 128 128
DH-DSS-AES128-SHA256 TLSv1/SSLv3 128 128
DHE-RSA-AES128-SHA TLSv1/SSLv3 128 128
DHE-DSS-AES128-SHA TLSv1/SSLv3 128 128
DH-RSA-AES128-SHA TLSv1/SSLv3 128 128
DH-DSS-AES128-SHA TLSv1/SSLv3 128 128
DHE-RSA-SEED-SHA TLSv1/SSLv3 128 128
DHE-DSS-SEED-SHA TLSv1/SSLv3 128 128
DH-RSA-SEED-SHA TLSv1/SSLv3 128 128
DH-DSS-SEED-SHA TLSv1/SSLv3 128 128
DHE-RSA-CAMELLIA128-SHA TLSv1/SSLv3 128 128
DHE-DSS-CAMELLIA128-SHA TLSv1/SSLv3 128 128
DH-RSA-CAMELLIA128-SHA TLSv1/SSLv3 128 128
DH-DSS-CAMELLIA128-SHA TLSv1/SSLv3 128 128
ECDH-RSA-AES128-GCM-SHA256 TLSv1/SSLv3 128 128
ECDH-ECDSA-AES128-GCM-SHA256 TLSv1/SSLv3 128 128
ECDH-RSA-AES128-SHA256 TLSv1/SSLv3 128 128
ECDH-ECDSA-AES128-SHA256 TLSv1/SSLv3 128 128
ECDH-RSA-AES128-SHA TLSv1/SSLv3 128 128
ECDH-ECDSA-AES128-SHA TLSv1/SSLv3 128 128
AES128-GCM-SHA256 TLSv1/SSLv3 128 128
AES128-SHA256 TLSv1/SSLv3 128 128
AES128-SHA TLSv1/SSLv3 128 128
SEED-SHA TLSv1/SSLv3 128 128
CAMELLIA128-SHA TLSv1/SSLv3 128 128
IDEA-CBC-SHA TLSv1/SSLv3 128 128
PSK-AES128-CBC-SHA TLSv1/SSLv3 128 128
ECDHE-RSA-RC4-SHA TLSv1/SSLv3 128 128
ECDHE-ECDSA-RC4-SHA TLSv1/SSLv3 128 128
ECDH-RSA-RC4-SHA TLSv1/SSLv3 128 128
ECDH-ECDSA-RC4-SHA TLSv1/SSLv3 128 128
RC4-SHA TLSv1/SSLv3 128 128
RC4-MD5 TLSv1/SSLv3 128 128
PSK-RC4-SHA TLSv1/SSLv3 128 128
ECDHE-RSA-DES-CBC3-SHA TLSv1/SSLv3 112 168
ECDHE-ECDSA-DES-CBC3-SHA TLSv1/SSLv3 112 168
SRP-DSS-3DES-EDE-CBC-SHA TLSv1/SSLv3 112 168
SRP-RSA-3DES-EDE-CBC-SHA TLSv1/SSLv3 112 168
SRP-3DES-EDE-CBC-SHA TLSv1/SSLv3 112 168
EDH-RSA-DES-CBC3-SHA TLSv1/SSLv3 112 168
EDH-DSS-DES-CBC3-SHA TLSv1/SSLv3 112 168
DH-RSA-DES-CBC3-SHA TLSv1/SSLv3 112 168
DH-DSS-DES-CBC3-SHA TLSv1/SSLv3 112 168
ECDH-RSA-DES-CBC3-SHA TLSv1/SSLv3 112 168
ECDH-ECDSA-DES-CBC3-SHA TLSv1/SSLv3 112 168
DES-CBC3-SHA TLSv1/SSLv3 112 168
PSK-3DES-EDE-CBC-SHA TLSv1/SSLv3 112 168

georgeu2000 · 2016-10-23T02:36:03Z

This is the issue:

ctx.tmp_ecdh_callback = lambda do |*args|
  OpenSSL::PKey::EC.new 'prime256v1'
end

_args -> *args
I had commented it out because it was failing, but didn't mention it.

kenichi · 2016-10-27T16:59:12Z

i also noted that curl with h2 support sends HTTP2-Settings so the h1 upgrade handler needs the case-insensitive hash as well.

coveralls · 2016-10-27T17:01:20Z

Coverage remained the same at 96.913% when pulling fb31af5 on kenichi:header_key_case into bf4b930 on igrigorik:master.

igrigorik · 2016-10-27T17:11:41Z

@kenichi overall, lgtm, modulo rubocop nitpicks: https://travis-ci.org/igrigorik/http-2/jobs/171152501#L712

coveralls · 2016-10-27T17:19:57Z

Coverage remained the same at 96.913% when pulling dbc8fbb on kenichi:header_key_case into bf4b930 on igrigorik:master.

kenichi · 2016-10-27T17:24:30Z

@georgeu2000 thanks for the info, i will be looking at incorporating this into my work here

@igrigorik updated to make rubocop happy but still running into some problems - will look into this in another issue/pr

coveralls · 2016-10-27T17:27:48Z

Coverage remained the same at 96.913% when pulling 5dc1e28 on kenichi:header_key_case into 7ab50f5 on igrigorik:master.

igrigorik · 2016-10-27T17:36:09Z

Coveralls spam is getting out of hand..

Thanks for the fix, merging!

igrigorik reviewed Oct 19, 2016

View reviewed changes

use a case-insenstive header hash for upgrade server

5dc1e28

kenichi force-pushed the header_key_case branch from 64f495e to 5dc1e28 Compare October 27, 2016 17:23

igrigorik merged commit 0ab8173 into igrigorik:master Oct 27, 2016

kenichi deleted the header_key_case branch October 28, 2016 17:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use a case-insenstive header hash for upgrade server #67

use a case-insenstive header hash for upgrade server #67

kenichi commented Oct 18, 2016

coveralls commented Oct 18, 2016

igrigorik Oct 19, 2016

kenichi Oct 22, 2016

georgeu2000 commented Oct 21, 2016

igrigorik commented Oct 21, 2016

georgeu2000 commented Oct 21, 2016

igrigorik commented Oct 21, 2016

georgeu2000 commented Oct 21, 2016 •

edited

igrigorik commented Oct 21, 2016

georgeu2000 commented Oct 22, 2016

igrigorik commented Oct 22, 2016

georgeu2000 commented Oct 22, 2016

kenichi commented Oct 22, 2016

georgeu2000 commented Oct 22, 2016

georgeu2000 commented Oct 23, 2016

kenichi commented Oct 27, 2016

coveralls commented Oct 27, 2016

igrigorik commented Oct 27, 2016

coveralls commented Oct 27, 2016

kenichi commented Oct 27, 2016

coveralls commented Oct 27, 2016 •

edited

igrigorik commented Oct 27, 2016

use a case-insenstive header hash for upgrade server #67

use a case-insenstive header hash for upgrade server #67

Conversation

kenichi commented Oct 18, 2016

coveralls commented Oct 18, 2016

igrigorik Oct 19, 2016

Choose a reason for hiding this comment

kenichi Oct 22, 2016

Choose a reason for hiding this comment

georgeu2000 commented Oct 21, 2016

igrigorik commented Oct 21, 2016

georgeu2000 commented Oct 21, 2016

igrigorik commented Oct 21, 2016

georgeu2000 commented Oct 21, 2016 • edited

igrigorik commented Oct 21, 2016

georgeu2000 commented Oct 22, 2016

igrigorik commented Oct 22, 2016

georgeu2000 commented Oct 22, 2016

kenichi commented Oct 22, 2016

georgeu2000 commented Oct 22, 2016

georgeu2000 commented Oct 23, 2016

kenichi commented Oct 27, 2016

coveralls commented Oct 27, 2016

igrigorik commented Oct 27, 2016

coveralls commented Oct 27, 2016

kenichi commented Oct 27, 2016

coveralls commented Oct 27, 2016 • edited

igrigorik commented Oct 27, 2016

georgeu2000 commented Oct 21, 2016 •

edited

coveralls commented Oct 27, 2016 •

edited