Merge pull request #1787 from PeppyH/T4958-openconnect-radius-accounting

T4958: ocserv: openconnect: Add RADIUS accounting support

16 files changed, 137 insertions, 28 deletions

diff --git a/data/templates/ocserv/ocserv_config.j2 b/data/templates/ocserv/ocserv_config.j2
index 3194354e6..aa1073bca 100644
--- a/data/templates/ocserv/ocserv_config.j2
+++ b/data/templates/ocserv/ocserv_config.j2
@@ -10,6 +10,10 @@ udp-port = {{ listen_ports.udp }}
 run-as-user = nobody
 run-as-group = daemon
 
+{% if accounting.mode.radius is vyos_defined %}
+acct = "radius [config=/run/ocserv/radiusclient.conf]"
+{% endif %}
+
 {% if "radius" in authentication.mode %}
 auth = "radius [config=/run/ocserv/radiusclient.conf{{ ',groupconfig=true' if authentication.radius.groupconfig is vyos_defined else '' }}]"
 {% elif "local" in authentication.mode %}
diff --git a/data/templates/ocserv/radius_conf.j2 b/data/templates/ocserv/radius_conf.j2
index b6612fee5..1ab322f69 100644
--- a/data/templates/ocserv/radius_conf.j2
+++ b/data/templates/ocserv/radius_conf.j2
@@ -1,20 +1,34 @@
 ### generated by vpn_openconnect.py ###
 nas-identifier VyOS
-{% for srv in server %}
-{%     if not "disable" in server[srv] %}
-{%         if "port" in server[srv] %}
-authserver {{ srv }}:{{ server[srv]["port"] }}
+
+#### Accounting
+{% if accounting.mode.radius is vyos_defined %}
+{%     for acctsrv, srv_conf in accounting.radius.server.items() if 'disable' not in srv_conf %}
+{%         if srv_conf.port is vyos_defined %}
+acctserver {{ acctsrv }}:{{ srv_conf.port }}
 {%         else %}
-authserver {{ srv }}
+acctserver {{ acctsrv }}
 {%         endif %}
-{%     endif %}
-{% endfor %}
-radius_timeout {{ timeout }}
-{% if source_address %}
-bindaddr {{ source_address }}
-{% else %}
+{%     endfor %}
+{% endif %}
+
+#### Authentication
+{% if authentication.mode.radius is vyos_defined %}
+{%     for authsrv, srv_conf in authentication.radius.server.items() if 'disable' not in srv_conf %}
+{%         if srv_conf.port is vyos_defined %}
+authserver {{ authsrv }}:{{ srv_conf.port }}
+{%         else %}
+authserver {{ authsrv }}
+{%         endif %}
+{%     endfor %}
+radius_timeout {{ authentication['radius']['timeout'] }}
+{%     if source_address %}
+bindaddr {{ authentication['radius']['source_address'] }}
+{%     else %}
 bindaddr *
+{%     endif %}
 {% endif %}
+
 servers /run/ocserv/radius_servers
 dictionary /etc/radcli/dictionary
 default_realm
diff --git a/interface-definitions/include/radius-acct-server-ipv4.xml.i b/interface-definitions/include/radius-acct-server-ipv4.xml.i
new file mode 100644
index 000000000..9365aa8e9
--- /dev/null
+++ b/interface-definitions/include/radius-acct-server-ipv4.xml.i
@@ -0,0 +1,26 @@
+<!-- include start from radius-acct-server-ipv4.xml.i -->
+<node name="radius">
+  <properties>
+    <help>RADIUS accounting for users OpenConnect VPN sessions OpenConnect authentication mode radius</help>
+  </properties>
+  <children>
+    <tagNode name="server">
+      <properties>
+        <help>RADIUS server configuration</help>
+        <valueHelp>
+          <format>ipv4</format>
+          <description>RADIUS server IPv4 address</description>
+        </valueHelp>
+        <constraint>
+          <validator name="ipv4-address"/>
+        </constraint>
+      </properties>
+      <children>
+        #include <include/generic-disable-node.xml.i>
+        #include <include/radius-server-key.xml.i>
+        #include <include/radius-server-acct-port.xml.i>
+      </children>
+    </tagNode>
+  </children>
+</node>
+<!-- include end -->
diff --git a/interface-definitions/include/radius-server-ipv4.xml.i b/interface-definitions/include/radius-auth-server-ipv4.xml.i
index ab4c8e10e..dc6f4d878 100644
--- a/interface-definitions/include/radius-server-ipv4.xml.i
+++ b/interface-definitions/include/radius-auth-server-ipv4.xml.i
@@ -1,4 +1,4 @@
-<!-- include start from radius-server-ipv4.xml.i -->
+<!-- include start from radius-auth-server-ipv4.xml.i -->
 <node name="radius">
   <properties>
     <help>RADIUS based user authentication</help>
@@ -19,7 +19,7 @@
       <children>
         #include <include/generic-disable-node.xml.i>
         #include <include/radius-server-key.xml.i>
-        #include <include/radius-server-port.xml.i>
+        #include <include/radius-server-auth-port.xml.i>
       </children>
     </tagNode>
   </children>
diff --git a/interface-definitions/include/radius-server-acct-port.xml.i b/interface-definitions/include/radius-server-acct-port.xml.i
new file mode 100644
index 000000000..0b356fa18
--- /dev/null
+++ b/interface-definitions/include/radius-server-acct-port.xml.i
@@ -0,0 +1,15 @@
+<!-- include start from radius-server-acct-port.xml.i -->
+<leafNode name="port">
+  <properties>
+    <help>Accounting port</help>
+    <valueHelp>
+      <format>u32:1-65535</format>
+      <description>Numeric IP port</description>
+    </valueHelp>
+    <constraint>
+      <validator name="numeric" argument="--range 1-65535"/>
+    </constraint>
+  </properties>
+  <defaultValue>1813</defaultValue>
+</leafNode>
+<!-- include end -->
diff --git a/interface-definitions/include/radius-server-port.xml.i b/interface-definitions/include/radius-server-auth-port.xml.i
index c6b691a0f..660fa540f 100644
--- a/interface-definitions/include/radius-server-port.xml.i
+++ b/interface-definitions/include/radius-server-auth-port.xml.i
@@ -1,4 +1,4 @@
-<!-- include start from radius-server-port.xml.i -->
+<!-- include start from radius-server-auth-port.xml.i -->
 <leafNode name="port">
   <properties>
     <help>Authentication port</help>
diff --git a/interface-definitions/include/radius-server-ipv4-ipv6.xml.i b/interface-definitions/include/radius-server-ipv4-ipv6.xml.i
index 5b12bec62..c593512b4 100644
--- a/interface-definitions/include/radius-server-ipv4-ipv6.xml.i
+++ b/interface-definitions/include/radius-server-ipv4-ipv6.xml.i
@@ -23,7 +23,7 @@
       <children>
         #include <include/generic-disable-node.xml.i>
         #include <include/radius-server-key.xml.i>
-        #include <include/radius-server-port.xml.i>
+        #include <include/radius-server-auth-port.xml.i>
       </children>
     </tagNode>
     <leafNode name="source-address">
diff --git a/interface-definitions/interfaces-wireless.xml.in b/interface-definitions/interfaces-wireless.xml.in
index aff5071b2..a9538d577 100644
--- a/interface-definitions/interfaces-wireless.xml.in
+++ b/interface-definitions/interfaces-wireless.xml.in
@@ -725,7 +725,7 @@
                       <constraintErrorMessage>Invalid WPA pass phrase, must be 8 to 63 printable characters!</constraintErrorMessage>
                     </properties>
                   </leafNode>
-                  #include <include/radius-server-ipv4.xml.i>
+                  #include <include/radius-auth-server-ipv4.xml.i>
                   <node name="radius">
                     <children>
                       <tagNode name="server">
diff --git a/interface-definitions/service-ipoe-server.xml.in b/interface-definitions/service-ipoe-server.xml.in
index ef8569437..d778f9de0 100644
--- a/interface-definitions/service-ipoe-server.xml.in
+++ b/interface-definitions/service-ipoe-server.xml.in
@@ -220,7 +220,7 @@
                   #include <include/accel-ppp/radius-additions-rate-limit.xml.i>
                 </children>
               </node>
-              #include <include/radius-server-ipv4.xml.i>
+              #include <include/radius-auth-server-ipv4.xml.i>
               #include <include/accel-ppp/radius-additions.xml.i>
             </children>
           </node>
diff --git a/interface-definitions/service-pppoe-server.xml.in b/interface-definitions/service-pppoe-server.xml.in
index 47ad96582..68592b96b 100644
--- a/interface-definitions/service-pppoe-server.xml.in
+++ b/interface-definitions/service-pppoe-server.xml.in
@@ -20,7 +20,7 @@
               #include <include/accel-ppp/auth-local-users.xml.i>
               #include <include/accel-ppp/auth-mode.xml.i>
               #include <include/accel-ppp/auth-protocols.xml.i>
-              #include <include/radius-server-ipv4.xml.i>
+              #include <include/radius-auth-server-ipv4.xml.i>
               #include <include/accel-ppp/radius-additions.xml.i>
               <node name="radius">
                 <children>
diff --git a/interface-definitions/vpn-ipsec.xml.in b/interface-definitions/vpn-ipsec.xml.in
index fa12d999c..4bb9ad145 100644
--- a/interface-definitions/vpn-ipsec.xml.in
+++ b/interface-definitions/vpn-ipsec.xml.in
@@ -923,7 +923,7 @@
                   #include <include/name-server-ipv4-ipv6.xml.i>
                 </children>
               </tagNode>
-              #include <include/radius-server-ipv4.xml.i>
+              #include <include/radius-auth-server-ipv4.xml.i>
               <node name="radius">
                 <children>
                   #include <include/radius-nas-identifier.xml.i>
diff --git a/interface-definitions/vpn-l2tp.xml.in b/interface-definitions/vpn-l2tp.xml.in
index 86aeb324e..0a92017bd 100644
--- a/interface-definitions/vpn-l2tp.xml.in
+++ b/interface-definitions/vpn-l2tp.xml.in
@@ -178,7 +178,7 @@
                   #include <include/accel-ppp/ppp-mppe.xml.i>
                   #include <include/accel-ppp/auth-mode.xml.i>
                   #include <include/accel-ppp/auth-local-users.xml.i>
-                  #include <include/radius-server-ipv4.xml.i>
+                  #include <include/radius-auth-server-ipv4.xml.i>
                   <node name="radius">
                     <children>
                       <tagNode name="server">
diff --git a/interface-definitions/vpn-openconnect.xml.in b/interface-definitions/vpn-openconnect.xml.in
index 82fe2bbc9..a426f604d 100644
--- a/interface-definitions/vpn-openconnect.xml.in
+++ b/interface-definitions/vpn-openconnect.xml.in
@@ -8,6 +8,27 @@
           <priority>901</priority>
         </properties>
         <children>
+          <node name="accounting">
+            <properties>
+              <help>Accounting for users OpenConnect VPN Sessions</help>
+            </properties>
+            <children>
+              <node name="mode">
+                <properties>
+                  <help>Accounting mode used by this server</help>
+                </properties>
+                <children>
+                  <leafNode name="radius">
+                    <properties>
+                      <help>Use RADIUS server for accounting</help>
+                      <valueless/>
+                    </properties>
+                  </leafNode>
+                </children>
+              </node>
+              #include <include/radius-acct-server-ipv4.xml.i>
+            </children>
+          </node>
           <node name="authentication">
             <properties>
               <help>Authentication for remote access SSL VPN Server</help>
@@ -137,7 +158,7 @@
                   </tagNode>
                 </children>
               </node>
-              #include <include/radius-server-ipv4.xml.i>
+              #include <include/radius-auth-server-ipv4.xml.i>
               <node name="radius">
                 <children>
                   #include <include/radius-timeout.xml.i>
diff --git a/interface-definitions/vpn-pptp.xml.in b/interface-definitions/vpn-pptp.xml.in
index 5e52965fd..00ffd26f9 100644
--- a/interface-definitions/vpn-pptp.xml.in
+++ b/interface-definitions/vpn-pptp.xml.in
@@ -108,7 +108,7 @@
                       </tagNode>
                     </children>
                   </node>
-                  #include <include/radius-server-ipv4.xml.i>
+                  #include <include/radius-auth-server-ipv4.xml.i>
                   #include <include/accel-ppp/radius-additions.xml.i>
                   #include <include/accel-ppp/radius-additions-rate-limit.xml.i>
                 </children>
diff --git a/interface-definitions/vpn-sstp.xml.in b/interface-definitions/vpn-sstp.xml.in
index 195d581df..9e912063f 100644
--- a/interface-definitions/vpn-sstp.xml.in
+++ b/interface-definitions/vpn-sstp.xml.in
@@ -16,7 +16,7 @@
               #include <include/accel-ppp/auth-local-users.xml.i>
               #include <include/accel-ppp/auth-mode.xml.i>
               #include <include/accel-ppp/auth-protocols.xml.i>
-              #include <include/radius-server-ipv4.xml.i>
+              #include <include/radius-auth-server-ipv4.xml.i>
               #include <include/accel-ppp/radius-additions.xml.i>
               <node name="radius">
                 <children>
diff --git a/src/conf_mode/vpn_openconnect.py b/src/conf_mode/vpn_openconnect.py
index 57eba17b0..63ffe2a41 100755
--- a/src/conf_mode/vpn_openconnect.py
+++ b/src/conf_mode/vpn_openconnect.py
@@ -82,13 +82,26 @@ def T2665_default_dict_cleanup(origin: dict, default_values: dict) -> dict:
         del origin['authentication']['radius']['server']['port']
         if not origin["authentication"]['radius']['server']:
             raise ConfigError(
-                'Openconnect mode radius required at least one radius server')
+                'Openconnect authentication mode radius required at least one radius server')
         default_values_radius_port = \
         default_values['authentication']['radius']['server']['port']
         for server, params in origin['authentication']['radius'][
             'server'].items():
             if 'port' not in params:
                 params['port'] = default_values_radius_port
+
+    if 'mode' in origin["accounting"] and "radius" in \
+            origin["accounting"]["mode"]:
+        del origin['accounting']['radius']['server']['port']
+        if not origin["accounting"]['radius']['server']:
+            raise ConfigError(
+                'Openconnect accounting mode radius required at least one radius server')
+        default_values_radius_port = \
+            default_values['accounting']['radius']['server']['port']
+        for server, params in origin['accounting']['radius'][
+            'server'].items():
+            if 'port' not in params:
+                params['port'] = default_values_radius_port
     return origin
 
 
@@ -121,6 +134,14 @@ def verify(ocserv):
                 not is_listen_port_bind_service(int(port), 'ocserv-main'):
             raise ConfigError(f'"{proto}" port "{port}" is used by another service')
 
+    # Check accounting
+    if "accounting" in ocserv:
+        if "mode" in ocserv["accounting"] and "radius" in ocserv["accounting"]["mode"]:
+            if "authentication" not in ocserv or "mode" not in ocserv["authentication"]:
+                raise ConfigError('Accounting depends on OpenConnect authentication configuration')
+            elif "radius" not in ocserv["authentication"]["mode"]:
+                raise ConfigError('RADIUS accounting must be used with RADIUS authentication')
+
     # Check authentication
     if "authentication" in ocserv:
         if "mode" in ocserv["authentication"]:
@@ -202,10 +223,18 @@ def generate(ocserv):
         return None
 
     if "radius" in ocserv["authentication"]["mode"]:
-        # Render radius client configuration
-        render(radius_cfg, 'ocserv/radius_conf.j2', ocserv["authentication"]["radius"])
-        # Render radius servers
-        render(radius_servers, 'ocserv/radius_servers.j2', ocserv["authentication"]["radius"])
+        if dict_search(ocserv, 'accounting.mode.radius'):
+            # Render radius client configuration
+            render(radius_cfg, 'ocserv/radius_conf.j2', ocserv)
+            merged_servers = ocserv["accounting"]["radius"]["server"] | ocserv["authentication"]["radius"]["server"]
+            # Render radius servers
+            # Merge the accounting and authentication servers into a single dictionary
+            render(radius_servers, 'ocserv/radius_servers.j2', {'server': merged_servers})
+        else:
+            # Render radius client configuration
+            render(radius_cfg, 'ocserv/radius_conf.j2', ocserv)
+            # Render radius servers
+            render(radius_servers, 'ocserv/radius_servers.j2', ocserv["authentication"]["radius"])
     elif "local" in ocserv["authentication"]["mode"]:
         # if mode "OTP", generate OTP users file parameters
         if "otp" in ocserv["authentication"]["mode"]["local"]: