[RFC PATCH network 06/16] sdn: ovn: add port security

["Rémi PAETA" <remi.paeta@comaite.com>]

Add a 'port-security' option to OVN zones:

- off (default): no filtering, like a Linux bridge
- mac: guests can only send with their own MAC
- mac-ip: guests can only send with their own MAC and IPAM addresses,
  and only receive traffic for them

The guest MAC is not passed to tap_plug, so the port addresses and port
security get set in add_bridge_fdb, which qemu-server calls on guest
start and pve-container on plug. The MAC is recorded on the port, so
that the next SDN reload applies a changed level to running guests
without restarting them.

With 'off' and 'mac', ports keep the 'unknown' address, so frames for
other MACs behind the guest (macvlan, VRRP, ...) still get delivered.

The IPs are looked up in the IPAM of the zone, by vnet: vnets without
subnets have no IPs, and the PVE IPAM does not even know their zone.
If the IPAM cannot be queried, for example because a NetBox server is
unreachable, a reload leaves the port untouched instead of lowering its
port security, while a starting guest only gets its MAC enforced, so
that it keeps its connectivity until the next reload.

Signed-off-by: Rémi PAETA <remi.paeta@comaite.com>
---
 src/PVE/Network/SDN/Ovn.pm             | 190 ++++++++++++++++++++++++-
 src/PVE/Network/SDN/Zones/OvnPlugin.pm |  45 ++++++
 src/test/run_test_ovn.pl               | 139 +++++++++++++++++-
 3 files changed, 365 insertions(+), 9 deletions(-)

diff --git a/src/PVE/Network/SDN/Ovn.pm b/src/PVE/Network/SDN/Ovn.pm
index a3fbe44..54d9c58 100644
--- a/src/PVE/Network/SDN/Ovn.pm
+++ b/src/PVE/Network/SDN/Ovn.pm
@@ -28,6 +28,7 @@ my $EXT_CONTROLLER = 'pve-sdn-controller';
 my $EXT_ZONE = 'pve-sdn-zone';
 my $EXT_VNET = 'pve-sdn-vnet';
 my $EXT_VMID = 'pve-sdn-vmid';
+my $EXT_MAC = 'pve-sdn-mac';
 
 # chassis settings written to the local Open_vSwitch table
 my @CHASSIS_KEYS = ('ovn-remote', 'ovn-encap-type', 'ovn-encap-ip', $EXT_CONTROLLER);
@@ -76,6 +77,7 @@ sub desired_state {
         next if !$controllerid || !$state->{$controllerid};
 
         $state->{$controllerid}->{switches}->{$vnetid} = {
+            'port-security' => $zone->{'port-security'} // 'off',
             external_ids => {
                 $EXT_CONTROLLER => $controllerid,
                 $EXT_ZONE => $zoneid,
@@ -87,14 +89,18 @@ sub desired_state {
     return $state;
 }
 
-=head3 sync_commands($controllerid, \%desired, \@switches, \@ports, \%vmlist, \%nics)
+=head3 sync_commands($controllerid, \%desired, \@switches, \@ports, \%vmlist, \%mac_ips, \%nics)
 
 Returns the list of C<ovn-nbctl> commands (each an array reference) that turn
 the current northbound content (C<\@switches> and C<\@ports>, as returned by
 C<list_rows>) into C<\%desired>, the entry of C<desired_state> for
 C<$controllerid>.
 
-Ports of guests that are not in C<\%vmlist> anymore get removed too.
+Ports of guests that are not in C<\%vmlist> anymore get removed too. Ports with a
+known guest MAC get their addresses and port security updated according to the
+port security level of their zone and to the IPs of C<\%mac_ips>, keyed by MAC.
+Ports whose MAC maps to C<undef>, as the IPAM could not be queried, are left
+untouched.
 
 If C<\%nics> is given, as returned by C<guest_nics>, ports of guest
 interfaces that are not on their vnet anymore get removed too, unless they are
@@ -104,7 +110,7 @@ guest config on hotplug.
 =cut
 
 sub sync_commands {
-    my ($controllerid, $desired, $switches, $ports, $vmlist, $nics) = @_;
+    my ($controllerid, $desired, $switches, $ports, $vmlist, $mac_ips, $nics) = @_;
 
     my $wanted = $desired->{switches} // {};
 
@@ -172,11 +178,36 @@ sub sync_commands {
                 next;
             }
         }
+
+        my $mac = $external_ids->{$EXT_MAC};
+        next if !defined($mac) || !defined($vnetid) || !$wanted->{$vnetid};
+        # keep the current addresses instead of lowering the port security
+        next if exists($mac_ips->{$mac}) && !defined($mac_ips->{$mac});
+
+        my ($addresses, $port_security) = port_addresses(
+            $wanted->{$vnetid}->{'port-security'}, $mac, $mac_ips->{$mac} // [],
+        );
+        if (
+            !same_set($addresses, $port->{addresses})
+            || !same_set($port_security, $port->{port_security})
+        ) {
+            push @$commands, port_addresses_commands($port->{name}, $addresses, $port_security)->@*;
+        }
     }
 
     return $commands;
 }
 
+# OVSDB returns a set with a single element as plain value
+sub same_set {
+    my ($wanted, $current) = @_;
+
+    $current = [] if !defined($current);
+    $current = [$current] if ref($current) ne 'ARRAY';
+
+    return join("\n", sort @$wanted) eq join("\n", sort @$current);
+}
+
 # optional booleans are an empty set when unset
 sub ovsdb_true {
     my ($value) = @_;
@@ -312,11 +343,19 @@ sub sync_northbound {
         eval {
             my $switches = list_rows($controller, 'Logical_Switch', 'name', 'external_ids');
             my $ports = list_rows(
-                $controller, 'Logical_Switch_Port', 'name', 'external_ids', 'up',
+                $controller,
+                'Logical_Switch_Port',
+                'name',
+                'external_ids',
+                'addresses',
+                'port_security',
+                'up',
             );
+            my $mac_ips = ipam_lookup($running_cfg, $ports);
 
-            my $commands =
-                sync_commands($id, $desired->{$id}, $switches, $ports, $vmlist, $guest_nics);
+            my $commands = sync_commands(
+                $id, $desired->{$id}, $switches, $ports, $vmlist, $mac_ips, $guest_nics,
+            );
             nbctl_transaction($controller, $commands);
         };
         push @errors, "OVN controller '$id': $@" if $@;
@@ -325,6 +364,51 @@ sub sync_northbound {
     die join('', @errors) if @errors;
 }
 
+=head3 guest_ips(\%running_cfg, $vnetid, $mac)
+
+Returns the IPs registered for C<$mac> in the IPAM of the zone of C<$vnetid>,
+if it has one. Vnets without subnets have no IPs, and the IPAM might not even
+know their zone. Returns C<undef> if the IPAM could not be queried.
+
+=cut
+
+sub guest_ips {
+    my ($running_cfg, $vnetid, $mac) = @_;
+
+    my $zoneid = $running_cfg->{vnets}->{ids}->{$vnetid}->{zone} // return [];
+    my $zone = $running_cfg->{zones}->{ids}->{$zoneid};
+    return [] if !$zone || !$zone->{ipam};
+
+    my $subnets = $running_cfg->{subnets}->{ids} // {};
+    return [] if !grep { ($_->{vnet} // '') eq $vnetid } values %$subnets;
+
+    my ($ip4, $ip6) =
+        eval { PVE::Network::SDN::Ipams::get_ips_from_mac($mac, $zoneid, $zone) };
+    if (my $err = $@) {
+        warn "unable to look up the IPs of $mac in the IPAM of zone '$zoneid' - $err";
+        return;
+    }
+
+    return [grep { defined($_) } $ip4, $ip6];
+}
+
+# returns the IPAM IPs of all managed ports with a known guest MAC, keyed by MAC,
+# undef if the IPAM could not be queried
+sub ipam_lookup {
+    my ($running_cfg, $ports) = @_;
+
+    my $mac_ips = {};
+    for my $port (@$ports) {
+        my $external_ids = $port->{external_ids} // {};
+        my ($mac, $vnetid) = $external_ids->@{ $EXT_MAC, $EXT_VNET };
+        next if !defined($mac) || !defined($vnetid);
+
+        $mac_ips->{$mac} = guest_ips($running_cfg, $vnetid, $mac) if !exists($mac_ips->{$mac});
+    }
+
+    return $mac_ips;
+}
+
 =head3 port_external_ids($controllerid, $zoneid, $vnetid, $iface)
 
 Returns the external_ids identifying the logical switch port of the guest
@@ -352,7 +436,8 @@ the logical switch C<$vnetid>. C<$current_switch> is the switch the port
 currently belongs to, if any.
 
 New ports get the C<unknown> address, so the logical switch behaves like a
-learning switch for them. Existing ports keep their addresses.
+learning switch for them until the guest MAC is known (see C<update_port>).
+Existing ports keep their addresses.
 
 =cut
 
@@ -379,6 +464,97 @@ sub port_commands {
     return $commands;
 }
 
+=head3 port_addresses($level, $mac, \@ips)
+
+Returns the addresses and the port security entries of a port for the guest MAC
+C<$mac> and its IPs C<\@ips>, according to the port security C<$level>:
+
+=over
+
+=item C<off>: no filtering. The port also keeps the C<unknown> address, so that
+frames for other MACs behind the guest (macvlan, VRRP virtual MACs, ...) still
+get delivered.
+
+=item C<mac>: like C<off>, but the guest can only send with its own MAC.
+
+=item C<mac-ip>: the guest can only send with its own MAC and IPs, and only
+receives traffic for them. Without known IPs, only the MAC is enforced.
+
+=back
+
+=cut
+
+sub port_addresses {
+    my ($level, $mac, $ips) = @_;
+
+    $mac = lc($mac);
+    my $entry = join(' ', $mac, @$ips);
+
+    return ([$entry, 'unknown'], []) if $level eq 'off';
+    return ([$entry, 'unknown'], [$mac]) if $level eq 'mac';
+    return ([$entry], [$entry]) if $level eq 'mac-ip';
+
+    die "unknown port security level '$level'\n";
+}
+
+sub port_addresses_commands {
+    my ($iface, $addresses, $port_security) = @_;
+
+    return [
+        ['lsp-set-addresses', $iface, @$addresses],
+        ['lsp-set-port-security', $iface, @$port_security],
+    ];
+}
+
+=head3 port_vnet($controller, $iface)
+
+Returns the vnet recorded on the logical switch port C<$iface>, if it exists.
+
+=cut
+
+sub port_vnet {
+    my ($controller, $iface) = @_;
+
+    my $output = nbctl(
+        $controller,
+        '--if-exists',
+        'get',
+        'Logical_Switch_Port',
+        $iface,
+        "external_ids:$EXT_VNET",
+    );
+    $output =~ s/^\s+|\s+$//g;
+    $output =~ s/^"(.*)"$/$1/;
+
+    return $output eq '' ? undef : $output;
+}
+
+=head3 update_port($controller, $iface, $level, \%running_cfg, $vnetid, $mac)
+
+Sets the guest MAC C<$mac> and its IPAM IPs on the logical switch port C<$iface>
+according to the port security C<$level>, and records the MAC so that later
+syncs can update the port.
+
+=cut
+
+sub update_port {
+    my ($controller, $iface, $level, $running_cfg, $vnetid, $mac) = @_;
+
+    # Without an answer from the IPAM, only the MAC can be enforced: the guest
+    # keeps its connectivity, and the next sync applies the IPs.
+    my $ips = guest_ips($running_cfg, $vnetid, $mac) // [];
+    my ($addresses, $port_security) = port_addresses($level, $mac, $ips);
+
+    nbctl_transaction(
+        $controller,
+        [
+            port_addresses_commands($iface, $addresses, $port_security)->@*,
+            # as written in the guest config, which is what the IPAM is keyed by
+            ['set', 'Logical_Switch_Port', $iface, "external_ids:$EXT_MAC=$mac"],
+        ],
+    );
+}
+
 =head3 ensure_port($controllerid, $controller, $zoneid, $vnetid, $iface)
 
 Creates or updates the logical switch port of the guest interface C<$iface> on
diff --git a/src/PVE/Network/SDN/Zones/OvnPlugin.pm b/src/PVE/Network/SDN/Zones/OvnPlugin.pm
index fc15e20..b25e70c 100644
--- a/src/PVE/Network/SDN/Zones/OvnPlugin.pm
+++ b/src/PVE/Network/SDN/Zones/OvnPlugin.pm
@@ -17,10 +17,25 @@ sub type {
     return 'ovn';
 }
 
+sub properties {
+    return {
+        'port-security' => {
+            description =>
+                "Filtering of the traffic of the guests: 'mac' only lets them send with"
+                . " their own MAC, 'mac-ip' additionally restricts them to their IPs from the"
+                . " IPAM.",
+            type => 'string',
+            enum => ['off', 'mac', 'mac-ip'],
+            default => 'off',
+        },
+    };
+}
+
 sub options {
     return {
         nodes => { optional => 1 },
         controller => { optional => 0 },
+        'port-security' => { optional => 1 },
         mtu => { optional => 1 },
         dns => { optional => 1 },
         reversedns => { optional => 1 },
@@ -95,6 +110,36 @@ sub tap_plug {
     die "VLAN tags and trunks are not supported on OVN vnet '$vnetid'\n" if $tag || $trunks;
 }
 
+# Called by qemu-server on guest start and by pve-container on plug, with the
+# guest MAC, which tap_plug does not get.
+sub add_bridge_fdb {
+    my ($class, $plugin_config, $iface, $macaddr) = @_;
+
+    return if !$macaddr;
+
+    my $running_config = PVE::Network::SDN::running_config();
+    my $controllerid = $plugin_config->{controller};
+    my $controller = $running_config->{controllers}->{ids}->{$controllerid} or return;
+
+    eval {
+        my $vnetid = PVE::Network::SDN::Ovn::port_vnet($controller, $iface)
+            or die "logical switch port does not exist\n";
+        my $vnet = $running_config->{vnets}->{ids}->{$vnetid}
+            or die "vnet '$vnetid' is not applied\n";
+        my $zone = $running_config->{zones}->{ids}->{ $vnet->{zone} };
+
+        PVE::Network::SDN::Ovn::update_port(
+            $controller,
+            $iface,
+            $zone->{'port-security'} // 'off',
+            $running_config,
+            $vnetid,
+            $macaddr,
+        );
+    };
+    warn "unable to set the addresses of OVN logical switch port '$iface' - $@" if $@;
+}
+
 sub on_update_hook {
     my ($class, $zoneid, $zone_cfg, $controller_cfg) = @_;
 
diff --git a/src/test/run_test_ovn.pl b/src/test/run_test_ovn.pl
index 16b7eef..3e97a92 100755
--- a/src/test/run_test_ovn.pl
+++ b/src/test/run_test_ovn.pl
@@ -34,6 +34,7 @@ my $running_cfg = {
                 type => 'ovn',
                 controller => 'ovnctl',
                 nodes => { node2 => 1 },
+                'port-security' => 'mac-ip',
             },
             evpn => { type => 'evpn', controller => 'evpnctl' },
         },
@@ -79,8 +80,8 @@ is_deeply(
     {
         ovnctl => {
             switches => {
-                vnet1 => { external_ids => ext('zone1', 'vnet1') },
-                vnet2 => { external_ids => ext('zone2', 'vnet2') },
+                vnet1 => { 'port-security' => 'off', external_ids => ext('zone1', 'vnet1') },
+                vnet2 => { 'port-security' => 'mac-ip', external_ids => ext('zone2', 'vnet2') },
             },
         },
     },
@@ -289,6 +290,72 @@ is(
     'firewall_port_name: container',
 );
 
+# port security
+
+my $mac = 'BC:24:11:00:00:01';
+is_deeply(
+    [PVE::Network::SDN::Ovn::port_addresses('off', $mac, ['10.0.0.5'])],
+    [['bc:24:11:00:00:01 10.0.0.5', 'unknown'], []],
+    'port_addresses: off keeps unknown, no filtering',
+);
+is_deeply(
+    [PVE::Network::SDN::Ovn::port_addresses('mac', $mac, ['10.0.0.5', 'fd00::5'])],
+    [['bc:24:11:00:00:01 10.0.0.5 fd00::5', 'unknown'], ['bc:24:11:00:00:01']],
+    'port_addresses: mac filters the source MAC only',
+);
+is_deeply(
+    [PVE::Network::SDN::Ovn::port_addresses('mac-ip', $mac, ['10.0.0.5'])],
+    [['bc:24:11:00:00:01 10.0.0.5'], ['bc:24:11:00:00:01 10.0.0.5']],
+    'port_addresses: mac-ip filters MAC and IPs, no unknown',
+);
+is_deeply(
+    [PVE::Network::SDN::Ovn::port_addresses('mac-ip', $mac, [])],
+    [['bc:24:11:00:00:01'], ['bc:24:11:00:00:01']],
+    'port_addresses: mac-ip without IPAM IPs only enforces the MAC',
+);
+eval { PVE::Network::SDN::Ovn::port_addresses('strict', $mac, []) };
+like($@, qr/unknown port security level 'strict'/, 'port_addresses: invalid level');
+
+my $sec_switches = [
+    { name => 'vnet1', external_ids => ext('zone1', 'vnet1') },
+    { name => 'vnet2', external_ids => ext('zone2', 'vnet2') },
+];
+my $sec_ports = [
+    { # zone2 is mac-ip, port still open from before the MAC was known
+        name => 'tap100i0',
+        external_ids => { ext('zone2', 'vnet2', 100)->%*, 'pve-sdn-mac' => $mac },
+        addresses => 'unknown',
+        port_security => ['set', []]->[1],
+    },
+    { # zone1 is off and the port is already up to date (single element set)
+        name => 'tap101i0',
+        external_ids =>
+            { ext('zone1', 'vnet1', 101)->%*, 'pve-sdn-mac' => 'BC:24:11:00:00:02' },
+        addresses => ['bc:24:11:00:00:02 10.0.0.6', 'unknown'],
+        port_security => [],
+    },
+    { # MAC not known yet
+        name => 'tap102i0',
+        external_ids => ext('zone2', 'vnet2', 102),
+        addresses => 'unknown',
+    },
+];
+is_deeply(
+    PVE::Network::SDN::Ovn::sync_commands(
+        'ovnctl',
+        $desired->{ovnctl},
+        $sec_switches,
+        $sec_ports,
+        { 100 => {}, 101 => {}, 102 => {} },
+        { $mac => ['10.0.0.5'], 'BC:24:11:00:00:02' => ['10.0.0.6'] },
+    ),
+    [
+        ['lsp-set-addresses', 'tap100i0', 'bc:24:11:00:00:01 10.0.0.5'],
+        ['lsp-set-port-security', 'tap100i0', 'bc:24:11:00:00:01 10.0.0.5'],
+    ],
+    'sync_commands: apply the port security level of the zone to known guests',
+);
+
 # guest interfaces removed from their vnet
 
 is_deeply(
@@ -307,6 +374,7 @@ is_deeply(
             { name => 'tap100i3', external_ids => ext('zone1', 'vnet1', 100), up => 1 },
         ],
         { 100 => {} },
+        {},
         { 100 => { net0 => 'vnet1', net1 => 'vmbr0' } },
     ),
     [
@@ -336,6 +404,73 @@ is_deeply(
     'guest_nics: bridges of the guest interfaces',
 );
 
+# IPAM lookup
+
+my $ipam_calls = 0;
+my $ipams_module = Test::MockModule->new('PVE::Network::SDN::Ipams');
+$ipams_module->mock(get_ips_from_mac => sub { $ipam_calls++; return ('10.0.0.5', undef) });
+my $ipam_cfg = {
+    zones => { ids => { zone1 => { type => 'ovn', ipam => 'pve' } } },
+    vnets => { ids => { vnet1 => { zone => 'zone1' }, vnet2 => { zone => 'zone1' } } },
+    subnets => { ids => { 'zone1-10.0.0.0-24' => { vnet => 'vnet1' } } },
+};
+is_deeply(
+    PVE::Network::SDN::Ovn::guest_ips($ipam_cfg, 'vnet1', $mac),
+    ['10.0.0.5'],
+    'guest_ips: vnet with subnet',
+);
+$ipam_calls = 0;
+is_deeply(
+    PVE::Network::SDN::Ovn::guest_ips($ipam_cfg, 'vnet2', $mac),
+    [],
+    'guest_ips: vnet without subnet has no IPs',
+);
+is($ipam_calls, 0, 'guest_ips: IPAM not queried for a vnet without subnet');
+
+$ipams_module->mock(get_ips_from_mac => sub { $ipam_calls++; die "netbox unreachable\n" });
+my @ipam_warnings;
+{
+    local $SIG{__WARN__} = sub { push @ipam_warnings, $_[0] };
+    is(
+        scalar(PVE::Network::SDN::Ovn::guest_ips($ipam_cfg, 'vnet1', $mac)),
+        undef,
+        'guest_ips: undef if the IPAM cannot be queried',
+    );
+    $ipam_calls = 0;
+    is_deeply(
+        PVE::Network::SDN::Ovn::ipam_lookup(
+            $ipam_cfg,
+            [
+                { external_ids => { 'pve-sdn-mac' => $mac, 'pve-sdn-vnet' => 'vnet1' } },
+                { external_ids => { 'pve-sdn-mac' => $mac, 'pve-sdn-vnet' => 'vnet1' } },
+            ],
+        ),
+        { $mac => undef },
+        'ipam_lookup: failed lookup recorded as undef',
+    );
+    is($ipam_calls, 1, 'ipam_lookup: failed lookup not repeated for the same MAC');
+}
+like(join('', @ipam_warnings), qr/netbox unreachable/, 'guest_ips: IPAM failure is logged');
+$ipams_module->unmock_all();
+
+is_deeply(
+    PVE::Network::SDN::Ovn::sync_commands(
+        'ovnctl',
+        $desired->{ovnctl},
+        [{ name => 'vnet2', external_ids => ext('zone2', 'vnet2') }],
+        [{
+            name => 'tap100i0',
+            external_ids => { ext('zone2', 'vnet2', 100)->%*, 'pve-sdn-mac' => $mac },
+            addresses => ['bc:24:11:00:00:01 10.0.0.5'],
+            port_security => ['bc:24:11:00:00:01 10.0.0.5'],
+        }],
+        { 100 => {} },
+        { $mac => undef },
+    ),
+    [['ls-add', 'vnet1'], set_switch('vnet1', 'zone1')],
+    'sync_commands: port kept as is if the IPAM cannot be queried',
+);
+
 # unsupported options
 
 eval {
-- 
2.47.3