Hello!
On Thu, May 12, 2022 at 01:03:37AM +0400, Sergey Kandaurov wrote:
> On Thu, Apr 21, 2022 at 01:18:58AM +0300, Maxim Dounin wrote:
> > # HG changeset patch
> > # User Maxim Dounin <mdou...@mdounin.ru>
> > # Date 1650492341 -10800
> > # Thu Apr 21 01:05:41 2022 +0300
> > # Node ID fa9751ffe7723a11159c158078e454671e81cb87
> > # Parent 2027b85971d4b8a7e33c018548468057cb57eaf7
> > Upstream: multiple WWW-Authenticate headers (ticket #485).
> >
> > When using proxy_intercept_errors and an error page for error 401
> > (Unauthorized), multiple WWW-Authenticate headers from the upstream server
> > response are now properly copied to the response.
> >
> > diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
> > --- a/src/http/ngx_http_upstream.c
> > +++ b/src/http/ngx_http_upstream.c
> > @@ -2647,7 +2647,7 @@ ngx_http_upstream_intercept_errors(ngx_h
> > {
> > ngx_int_t status;
> > ngx_uint_t i;
> > - ngx_table_elt_t *h;
> > + ngx_table_elt_t *h, *ho, **ph;
> > ngx_http_err_page_t *err_page;
> > ngx_http_core_loc_conf_t *clcf;
> >
> > @@ -2676,18 +2676,26 @@ ngx_http_upstream_intercept_errors(ngx_h
> > if (status == NGX_HTTP_UNAUTHORIZED
> > && u->headers_in.www_authenticate)
> > {
> > - h = ngx_list_push(&r->headers_out.headers);
> > -
> > - if (h == NULL) {
> > - ngx_http_upstream_finalize_request(r, u,
> > + h = u->headers_in.www_authenticate;
> > + ph = &r->headers_out.www_authenticate;
> > +
> > + while (h) {
> > + ho = ngx_list_push(&r->headers_out.headers);
> > +
> > + if (ho == NULL) {
> > + ngx_http_upstream_finalize_request(r, u,
> >
> > NGX_HTTP_INTERNAL_SERVER_ERROR);
> > - return NGX_OK;
> > + return NGX_OK;
> > + }
> > +
> > + *ho = *h;
> > + ho->next = NULL;
> > +
> > + *ph = ho;
> > + ph = &ho->next;
> > +
> > + h = h->next;
> > }
> > -
> > - *h = *u->headers_in.www_authenticate;
> > - h->next = NULL;
> > -
> > - r->headers_out.www_authenticate = h;
> > }
> >
> > #if (NGX_HTTP_CACHE)
> >
>
> While the patch certainly looks correct,
> I'm not sure about usefulness of r->headers_out.www_authenticate.
> Is the header accessed directly through this pointer ever?
> For read purposes I mean. Header filters seem not.
>
> The only place is in the auth_request module,
> but it doesn't seem to make sense in subrequest.
The auth request module uses it to copy the WWW-Authenticate
headers from the subrequest response to the main request. For
example, in a configuration like:
location / { auth_request /auth; ... }
location /auth { auth_basic foo; ... }
While such configurations are unlikely, as using auth_basic
directly should be easier, these are certainly possible.
Similarly, r->headers_out.www_authenticate can be used with
proxy_intercept_errors + error_page 401 in the subrequest (since
r->upstream can be cleared by the error page).
BTW, looking at this once again, I tend to think we also need
ngx_http_core_access_phase() changes for proper handling of
multiple WWW-Authenticate headers. It might not be a good idea to
commit it right now though, see commit log.
On the other hand, breaking it all at once might be less painful
(after all, the auth_request change will already made such modules
to segfault when used with auth_request; though this is unlikely,
since such configurations hardly make sense).
# HG changeset patch
# User Maxim Dounin <mdou...@mdounin.ru>
# Date 1652405911 -10800
# Fri May 13 04:38:31 2022 +0300
# Node ID 0c2cf997bde390c9be27e0685ee2a00024c57e4e
# Parent c53cb6f31b9855020d85476fa98ef1cdca809aed
Multiple WWW-Authenticate headers with "satisfy any;".
If a module adds multiple WWW-Authenticate headers (ticket #485) to the
response, linked in r->headers_out.www_authenticate, all headers are now
cleared if another module later allows access.
This change is a nop for standard modules, since the only access module which
can add multiple WWW-Authenticate headers is the auth request module, and
it is checked after other standard access modules. Though this might
affect some third party access modules.
Note that if a 3rd party module adds a single WWW-Authenticate header
and not yet modified to set the header's next pointer to NULL, attempt to
clear such a header with this change will result in a segmentation fault.
diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c
+++ b/src/http/ngx_http_core_module.c
@@ -1088,6 +1088,7 @@ ngx_int_t
ngx_http_core_access_phase(ngx_http_request_t *r, ngx_http_phase_handler_t *ph)
{
ngx_int_t rc;
+ ngx_table_elt_t *h;
ngx_http_core_loc_conf_t *clcf;
if (r != r->main) {
@@ -1122,8 +1123,8 @@ ngx_http_core_access_phase(ngx_http_requ
if (rc == NGX_OK) {
r->access_code = 0;
- if (r->headers_out.www_authenticate) {
- r->headers_out.www_authenticate->hash = 0;
+ for (h = r->headers_out.www_authenticate; h; h = h->next) {
+ h->hash = 0;
}
r->phase_handler = ph->next;
--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-le...@nginx.org
