Improve handling for local server SID

Do not rely on Samba to generate a new system SID and make randomized
SID persistent across server changes. This is to help prevent admin
foot-shooting when they choose to make major server changes that force
Samba to regnerate a new SID and thus invalidate their share ACLs.
on production servers.

src/middlewared/middlewared/plugins/smb.py

@@ -368,7 +368,7 @@ async def configure(self, job, create_paths=True):
             self.logger.warning("Failed to set immutable flag on /var/empty", exc_info=True)
 
         job.set_progress(30, 'Setting up server SID.')
-        await self.middleware.call('smb.set_sid', data['cifs_SID'])
+        await self.middleware.call('smb.set_system_sid')
 
         """
         If the ldap passdb backend is being used, then the remote LDAP server
@@ -697,9 +697,7 @@ async def do_update(self, app, data):
             await self.apply_aapl_changes()
 
         if old['netbiosname_local'] != new_config['netbiosname_local']:
-            new_sid = await self.middleware.call("smb.get_system_sid")
-            await self.middleware.call("smb.set_database_sid", new_sid)
-            new_config["cifs_SID"] = new_sid
+            await self.middleware.call('smb.set_system_sid')
             await self.middleware.call('idmap.gencache.flush')
             await self.middleware.call("smb.synchronize_group_mappings")
             srv = (await self.middleware.call("network.configuration.config"))["service_announcement"]

src/middlewared/middlewared/plugins/smb_/sid.py

@@ -1,10 +1,13 @@
+import subprocess
+
 from middlewared.service import Service, private
 from middlewared.utils import run
-from middlewared.plugins.smb import SMBCmd
-
-import re
+from middlewared.utils.functools_ import cache
+from threading import Lock
+from .constants import SMBCmd
+from .utils import random_sid
 
-RE_SID = re.compile(r"S-\d-\d+-(\d+-){1,14}\d+$")
+SID_LOCK = Lock()
 
 
 class SMBService(Service):
@@ -13,70 +16,25 @@ class Config:
         service = 'cifs'
         service_verb = 'restart'
 
+    @cache
     @private
-    async def get_system_sid(self):
-        getSID = await run([SMBCmd.NET.value, "-d", "0", "getlocalsid"], check=False)
-        if getSID.returncode != 0:
-            self.logger.debug('Failed to retrieve local system SID: %s',
-                              getSID.stderr.decode())
-            return None
+    def local_server_sid(self):
+        with SID_LOCK:
+            if (db_sid := self.middleware.call_sync('smb.config')['cifs_SID']):
+                return db_sid
 
-        m = RE_SID.search(getSID.stdout.decode().strip())
-        if m:
-            return m.group(0)
-
-        self.logger.debug("getlocalsid returned invalid SID: %s",
-                          getSID.stdout.decode().strip())
-        return None
+            new_sid = random_sid()
+            self.middleware.call_sync('datastore.update', 'services.cifs', 1, {'cifs_SID': new_sid})
+            return new_sid
 
     @private
-    async def set_sid(self, db_sid):
-        system_SID = await self.get_system_sid()
-
-        if system_SID == db_sid:
-            return True
+    def set_system_sid(self):
+        server_sid = self.local_server_sid()
 
-        if db_sid:
-            if not await self.set_system_sid(db_sid):
-                self.logger.debug('Unable to set set SID to %s', db_sid)
-                return False
-        else:
-            if not system_SID:
-                self.logger.warning('Unable to determine system and database SIDs')
-                return False
-
-            await self.set_database_sid(system_SID)
-            return True
-
-    @private
-    async def set_database_sid(self, SID):
-        await self.middleware.call('datastore.update', 'services.cifs', 1, {'cifs_SID': SID})
-
-    @private
-    async def set_system_sid(self, SID):
-        if not SID:
-            return False
-
-        setSID = await run([SMBCmd.NET.value, "-d", "0", "setlocalsid", SID], check=False)
-        if setSID.returncode != 0:
-            self.logger.debug("setlocalsid failed with error: %s",
-                              setSID.stderr.decode())
-            return False
-
-        return True
-
-    @private
-    async def fixsid(self, groupmap=None):
-        """
-        Samba generates a new domain sid when its netbios name changes or if samba's secrets.tdb
-        has been deleted. passdb.tdb will automatically reflect the new mappings, but the groupmap
-        database is not automatically updated in these circumstances. This check is performed when
-        synchronizing group mapping database. In case there entries that no longer match our local
-        system sid, group_mapping.tdb will be removed and re-generated.
-        """
-        db_SID = (await self.middleware.call('smb.config'))['cifs_SID']
-        system_sid = await self.get_system_sid()
+        setsid = subprocess.run([
+            SMBCmd.NET.value, '-d', '0',
+           'setlocalsid', server_sid,
+        ], capture_output=True, check=False)
 
-        if db_SID != system_sid:
-            self.logger.warning(f"Domain SID in group_mapping.tdb ({system_sid}) is not SID in nas config ({db_SID}). Updating db")
-            await self.set_database_sid(system_sid)
+        if setsid.returncode != 0:
+            raise CallError(f'setlocalsid failed: {setsid.stderr.decode()}')

src/middlewared/middlewared/plugins/smb_/utils.py

@@ -1,4 +1,14 @@
 from .constants import SMBSharePreset
+from random import randbytes
+
+
+def random_sid():
+    sid_bytes = randbytes(12)
+    subauth_1 = int.from_bytes(sid_bytes[0:4])
+    subauth_2 = int.from_bytes(sid_bytes[4:8])
+    subauth_3 = int.from_bytes(sid_bytes[8:])
+
+    return f'S-1-5-21-{subauth_1}-{subauth_2}-{subauth_3}'
 
 
 def smb_strip_comments(auxparam_in):