NAS-118851 / 23.10 / API authentication using Directory Services #10054
Conversation
bugclerk
changed the title
src/middlewared/Makefile
Outdated
| @@ -23,6 +23,7 @@ install_client: | |||
|
|
|||
| install_test: | |||
| bash install-dev-tools | |||
| apt install -y python3-pampy | |||
There was a problem hiding this comment.
This is temporary to be able to run CI tests from this branch
| @@ -13,7 +15,7 @@ class Config: | |||
| async def authenticate(self, username, password): | |||
| if '@' in username: | |||
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
Using PAM file will also allow us to avoid a datastore.query in potentially hot code path.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
In theory we could maintain a berkley db with local overrides (like root password) and place pam_userdb.so ahead of pam_unix.so.
themylogin
force-pushed
the
rootless-login-ds
branch
2 times, most recently
from
d55b334
to
a2c1035
Compare
| async def ds_authenticate(self, username, password): | ||
| return None # FIXME | ||
| async def authenticate_user(self, query, local): | ||
| user = await self.middleware.call('user.get_user_obj', {**query, 'get_groups': True}) |
There was a problem hiding this comment.
Do we need to handle KeyError if for some reason user doesn't exist?
| @@ -279,6 +279,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') | |||
There was a problem hiding this comment.
Should this be pam_middleware
There was a problem hiding this comment.
Or did you intend to move the middleware pam into the generic pam etc group?
bugclerk
changed the title
themylogin
force-pushed
the
rootless-login-ds
branch
from
08bf61b
to
7565a45
Compare
|
time 2:00 |
themylogin
force-pushed
the
rootless-login-ds
branch
2 times, most recently
from
60023c2
to
b619579
Compare
…oot_password_enabled)
themylogin
force-pushed
the
rootless-login-ds
branch
from
b619579
to
3600c6e
Compare
If group is removed from file then NSS query will fall through to nss_winbind, which may have cached result. In principle, winbind shouldn't be giving a response here, but fix will be more involved and so for now we will simply flush cached entries on user or group removal.
|
time 4:00 |
|
Don't forget to squash this. LGTM, we can deal with any issues that come up in future commits. Don't want to let this bitrot again. :) |
No description provided.