Merge pull request #5016 from c-po/ospf-auth

ospf: T7679: fix plaintext authentication on specific interface

4 files changed, 90 insertions, 26 deletions

diff --git a/data/templates/frr/ospfd.frr.j2 b/data/templates/frr/ospfd.frr.j2
index 7277597e7..a3a8c5bcb 100644
--- a/data/templates/frr/ospfd.frr.j2
+++ b/data/templates/frr/ospfd.frr.j2
@@ -3,6 +3,7 @@
 {%     for iface, iface_config in interface.items() %}
 interface {{ iface }}
 {%         if iface_config.authentication.plaintext_password is vyos_defined %}
+ ip ospf authentication
  ip ospf authentication-key {{ iface_config.authentication.plaintext_password }}
 {%         elif iface_config.authentication.md5 is vyos_defined %}
  ip ospf authentication message-digest
@@ -11,6 +12,8 @@ interface {{ iface }}
  ip ospf message-digest-key {{ key }} md5 {{ key_config.md5_key }}
 {%                 endfor %}
 {%             endif %}
+{%         elif iface_config.authentication.null is vyos_defined %}
+ ip ospf authentication null
 {%         endif %}
 {%         if iface_config.area is vyos_defined %}
  ip ospf area {{ iface_config.area }}
@@ -121,11 +124,14 @@ router ospf {{ 'vrf ' ~ vrf if vrf is vyos_defined }}
 {%         if area_config.virtual_link is vyos_defined %}
 {%             for link, link_config in area_config.virtual_link.items() %}
 {%                 if link_config.authentication.plaintext_password is vyos_defined %}
+ area {{ area_id }} virtual-link {{ link }} authentication
  area {{ area_id }} virtual-link {{ link }} authentication-key {{ link_config.authentication.plaintext_password }}
 {%                 elif link_config.authentication.md5.key_id is vyos_defined %}
 {%                     for key, key_config in link_config.authentication.md5.key_id.items() %}
  area {{ area_id }} virtual-link {{ link }} message-digest-key {{ key }} md5 {{ key_config.md5_key }}
 {%                     endfor %}
+{%                 elif link_config.authentication.null is vyos_defined %}
+ area {{ area_id }} virtual-link {{ link }} authentication null
 {%                 endif %}
 {#         The following values are default values #}
  area {{ area_id }} virtual-link {{ link }} hello-interval {{ link_config.hello_interval }} retransmit-interval {{ link_config.retransmit_interval }} retransmit-window {{ link_config.retransmit_window }} transmit-delay {{ link_config.transmit_delay }} dead-interval {{ link_config.dead_interval }}
diff --git a/interface-definitions/include/ospf/authentication.xml.i b/interface-definitions/include/ospf/authentication.xml.i
index 8e8cad067..f38a37269 100644
--- a/interface-definitions/include/ospf/authentication.xml.i
+++ b/interface-definitions/include/ospf/authentication.xml.i
@@ -38,6 +38,12 @@
         </tagNode>
       </children>
     </node>
+    <leafNode name="null">
+      <properties>
+        <help>Use null authentication</help>
+        <valueless/>
+      </properties>
+    </leafNode>
     <leafNode name="plaintext-password">
       <properties>
         <help>Plain text password</help>
diff --git a/smoketest/scripts/cli/test_protocols_ospf.py b/smoketest/scripts/cli/test_protocols_ospf.py
index dfa09dadc..d942a8b98 100755
--- a/smoketest/scripts/cli/test_protocols_ospf.py
+++ b/smoketest/scripts/cli/test_protocols_ospf.py
@@ -612,5 +612,48 @@ class TestProtocolsOSPF(VyOSUnitTestSHIM.TestCase):
         self.assertIn(f' area {area} {area_type} translate-never no-summary', frrconfig)
         self.assertIn(f' network {network} area {area}', frrconfig)
 
+    def test_ospf_19_authentication(self):
+        md5_key = 'vyosMD5'
+        md5_id = '10'
+        plaintext_key = 'vyos123'
+
+        self.cli_set(base_path + ['area', '0'])
+        self.cli_set(base_path + ['interface', dummy_if, 'authentication', 'md5', 'key-id', md5_id, 'md5-key', md5_key])
+        self.cli_commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = self.getFRRconfig(f'interface {dummy_if}', stop_section='^exit')
+        self.assertIn( ' ip ospf authentication message-digest', frrconfig)
+        self.assertIn(f' ip ospf message-digest-key {md5_id} md5 {md5_key}', frrconfig)
+
+        self.cli_set(base_path + ['interface', dummy_if, 'authentication', 'plaintext-password', plaintext_key])
+        # FRR only allows a single authentication mode (MD5, NULL or plaintext) at a time
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+        self.cli_delete(base_path + ['interface', dummy_if, 'authentication', 'md5'])
+        self.cli_commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = self.getFRRconfig(f'interface {dummy_if}', stop_section='^exit')
+        self.assertNotIn( ' ip ospf authentication message-digest', frrconfig)
+        self.assertNotIn(f' ip ospf message-digest-key {md5_id} md5 {md5_key}', frrconfig)
+        self.assertIn( ' ip ospf authentication', frrconfig)
+        self.assertIn(f' ip ospf authentication-key {plaintext_key}', frrconfig)
+
+        self.cli_set(base_path + ['interface', dummy_if, 'authentication', 'null'])
+        # FRR only allows a single authentication mode (MD5, NULL or plaintext) at a time
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+        self.cli_delete(base_path + ['interface', dummy_if, 'authentication', 'plaintext-password'])
+        self.cli_commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = self.getFRRconfig(f'interface {dummy_if}', stop_section='^exit')
+        self.assertNotIn( ' ip ospf authentication message-digest', frrconfig)
+        self.assertNotIn(f' ip ospf message-digest-key {md5_id} md5 {md5_key}', frrconfig)
+        self.assertNotRegex(r'^ ip ospf authentication$', frrconfig)
+        self.assertNotIn(f' ip ospf authentication-key {plaintext_key}', frrconfig)
+        self.assertIn(' ip ospf authentication null', frrconfig)
+
 if __name__ == '__main__':
     unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on())
diff --git a/src/conf_mode/protocols_ospf.py b/src/conf_mode/protocols_ospf.py
index 848079ba1..46023e486 100755
--- a/src/conf_mode/protocols_ospf.py
+++ b/src/conf_mode/protocols_ospf.py
@@ -62,30 +62,30 @@ def verify(config_dict):
 
     # Validate if configured Access-list exists
     if 'area' in ospf:
-          networks = []
-          for area, area_config in ospf['area'].items():
-              # Implemented as warning to not break existing configurations
-              if area == '0' and dict_search('area_type.nssa', area_config) != None:
-                  Warning('You cannot configure NSSA to backbone!')
-              # Implemented as warning to not break existing configurations
-              if area == '0' and dict_search('area_type.stub', area_config) != None:
-                  Warning('You cannot configure STUB to backbone!')
-              # Implemented as warning to not break existing configurations
-              if len(area_config['area_type']) > 1:
-                  Warning(f'Only one area-type is supported for area "{area}"!')
-
-              if 'import_list' in area_config:
-                  acl_import = area_config['import_list']
-                  if acl_import: verify_access_list(acl_import, ospf)
-              if 'export_list' in area_config:
-                  acl_export = area_config['export_list']
-                  if acl_export: verify_access_list(acl_export, ospf)
-
-              if 'network' in area_config:
-                  for network in area_config['network']:
-                      if network in networks:
-                          raise ConfigError(f'Network "{network}" already defined in different area!')
-                      networks.append(network)
+        networks = []
+        for area, area_config in ospf['area'].items():
+            # Implemented as warning to not break existing configurations
+            if area == '0' and dict_search('area_type.nssa', area_config) != None:
+                Warning('You cannot configure NSSA to backbone!')
+            # Implemented as warning to not break existing configurations
+            if area == '0' and dict_search('area_type.stub', area_config) != None:
+                Warning('You cannot configure STUB to backbone!')
+            # Implemented as warning to not break existing configurations
+            if len(area_config['area_type']) > 1:
+                Warning(f'Only one area-type is supported for area "{area}"!')
+
+            if 'import_list' in area_config:
+                if acl_import := area_config['import_list']:
+                    verify_access_list(acl_import, ospf)
+            if 'export_list' in area_config:
+                if acl_export := area_config['export_list']:
+                    verify_access_list(acl_export, ospf)
+
+            if 'network' in area_config:
+                for network in area_config['network']:
+                    if network in networks:
+                        raise ConfigError(f'Network "{network}" already defined in different area!')
+                    networks.append(network)
 
     if 'interface' in ospf:
         for interface, interface_config in ospf['interface'].items():
@@ -102,8 +102,17 @@ def verify(config_dict):
             if 'area' in ospf and 'area' in interface_config:
                 for area, area_config in ospf['area'].items():
                     if 'network' in area_config:
-                        raise ConfigError('Can not use OSPF interface area and area ' \
-                                          'network configuration at the same time!')
+                        raise ConfigError('Can not use OSPF "interface area" and ' \
+                                          '"area network" configuration at the same time!')
+
+            # FRR only allows a single authentication mode (MD5, NULL or plaintext)
+            # at a time. Prevent users from defining more than one authentication mode.
+            if 'authentication' in interface_config:
+                auth_keys = set(interface_config['authentication'])
+                exclusive_auth_keys = {'md5', 'null', 'plaintext_password'}
+                if len(auth_keys & exclusive_auth_keys) >= 2:
+                    raise ConfigError('Can not use multiple authentication modes '
+                                      f'simultaneously for interface "{interface}"!')
 
             # If interface specific options are set, we must ensure that the
             # interface is bound to our requesting VRF. Due to the VyOS