Opened 12 years ago

Closed 11 years ago

#285 closed defect (fixed)

When "satisfy any" is set the unauth access_code (401) should not be overriden by forbidden (403) access_code

Reported by: Jan Marc Hoffmann Owned by:
Priority: major Milestone:
Component: nginx-core Version: 1.3.x
Keywords: satisfy access Cc:
uname -a: Linux cache 2.6.32-17-pve #1 SMP Tue Jan 8 12:40:35 UTC 2013 x86_64 Intel(R) Core(TM) i7-3770S CPU @ 3.10GHz GenuineIntel GNU/Linux
nginx -V: nginx version: nginx/1.3.11
TLS SNI support enabled
configure arguments: --prefix=/usr --sbin-path=/usr/sbin/nginx --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error_log --pid-path=/var/run/nginx.pid --lock-path=/var/lock/nginx.lock --user=nginx --group=nginx --with-cc-opt=-I/usr/include --with-ld-opt=-L/usr/lib --http-log-path=/var/log/nginx/access_log --http-client-body-temp-path=/var/tmp/nginx/client --http-proxy-temp-path=/var/tmp/nginx/proxy --http-fastcgi-temp-path=/var/tmp/nginx/fastcgi --http-scgi-temp-path=/var/tmp/nginx/scgi --http-uwsgi-temp-path=/var/tmp/nginx/uwsgi --with-debug --with-pcre --with-http_sub_module --with-http_realip_module --add-module=/var/tmp/portage/www-servers/nginx-1.3.11/work/ngx_pagespeed-master --add-module=/var/tmp/portage/www-servers/nginx-1.3.11/work/nginx-auth-ldap-master --with-http_ssl_module --without-mail_imap_module --without-mail_pop3_module --without-mail_smtp_module

Description

Current behaviour:

Two access modules are registered. Module1 returns NGX_HTTP_FORBIDDEN and module2 returns NGX_HTTP_UNAUTHORIZED on the first request. Nginx returns the code of the first module loaded. So the first request will return NGX_HTTP_FORBIDDEN.

Problem:

This does not go along with the documentation which states that any module can grant access. After returning 403 the browser will not send Basic Authentication. So module2 never gets a chance to grant access.

Expected:

If "satisfy any" is selected modules will not overwrite unauth with forbidden.

I have attached a patch which fixes this problem.

Attachments (2)

satisfy_any_unauth_forbidden.patch (597 bytes ) - added by Jan Marc Hoffmann 12 years ago.
satisfy_any_unauth_forbidden.2.patch (556 bytes ) - added by Jan Marc Hoffmann 12 years ago.
-----------------------------87109875749142727153684006 Content-Disposition: form-data; name="replace" on

Download all attachments as: .zip

Change History (8)

by Jan Marc Hoffmann, 12 years ago

comment:1 by Maxim Dounin, 12 years ago

Status: newaccepted

The patch looks wrong as it contains check which is always false, but in general change to prefer 401 over 403 looks right. As of now there is no problem with official nginx modules due to access module being checked before auth basic, but it would be clearly beneficial for 3rd party modules.

comment:2 by Jan Marc Hoffmann, 12 years ago

Hi Maxim,

which check is always false?

A = clcf->satisfy == NGX_HTTP_SATISFY_ALL (true on satisfy all, false on satisfy any)
B = r->access_code != NGX_HTTP_UNAUTHORIZED (true on forbidden, false on unauth)

A, B, A or B
true true true
true false true
false true true
false false false

So its only false on satisfy any and unauth, which is correct since in this case it should not get overriden.

comment:3 by Maxim Dounin, 12 years ago

The code is in else part of the following if():

    if (clcf->satisfy == NGX_HTTP_SATISFY_ALL) {
        ...
    } else {
        ...
    }

Hence clcf->satisfy can't be NGX_HTTP_SATISFY_ALL there. And it's logical, as with NGX_HTTP_SATISFY_ALL the error is immediately returned.

comment:4 by Jan Marc Hoffmann, 12 years ago

Right. It's a dead check so it can get safely removed.

Version 0, edited 12 years ago by Jan Marc Hoffmann (next)

by Jan Marc Hoffmann, 12 years ago


Content-Disposition: form-data; name="replace"

on

comment:5 by Maxim Dounin <mdounin@…>, 11 years ago

In fcecb9c6a0573f2edd48ff87cef69bd7e7523729/nginx:

Fixed "satisfy any" if 403 is returned after 401 (ticket #285).

The 403 (Forbidden) should not overwrite 401 (Unauthorized) as the
latter should be returned with the WWW-Authenticate header to request
authentication by a client.

The problem could be triggered with 3rd party modules and the "deny"
directive, or with auth_basic and auth_request which returns 403
(in 1.5.4+).

Patch by Jan Marc Hoffmann.

comment:6 by Maxim Dounin, 11 years ago

Resolution: fixed
Status: acceptedclosed

Patch committed, thanks.

Note: See TracTickets for help on using tickets.