Re: svn commit: r1406719 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number include/http_core.h server/core.c server/protocol.c

2012-11-08 Thread André Malo
On Wednesday 07 November 2012 22:16:46 Graham Leggett wrote:
> On 07 Nov 2012, at 10:35 PM, André Malo  wrote:
> >> It feels wrong targeting 0.9 only, would it be possible to do this in a
> >> generic way, say by listing the ones accepted, or by specifying a
> >> minimum?
> >
> > Hmm, what would be the use case? I see it with HTTP/0.9, but I don't see
> > it with >= HTTP/1.0. I'd prefer kind of "duck typing" for those, like
> > asking "is a valid host header present?" - which is already possible.
>
> I've had problems in the past with HTTP/1.0 requests to restful services
> causing keepalive problems with load balancers, and I'd like to be able to
> ban HTTP/1.0 requests, allowing HTTP/1.1 only.

Huh. It sounds more like fixing the loadbalancers would be the way to go. Most 
non-browser clients don't speak HTTP/1.1 anyway.

nd


Re: Rethinking "be liberal in what you accept"

2012-11-08 Thread Stefan Fritsch

On Wed, 7 Nov 2012, Tim Bannister wrote:


On 7 Nov 2012, at 11:26, Stefan Fritsch wrote:

If a method is not registered, bail out early.



Good idea, but it would be nice to be able to use  or  to 
re-allow it.


I intended to add a directive to easily register custom methods (i.e. call 
ap_method_register()). Do you think there is reason to allow arbitrary 
methods, and not just a configured list of allowed ones?



About /: I think they are inherently broken and won't 
add any new functions to them. See e.g.


http://mail-archives.apache.org/mod_mbox/httpd-dev/201010.mbox/%3C201010192146.28217.sf%40sfritsch.de%3E 
http://mail-archives.apache.org/mod_mbox/httpd-dev/201010.mbox/%3C201010221727.45305.sf%40sfritsch.de%3E


if you are interested in the why.


Re: svn commit: r1406719 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number include/http_core.h server/core.c server/protocol.c

2012-11-08 Thread Stefan Fritsch

On Wed, 7 Nov 2012, Tim Bannister wrote:

On 7 Nov 2012, at 18:12, Stefan Fritsch wrote:

On Wed, 7 Nov 2012, Graham Leggett wrote:


New directive HttpProtocol which allows to disable HTTP/0.9 support.


It feels wrong targeting 0.9 only, would it be possible to do this in a generic 
way, say by listing the ones accepted, or by specifying a minimum?


Any suggestions for a syntax? Maybe:

HttpProtocol 1.1# only 1.1
HttpProtocol 1.0-   # 1.0 and above
HttpProtocol 1.0-1.1# 1.0 and 1.1
HttpProtocol -1.0   # 1.0 and below


Does it need its own directive? How about a new environment variable and 
Require:

Require expr %{HTTP_PROTOCOL} -gt 1.1


I realise that won't work as things stand, because -gt only handles integers. 
Maybe another binary operator could allow decimals?

NB. SERVER_PROTOCOL would not be suitable because the initial “HTTP/” makes it 
harder to do math.


I would prefer a dedicated directive: If you use authorization for that, 
you have to take care that it is not overriden by per-directory authz 
directives. Also, while evaluating an ap_expr is faster than e.g. using 
mod_lua, it is still a relatively complex operation. And I expect a lot of 
admins would like to disable 0.9, so having a way that has only minimal 
impact on performance would be better. Finally, I am not 100% sure that 
there are no code paths that cause a HTTP/0.9 error response to be sent 
before the Requrire or  is executed. The dedicated directive is 
certain to catch all uses of HTTP/0.9.

Re: svn commit: r1406719 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number include/http_core.h server/core.c server/protocol.c

2012-11-08 Thread Stefan Fritsch

On Wed, 7 Nov 2012, Graham Leggett wrote:


On 07 Nov 2012, at 8:12 PM, Stefan Fritsch  wrote:


Any suggestions for a syntax? Maybe:

HttpProtocol 1.1# only 1.1
HttpProtocol 1.0-   # 1.0 and above
HttpProtocol 1.0-1.1# 1.0 and 1.1
HttpProtocol -1.0   # 1.0 and below

We could then still add additional flags like +/- strict.


The "-" in front of something means "switch this off" in other 
directives, I suspect it might cause confusion.


I have already used a similar syntax in RequestReadTimeout. But do you 
think it would be clearer to require two numbers:


HttpProtocol 0.0-1.0# 1.0 and below
HttpProtocol 1.0-10.100 # 1.0 and above

The upper limit would be rather arbitrary. Of course, I don't see HTTP/2.x 
arriving any time soon.


Would it make sense to try use globbing (apr_fnmatch)? Perhaps multiple 
options separated by commas, or an ITERATE separated by spaces?


HttpProtocol *  # any version
HttpProtocol 1.1# only 1.1
HttpProtocol 1.*# 1.0 and above
HttpProtocol 1.0,1.1# 1.0 and 1.1
HttpProtocol 0.*,1.0# 1.0 and below


A list of allowed versions would be somewhat more complex to implement, 
because it would require space in core_server_config to store a list (e.g. 
with an apr_array) and we would need to iterate that list when checking 
the request. Only having a minimum and maximum value seems like a less 
over-engineered solution.


Alternatively, if we use a list, then we could limit ourselves to the 
three versions actually in use and not support arbitrary values. Not sure 
if that is a future-proof solution.



RFC2616 defines the version as follows:

  HTTP-Version   = "HTTP" "/" 1*DIGIT "." 1*DIGIT

We could potentially also add a check to make sure that "DIGIT" part is 
checked to be actual digits, and reject it otherwise.


In the received request? Yes, I think I had that one on my list already.


Re: Rethinking "be liberal in what you accept"

2012-11-08 Thread Apache Lounge

What about mod_security, has a lot of similar checks and even more.

-Original Message- 
From: Stefan Fritsch

Sent: Wednesday, November 7, 2012 12:26 Newsgroups: gmane.comp.apache.devel
To: dev@httpd.apache.org
Subject: Rethinking "be liberal in what you accept"

Hi,

considering the current state of web security, the old principle of "be
liberal in what you accept" seems increasingly inadequate for web servers.
It causes lots of issues like response splitting, header injection, cross
site scripting, etc. The book "Tangled Web" by Michal Zalewski is a good
read on this topic, the chapter on HTTP is available for free download at
http://nostarch.com/tangledweb .

Also, nowadays performance bottle necks are usually in other places than
request parsing. A few more cycles spent for additional checks won't make
much difference. Therefore, I think it would make sense to integrate some
sanity checks right into the httpd core. For a start, these would need to
be enabled in the configuration.

Examples for such checks [RFC 2616 sections in brackets]:

Request line:
- Don't interpret all kinds of junk as "HTTP/1.0" (like "HTTP/ab" or
  "FOO") [3.1]
- If a method is not registered, bail out early.
  This would prevent CGIs from answering requests to strange methods like
  "HELO" or "http://foo/bar";. This must be configurable or there must be
  at least a directive to easily register custom methods.  Otherwise, at
  least forbid strange characters in the method. [The method is a token,
  which should not contain control characters and separators; 2.2, 5.1]
- Forbid control characters in URL
- Forbid fragment parts in the URL (i.e. "#..." which should never be sent
  by the browser)
- Forbid special characters in the scheme part of absoluteURL requests,
  e.g. "<>"

Request headers:
- In Host header, only allow reasonable characters, i.e. no control
  characters, no "<>&". Maybe: only allow ascii letters, digits, and
  "-_.:[]"
- Maybe replace the Host header with the request's hostname, if they are
  different. In:
 GET http://foo/ HTTP/1.1
 Host: bar
  The "Host: bar" MUST be ignored by RFC 2616 [5.2]. As many webapps likely
  don't do that, we could replace the Host header to avoid any confusion.
- Don't accept requests with multiple Content-Length headers. [4.2]
- Don't accept control characters in header values (in particular single 
CRs,

  which we don't treat specially, but other proxies may. [4.2]

Response headers:
- Maybe error out if an output header value or name contains CR/LF/NUL (or
  all control characters?) [4.2]
- Check that some headers appear only once, e.g. Content-Length.
- Potentially check in some headers (e.g. Content-Disposition) that 
key=value

  pairs appear only once (this may go too far / or be too expensive).

Other:
- Maybe forbid control characters in username + password (after base64
  decoding)

As a related issue, it should be possible to disable HTTP 0.9.

The dividing line to modules like mod_security should be that we only
check things that are forbidden by some standard and that we only look at
the protocol and not the body.  Also, I would only allow to switch the
checks on and off, no further configurability. And the checks should be
implemented efficiently, i.e. don't parse things several times to do the
checks, normally don't use regexes, etc.

What do you think?

Cheers,
Stefan 



Re: Rethinking "be liberal in what you accept"

2012-11-08 Thread Christian Folini
On Thu, Nov 08, 2012 at 11:47:31AM +0100, Apache Lounge wrote:
> What about mod_security, has a lot of similar checks and even more.

ModSec can perform all these checks via regexes, but it bears a 
certain overhead in performance and administration. The protocol 
checks are part of bigger rulesets and positives will be mixed 
in the logs with other security findings of varying severity.

The standard state of the art ModSec deployment with the official
Core-Ruleset works with a scoring mechanism, that does not block
a request instantly. So depending on the combination of violations
in a request, a bogus request line may pass beneath the threshold
of the Core-Rules.

A simple, single directive to stop any protocol violations once 
and for all is preferable in my eyes.

regs,

Christian Folini

> 
> -Original Message- From: Stefan Fritsch
> Sent: Wednesday, November 7, 2012 12:26 Newsgroups: gmane.comp.apache.devel
> To: dev@httpd.apache.org
> Subject: Rethinking "be liberal in what you accept"
> 
> Hi,
> 
> considering the current state of web security, the old principle of "be
> liberal in what you accept" seems increasingly inadequate for web servers.
> It causes lots of issues like response splitting, header injection, cross
> site scripting, etc. The book "Tangled Web" by Michal Zalewski is a good
> read on this topic, the chapter on HTTP is available for free download at
> http://nostarch.com/tangledweb .
> 
> Also, nowadays performance bottle necks are usually in other places than
> request parsing. A few more cycles spent for additional checks won't make
> much difference. Therefore, I think it would make sense to integrate some
> sanity checks right into the httpd core. For a start, these would need to
> be enabled in the configuration.
> 
> Examples for such checks [RFC 2616 sections in brackets]:
> 
> Request line:
> - Don't interpret all kinds of junk as "HTTP/1.0" (like "HTTP/ab" or
>   "FOO") [3.1]
> - If a method is not registered, bail out early.
>   This would prevent CGIs from answering requests to strange methods like
>   "HELO" or "http://foo/bar";. This must be configurable or there must be
>   at least a directive to easily register custom methods.  Otherwise, at
>   least forbid strange characters in the method. [The method is a token,
>   which should not contain control characters and separators; 2.2, 5.1]
> - Forbid control characters in URL
> - Forbid fragment parts in the URL (i.e. "#..." which should never be sent
>   by the browser)
> - Forbid special characters in the scheme part of absoluteURL requests,
>   e.g. "<>"
> 
> Request headers:
> - In Host header, only allow reasonable characters, i.e. no control
>   characters, no "<>&". Maybe: only allow ascii letters, digits, and
>   "-_.:[]"
> - Maybe replace the Host header with the request's hostname, if they are
>   different. In:
>  GET http://foo/ HTTP/1.1
>  Host: bar
>   The "Host: bar" MUST be ignored by RFC 2616 [5.2]. As many webapps likely
>   don't do that, we could replace the Host header to avoid any confusion.
> - Don't accept requests with multiple Content-Length headers. [4.2]
> - Don't accept control characters in header values (in particular
> single CRs,
>   which we don't treat specially, but other proxies may. [4.2]
> 
> Response headers:
> - Maybe error out if an output header value or name contains CR/LF/NUL (or
>   all control characters?) [4.2]
> - Check that some headers appear only once, e.g. Content-Length.
> - Potentially check in some headers (e.g. Content-Disposition) that
> key=value
>   pairs appear only once (this may go too far / or be too expensive).
> 
> Other:
> - Maybe forbid control characters in username + password (after base64
>   decoding)
> 
> As a related issue, it should be possible to disable HTTP 0.9.
> 
> The dividing line to modules like mod_security should be that we only
> check things that are forbidden by some standard and that we only look at
> the protocol and not the body.  Also, I would only allow to switch the
> checks on and off, no further configurability. And the checks should be
> implemented efficiently, i.e. don't parse things several times to do the
> checks, normally don't use regexes, etc.
> 
> What do you think?
> 
> Cheers,
> Stefan

-- 
Christian Folini - 


Re: Rethinking "be liberal in what you accept"

2012-11-08 Thread Nick Kew
On Thu, 8 Nov 2012 11:18:37 +0100 (CET)
Stefan Fritsch  wrote:

> On Wed, 7 Nov 2012, Tim Bannister wrote:
> 
> > On 7 Nov 2012, at 11:26, Stefan Fritsch wrote:
> >> If a method is not registered, bail out early.
> >
> >
> > Good idea, but it would be nice to be able to use  or  
> > to re-allow it.

I'd disagree with that mechanism.  But as to your point:

> I intended to add a directive to easily register custom methods (i.e. call 
> ap_method_register()). Do you think there is reason to allow arbitrary 
> methods, and not just a configured list of allowed ones?

If methods are to be actively checked, a module needs an API to
register a method with the checker.  A configuration option might
still be required for backends (e.g. proxy or CGI) but would perhaps
be secondary?


-- 
Nick Kew


Re: mpm-itk and upstream Apache, once again

2012-11-08 Thread Steinar H. Gunderson
On Mon, Sep 24, 2012 at 08:44:21AM -0400, Jeff Trawick wrote:
> I went ahead and committed this to trunk as r1389339.  Hopefully this
> completes the ability to enable mpm-itk without patches to httpd core.

I've looked at this now; sorry for the long delay. It would seem it is not
sufficient for removing the patches from server/config.c (which exit if
.htaccess files cannot be opened); or am I misunderstanding something?

/* Steinar */
-- 
Homepage: http://www.sesse.net/


Re: mpm-itk and upstream Apache, once again

2012-11-08 Thread Steinar H. Gunderson
On Thu, Nov 08, 2012 at 08:51:50PM +0100, Steinar H. Gunderson wrote:
> I've looked at this now; sorry for the long delay. It would seem it is not
> sufficient for removing the patches from server/config.c (which exit if
> .htaccess files cannot be opened); or am I misunderstanding something?

Sorry, I'm just confused. Back to coding...

/* Steinar */
-- 
Homepage: http://www.sesse.net/


Re: mpm-itk and upstream Apache, once again

2012-11-08 Thread Steinar H. Gunderson
On Thu, Nov 08, 2012 at 08:53:12PM +0100, Steinar H. Gunderson wrote:
>> I've looked at this now; sorry for the long delay. It would seem it is not
>> sufficient for removing the patches from server/config.c (which exit if
>> .htaccess files cannot be opened); or am I misunderstanding something?
> Sorry, I'm just confused. Back to coding...

Yes, indeed, it works. Out-of-tree-built mpm-itk, yay!

But it's not ideal, since I don't get the status of the ap_pcfg_openfile()
call in the htaccess hook (since it's before that), and I also can't return a
file pointer. The only way I can see to do this in mpm-itk is to run
ap_pcfg_openfile() myself to see if it goes through or not, and then close it
and return DECLINED if the call was successful. That would seem somewhat
inefficient.

/* Steinar */
-- 
Homepage: http://www.sesse.net/


Re: Rethinking "be liberal in what you accept"

2012-11-08 Thread Stefan Fritsch
On Thursday 08 November 2012, Nick Kew wrote:
> > I intended to add a directive to easily register custom methods
> > (i.e. call  ap_method_register()). Do you think there is reason
> > to allow arbitrary methods, and not just a configured list of
> > allowed ones?
> 
> If methods are to be actively checked, a module needs an API to
> register a method with the checker.

ap_method_register() already exists (probably since 2.0). AFAICS, 
modules that handle custom methods internally already have to use it.

> A configuration option might
> still be required for backends (e.g. proxy or CGI) but would
> perhaps be secondary?

Still, that was the case I was talking about. I have checked that CGI 
forwards the method without checking. But i expect that proxy and e.g. 
mod_fcgid do that, too.