Feature Request: support for port based firewall

[nhas]

Feature Request: support for port based firewall

[NHAS]

Have a build to try out: https://github.com/NHAS/wag/releases/tag/v5.0.0-pre-release

[NHAS]

Huh. That definitely one way you could work around it.
But in reality you shouldnt have to work around it, otherwise things can go wrong. So yep, I'll add your voice to the list of people saying "hey maybe your firewall should actually be a firewall" :P

[NHAS]

Unfortunately for the syntax you've proposed you can't do that with json as you'd require the whole rule to be an object e.g

"Mfa": {...}

Im generally trying to keep the syntax more of less the same.

There will be an any/any, which is just as it is now if you define an address/domain without any port declaration then it'll assume any/any..

The default will be block. So if an address isn't specified it will be dropped and if a port isn't specified in a group of ports it'll be dropped

[NHAS]

Well. Im not going to say that was a particularly easy or fun experience. But the feature now exists on the unstable branch.

From now on the acl rules will respect more complicated definitions of rules for examples:

Any rules:
"1.1.1.1": Allows all ports and protocols to 1.1.1.1/32
"1.1.1.1 54/any": Allows both tcp and udp to 1.1.1.1/32

Single ports:
192.168.1.1 22/tcp 53/udp: Fairly self explanatory, allows you to hit 22/tcp and 53/udp on a host

Icmp:
1.1.1.1 icmp: As icmp doesnt have ports really you dont need it either

Port ranges:
192.168.1.1 22-1024/tcp 53-23/any: Format is low port-high port/service

I've accomplished this in a fairly round about way. Previously wag used 2 LPM (longest prefix match) trie structures that effectively took the IPv4 (32 bits) as a key to look up a src ip was in the MFA table, or Public access table.

Now we have 3 different types key data.
Range, Any, and Single.

Without going in to too much detail Range and Any contain less information in the key but use the resulting looked-up value (unlike how I was doing before where it was effectively a yes/no lookup).

That value contains information such as the port range, or the port, for Range and Any respectively.

This does mean we now do three successive look-ups with differing amounts of information to see if specific rules apply.
Im not entirely happy with this solution as it feels messy, so am looking forward to seeing @Beshelmek 's pull request to see if they've done it cleaner.

Anyway there is still work to be done, such as writing more tests and cleaning things up. But the current test suite pass and I'd appreciate if someone gave it a shot.

[paulb-opusvl]

I tried to think up some use cases where this would be a problem and couldn't really. Most requirements would be single port. I also struggled to see what protocols beyond tcp/udp would be required. Maybe outside the user/site setup, but can't see it being a big deal.

Good to see that edge cases would be handled if you expand your code though. I often find myself writing a bit of code that is open to cover use cases that don't exist - yet, but would work if required.

[NHAS]

Hiya!

Honestly, it'd be mostly easy for everything other than icmp.

I've been thinking about this for quite a while whether I should implement this or not.

It does add a bunch of additional complexity and expands the xdp code base which is something I'm always a little wary about doing.

So currently a Web administrator portal is in the works which is taking a substantial amount of effort for me as I'm not a Web dev.

But after that I'll look into the viability of this.

[bluecraank]

Please add this! Would be awesome to define ports!

[bluecraank]

btw: i can really help you to develop a web portal for this

[NHAS]

@bluecraank Thanks for your offer of help! I'm basically done with what I wanted to get done for it, so I'd be more than happy for you to look over it when I've released and give advice/feedback or pull requests to make it better.

Im definitely not a web dev is what this experience has taught me :D

[Beshelmek]

@NHAS I made port restriction support. Do you have guidelines for code contribution? My implementation need some refactoring and i want to do it right.

[NHAS]

@Beshelmek hi there!

That's awesome to hear, in order to get things moving on this could you make sure you've written tests for your contribution in the router package (have a look at the other _test.go files for example)

And I'll probably ask for you to make your contribution to the unstable branch as a lot of work has taken place there. (and may mean that your current work doesn't fit with the state of the project, sorry if this is the case)

By all means open a pull request and we can discuss it there

[bluecraank]

@NHAS I made port restriction support. Do you have guidelines for code contribution? My implementation need some refactoring and i want to do it right.

Hey,
chance to get the code and test it?

[NHAS]

Just another note on this, I will be a tad picky on how this is implemented. I've been quite back and forth on whether or not I even want this to be a feature in wag as it starts to stray away from being a pure VPN + MFA solution and into the realms of becoming firewall, which Im not as keen to implement.

How I currently run our deployment of this is that wag does ingress and MFA for our sensitive paths but it passes through a firewall (fortigate) which does the actual port control.

I know that might feel quite clunky as you're managing things in two different places, but Im very aware of extending wag past what I believe the scope of the project is.
Which is to add MFA to wireguard.

[Beshelmek]

@NHAS I made port restriction support. Do you have guidelines for code contribution? My implementation need some refactoring and i want to do it right.

Hey, chance to get the code and test it?

I will refactor the code this weekend and then create a pull request

Just another note on this, I will be a tad picky on how this is implemented. I've been quite back and forth on whether or not I even want this to be a feature in wag as it starts to stray away from being a pure VPN + MFA solution and into the realms of becoming firewall, which Im not as keen to implement.

How I currently run our deployment of this is that wag does ingress and MFA for our sensitive paths but it passes through a firewall (fortigate) which does the actual port control.

I know that might feel quite clunky as you're managing things in two different places, but Im very aware of extending wag past what I believe the scope of the project is. Which is to add MFA to wireguard.

Of course I understand the concept and that it was created to enterprice solutions. But the concept of multiple devices for a single user makes it a little more difficult to manage these things through firewalls. Given that you are already using XDP, which in most cases is used to create custom firewalls, this feature is quite suitable. It was born to solve the problems of remote access for employees and separating them by department :)

[NHAS]

Hm not entirely certain what you mean in terms of multiple devices per user making it difficult for a firewall to manage the access.

Wag manages what hosts the user can access and a firewall manages what services on the hosts the user can access. If that makes sense.

To give a bit of an example. In most environments your vpn host machine is separated from other hosts via a firewall application whether that is a physical device or a router of some description.

In that setup, to make changes you have to edit your firewall to allow the vpn host to access the network services on your other hosts anyway. Hence you'd have to configure it both in wag and the firewall, which may become annoying.

Mainly. I'm trying to reduce the complexity of the solution.

I do take your point that allowing wag to be more granular in defining specific services that users can access is a good point.
Both in terms of flexibility of configuration and in terms of defense in depth, where you'd have to have two configuration mistakes rather than just one in order to expose something unintentionally.

Super interested to see your pull request and have a bit more of a brain about it!

[paulb-opusvl]

I'm mixed about this. Up until this week I would have said that I too, manage access to those resources through another firewall anyhow, but this week had to allow a user access to a system that had multiple services on the same IP address. This meant allowing them in, but then having to get creative to allow onto ldap, but not https to that system.

So it could be useful. But how complex do you make it? A simple

"10.0.0.1:53,389,443"

or

"10.0.0.1": {
  ["53", "udp"],
  ["389", "tcp"]
  ...
}

I like simple.

[NHAS]

Ah right I see what you mean. So you had a firewall entry that allowed https/ldap/etc to the host for other users but when you added an entry for this user you only wanted it to touch ldap which wag effectively cant selectively do.

Hrmm. I dont have that requirement myself but it does make sense.

I was hoping that I could look at the work done by @Beshelmek before implementing it myself.

[Beshelmek]

I'm mixed about this. Up until this week I would have said that I too, manage access to those resources through another firewall anyhow, but this week had to allow a user access to a system that had multiple services on the same IP address. This meant allowing them in, but then having to get creative to allow onto ldap, but not https to that system.

So it could be useful. But how complex do you make it? A simple

"10.0.0.1:53,389,443"

or

"10.0.0.1": {
  ["53", "udp"],
  ["389", "tcp"]
  ...
}

I like simple.

My implementation only supports TCP protocol. I'll see if I can do it for UDP as well. Unfortunately, due to work, I'm a bit late with improving the code, I'll definitely create pull request within a one week.

[NHAS]

It'll need to be for TCP, UDP and ICMP,

This is a fairly good example of how you might go about doing that:

https://android.googlesource.com/kernel/common/+/f88eb7c0d002/samples/bpf/xdp_tx_iptunnel_kern.c

I think since folk seem to want this I'll probably go and build it.

[bluecraank]

We solved it by routing wireguard traffic, instead of nat it. So we can create rule by wireguard peer ip address. But please still add this feature, this would be way more simple than now.

[NHAS]

Hi All,
Just getting things moving and have done a lot of the eBPF leg work for this and am just wiring it up. Im planning on the following syntax:

"ip/mask [port or port range/service]..."

Where [] indicates optional. When the port and service is not defined this will default to creating an "any" rule to keep compatibility with the current scheme.

An example:

            "*": {
                "Mfa": [
                    "1.1.1.1 43/tcp 550/udp 443/any"
                ],
                "Allow": [
                    "192.168.0.0/16 100-10024/tcp 22/any"
                ]
            },

Im not entirely happy with this as I believe it'll get pretty hard to scan by eye when complex rules are in place. However as this is pretty easy to change in the future Im not too worried about it.

This is, however, your time to chime in. If you've got a better syntax for me by all means, speak up

[paulb-opusvl]

Will there be an "any/any" and a default of block or forward?

Personally, I'd have gone with

            "*": {
                "Mfa": [
                    "1.1.1.1": { 
                          ["43/tcp", "550/udp", "443/any"]
                      }
                ],
                "Allow": [
                    "192.168.0.0/16": { 
                       ["100-10024/tcp", "22/any"]
                    }
                ]
            },

Just because I would have thought less programming to parse it, but from a user's perspective, it does not affect me.

[NHAS]

Just giving it a little try out myself shows its a little broken but it passes the test suite!

Good to know, but does mean unstable is a little broken right now

[NHAS]

Unbroken. So yeah, this is fully ready to go now. Just a few more tests should be written in order to make sure this is 100%.

Will also need to clean up the code as currently it wont respect my mental model for how MFA routes should take preference over public routes.

[NHAS]

Eh my solution is a bit poor as it only allows one range and one any rule per IP, due to how the LPM tries work.

You can make as many single port rules as you like though! Going to have a rethink about my approach.

[NHAS]

Its not necessarily a problem per say, but its is very counterintuitive and doesnt behave as I expect most firewall rules would.
So really Im just trying to bring it in line with expected firewall behavior .

In my humble opinion the best and simplest solution would be to only allow port/protocol rather than ranges and any as it decreases complexity and makes an overall more robust solution.

I'll write out my new solution, see how it performs, see how I feel about the code then I might throw it away in favor of my simple option.

[NHAS]

Sweet, I've got a solution that fixes the issue of only being able to define one any/range rule.

How I've done this is by putting a fixed length array as the value of the route trie structures. Its not particularly memory efficient but frankly its very simple to maintain.

This means you can define up to 128 policies per route. These can be a mix of any, range, or single rules.

This now exists on the unstable branch :)