Merge pull request #4824 from alexandr-san4ez/T4251-current

syslog: T4251: Rename "permitted-peers" to "permitted-peer" and improve TLS checks

6 files changed, 101 insertions, 19 deletions

diff --git a/data/templates/rsyslog/rsyslog.conf.j2 b/data/templates/rsyslog/rsyslog.conf.j2
index 9b9153010..714788df0 100644
--- a/data/templates/rsyslog/rsyslog.conf.j2
+++ b/data/templates/rsyslog/rsyslog.conf.j2
@@ -120,9 +120,12 @@ if prifilt("{{ tmp | join(',') }}") then {
         StreamDriverMode="1"
         # Select the authentication mode
         StreamDriverAuthMode="{{ auth_mode if auth_mode == 'anon' else 'x509/' + auth_mode }}"
-{%                 if tls.permitted_peers is vyos_defined and auth_mode in ('fingerprint', 'name') %}
+{%                 if tls.permitted_peer is vyos_defined and auth_mode in ('fingerprint', 'name') %}
+{%                     set permitted_peers = tls.permitted_peer | map('trim') | select | join(',') %}
+{%                     if permitted_peers %}
         # Only include permitted peers (list of allowed fingerprints or names)
-        StreamDriverPermittedPeers="{{ tls.permitted_peers }}"
+        StreamDriverPermittedPeers="{{ permitted_peers }}"
+{%                     endif %}
 {%                 endif %}
 {%                 if tls.ca_certificate_path is vyos_defined %}
         # Include the path to the CA certificate file
diff --git a/interface-definitions/include/version/system-version.xml.i b/interface-definitions/include/version/system-version.xml.i
index 5cdece74a..a9987e8e4 100644
--- a/interface-definitions/include/version/system-version.xml.i
+++ b/interface-definitions/include/version/system-version.xml.i
@@ -1,3 +1,3 @@
 <!-- include start from include/version/system-version.xml.i -->
-<syntaxVersion component='system' version='29'></syntaxVersion>
+<syntaxVersion component='system' version='30'></syntaxVersion>
 <!-- include end -->
diff --git a/interface-definitions/system_syslog.xml.in b/interface-definitions/system_syslog.xml.in
index 4acbc729d..221b4320f 100644
--- a/interface-definitions/system_syslog.xml.in
+++ b/interface-definitions/system_syslog.xml.in
@@ -86,7 +86,7 @@
                       </valueHelp>
                       <valueHelp>
                         <format>fingerprint</format>
-                        <description>Authenticate peer by matching its certificate fingerprint to a configured, permitted list (`permitted-peers` option)</description>
+                        <description>Authenticate peer by matching its certificate fingerprint to a configured, permitted list (`permitted-peer` option)</description>
                       </valueHelp>
                       <valueHelp>
                         <format>certvalid</format>
@@ -94,7 +94,7 @@
                       </valueHelp>
                       <valueHelp>
                         <format>name</format>
-                        <description>Authenticate peer by verifying its certificate subject name against a configured value (`permitted-peers` option)</description>
+                        <description>Authenticate peer by verifying its certificate subject name against a configured value (`permitted-peer` option)</description>
                       </valueHelp>
                       <constraint>
                         <regex>(anon|fingerprint|certvalid|name)</regex>
@@ -102,13 +102,14 @@
                     </properties>
                     <defaultValue>anon</defaultValue>
                   </leafNode>
-                  <leafNode name="permitted-peers">
+                  <leafNode name="permitted-peer">
                     <properties>
-                      <help>Comma-separated list of allowed peer certificate fingerprints or subject names</help>
+                      <help>Allowed peer certificate fingerprint or subject name</help>
                       <valueHelp>
                         <format>txt</format>
-                        <description>Comma-separated fingerprints or peer names.\nFor example:\n - 'SHA1:DD:23:E3:E7:70:F5:B4:13:44:16:78:A5:5A:8C:39:48:53:A6:DD:25,SHA256:10:C4:26:1D:CB:3C:AB:12:DB:1A:F0:47:37:AE:6D:D2:DE:66:B5:71:B7:2E:5B:BB:AE:0C:7E:7F:5F:0D:E9:64'\n - 'logs.example.com'</description>
+                        <description>Peer fingerprint - SHA1:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX:XX or subject name - logs.example.com</description>
                       </valueHelp>
+                      <multi/>
                     </properties>
                   </leafNode>
                 </children>
diff --git a/smoketest/scripts/cli/test_system_syslog.py b/smoketest/scripts/cli/test_system_syslog.py
index b04c3ea58..79fbe27ec 100755
--- a/smoketest/scripts/cli/test_system_syslog.py
+++ b/smoketest/scripts/cli/test_system_syslog.py
@@ -308,13 +308,13 @@ class TestRSYSLOGService(VyOSUnitTestSHIM.TestCase):
             '172.10.0.1': {
                 'facility': {'all': {'level': 'debug'}},
                 'port': '6514',
-                'protocol': 'udp',
+                'protocol': 'tcp',
                 'tls': {},
             },
             '172.10.0.2': {
                 'facility': {'all': {'level': 'debug'}},
                 'port': '6514',
-                'protocol': 'udp',
+                'protocol': 'tcp',
                 'tls': {
                     'auth-mode': 'anon',
                 },
@@ -336,7 +336,10 @@ class TestRSYSLOGService(VyOSUnitTestSHIM.TestCase):
                     'ca-certificate': ca_cert_name,
                     'certificate': client_cert_name,
                     'auth-mode': 'fingerprint',
-                    'permitted-peers': 'SHA1:E1:DB:C4:FF:83:54:85:40:2D:56:E7:1A:C3:FF:70:22:0F:21:74:ED',
+                    'permitted-peer': [
+                        'SHA1:E1:DB:C4:FF:83:54:85:40:2D:56:E7:1A:C3:FF:70:22:0F:21:74:ED',
+                        ' SHA1:FF:70:22:0F:21:74:ED:54:85:40:2D:56:E7:1A:C3:E1:DB:C4:FF:83 ',
+                    ],
                 },
             },
             '172.10.0.5': {
@@ -347,7 +350,10 @@ class TestRSYSLOGService(VyOSUnitTestSHIM.TestCase):
                     'ca-certificate': ca_cert_name,
                     'certificate': client_cert_name,
                     'auth-mode': 'name',
-                    'permitted-peers': 'logs.example.com',
+                    'permitted-peer': [
+                        'logs.example.com',
+                        '   ',
+                    ],
                 },
             },
         }
@@ -372,7 +378,12 @@ class TestRSYSLOGService(VyOSUnitTestSHIM.TestCase):
             tls = remote_options['tls']
             if tls:
                 for key, value in tls.items():
-                    self.cli_set(remote_base + ['tls', key], value=value)
+                    if type(value) is list:
+                        values = value
+                        for value in values:
+                            self.cli_set(remote_base + ['tls', key], value=value)
+                    else:
+                        self.cli_set(remote_base + ['tls', key], value=value)
             else:
                 self.cli_set(remote_base + ['tls'])
 
@@ -412,13 +423,28 @@ class TestRSYSLOGService(VyOSUnitTestSHIM.TestCase):
                     auth_mode = value if value == 'anon' else f'x509/{value}'
                     self.assertIn(f'StreamDriverAuthMode="{auth_mode}"', config)
 
-                if 'permitted-peers' in tls:
-                    value = tls['permitted-peers']
+                if 'permitted-peer' in tls:
+                    values = tls['permitted-peer']
+                    value = ','.join([v.strip() for v in values if v.strip()])
                     self.assertIn(f'StreamDriverPermittedPeers="{value}"', config)
 
                 if not tls:
                     self.assertIn(f'StreamDriverAuthMode="anon"', config)
 
+    def test_remote_tls_protocol_udp(self):
+        remote_base = base_path + ['remote', '172.11.0.1']
+        self.cli_set(remote_base + ['port'], value='6514')
+        self.cli_set(remote_base + ['facility', 'all', 'level'], value='debug')
+        self.cli_set(remote_base + ['protocol'], value='udp')
+        self.cli_set(remote_base + ['tls'])
+
+        err_msg = "TLS is enabled for remote \"172.11.0.1\", but protocol is set to UDP"
+        with self.assertRaisesRegex(ConfigSessionError, err_msg):
+            self.cli_commit()
+
+        self.cli_set(base_path + ['remote', '172.11.0.1', 'protocol'], value='tcp')
+        self.cli_commit()
+
     def test_vrf_source_address(self):
         rhosts = {
             '169.254.0.10': { },
diff --git a/src/conf_mode/system_syslog.py b/src/conf_mode/system_syslog.py
index 76999f311..90bb45c51 100755
--- a/src/conf_mode/system_syslog.py
+++ b/src/conf_mode/system_syslog.py
@@ -71,15 +71,15 @@ def _verify_tls_remote_options(remote, remote_options, syslog):
     if ca_certificate:
         verify_pki_ca_certificate(syslog, ca_certificate)
 
-    permitted_peers = dict_search('tls.permitted_peers', remote_options)
+    permitted_peers = dict_search('tls.permitted_peer', remote_options)
     if not permitted_peers:
         if auth_mode == "fingerprint":
             raise ConfigError(
-                f'Auth mode "fingerprint" for remote "{remote}" requires "permitted-peers" to be configured!'
+                f'Auth mode "fingerprint" for remote "{remote}" requires "permitted-peer" to be configured!'
             )
         elif auth_mode == "name":
             raise ConfigError(
-                f'Auth mode "name" for remote "{remote}" requires "permitted-peers" to specify allowed subject names!'
+                f'Auth mode "name" for remote "{remote}" requires "permitted-peer" to specify allowed subject names!'
             )
 
 
@@ -181,7 +181,7 @@ def verify(syslog):
                 _verify_tls_remote_options(remote, remote_options, syslog)
 
                 if 'protocol' in remote_options and remote_options['protocol'] == 'udp':
-                    Warning(
+                    raise ConfigError(
                         f'TLS is enabled for remote "{remote}", but protocol is set to UDP. TLS is only supported with protocol TCP!'
                     )
 
diff --git a/src/migration-scripts/system/29-to-30 b/src/migration-scripts/system/29-to-30
new file mode 100644
index 000000000..7babde3f3
--- /dev/null
+++ b/src/migration-scripts/system/29-to-30
@@ -0,0 +1,52 @@
+# Copyright 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
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this library.  If not, see <http://www.gnu.org/licenses/>.
+
+# T4251:
+#   - drop "tls enable" node (make "tls" a standalone key)
+#   - split "tls permitted-peers" list by commas into multiple "tls permitted-peer" entries
+
+from vyos.configtree import ConfigTree
+
+base = ['system', 'syslog', 'remote']
+
+
+def migrate(config: ConfigTree) -> None:
+    if not config.exists(base):
+        return
+
+    # Iterate over all remote syslog server entries (like 172.18.0.5)
+    for remote_addr in config.list_nodes(base):
+        remote_base = base + [remote_addr]
+        tls_base = remote_base + ['tls']
+
+        # (1) Remove "tls enable" -> migrate to simple "tls"
+        enable_path = tls_base + ['enable']
+        if config.exists(enable_path):
+            # Remove obsolete "enable" node
+            config.delete(enable_path)
+
+        # (2) Split "tls permitted-peers" (comma-separated string)
+        permitted_peers_path = tls_base + ['permitted-peers']
+        if config.exists(permitted_peers_path):
+            peers_str = config.return_value(permitted_peers_path)
+            # Split CSV values and normalize whitespace
+            peers = [p.strip() for p in peers_str.split(',') if p.strip()]
+
+            # Create a new "permitted-peer" entry per item
+            for peer in peers:
+                config.set(tls_base + ['permitted-peer'], value=peer, replace=False)
+
+            # Remove the old combined node
+            config.delete(permitted_peers_path)