From 33b9bc55f9e3d7ce6a5ff447a6b545a96915daf3 Mon Sep 17 00:00:00 2001
From: Giga Murphy <giga1699@gmail.com>
Date: Fri, 18 Aug 2023 00:26:51 +0000
Subject: T5447: Initial support for MACsec static keys

---
 python/vyos/ifconfig/macsec.py | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

(limited to 'python')

diff --git a/python/vyos/ifconfig/macsec.py b/python/vyos/ifconfig/macsec.py
index 1a78d18d8..6318a1688 100644
--- a/python/vyos/ifconfig/macsec.py
+++ b/python/vyos/ifconfig/macsec.py
@@ -39,12 +39,36 @@ class MACsecIf(Interface):
     def _create(self):
         """
         Create MACsec interface in OS kernel. Interface is administrative
-        down by default.
+        down by default when not using static keys.
         """
+
         # create tunnel interface
         cmd  = 'ip link add link {source_interface} {ifname} type {type}'.format(**self.config)
         cmd += f' cipher {self.config["security"]["cipher"]}'
         self._cmd(cmd)
 
-        # interface is always A/D down. It needs to be enabled explicitly
-        self.set_admin_state('down')
+        # Check if using static keys
+        if 'static' in self.config["security"]:
+            # Set static TX key
+            cmd = 'ip macsec add {ifname} tx sa 0 pn 1 on key 00'.format(**self.config)
+            cmd += f' {self.config["security"]["static"]["tx_key"]}'
+            self._cmd(cmd)
+
+            for peer, peer_config in self.config["security"]["static"]["peer"].items():
+                if 'disable' in peer_config:
+                    continue
+
+                # Create the address
+                cmd = 'ip macsec add {ifname} rx port 1 address'.format(**self.config)
+                cmd += f' {peer_config["mac"]}'
+                self._cmd(cmd)
+                # Add the rx-key to the address
+                cmd += f' sa 0 pn 1 on key 01 {peer_config["rx_key"]}'
+                self._cmd(cmd)
+
+            # Set admin state to up
+            self.set_admin_state('up')
+
+        else:
+            # interface is always A/D down. It needs to be enabled explicitly
+            self.set_admin_state('down')
-- 
cgit v1.2.3


From 525beb3202bb3ccb15c28d2c41e24e68762af253 Mon Sep 17 00:00:00 2001
From: Giga Murphy <giga1699@gmail.com>
Date: Sun, 20 Aug 2023 15:46:50 +0000
Subject: T5447: Implement maintainer feedback

---
 .../include/interface/macsec-key.xml.i             | 15 +++++++
 interface-definitions/interfaces-macsec.xml.in     | 34 +++------------
 python/vyos/ifconfig/macsec.py                     |  6 +--
 smoketest/scripts/cli/test_interfaces_macsec.py    | 11 +++--
 src/conf_mode/interfaces-macsec.py                 | 50 +++++++++++-----------
 5 files changed, 54 insertions(+), 62 deletions(-)
 create mode 100644 interface-definitions/include/interface/macsec-key.xml.i

(limited to 'python')

diff --git a/interface-definitions/include/interface/macsec-key.xml.i b/interface-definitions/include/interface/macsec-key.xml.i
new file mode 100644
index 000000000..5a857a612
--- /dev/null
+++ b/interface-definitions/include/interface/macsec-key.xml.i
@@ -0,0 +1,15 @@
+<!-- include start from interface/macsec-key.xml.i -->
+<leafNode name="key">
+  <properties>
+    <help>MACsec static key</help>
+    <valueHelp>
+      <format>txt</format>
+      <description>16-byte (128-bit) hex-string (32 hex-digits) for gcm-aes-128 or 32-byte (256-bit) hex-string (64 hex-digits) for gcm-aes-256</description>
+    </valueHelp>
+    <constraint>
+      <regex>[A-Fa-f0-9]{32}</regex>
+      <regex>[A-Fa-f0-9]{64}</regex>
+    </constraint>
+  </properties>
+</leafNode>
+<!-- include end -->
diff --git a/interface-definitions/interfaces-macsec.xml.in b/interface-definitions/interfaces-macsec.xml.in
index b81c9b40c..766b0bede 100644
--- a/interface-definitions/interfaces-macsec.xml.in
+++ b/interface-definitions/interfaces-macsec.xml.in
@@ -54,46 +54,22 @@
               </leafNode>
               <node name="static">
                 <properties>
-                  <help>Assign static MACSec keys instead of using MKA</help>
+                  <help>Use static keys for MACsec [static Secure Authentication Key (SAK) mode]</help>
                 </properties>
                 <children>
-                  <leafNode name="tx-key">
-                    <properties>
-                      <help>Set the static transmit key</help>
-                      <valueHelp>
-                        <format>txt</format>
-                        <description>16-byte (128-bit) hex-string (32 hex-digits) for gcm-aes-128 or 32-byte (256-bit) hex-string (64 hex-digits) for gcm-aes-256</description>
-                      </valueHelp>
-                      <constraint>
-                        <regex>[A-Fa-f0-9]{32}</regex>
-                        <regex>[A-Fa-f0-9]{64}</regex>
-                      </constraint>
-                    </properties>
-                  </leafNode>
+                  #include <include/interface/macsec-key.xml.i>
                   <tagNode name="peer">
                     <properties>
-                      <help>peer alias</help>
+                      <help>MACsec peer name</help>
                       <constraint>
                         <regex>[^ ]{1,100}</regex>
                       </constraint>
-                      <constraintErrorMessage>peer alias too long (limit 100 characters)</constraintErrorMessage>
+                      <constraintErrorMessage>MACsec peer name exceeds limit of 100 characters</constraintErrorMessage>
                     </properties>
                     <children>
                       #include <include/generic-disable-node.xml.i>
                       #include <include/interface/mac.xml.i>
-                      <leafNode name="rx-key">
-                        <properties>
-                          <help>Set the static receive key for peer</help>
-                          <valueHelp>
-                            <format>txt</format>
-                            <description>16-byte (128-bit) hex-string (32 hex-digits) for gcm-aes-128 or 32-byte (256-bit) hex-string (64 hex-digits) for gcm-aes-256</description>
-                          </valueHelp>
-                          <constraint>
-                            <regex>[A-Fa-f0-9]{32}</regex>
-                            <regex>[A-Fa-f0-9]{64}</regex>
-                          </constraint>
-                        </properties>
-                      </leafNode>
+                      #include <include/interface/macsec-key.xml.i>
                     </children>
                   </tagNode>
                 </children>
diff --git a/python/vyos/ifconfig/macsec.py b/python/vyos/ifconfig/macsec.py
index 6318a1688..e14f22c58 100644
--- a/python/vyos/ifconfig/macsec.py
+++ b/python/vyos/ifconfig/macsec.py
@@ -51,7 +51,7 @@ class MACsecIf(Interface):
         if 'static' in self.config["security"]:
             # Set static TX key
             cmd = 'ip macsec add {ifname} tx sa 0 pn 1 on key 00'.format(**self.config)
-            cmd += f' {self.config["security"]["static"]["tx_key"]}'
+            cmd += f' {self.config["security"]["static"]["key"]}'
             self._cmd(cmd)
 
             for peer, peer_config in self.config["security"]["static"]["peer"].items():
@@ -63,11 +63,11 @@ class MACsecIf(Interface):
                 cmd += f' {peer_config["mac"]}'
                 self._cmd(cmd)
                 # Add the rx-key to the address
-                cmd += f' sa 0 pn 1 on key 01 {peer_config["rx_key"]}'
+                cmd += f' sa 0 pn 1 on key 01 {peer_config["key"]}'
                 self._cmd(cmd)
 
             # Set admin state to up
-            self.set_admin_state('up')
+            self.set_admin_state('down')
 
         else:
             # interface is always A/D down. It needs to be enabled explicitly
diff --git a/smoketest/scripts/cli/test_interfaces_macsec.py b/smoketest/scripts/cli/test_interfaces_macsec.py
index 8c9867500..30d1ad659 100755
--- a/smoketest/scripts/cli/test_interfaces_macsec.py
+++ b/smoketest/scripts/cli/test_interfaces_macsec.py
@@ -245,10 +245,10 @@ class MACsecInterfaceTest(BasicInterfaceTest.TestCase):
             self.cli_commit()
 
         # check validate() - tx-key length must match cipher
-        self.cli_set(self._base_path + [interface, 'security', 'static', 'tx-key', tx_key_2])
+        self.cli_set(self._base_path + [interface, 'security', 'static', 'key', tx_key_2])
         with self.assertRaises(ConfigSessionError):
             self.cli_commit()
-        self.cli_set(self._base_path + [interface, 'security', 'static', 'tx-key', tx_key_1])
+        self.cli_set(self._base_path + [interface, 'security', 'static', 'key', tx_key_1])
 
         # check validate() - at least one peer must be defined
         with self.assertRaises(ConfigSessionError):
@@ -262,23 +262,22 @@ class MACsecInterfaceTest(BasicInterfaceTest.TestCase):
         with self.assertRaises(ConfigSessionError):
             self.cli_commit()
         self.cli_delete(self._base_path + [interface, 'security', 'static', 'peer', 'TESTPEER', 'mac'])
-        self.cli_set(self._base_path + [interface, 'security', 'static', 'peer', 'TESTPEER', 'rx-key', rx_key_1])
+        self.cli_set(self._base_path + [interface, 'security', 'static', 'peer', 'TESTPEER', 'key', rx_key_1])
         with self.assertRaises(ConfigSessionError):
             self.cli_commit()
         self.cli_set(self._base_path + [interface, 'security', 'static', 'peer', 'TESTPEER', 'mac', peer_mac])
 
         # check validate() - peer rx-key length must match cipher
         self.cli_set(self._base_path + [interface, 'security', 'cipher', cipher2])
-        self.cli_set(self._base_path + [interface, 'security', 'static', 'tx-key', tx_key_2])
+        self.cli_set(self._base_path + [interface, 'security', 'static', 'key', tx_key_2])
         with self.assertRaises(ConfigSessionError):
             self.cli_commit()
-        self.cli_set(self._base_path + [interface, 'security', 'static', 'peer', 'TESTPEER', 'rx-key', rx_key_2])
+        self.cli_set(self._base_path + [interface, 'security', 'static', 'peer', 'TESTPEER', 'key', rx_key_2])
 
         # final commit and verify
         self.cli_commit()
         self.assertIn(interface, interfaces())
         self.assertEqual(cipher2, get_cipher(interface))
-        # Add checks for keys?
         self.assertTrue(os.path.isdir(f'/sys/class/net/{interface}'))
 
 if __name__ == '__main__':
diff --git a/src/conf_mode/interfaces-macsec.py b/src/conf_mode/interfaces-macsec.py
index 023fcdffc..0cd733294 100755
--- a/src/conf_mode/interfaces-macsec.py
+++ b/src/conf_mode/interfaces-macsec.py
@@ -43,6 +43,14 @@ airbag.enable()
 # XXX: wpa_supplicant works on the source interface
 wpa_suppl_conf = '/run/wpa_supplicant/{source_interface}.conf'
 
+# Constants
+## gcm-aes-128 requires a 128bit long key - 32 characters (string) = 16byte = 128bit
+GCM_AES_128_LEN: int = 32
+GCM_128_KEY_ERROR = 'gcm-aes-128 requires a 128bit long key!'
+## gcm-aes-256 requires a 256bit long key - 64 characters (string) = 32byte = 256bit
+GCM_AES_256_LEN: int = 64
+GCM_256_KEY_ERROR = 'gcm-aes-256 requires a 256bit long key!'
+
 def get_config(config=None):
     """
     Retrive CLI config as dictionary. Dictionary can never be empty, as at least the
@@ -96,18 +104,16 @@ def verify(macsec):
         # Logic to check static configuration
         if dict_search('security.static', macsec) != None:
             # tx-key must be defined
-            if dict_search('security.static.tx_key', macsec) == None:
+            if dict_search('security.static.key', macsec) == None:
                 raise ConfigError('Static MACsec tx-key must be defined.')
 
-            tx_len = len(dict_search('security.static.tx_key', macsec))
+            tx_len = len(dict_search('security.static.key', macsec))
 
-            if dict_search('security.cipher', macsec) == 'gcm-aes-128' and tx_len != 32:
-                # gcm-aes-128 requires a 128bit long key - 32 characters (string) = 16byte = 128bit
-                raise ConfigError('gcm-aes-128 requires a 128bit long key!')
+            if dict_search('security.cipher', macsec) == 'gcm-aes-128' and tx_len != GCM_AES_128_LEN:
+                raise ConfigError(GCM_128_KEY_ERROR)
 
-            if dict_search('security.cipher', macsec) == 'gcm-aes-256' and tx_len != 64:
-                # gcm-aes-256 requires a 256bit long key - 64 characters (string) = 32byte = 256bit
-                raise ConfigError('gcm-aes-256 requires a 256bit long key!')
+            if dict_search('security.cipher', macsec) == 'gcm-aes-256' and tx_len != GCM_AES_256_LEN:
+                raise ConfigError(GCM_256_KEY_ERROR)
 
             # Make sure at least one peer is defined
             if 'peer' not in macsec['security']['static']:
@@ -115,19 +121,17 @@ def verify(macsec):
 
             # For every enabled peer, make sure a MAC and rx-key is defined
             for peer, peer_config in macsec['security']['static']['peer'].items():
-                if 'disable' not in peer_config and ('mac' not in peer_config or 'rx_key' not in peer_config):
+                if 'disable' not in peer_config and ('mac' not in peer_config or 'key' not in peer_config):
                     raise ConfigError('Every enabled MACsec static peer must have a MAC address and rx-key defined.')
 
                 # check rx-key length against cipher suite
-                rx_len = len(peer_config['rx_key'])
+                rx_len = len(peer_config['key'])
 
-                if dict_search('security.cipher', macsec) == 'gcm-aes-128' and rx_len != 32:
-                    # gcm-aes-128 requires a 128bit long key - 32 characters (string) = 16byte = 128bit
-                    raise ConfigError('gcm-aes-128 requires a 128bit long key!')
+                if dict_search('security.cipher', macsec) == 'gcm-aes-128' and rx_len != GCM_AES_128_LEN:
+                    raise ConfigError(GCM_128_KEY_ERROR)
 
-                if dict_search('security.cipher', macsec) == 'gcm-aes-256' and rx_len != 64:
-                    # gcm-aes-256 requires a 256bit long key - 64 characters (string) = 32byte = 256bit
-                    raise ConfigError('gcm-aes-256 requires a 256bit long key!')
+                if dict_search('security.cipher', macsec) == 'gcm-aes-256' and rx_len != GCM_AES_256_LEN:
+                    raise ConfigError(GCM_256_KEY_ERROR)
 
         # Logic to check MKA configuration
         else:
@@ -136,13 +140,11 @@ def verify(macsec):
 
             cak_len = len(dict_search('security.mka.cak', macsec))
 
-            if dict_search('security.cipher', macsec) == 'gcm-aes-128' and cak_len != 32:
-                # gcm-aes-128 requires a 128bit long key - 32 characters (string) = 16byte = 128bit
-                raise ConfigError('gcm-aes-128 requires a 128bit long key!')
+            if dict_search('security.cipher', macsec) == 'gcm-aes-128' and cak_len != GCM_AES_128_LEN:
+                raise ConfigError(GCM_128_KEY_ERROR)
 
-            elif dict_search('security.cipher', macsec) == 'gcm-aes-256' and cak_len != 64:
-                # gcm-aes-256 requires a 256bit long key - 64 characters (string) = 32byte = 256bit
-                raise ConfigError('gcm-aes-256 requires a 256bit long key!')
+            elif dict_search('security.cipher', macsec) == 'gcm-aes-256' and cak_len != GCM_AES_256_LEN:
+                raise ConfigError(GCM_256_KEY_ERROR)
 
     if 'source_interface' in macsec:
         # MACsec adds a 40 byte overhead (32 byte MACsec + 8 bytes VLAN 802.1ad
@@ -186,8 +188,8 @@ def apply(macsec):
     i = MACsecIf(**macsec)
     i.update(macsec)
 
-    #Only reload/restart if not using static keys
-    if dict_search('security.static', macsec) == None:
+    # Only reload/restart if using MKA
+    if dict_search('security.mka.cak', macsec):
         if not is_systemd_service_running(systemd_service) or 'shutdown_required' in macsec:
             call(f'systemctl reload-or-restart {systemd_service}')
 
-- 
cgit v1.2.3


From 69c60327ab4467417169c23d288cfc0fec628318 Mon Sep 17 00:00:00 2001
From: Giga Murphy <giga1699@gmail.com>
Date: Sun, 20 Aug 2023 15:50:21 +0000
Subject: T5447: Corrected comment for interface down

---
 python/vyos/ifconfig/macsec.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'python')

diff --git a/python/vyos/ifconfig/macsec.py b/python/vyos/ifconfig/macsec.py
index e14f22c58..861fc2486 100644
--- a/python/vyos/ifconfig/macsec.py
+++ b/python/vyos/ifconfig/macsec.py
@@ -66,7 +66,7 @@ class MACsecIf(Interface):
                 cmd += f' sa 0 pn 1 on key 01 {peer_config["key"]}'
                 self._cmd(cmd)
 
-            # Set admin state to up
+            # interface is always A/D down. It needs to be enabled explicitly
             self.set_admin_state('down')
 
         else:
-- 
cgit v1.2.3


From 0910cc09c46555e4a5e797d508874f1d546b3444 Mon Sep 17 00:00:00 2001
From: Giga Murphy <giga1699@gmail.com>
Date: Sun, 20 Aug 2023 15:51:07 +0000
Subject: T5447: Corrected comment in _create header

---
 python/vyos/ifconfig/macsec.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'python')

diff --git a/python/vyos/ifconfig/macsec.py b/python/vyos/ifconfig/macsec.py
index 861fc2486..abecda2f9 100644
--- a/python/vyos/ifconfig/macsec.py
+++ b/python/vyos/ifconfig/macsec.py
@@ -39,7 +39,7 @@ class MACsecIf(Interface):
     def _create(self):
         """
         Create MACsec interface in OS kernel. Interface is administrative
-        down by default when not using static keys.
+        down by default.
         """
 
         # create tunnel interface
-- 
cgit v1.2.3


From ae6b46b3eef3e51ac14a1b9d2e82dffd555fec66 Mon Sep 17 00:00:00 2001
From: Giga Murphy <giga1699@gmail.com>
Date: Sun, 20 Aug 2023 15:53:58 +0000
Subject: T5447: Update copyright years

---
 python/vyos/ifconfig/macsec.py     | 2 +-
 src/conf_mode/interfaces-macsec.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

(limited to 'python')

diff --git a/python/vyos/ifconfig/macsec.py b/python/vyos/ifconfig/macsec.py
index abecda2f9..e7aee4c1e 100644
--- a/python/vyos/ifconfig/macsec.py
+++ b/python/vyos/ifconfig/macsec.py
@@ -1,4 +1,4 @@
-# Copyright 2020-2021 VyOS maintainers and contributors <maintainers@vyos.io>
+# Copyright 2020-2023 VyOS maintainers and contributors <maintainers@vyos.io>
 #
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
diff --git a/src/conf_mode/interfaces-macsec.py b/src/conf_mode/interfaces-macsec.py
index 0cd733294..ebe46f0c7 100755
--- a/src/conf_mode/interfaces-macsec.py
+++ b/src/conf_mode/interfaces-macsec.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 #
-# Copyright (C) 2020-2022 VyOS maintainers and contributors
+# Copyright (C) 2020-2023 VyOS maintainers and contributors
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 2 or later as
-- 
cgit v1.2.3


From b4bacbaa2b57644f141f9b53531317c2e6db7040 Mon Sep 17 00:00:00 2001
From: Giga Murphy <giga1699@gmail.com>
Date: Sun, 20 Aug 2023 19:11:35 +0000
Subject: T5447: Remove redundant self.set_admin_state

---
 python/vyos/ifconfig/macsec.py | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

(limited to 'python')

diff --git a/python/vyos/ifconfig/macsec.py b/python/vyos/ifconfig/macsec.py
index e7aee4c1e..9329c5ee7 100644
--- a/python/vyos/ifconfig/macsec.py
+++ b/python/vyos/ifconfig/macsec.py
@@ -66,9 +66,5 @@ class MACsecIf(Interface):
                 cmd += f' sa 0 pn 1 on key 01 {peer_config["key"]}'
                 self._cmd(cmd)
 
-            # interface is always A/D down. It needs to be enabled explicitly
-            self.set_admin_state('down')
-
-        else:
-            # interface is always A/D down. It needs to be enabled explicitly
-            self.set_admin_state('down')
+        # interface is always A/D down. It needs to be enabled explicitly
+        self.set_admin_state('down')
-- 
cgit v1.2.3