Roundup Tracker - Issues

Issue 2550837

classification
New option for web auth (also http header passing)
Type: rfe Severity: major
Components: Web interface Versions: 1.5
process
Status: fixed fixed
:
: rouilj : antmail, ber, jerrykan, rouilj, techtonik
Priority: normal : patch

Created on 2014-04-01 17:55 by antmail, last changed 2021-06-14 23:34 by rouilj.

Files
File name Uploaded Description Edit Remove
spec_auth_var.diff antmail, 2014-04-01 17:55 Patch to add uid_variable option
Customize_auth_headers.patch rouilj, 2021-05-27 05:00 Patch to allow admin to set header with username of user. Allows replacing REMOTE_USER
Messages
msg5055 Author: [hidden] (antmail) Date: 2014-04-01 17:55
I had some issues in geterogenius environment when reverse proxing to
roundup-tracker while authentication is external.

To solve this i propose a little patch (included).

This patch do the next things:

1. From user point of view:
In config.ini new option "uid_variable" is appear in section [web] with
following description:
"
In case of http_auth option is set to 'yes' this option
controls which envirionment variable is used to determine
user login name. In most cases your web server supply
the REMOTE_USER variable, which is default value.
So, change this option only in special cases. 
Note: HTTP-request options is available as corresponding
'HTTP_' prefixed variable.
Default: REMOTE_USER
"

This option is compatible with any previous configuration and doesn't
make any change in roundup-tracker behavior.

2. To internals of roundup-tracker:
a) The option is added in configuration.py
b) Change behaviour of determining user during request. Now it will
search "uid_variable" variable instead of hardcoded REMOTE_USER.
c) Make all HTTP-request options available in internal environment as
"HTTP_" prefixed variable, not just few hardcoded.
This made with 5 lines stolled from WebWare
http://www.webwareforpython.org/. 1 line comment in roundup-code note
this regrettable fact.

Patch included.
msg5077 Author: [hidden] (ber) Date: 2014-04-11 08:41
Hi Anthony,
thanks for the proposed patch!

There a number of people that are able to test or commit
patches to roundup. Best is to get somebody else to test it and report here.
Ask on the development list (one way to get more attention is to test other 
people's patches, so they may be willing to test yours).

Best,
Bernhard
msg5078 Author: [hidden] (jerrykan) Date: 2014-04-14 13:36
Hello Anthony,

At a quick glance it looks like this should work, but I have a question
about the headersToEnviron() function.

I am wondering if there are an security implications as a result of
converting all headers to environment variables? Would it be possible
for a client to craft a request with a custom header that could then
potentially lead to a particular environment variable being set that
could have unintended consequences?

If the headersToEnviron() function is only being used to ensure that the
one header specified by the WEB_UID_VARIABLE is set as an environment
variable, might it be better to convert only that one header to an
environment variable instead of all of them? or am I missing something
else in the use of the headersToEnviron() function?
msg5081 Author: [hidden] (antmail) Date: 2014-04-15 08:29
Hello John.

> I am wondering if there are an security implications as a result of
> converting all headers to environment variables?

1. Environment variable prefixed by HTTP_ always treated as externally
defined variable. So it is assumed that they are on the evil side.

2.  Roundup-tracker  (RT) manually set they own "env" dictionary after
headersToEnviron()   fill  this  dictionary.  If  some  variable  name
collision  occur  RT win because of it last word.  As i see.

> If the headersToEnviron() function is only being used to ensure that the
> one header specified by the WEB_UID_VARIABLE is set as an environment
> variable, might it be better to convert only that one header to an
> environment variable instead of all of them?

3. RT manually hardcode only 5 variable (see code). As i remember there
are more  than  5  in  HTTP standart.  May be, the better way is to just
remove this code and rely on  headersToEnviron() function only.


Code (roundup-server.py):
        if co:
            env['HTTP_COOKIE'] = ', '.join(co)
        env['HTTP_AUTHORIZATION'] = self.headers.getheader('authorization')
        env['SCRIPT_NAME'] = ''
        env['SERVER_NAME'] = self.server.server_name
        env['SERVER_PORT'] = str(self.server.server_port)
        try:
            env['HTTP_HOST'] = self.headers ['host']
        except KeyError:
            env['HTTP_HOST'] = ''
        if os.environ.has_key('CGI_SHOW_TIMING'):
            env['CGI_SHOW_TIMING'] = os.environ['CGI_SHOW_TIMING']
        env['HTTP_ACCEPT_LANGUAGE'] = self.headers.get('accept-language')
        range = self.headers.getheader('range')
        if range:
            env['HTTP_RANGE'] = range
msg5086 Author: [hidden] (ber) Date: 2014-04-23 07:36
From the sidelines:
In my understanding, programming for safety usually means to only
let variables (or values if this applies) pass through that are on a whitelist.
msg5087 Author: [hidden] (ber) Date: 2014-04-23 10:10
To unify the communication, here are two emails from roundup-devel:


----------Original Message----------
From: anatoly techtonik <techtonik@gmail.com>
Sent: Monday 14 April 2014, 17:02:08
To: Anthony Pankov <ant_mail@inbox.ru>
Subject: Re: [Roundup-devel] patch for more flexible web auth

Hi Anthony,

On Mon, Apr 14, 2014 at 10:37 AM, Anthony Pankov <ant_mail@inbox.ru> 
wrote:
>
> Some     days     ago    i    created    roundup-tracker    issue    (
> http://issues.roundup-tracker.org/issue2550837 ) but have no reply.
>
> Can somebody   audit  and  apply the  patch  to
> roundup-tracker source tree?

REMOTE_USER is not default standard of authentication in web
applications. For example, Django doesn't use it by default and
in Django backend for it I don't see a way to customize the name
of variable used to fetch it:
https://docs.djangoproject.com/en/dev/howto/auth-remote-user/

You have a very specific use case that is needed to less that 1%
of Roundup installations, and I don't like the idea or using another
global configuration option, because all other 99% will have to read
through it and waste time on guessing what does this option mean.


I'd propose instead to add chapter named "Authentication"
to admin guide, probably renaming this one:
http://roundup.sourceforge.net/docs/admin_guide.html#users-and-security
and document the current way of writing authentication extensions.

Your scenario is a good candidate to be included as a basic
example. An alternative would be to review the current scheme
and submit a patch that makes it improved.
-------------------------------------------------------

----------Original Message----------
From: Anthony Pankov <ant_mail@inbox.ru>
Sent: Wednesday 16 April 2014, 09:55:02
Subject: Re: [Roundup-devel] patch for more flexible web auth


> REMOTE_USER is not default standard of authentication in web
> applications.

I suspected this. So i suppose the way to control  it somehow.

> You have a very specific use case that is needed to less that 1%
> of Roundup installations, and I don't like the idea or using another
> global configuration option, because all other 99% will have to read
> through it and waste time on guessing what does this option mean.

There  is  issue  with  apache   that it  silently suppress REMOTE_USER
variable  which  come not from it auth modules.  I don't find any way
to  pervert  apache configuration to let roundup-tracker do the job in
it current state.



> I'd propose instead to add chapter named "Authentication"
> to admin guide, probably renaming this one:
> http://roundup.sourceforge.net/docs/admin_guide.html#users-and-security
> and document the current way of writing authentication extensions.

I   don't   see  something  like  authentication  driver  in  internal
roundup-tracker     code.     There    is    a    direct    call    to
determine_user() in client.py which is defined as:

        # first up, try http authorization if enabled
        if self.instance.config['WEB_HTTP_AUTH']:
            if 'REMOTE_USER' in self.env:
                # we have external auth (e.g. by Apache)
                user = self.env['REMOTE_USER']
            elif self.env.get('HTTP_AUTHORIZATION', ''):
                # try handling Basic Auth ourselves
                auth = self.env['HTTP_AUTHORIZATION']
                scheme, challenge = auth.split(' ', 1)
                if scheme.lower() == 'basic':

So,  any extension will go after this code has executed which mean low
effectiveness. May be i've missed something.
msg5088 Author: [hidden] (antmail) Date: 2014-04-25 08:09
Hello, Bernhard.

HTTP RFC define HTTP header field (HTTP options) semantics and meaning
of standart options.

So, according to RFC, there is a HTTP options and some specific subset
of options that have a predefined name and meaning. 

Therefore having all HTTP options in roundup-tracker (RT) internals
seems to be more standart complying than drop it all at begining of
request processing.

For example. If your Web sever reverse proxying to RT you may add to
HTTP options something like "X-Free-Registration: yes" according to
external condition. In current state you will never get this value
inside RT. I offer to have possibility to use it when needed.
msg5119 Author: [hidden] (rouilj) Date: 2014-07-20 03:31
Looks like  my email update on 1 april confused the gateway due to 
[proposed patch] on the subject line being interpreted as trying to
set some properties. We probably need to fix that somehow.

Anyhow here is thought.

Date: Tue, 01 Apr 2014 15:19:06 -0400

In message
<1396374939.41.0.103081305534.issue2550837@psf.upfronthosting.co.za>
 <1396374939.41.0.103081305534.issue2550837@psf.upfronthosting.co.za>,
Anthony writes:
>2. To internals of roundup-tracker:
>a) The option is added in configuration.py
>b) Change behaviour of determining user during request. Now it will
>   search "uid_variable" variable instead of hardcoded REMOTE_USER.
>c) Make all HTTP-request options available in internal environment as
>   "HTTP_" prefixed variable, not just few hardcoded.

I like the idea of allowing a different username variable, however I
personally consider anything that doesn't set the standard
REMOTE_USER broken.

I am not sure why all of the HTTP variables would need to be copied
though. Do you have a particular use case for these variables?

>This made with 5 lines stolled from WebWare
>http://www.webwareforpython.org/. 1 line comment in roundup-code note
>this regrettable fact.

The license for webwareforpython is at:

  http://www.webwareforpython.org/Docs/Copyright.html

and reads in part:

  Webware for Python is open source, but there is no requirement that
  products developed with or derivative to Webware become open source.

  Webware for Python is copyrighted, but you can freely use and copy it
  as long as you don't change or remove this copyright notice. The
  license is a clone of the original OSI-approved Python license.


So I think we could take this patch by including

  Copyright © 1999-2010 by Chuck Esterbrook.
  All Rights Reserved

along side the zope license as it doesn't seem to be more restrictive
than the current licensing. But IANAL.
msg5227 Author: [hidden] (jerrykan) Date: 2015-02-26 12:28
Hello Anthony,

I tend to agree with Anatoly in that this does seem to be a pretty niche
requirement and adding another configuration option may not be desirable.

Might it be possible to implement this functionality using a custom
login action? There is some information about creating new actions at:

 
http://roundup.sourceforge.net/docs/customizing.html#defining-new-web-actions

and more specifically some login action examples listed under 'security' at:

  http://www.roundup-tracker.org/cgi-bin/moin.cgi/CustomisationExamples


You also mention you are hitting this because the REMOTE_USER variable
is not being passed through the reverse proxy. Might is be possible to
do something like what is mentioned in the following serverfault answer
to work around the issue without needing to make any changes to roundup?

  http://serverfault.com/a/231577
msg5229 Author: [hidden] (antmail) Date: 2015-02-27 08:40
Greetings.

Because  of  facing  the problem with "REMOTE_USER" i can't appreciate
evenly how many users face the same.

Also i want you to pay attention to another part of patch:

2.c " Make all HTTP-request options available in internal environment as
"HTTP_"  prefixed variable, not just few hardcoded" which make benefit
of passing options from reverse proxy (may be not in the same system) 
to roundup internals.

P.S. May be somebody do a review of another patch
http://issues.roundup-tracker.org/issue2550871

P.P.S. The text above sended via email has returned with folowing error:

There were problems handling your subject line argument list:
- not of form [arg=value,value,...;arg=value,value,...]
msg5252 Author: [hidden] (jerrykan) Date: 2015-03-03 13:52
On 27/02/15 19:40, Anthony wrote:
> Greetings.
>
> Because  of  facing  the problem with "REMOTE_USER" i can't appreciate
> evenly how many users face the same.
>
> Also i want you to pay attention to another part of patch:
>
> 2.c " Make all HTTP-request options available in internal environment as
> "HTTP_"  prefixed variable, not just few hardcoded" which make benefit
> of passing options from reverse proxy (may be not in the same system)
> to roundup internals.

If it is possible to write a Login action that makes use of HTTP 
environment variables, then I can see some value in passing these 
through without being filtered. But are there any other use-cases where 
the environment variables would actually be used? Would these use-cases 
be any different from the existing "is a username set? if so, log them in".

I'm not very familiar with this section of code though, so maybe someone 
else can provide some comment on this.

Did you have a look at the serverfault link I mentioned in my last 
response? Was it a workable solution? If it is then I think it would be 
a preferable solution to fix this sort of issue outside of round (and 
document these sort of solutions) than attempting to solve them within 
roundup.

>
> P.S. May be somebody do a review of another patch
> http://issues.roundup-tracker.org/issue2550871
>
> P.P.S. The text above sended via email has returned with folowing error:
>
> There were problems handling your subject line argument list:
> - not of form [arg=value,value,...;arg=value,value,...]
>

My guess is because of the "[proposed patch]" in the subject (which may 
now be removed because I have snipped it from my response... wei'll see.
msg5253 Author: [hidden] (antmail) Date: 2015-03-04 11:59
> John Kristensen added the comment:

> On 27/02/15 19:40, Anthony wrote:
>> Greetings.
>>
>> Because  of  facing  the problem with "REMOTE_USER" i can't appreciate
>> evenly how many users face the same.
>>
>> Also i want you to pay attention to another part of patch:
>>
>> 2.c " Make all HTTP-request options available in internal environment as
>> "HTTP_"  prefixed variable, not just few hardcoded" which make benefit
>> of passing options from reverse proxy (may be not in the same system)
>> to roundup internals.

> If it is possible to write a Login action that makes use of HTTP 
> environment variables, then I can see some value in passing these 
> through without being filtered. But are there any other use-cases where
> the environment variables would actually be used? Would these use-cases
> be any different from the existing "is a username set? if so, log them in".

First  of  all,  i explored this few month ago so i can forget
somethings about the issue.

It  is  possible to write a Login action that makes use of HTTP
variables. Unfortunately, there is  a lack of HTTP environment
variables. There are few hardcoded.

> I'm not very familiar with this section of code though, so maybe someone
> else can provide some comment on this.

> Did you have a look at the serverfault link I mentioned in my last 
> response? Was it a workable solution? If it is then I think it would be
> a preferable solution to fix this sort of issue outside of round (and 
> document these sort of solutions) than attempting to solve them within
> roundup.

They  suggest to change REMOTE_USER to other name (proxy-user) before proxying and
on the backend rename it back REMOTE_USER . The problem is that 'backend' is roundup.

As  i  remember  there are a some more problem passing variables. They
prefixed with REDIRECT_.

>>
>> P.S. May be somebody do a review of another patch
>> http://issues.roundup-tracker.org/issue2550871
>>
>> P.P.S. The text above sended via email has returned with folowing error:
>>
>> There were problems handling your subject line argument list:
>> - not of form [arg=value,value,...;arg=value,value,...]
>>

> My guess is because of the "[proposed patch]" in the subject (which may
> now be removed because I have snipped it from my response... wei'll see.

> ----------
> title: New option for web auth [proposed patch] -> New option for web auth

> ________________________________________________
> Roundup tracker <issues@roundup-tracker.org>
> <http://issues.roundup-tracker.org/issue2550837>
> ________________________________________________
msg5291 Author: [hidden] (antmail) Date: 2015-03-18 15:21
Just note about using this patch.

In my installation static content (images, etc.) for roundup-tracker
templates delivered via Apache via:

In httpd.conf:

<VirtualHost ...>
Define CSURL /static
...
 <Location /tracker/>
    ProxyPass ...

    #Pass variable
    RequestHeader set CSURL ${CSURL}
 </Location>
</VirtualHost>

In jinja2 template:

{# static content url#}
{%- set csurl = request.env['HTTP_CSURL']  -%}
...
 <link rel="stylesheet" type="text/css" href="{{csurl}/bootstrap.min.css">
...
msg5956 Author: [hidden] (rouilj) Date: 2017-04-21 02:17
Hi all:

Does this sound like a reasonable idea?

Modify the http_auth variable to support:

  yes - use REMOTE_USER, HTTP_AUTHORIZATION (as current)
  no - disable (as current)
  HTTP_*  - look for the HTTP_* variable in the env array
            and use the value of that variable like REMOTE_USER.
	    E.G. the value HTTP_PROXY-USER or http_proxy-USER
	    looks for env['HTTP_PROXY-USER']
  
We pair that with a new config variable

  pass_headers = proxy-user, X-Free-Registration

which generates entries:

   env[HTTP_PROXY_USER], env[HTTP_X_FREE_REGISTRATION]

if the corresponding headers exist.

So for Anthony to get what he wanted (assuming the header uid_variable
contains the username) the config:

  http_auth = HTTP_UID_VARIABLE
  pass_headers = uid-variable

Would your use case be handled by this proposal Anthony?

Thoughts?
msg5957 Author: [hidden] (antmail) Date: 2017-04-21 07:41
> Would your use case be handled by this proposal Anthony?

It seems to yes.

I  advocate this options again and bring back that Apache have a special
care   of  REMOTE_USER  variable  and  there  is  no power to leave it
untouched  in a proxy chain.

> John Rouillard added the comment:

> Hi all:

> Does this sound like a reasonable idea?

> Modify the http_auth variable to support:

>   yes - use REMOTE_USER, HTTP_AUTHORIZATION (as current)
>   no - disable (as current)
>   HTTP_*  - look for the HTTP_* variable in the env array
>             and use the value of that variable like REMOTE_USER.
>             E.G. the value HTTP_PROXY-USER or http_proxy-USER
>             looks for env['HTTP_PROXY-USER']
>   
> We pair that with a new config variable

>   pass_headers = proxy-user, X-Free-Registration

> which generates entries:

>    env[HTTP_PROXY_USER], env[HTTP_X_FREE_REGISTRATION]

> if the corresponding headers exist.

> So for Anthony to get what he wanted (assuming the header uid_variable
> contains the username) the config:

>   http_auth = HTTP_UID_VARIABLE
>   pass_headers = uid-variable

> Would your use case be handled by this proposal Anthony?

> Thoughts?

> ----------
> title: New option for web auth -> New option for web auth (also http header passing)

> ________________________________________________
> Roundup tracker <issues@roundup-tracker.org>
> <http://issues.roundup-tracker.org/issue2550837>
> ________________________________________________
msg5958 Author: [hidden] (antmail) Date: 2017-04-21 14:57
Nevertheless,  John, are you sure that adding pass_headers option is a
really good idea?

Do   the   passing   all http  headers  like  others programs do and
which is conformed to http/cgi standard  is a bad thing?

In this topic I see only one cons: "programming for safety usually means to only
let  variables  (or values if this applies) pass through that are on a
whitelist".
msg5959 Author: [hidden] (rouilj) Date: 2017-04-22 01:34
Hi Anthony:

In message <405313549.20170421175726@inbox.ru>, Anthony writes:
>Nevertheless,  John, are you sure that adding pass_headers option is a
>really good idea?
>
>Do the passing all http headers like others programs do and
>which is conformed to http/cgi standard is a bad thing?

Well we had https://httpoxy.org/ and there is an open ticket for this:

    http://issues.roundup-tracker.org/issue2550925

so I would say yes there can be exploits. I also hope that no program
in the future will use HTTP_* as an environment variable but....

Also didn't the shellshock bash bug depend on creating a specially
crafted environment variable?

>In this topic I see only one cons: "programming for safety
>usually means to only let variables (or values if this applies)
>pass through that are on a whitelist".

Yup, that's where pass_headers came from. It's the whitelist.

Have a great weekend.
--
				-- rouilj
John Rouillard
===========================================================================
My employers don't acknowledge my existence much less my opinions.
msg5960 Author: [hidden] (antmail) Date: 2017-04-22 10:00
Ok, I'm convinced by your explanation.

> In message <405313549.20170421175726@inbox.ru>, Anthony writes:
>>Nevertheless,  John, are you sure that adding pass_headers option is a
>>really good idea?
>>
>>Do the passing all http headers like others programs do and
>>which is conformed to http/cgi standard is a bad thing?

> Well we had https://httpoxy.org/ and there is an open ticket for this:

>     http://issues.roundup-tracker.org/issue2550925

> so I would say yes there can be exploits. I also hope that no program
> in the future will use HTTP_* as an environment variable but....

> Also didn't the shellshock bash bug depend on creating a specially
> crafted environment variable?

>>In this topic I see only one cons: "programming for safety
>>usually means to only let variables (or values if this applies)
>>pass through that are on a whitelist".

> Yup, that's where pass_headers came from. It's the whitelist.

> Have a great weekend.
> --
>                                 -- rouilj
> John Rouillard
> ===========================================================================
> My employers don't acknowledge my existence much less my opinions.

> ________________________________________________
> Roundup tracker <issues@roundup-tracker.org>
> <http://issues.roundup-tracker.org/issue2550837>
> ________________________________________________
msg7251 Author: [hidden] (rouilj) Date: 2021-05-27 05:00
Hi Anthony:

I have attached patches to implement the passing of variables.

You can change the REMOTE_USER variable by setting the

   http_auth_header

in the tracker's config.ini.

If you are using roundup-server, there is a new command line option: -I.
If you use a config file for the server the option is include_headers.

This is a comma separated list of headers. They find a matching
header (case insensitive) and create a new entry (with the case
specified in -I) in the env array preserving the case of the header.

So to implement your use case:

  roundup-server -I proxy-user

in tracker config.ini:

  http_auth_header = proxy-user

If you are not using roundup-server (e.g using gnuicorn or uwsgi)
I think all you need is the config.ini change.

I am trying to get this into the next release and plan on releasing
a beta on June 10 so we have a month with the 2.1.0 release on
July 13.

Do you think you would be able to test these patches before then?
I won't include them if it doesn't solve your use case.

-- rouilj
msg7260 Author: [hidden] (rouilj) Date: 2021-06-06 04:09
Anthony are you still around? Think there is any chance for you to test 
this and see if it handles your use case before release?

Thanks.

-- rouilj
msg7286 Author: [hidden] (rouilj) Date: 2021-06-14 23:34
Committed the patch as experimental in:

changeset:   6436:1f2f7c0b8968
History
Date User Action Args
2021-06-14 23:34:56rouiljsetkeywords: - Blocker
status: open -> fixed
resolution: fixed
messages: + msg7286
2021-06-06 04:09:18rouiljsetmessages: + msg7260
2021-05-27 05:00:35rouiljsetstatus: new -> open
files: + Customize_auth_headers.patch
messages: + msg7251
priority: normal
assignee: rouilj
components: + Web interface, - Infrastructure
2021-05-19 03:48:03rouiljsetkeywords: + Blocker
2017-04-22 10:00:58antmailsetmessages: + msg5960
2017-04-22 01:34:08rouiljsetmessages: + msg5959
2017-04-21 14:57:32antmailsetmessages: + msg5958
2017-04-21 07:41:51antmailsetmessages: + msg5957
2017-04-21 02:17:14rouiljsetmessages: + msg5956
title: New option for web auth -> New option for web auth (also http header passing)
2016-07-31 20:33:32rouiljsettype: behavior -> rfe
2015-03-18 15:21:22antmailsetmessages: + msg5291
2015-03-04 11:59:20antmailsetmessages: + msg5253
2015-03-03 13:52:15jerrykansetmessages: + msg5252
title: New option for web auth [proposed patch] -> New option for web auth
2015-02-27 08:40:40antmailsetmessages: + msg5229
2015-02-26 12:28:53jerrykansetmessages: + msg5227
2014-07-20 03:31:40rouiljsetnosy: + rouilj
messages: + msg5119
2014-04-25 08:09:51antmailsetmessages: + msg5088
2014-04-23 10:10:31bersetnosy: + techtonik
messages: + msg5087
2014-04-23 07:36:48bersetmessages: + msg5086
2014-04-15 08:29:08antmailsetmessages: + msg5081
2014-04-14 13:36:30jerrykansetnosy: + jerrykan
messages: + msg5078
2014-04-11 08:41:36bersetnosy: + ber
messages: + msg5077
2014-04-01 17:55:39antmailcreate