Firewall Implementation: NAT

[Hazingoo]

Implements #188

This PR adds NAT (Network Address Translation) support to the LionsOS firewall, enabling internal network devices to share a single public IP address when communicating with external networks.

Design

The NAT implementation is split across two layers:

NAT Module (nat_module.h / nat_module.c)
A translation engine used by both the RX and TX virtualisers. Each virtualiser holds two module instances, one for TCP, one for UDP. The module handles:

• SNAT (outbound): rewrites the internal source IP:port to the public SNAT IP and an assigned ephemeral port
• DNAT (inbound/returning): uses the ephemeral port as a direct table index to reverse the translation back to the original internal IP:port
• Checksum recalculation for both the IP header and TCP/UDP transport header after any modification

Port Table
A fixed pre allocated array shared between the RX and TX virtualisers for the same interface and protocol. Each slot stores the original internal IP and port, a validity flag, a last used timestamp, and a linked list pointer for the free pool. The port number itself is used as the array index (port - base_port), giving O(1) lookup with no hashing.

Webserver State
A shared memory region mapped into the RX virtualiser, TX virtualiser, and webserver simultaneously. The webserver writes the SNAT IP (configured via the web UI) and timeout value; the NAT modules read them directly. A magic number prevents double initialisation when multiple virtualisers start up.

Features to be added later

• Timer integration: now is currently hardcoded to 0
• Periodic cleanup: nat_module_cleanup_expired() exists but is never called, needs to be implemented.
• Persistent SNAT IP config: the configured SNAT IP lives in runtime memory only and is lost on reboot
• Per protocol timeouts: TCP and UDP currently share the same 5-second default timeout
• Fragmented packet handling: assumes the transport header is present in the first fragment

[Courtney3141]

I spent a couple of hours looking through, I have identified a number of things to work on.

1. I would start by reading through my comments on config.h, which motivate my other comments.
2. The main issue I have identified so far with the implementation is that you half-half whether you implement NAT on a protocol level or an interface level. My comments assume that we should implement on a protocol level, which I don't think is a bad idea. But a lot of your code assumes that NAT is either enabled or disabled on an interface - you need to fix that.
3. If NAT is enabled, the virtualisers are required to search through their port tables, which reside in shared memory. This can actually be expensive if it is done on a per-packet basis. Thus, I think we need to implement some optimisations for the case where NAT is disabled. I described how the enabled boolean should reside in the port table itself - this still stands, but you may want to introduce a local cache for each virtualiser of whether any protocol NAT is enabled. This allows the virtualiser to skip port table searching in the totally disabled case. The virt can then rely on a ppc from the webserver (or potentially other virt) to indicate whether the cache needs to be updated. (This cache could actually be a uint8_t indicating how many protocols it is enabled on). Feel free to discuss this more with me next week if you want.
4. I deleted some old code from ui_server.py that seemed to be lingering in your repository, however your endpoints may have depended on it. You will need to update them if so (this was old code assuming that there are only 2 interfaces, so you can't assume this any more.)

If you are confused about any of my comments so far, let me know and we can discuss online or in-person.

[Courtney3141]

We have now wrapped this error handling in a function, please do the same:

uint8_t interface_idx = mp_obj_get_int(interface_idx_in);
if (!check_interface_index(interface_idx)) {
    return mp_const_none;
}

[Courtney3141]

Max NAT what? what is a NAT?

[Courtney3141]

I see that this is the max number of port tables per interface (although this is not used consistently in the webserver config).

Since for each interface, there is one port table per protocol, I think you can use the value FW_MAX_FILTERS instead, which is the maximum number of (protocol) filters per interface. Then you can remove this new macro.

[Courtney3141]

This is not an "interface" config (i.e. a config that is allocated per interface). Each interface will have multiple of these. Thus I would rename this to fw_nat_port_table_config -> as it contains all the initial information needed to use a port mapping table.

[Courtney3141]

Since the IP address is fixed (source IP --> IP of the firewall on the transmission interface), this field should not be here (as it does not vary per port mapping). Since the virtualiser's do not have the interface IP address in their config by default, you can add it as a field in the fw_net_virt_[rt]x_config_t structs.

[Courtney3141]

Firstly, this name does not make sense since the type is used by both the rx and tx virtualisers. More importantly though, I don't think the protocol and enabled fields need to be in a separate struct - I think you should move them into the above struct.

[Courtney3141]

It might be cleaner to always use the read write permission region (shouldn't do any harm if you are not modifying the packets)

[Courtney3141]

NAT is performed per protocol, so you need to extract the protocol and check the corresponding table...

[Courtney3141]

This variable is always true, I don't think you need to define it, you can just pass true directly.

[Courtney3141]

This will be a for loop through your port tables, searching for the protocol number. If the protocol number is found, check the table to see if NAT is currently enabled.

[Courtney3141]

NOTE: We may want to have a special case, where if no NAT is enabled, we skip the search altogether (i.e. cache the value of enabled_0 || enabled_1 || ... || enabled_n)

[Courtney3141]

Perhaps we want to configure what we do if the protocol does not match with a port table entry

[Courtney3141]

Additionally, we may want to have a way to add a port table wildcard protocol - i.e., you receive a packet whose protocol does not have a table, it can be placed in the wildcard table.

[Courtney3141]

Is this really required? Doesn't seem like it.

[Courtney3141]

In other places in the code, we use enums for this, could you please match the surrounding style?

typedef enum {
    /* no error */
    ROUTING_ERR_OKAY = 0,
    /* data structure is full */
    ROUTING_ERR_FULL,
    /* duplicate entry exists */
    ROUTING_ERR_DUPLICATE,
    /* entry clashes with existing entry */
    ROUTING_ERR_CLASH,
    /* node ID does not point to a valid node */
    ROUTING_ERR_INVALID_ID,
    /* route is invalid */
    ROUTING_ERR_INVALID_ROUTE,
} fw_routing_err_t;

[Courtney3141]

Also, best to add descriptions, what is NAT_FAILURE?

[Courtney3141]

Is this the amount of time that passes before a port mapping becomes invalid? If so, 5 seconds seems a little short.

[Courtney3141]

If you have a .c file, you may as well define all your functions in the C file rather than some in the header...

[Courtney3141]

I don't understand why you are looking through all interfaces here, shouldn't we only look through the port mappings of the interface it was received on ?

[Courtney3141]

I really think these should be different functions to be honest. (Translation on the way in/out)

[Courtney3141]

ip_hdr->dst_ip != nat->interface_config->ip this should never happen - the firewall won't send out a packet for itself.

[Courtney3141]

I also feel like the nat enabled check should be done once at the beginning of the function (or even before calling it).

[Courtney3141]

actually, this always has to be done. The reason the check enabled exists is about whether the transport layer protocol has a checksum. UDP and TCP do, ICMP doesn't. But regardless of transport layer, IP will always need to be recalculated.

[Courtney3141]

Suggested change

	ip_hdr->check = fw_internet_checksum(ip_hdr, ihl_bytes);
	ip_hdr->check = fw_internet_checksum(ip_hdr, ipv4_header_length(ip_hdr))```

[Courtney3141]

These should really be arrays