Skip to content
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

net/http: Server rejects CONNECT requests without a Host header, per the spec #18215

Closed
johnmah opened this issue Dec 6, 2016 · 19 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@johnmah
Copy link

johnmah commented Dec 6, 2016

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.7.4 (problem exists on master branch as well)

What operating system and processor architecture are you using (go env)?

linux-amd64

What did you do?

Sample code here: https://play.golang.org/p/dpLc5rnHJZ

We have created a proxy server using the net/http package. If a HTTP client using the proxy server issues a CONNECT request without a Host: header, the golang HTTP server implementation will close the connection with 400 Bad Request: missing required Host header error.

CONNECT api.surfeasy.com:443 HTTP/1.1
...

as opposed to:

CONNECT api.surfeasy.com:443 HTTP/1.1
Host: api.surfeasy.com:443
...

Now this could be a contentious issue as the HTTP 1.1 spec (see https://tools.ietf.org/html/rfc7230#section-5.4) specifies that a Host header is mandatory, but we see some VERY popular devices (i.e.: iOS devices) implement proxy clients that do not send the Host: header when configured to use proxies (and as a result issue CONNECT requests). This could be an artifact of interop with older HTTP 1.0 proxies.

It appears that the HTTP request code in src/net/http/request.go will synthesize a Host attribute based on the CONNECT parameters (authority-form) and ignores any Host: header, so relaxing the Host header restriction would allow for better compatibility. As it stands now, any HTTP server implementing some form of CONNECT handler might not work with a large range of devices.

What did you expect to see?

Client CONNECT request completes as expected and leaves connection open for further requests.

What did you see instead?

net/http Server implementation returns a 400 Bad Request: missing required Host header and closes the connection.

@bradfitz bradfitz changed the title net/http CONNECT closes client connections if no Host: header is specified net/http: Server rejects CONNECT requests without a Host header, per the spec Dec 6, 2016
@bradfitz bradfitz added this to the Unplanned milestone Dec 6, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Dec 6, 2016

Does iOS itself have this bug, or certain app(s)?

Did an older version of various specs permit CONNECT without a host header?

I'm inclined to say that you can use http.ReadRequest and handle this yourself if you're a proxy supporting buggy clients, unless you can convince me that this is widespread.

/cc @tombergan

@bradfitz bradfitz added Thinking WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Dec 6, 2016
@johnmah
Copy link
Author

johnmah commented Dec 6, 2016

I am going to need some more time to look at some proper tcpdumps but we see this issue with iOS (set to use a HTTP proxy written in Go) and the native iOS Facebook app. I have heard rumours that Facebook may bypass the default iOS networking stack but I don't have any concrete evidence.

The original proxy spec written by Ari Luotonen in 1998 did not require a Host header. (see https://tools.ietf.org/html/draft-luotonen-web-proxy-tunneling-01)

I do have a branch that I can submit with a sample 1-line fix.

@bradfitz
Copy link
Contributor

bradfitz commented Dec 6, 2016

draft-luotonen-web-proxy-tunneling-01 shows examples using HTTP/1.0, though.

@bradfitz bradfitz modified the milestones: Go1.9Maybe, Unplanned Dec 6, 2016
@johnmah
Copy link
Author

johnmah commented Dec 6, 2016

I understand that Ari's draft uses HTTP 1.0 but the spec for CONNECT (https://tools.ietf.org/html/rfc2817, 2000) was only superseded by the HTTP 1.1 (https://tools.ietf.org/html/rfc7230, 2014). At the time of RFC2817 CONNECT was a reserved method but not official.

My fix looks like this:

diff --git a/src/net/http/server.go b/src/net/http/server.go
index 6df9c260e4..c0b8914ee1 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -936,7 +936,7 @@ func (c *conn) readRequest(ctx context.Context) (w *response, err error) {
 
        hosts, haveHost := req.Header["Host"]
        isH2Upgrade := req.isH2Upgrade()
-       if req.ProtoAtLeast(1, 1) && (!haveHost || len(hosts) == 0) && !isH2Upgrade {
+       if req.ProtoAtLeast(1, 1) && ((!haveHost || len(hosts) == 0) && len(req.Host) == 0) && !isH2Upgrade {
                return nil, badRequestError("missing required Host header")
        }
        if len(hosts) > 1 {

@bradfitz
Copy link
Contributor

bradfitz commented Dec 6, 2016

Yes, the change itself is trivial. (adding a test will likely be much more code)

This bug isn't about figuring out the change, though, but about making a decision on whether we should, understanding how wide-spread this is or if it was ever allowed.

If you want a change to go in, convince me of either:

  • this is widespread
  • an RFC used to allow this (HTTP/1.1 + CONNECT with no host header)

Related: #14952 is about adding opt-in knobs for accepting malformed HTTP.

@johnmah
Copy link
Author

johnmah commented Dec 6, 2016

Understood. We will have more data in the upcoming weeks as we start more in-depth testing across the entire app ecosystem. Breaking the native Facebook app is a doozy though.

@bradfitz
Copy link
Contributor

bradfitz commented Dec 6, 2016

Facebook might be big enough on its own to make this worth changing.

Let's still figure out whose networking stack it is first. Then, if Facebook, we can ask Facebook to fix it, or at least see what they say. Maybe they'll just fix it quickly.

If it's Apple, that's a bit harder, and almost certainly worth adding a special case at that point.

@johnmah
Copy link
Author

johnmah commented Dec 6, 2016

I can take care of gathering the data for this issue in the next few days and will update here.

@bradfitz
Copy link
Contributor

bradfitz commented Jan 4, 2017

@johnmah, any update?

@walkert
Copy link
Contributor

walkert commented Mar 21, 2017

Just came here to add that while I'm sure this won't count as widespread, I'm encountering this exact problem with wget-1.13.4-3+deb7u2 (see https://bugs.launchpad.net/ubuntu/+source/wget/+bug/994097). This is in a corporate environment where I can't just go an upgrade to a working version.

@bradfitz
Copy link
Contributor

@walkert, what did you just link me to? I see nothing about Go or CONNECT or HTTP/1.0 or HTTP/1.1 there. Are you on the wrong bug?

@walkert
Copy link
Contributor

walkert commented Mar 22, 2017

@bradfitz, I thought you were looking for more cases of clients that were not sending 'Host' headers when making a CONNECT request. The linked bug relates to version of wget which exhibit this behaviour though I can see reading through it now that it isn't detailed. I'll see if I can dig up something more official.

@bradfitz
Copy link
Contributor

Yeah, that bug doesn't mention "Host" or "header" or "version" or "CONNECT".

Is it tracking an upstream bug or something?

Hah, and the bottom comment from "Admin (ilw10)" is such an internet comment. Yet somehow that angry dude can understand what's happening in that bug more than me.

@walkert
Copy link
Contributor

walkert commented Mar 22, 2017

Hah - yeah that's for sure. OK - so it would seem that wget was updated way back when - (http://git.savannah.gnu.org/cgit/wget.git/commit/src?id=aed7d4163a9e2083d294a9471e1347ab13d6f2ab) but alas the problem I have in my environment is dealing with old sources where I don't have the luxury of upgrading everyone (the version we have available pre-dates that update). I realize you're doing the right thing here and clients should be adhering to the specs, but it would certainly be nice if we had the option to determine how vehemently net/http enforces the rule(s).

Incidentally, I've tried the modification above followed by a 'go build -a' and various other permutations and haven't been able to get the updates into my newly built executable. Would appreciate any pointers (this is on go 1.8). Cheers.

@thomasberger
Copy link

+1 here. Working with legacy embedded devices, many of these also leave out the Host in requests.

@bradfitz
Copy link
Contributor

bradfitz commented May 5, 2017

@thomasberger, any details on those devices or software/versions?

@johnmah
Copy link
Author

johnmah commented May 12, 2017

@bradfitz sorry for not updating earlier but i got pulled into other projects.

not sure what the best way to gather information but i did grab output from tcpdump between a proxied iPhone and the proxy server.

96.45.199.251.54971 = iPhone 10.3.2 handset
138.197.151.254.7070 = proxy server

20:25:22.640140 IP 96.45.199.251.54971 > 138.197.151.254.7070: Flags [S], seq 3925157134, win 65535, options [mss 1460,nop,wscale 5,nop,nop,TS val 203808660 ecr 0,sackOK,eol], length 0
	0x0000:  4500 0040 9529 4000 3c06 5ea2 602d c7fb  E..@.)@.<.^.`-..
	0x0010:  8ac5 97fe d6bb 1b9e e9f5 250e 0000 0000  ..........%.....
	0x0020:  b002 ffff fef8 0000 0204 05b4 0103 0305  ................
	0x0030:  0101 080a 0c25 df94 0000 0000 0402 0000  .....%..........
20:25:22.640367 IP 138.197.151.254.7070 > 96.45.199.251.54971: Flags [S.], seq 3565914418, ack 3925157135, win 28960, options [mss 1460,sackOK,TS val 2895815 ecr 203808660,nop,wscale 8], length 0
	0x0000:  4500 003c 0000 4000 4006 efcf 8ac5 97fe  E..<..@.@.......
	0x0010:  602d c7fb 1b9e d6bb d48b 8932 e9f5 250f  `-.........2..%.
	0x0020:  a012 7120 4b1b 0000 0204 05b4 0402 080a  ..q.K...........
	0x0030:  002c 2fc7 0c25 df94 0103 0308            .,/..%......
20:25:22.647900 IP 96.45.199.251.54971 > 138.197.151.254.7070: Flags [.], ack 1, win 4117, options [nop,nop,TS val 203808668 ecr 2895815], length 0
	0x0000:  4500 0034 f121 4000 3c06 02b6 602d c7fb  E..4.!@.<...`-..
	0x0010:  8ac5 97fe d6bb 1b9e e9f5 250f d48b 8933  ..........%....3
	0x0020:  8010 1015 a0e8 0000 0101 080a 0c25 df9c  .............%..
	0x0030:  002c 2fc7                                .,/.
20:25:22.649324 IP 96.45.199.251.54971 > 138.197.151.254.7070: Flags [P.], seq 1:339, ack 1, win 4117, options [nop,nop,TS val 203808672 ecr 2895815], length 338
	0x0000:  4500 0186 21f4 4000 3c06 d091 602d c7fb  E...!.@.<...`-..
	0x0010:  8ac5 97fe d6bb 1b9e e9f5 250f d48b 8933  ..........%....3
	0x0020:  8018 1015 6f20 0000 0101 080a 0c25 dfa0  ....o........%..
	0x0030:  002c 2fc7 434f 4e4e 4543 5420 6170 692e  .,/.CONNECT.api.
	0x0040:  6661 6365 626f 6f6b 2e63 6f6d 3a34 3433  facebook.com:443
	0x0050:  2048 5454 502f 312e 310d 0a55 7365 722d  .HTTP/1.1..User-
	0x0060:  4167 656e 743a 204d 6f7a 696c 6c61 2f35  Agent:.Mozilla/5
	0x0070:  2e30 2028 6950 686f 6e65 3b20 4350 5520  .0.(iPhone;.CPU.
	0x0080:  6950 686f 6e65 204f 5320 3130 5f33 5f31  iPhone.OS.10_3_1
	0x0090:  206c 696b 6520 4d61 6320 4f53 2058 2920  .like.Mac.OS.X).
	0x00a0:  4170 706c 6557 6562 4b69 742f 3630 332e  AppleWebKit/603.
	0x00b0:  312e 3330 2028 4b48 544d 4c2c 206c 696b  1.30.(KHTML,.lik
	0x00c0:  6520 4765 636b 6f29 204d 6f62 696c 652f  e.Gecko).Mobile/
	0x00d0:  3134 4533 3034 205b 4642 414e 2f46 4249  14E304.[FBAN/FBI
	0x00e0:  4f53 3b46 4241 562f 3932 2e30 2e30 2e34  OS;FBAV/92.0.0.4
	0x00f0:  362e 3730 3b46 4242 562f 3537 3733 3334  6.70;FBBV/577334
	0x0100:  3230 3b46 4244 562f 6950 686f 6e65 372c  20;FBDV/iPhone7,
	0x0110:  323b 4642 4d44 2f69 5068 6f6e 653b 4642  2;FBMD/iPhone;FB
	0x0120:  534e 2f69 4f53 3b46 4253 562f 3130 2e33  SN/iOS;FBSV/10.3
	0x0130:  2e31 3b46 4253 532f 323b 4642 4352 2f52  .1;FBSS/2;FBCR/R
	0x0140:  6f67 6572 733b 4642 4944 2f70 686f 6e65  ogers;FBID/phone
	0x0150:  3b46 424c 432f 656e 5f55 533b 4642 4f50  ;FBLC/en_US;FBOP
	0x0160:  2f35 3b46 4252 562f 305d 0d0a 436f 6e6e  /5;FBRV/0]..Conn
	0x0170:  6563 7469 6f6e 3a20 6b65 6570 2d61 6c69  ection:.keep-ali
	0x0180:  7665 0d0a 0d0a                           ve....
20:25:22.649391 IP 138.197.151.254.7070 > 96.45.199.251.54971: Flags [.], ack 339, win 118, options [nop,nop,TS val 2895817 ecr 203808672], length 0
	0x0000:  4500 0034 a6fa 4000 4006 48dd 8ac5 97fe  E..4..@.@.H.....
	0x0010:  602d c7fb 1b9e d6bb d48b 8933 e9f5 2661  `-.........3..&a
	0x0020:  8010 0076 4b13 0000 0101 080a 002c 2fc9  ...vK........,/.
	0x0030:  0c25 dfa0                                .%..
20:25:22.649885 IP 138.197.151.254.7070 > 96.45.199.251.54971: Flags [P.], seq 1:164, ack 339, win 118, options [nop,nop,TS val 2895817 ecr 203808672], length 163
	0x0000:  4500 00d7 a6fb 4000 4006 4839 8ac5 97fe  E.....@.@.H9....
	0x0010:  602d c7fb 1b9e d6bb d48b 8933 e9f5 2661  `-.........3..&a
	0x0020:  8018 0076 4bb6 0000 0101 080a 002c 2fc9  ...vK........,/.
	0x0030:  0c25 dfa0 4854 5450 2f31 2e31 2034 3030  .%..HTTP/1.1.400
	0x0040:  2042 6164 2052 6571 7565 7374 3a20 6d69  .Bad.Request:.mi
	0x0050:  7373 696e 6720 7265 7175 6972 6564 2048  ssing.required.H
	0x0060:  6f73 7420 6865 6164 6572 0d0a 436f 6e74  ost.header..Cont
	0x0070:  656e 742d 5479 7065 3a20 7465 7874 2f70  ent-Type:.text/p
	0x0080:  6c61 696e 3b20 6368 6172 7365 743d 7574  lain;.charset=ut
	0x0090:  662d 380d 0a43 6f6e 6e65 6374 696f 6e3a  f-8..Connection:
	0x00a0:  2063 6c6f 7365 0d0a 0d0a 3430 3020 4261  .close....400.Ba
	0x00b0:  6420 5265 7175 6573 743a 206d 6973 7369  d.Request:.missi
	0x00c0:  6e67 2072 6571 7569 7265 6420 486f 7374  ng.required.Host
	0x00d0:  2068 6561 6465 72                        .header
20:25:22.650165 IP 138.197.151.254.7070 > 96.45.199.251.54971: Flags [F.], seq 164, ack 339, win 118, options [nop,nop,TS val 2895817 ecr 203808672], length 0
	0x0000:  4500 0034 a6fc 4000 4006 48db 8ac5 97fe  E..4..@.@.H.....
	0x0010:  602d c7fb 1b9e d6bb d48b 89d6 e9f5 2661  `-............&a
	0x0020:  8011 0076 4b13 0000 0101 080a 002c 2fc9  ...vK........,/.
	0x0030:  0c25 dfa0
20:25:22.653414 IP 96.45.199.251.54971 > 138.197.151.254.7070: Flags [.], ack 164, win 4112, options [nop,nop,TS val 203808674 ecr 2895817], length 0
	0x0000:  4500 0034 b3c4 4000 3c06 4013 602d c7fb  E..4..@.<.@.`-..
	0x0010:  8ac5 97fe d6bb 1b9e e9f5 2661 d48b 89d6  ..........&a....
	0x0020:  8010 1010 9ef0 0000 0101 080a 0c25 dfa2  .............%..
	0x0030:  002c 2fc9                                .,/.
20:25:22.653447 IP 96.45.199.251.54971 > 138.197.151.254.7070: Flags [.], ack 165, win 4112, options [nop,nop,TS val 203808674 ecr 2895817], length 0
	0x0000:  4500 0034 b9e4 4000 3c06 39f3 602d c7fb  E..4..@.<.9.`-..
	0x0010:  8ac5 97fe d6bb 1b9e e9f5 2661 d48b 89d7  ..........&a....
	0x0020:  8010 1010 9eef 0000 0101 080a 0c25 dfa2  .............%..
	0x0030:  002c 2fc9                                .,/.
20:25:22.653461 IP 96.45.199.251.54971 > 138.197.151.254.7070: Flags [F.], seq 339, ack 165, win 4113, options [nop,nop,TS val 203808674 ecr 2895817], length 0
	0x0000:  4500 0034 8eb4 4000 3c06 6523 602d c7fb  E..4..@.<.e#`-..
	0x0010:  8ac5 97fe d6bb 1b9e e9f5 2661 d48b 89d7  ..........&a....
	0x0020:  8011 1011 9eed 0000 0101 080a 0c25 dfa2  .............%..
	0x0030:  002c 2fc9                                .,/.
20:25:22.653524 IP 138.197.151.254.7070 > 96.45.199.251.54971: Flags [.], ack 340, win 118, options [nop,nop,TS val 2895818 ecr 203808674], length 0
	0x0000:  4500 0034 739e 4000 4006 7c39 8ac5 97fe  E..4s.@.@.|9....
	0x0010:  602d c7fb 1b9e d6bb d48b 89d7 e9f5 2662  `-............&b
	0x0020:  8010 0076 ae87 0000 0101 080a 002c 2fca  ...v.........,/.
	0x0030:  0c25 dfa2                                .%..

@bradfitz
Copy link
Contributor

@johnmah, thanks. That's unfortunate. I guess this warrants fixing, then.

@bradfitz bradfitz self-assigned this May 16, 2017
@bradfitz bradfitz added NeedsFix The path to resolution is known, but the work has not been done. and removed Thinking WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels May 16, 2017
@gopherbot
Copy link

CL https://golang.org/cl/44004 mentions this issue.

@golang golang locked and limited conversation to collaborators May 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants