I use the wsgi_handler instead of roundup-server. And I can not get
HTTP_X-REQUESTED-WITH set in environment because WSGI server convert all
'-' into '_'. So the X-Requested-With header becomes
HTTP_X_REQUESTED_WITH. But roundup-server use HTTP_X-REQUESTED-WITH key.
I think roundup-server behavior should be normalized with other WSGI
server and not use '-' in HTTP_*.
I could not find in PEP3333 that '-' should be converted into '_' for
HTTP_. There is only a reference to server-defined variables.

https://www.python.org/dev/peps/pep-3333/#environ-variables

I find this in the wsgiref implementation:
https://github.com/python/cpython/blob/master/Lib/wsgiref/simple_server.py#L102..L103
So it seems to confirm that '-' must be replaced by '_' in HTTP_ headers.
I will work on a patch.

Hi Cédric,

In message <1550402207.92.0.371676542223.issue2551023@roundup.psfhosted.org>,
=?utf-8?q?C=C3=A9dric_Krier?= writes:
>I use the wsgi_handler instead of roundup-server. And I can not get
>HTTP_X-REQUESTED-WITH set in environment because WSGI server convert all
>'-' into '_'.

What server are you using? That translation of '-' to _' is not
right/supported according to mod_wsgi or wsgi docs. It also seriously
breaks the compatibility with CGI.

>So the X-Requested-With header becomes HTTP_X_REQUESTED_WITH. But
>roundup-server use HTTP_X-REQUESTED-WITH key.

HTTP_X-REQUESTED-WITH is the correct form as the header is: X-REQUESTED-WITH

Do you see the same mangling with X-Forward-Host or other http
headers?

>I think roundup-server behavior should be normalized with other WSGI
>server and not use '-' in HTTP_*.
>I could not find in PEP3333 that '-' should be converted into '_' for
>HTTP_. There is only a reference to server-defined variables.
>
>https://www.python.org/dev/peps/pep-3333/#environ-variables

Wow that seems wsgi is seriously broken. It is not what is documented.
From mod_wsgi:

  https://modwsgi.readthedocs.io/en/develop/release-notes/version-4.3.0.html

it says in part:

  bugs fixed:

    Under Apache 2.4, when creating the environ dictionary for passing
    into access/authentication/authorisation handlers, the behvaiour of
    Apache 2.4 as it pertained to the WSGI application, whereby it blocked
    the passing of any HTTP headers with a name which did not contain just
    alphanumerics or ‘-‘, was not being mirrored. This created the
    possibility of HTTP header spoofing in certain circumstances. Such
    headers are now being ignored.

and under features:

    In Apache 2.4, any headers with a name which does not include only
    alphanumerics or ‘-‘ are blocked from being passed into a WSGI
    application when the CGI like WSGI environ dictionary is created. This
    is a mechanism to prevent header spoofing when there are multiple
    headers where the only difference is the use of non alphanumerics in a
    specific character position.

    This protection mechanism from Apache 2.4 is now being
    restrospectively applied even when Apache 2.2 is being used and even
    though Apache itself doesn’t do it. This may technically result in
    headers that were previously being passed, no longer being passed. The
    change is also technically against what the HTTP RFC says is allowed
    for HTTP header names, but such blocking would occur in Apache 2.4
    anyway due to changes in Apache. It is also understood that other web
    servers such as nginx also perform the same type of blocking. Reliance
    on HTTP headers which use characters other than alphanumerics and ‘-‘
    is therefore dubious as many servers will now discard them when
    needing to be passed into a system which requires the headers to be
    passed as CGI like variables such as is the case for WSGI.

While it does not say that the - is preserved, not saying it's converted to _ would seem a massive oversight.

Also:

  https://wsgi.readthedocs.io/en/latest/definitions.html

says:

  HTTP_ Variables

    Variables corresponding to the client-supplied HTTP request
    headers (i.e., variables whose names begin with HTTP_). The
    presence or absence of these variables should correspond with the
    presence or absence of the appropriate HTTP header in the request.

which is the same as in your cite of:
   https://www.python.org/dev/peps/pep-3333/#environ-variables

The names for the http request headers are most definitely '-'
separated. In cgi mode, the vars are precisely:

  HTTP_<uppercase version of http header field name>

and that is exactly how the vars will be presented to the application.

Tom any ideas here? You have the most wsgi experience.

Here is a patch that replace the - by _

Hi,

On 2019-02-17 15:50, John Rouillard wrote:
> In message <1550402207.92.0.371676542223.issue2551023@roundup.psfhosted.org>,
> =?utf-8?q?C=C3=A9dric_Krier?= writes:
> >I use the wsgi_handler instead of roundup-server. And I can not get
> >HTTP_X-REQUESTED-WITH set in environment because WSGI server convert all
> >'-' into '_'.
> 
> What server are you using? That translation of '-' to _' is not
> right/supported according to mod_wsgi or wsgi docs. It also seriously
> breaks the compatibility with CGI.

I'm using ngnix which passes with fastcgi to flup WSGIServer which uses
RequestDispatcher.

> >So the X-Requested-With header becomes HTTP_X_REQUESTED_WITH. But
> >roundup-server use HTTP_X-REQUESTED-WITH key.
> 
> HTTP_X-REQUESTED-WITH is the correct form as the header is: X-REQUESTED-WITH
> 
> Do you see the same mangling with X-Forward-Host or other http
> headers?

Yes it is done for all headers with '-'.

> >I think roundup-server behavior should be normalized with other WSGI
> >server and not use '-' in HTTP_*.
> >I could not find in PEP3333 that '-' should be converted into '_' for
> >HTTP_. There is only a reference to server-defined variables.
> >
> >https://www.python.org/dev/peps/pep-3333/#environ-variables
> 
> Wow that seems wsgi is seriously broken. It is not what is documented.
> >From mod_wsgi:
> 
>   https://modwsgi.readthedocs.io/en/develop/release-notes/version-4.3.0.html
> 
> While it does not say that the - is preserved, not saying it's converted to _ would seem a massive oversight.

I have seen similar bug fix for django:
https://www.djangoproject.com/weblog/2015/jan/13/security/
An clearly the issue is that headers can collide when '-' is replaced by
'_'.

> Also:
> 
>   https://wsgi.readthedocs.io/en/latest/definitions.html
> 
> says:
> 
>   HTTP_ Variables
> 
>     Variables corresponding to the client-supplied HTTP request
>     headers (i.e., variables whose names begin with HTTP_). The
>     presence or absence of these variables should correspond with the
>     presence or absence of the appropriate HTTP header in the request.
> 
> which is the same as in your cite of:
>    https://www.python.org/dev/peps/pep-3333/#environ-variables
> 
> The names for the http request headers are most definitely '-'
> separated. In cgi mode, the vars are precisely:
> 
>   HTTP_<uppercase version of http header field name>
> 
> and that is exactly how the vars will be presented to the application.

Here is where nginx make this replacement:

for fastcgi:
https://trac.nginx.org/nginx/browser/nginx/src/http/modules/ngx_http_fastcgi_module.c#L947

for uwsgi:
https://trac.nginx.org/nginx/browser/nginx/src/http/modules/ngx_http_uwsgi_module.c#L940

So we have at least two implementation that does this replacement:
wsgiref and nginx. Even if it is not explicit in the standard, it is how
it is implemented.

* John P. Rouillard <rouilj@cs.umb.edu> [20190217 16:50]:
> In message <1550402207.92.0.371676542223.issue2551023@roundup.psfhosted.org>,
> =?utf-8?q?C=C3=A9dric_Krier?= writes:
> >I use the wsgi_handler instead of roundup-server. And I can not get
> >HTTP_X-REQUESTED-WITH set in environment because WSGI server convert all
> >'-' into '_'.
> 
> What server are you using? That translation of '-' to _' is not
> right/supported according to mod_wsgi or wsgi docs. It also seriously
> breaks the compatibility with CGI.

On the other hand I get variables like HTTP_USER_AGENT or
HTTP_X_FORWARDED_FOR in a non-python CGI environment, too.

From https://www.ietf.org/rfc/rfc3875:
(The Common Gateway Interface (CGI) Version 1.1)

| 4.1.18.  Protocol-Specific Meta-Variables
|
|    Meta-variables with names beginning with "HTTP_" contain values read
|    from the client request header fields, if the protocol used is HTTP.
|    The HTTP header field name is converted to upper case, has all
|    occurrences of "-" replaced with "_" and has "HTTP_" prepended to
|    give the meta-variable name.

> >So the X-Requested-With header becomes HTTP_X_REQUESTED_WITH. But
> >roundup-server use HTTP_X-REQUESTED-WITH key.
> 
> HTTP_X-REQUESTED-WITH is the correct form as the header is: X-REQUESTED-WITH
> headers?

And while not a PEP or RFC, this talk on Euro Python 2010 explicitly
states replacing hyphens with underscores:
https://gustavonarea.net/files/talks/europython2010/wsgi-cheatsheet.pdf

| HTTP_* variables: Those present in the HTTP request, in upper case
| and with hyphens replaced with underscores. For example, User-Agent
| becomes HTTP_USER_AGENT.

> >I think roundup-server behavior should be normalized with other WSGI
> >server and not use '-' in HTTP_*.
> >I could not find in PEP3333 that '-' should be converted into '_' for
> >HTTP_. There is only a reference to server-defined variables.
> >
> >https://www.python.org/dev/peps/pep-3333/#environ-variables
> 
> Wow that seems wsgi is seriously broken. It is not what is documented.
> From mod_wsgi:
> 
>   https://modwsgi.readthedocs.io/en/develop/release-notes/version-4.3.0.html
> 
> it says in part:
> 
>   bugs fixed:
> 
>     Under Apache 2.4, when creating the environ dictionary for passing
>     into access/authentication/authorisation handlers, the behvaiour of
>     Apache 2.4 as it pertained to the WSGI application, whereby it blocked
>     the passing of any HTTP headers with a name which did not contain just
>     alphanumerics or ‘-‘, was not being mirrored. This created the
>     possibility of HTTP header spoofing in certain circumstances. Such
>     headers are now being ignored.
> 
> and under features:
> 
>     In Apache 2.4, any headers with a name which does not include only
>     alphanumerics or ‘-‘ are blocked from being passed into a WSGI
>     application when the CGI like WSGI environ dictionary is created. This
>     is a mechanism to prevent header spoofing when there are multiple
>     headers where the only difference is the use of non alphanumerics in a
>     specific character position.
> 
>     This protection mechanism from Apache 2.4 is now being
>     restrospectively applied even when Apache 2.2 is being used and even
>     though Apache itself doesn’t do it. This may technically result in
>     headers that were previously being passed, no longer being passed. The
>     change is also technically against what the HTTP RFC says is allowed
>     for HTTP header names, but such blocking would occur in Apache 2.4
>     anyway due to changes in Apache. It is also understood that other web
>     servers such as nginx also perform the same type of blocking. Reliance
>     on HTTP headers which use characters other than alphanumerics and ‘-‘
>     is therefore dubious as many servers will now discard them when
>     needing to be passed into a system which requires the headers to be
>     passed as CGI like variables such as is the case for WSGI.
> 
> While it does not say that the - is preserved, not saying it's converted to _ would seem a massive oversight.

It is an oversight, because everyone (including those writing
PEP333/PEP3333) just assumes that it behaves like CGI.

> Also:
> 
>   https://wsgi.readthedocs.io/en/latest/definitions.html
> 
> says:
> 
>   HTTP_ Variables
> 
>     Variables corresponding to the client-supplied HTTP request
>     headers (i.e., variables whose names begin with HTTP_). The
>     presence or absence of these variables should correspond with the
>     presence or absence of the appropriate HTTP header in the request.
> 
> which is the same as in your cite of:
>    https://www.python.org/dev/peps/pep-3333/#environ-variables
> 
> The names for the http request headers are most definitely '-'
> separated. In cgi mode, the vars are precisely:
> 
>   HTTP_<uppercase version of http header field name>
> 
> and that is exactly how the vars will be presented to the application.

Uppercase of "-" is "_", because I have to press the shift key to
get it (on a US keyboard) :-)

And apart from all this, we're still talking about environment
variables here:
http://pubs.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap08.html

| Environment variable names used by the utilities in the Shell and
| Utilities volume of IEEE Std 1003.1-2001 consist solely of uppercase
| letters, digits, and the '_' (underscore) from the characters
| defined in Portable Character Set and do not begin with a digit.
| Other characters may be permitted by an implementation

This is just the context of "utilities in the Shell and Utilities
volume" and depending on the context you can use lowercase letters
or even everything except = and \0, but you can't even set an
environment variable with a "-" in bash without some dirty tricks.

So in short, most people just assume that all "-" are converted to
"_", even those writing the specs/PEPs, because it has always been
this way and it is the only way it consistently works. Now, with the
security updates for web servers in place, there is no longer an
ambiguity: An underscore in the environment variable is always a
dash in the header, because headers with underscores are not
allowed.

Regards,
Thomas

Hi Thomas:

In message <20190218083840.565620063.thomas@intevation.de>,
Thomas Arendsen Hein writes:
>* John P. Rouillard <rouilj@cs.umb.edu> [20190217 16:50]:
>> In message <1550402207.92.0.371676542223.issue2551023@roundup.psfhosted.org>,
>> =?utf-8?q?C=C3=A9dric_Krier?= writes:
>> >I use the wsgi_handler instead of roundup-server. And I can not get
>> >HTTP_X-REQUESTED-WITH set in environment because WSGI server convert all
>> >'-' into '_'.
>> 
>> What server are you using? That translation of '-' to _' is not
>> right/supported according to mod_wsgi or wsgi docs. It also seriously
>> breaks the compatibility with CGI.

Ok, I was talking nonsense.

>On the other hand I get variables like HTTP_USER_AGENT or
>HTTP_X_FORWARDED_FOR in a non-python CGI environment, too.

>I just checked 
>
>From https://www.ietf.org/rfc/rfc3875:
>(The Common Gateway Interface (CGI) Version 1.1)
>
>| 4.1.18.  Protocol-Specific Meta-Variables
>|
>|    Meta-variables with names beginning with "HTTP_" contain values read
>|    from the client request header fields, if the protocol used is HTTP.
>|    The HTTP header field name is converted to upper case, has all
>|    occurrences of "-" replaced with "_" and has "HTTP_" prepended to
>|    give the meta-variable name.
>

Ok, so I totally misremebered this.

>>Cedric said:
>> >I think roundup-server behavior should be normalized with other WSGI
>> >server and not use '-' in HTTP_*.
>> >I could not find in PEP3333 that '-' should be converted into '_' for
>> >HTTP_. There is only a reference to server-defined variables.
>> >
>> >https://www.python.org/dev/peps/pep-3333/#environ-variables
>> 

Apparently I implemented the wrong thing. I must have misconfigued my
test where I ran roundup as a pure cgi script. Worse, I relied on my
memory rather than looking up the cgi spec when I answered this
originally 8-(.

I agree 100% with Cedric's proposal to replace - with _ in roundup
server's mapping of HTTP headers into variables and changing the code
to match the _ form.

Thanks for the correction Thomas.

Hi Cédric:

I applied the patch to my test instance and ran the unit tests
without any failures. I will try testing using straight CGI to
verify that it works there. Also I'll test with my
roundup-server instance.

I assume this has fixed your issue with wsgi and the CSRF code is now
working ok?

Where should this patch be applied? If I understand you correctly
this breaks CSRF behind a proxy (as X-FORWARDED-HOST is not mapped
correctly). It also breaks xmlrpc (but not REST yet) when
the X-REQUESTED-WITH header is required.

It certainly needs to be applied to the tip, but maybe we also need
to create a branch from 1.6.0 and somebody can release a 1.6.1?

So what do you think?

-- rouilj

On 2019-02-19 00:39, John Rouillard wrote:
> I assume this has fixed your issue with wsgi and the CSRF code is now
> working ok?

Yes it did.

> Where should this patch be applied? If I understand you correctly
> this breaks CSRF behind a proxy (as X-FORWARDED-HOST is not mapped
> correctly). It also breaks xmlrpc (but not REST yet) when
> the X-REQUESTED-WITH header is required.

I got issue only with xmlrpc API for now. But no issue with CSRF because
my configuration is 'yes' and not 'required'.

> It certainly needs to be applied to the tip, but maybe we also need
> to create a branch from 1.6.0 and somebody can release a 1.6.1?
> 
> So what do you think?

Yes it will be great to have a bugfix release.
Otherwise as the roundup package manager for Gentoo, I will have to
apply this patch on the 1.6.0 ebuild.

Here is an updated version of the patch because I missed one check using
the same variable for config and env.

But maybe it will be better to also remove the - in the configuration.

Hi Cédric:

In message <1550566540.06.0.890861302485.issue2551023@roundup.psfhosted.org>,
=?utf-8?q?C=C3=A9dric_Krier?= writes:
>Cédric Krier added the comment:
>
>Here is an updated version of the patch because I missed one check using
>the same variable for config and env.

Deployed to my instances.

>But maybe it will be better to also remove the - in the configuration.

Do you mean changing:

  csrf_enforce_header_X-REQUESTED-WITH

to

  csrf_enforce_header_X_REQUESTED_WITH

In this case the header's name in the http protocol is
X-REQUESTED-WITH right? This describes the actual header name one
would look for to find documentation. It does not describe the CGI
variable name mapped to that header. As a result I think it makes more
sense to stay with csrf_enforce_header_X-REQUESTED-WITH.

I'll see if somebody on the dev list is able/interested to do a 1.6.1
release. That should handle your immediate issue for a gentoo release.

I have applied:

 https://issues.roundup-tracker.org/file1703/roundup-dash-underscore.patch

to the default (b3618882f906) and maint-1.6 (8e3df461d316) branches.

Cédric thanks for the fix. Sorry for the trouble.

-- rouilj