Skip to content

Conversation

@hyunwoo-alps
Copy link
Contributor

@hyunwoo-alps hyunwoo-alps commented Aug 6, 2025

Summary

Implement centralized sudo privilege verification through MFA authentication system.

Key Features

1. Auth Manager (Unix Socket Server)

  • Creates Unix domain socket server at /var/run/alpamon/auth.sock
  • Handles check_user requests from pam_alpamon.so
  • Handles sudo_approval requests from alpacon_approval.so
  • Manages PID-to-session mapping for Alpacon users

2. MFA Request Flow

  • Detects sudo command execution via PAM module
  • Routes MFA requests to alpacon-server for authenticated users
  • Returns {success: false, reason: "session missing"} for non-Alpacon users
  • Delivers MFA responses back to PAM module

3. User Management

  • Automatically adds new Alpacon users to sudo group on creation
  • Enables sudo access for Alpacon-managed users

4. Development Improvements

  • Added local test user in Dockerfile for convenient testing
  • Integrated PAM module build tools for easier development

Integration Points

  • alpacon-server: MFA verification endpoint
  • pam_alpamon.so: User authentication module
  • alpacon_approval.so: Sudo approval plugin

Related Issues

Closes #99

Copilot AI review requested due to automatic review settings August 6, 2025 07:44

This comment was marked as outdated.

@hyunwoo-alps hyunwoo-alps reopened this Aug 6, 2025
@hyunwoo-alps hyunwoo-alps requested a review from junho226 August 6, 2025 08:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements sudo privilege verification and centralized management via MFA authentication by adding an Auth Manager component that handles sudo requests through a Unix domain socket and communicates with the alpacon server for MFA approval.

  • Introduces Auth Manager as a singleton service that maps PTY sessions to process IDs for sudo request handling
  • Adds Unix domain socket communication between PAM module and Auth Manager for sudo approval workflow
  • Extends WebSocket client to handle sudo approval responses from alpacon server and automatically adds new users to sudo group

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/runner/auth_manager.go New Auth Manager implementation with sudo request handling and MFA communication
pkg/runner/pty.go Adds PID-to-session mapping when PTY sessions are created
pkg/runner/command.go Adds sudo group assignment for new users and sudo approval response handling
pkg/runner/client.go Extends WebSocket client to process sudo approval responses
cmd/alpamon/command/root.go Integrates Auth Manager startup and shutdown into main application lifecycle
configs/tmpfile.conf Adds directory configuration for auth socket
Dockerfiles/ubuntu/22.04/Dockerfile Adds development tools and test user for sudo functionality
Dockerfiles/ubuntu/22.04/entrypoint.sh Creates auth socket directory at runtime

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

hyunwoo-alps and others added 8 commits August 19, 2025 10:45
…nt-via-mfa-authentication' of https://github.com/alpacax/alpamon into 99-sudo-privilege-verification-and-centralized-management-via-mfa-authentication
…tion-and-centralized-management-via-mfa-authentication
- Add alpamon-pam as recommended dependency in .goreleaser.yaml
  - Debian/Ubuntu: recommends alpamon-pam
  - CentOS/RHEL: recommends alpamon-pam
- Update README.md with PAM module documentation
  - Add installation instructions with/without PAM module
  - Document PAM configuration steps for /etc/pam.d/sudo and /etc/sudo.conf
  - Add note about Alpamon service requirement for PAM authentication
…tion-and-centralized-management-via-mfa-authentication
Convert Korean comment in Dockerfile to English for better maintainability.

Changes:
- Dockerfiles/ubuntu/22.04/Dockerfile: Convert GOARCH architecture comment
Add AuthManager for centralized sudo privilege verification:
- Unix domain socket server (/var/run/alpamon/auth.sock)
- Handle check_user requests from pam_alpamon.so
- Handle sudo_approval requests from alpacon_approval.so
- Distinguish Alpacon users (pidToSessionMap) vs local users (localSudoRequests)
- Retry logic with exponential backoff for WebSocket communication
- Response routing back to PAM/sudo plugin via Unix socket

Security improvements:
- Root-only socket permissions (0600)
- 30-second timeout to prevent DoS
- Request ID based mapping for concurrent requests
- Proper cleanup on timeout and connection errors

Integration:
- WebSocket communication with alpacon-server
- Coordinate with PtyClient session management
- Support both authenticated and local user approval flows
…atting

Replace Msg(fmt.Sprintf(...)) with Msgf(...) to fix staticcheck SA1006 linting error.
This resolves the golangci-lint failure in CI while maintaining the same functionality.
Use Str() instead of Err(fmt.Errorf()) to fix staticcheck SA1006 error.
The result variable is already a string, so we use structured logging with Str().
@junho226 junho226 changed the title 99 sudo privilege verification and centralized management via mfa authentication feat: implement MFA-based sudo approval system Nov 25, 2025
@junho226 junho226 requested review from geunwoonoh and removed request for junho226 November 25, 2025 08:41
- Fix critical mutex double unlock bug in HandleSudoApprovalResponse
  - Previously unlocked mutex inside loop, causing panic on second unlock
  - Now unlocks once after checking both alpacon and local requests

- Improve connection lifecycle management in handleSudoRequest
  - Remove defer close() to prevent double-close with manual cleanup
  - Explicitly close connections after check_user requests
  - Document that sudo_approval connections are managed by response handlers

- Add default case for unknown request types with proper cleanup
- Improve timeout handling with explicit service shutdown cleanup
- Rename is_alpcon_user to is_alpacon_user for consistency
  - SudoApprovalRequest, SudoApprovalResponse structs
  - MFAResponse, IsAlpconResponse structs
@junho226 junho226 marked this pull request as draft November 26, 2025 07:57
…tion-and-centralized-management-via-mfa-authentication
- Remove hardcoded sudo group addition in adduser
- Sudo privilege is now controlled by alpacon-server via gids
- Add ControlClient to handle control WebSocket connection
- Refactor client.go to separate control logic into control_client.go
- Support sudo_approval request/response via control channel
@junho226 junho226 marked this pull request as ready for review December 1, 2025 10:22
@junho226 junho226 self-assigned this Dec 1, 2025
- Rename variables to camelCase (unix_conn -> unixConn, sudo_approval_req -> sudoApprovalReq)
- Add BaseRequest struct for type-safe request parsing
- Extract createSendOperation method from sendSudoRequestWithRetry
- Add completion channels for proper timeout handling
- Close connection after sendSudoApprovalResponse in cleanupTimeoutRequest
- Call RemovePIDSessionMapping when PtyClient disconnects
geunwoonoh
geunwoonoh previously approved these changes Dec 23, 2025
Copy link
Collaborator

@geunwoonoh geunwoonoh left a comment

Choose a reason for hiding this comment

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

I think we can merge this PR as soon as the conflicts are resolved. Thank you!

…tion-and-centralized-management-via-mfa-authentication
@junho226 junho226 merged commit 66268e2 into main Dec 23, 2025
5 checks passed
@junho226 junho226 deleted the 99-sudo-privilege-verification-and-centralized-management-via-mfa-authentication branch December 23, 2025 02:13
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.

Sudo Privilege Verification and Centralized Management via MFA Authentication

5 participants