Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NAS-118851 / 23.10 / API authentication using Directory Services #10054

Merged
merged 20 commits into from
Feb 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions src/freenas/debian/rules
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ override_dh_auto_install:
cp etc/iso_3166_2_countries.csv debian/truenas-files/etc/; \
cp -a etc/logrotate.d debian/truenas-files/etc/; \
cp etc/nsswitch.conf debian/truenas-files/etc/; \
cp -a etc/pam.d debian/truenas-files/etc/; \
cp -a etc/syslog-ng debian/truenas-files/etc/; \
cp -a etc/systemd debian/truenas-files/etc/; \
mkdir debian/truenas-files/home; \
Expand Down
2 changes: 2 additions & 0 deletions src/freenas/etc/pam.d/common-auth-unix
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
auth requisite pam_deny.so
auth required pam_permit.so
2 changes: 2 additions & 0 deletions src/middlewared/debian/control
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ Build-Depends: alembic,
python3-onedrivesdk,
python3-packaging,
python3-parted,
python3-pampy,
python3-passlib,
python3-prctl,
python3-psutil,
Expand Down Expand Up @@ -159,6 +160,7 @@ Depends: alembic,
python3-onedrivesdk,
python3-packaging,
python3-parted,
python3-pampy,
python3-passlib,
python3-prctl,
python3-psutil,
Expand Down
3 changes: 1 addition & 2 deletions src/middlewared/middlewared/etc_files/pam.d/common-auth.mako
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,5 @@
%>\

${'\n'.join(dsp['primary'])}
auth requisite pam_deny.so
auth required pam_permit.so
@include common-auth-unix
${'\n'.join(dsp['additional'])}
18 changes: 18 additions & 0 deletions src/middlewared/middlewared/etc_files/pam.d/middleware.mako
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<%
ds_auth = middleware.call_sync('datastore.config', 'system.settings')['stg_ds_auth']
%>\
# PAM configuration for the middleware (Web UI login)

%if ds_auth:
@include common-auth
%else:
<%namespace name="pam" file="pam.inc.mako" />\
<%
dsp = pam.getNoDirectoryServicePam().pam_auth()
%>\
${'\n'.join(dsp['primary'])}
@include common-auth-unix
%endif
account required pam_deny.so
password required pam_deny.so
session required pam_deny.so
3 changes: 3 additions & 0 deletions src/middlewared/middlewared/etc_files/pam.d/pam.inc.mako
Original file line number Diff line number Diff line change
Expand Up @@ -260,3 +260,6 @@
<%def name="getDirectoryServicePam(**kwargs)">
<% return DirectoryServicePam(**kwargs) %>
</%def>
<%def name="getNoDirectoryServicePam()">
<% return DirectoryServicePamBase() %>
</%def>
47 changes: 32 additions & 15 deletions src/middlewared/middlewared/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,9 @@ def send_error(self, message, errno, reason=None, exc_info=None, etype=None, ext
async def call_method(self, message, serviceobj, methodobj):
params = message.get('params') or []

if mock := self.middleware._mock_method(message['method'], params):
methodobj = mock

try:
async with self._softhardsemaphore:
result = await self.middleware._call(message['method'], serviceobj, methodobj, params, app=self)
Expand Down Expand Up @@ -887,7 +890,7 @@ def __init__(
self.__console_io = False if os.path.exists(self.CONSOLE_ONCE_PATH) else None
self.__terminate_task = None
self.jobs = JobsQueue(self)
self.mocks = {}
self.mocks = defaultdict(list)
self.socket_messages_queue = deque(maxlen=200)
self.tasks = set()

Expand Down Expand Up @@ -1330,7 +1333,7 @@ async def _call(
self.logger.trace('Calling %r in current IO loop', name)
return await methodobj(*prepared_call.args)

if name not in self.mocks and serviceobj._config.process_pool:
if not self.mocks.get(name) and serviceobj._config.process_pool:
self.logger.trace('Calling %r in process pool', name)
if isinstance(serviceobj, middlewared.service.CRUDService):
service_name, method_name = name.rsplit('.', 1)
Expand All @@ -1352,6 +1355,9 @@ def dump_args(self, args, method=None, method_name=None):
except Exception:
return args

if mock := self._mock_method(method_name, args):
method = mock

if (not hasattr(method, 'accepts') and
method.__name__ in ['create', 'update', 'delete'] and
hasattr(method, '__self__')):
Expand All @@ -1376,6 +1382,9 @@ def dump_result(self, result, method):
async def call(self, name, *params, pipes=None, job_on_progress_cb=None, app=None, profile=False):
serviceobj, methodobj = self._method_lookup(name)

if mock := self._mock_method(name, params):
methodobj = mock

if profile:
methodobj = profile_wrap(methodobj)

Expand All @@ -1390,6 +1399,9 @@ def call_sync(self, name, *params, job_on_progress_cb=None, background=False):

serviceobj, methodobj = self._method_lookup(name)

if mock := self._mock_method(name, params):
methodobj = mock

prepared_call = self._call_prepare(name, serviceobj, methodobj, params, job_on_progress_cb=job_on_progress_cb,
in_event_loop=False)

Expand Down Expand Up @@ -1570,9 +1582,10 @@ def _tracemalloc_start(self, limit, interval):

time.sleep(interval)

def set_mock(self, name, mock):
if name in self.mocks:
raise ValueError(f'{name!r} is already mocked')
def set_mock(self, name, args, mock):
for _args, _mock in self.mocks[name]:
if args == _args:
raise ValueError(f'{name!r} is already mocked with {args!r}')

serviceobj, methodobj = self._method_lookup(name)

Expand All @@ -1587,18 +1600,22 @@ def f(*args, **kwargs):
f._job = methodobj._job
copy_function_metadata(mock, f)

self.mocks[name] = f
self.mocks[name].append((args, f))

def remove_mock(self, name):
self.mocks.pop(name)

def _method_lookup(self, name):
serviceobj, methodobj = super()._method_lookup(name)

if mock := self.mocks.get(name):
return serviceobj, mock
def remove_mock(self, name, args):
for i, (_args, _mock) in enumerate(self.mocks[name]):
if args == _args:
del self.mocks[name][i]
break

return serviceobj, methodobj
def _mock_method(self, name, params):
if mocks := self.mocks.get(name):
for args, mock in mocks:
if args == list(params):
return mock
for args, mock in mocks:
if args is None:
return mock

async def ws_handler(self, request):
ws = web.WebSocketResponse()
Expand Down
2 changes: 2 additions & 0 deletions src/middlewared/middlewared/plugins/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,7 @@ async def do_delete(self, pk, options):

await self.middleware.call('datastore.delete', 'account.bsdusers', pk)
await self.middleware.call('service.reload', 'user')
await self.middleware.call('idmap.flush_gencache')

return pk

Expand Down Expand Up @@ -1566,6 +1567,7 @@ async def do_delete(self, pk, options):
await gm_job.wait()

await self.middleware.call('service.reload', 'user')
await self.middleware.call('idmap.flush_gencache')

return pk

Expand Down
20 changes: 12 additions & 8 deletions src/middlewared/middlewared/plugins/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -651,16 +651,20 @@ def check_permission(middleware, app):
user = middleware.call_sync('auth.authenticate_root')
else:
try:
local_user = middleware.call_sync(
'datastore.query',
'account.bsdusers',
[['bsdusr_uid', '=', origin.uid]],
{'get': True, 'prefix': 'bsdusr_'},
)
query = {
'username': middleware.call_sync(
'datastore.query',
'account.bsdusers',
[['uid', '=', origin.uid]],
{'get': True, 'prefix': 'bsdusr_'},
)['username'],
}
local = True
except MatchNotFound:
return
query = {'uid': origin.uid}
local = False

user = middleware.call_sync('auth.authenticate_local_user', local_user['id'], local_user['username'])
user = middleware.call_sync('auth.authenticate_user', query, local)
if user is None:
return

Expand Down
66 changes: 25 additions & 41 deletions src/middlewared/middlewared/plugins/auth_/authenticate.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import crypt
import hmac

import pam

from middlewared.service import Service, private


Expand All @@ -12,62 +14,44 @@ class Config:
@private
async def authenticate(self, username, password):
if '@' in username:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have PAM library, you can just use that for all authentication other than root. You just need to have a special pam config file that you regenerate when directory services settings are changed (for middleware auth).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using PAM file will also allow us to avoid a datastore.query in potentially hot code path.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sample pam file for etc_files if you're not familiar (note didn't test this particular one):

#
# /etc/pam.d/middleware - authentication settings for middlewared service
#

<%namespace name="pam" file="pam.inc.mako" />\
<%
        dsp = None
        if middleware.call_sync('datastore.config', 'system.settings')['stg_ds_auth']:
            dsp = pam.getDirectoryServicePam(middleware=middleware, render_ctx=render_ctx)
%>\

% if dsp:
${dsp.pam_auth()}
% endif

auth    [success=1 default=ignore]      pam_unix.so obscure sha512
# here's the fallback if no module succeeds
auth    requisite                       pam_deny.so
# prime the stack with a positive return value if there isn't one already;
# this avoids us returning an error just because nothing sets a success code
# since the modules above will each just jump around
auth    required                        pam_permit.so
# and here are more per-package modules (the "Additional" block)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some cases we need to allow root user login even if it has no password set in /etc/shadow, is there a method to do this with pam?

Copy link
Contributor

@anodos325 anodos325 Nov 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

root would be the one exception to this. I can't think of a clever way to avoid this. Maybe if pam.authenticate() fails, we can have special error handling for root that will fallback to comparing the password hash with what's in our db (using same logic agreed-to earlier).

You'll probably want to specify nodelay for pam_unix.so (assuming we don't want 2+ second delays on failure path).

Copy link
Contributor

@anodos325 anodos325 Nov 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory we could maintain a berkley db with local overrides (like root password) and place pam_userdb.so ahead of pam_unix.so.

if (await self.middleware.call('datastore.config', 'system.settings'))['stg_ds_auth']:
return await self.ds_authenticate(username, password)
else:
return None
username = username.split('@')[0]
local = False
else:
return await self.local_authenticate(username, password)
local = True

@private
async def local_authenticate(self, username, password):
try:
user = await self.middleware.call(
if username == 'root' and await self.middleware.call('privilege.always_has_root_password_enabled'):
root = await self.middleware.call(
'datastore.query',
'account.bsdusers',
[
('username', '=', username),
('locked', '=', False),
],
[('username', '=', 'root')],
{'get': True, 'prefix': 'bsdusr_'},
)
except IndexError:
return None

if user['unixhash'] in ('x', '*'):
return None

if user['password_disabled']:
if user['username'] == 'root':
if not await self.middleware.call('privilege.always_has_root_password_enabled'):
return None
else:
if root['unixhash'] in ('x', '*'):
return None

if not hmac.compare_digest(crypt.crypt(password, user['unixhash']), user['unixhash']):
if not hmac.compare_digest(crypt.crypt(password, root['unixhash']), root['unixhash']):
return None
elif not await self.middleware.call('auth.libpam_authenticate', username, password):
return None

return await self.authenticate_local_user(user['id'], username)
return await self.authenticate_user({'username': username}, local)

@private
async def authenticate_local_user(self, user_id, username):
gids = {
member['bsdgrpmember_group']['bsdgrp_gid']
for member in await self.middleware.call(
'datastore.query',
'account.bsdgroupmembership',
[['bsdgrpmember_user', '=', user_id]],
)
}

return await self.common_authenticate(username, 'local_groups', gids)
def libpam_authenticate(self, username, password):
p = pam.pam()
return p.authenticate(username, password, service='middleware')

@private
async def ds_authenticate(self, username, password):
return None # FIXME
async def authenticate_user(self, query, local):
try:
user = await self.middleware.call('user.get_user_obj', {**query, 'get_groups': True})
except KeyError:
return None

groups = set(user['grouplist'])
groups_key = 'local_groups' if local else 'ds_groups'

@private
async def common_authenticate(self, username, groups_key, groups):
privileges = [
privilege for privilege in await self.middleware.call('datastore.query', 'account.privilege')
if set(privilege[groups_key]) & groups
Expand All @@ -76,7 +60,7 @@ async def common_authenticate(self, username, groups_key, groups):
return None

return {
'username': username,
'username': user['pw_name'],
'privilege': await self.middleware.call('privilege.compose_privilege', privileges),
}

Expand Down
3 changes: 3 additions & 0 deletions src/middlewared/middlewared/plugins/etc.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ class EtcService(Service):
{'type': 'mako', 'path': 'pam.d/common-session'},
]
},
'pam_middleware': [
{'type': 'mako', 'path': 'pam.d/middleware'},
],
'ftp': [
{'type': 'mako', 'path': 'proftpd/proftpd.conf',
'local_path': 'local/proftpd.conf'},
Expand Down
10 changes: 7 additions & 3 deletions src/middlewared/middlewared/plugins/idmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,12 @@ async def get_sssd_low_range(self, domain, sssd_config=None, seed=0xdeadbeef):

return (hash % max_slices) * range_size + range_size

@private
async def flush_gencache(self):
gencache_flush = await run(['net', 'cache', 'flush'], check=False)
if gencache_flush.returncode != 0:
raise CallError(f'Attempt to flush gencache failed with error: {gencache_flush.stderr.decode().strip()}')

@accepts()
@job(lock='clear_idmap_cache', lock_queue_size=1)
async def clear_idmap_cache(self, job):
Expand Down Expand Up @@ -441,9 +447,7 @@ async def clear_idmap_cache(self, job):
except Exception:
self.logger.debug("Failed to remove winbindd_cache.tdb.", exc_info=True)

gencache_flush = await run(['net', 'cache', 'flush'], check=False)
if gencache_flush.returncode != 0:
raise CallError(f'Attempt to flush gencache failed with error: {gencache_flush.stderr.decode().strip()}')
await self.flush_gencache()

await self.middleware.call('service.start', 'idmap')
if smb_started:
Expand Down
3 changes: 3 additions & 0 deletions src/middlewared/middlewared/plugins/system_general/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,9 @@ async def do_update(self, data):
if config['crash_reporting'] != new_config['crash_reporting']:
await self.middleware.call('system.general.set_crash_reporting')

if config['ds_auth'] != new_config['ds_auth']:
await self.middleware.call('etc.generate', 'pam_middleware')

await self.middleware.call('service.start', 'ssl')

if rollback_timeout is not None:
Expand Down
8 changes: 4 additions & 4 deletions src/middlewared/middlewared/plugins/test/mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class TestService(Service):
class Config:
private = True

async def set_mock(self, name, description):
async def set_mock(self, name, args, description):
if isinstance(description, str):
exec(description)
try:
Expand All @@ -25,10 +25,10 @@ def method(*args, **kwargs):
else:
raise CallError("Invalid mock declaration")

self.middleware.set_mock(name, method)
self.middleware.set_mock(name, args, method)

async def remove_mock(self, name):
self.middleware.remove_mock(name)
async def remove_mock(self, name, args):
self.middleware.remove_mock(name, args)

# Dummy methods to mock for internal infrastructure testing (i.e. jobs manager)

Expand Down
2 changes: 1 addition & 1 deletion src/middlewared/middlewared/test/integration/utils/call.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@


def call(*args, **kwargs):
with client() as c:
with client(**kwargs.pop("client_kwargs", {})) as c:
return c.call(*args, **kwargs)