From f482df7d71293ae98dd4ea4f4c168439e2e48f2b Mon Sep 17 00:00:00 2001
From: Franco Fichtner <franco@opnsense.org>
Date: Tue, 18 Mar 2025 16:32:24 +0100
Subject: [PATCH] system: remove optional defaults, secure access to sysctl
 node

The node is now empty by default so code peeking into the items
should check beforehand or use a safe iterator like config_read_array().

This is for 25.7 with the infamous two security sysctls flipping back
to FreeBSD defaults.
---
 src/etc/config.xml.sample                           | 13 +------------
 src/etc/inc/system.inc                              | 13 +++++++++----
 .../OPNsense/Core/Api/TunablesController.php        |  2 +-
 src/opnsense/scripts/interfaces/carp_set_status.php |  2 +-
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/src/etc/config.xml.sample b/src/etc/config.xml.sample
index 170f80c73..5e8b101e5 100644
--- a/src/etc/config.xml.sample
+++ b/src/etc/config.xml.sample
@@ -2,18 +2,7 @@
 <opnsense>
   <trigger_initial_wizard/>
   <theme>opnsense</theme>
-  <sysctl>
-    <item>
-      <descr><![CDATA[Disable Indirect Branch Restricted Speculation (Spectre V2 mitigation)]]></descr>
-      <tunable>hw.ibrs_disable</tunable>
-      <value>default</value>
-    </item>
-    <item>
-      <descr><![CDATA[Page Table Isolation (Meltdown mitigation, requires reboot.)]]></descr>
-      <tunable>vm.pmap.pti</tunable>
-      <value>default</value>
-    </item>
-  </sysctl>
+  <sysctl/>
   <system>
     <optimization>normal</optimization>
     <hostname>OPNsense</hostname>
diff --git a/src/etc/inc/system.inc b/src/etc/inc/system.inc
index 5fbcc100b..92e2ae52a 100644
--- a/src/etc/inc/system.inc
+++ b/src/etc/inc/system.inc
@@ -69,12 +69,18 @@ function system_sysctl_defaults()
 {
     global $config;
 
+    /* notes:
+     *  - set 'optional' => true for values that should no longer be enforced by default
+    /*  - 'decription' and 'type' can be used to indicate persistence of these values
+     *    when in reality these are not available due to kernel modules unloaded or
+     *    compile time options unset (like debugging)
+     *  - always set 'default' so the backend can use it and the user can see it
+     */
+
     return [
         'debug.kassert.warn_only' => [ 'default' => '1', 'description' => 'KASSERT triggers a panic (0) or just a warning (1)', 'type' => 'w' ],
-        'hw.ibrs_disable' => [ 'default' => '0', 'optional' => true ],
         'hw.ixl.enable_head_writeback' => [ 'default' => '0' ],
         'hw.syscons.kbd_reboot' => [ 'default' => '0' ],
-        'hw.uart.console' => [ 'default' => 'io:0x3f8,br:' . system_console_speed(), 'type' => 't', 'optional' => true ], /* XXX remove in a future release */
         'hw.vtnet.csum_disable' => [ 'default' => '1' ],
         'kern.coredump' => [ 'default' => '0' ],
         'kern.ipc.maxsockbuf' => [ 'default' => '4262144' ],
@@ -131,7 +137,6 @@ function system_sysctl_defaults()
         'vfs.zfs.dirty_data_sync_percent' => [ 'default' => '5' ],
         'vfs.zfs.txg.timeout' => [ 'default' => '90' ],
         'vm.numa.disabled' => [ 'default' => '1' ],
-        'vm.pmap.pti' => [ 'default' => '1', 'optional' => true ],
     ];
 }
 
@@ -143,7 +148,7 @@ function system_sysctl_get()
     foreach ($defaults as $name => $info) {
         /* compile list of required sysctls not necessarily present in config */
         if (empty($info['optional'])) {
-            $sysctls[$name] = 'default';
+            $sysctls[$name] = '';
         }
     }
 
diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/TunablesController.php b/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/TunablesController.php
index f13e72d35..70e15a482 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/TunablesController.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/TunablesController.php
@@ -74,7 +74,7 @@ class TunablesController extends ApiMutableModelControllerBase
                 Config::getInstance()->lock();
                 $factory_config = Config::getInstance()->toArrayFromFile('/usr/local/etc/config.xml', []);
                 $mdl = $this->getModel()->Default();
-                if (!empty($factory_config['sysctl']) && !empty($factory_config['sysctl']['item'])) {
+                if (!empty($factory_config['sysctl']['item'])) {
                     foreach ($factory_config['sysctl']['item'] as $item) {
                         $node = $mdl->item->Add();
                         foreach ($item as $key => $val) {
diff --git a/src/opnsense/scripts/interfaces/carp_set_status.php b/src/opnsense/scripts/interfaces/carp_set_status.php
index 231c73433..925a82889 100755
--- a/src/opnsense/scripts/interfaces/carp_set_status.php
+++ b/src/opnsense/scripts/interfaces/carp_set_status.php
@@ -38,7 +38,7 @@ if ($action == 'maintenance') {
     if (isset($config["virtualip_carp_maintenancemode"])) {
         unset($config["virtualip_carp_maintenancemode"]);
         $carp_demotion_default = '0';
-        foreach ($config['sysctl']['item'] as $tunable) {
+        foreach (config_read_array('sysctl', 'item') as $tunable) {
             if ($tunable['tunable'] == 'net.inet.carp.demotion' && ctype_digit($tunable['value'])) {
                 $carp_demotion_default = $tunable['value'];
             }