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

Add TRACE to Safe Methods #168

Closed
snuggs opened this issue Apr 7, 2018 · 7 comments
Closed

Add TRACE to Safe Methods #168

snuggs opened this issue Apr 7, 2018 · 7 comments

Comments

@snuggs
Copy link
Member

snuggs commented Apr 7, 2018

@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.

@brandondees
Copy link
Collaborator

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

@snuggs
Copy link
Member Author

snuggs commented Apr 7, 2018

@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?

@snuggs
Copy link
Member Author

snuggs commented Apr 7, 2018

@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?

capture d ecran 2018-04-07 a 17 06 25

@brandondees
Copy link
Collaborator

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.

@brandondees
Copy link
Collaborator

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.

@snuggs
Copy link
Member Author

snuggs commented Apr 13, 2018

@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

image

@brandondees
Copy link
Collaborator

brandondees commented Apr 13, 2018 via email

snuggs added a commit that referenced this issue Jun 6, 2018
snuggs added a commit that referenced this issue Jun 11, 2018
@snuggs snuggs closed this as completed Dec 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants