You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2023/03/31 06:28:47 UTC

Re: svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c


On 3/31/23 2:11 AM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Fri Mar 31 00:11:02 2023
> New Revision: 1908827
> 
> URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
> Log:
> mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.

Can this really happen? Wouldn't this path be rejected during request line parsing?

Regards

Rüdiger


Re: svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Mar 31, 2023 at 11:23 AM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 3/31/23 10:25 AM, Yann Ylavic wrote:
> > On Fri, Mar 31, 2023 at 10:15 AM Ruediger Pluem <rp...@apache.org> wrote:
> >>
> >> On 3/31/23 9:56 AM, Yann Ylavic wrote:
> >>> On Fri, Mar 31, 2023 at 8:28 AM Ruediger Pluem <rp...@apache.org> wrote:
> >>>>
> >>>> On 3/31/23 2:11 AM, ylavic@apache.org wrote:
> >>>>> Author: ylavic
> >>>>> Date: Fri Mar 31 00:11:02 2023
> >>>>> New Revision: 1908827
> >>>>>
> >>>>> URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
> >>>>> Log:
> >>>>> mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.
> >>>>
> >>>> Can this really happen? Wouldn't this path be rejected during request line parsing?
> >>>
> >>> This is on the suspenders plus belt side (check what we send), but I
> >>> suppose that an unfortunate NE[,P] rule could cause it.
> >>
> >> Don't get me wrong I like better safe than sorry, but can we have nocanon with RewriteRules?
> >
> > NE,P forces nocanon in mod_rewrite for instance.
>
> Good catch. I missed this.

I was tempted to not do it for "noencode" (i.e. mapping=encoded)
because it is the (most-)fast path for proxying, likely w/o
RewriteRules or the users really knowing what they are doing here, but
finally went with common code. I could be convinced though :)

>
> >
> >> And if we think that this is a possibility shouldn't we check
> >>
> >> *ap_scan_vchar_obstext(r->filename)
> >>
> >> just before returning, to be really sure we missed no edge case?
> >
> > We could do that, but the path and search parts of the constructed
> > r->filename seem to be the only ones we haven't "parsed" in
> > canon_handler/ap_proxy_canon_netloc already.
>
> Ok, fair enough. Do you care to propose for backport such that we get it in before the tag?

Done (STATUS + backport PR #354).


Thanks;
Yann.

Re: svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 3/31/23 10:25 AM, Yann Ylavic wrote:
> On Fri, Mar 31, 2023 at 10:15 AM Ruediger Pluem <rp...@apache.org> wrote:
>>
>> On 3/31/23 9:56 AM, Yann Ylavic wrote:
>>> On Fri, Mar 31, 2023 at 8:28 AM Ruediger Pluem <rp...@apache.org> wrote:
>>>>
>>>> On 3/31/23 2:11 AM, ylavic@apache.org wrote:
>>>>> Author: ylavic
>>>>> Date: Fri Mar 31 00:11:02 2023
>>>>> New Revision: 1908827
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
>>>>> Log:
>>>>> mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.
>>>>
>>>> Can this really happen? Wouldn't this path be rejected during request line parsing?
>>>
>>> This is on the suspenders plus belt side (check what we send), but I
>>> suppose that an unfortunate NE[,P] rule could cause it.
>>
>> Don't get me wrong I like better safe than sorry, but can we have nocanon with RewriteRules?
> 
> NE,P forces nocanon in mod_rewrite for instance.

Good catch. I missed this.

> 
>> And if we think that this is a possibility shouldn't we check
>>
>> *ap_scan_vchar_obstext(r->filename)
>>
>> just before returning, to be really sure we missed no edge case?
> 
> We could do that, but the path and search parts of the constructed
> r->filename seem to be the only ones we haven't "parsed" in
> canon_handler/ap_proxy_canon_netloc already.

Ok, fair enough. Do you care to propose for backport such that we get it in before the tag?

Regards

Rüdiger


Re: svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Mar 31, 2023 at 10:15 AM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 3/31/23 9:56 AM, Yann Ylavic wrote:
> > On Fri, Mar 31, 2023 at 8:28 AM Ruediger Pluem <rp...@apache.org> wrote:
> >>
> >> On 3/31/23 2:11 AM, ylavic@apache.org wrote:
> >>> Author: ylavic
> >>> Date: Fri Mar 31 00:11:02 2023
> >>> New Revision: 1908827
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
> >>> Log:
> >>> mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.
> >>
> >> Can this really happen? Wouldn't this path be rejected during request line parsing?
> >
> > This is on the suspenders plus belt side (check what we send), but I
> > suppose that an unfortunate NE[,P] rule could cause it.
>
> Don't get me wrong I like better safe than sorry, but can we have nocanon with RewriteRules?

NE,P forces nocanon in mod_rewrite for instance.

> And if we think that this is a possibility shouldn't we check
>
> *ap_scan_vchar_obstext(r->filename)
>
> just before returning, to be really sure we missed no edge case?

We could do that, but the path and search parts of the constructed
r->filename seem to be the only ones we haven't "parsed" in
canon_handler/ap_proxy_canon_netloc already.


Regards;
Yann.

Re: svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 3/31/23 9:56 AM, Yann Ylavic wrote:
> On Fri, Mar 31, 2023 at 8:28 AM Ruediger Pluem <rp...@apache.org> wrote:
>>
>> On 3/31/23 2:11 AM, ylavic@apache.org wrote:
>>> Author: ylavic
>>> Date: Fri Mar 31 00:11:02 2023
>>> New Revision: 1908827
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
>>> Log:
>>> mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.
>>
>> Can this really happen? Wouldn't this path be rejected during request line parsing?
> 
> This is on the suspenders plus belt side (check what we send), but I
> suppose that an unfortunate NE[,P] rule could cause it.

Don't get me wrong I like better safe than sorry, but can we have nocanon with RewriteRules?
And if we think that this is a possibility shouldn't we check

*ap_scan_vchar_obstext(r->filename)

just before returning, to be really sure we missed no edge case?

Regards

Rüdiger


Re: svn commit: r1908827 - in /httpd/httpd/trunk/modules: http2/mod_proxy_http2.c proxy/mod_proxy_ajp.c proxy/mod_proxy_balancer.c proxy/mod_proxy_fcgi.c proxy/mod_proxy_http.c proxy/mod_proxy_uwsgi.c proxy/mod_proxy_wstunnel.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Mar 31, 2023 at 8:28 AM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 3/31/23 2:11 AM, ylavic@apache.org wrote:
> > Author: ylavic
> > Date: Fri Mar 31 00:11:02 2023
> > New Revision: 1908827
> >
> > URL: http://svn.apache.org/viewvc?rev=1908827&view=rev
> > Log:
> > mod_proxy: Check for space/ctrls in nocanon path/urls before forwarding.
>
> Can this really happen? Wouldn't this path be rejected during request line parsing?

This is on the suspenders plus belt side (check what we send), but I
suppose that an unfortunate NE[,P] rule could cause it.

Regards;
Yann.