Conversation
- Add llng_ssh_cert_info_t structure to send SSH certificate details to LLNG server during authorization (#10, #12) - Add llng_permissions_t structure to parse sudo_allowed/sudo_nopasswd from /pam/authorize response (#13) - Add llng_authorize_user_with_cert() function for SSH cert auth - Detect service type (sshd vs sudo) in pam_sm_acct_mgmt (#11) - Extract SSH certificate info from PAM environment (SSH_USER_AUTH, SSH_CERT_* variables) - Deny sudo access when permissions.sudo_allowed is false - Store LLNG_SUDO_ALLOWED and LLNG_SUDO_NOPASSWD in PAM environment - Add unit tests for new structures and functions Closes: #10, #11, #12, #13
Implements issue #36: SSH/sudo authorization when LLNG server is unavailable. PAM module changes: - Add auth_cache.c/h: AES-256-GCM encrypted cache with machine-id derived key - Add config options: auth_cache_enabled, auth_cache_dir, auth_cache_force_online - Integrate cache fallback in pam_sm_acct_mgmt() when server unreachable - Store permissions (sudo_allowed, sudo_nopasswd) and user attributes in cache - Support force-online file to bypass cache for specific users LLNG server changes (in llng.patch): - Add pamAccessOfflineEnabled (boolOrExpr) config option - Add pamAccessOfflineTtl (int, default 86400) config option - Add offline{} block in /pam/authorize response with enabled and ttl - Include user attributes from pamAccessExportedVars in response RPM spec fix: - Add missing NSS module files to package Tests: - Add test_auth_cache.c with 12 unit tests - Add 78-PamAccess-Offline.t with 77 tests for LLNG side Closes: #36
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Security fixes identified by CodeQL and GitHub Copilot: - tests/test_config.c: Use open() with mode 0600 instead of fopen()+chmod() to avoid race condition between file creation and permission setting - tests/test_rate_limiter.c, tests/test_secret_store.c: Replace lstat()+unlink() with fstatat()+unlinkat() using directory fd to prevent TOCTOU attacks in recursive directory removal - src/auth_cache.c: In auth_cache_force_online(), open file first then use fstat() on fd instead of stat() on path to avoid race condition - .github/workflows/ci.yml: Add explicit permissions block (contents: read) to restrict GITHUB_TOKEN scope per least privilege principle Fixes CodeQL alerts #1-8 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements SSH certificate-based authentication and offline authorization mode with caching capabilities, enabling secure bastion server functionality.
Key Changes
- Added SSH certificate extraction and validation from PAM environment variables
- Implemented authorization caching system with AES-256-GCM encryption for offline mode
- Enhanced authorization flow with sudo permission checking and force-online capability
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pam_llng.c | Added SSH cert extraction, offline cache integration, and sudo authorization checks |
| src/llng_client.c | Implemented cert-aware authorization endpoint and refactored internal authorization logic |
| src/auth_cache.c | New authorization cache implementation with encryption and TTL management |
| src/config.c | Added configuration options for authorization cache and force-online settings |
| include/llng_client.h | Added structures for permissions, offline settings, and SSH certificate info |
| include/config.h | Added auth cache configuration fields to config structure |
| include/auth_cache.h | New header defining authorization cache API |
| tests/test_llng_client.c | New comprehensive unit tests for LLNG client structures and functions |
| tests/test_auth_cache.c | New unit tests for authorization cache functionality |
| tests/test_config.c | Updated file creation to use secure O_CREAT with permissions from start |
| tests/test_secret_store.c | Refactored directory removal to use openat/unlinkat for TOCTOU safety |
| tests/test_rate_limiter.c | Refactored directory removal to use openat/unlinkat for TOCTOU safety |
| tests/CMakeLists.txt | Added build configuration for new test executables |
| CMakeLists.txt | Added auth_cache.c to PAM module sources |
| rpm/pam-llng.spec | Added NSS library and man page references |
| .github/workflows/ci.yml | Restricted GITHUB_TOKEN permissions to read-only |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Security improvements:
- Add safe_json_strdup() helper to prevent NULL pointer dereference in JSON parsing
- Add O_NOFOLLOW flag to cache file writes to prevent symlink attacks
- Change directory permissions from 0755 to 0700 for sensitive directories
- Add MAX_SSH_ENV_LEN (4096) validation for SSH environment variables
Code quality improvements:
- Add llng_response_init() function for safe response initialization
- Refactor llng_authorize_user() to delegate to internal function (removes code duplication)
- Replace magic number 86400 with DEFAULT_OFFLINE_CACHE_TTL constant
- Replace system("rm -rf") with safe recursive removal using fstatat()/unlinkat()
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Security improvements identified by specialized security agents: - Add O_NOFOLLOW to all open() calls with ELOOP handling to prevent symlink attacks - Fix TOCTOU race conditions using open+fstat+fdopen pattern in config and token loading - Add input validation: MAX_TOKEN_LENGTH (8192), MAX_URL_LENGTH (512) - Sanitize GECOS field to prevent /etc/passwd corruption - Replace /dev/urandom with OpenSSL RAND_bytes() for cryptographically secure random generation - Check file is regular (S_ISREG) after opening to prevent device/FIFO attacks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Apply CodeQL recommendation: set explicit permissions block on each job for principle of least privilege (contents: read). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.