Project

General

Profile

Feature #322

FastCGI Authorizer support for Variable-name variable passing

Added by Anonymous about 12 years ago. Updated about 1 year ago.

Status:
Fixed
Priority:
Normal
Assignee:
-
Category:
mod_fastcgi
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
Missing in 1.5.x:
No

Description

The FastCGI Spec states that authorizers can emit headers of the format Variable-name: value and those variables will be placed into the environment of all subsequent authorized requests as name: value. It would be great if lighttpd supported this.

See http://www.fastcgi.com/devkit/doc/fcgi-spec.html#S6.3 for further information.

-- cpisto

fastcgi-authorizer-fixes.diff (6.61 KB) fastcgi-authorizer-fixes.diff All fastcgi mode=authorizer fixes (Variable- env works, proper re-dispatching, and assert failure fix when auth is running in front of cgi). maherb, 2006-06-20 11:58
Lighty1.patch (3.92 KB) Lighty1.patch Updated original patch for 1.4.24. EDevil, 2009-10-28 11:38
feature-322.patch (5.8 KB) feature-322.patch Updated patch for 1.4.35, no longer relying on con->authed_user ckreutzer, 2016-01-09 08:24

Associated revisions

Revision 2dcfe173 (diff)
Added by gstrauss about 1 year ago

[mod_fastcgi] Authorizer support with Responder (fixes #321, fixes #322)

import Variable-* from FastCGI authorizer response into con->environment
restart request after FastCGI authorizer if no fastcgi.server docroot

(thx Christoph Kreutzer for initial patch attempt)

x-ref:
"mod_fastcgi authorizers cannot protect fastcgi responders"
http://redmine.lighttpd.net/issues/321

x-ref:
"FastCGI Authorizer support for Variable-name variable passing"
http://redmine.lighttpd.net/issues/322

github: closes #70

Revision 7ef569b2 (diff)
Added by Christoph Kreutzer about 1 year ago

[tests] test coverage for issues (#321, #322)

FastCGI Authorizer support with FastCGI Responders

x-ref:
"mod_fastcgi authorizers cannot protect fastcgi responders"
http://redmine.lighttpd.net/issues/321

x-ref:
"FastCGI Authorizer support for Variable-name variable passing"
http://redmine.lighttpd.net/issues/322

History

#1

Updated by maherb over 11 years ago

I agree, this support would be excellent, especially if {{{mod@mod_accesslog@ allowed you to print these environment variables in your access log. For example, the most common use case for an authorizer FastCGI process is when you want to set the REMOTE_USER cgi environment variable, and most people will want to log the contents of that environment variable.

#2

Updated by maherb over 11 years ago

If you download and apply the attached fastcgi-authorizer-fixes.diff file, both #321 and #322 will be fixed (as well as a fix for infinite 301 redirection when specifying "/" as your authorizer).

#3

Updated by Anonymous almost 11 years ago

This is excellent news. When can this be applied to the main trunk?

-- André Cruz

#4

Updated by EDevil almost 11 years ago

I think that with this patch we still have to opt between serving static content or another fcgi application, by configuring or not a docroot.

Why can't we serve both? If I protect the / URL with an authorizer I want to serve static content, the site CSS, and process the PHP files using a fcgi application.

I enabled this by setting con->status = 0 on line 3114 of mod_fastcgi.c (after the patch). This way mod_staticfile catches the request.

Is there something I am forgetting with this small change?

#5

Updated by maherb almost 11 years ago

Replying to EDevil:

I think that with this patch we still have to opt between serving static content or
another fcgi application, by configuring or not a docroot.

This should not be the case. I've used this patch successfully for many months to serve static content, cgi content, and fastcgi content "behind" a fastcgi authorizer.

Why can't we serve both? If I protect the / URL with an authorizer I want to serve
static content, the site CSS, and process the PHP files using a fcgi application.

I enabled this by setting con->status = 0 on line 3114 of mod_fastcgi.c (after the
patch). This way mod_staticfile catches the request.

The connection struct (typedef struct defined in base.h) does not have a status field. It does have an http_status field. Perhaps you tried to apply the patch against an older version of lighttpd? I've applied the patch against svn://svn.lighttpd.net/lighttpd/branches/lighttpd-1.4.x/.

Is there something I am forgetting with this small change?

#6

Updated by Anonymous over 9 years ago

FYI, this patch does not apply cleanly since 1.4.15. It's about time this patch is integrated.

#7

Updated by EDevil over 9 years ago

You're right, it's http_status.

It ends up like this:


                             con->http_status == 0)) {
                                /*
                                 * If we are here in AUTHORIZER mode then a request for autorizer
                                 * was proceeded already, and status 200 has been returned. We need
                                 * now to handle autorized request.
                                 */
                                con->http_status = 0;
                                if ( ! buffer_is_empty(host->docroot) ) {
                                        /*
                                         * Serve local file if they specified
                                         * a docroot
                                         */
#8

Updated by stbuehler over 9 years ago

An enhancement with severity "blocker"? you are funny...

I think we shouldn't use con->authed_user to decide if we should do a authorizer request - that would mean every authorizer behind mod_auth gets broken, which is not acceptable imho.

#9

Updated by stbuehler about 9 years ago

  • Target version changed from 1.4.20 to 1.4.21
#10

Updated by icy about 9 years ago

  • Priority changed from Urgent to Normal
  • Patch available set to No
#11

Updated by icy almost 9 years ago

  • Target version changed from 1.4.21 to 1.4.22
#12

Updated by stbuehler almost 9 years ago

  • Target version changed from 1.4.22 to 1.4.23
#13

Updated by stbuehler over 8 years ago

  • Assignee deleted (jan)
  • Target version changed from 1.4.23 to 1.4.x
#14

Updated by EDevil over 8 years ago

This bug has a patch available. It is attached.

#15

Updated by mimic about 8 years ago

I have tried to patch a more recent version but it seems authorizer does not work for FastCGI URL prefix based content. Could somebody provide an example of configuration for using FastCGI in both authorizer and responder for URL prefix? Could somebody provide an updated patch?

#16

Updated by EDevil about 8 years ago

Attached is the updated patch for 1.4.24.

Several people have been using this for years with 0 problems so I would appreciate it if this was included in trunk. If more changes are needed so that this patch is applied please state them so that this issue can finally be closed.

Best regards.

#17

Updated by stbuehler about 8 years ago

  • Target version changed from 1.4.25 to 1.4.26
  • Missing in 1.5.x set to No
#18

Updated by stbuehler almost 8 years ago

  • Target version changed from 1.4.26 to 1.4.27
#19

Updated by stbuehler over 7 years ago

  • Target version changed from 1.4.27 to 1.4.x
#20

Updated by ckreutzer almost 2 years ago

EDevil wrote:

Attached is the updated patch for 1.4.24.

In 1.4.33 there were massive changes to mod_fastcgi.c, and con->authed_user has been removed. I've now modified the current Debian stable version (1.4.35-4) using a custom environment variable. This works fine for me using the Shibboleth SP 2.5.3 and a lighttpd FastCGI configuration based on the one given in the SHIB2 wiki.

I've attached the patch. Maybe someone's interested, since the lighttpd2 snapshots don't seem the have any authorizer mode :/

#21

Updated by gstrauss over 1 year ago

  • Status changed from New to Need Feedback

ckreutzer: this patch purportedly combines three different issues. Besides the feature for setting Variable-* in the environment, are the other two still valid? Please review the commits in the past few months which improve robustness of fastcgi in lighttpd. My read of the patch looks like it extends FastCGI authorizer mode to send static file (if authorized) if host->docroot is set (which is the current behavior), or else (if host->docroot is not set), create a new behavior which sets an environment variable and restarts request processing so that any module can handle the subsequent request, such as mod_cgi.

To apply to current master, the patch needs to be updated to at least
- array search for "FastCGI-Authorizer" should be lifted outside loop
(auth_cnt = (data_string *)array_get_element(con->environment, "FastCGI-Authorizer");)
- access to buffer->used member should be modified to employ buffer.h interfaces instead.
- ... possibly other changes

ckreutzer: Would you be interested in explaining what the patch is trying to accomplish and to help redo the patch? Thanks.

#22

Updated by ckreutzer over 1 year ago

Sorry for the late response, didn't watched this issue until now.

gstrauss wrote:

ckreutzer: this patch purportedly combines three different issues. Besides the feature for setting Variable-* in the environment, are the other two still valid? Please review the commits in the past few months which improve robustness of fastcgi in lighttpd. My read of the patch looks like it extends FastCGI authorizer mode to send static file (if authorized) if host->docroot is set (which is the current behavior), or else (if host->docroot is not set), create a new behavior which sets an environment variable and restarts request processing so that any module can handle the subsequent request, such as mod_cgi.

That's correct. I added the decision on host->docroot to not totally reverse the original lighty way of handling.

For the issues:
  • as you pointed out, this handles #322
  • #321 is also handled - as you pointed out -, because the request handling will be restarted and the fastcgi responder jumps in (in my case PHP)
  • "(as well as a fix for infinite 301 redirection when specifying "/" as your authorizer)" - this should be fixed, too, I use it like this: "/" => (( [...] "mode" => "authorizer" )),

To apply to current master, the patch needs to be updated to at least

I would be happy if this could be contributed upstream!

ckreutzer: Would you be interested in explaining what the patch is trying to accomplish and to help redo the patch? Thanks.

I will try to help. C is not my day to day job and I just digged into the lighty sources for this issue ;)
My objective was to use the Shibboleth SP as FastCGI Authorizer in front of a PHP FastCGI responder (like a lot of people here, I like it much more than Apache). I used a lot of code from EDevil's patch and just tried to mangle it in the much reworked fastcgi code. Things he did which weren't working any longer I just rewrote with workarounds I found screening the existing code.

I will review the changes between 1.4.39 and 1.4.40 in the next few days, I hope, and try to come up with a clean solution.

#23

Updated by gstrauss over 1 year ago

Great to hear back!

I haven't looked at this patch in a while, but recently made a commit to mod_cgi to enable internal redirect from a subrequest handler. Take a look at 8861c2bb for some inspiration. :)

#24

Updated by ckreutzer over 1 year ago

I started working on it yesterday and created a modified fastcgi-auth.conf and corresponding fcgi-auth.c, fcgi-responder.c, so I can check the results.

gstrauss wrote:

I haven't looked at this patch in a while, but recently made a commit to mod_cgi to enable internal redirect from a subrequest handler. Take a look at 8861c2bb for some inspiration. :)

Do I understand correctly that if cgi_handle_fdevent(server *srv, void *ctx, int revents) returns HANDLER_COMEBACK, the request will be restarted? Like it was done in the patch before using something like:
+ fcgi_connection_close(srv, hctx);
+ con->mode = DIRECT;
+ con->file_started = 0;
+ con->file_finished = 0;
+ buffer_reset(con->physical.path);

Then it should be possible to use the same behavior in fcgi_handle_fdevent?

I need to study the old patch and the new mod_fastcgi.c more closely in the evening.

#25

Updated by gstrauss over 1 year ago

Yes. It sounds like you're on the right track. The handler needs to fully clean up if the request is to be restarted, since a new handler might be used, or a new fastcgi handler might be used.

#26

Updated by ckreutzer over 1 year ago

Hi,

I opened a PR on GitHub: https://github.com/lighttpd/lighttpd1.4/pull/70

If there are any improvements needed, feel free to comment in the diffs :)

Regards,
Christoph

#27

Updated by gstrauss over 1 year ago

sorry, I haven't had a chance to dive into this again. I'll try to do so the weekend of Sep 10.

#28

Updated by ckreutzer over 1 year ago

No problem, take your time. Looking forward to your feedback ;)

#29

Updated by gstrauss about 1 year ago

  • Status changed from Need Feedback to Patch Pending
  • Target version changed from 1.4.x to 1.4.42
#30

Updated by gstrauss about 1 year ago

  • Status changed from Patch Pending to Fixed
  • % Done changed from 90 to 100

Also available in: Atom