Comments (20)
Ok, i found the method, odd, but github search didn't find it for some reason. https://github.com/xplicit/HyperFastCgi/blob/v0.3_stable/src/Mono.WebServer.HyperFastCgi/Request.cs#L134
Here is an example set of headers from tcpdump that cause it to crash, as you'll notice there are no duplicate keys:
User-Agent: Mozilla/5.0 (iPad; CPU OS 6_1_3 like Mac OS X) AppleWebKit/536.26 (KHTML, like Gecko) Mobile/10B329
Accept-Language: zh-cn
Accept: */*
X-Akamai-CONFIG-LOG-DETAIL: true
TE: chunked;q=1.0
Connection: TE
Accept-Encoding: gzip
Akamai-Origin-Hop: 1
Via: 1.1 akamai.net(ghost) (AkamaiGHost)
X-Forwarded-For: 1.2.3.4
I have tried forcing all those headers/values for some test calls, and it doesn't reproduce the crash. :/
from hyperfastcgi.
My /etc/nginx/fastcgi_params that gets loaded for the host is:
fastcgi_param QUERY_STRING $query_string;
fastcgi_param REQUEST_METHOD $request_method;
fastcgi_param CONTENT_TYPE $content_type;
fastcgi_param CONTENT_LENGTH $content_length;
fastcgi_param SCRIPT_NAME $fastcgi_script_name;
fastcgi_param REQUEST_URI $request_uri;
fastcgi_param DOCUMENT_URI $document_uri;
fastcgi_param DOCUMENT_ROOT $document_root;
fastcgi_param SERVER_PROTOCOL $server_protocol;
fastcgi_param HTTPS $https if_not_empty;
fastcgi_param GATEWAY_INTERFACE CGI/1.1;
fastcgi_param SERVER_SOFTWARE nginx/$nginx_version;
fastcgi_param REMOTE_ADDR $remote_addr;
fastcgi_param REMOTE_PORT $remote_port;
fastcgi_param SERVER_ADDR $server_addr;
fastcgi_param SERVER_PORT $server_port;
fastcgi_param SERVER_NAME $server_name;
# mono
fastcgi_param PATH_INFO "";
fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;
from hyperfastcgi.
I may have misidentified the request that was crashing the service. Now i'm seeing these problematic requests have two X-Forwarded-For headers, which seem to be injected by my haproxy loadbalancer.
Looking at their documentation at https://cbonte.github.io/haproxy-dconv/configuration-1.5.html#4.2-option%20forwardfor It says: Since this
header is always appended at the end of the existing header list, the server
must be configured to always use the last occurrence of this header only. See
the server's manual to find how to enable use of this standard header. Note
that only the last occurrence of the header must be used, since it is really
possible that the client has already brought one.
It looks like this is likely my case, where there is already one supplied, and its adding another instead of appending to a list of values.
Is it really the expected behavior of hyperfastcgi that it should crazy if multiple of the same header are received? This seems like a huge potential for a DoS attack, right?
from hyperfastcgi.
Please check that you don't have duplicate fast_cgi parameters in nginx config. You issue looks like that #18. Duplicate headers should be handled normally.
To localize the parameter, which breaks your request you can wrap method ParseParameters into try/catch and log the variable name
.
If hyperfastcgi can be broken by duplicate headers, it should be easily reproduced by plain HTTP request with two strings of the same header name. If you have such one request, please post it here, because this request should not generate an exception
from hyperfastcgi.
Yeah, I triple checked that i don't have duplicate values in nginx.conf / fastcgi_params, as i came across that ticket first.
I'll recompile a copy of the binary with the try/catch to try to isolate the parameter causing the issue as you suggested. I'll also try breaking it with some calls with duplicate headers, to ensure that's not a point of attack.
from hyperfastcgi.
First thing, the duplicate header test. I just ran this, and on the first request it crashes the fastcgi process instantly. :(
curl -X GET -H "User-Agent: Mozilla/5.0 Gecko/2010 Firefox/5" -H "User-Agent: Mozilla/5.0 Gecko/2010 Firefox/6" "http://192.168.1.2:80/"
I think that header parsing needs to check for existence of a given header in the collection, and overwrite it if it already exists (last one wins) or something. Because this is not good for production use if someone can take down a process this easy. :/
Before I put together a pull request, is that a change you'd be open to?
from hyperfastcgi.
Headers should not be owerwriten if they already exist in the array. According to RFC multiple headers should be concatenated by comma. I supposed that nginx should do that work on the front-end before sending requests to FastCGI backend. Need to check this nginx behaviour before doing fix. Also the fix should work carefully with nginx fast_cgi server variables (so it should concatenate only headers, but not server variables).
http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
Multiple message-header fields with the same field-name MAY be present in a message if and only if the entire field-value for that header field is defined as a comma-separated list [i.e., #(values)]. It MUST be possible to combine the multiple header fields into one "field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field-value to the first, each separated by a comma. The order in which header fields with the same field-name are received is therefore significant to the interpretation of the combined field value, and thus a proxy MUST NOT change the order of these field values when a message is forwarded.
from hyperfastcgi.
My test above was running through nginx and both still made it separately to the fastcgi process. So it doesn't look like nginx is doing anything fancy here (at least by default) to combine those duplicate headers into single header with multiple values, etc.
How I understand this, is if someone did want send multiple header values with the same name,then they are expected to comma separate them but only pass a single header name with the multiple values. If they did that then we don't have this issue. If they however sent a malformed request (as i did), I think all bets are off as it's not supported by the protocol. In this case of the malformed request, I'd personally be OK taking either the first or the last, as long as the request doesn't crash the server, but maybe i'm missing something in protocol spec on how to handle it.
That said, I think this exposes a bigger issue in that there isn't enough isolation in the current request handling to prevent a malformed request from taking down the process, making it very susceptible to DoS attacks if any bug is found in request handling. If there is a malformed request, or something unexpected happens while handling the request, it should only impact that specific request, not the entire process, IMO.
from hyperfastcgi.
RFC says that multiple headers values must be equal to comma-separated values of single header. So the logic in hyperfastcgi (if nginx does not combine multiple headers into single line) should be:
- check the header exists in the dictionary
- if not, add the header name and value
- if exists concatenate new value with the old one.
example.
After parsing
X-Test: value1
X-Test: value2
HyperFastCgi should have an item in headers dictionary with the name X-Test
and value value1, value2
. And yes, it must not become down.
from hyperfastcgi.
It seems to me that concatenating them together (if mulitple values are intended) would be the responsibility of the caller (thing adding the additional value) though, and if they didn't adhere to the RFC in sending the request then that's their problem.
Here's the issue I see with just concatenating everything:
Some things are expected to contain multiple values.
Example:
Accept-Encoding: gzip, deflate
or
Cache-Control: no-cache, max-age=0
Some things are (from what I've seen) assumed to only contain one, examples:
User-Agent
Host
Referer
If someone decided to send multiple values of one of those headers that are more or less assumed to have just one, and multiple get concatenated together, do we end up breaking more things downstream in frameworks/applications that have always assumed a single value?
from hyperfastcgi.
We don't know which headers supports lists and which are not (also note that some headers can be user defined). User can send malformed HTTP request with two headers which do not support lists, or it can send the same request with one header with manually concatenated values. Both will be malformed and detecting and dropping out these malformed request is not the job of fastcgi backend, this should be done by front-end or even by routers/firewalls.
FastCGI just should pass HTTP request from nginx front-end to web application and return the response from web application to nginx front end, without any additional logic and without crashes.
from hyperfastcgi.
Ok, so if it's not the job of fastcgi to scrub these requests, I'm fine with that. But that said, if one malformed reques tis received, and the entire process crashes, that is not good...
So I guess in your description of what fastcgi responsibility is and is not... The headers probably shouldn't be stored in a Dictionary, if the process doesn't want to take responsibility for enforcing uniqueness of the headers. Does the fastcgi even need to have access to these headers at all, or can it just proxy them on through to the application?
from hyperfastcgi.
As I wrote before, the working logic with headers should be changed in HFC. Multiple headers should be concatenated into single one in HyperFastCGI to avoid crashing.
The headers are just collected before sending to web application by creating corresponding ASP.NET HttpRequest. In ASP.NET headers are stored as NameValueCollection, so we can't cardinally change the behaviour of headers storing (for example in NodeJs multiple headers values stored as array). Anyway we must add only one header name to the final NameValueCollection in ASP.NET HttpRequest.
from hyperfastcgi.
Ok, want me to take a swing at it?
from hyperfastcgi.
All fixes will be appreciated
from hyperfastcgi.
Regarding:
Also the fix should work carefully with nginx fast_cgi server variables (so it should concatenate only headers, but not server variables).
To be clear, when you say "server variables" do you mean something matching GetKnownRequestHeaderIndex(header)
?
Or does it mean the stuff pulled by name from the parameters_list
like GetParameter ("SCRIPT_NAME")
? And if so, is there an enum or comprehensive list of those anywhere?
from hyperfastcgi.
All headers are sent by FCGI protocol (except some special one like "Content-Length") start from the prefix "HTTP_". During parsing FCGI request prefix "HTTP_" is removing and headers are adding to the headers dictionary. All other parameters not started with "HTTP_" prefix are server variables. So only headers should be concatenated, but not server variables.
In v0.4 there are functions ParseParameters
and ReformatHttpHeader
which do this job. If ReformatHttpHeader
returns empty string then we deal with server variable, if string is not empty then we've got header. In v0.3 the logic is the same.
from hyperfastcgi.
OK thanks for the clarification. I will do my changes based on v0.3, since it is stable release and I think this is a stability fix.
from hyperfastcgi.
Right now "known headers" get completely overwritten by knownHeaders [idx] = value;
I'm assuming we should modify that to do the add or concatenate logic as well, right?
from hyperfastcgi.
You are right, known headers must be also modified by concatenating new value
from hyperfastcgi.
Related Issues (20)
- Seeing requests with about 46k of request body data fail with 502 under mono-server-hyperfastcgi4 HOT 3
- Will HyperFastCgi support .NET Core (ASP .NET 5)? HOT 1
- hyperfastcgi stop working but not crash HOT 11
- Trying to test NativeTransport and getting DllNotFoundException HOT 12
- compile agains mono 4.8.0 fail HOT 4
- 502 with Nginx HOT 5
- segfault after interrupting hung process with /stoppable=true HOT 2
- Weird Exception while initOnce HOT 1
- Similar issue with #67, 502 after 50k requests HOT 4
- Hyperfastcgi crashing HOT 4
- HyperFastCgi crashes if there is nothing in the webapp folder HOT 1
- configfile difficult to use HOT 1
- fcgi-transport.c:444: parse_params(): Can't find app! HOST='my.host' port=443 path='/index.aspx' HOT 4
- How can I set the umask for the process?
- No package 'mono-2' found, хотя mono установлен HOT 1
- Configuration location for ApplicationPoolRecycling HOT 2
- Error calling 'bufferevent_free' twice in NativeTransport on request end
- Probably race condition? HOT 1
- hyperfastcgi in nginx default server
- Managed Listener leaks sockets
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from hyperfastcgi.