Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

satisfy any; will always allow #61

Closed
Zoey2936 opened this issue Jan 31, 2024 · 8 comments · Fixed by #62
Closed

satisfy any; will always allow #61

Zoey2936 opened this issue Jan 31, 2024 · 8 comments · Fixed by #62

Comments

@Zoey2936
Copy link

Zoey2936 commented Jan 31, 2024

when having the following auth_basic and access configuration inside nginx, while also using crowdsec, it will always return 200 and not 401/403

    auth_basic "Authorization required";
    auth_basic_user_file /data/etc/access/3;
    allow 127.0.0.1;
    deny all;
    satisfy any;

if I disable crowdsec it will return 401 and ask for a password and username

@blotus
Copy link
Member

blotus commented Jan 31, 2024

Hello,

This is because the bouncer checks if the IP is allowed during the `NGX_HTTP_ACCESS_PHASE ' which is the phase where the auth basic module performs the credentials check.

satisfy any will allow the request if any of the access handlers let the request through.
So because the bouncer accepts the request, it's enough for nginx, and the credentials are not checked.

If you only need basic auth (ie, not allow 1.2.3.4;), I think satisfy all should work.

Another solution would be to move the bouncer to an earlier phase to ensure no conflict with any other access handler. The rewrite phase is a good candidate for this as it runs before the access phase (but it's not semantically correct).

You can try this for yourself if you want: you can edit the file /etc/nginx/conf.d/crowdsec_nginx.conf (or the equivalent one if using openresty) and replace the access_by_lua_block line with rewrite_by_lua_block, then reload nginx.

I did a very quick test, and it seems to work, but keep in mind that this might have some unintended side effects (for example, if you are using the appsec component, there's a risk the bouncer will run before the rewrite (this depends on which order nginx will load the modules, which depends on either the compile options when using static modules, or the module itself when using dynamic modules) and thus pass a potentially wrong URL to crowdsec)

@blotus
Copy link
Member

blotus commented Feb 1, 2024

I think I found a much better fix: our handler does not explicitly decline to handle the request so nginx (or mod_lua, not sure) seems to assume that we allow it by default.

Making the handler return ngx.DECLINED if there's no decision for the IP seems to solve the issue, I'll investigate more tomorrow.

@Zoey2936
Copy link
Author

Zoey2936 commented Feb 1, 2024

Thanks!

@Zoey2936
Copy link
Author

Zoey2936 commented Feb 6, 2024

when will it be released?

@blotus
Copy link
Member

blotus commented Feb 6, 2024

We are preparing the release, so most likely this afternoon or tomorrow at the latest.

@Zoey2936
Copy link
Author

Zoey2936 commented Feb 6, 2024

thanks

@blotus
Copy link
Member

blotus commented Feb 6, 2024

@Zoey2936 Looking at your Dockerfile, it looks like you are cloning the nginx bouncer main branch directly.
You don't have to wait for a proper github release, calling make will clone the main branch of cs-lua-bouncer, and the version has already been updated for cs-nginx-bouncer, so you should be good to go.

@Zoey2936
Copy link
Author

Zoey2936 commented Feb 6, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants