opnsense / plugins

OPNsense plugin collection
https://opnsense.org/
BSD 2-Clause "Simplified" License
842 stars 635 forks source link

www/nginx: IP ACL empty on nginx.conf #1363

Closed Oxima69 closed 4 years ago

Oxima69 commented 5 years ago

With IP ACL defined and enabled in WebGUI, and assign it to a Location there is only # IP ACL entry in the nginx.conf

fabianfrz commented 5 years ago

It works on my machine - did you configure some rules?

Oxima69 commented 5 years ago

I don't understand your question exactly. (rules on what ?) To the understanding ....when i set IP ACL on HTTP Server the entries are shown in the nginx.conf

Szeraax commented 4 years ago

I also have this issue. I will attempt to find the root cause.

I am using 19.1.10 (I just saw there is 19.7 available, I'll upgrade to latest ASAP and report back if bug is still present).

I created a dummy IP ACL with default: deny and added an IP address to the allow list (e.g. 192.168.2.3) I created a dummy location for / and added the IP ACL to the location. (I tried both with and without a upstream assigned - same result each time) I created a dummy http server and added the dummy location to it. When I clicked "generate nginx.conf" from the UI, my resulting location does not have any entry under the IP ACL:

location  / {
    BasicRule wl:19;
    DeniedUrl "/waf_denied.html";
        # IP ACL
    autoindex off;
    http2_push_preload off;
    proxy_set_header Host $host;
    proxy_set_header X-TLS-Cipher $ssl_cipher;
    proxy_set_header X-TLS-Protocol $ssl_protocol;
    proxy_set_header X-TLS-SNI-Host $ssl_server_name;
    # proxy headers for backend server
    proxy_set_header X-Real-IP $remote_addr;
    proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
    proxy_set_header X-Forwarded-Proto $scheme;
    proxy_set_header X-TLS-Client-Intercepted $tls_intercepted;
    proxy_ignore_client_abort off;
    proxy_request_buffering on;
    proxy_max_temp_file_size 1024m;
    proxy_buffering on;
    proxy_pass http://upstreamd2be59635f4b4474865ac9e8f358e745;
    proxy_hide_header X-Powered-By;
fabianfrz commented 4 years ago

can you run configctl template reload OPNsense/Nginx on cli to see if it has an error?

Oxima69 commented 4 years ago
root@ah-fw-01:/ # configctl template reload OPNsense/Nginx
OK

Still the issue.....

location  /api {
    BasicRule wl:19;
    DeniedUrl "/waf_denied.html";
        # IP ACL
    autoindex off;
    http2_push_preload off;
    proxy_set_header Host $host;

I'am on OPNsense 19.7.6-amd64

Szeraax commented 4 years ago

I have added my IP ACL to any location again and ran that command. Gives no error, says "OK" as only output. IP ACL actually has an IP ACL entry for this location. Except, its not the correct entry. And no matter what IP ACL entry I set my location to, running configctl again does not change what gets put into the nginx config file.

Oxima69 commented 4 years ago

Just tested again

Apparently, it looks that way now.

  1. If an IP ACL is set in the HTTP server, it is inherited to the location IP ACL... even if a different rule is set for the location.

  2. If an IP ACL is only set in the location block, it does not appear at all

Szeraax commented 4 years ago

That's consistent with what I'm seeing. Thanks for tracking that down, @Oxima69 !

Szeraax commented 4 years ago

Seems like this is the cause: https://github.com/opnsense/plugins/blob/10727e31029e3138573345620b3688b3f29c30ee/www/nginx/src/opnsense/service/templates/OPNsense/Nginx/location.conf#L46

Szeraax commented 4 years ago

Ok, @fabianfrz . Because I really wanted this functionality, I have done the work to make a new package that I can confirm DOES work: image

Note: I have NO IDEA how we do code tests because that was not mentioned in the workflow document. Steps take to create a new package and test it:

  1. Install OPNSense to a new VM (I use hyper-V on windows 10v1909 pro). Install via SSH due to bug
  2. pkg install git
  3. git clone https://github.com/opnsense/plugins.git
  4. cd plugins/www/nginx
  5. nano src/opnsense/service/templates/OPNsense/Nginx/location.conf
  6. Make needed change to line 46 (set to location.ip_acl instead of server.ip_acl) and exit
  7. make package
  8. pkg install work/pkg/os-nginx-devel-1.17.txz
  9. Go to GUI, bypass wizard, go to nginx plugin, enable nginx
  10. Create 2 IP ACLs (one with a 1.1.1.1 allow called 'serverACL', one with a 2.2.2.2 allow called 'locationACL')
  11. Create a new location and apply the IP acl to it
  12. Create a new http server and apply the location to the server
  13. Click the 'build nginx.conf' gui button
  14. Verify that the file at /usr/local/etc/nginx/nginx.conf only contains 1 # IP ACL block and that it contains the locationACL with 2.2.2.2
  15. Edit the HTTP server and add the siteACL
  16. Click the 'build nginx.conf' gui button
  17. Verify that the file at /usr/local/etc/nginx/nginx.conf only contains 2 # IP ACL blocks and that the server one contains 1.1.1.1 and that the location one still contains 2.2.2.2
  18. Success! image

I hope that this comment will serve as a guide to anyone who wants to make changes to a plugin and test. I will now go submit the PR for this plugin.

Szeraax commented 4 years ago

PR: https://github.com/opnsense/plugins/pull/1653

How long after this PR gets approved will it take before @Oxima69 will be able to use the updated plugin? Should he build the same himself? Should I post my pkg somewhere and have him download+install (if he wants it)?

fabianfrz commented 4 years ago

@Szeraax the fastest way is to use the OPNsense patch utility. You can pass your user repository to apply it. Since there was no release over the holidays I think there will be one soon and if that get accepted it will be included.

fabianfrz commented 4 years ago

merged. Execute the following command to apply the patch:

opnsense-patch -c plugins 0b3b0be
Oxima69 commented 4 years ago

is now working as expected !

thank you

Szeraax commented 4 years ago

@Oxima69 Can you close this issue?