Skip to content

Force to send custom headers back to the client and fix POST 500 error #826

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 11 commits into from
Closed

Force to send custom headers back to the client and fix POST 500 error #826

wants to merge 11 commits into from

Conversation

ton31337
Copy link

@ton31337 ton31337 commented Jan 8, 2015

Custom headers are skipped when using nginx as proxy_pass to origin. They are displayed only in audit_log, but the client doesn't see them. For example X-Flash-Message header.

Felipe Zimmerle and others added 11 commits July 30, 2014 14:36
nginx refactoring
93ab6c3 
Refactoring on the nginx module, including:
 - Better handling larger posts;
 - Now using nginx echo module during the regression tests.
 - Better interacting with neginx chain rules
 - Separation of the request handling and content filters.
 - Better handling nginx sessions and resource counts to allow a
   more efficient garbage collector.
 - Handling both http/1.0 and 1.1, including keep-alive.
 - Tests are now capable to test nginx as a proxy or end-server.
 - Tested agains nginx 1.6 and 1.7.
nginx: looking for segfaults on the regression test.
15b5b7a 
If nginx segfaults it will return, warning that the test failed.
nginx: better dealing with chunked request body
82e5684
nginx: copies the req body chain to be processed instead of move
d44158f 
Add a check for the definition MOVE_REQUEST_CHAIN_TO_MODSEC, whenever it is
set the chain will be moved into the brigade. If it was not set the chain
will be only copied. Moving was causing segfaults on the following
regression tests:

 owasp-modsecurity#15 - SecRequestBodyInMemoryLimit
 owasp-modsecurity#16 - SecRequestBodyInMemoryLimit (greater)
 owasp-modsecurity#19 - SecRequestBodyLimitAction ProcessPartial (multipart/greater - chunked)
 (from: regression/config/10-request-directives.t)
nginx: cosmetics: Splits lines longer than 80 characters
28af5c0
nginx: cosmetics: Removes trailing whitespace
764fa06
niginx: cosmetics: Changes CRLF to LF
e3583de
@InfoHunter
Bugfix: add -P option in test script
9c1cb84 
Otherwise nginx's installation directory could not be specified.

Signed-off-by: paulyang <[email protected]>
@defanator
Removed unneeded and invalid initialization.
dbc4021
@defanator
Obtain port from r->connection->local_sockaddr.
33b8760 
This eliminates segfaults caused by unset (NULL) r->port_start
and non-NULL r->port_end. In fact, r->port_start is always NULL,
so it is useless to rely on this pointer.
force to send x-* headers back to the client (browser)
2bb9c00
@jdoss
Copy link

jdoss commented Jan 15, 2015

I was getting 500 errors on POST and this fixed the problem. If there was a +1 button I would push it.

@ton31337
Copy link
Author

Yes this patch solves 500 and x headers issues.
On 15 Jan 2015 08:06, "Joe Doss" [email protected] wrote:

I was getting 500 errors on POST and this fixed the problem. If there was
a +1 button I would push it.

—
Reply to this email directly or view it on GitHub
#826 (comment)
.

@bradcosine
Copy link

I can confirm that this resolved the POST 500 error. Thanks!

@zimmerle zimmerle force-pushed the nginx_refactoring branch from c2e4e03 to 1f1eea3 Compare March 24, 2015 12:45
@zimmerle zimmerle mentioned this pull request Mar 24, 2015
Closed
@zimmerle
Copy link
Contributor

Hi @ton31337 the current version of our nginx_refactoring branch returns all the headers to the users, including those "X-Flash-Message", can you confirm that it is working for you?

@ton31337
Copy link
Author

It misses new_h->hash = 1;
https://github.com/SpiderLabs/ModSecurity/blob/nginx_refactoring/nginx/modsecurity/ngx_http_modsecurity.c#L860

And we are getting 500 POST with SecRequestBodyAccess On. Need the last
commit from this PR:
#826

On Tue, Mar 24, 2015 at 10:39 PM, Felipe Zimmerle [email protected]
wrote:

Hi @ton31337 https://github.com/ton31337 the current version of our
nginx_refactoring branch returns all the headers to the users, including
those "X-Flash-Message", can you confirm that it is working for you?

—
Reply to this email directly or view it on GitHub
#826 (comment)
.

Donatas

@zimmerle
Copy link
Contributor

Not sure if I am looking to the right commit. I am looking to this one:
ton31337@2bb9c00

The content is:
A) Line 1-9: Contain modifications on the file copyright.
B) Line 73: Adds the variable x_headers
C) Line 162-167: Adds the ModSecurityXHeaders command.
D) Line 547-550: returns if trying to set the header Host on the client (similar to what was suggested on #740)
E) Line 819-823: Reorganization of the variables declaration plus the addition of the tmp_header.
F) Line 843-856: Adding the header back to the client.
G) Line 1021 and 1035: Add support to configuration merge and set the default conf->x_headers value.

Is that correct?

If so, the blocks A, B, C, E, F and G will no longer be necessary as we are saving all the headers already. Only left the block "D" (also covered by #740) and the problem that you have mentioned about "new_h->hash = 1;".

Do you mind to share the scenario of the crash, it will be valuable. I can place it under our regression tests.

@uxbod
Copy link

uxbod commented May 9, 2015

Using @ton31337 branch for the XHeaders PR has resolved the 500 errors we were getting. Even a simple wp-login.php POST was the error. Will it be merged soon ?

@ton31337
Copy link
Author

@zimmerle I don't have now any chance to make you crash test, because I don't work for that company they are using ModSecurity, sorry.

@ton31337
Copy link
Author

@zimmerle

  • You can just use simple curl request with POST and test this case. (HTTP 500 issue)
  • Set your service (nginx/apache) with responding with header e.g. X-Rails-Message and see if you receive this header as a client. (Custom headers issue)

@ton31337 ton31337 changed the title force to send x-* headers back to the client (browser) Force to send custom headers back to the client and fix POST 500 error Jun 8, 2015
@zimmerle zimmerle force-pushed the nginx_refactoring branch from 2c95bcd to 8a49998 Compare June 30, 2015 19:05
@ton31337
Copy link
Author

ton31337 commented Aug 5, 2015

@zimmerle should I rebase my branch or can I delete it at all (suppose you don't use it)?

@zimmerle
Copy link
Contributor

zimmerle commented Aug 9, 2015

Hi @ton31337, keep it here. It is used as a reference in too many issues now.

I am working on libmodsecurity -
https://github.com/SpiderLabs/ModSecurity/tree/libmodsecurity/

Expecting to solve all those bugs with this new version of ModSecurity.

@ton31337
Copy link
Author

@zimmerle you should pay more attention on POST 500 error. Take a look at this line: ton31337@2bb9c00#diff-a80339e1bd9e20765a4f9539ae3c09ecR547 (if you debug POST 500 with gdb you will see that sometimes header names come with some trashes - without new line character) and about sending headers from back-end to browser there is missing ton31337@2bb9c00#diff-a80339e1bd9e20765a4f9539ae3c09ecR851.

@zimmerle zimmerle force-pushed the nginx_refactoring branch 2 times, most recently from f4caddc to 169e918 Compare February 2, 2016 15:49
@zimmerle
Copy link
Contributor

zimmerle commented Apr 5, 2016

Hi @ton31337, I would like to thank you again for the patches. I am closing this issue as pieces of your merge request was already merged into the nginx_refactoring branch. Thank you.

@zimmerle zimmerle closed this Apr 5, 2016
@ton31337 ton31337 deleted the nginx_refactoring_ton31337 branch April 5, 2016 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Platform - Nginx
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ton31337 @jdoss @bradcosine @zimmerle @uxbod @InfoHunter @defanator