From ced69d63c5174a7819ff4d2aa8a5ddfc3d144d59 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Fri, 5 Jul 2024 11:48:04 -0700 Subject: [PATCH] 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 | 6 +- .../middlewared/plugins/smb_/sid.py | 88 +++++-------------- .../middlewared/plugins/smb_/utils.py | 10 +++ 3 files changed, 35 insertions(+), 69 deletions(-) diff --git a/src/middlewared/middlewared/plugins/smb.py b/src/middlewared/middlewared/plugins/smb.py index 5218f32d528f4..590deccd34b2e 100644 --- a/src/middlewared/middlewared/plugins/smb.py +++ b/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"] diff --git a/src/middlewared/middlewared/plugins/smb_/sid.py b/src/middlewared/middlewared/plugins/smb_/sid.py index 0cb16fe44ef27..67992503ca9c3 100644 --- a/src/middlewared/middlewared/plugins/smb_/sid.py +++ b/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()}') diff --git a/src/middlewared/middlewared/plugins/smb_/utils.py b/src/middlewared/middlewared/plugins/smb_/utils.py index a391455767570..dd920ece79fa3 100644 --- a/src/middlewared/middlewared/plugins/smb_/utils.py +++ b/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):