Repository navigation
http: missing host header violation #3094
Description
Per RFC 7230: "A server MUST respond with a 400 (Bad Request) status code to any HTTP/1.1 request message that lacks a Host header field and to any request message that contains more than one Host header field or a Host header field with an invalid field-value."
Node currently ignores this requirement. To test, create a simple server:
http.createServer(function(req,res) {
res.end('ok');
}).listen(8080);First, test the missing Host header
$ telnet localhost 8080
GET / HTTP/1.1
HTTP/1.1 200 OKSecond, test duplicate Host headers:
$ telnet localhost 8080
GET / HTTP/1.1
Host: A
Host: B
HTTP/1.1 200 OKThird, test malformed Host headers:
$ telnet localhost 8080
GET / HTTP/1.1
Host: A, B
HTTP/1.1 200 OK
Activity
bnoordhuis commentedon Sep 28, 2015
See my comment here: enforcing that requirement is moving into policy territory.
jasnell commentedon Sep 28, 2015
Strongly disagree. There are very good reasons for the spec including these kinds of requirements. Namely, a server that does not handle the Host header appropriately can be tricked into disclosing sensitive information about the server itself (https://support.microsoft.com/en-us/kb/967342). While Node itself is not giving up any information currently, an application building on top of Node may not be prepared to deal appropriately with the missing header. In fact, it's reasonable for the application to expect Node to be handling this kind of thing for them. It's a simple check.
rvagg commentedon Sep 29, 2015
I'm torn on this one, I see the argument for adding such a check, however
Hostis only meaningful in Node applications that actually read the host header, I suspect not a whole lot do because they are typically single-purposed and not akin to nginx, Apache or IIS (all of which do400on missing [and malformed?]Host). If an application does checkHostthen it'd be doing whatever checks make sense to it and if you're implementing something that's doing virtual-host style routing then you'd be implementing your own logic for that routing anyway which is where any such checks should be done.So, I could probably be +0 for adding a check in core for the sake of correctness but that'd be all it's about, matching the header to a correct value has to be an application-level thing by definition. Core has no ability to decide what's correct and what's not except for malformed, missing or duplicate but none of those go to the security argument mentioned above.
jasnell commentedon Sep 29, 2015
At most I'd suggest returning a 400 if the header is missing. If it's duplicated, I'd pick the first. Then, as you suggest, leave any further validation of the content to the application
rvagg commentedon Sep 30, 2015
There's a philosophical discussion here that at least TSC members should tune in to, see also #3096. I suspect we're going to have to either come up with a clearer definition of what we're providing to users with the
httpmodule or make a compromise of some kind to resolve disagreements around this functionality. There's also a bunch of related issues for HTTP that both @jasnell and @ChALkeR have been cataloging that go to similar questions about what guarantees thehttpmodule should and should not be providing.ChALkeR commentedon Sep 30, 2015
@rvagg There is no reason to force everyone who is using
httpto re-implement the same basic validation logic. Imo, restrictions imposed by corresponding RFCs should be enforced (especially in the cases where omitting them could lead to security problems in applications). As it looks to me, most of the current modules do not expect invalid values coming from thehttpmodule (meaning that they do not check for it and everything is in the hands of the user).gtramontina commentedon Oct 6, 2015
I have to agree with @ChALkeR on this one. These restrictions can be read as "expected behavior" of any
httpserver implementation.bnoordhuis commentedon Oct 6, 2015
It's not that clear cut. There are plenty instances where the spec is at odds with the real world.
jasnell commentedon Oct 6, 2015
While historically true, the updated RFC7230 has been updated to be a much closer match to real world implementation and needs.
LewisJEllis commentedon Mar 30, 2016
While I very much wish node were fully http compliant, that's somewhat idealist and few servers really are to begin with. Much of the non-compliant behavior in the wild, by node and many others, is just a matter of "when it was implemented, that behavior wasn't well-specified by the standard." See RFC7230 Appendix A.2 which summarizes many such gaps in the old RFC2616 that have been filled. For this reason, I can't take too much of an issue with the "discard duplicates" behavior in @jasnell's second test, for which the correct behavior was only specified in RFC7230 5.4 quoted above. (But I would prefer to return a 400.)
@jasnell's first test, on the other hand, isn't just a matter of previously unspecified behavior, and I don't think it's some hairy matter of spec vs real world either. An HTTP/1.1 request without a host header is not an HTTP/1.1 request. Requiring a host header so as to enable multitenancy was one of the primary points of HTTP/1.1, and the following bit from RFC7230 Appendix A.1.1 (an only slightly modified carryover from RFC2616 19.6.1.1) makes that very clear:
Apparently node is missing one of "the most important changes defined by HTTP/1.1"!
If I'm implementing a web application framework and I see that
message.httpVersion === '1.1', I might very well neglect to check formessage.headers['host'] == null. If I want to be HTTP compliant, I instead need to figure out exactly which half of the spec I can rely on thehttpmodule to enforce for me, and then go enforce the other half myself.I don't know why I might not want to be HTTP compliant, but if I did, surely I'd want total freedom in it instead of being surrounded by an incomplete fence that gets in my way from time to time by following that pesky "protocol."
It seems that, whether one wants to be HTTP compliant or not, the current state of the
httpmodule isn't ideal. I very much like @bnoordhuis's idea on #3096 as an answer for that. A high-level module, thoroughly enforcing standards for users not wishing to do so themselves, built on top of a low-level "building blocks" module, for people who know what they're doing, would better serve both uses.If we can't get that far, can we at least give a 400 to any HTTP/1.1 request which omits the host header? I just can't see any reason to keep the current behavior in that case.
ChALkeR commentedon Apr 4, 2016
Labeling as security-related. This is not a vulnerability in Node.js itself of course, but this is one of those things that could cause security issues in real-world apps.