Change rules for 'Disallow assigning bcast/networks to interfaces'

[PieterL75]

NetBox version

v3.5.2

Feature type

Change to existing functionality

Proposed functionality

#9068 introduced a new limitation to adding IPs to device interfaces. The parameters chosen for this FR are limiting how IPs are used.

Use case

IPv6 does not have broadcast or network addresses. All is handled with linklocal addresses and multicast.. You can used to :: or :ffff ip's without problems. We tend to use the lowest address (aaaa:bbbb:1234:5678::) as gateway.

For IPv4, there are use cases for the network and broadcast address. When I route an entire subnet to a firewall, we can use all ips for NAT translations. Those IPs are added to the device on a virtual interface. I tend to flag that prefix as a 'pool' (All IP addresses within this prefix are considered usable). Can that 'pool' be taken into account in the validation ?

Database changes

No response

External dependencies

No response

[decoupca]

For IPv6, does it therefore make sense to remove any limits on interface assignment?

For v4, In general I agree that limiting choice is a bad thing, because there's always an edge case I haven't thought of. But I also like to weigh that against the helpfulness of preventing common problems with sanity checking.

Two suggestions come to mind:

1. Change the ValidationError into a warning. Not sure if there is already GUI components in place to do this, but warning the user that what they're doing is typically not valid seems like a good balance.
2. Leave the ValidationError in place but introduce a config flag that disables the validation check

Thoughts?

[PieterL75]

The GUI prevented from creating broadcast/network addresses in the prefix view, unless the 'Pool' box is checked.

The only way to create a broadcast/network address around that GUI, is using the API, or by manually changing the IP at time of creation.

My view is IPv6

• remove the validation

IPv4

• allow all ips when the prefix is a 'pool'
• in the GUI, when an IP address is entered, do a clientside check if it is a network/broadcast and show a warning there, even before the save button is pressed
• in the API, return a warning that a broadcast/network address is used.

but i would not prevent the IPs from being created...

[decoupca]

Agreed on IPv6, validation is not appropriate and I support removing it entirely.

As for your IPv4 suggestions, I'll defer to maintainers for guidance. IMHO I think this needs some level of user customizability, as I can imagine use cases where disabling this kind of validation could cause as many problems as leaving it enabled.

In any case, the validation only happens when assigning the net ID/bcast to an interface. You can create those objects directly with no errors.

[matejv]

In our network we use subnets /31 for IPv4 and /127 for IPv6 on all point to point links between routers. With this change we can no longer assign IP addresses to interfaces which is a major headache.

At least for /31 and /127 subnets there should be no validation of network and broadcast addresses, as both addresses in subnet are considered host addresses.

[decoupca]

@matejv there is no such validation on /31 and /127, they were excluded for exactly this reason. Are you sure they don't work?

[matejv]

I looked at the code and there are exceptions when validating network (first) address but not broadcast (last) address. And my colleague did show me the broadcast error message.

When assigning /31 (or /127) IPs for point to point links, one address is assigned to one router and the other to the other router. So I think the validation exclusion should apply to broadcast as well.

[decoupca]

The validator won't fail on either end of a PtP, since in such cases network.broadcast will evaluate to None. The code is a bit confusing, granted, but that's how netaddr.IPNetwork objects behave.

If you have a confirmed failure of assigning a /31 address, open a bug report with full details on how to reproduce.

[matejv]

Apologies for causing confusion before regarding validation of PtP subnets. It seems only IPv6 is affected and not IPv4.

If you try to assign the last IPv6 address with /127 mask to an interface you get a validation error that this is a broadcast address. The same error happens with IPv6 with /128 mask.

I've tested all possible combinations with /31, /32, /127 and /128 masks and only above two cases are problematic.

Steps to Reproduce:

1. Navigate to device interface
2. Click "Add IP address"
3. On "Add a new IP address" form set Address: 2001:db8::1/127 (or 2001:db8::1/128) and click Create

An error is shown:

2001:db8::1/127 is a broadcast address, which may not be assigned to an interface.

The same error will be shown if you try to assign 2001:db8::1/128

Expected Behavior:

A new IP Address object for 2001:db8::1/127 (or 2001:db8::1/128) should be created and assigned to interface. As /127 and /128 are considered subnet sizes that should be excluded from network/broadcast validation.

[MichaelSasser]

It would be great to remove those limitations, at least for IPv6. I encountered the same error message @matejv mentioned while trying to create IPv6-Addresses for Tailscale (Headscale) interfaces like fd7a:115c:a1e0::31/128 (the default prefix is fd7a:115c:a1e0::/48).

[decoupca]

@matejv Thanks for the info. Opening a separate issue to address IPv6 validation.

This issue should now focus solely on how to handle IPv4 validation

I see two ways forward:

1. Add a configuration option to disable validation
2. Change validation to raise a warning instead, ideally in TypeScript (before the form is submitted and any potential webhooks are fired)

Awaiting maintainer input.

[candlerb]

IPv6 does not have broadcast or network addresses. All is handled with linklocal addresses and multicast.. You can used to :: or :ffff ip's without problems.

This is not actually true.

With the exception of /127 point-to-point links, the all-zeros address is reserved in IPv6 as the "Subnet-Router Anycast address": see https://datatracker.ietf.org/doc/html/rfc4291#section-2.6.1 - it cannot be assigned to an individual host.

And address which end with all-ones and 80 to FF for the last byte are also reserved anycast addresses: https://www.rfc-editor.org/rfc/rfc2526.txt

[decoupca]

@candlerb Interesting. So the validation should not allow ::0 addresses to be assigned to interfaces unless the prefix length is 127? Would there ever be an appropriate case for assigning this address to an interface, even to a router?

And address which end with all-ones and 80 to FF for the last byte are also reserved anycast addresses: https://www.rfc-editor.org/rfc/rfc2526.txt

If I'm reading this right, anycast addresses can be assigned to interfaces (in fact, multiple interfaces on different devices), and therefore should not be excluded by this type of validation, or have I missed something?

[candlerb]

A machine can receive packets on the anycast address: for example, a packet to the "Subnet-Router Anycast address" should be received by all routers on that link. But it can't send packets with that as a source address, and therefore cannot have that address assigned as one of its addresses.

If you wanted to model that in Netbox , I think the closest would be to assign the address to an FHRP group, and then link that FHRP group to multiple devices/interfaces.

[andretlemos]

I had the same problem and decide to dowgrade to version 3.5.1

[DanSheps]

If you wanted to model that in Netbox , I think the closest would be to assign the address to an FHRP group, and then link that FHRP group to multiple devices/interfaces.

I think it would be perfectly valid to also allow you to assign directly to an interface as long as the role is "Anycast"

[dotie]

For anyone else that uses /127s on point to point links, it's possible to work around the broadcast error in v3.5.3 by initially assigning the IPv6 address to an interface with a larger prefix length (e.g. 2001:db8::1/64). Then use the bulk edit option (IPAM - Prefixes - [prefix] - IP Addresses) to change the mask length to 127.

[PieterL75]

I don't see the exception for a 'pool'. If a prefix is a POOL, then ALL ip's should be allowed to be assigned to an interface

[candlerb]

In that case, the individual IP assignments will be /32 or /128 loopbacks?

[PieterL75]

We use pools to document an entire public IP range that is routed to a firewall. There all IPS can be used, as they are not attached in a layer2. So that can be any range (/24 to /32)

[ITJamie]

I think we're going to need a separate issue opened (or reopen this issue) for pool related logic.

There is no relationship between ip object > prefix object in the current models, It would have to be assumed based on VRF, site, etc.

The IPAddress class could do with a get_parent_prefix function, similar to the get_child_ips function on the Prefix class.

I believe it has come up before that the "assumed" connection between prefixes + IP addresses can cause issues. for example, if we were looking up the parent prefix of an IP, do we filter for prefixes with/without a specific status? (container, active, reserved, deprecated),

because there is no current logic to reverse lookup the prefix of an IP in the current models, and because there are some potential gotchas related to this, I would recommend a new issue to discuss the problem in-depth

[ITJamie]

here is a quick hack I put together. it checks for a prefix with the supplied ip's network+prefix length. if it finds it it will check to see if is_pool is true and skip the network/broadcast IP checks.

        # Do not allow assigning a network ID or broadcast address to an interface.
        if interface and (address := self.cleaned_data.get('address')):
            prefix_str = f"{self.instance.address.network}/{self.instance.address.prefixlen}"
            allow_assignment_error = True
            if self.instance.vrf is None:
                prefix_obj =  Prefix.objects.get(prefix=prefix_str)
            else:
                prefix_obj =  Prefix.objects.get(prefix=prefix_str, vrf=self.vrf)
            if prefix_obj and prefix_obj.is_pool:
                    allow_assignment_error = False

            if allow_assignment_error:
                if address.ip == address.network:
                    msg = f"{address} is a network ID, which may not be assigned to an interface unless the prefix is a pool."
                    if address.version == 4 and address.prefixlen not in (31, 32):
                        raise ValidationError(msg)
                    if address.version == 6 and address.prefixlen not in (127, 128):
                        raise ValidationError(msg)
                if address.version == 4 and address.ip == address.broadcast and address.prefixlen not in (31, 32):
                    msg = f"{address} is a broadcast address, which may not be assigned to an interface unless the prefix is a pool."
                    raise ValidationError(msg)

https://github.com/netbox-community/netbox/compare/develop...ITJamie:netbox:ipaddress_prefix_pool_fixes

I'm not sure if that's the right approach and would like maintainer feedback

[andretlemos]

Why use validation error for /31 and /127? My ISP we're using this range to allocation point-to-point interfaces, we don't have much IPv4 address left and to habe the same policy we use /127 in IPv6. In my opinion this is'nt necessary.

[DanSheps]

Why use validation error for /31 and /127? My ISP we're using this range to allocation point-to-point interfaces, we don't have much IPv4 address left and to habe the same policy we use /127 in IPv6. In my opinion this is'nt necessary.

That is a "not" check for 31 and 127. So it will only trigger if they aren't point-to-point or loopback.

If you are asking why use a validation error? That is what triggers the error toast.