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

Allow multiple IP addresses #39

Open
rgomezceis opened this issue Dec 25, 2022 · 9 comments
Open

Allow multiple IP addresses #39

rgomezceis opened this issue Dec 25, 2022 · 9 comments
Labels
enhancement New feature or request

Comments

@rgomezceis
Copy link

Could be useful set multiple ip addresses (now whalewall only allow ranges or a single IP addr) delimited by commas in a single whalewall rule to simplify it.

Actual:

whalewall.enabled: true
whalewall.rules: |
     output:
         # allow ssh server 1
          - ip: "172.16.0.1"
            proto: tcp
            port: 22
          # allow ssh server 2
          - ip: "192.168.1.2"
            proto: tcp
            port: 22

Enhanced:

whalewall.enabled: true
whalewall.rules: |
     output:
         # allow ssh for servers
         - ip: "172.16.0.1, 192.168.1.2"
           proto: tcp
           port: 22
@rgomezceis rgomezceis changed the title Allow multiple IP Allow multiple IP addresses Dec 25, 2022
@capnspacehook capnspacehook added the enhancement New feature or request label Dec 26, 2022
@sammcj
Copy link

sammcj commented Aug 22, 2023

I just found that this isn't currently possible, unfortunately this + #72 is a show stopper for me, I might try and have a look through the code to see if I can contribute but I'm not much of a dev 😅

A example of where multiple IPs might be required is if you have one IP that's a load balancer (say traefik) for local network routing, and one or more IPs that can access it directly.

@capnspacehook
Copy link
Owner

I agree the ability to specify multiple IPs per rule would be nice, but to be clear it wouldn't add functionality that isn't currently possible now. Creating multiple whalewall rules with the same options but different IPs would create the same nftable rules as creating one whalewall rule with multiple IPs.

Unless I'm missing something, could you create multiple rules in the meantime @sammcj?

I welcome contributions, if you do decide to take a stab at it and need help feel free to open a discussion and I'll help as best I can.

@sammcj
Copy link

sammcj commented Aug 24, 2023

Oh can you @capnspacehook?

I tried a bunch of different things that seemed to be invalid:

mapped_ports:
  external:
    allow: true
    ip:
      - 192.168.0.0/24
      - 172.18.0.0/8

mapped_ports:
  external:
    allow: true
    ip:
      192.168.0.0/24
      172.18.0.0/8

mapped_ports:
  external:
    allow: true
    ip: 192.168.0.0/24
    ip: 172.18.0.0/8

mapped_ports:
  external:
    allow: true
    ip: 192.168.0.0/24,172.18.0.0/8

mapped_ports:
  external:
    allow: true
      - 192.168.0.0/24
	  - 172.18.0.0/8

mapped_ports:
  external:
    allow: true
    ip: ["192.168.0.0/24", "172.18.0.0/8"]

etc...

Also the wording around multiple items is a little confusion (for me at least).

#optional; an IP address, CIDR, or range of IP addresses to allow traffic from
ip: ""

Does "range" here mean:

  • 192.168.0.2-192.168.0.254
  • 192.168.0.2:192.168.0.254
  • 192.168.0.[2-254]
  • 192.168.0.[2:254]

@capnspacehook
Copy link
Owner

Oh you're trying to set multiple IP addresses with mapped port rules... Yeah that isn't possible now unfortunately. Now what you were saying before makes more sense. In this case the ability to specify multiple IP addresses per rule would actually add new functionality. I think now you should be able to specify multiple mapped port rules anyway, but being able to specify multiple IPs per rule would be a good start.

As for IP ranges, the format is as you first stated: 192.168.0.2-192.168.0.254. I'll add examples of the to the README.

@capnspacehook
Copy link
Owner

capnspacehook commented Aug 27, 2023

@sammcj just added support for multiple IP addresses per rule, can you try with the latest version of whalewall and confirm it works for your use case?

Note that you'll now need to use ips instead of ip, see the updated README for more info.

@sammcj
Copy link

sammcj commented Aug 28, 2023

Awesome, thanks so much for looking into this @capnspacehook !

I just tried it and it complains that no ip (without s) rule was found:

~ docker pull ghcr.io/capnspacehook/whalewall:latest

latest: Pulling from capnspacehook/whalewall
Digest: sha256:2ee79671290e7accddc4e3ab1f3c82f2045ddb2d0de2eee50e95c469bea30ffc
Status: Image is up to date for ghcr.io/capnspacehook/whalewall:latest
ghcr.io/capnspacehook/whalewall:latest

~ docker logs -f whalewall

{"level":"info","time":"2023-08-28T22:36:23.056065333Z","msg":"creating rules","container.id":"a348b9fab27e","container.name":"mqtt","container.is_new":true}
{"level":"error","time":"2023-08-28T22:36:23.056150865Z","msg":"error creating rules","container.id":"a348b9fab27e","container.name":"mqtt","error":"error parsing rules: yaml: line 12: did not find expected key","stacktrace":"github.com/capnspacehook/whalewall.(*RuleManager).createRules\n\tgithub.com/capnspacehook/whalewall/create.go:63\ngithub.com/capnspacehook/whalewall.(*RuleManager).Start.func1\n\tgithub.com/capnspacehook/whalewall/manager.go:118"}

config:

      whalewall.enabled: true
      whalewall.rules: |

        mapped_ports: ## inbound traffic
          external:
            allow: true
            ips:
              - 172.16.0.0/13 # all internal services
              - 192.168.0.169 # local admin ip for testing
              - 192.168.0.199 # local admin ip for testing
          localhost:
            allow: true
        output: ## outbound traffic
          ips:
            - 172.16.0.0/13 # all internal services
            - 192.168.0.169 # local admin ip for testing
            - 192.168.0.199 # local admin ip for testing
          - proto: tcp
            dst_ports:
              - 443
              - 80
              - 1883 # allow mqtt out
          - proto: udp
            dst_ports:
              - 53 # allow DNS requests out
              - 1883

Looking at the updated docs, I saw that external doesn't have the ips option like output has, so I removed the external ips but the same error occurred:

{"level":"info","time":"2023-08-28T22:40:46.563022736Z","msg":"creating rules","container.id":"614cfa9ca740","container.name":"mqtt","container.is_new":true}
{"level":"error","time":"2023-08-28T22:40:46.56307783Z","msg":"error creating rules","container.id":"614cfa9ca740","container.name":"mqtt","error":"error parsing rules: yaml: line 9: did not find expected key","stacktrace":"github.com/capnspacehook/whalewall.(*RuleManager).createRules\n\tgithub.com/capnspacehook/whalewall/create.go:63\ngithub.com/capnspacehook/whalewall.(*RuleManager).Start.func1\n\tgithub.com/capnspacehook/whalewall/manager.go:118"}

Is there a way to get the error logs to print the lint that it has an issue with?

@capnspacehook
Copy link
Owner

capnspacehook commented Aug 28, 2023

'mapped_ports.external' does have an 'ips' field, that's a mistake on my part. Updated the README.

I think the issue is that your first 'output' rule is not a list entry, but a key instead. As for improving the error message, I don't think that's possible unfortunately, I'm pretty sure that error is directly from the yaml parser I'm using. I can look to see if I can improve the error message but can't make any promises.

@sammcj
Copy link

sammcj commented Aug 28, 2023

Ah ok, I might submit a PR to add to the documentation so that idiots like me find it easier to follow 😄

I think what keeps throwing me is the combination of terminology (external vs outbound vs output vs inbound) and the difference / mix of the use of lists and keys.

For example:

mapped_ports:      <- (inbound rules)
  localhost:       <- K:V but not a list like outbound rules
    allow: false
    log_prefix: ""  <- log_prefix k/v
    verdict:
      chain: ""
...
  external:
    allow: false
    log_prefix: ""
    ip: []
output:              <- output (outbound rules) are not K/V like mapped_ports (inbound rules) but are a list
  - log_prefix: ""   <- log_prefix list
    network: ""
    ips: []

So while this seems to run without errors:

      whalewall.enabled: true
      whalewall.rules: |

        mapped_ports: ## inbound traffic
          external:
            allow: true
            ips:
              - 172.16.0.0-172.21.0.254
              - 172.16.0.0-172.21.0.254
              - 192.168.0.2-192.168.0.254
          localhost:
            allow: true
        output: ## outbound traffic
          - proto: tcp
            dst_ports:
              - 443
              - 80
          - proto: udp
            dst_ports:
              - 53 # allow DNS requests out

Looking at the documentation, I think it should be more like:

      whalewall.enabled: true
      whalewall.rules: |

        mapped_ports: ## inbound traffic
          external:
            log_prefix: "internal-services"
            allow: true
            ips:
              - 172.16.0.0-172.21.0.254
              - 192.168.0.2-192.168.0.254
          localhost:
            allow: true
        output: ## outbound traffic
          - log_prefix: "http"
            proto: tcp
            dst_ports:
              - 443
              - 80
          - log_prefix: "dns"
            proto: udp
            dst_ports:
              - 53

Which also seems to work without errors.

@capnspacehook
Copy link
Owner

Yeah feel free to make a PR to improve the docs, I'm sure they could be improved.

I'm not exactly sure what you're trying to convey with your two different versions of the config, the main difference I see is the second config uses 'log_prefix'?

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

No branches or pull requests

3 participants