[PATCH 18 of 20] Upstream: multiple WWW-Authenticate headers (ticket #485)

[Maxim Dounin]

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 <mdounin at 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 <mdounin at 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/