Issue 2550837
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:56 | rouilj | set | keywords:
- Blocker status: open -> fixed resolution: fixed messages: + msg7286 |
2021-06-06 04:09:18 | rouilj | set | messages: + msg7260 |
2021-05-27 05:00:35 | rouilj | set | status: new -> open files: + Customize_auth_headers.patch messages: + msg7251 priority: normal assignee: rouilj components: + Web interface, - Infrastructure |
2021-05-19 03:48:03 | rouilj | set | keywords: + Blocker |
2017-04-22 10:00:58 | antmail | set | messages: + msg5960 |
2017-04-22 01:34:08 | rouilj | set | messages: + msg5959 |
2017-04-21 14:57:32 | antmail | set | messages: + msg5958 |
2017-04-21 07:41:51 | antmail | set | messages: + msg5957 |
2017-04-21 02:17:14 | rouilj | set | messages:
+ msg5956 title: New option for web auth -> New option for web auth (also http header passing) |
2016-07-31 20:33:32 | rouilj | set | type: behavior -> rfe |
2015-03-18 15:21:22 | antmail | set | messages: + msg5291 |
2015-03-04 11:59:20 | antmail | set | messages: + msg5253 |
2015-03-03 13:52:15 | jerrykan | set | messages:
+ msg5252 title: New option for web auth [proposed patch] -> New option for web auth |
2015-02-27 08:40:40 | antmail | set | messages: + msg5229 |
2015-02-26 12:28:53 | jerrykan | set | messages: + msg5227 |
2014-07-20 03:31:40 | rouilj | set | nosy:
+ rouilj messages: + msg5119 |
2014-04-25 08:09:51 | antmail | set | messages: + msg5088 |
2014-04-23 10:10:31 | ber | set | nosy:
+ techtonik messages: + msg5087 |
2014-04-23 07:36:48 | ber | set | messages: + msg5086 |
2014-04-15 08:29:08 | antmail | set | messages: + msg5081 |
2014-04-14 13:36:30 | jerrykan | set | nosy:
+ jerrykan messages: + msg5078 |
2014-04-11 08:41:36 | ber | set | nosy:
+ ber messages: + msg5077 |
2014-04-01 17:55:39 | antmail | create |