Import: networks.conf is not updated with new IP addresses (dns, dhcp and gateway)

[nqb]

Describe the bug
When you import an export coming from a PacketFence server with content in networks.conf, the file is copied as-is without any changes.

It means that if your PacketFence IP address has changed, registration and isolation networks are not correctly configured. Next hops need to be changed too.

To Reproduce
Steps to reproduce the behavior:

1. Do a PacketFence export on a server with content in networks.conf
2. Import your export on a server with a different network configuration
3. Restart services

=> Your networks.conf contained IP addresses of your previous setup

Expected behavior
Old IP addresses in networks.conf should be replaced by new IP addresses.

Additionnal context
As a short term fix, we can warn users about this at end of import.

EDIT: and update current limitations of mechanism.

[julsemaan]

The problem with that is that if the network (and not only the IP changes), then the netmask, dhcp start/end and many other parameters change so IMO we should warn the user since I don't think we can get the right recipe in place to migrate it for all the cases

If the network doesn't change (same IP+netmask), we could rewrite it though but that leads to inconsistent behavior in the script which we'll have to see if its acceptable or not

[nqb]

The problem with that is that if the network (and not only the IP changes), then the netmask, dhcp start/end and many other parameters change so IMO we should warn the user since I don't think we can get the right recipe in place to migrate it for all the cases

100% agree.

[julsemaan]

I pushed a commit that attempts to migrate the networks configuration if its in the same subnet

@nqb, let me know if it looks good to you

[nqb]

Default ipcalc binary on Debian doesn't have --class-prefix option.

This option is provided by ipcalc-ng package on Debian 11 which seems to be almost similar to ipcalc binary provided on EL but versions and some options are different:

Versions:

root@pfdeb11dev:/home/vagrant# ipcalc-ng --version                                                                    │[vagrant@pfel8dev ~]$ ipcalc --version
ipcalc-ng 1.0.0                                                                                                       │ipcalc 0.2.4
root@pfdeb11dev:/home/vagrant#                                                                                        │[vagrant@pfel8dev ~]$

Options:

root@pfdeb11dev:/home/vagrant# ipcalc-ng --help                                                                       │[vagrant@pfel8dev ~]$ ipcalc -h
Usage: ipcalc-ng [OPTION...]                                                                                          │ipcalc: ip address expected
  -c, --check                     Validate IP address                                                                 │Usage: ipcalc [OPTION...]
  -r, --random-private=PREFIX     Generate a random private IP network using                                          │  -c, --check                     Validate IP address
                                  the supplied prefix or mask.                                                        │  -r, --random-private=PREFIX     Generate a random private IP network using
  -S, --split=PREFIX              Split the provided network using the                                                │  -S, --split=PREFIX              Split the provided network using the
                                  provided prefix/netmask                                                             │                                  provided prefix/netmask
  -d, --deagrregate=IP1-IP2       Deaggregate the provided address range                                              │  -i, --info                      Print information on the provided IP address
  -i, --info                      Print information on the provided IP address                                        │                                  (default)
                                  (default)                                                                           │      --all-info                  Print verbose information on the provided IP
      --all-info                  Print verbose information on the provided IP                                        │                                  address
                                  address                                                                             │      --reverse-dns               Print network in a the reverse DNS format
                                                                                                                      │  -4, --ipv4                      Explicitly specify the IPv4 address family
Specific info options:                                                                                                │  -6, --ipv6                      Explicitly specify the IPv6 address family
      --reverse-dns               Print network in a the reverse DNS format                                           │
  -a, --address                   Display IP address                                                                  │Specific info options:
  -b, --broadcast                 Display calculated broadcast address                                                │  -b, --broadcast                 Display calculated broadcast address
  -m, --netmask                   Display netmask for IP                                                              │  -m, --netmask                   Display netmask for IP
  -n, --network                   Display network address                                                             │  -n, --network                   Display network address
  -p, --prefix                    Display network prefix                                                              │  -p, --prefix                    Display network prefix
      --minaddr                   Display the minimum address in the network                                          │      --minaddr                   Display the minimum address in the network
      --maxaddr                   Display the maximum address in the network                                          │      --maxaddr                   Display the maximum address in the network
      --addresses                 Display the maximum number of addresses in                                          │      --addresses                 Display the maximum number of addresses in
                                  the network                                                                         │                                  the network
      --addrspace                 Display the address space the network                                               │      --addrspace                 Display the address space the network
                                  resides on                                                                          │                                  resides on
  -h, --hostname                  Show hostname determined via DNS                                                    │  -h, --hostname                  Show hostname determined via DNS
  -o, --lookup-host=STRING        Show IP as determined via DNS                                                       │  -o, --lookup-host=STRING        Show IP as determined via DNS
  -g, --geoinfo                   Show Geographic information about the                                               │  -g, --geoinfo                   Show Geographic information about the
                                  provided IP                                                                         │                                  provided IP
                                                                                                                      │
Other options:                                                                                                        │Other options:
  -4, --ipv4                      Explicitly specify the IPv4 address family                                          │      --class-prefix              When specified the default prefix will be determined
  -6, --ipv6                      Explicitly specify the IPv6 address family                                          │                                  by the IPv4 address class
      --class-prefix              When specified the default prefix will be determined                                │      --no-decorate               Print only the requested information
                                  by the IPv4 address class                                                           │  -s, --silent                    Don't ever display error messages
      --no-decorate               Print only the requested information                                                │  -v, --version                   Display program version
  -j, --json                      JSON output                                                                         │  -?, --help                      Show this help message
  -s, --silent                    Don't ever display error messages                                                   │      --usage                     Display brief usage message
  -v, --version                   Display program version                                                             │[vagrant@pfel8dev ~]$
  -?, --help                      Show this help message                                                              │
      --usage                     Display brief usage message                                                         │

IMHO, we should require ipcalc-ng on Debian 11 and use it in place of ipcalc but we should validate that all ipcalc return same results on both distributions.
If you agree, I will do it.

[julsemaan]

If there is a more EL8 compatible ipcalc on Debian 11, I'd say we use it so that we have less if else conditions in our upgrade but I'm open to both approaches

[nqb]

My proposal is to use ipcalc-ng on Debian 11 and keep ipcalc on EL8. Binary on Debian is located on /usr/bin/ipcalc-ng, EL8: /usr/bin/ipcalc

Regarding ed3db78, get_os_netmask is broken :

root@pfdeb11dev:/home/vagrant# ipcalc-ng 192.168.121.204                                                              │[vagrant@pfel8dev ~]$ ipcalc 192.168.121.204
Address:        192.168.121.204                                                                                       │Address:        192.168.121.204
Address space:  Private Use                                                                                           │Address space:  Private Use
root@pfdeb11dev:/home/vagrant#                                                                                        │Address class:  Class C
                                                                                                                      │[vagrant@pfel8dev ~]$

=> You expect a "Network" in output.

[julsemaan]

The output of ipcalc-ng is not complete though as we need the network identifier of the IP address (ex: 192.168.1.1/24 => 192.168.1.0)

I'll dig into that and see if I can find a Debian equivalent

[nqb]

@julsemaan, look at my last comment, there is the output of ipcalc on EL8 (on the right) which is identical.

[julsemaan]

@julsemaan, look at my last comment, there is the output of ipcalc on EL8 (on the right) which is identical.

You're missing a parameter in the command you made which is --class-prefix, see below for the exact call:
https://github.com/inverse-inc/packetfence/blob/devel/addons/functions/configuration.functions#L46-L48

[julsemaan]

I pushed a fix in get_ip_network which is where that was broken, get_os_netmask works fine on Debian and EL based on my tests but I'll give it a try when the package is built to confirm

[nqb]

Ok @julsemaan. For me get_os_netmask should not work on EL8 or on Debian 11 (with ipcalc-ng only) because you expected a "Network" in ipcalc's output which is not here (see #6636 (comment))

packetfence/addons/functions/configuration.functions

Lines 37 to 40 in 4560ace

	function get_os_netmask() {
	os_ip=`get_os_ip_address $1`
	ipcalc $os_ip | grep "^Netmask:" | egrep -o '([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)'
	}

[julsemaan]

We're not using ipcalc-ng, we're using ipcalc, not sure why your instance has ipcalc-ng

Here is what I have on Debian 11 with a base unaltered install:

root@test-pf:~# ipcalc 192.168.1.1/24
Address:   192.168.1.1          11000000.10101000.00000001. 00000001
Netmask:   255.255.255.0 = 24   11111111.11111111.11111111. 00000000
Wildcard:  0.0.0.255            00000000.00000000.00000000. 11111111
=>
Network:   192.168.1.0/24       11000000.10101000.00000001. 00000000
HostMin:   192.168.1.1          11000000.10101000.00000001. 00000001
HostMax:   192.168.1.254        11000000.10101000.00000001. 11111110
Broadcast: 192.168.1.255        11000000.10101000.00000001. 11111111
Hosts/Net: 254                   Class C, Private Internet

[julsemaan]

It should be fine now, @nqb, please check again and I think we can close this after

[nqb]

@julsemaan, still an issue on my side with get_os_netmask function which call ipcalc-ng on IP address in place of CIDR:

++ get_os_netmask eth0
+++ get_os_ip_address eth0
+++ ip -br -o a show dev eth0
+++ head -1
+++ egrep -o '[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+'
++ os_ip=192.168.121.213
++ ipcalc_wrapper 192.168.121.213
++ is_deb_based
++ which apt
++ egrep -o '([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)'
++ ipcalc-ng 192.168.121.213
++ grep '^Netmask:'
+ os_netmask=

EDIT: issue should be also on EL8

[julsemaan]

I pushed another fix, I think we should be good to go now 🤞

[nqb]

@julsemaan, stil a problem with the rewrite due to relative path to networks.conf file:

packetfence/addons/functions/configuration.functions

Lines 91 to 93 in 3430fac

	echo "Rewriting $old_ip to $new_ip in networks.conf"
	old_ip_escaped=`echo "$old_ip" | sed 's/\./\\./g'`
	sed -i 's/'$old_ip_escaped'/'$new_ip'/g' conf/networks.conf

[julsemaan]

should hopefully be fine now

[nqb]

I'm sorry but I found two issues:

• Output of import wrote: Rewriting 192.168.121.162 to 192.168.121.213 in networks.conf but I don't have this IP address in networks.conf
• The way we modified networks.conf need to be adjusted because I got following behavior:

---------------------------------------------------------------------------------
Found interface eth3 in pf.conf
---------------------------------------------------------------------------------
IP address on interface eth3 differs from the one in pf.conf. If you wish to use 172.17.3.2 as the IP address on this server, you need to change the IP before running this script or after. Do you want to adjust pf.conf to use 172.17.3.252 on eth3? (y/n): y
Rewritting IP address for eth3 to 172.17.3.252 with netmask 255.255.255.0 in pf.conf
Changed IP settings of eth3 to 172.17.3.252/255.255.255.0 in pf.conf
Rewriting gateway and DNS server of 172.17.3.0 to point to 172.17.3.252
Rewriting 172.17.3.2 to 172.17.3.252 in networks.conf

Resulting file:

[172.17.3.0]
pool_backend=memory
dhcp_end=172.17.3.25246    <<< was 172.17.3.246
fake_mac_enabled=disabled
nat_dns=disabled
netflow_accounting_enabled=disabled
dns=172.17.3.25252            <<< was 172.17.3.2
dhcpd=enabled
dhcp_default_lease_time=30
domain-name=vlan-isolation.example.lan
netmask=255.255.255.0
type=vlan-isolation
tenant_id=1
coa=disabled
gateway=172.17.3.25252    <<< was 172.17.3.2
named=enabled
dhcp_max_lease_time=30
dhcp_start=172.17.3.10
split_network=disabled
nat_enabled=disabled
id=172.17.3.0

[julsemaan]

I think your first issue is related to the fact you may be importing from a cluster which means the IPs in networks.conf may not match what was in pf.conf. Let me know if I'm on the right track. If that's the case I'm not sure we can do much at this stage (that's why I didn't really like the idea to rewrite networks.conf in the first place since its pretty complex)

I'll fix the bad replacement by making the regex stricter

[nqb]

I think your first issue is related to the fact you may be importing from a cluster which means the IPs in networks.conf may not match what was in pf.conf. Let me know if I'm on the right track

No, I'm testing an import from a standalone with identical number of interfaces.

[nqb]

I wonder if the rewrite is really the good way to solve that problem.

I don't think we handle here the case of routed networks so it means that some users will have to modify anyway their networks.conf by hand.

[julsemaan]

No, I'm testing an import from a standalone with identical number of interfaces.

The old IP comes from pf.conf and the new from the OS settings, can you detail what is wrong about the line Rewriting 192.168.121.162 to 192.168.121.213 in networks.conf . Its possible that there is nothing to rewrite in some cases but its still going to attempt to do it

I wonder if the rewrite is really the good way to solve that problem.

I don't think we handle here the case of routed networks so it means that some users will have to modify anyway their networks.conf by hand.

I tend to agree with that, this code attempts to be a best effort but I don't think it can cover all the cases

[nqb]

The old IP comes from pf.conf and the new from the OS settings, can you detail what is wrong about the line Rewriting 192.168.121.162 to 192.168.121.213 in networks.conf . Its possible that there is nothing to rewrite in some cases but its still going to attempt to do it

The IP address 192.168.121.162 was only present in pf.conf on setup where I did the export. It was a dhcp-listener interface. So import process should not try to write something related to that interface in networks.conf.

I tend to agree with that, this code attempts to be a best effort but I don't think it can cover all the cases

How do you want to proceed ?

[julsemaan]

I think that having this is better than having nothing (which was the case in 11.0) so as long as we're good to live with the limitations it has, its better than what we had.

We can polish it to ignore network rewrites of interfaces that don't have the type 'internal' in the post-release

[nqb]

Looks good to me, let keep as-is.
I'm moving milestone to see how we can handle:

• routed networks
• interfaces without 'internal'

[nqb]

@julsemaan, could you backport all your fixes to maintenance/11.0 ?

At the moment, import is broken on maintenance/11.0 on Debian 11 (ipcalc-ng is not installed).

[nqb]

Here are the commits to backport:

• f53eca1 (only debian/control)
• 1c423d2
• 626c314
• 7985193

[julsemaan]

Before I do that I want to make sure:

• Do we backport the fixes?
• Do we only add a warning in maintenance/11.0 and leave the fix to 11.1+

Both are fine for me, the backport has a bit more risk since we don't have good test coverage of these tools yet

[nqb]

I was certainly unclear (again) in my first comment this morning but currently some fixes for this issue have been backported but some not.

So I suggest we backport everything.

[julsemaan]

[julsemaan]

I'm building maintenance/11.0-6636 with (hopefully) all the fixes

We can then try the build and merge that branch into maintenance/11.0 for more confidence in the fixes

[julsemaan]

maintenance/11.0-6636 is on par with devel as far as how this behaves

[nqb]

@julsemaan, can we backport remaining things to maintenance/11.0 ?

[julsemaan]

I never actually tried maintenance/11.0-6636 (only tried the devel fixes from what I recall), would you have time to give it a try to make sure it works as expected and we can then merge

Or I can put this in my pipeline and test it at some point, not exactly sure when though

[nqb]

I will test fix on maintenance/11.0-6636 when I have time.

[nqb]

@julsemaan, it looks like maintenance/11.0-6636 has no difference regarding import with maintenance/11.0.

Fix: 9220701 has been already backported to maintenance/11.0.

I would expect to see almost all commits listed here

[julsemaan]

I just checked and the changes are in maintenance/11.0 so it must have been done at some point but we forgot to update this issue.

@nqb, I'll let you confirm what I found and we can then close

[nqb]

I'm closing this issue because I tested an import this morning and networks.conf has been migrated as I expect.

However, when you have routed networks, there is some adjustments to perform. I will open another issue for that.