Skip to content

Commit f47b611

Browse files
mergify[bot]mergify[bot]
authored
Merge pull request #23 from grondo/issue#22
pam: fix `pam_flux.so` interaction with systemd-user PAM service
2 parents f1f2343 + 61eba96 commit f47b611

4 files changed

Lines changed: 107 additions & 25 deletions

File tree

‎doc/man8/pam_flux.rst‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,21 @@ session module and are handled by subsequent modules::
143143
NOTES
144144
=====
145145

146+
systemd-user Service
147+
--------------------
148+
149+
``pam_flux.so`` automatically skips when invoked from the systemd-user PAM
150+
service (the service systemd uses to start ``user@$UID.service``). This
151+
prevents a circular dependency: the systemd-user stack runs during the
152+
startup of ``user@$UID.service``, but ``pam_flux.so`` needs to query and
153+
interact with that service. The module returns ``PAM_IGNORE`` for both
154+
account and session functions when ``PAM_SERVICE`` is ``systemd-user``.
155+
156+
In practice, ``pam_flux.so`` should not normally appear in
157+
``/etc/pam.d/systemd-user`` anyway. However, if your site uses shared PAM
158+
includes (e.g., ``@include common-account``) that bring ``pam_flux.so`` into
159+
the systemd-user stack, this automatic skip ensures correct behavior.
160+
146161
cgroup v2 Requirement
147162
---------------------
148163

‎src/pam/pam_flux.c‎

Lines changed: 35 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,8 @@ static int parse_options (pam_handle_t *pamh,
407407
* The service is started by the Flux prolog when a job begins and stopped
408408
* by housekeeping when the last job ends, so inactive means no active job.
409409
*
410-
* Returns 0 if service is running (ActiveState=active, SubState=running).
410+
* Returns 0 if service is active or activating (mirrors systemd's
411+
* UNIT_IS_ACTIVE_OR_ACTIVATING macro).
411412
* Returns -1 if service is not running or an error occurred with specific
412413
* reason set in errmsg
413414
*/
@@ -423,7 +424,6 @@ static int check_user_service_active (pam_handle_t *pamh,
423424
const char *unit_path_raw = NULL;
424425
char *unit_path = NULL;
425426
char *active_state = NULL;
426-
char *sub_state = NULL;
427427
int rc = -1;
428428

429429
*errmsg = "Unable to determine unit state";
@@ -507,29 +507,17 @@ static int check_user_service_active (pam_handle_t *pamh,
507507
}
508508
sd_bus_error_free (&error);
509509

510-
/* Get SubState property.
510+
/* Mirror systemd's own UNIT_IS_ACTIVE_OR_ACTIVATING macro when
511+
* checking for an active or activating user@UID service:
511512
*/
512-
if (sd_bus_get_property_string (bus,
513-
"org.freedesktop.systemd1",
514-
unit_path,
515-
"org.freedesktop.systemd1.Unit",
516-
"SubState",
517-
&error,
518-
&sub_state) < 0) {
519-
pam_syslog (pamh,
520-
LOG_ERR,
521-
"failed to get SubState for %s: %s",
522-
unit_name,
523-
error.message ? error.message : "unknown error");
524-
goto out;
525-
}
526-
527513
if (strcmp (active_state, "active") == 0
528-
&& strcmp (sub_state, "running") == 0) {
514+
|| strcmp (active_state, "activating") == 0
515+
|| strcmp (active_state, "reloading") == 0
516+
|| strcmp (active_state, "refreshing") == 0) {
529517
if (debug)
530518
pam_syslog (pamh,
531519
LOG_INFO,
532-
"%s is active",
520+
"%s is active or activating",
533521
unit_name);
534522
rc = 0;
535523
}
@@ -540,18 +528,16 @@ static int check_user_service_active (pam_handle_t *pamh,
540528
*/
541529
pam_syslog (pamh,
542530
LOG_INFO,
543-
"%s not running: ActiveState=%s SubState=%s",
531+
"%s not active or activating: ActiveState=%s",
544532
unit_name,
545-
active_state,
546-
sub_state);
547-
*errmsg = "unit not running";
533+
active_state);
534+
*errmsg = "unit not active or activating";
548535
rc = -1;
549536
}
550537

551538
out:
552539
free (unit_path);
553540
free (active_state);
554-
free (sub_state);
555541
sd_bus_message_unref (reply);
556542
sd_bus_error_free (&error);
557543
sd_bus_unref (bus);
@@ -768,6 +754,7 @@ PAM_EXTERN int
768754
pam_sm_acct_mgmt (pam_handle_t *pamh, int flags, int argc, const char **argv)
769755
{
770756
const char *user;
757+
const char *service = NULL;
771758
uid_t uid;
772759
int auth = PAM_PERM_DENIED;
773760
flux_auth_t result;
@@ -779,6 +766,17 @@ pam_sm_acct_mgmt (pam_handle_t *pamh, int flags, int argc, const char **argv)
779766
if (parse_options (pamh, &opts, argc, argv) < 0)
780767
return PAM_SYSTEM_ERR;
781768

769+
/* Skip systemd-user service - it's starting user@UID.service itself.
770+
* Checking the slice from within that service's own startup is circular
771+
* and not meaningful.
772+
*/
773+
pam_get_item (pamh, PAM_SERVICE, (const void **) &service);
774+
if (service && strcmp (service, "systemd-user") == 0) {
775+
if (opts.debug)
776+
pam_syslog (pamh, LOG_INFO, "skipping for systemd-user service");
777+
return PAM_IGNORE;
778+
}
779+
782780
result = flux_check_user (pamh, &opts, uid);
783781
if (result != FLUX_AUTH_DENIED) {
784782
/* User has a local job or allow-guest-user is true. In either case
@@ -824,6 +822,7 @@ pam_sm_open_session (pam_handle_t *pamh,
824822
{
825823
uid_t uid;
826824
const char *user;
825+
const char *service = NULL;
827826
const void *pam_flux_authorized = NULL;
828827
int manage_slice;
829828
struct options opts = {
@@ -836,6 +835,17 @@ pam_sm_open_session (pam_handle_t *pamh,
836835
if (parse_options (pamh, &opts, argc, argv) < 0)
837836
return PAM_SESSION_ERR;
838837

838+
/* Skip systemd-user service - it's starting user@UID.service itself.
839+
* Creating a scope under user-UID.slice from within that service's own
840+
* startup is circular and not meaningful.
841+
*/
842+
pam_get_item (pamh, PAM_SERVICE, (const void **) &service);
843+
if (service && strcmp (service, "systemd-user") == 0) {
844+
if (opts.debug)
845+
pam_syslog (pamh, LOG_INFO, "skipping for systemd-user service");
846+
return PAM_IGNORE;
847+
}
848+
839849
/* Session management decision table:
840850
*
841851
* pam_flux_authorized is a sentinel set by pam_sm_acct_mgmt when it

‎t/t0001-pam_flux.t‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,4 +131,32 @@ test_expect_success 'pam_flux: access denied if not rank 0 of job' '
131131
flux cancel $id &&
132132
flux job wait-event -vt 15 $id clean
133133
'
134+
test_expect_success 'pam_flux: create systemd-user PAM stack' '
135+
cat <<-EOF >systemd-user
136+
auth required pam_localuser.so
137+
account required ${PAM_FLUX_PATH} debug
138+
account required pam_permit.so
139+
EOF
140+
'
141+
test_expect_success 'pam_flux: module skips systemd-user service (no job)' '
142+
LD_PRELOAD=libpam_wrapper.so \
143+
PAM_WRAPPER=1 \
144+
PAM_WRAPPER_DEBUGLEVEL=2 \
145+
PAM_WRAPPER_SERVICE_DIR=$(pwd) \
146+
${PAMTEST} -v -s systemd-user -u ${USER} >systemd-user.out 2>&1 &&
147+
test_debug "cat systemd-user.out" &&
148+
grep "skipping for systemd-user service" systemd-user.out
149+
'
150+
test_expect_success 'pam_flux: module skips systemd-user service (with job)' '
151+
jobid=$(flux submit --wait-event=alloc sleep 300) &&
152+
LD_PRELOAD=libpam_wrapper.so \
153+
PAM_WRAPPER=1 \
154+
PAM_WRAPPER_DEBUGLEVEL=2 \
155+
PAM_WRAPPER_SERVICE_DIR=$(pwd) \
156+
${PAMTEST} -v -s systemd-user -u ${USER} >systemd-user2.out 2>&1 &&
157+
test_debug "cat systemd-user2.out" &&
158+
grep "skipping for systemd-user service" systemd-user2.out &&
159+
flux cancel $jobid &&
160+
flux job wait-event -vt 15 $jobid free
161+
'
134162
test_done

‎t/t0003-pam-session-tests.t‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,35 @@ test_expect_success 'lock-dir-perms: cancel last test job' '
275275
test_expect_success 'lock-dir-perms: cleanup other-writable directory' '
276276
sudo rm -rf bad-lock-dir2
277277
'
278+
test_expect_success 'systemd-user: create PAM stack' '
279+
cat <<-EOF >systemd-user
280+
auth required pam_localuser.so
281+
account sufficient pam_succeed_if.so uid < 500
282+
account sufficient ${PAM_FLUX_PATH}
283+
account required pam_permit.so
284+
session requisite ${PAM_FLUX_PATH} debug
285+
session required pam_unix.so
286+
EOF
287+
'
288+
test_expect_success 'systemd-user: submit test job' '
289+
jobid=$(submit_as_guest 5m sleep 300) &&
290+
flux job wait-event $jobid start
291+
'
292+
test_expect_success 'systemd-user: session skips systemd-user service' '
293+
sudo FLUX_URI=${FLUX_URI} \
294+
LD_PRELOAD=libpam_wrapper.so \
295+
PAM_WRAPPER=1 \
296+
PAM_WRAPPER_DEBUGLEVEL=2 \
297+
PAM_WRAPPER_SERVICE_DIR=$(pwd) \
298+
${PAMTEST} -v -S -s systemd-user -u ${TEST_USER} \
299+
>systemd-user.out 2>&1 &&
300+
test_debug "cat systemd-user.out" &&
301+
grep "skipping for systemd-user service" systemd-user.out
302+
'
303+
test_expect_success 'systemd-user: cancel test job' '
304+
flux cancel $jobid &&
305+
flux job wait-event -vt 20 $jobid clean
306+
'
278307
test_expect_success 'cleanup test scopes' '
279308
reset_test_scopes
280309
'

0 commit comments

Comments
 (0)