Add TRACE to Safe Methods
snuggs opened this issue · 7 comments
@tmornini @brandondees @kurtcagle @btakita I see the words...But difficult to put together sentences:
HTTP Spec 9.8 TRACE
https://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.8
9.8 TRACE
The TRACE method is used to invoke a remote, application-layer loop- back of the request message. The final recipient of the request SHOULD reflect the message received back to the client as the entity-body of a 200 (OK) response. The final recipient is either the origin server or the first proxy or gateway to receive a Max-Forwards value of zero (0) in the request (see section 14.31). A TRACE request MUST NOT include an entity.
TRACE allows the client to see what is being received at the other end of the request chain and use that data for testing or diagnostic information. The value of the Via header field (section 14.45) is of particular interest, since it acts as a trace of the request chain. Use of the Max-Forwards header field allows the client to limit the length of the request chain, which is useful for testing a chain of proxies forwarding messages in an infinite loop.
If the request is valid, the response SHOULD contain the entire request message in the entity-body, with a Content-Type of "message/http". Responses to this method MUST NOT be cached.
I will add TRACE to SAFE_METHODS in #164 but will save actual implementation within Resource for a subsequent PR related to this issue.
seems like there are a number of security problems with supporting it, and the universal recommendation is to keep it strictly disabled: https://www.beyondsecurity.com/scan_pentest_network_vulnerabilities_http_trace_method_xss_vulnerability
@brandondees MY MAN 50 GRAND! I'm gonna have to weigh in on this after reading. But knowing it's STRICTLY DISABLED is a "feature" I believe myself. I'm all for MORE EXPLICIT security in my servers. Since the CSP stuff being so important. #160 which gets merged immediately after #164 We can't (should't) just 404 EVERYTHING. That's a lazy practice we've gotten used to for years and years.
How the heck is TRACE still in the HTTP spec @tmornini ? I'd think it would have given great discovery for proxies. @brandondees is there ever ANY good reason for it?
@brandondees so sticking to the following (As defined in MDN) is optimal? I'd really like to support SUBSCRIBE and UNSUBSCRIBE. If anything I think they are underused.
VERY interesting because this means all Javascript servers which use require ('http').METHODS (Which is pretty much every JS server from Express to Koa, react server, anything that uses method library, and quite possibly ruby servers as well via Rack) ALL support TRACE enabled by default. (Whether they are used is another story). For instance with node the canonical HTTP native library (which we use and quite possibly the main culprit of this conversation) has TRACE embedded as default. They must not have gotten your memo @brandondees ;-) /cc @mrbernnz @btakita @tmornini
https://github.com/nodejs/node/blob/master/test/parallel/test-http-methods.js#L38
That would be kinda a big deal to bring awareness to "out of the box". YES?
Well, I hadn't seen it used anywhere before and the type of functionality it's supposed to have seemed like the kind of thing we usually try to prevent servers from doing so I just did a quick search for "http trace security" and found a number of "just disable it" tips from various sources. Seems like it's normally dealt with at the apache / nginx layer rather than on the application side, so it's not surprising it's been left untouched in frameworks that all expect to be behind one of those.
documentation on the exact risks and exact best practices wasn't immediately obvious based on my quick search, so maybe it's not that big of a problem, maybe it's just universally disabled by everybody to avoid thinking about it in more detail, or maybe i just didn't look long enough to find more clarity about what the exact implications and tradeoffs are (what, 20 seconds ain't thorough research?)
I can't think of any obvious use case for this feature in the first place, so I'm assuming we don't really need it. From a security standpoint, I'm a big believer in not shipping attack surface area that isn't well justified.
@brandondees hmmmm.... leaning more towards agreeing with you. Perhaps not a feature but a punt/WIP? Tom and I have been revisiting some parts of the HTTP spec and realizing there is a ton that was just an afterthought that developed into anti-patterns because of monkeys on laddars. (myself included). We have to draw the line between "people shouldn't have to use nginx to develop an application. Even the best devs gradually get to CDN" and YAGNI. Similar to the (long winded) discussion about "HTML partial includes". Tom got the entire thread to slow its roll when he stated "Forcing developers to REQUIRE javascript just to include a partial HTML snippet is just kicking the tech debt can" or something to that flavor.
To be clear the 60,000 ft. view is less about making YET NOTHER web server. (we already use KOA which is a lightweight version of express which itself is a heavyweight wrapper around net/http made by TJ). This is more about how can I be lazy and automate best practices as far as HTTP conventions go. Instead of "rounder wheel" a more Armor All'ed one. But we should stay away from that 5th wheel. Nobody needs that ;-)
/cc @tmornini
