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

Proxy action imrpovements #961

Closed
wants to merge 0 commits into from

Conversation

mariodsantana
Copy link

These two small patches fix issues I'm having when trying to do some tricky proxy actions. For example if I do:

SecDefaultAction "phase:2,log,setvar:SESSION.badness=1,proxy:'http://192.168.190.12%{REQUEST_URI_RAW}"

I've been applying them to new releases without issue for my use case. I posted them a while back as "issues" but they never got answered there? Submitting as a pull request now in case that's the preferred method. The fixes are:

  1. when executing disruptive actions, it's necessary to execute the intercept action as well. I don't think this will break anything, but it certainly fixes this scenario where mod_proxy doesn't expand the REQUEST_URI_RAW macro. Maybe we should only do this for proxy actions?
  2. don't "canonicalize" the proxy URL - if we were creating proxy rules manually for this, we'd probably use the nocanon flag in the proxy rule, because we want to send the URI unchanged to the backend server to whom we are proxying. Maybe this one-line fix can be turned into a user choice with some more work...

@zimmerle
Copy link
Contributor

Hi @mariodsantana,

Thanks for you patch! I will have a look at it.

@zimmerle zimmerle self-assigned this Dec 10, 2015
@zimmerle
Copy link
Contributor

Hi @mariodsantana,

I think the safest thing to do is to have the proxy nocanon as an user option. Maybe we can create another proxy action named: "proxy_nocanon" or "proxy_raw" so that we can treat this specific case, what do you think?

@mariodsantana
Copy link
Author

Hi, @zimmerle, thanks for taking a look! It will be nice to use an
unpatched modsecurity for my use case.

And yes, making the nocanon option be user-selectable seems safest, so
nothing changes for folks already using the proxy action with
canonicalization. I haven't looked at the sources for a while, but would
be happy to whip up a patch if you don't have the bandwidth...

Cheers!
On Dec 10, 2015 10:26 AM, "Felipe Zimmerle" [email protected]
wrote:

Hi @mariodsantana https://github.com/mariodsantana,

I think the safest thing to do is to have the proxy nocanon as an user
option. Maybe we can create another proxy action named: "proxy_nocanon" or
"proxy_raw" so that we can treat this specific case, what do you think?


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

@mariodsantana
Copy link
Author

@zimmerle - Looking into it, it feels like overkill to have a whole new action just to turn on nocanon - what if we could pass proxy flags somehow? One easy way could be to allow the string "[nocanon]" at the beginning of the action's parameter. See attached patch, for example. I can implement either way, just seems like a lot of extra code for a small tweak to an existing function...
optional_nocanon_proxy.patch.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants