Skip to content

auth: introducing PAM account management for daemon - #1109

Open
seks99x wants to merge 5 commits into
RsyncProject:masterfrom
seks99x:rsync-pam-management
Open

seks99x wants to merge 5 commits into
RsyncProject:masterfrom
seks99x:rsync-pam-management

Conversation

@seks99x

@seks99x seks99x commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

This introduces PAM (Pluggable Authentication Modules) support specifically targeted at account management in the rsync daemon, alongside a structural refactor of the authentication codebase (#968) .

Following @tridge's architectural suggestions during the initial PAM discussions on discord, which was meant to be made firstly on the VFS branch , the authentication logic has been modularized into a dedicated subsystem. Created a new auth/ directory for authentication-related code, Moved authenticate.c to auth/authenticate.c, Introduced auth/pam.c and auth/auth.h to encapsulate the new PAM function (also supporting future changes) and cleanly expose prototypes.

PAM Account Management (pam_acct_mgmt):
To avoid transmitting credentials over the wire, this PAM integration is strictly limited to account management till now.
Implemented the use pam = yes configuration directive for rsyncd.conf which is off by default.
The daemon continues to use the standard MD5 challenge-response for cryptographic authentication. Once the MD5 hash is validated, the daemon calls pam_acct_mgmt() to ensure the underlying system account is active, valid, and not locked/expired before granting session access.

Testing & Coverage:
Expanded testsuite/daemon-auth_test.py to validate the new code paths across the daemon. Verified that non-existent system users (who pass the MD5 check but fail PAM validation) are safely rejected. Verified that real, active system users successfully pass both the MD5 and PAM management checks. I used Samba's pam_wrapper alongside a mock plugin (testsuite/pam/pam_mock.c). Used pkg-config to dynamically find the exact library path. If pam_wrapper or pkg-config is missing, or if running on macOS, the PAM tests are gracefully skipped.

I'm also trying to figure out an optimal way , which we could apply later or on this pr, that we could integrate PAM as an authentication method without changing a lot of the core protocol and transmitting the credentials safely.

This still needs some more work on the documentation, and maybe fixes for other stuff I missed or implemented incorrectly.

@seks99x
seks99x force-pushed the rsync-pam-management branch 9 times, most recently from 21e78d4 to 0e23ae7 Compare September 29, 2026 14:27
@seks99x
seks99x force-pushed the rsync-pam-management branch from 0e23ae7 to 3763d0d Compare September 29, 2026 14:46
@steadytao

steadytao commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Do note that PAM should be opt-in during its initial rollout. Also, for those looking at this PR -- this feature will more or less allow PAM account-policy; at least for now.

@steadytao

Copy link
Copy Markdown
Member

Ah I also don't need co-author on your commits. Primarily to ensure they're verified.

@seks99x
seks99x force-pushed the rsync-pam-management branch from 2adb892 to 647a83c Compare September 30, 2026 13:31
@seks99x
seks99x force-pushed the rsync-pam-management branch from 647a83c to 6779d97 Compare September 30, 2026 13:32
@seks99x

seks99x commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

I modified it to be as an optional argument of the compilation phase (./configure —enable-pam). The CI workflows will need to have pam_wrapper , pam (MacOS,solaris,netbsd,openbsd already do have pam only installed) and pkg-config installed. It also needs to be configured to compile with —enable-pam so it can actually run the PAM test case.
I’m also feeling its better to make the PAM test independent from the default auth so i can track well if any is really skipping/failing not gracefully skipping.

@steadytao steadytao left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Keep authenticate.c in place, document use pam in rsyncd.conf.5.md and add a CI job configured with --enable-pam (let me know if I will have to do that, I may) -- Ah. Do note to document it as account-policy verification rather than PAM auth.

Comment thread auth/pam.c Outdated
Comment on lines +27 to +37
static int rsync_pam_conv(int num_msg, PAM_MSG_CONST struct pam_message **msg,
struct pam_response **resp, void *appdata_ptr)
{
/* Suppress unused variable warnings */
(void)num_msg;
(void)msg;
(void)resp;
(void)appdata_ptr;

return PAM_CONV_ERR;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This rejects informational PAM messages. Accept PAM_TEXT_INFO and PAM_ERROR_MSG with empty responses and reject only interactive prompts.

Comment thread auth/authenticate.c Outdated
Comment on lines +423 to +424
if (!err && use_pam) {
if (am_root != 1)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

PAM does not universally require root. Call pam_acct_mgmt and let the application policy return permission errors.

Comment on lines +128 to +131
res = subprocess.run([pkg_config, "--libs", "pam_wrapper"], capture_output=True, text=True, check=True)
discovered_path = res.stdout.strip()
if os.path.exists(discovered_path):
pam_wrapper_so = discovered_path

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

pkg-config --libs returns linker flags rather than the path so this test normally skips. Resolve the actual shared object and add a PAM-enabled CI job which fails if the test skips -- let me know if you require me to do that, I think you might :D

@seks99x seks99x Oct 1, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

pkg-config --libs return with the path not the flags on pam_wrapper.

pkg-config --libs pam_wrapper
/usr/lib/x86_64-linux-gnu/libpam_wrapper.so

pam_wrapper packages can be installed like this

## pam_wrapper.pc
modules=/usr/lib/x86_64-linux-gnu/pam_wrapper

Name: pam_wrapper
Description: The pam_wrapper library
Version: 1.1.8
Libs: /usr/lib/x86_64-linux-gnu/libpam_wrapper.so                                              

I initially tried using ctypes.util.find_library("pam_wrapper") but it didn't resolve properly, which is why I fell back to the pkg-config approach. Since .pc files can differ from machine to another, I'll push an update that dynamically handle two cases. If --libs returns an absolute path, it will use it; otherwise it will fallback to --variable=libdir to construct the correct path (which should be the standard).
Regarding CI I get an authorization error trying to modify anything under .github/workflows/, so I can't add it myself :(

Comment thread testsuite/pam/pam_mock.c Outdated
Comment on lines +6 to +7
#include <security/pam_appl.h>
#include <security/pam_modules.h>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Configure accepts alternate PAM header layouts but this mock hard-codes the Linux paths. Use the configure results for both required headers.

Comment on lines +189 to +191
proc = push(bad, target_module='pam_auth', user='tuser')
if proc.returncode == 0 or "PAM: Account validation successful for user" in log_content:
test_fail("PAM module unexpectedly succeeded with the wrong password (fake user)")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

log_content predates this request so the test does not prove PAM was skipped after a bad password. Reload the log or record mock invocation count.

@seks99x

seks99x commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

@steadytao do you mean you want a revert back and remove the auth/ dir? I'll append all in the authenticate.c , or do you prefer a pam.c / pam.h file?

@seks99x
seks99x force-pushed the rsync-pam-management branch 2 times, most recently from 6f90d08 to 8bf6af6 Compare October 1, 2026 11:40
@steadytao

Copy link
Copy Markdown
Member

I only meant to keep authenticate.c in its existing location rather than moving the whole authentication subsystem. The current layout is fine. There is no need to move the PAM code again.

@seks99x
seks99x force-pushed the rsync-pam-management branch from 8bf6af6 to e834eb6 Compare October 1, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants