fix: Add security and reliability improvements
1. File locking for config operations
- add_server_to_config() and remove_server_from_config() now use
exclusive file locking (fcntl.LOCK_EX) to prevent race conditions
- Prevents data loss from concurrent modifications
2. Bulk server limit
- Add MAX_BULK_SERVERS = 10 constant
- haproxy_add_servers() now rejects requests exceeding limit
- Prevents potential DoS via large payloads
3. HAProxy command response validation
- Add haproxy_cmd_checked() helper function
- Validates responses for error indicators (No such, not found, etc.)
- State-modifying commands now properly detect and report failures
- Read-only commands continue using haproxy_cmd()
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -82,6 +82,7 @@ STARTUP_RETRY_COUNT = 10 # HAProxy ready check retries
|
||||
STATE_MIN_COLUMNS = 19 # Minimum columns in HAProxy server state output
|
||||
SOCKET_TIMEOUT = 5 # seconds for HAProxy socket connection
|
||||
SOCKET_RECV_TIMEOUT = 30 # seconds for HAProxy socket recv loop
|
||||
MAX_BULK_SERVERS = 10 # Max servers per bulk add call
|
||||
|
||||
|
||||
class HaproxyError(Exception):
|
||||
@@ -169,6 +170,29 @@ def haproxy_cmd(command: str) -> str:
|
||||
raise HaproxyError(str(e)) from e
|
||||
|
||||
|
||||
def haproxy_cmd_checked(command: str) -> str:
|
||||
"""Send command to HAProxy and raise on error response.
|
||||
|
||||
Args:
|
||||
command: HAProxy command to execute
|
||||
|
||||
Returns:
|
||||
Command response
|
||||
|
||||
Raises:
|
||||
HaproxyError: If HAProxy returns an error message
|
||||
"""
|
||||
result = haproxy_cmd(command)
|
||||
# HAProxy returns empty string on success, error messages on failure
|
||||
error_indicators = ["No such", "not found", "error", "failed", "invalid", "unknown"]
|
||||
if result:
|
||||
result_lower = result.lower()
|
||||
for indicator in error_indicators:
|
||||
if indicator.lower() in result_lower:
|
||||
raise HaproxyError(f"HAProxy command failed: {result.strip()}")
|
||||
return result
|
||||
|
||||
|
||||
def reload_haproxy() -> Tuple[bool, str]:
|
||||
"""Validate and reload HAProxy configuration.
|
||||
|
||||
@@ -485,7 +509,7 @@ def save_servers_config(config: Dict[str, Any]) -> None:
|
||||
|
||||
|
||||
def add_server_to_config(domain: str, slot: int, ip: str, http_port: int) -> None:
|
||||
"""Add server configuration to persistent storage.
|
||||
"""Add server configuration to persistent storage with file locking.
|
||||
|
||||
Args:
|
||||
domain: Domain name
|
||||
@@ -493,29 +517,38 @@ def add_server_to_config(domain: str, slot: int, ip: str, http_port: int) -> Non
|
||||
ip: Server IP address
|
||||
http_port: HTTP port
|
||||
"""
|
||||
config = load_servers_config()
|
||||
if domain not in config:
|
||||
config[domain] = {}
|
||||
config[domain][str(slot)] = {
|
||||
"ip": ip,
|
||||
"http_port": http_port
|
||||
}
|
||||
save_servers_config(config)
|
||||
lock_path = f"{SERVERS_FILE}.lock"
|
||||
with open(lock_path, 'w') as lock_file:
|
||||
fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX)
|
||||
try:
|
||||
config = load_servers_config()
|
||||
if domain not in config:
|
||||
config[domain] = {}
|
||||
config[domain][str(slot)] = {"ip": ip, "http_port": http_port}
|
||||
save_servers_config(config)
|
||||
finally:
|
||||
fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN)
|
||||
|
||||
|
||||
def remove_server_from_config(domain: str, slot: int) -> None:
|
||||
"""Remove server configuration from persistent storage.
|
||||
"""Remove server configuration from persistent storage with file locking.
|
||||
|
||||
Args:
|
||||
domain: Domain name
|
||||
slot: Server slot to remove
|
||||
"""
|
||||
config = load_servers_config()
|
||||
if domain in config and str(slot) in config[domain]:
|
||||
del config[domain][str(slot)]
|
||||
if not config[domain]:
|
||||
del config[domain]
|
||||
save_servers_config(config)
|
||||
lock_path = f"{SERVERS_FILE}.lock"
|
||||
with open(lock_path, 'w') as lock_file:
|
||||
fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX)
|
||||
try:
|
||||
config = load_servers_config()
|
||||
if domain in config and str(slot) in config[domain]:
|
||||
del config[domain][str(slot)]
|
||||
if not config[domain]:
|
||||
del config[domain]
|
||||
save_servers_config(config)
|
||||
finally:
|
||||
fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN)
|
||||
|
||||
|
||||
def remove_domain_from_config(domain: str) -> None:
|
||||
@@ -891,8 +924,8 @@ def haproxy_add_server(domain: str, slot: int, ip: str, http_port: int = 80) ->
|
||||
results = []
|
||||
for suffix, port in get_server_suffixes(http_port):
|
||||
server = f"{server_prefix}{suffix}_{slot}"
|
||||
haproxy_cmd(f"set server {backend}/{server} addr {ip} port {port}")
|
||||
haproxy_cmd(f"set server {backend}/{server} state ready")
|
||||
haproxy_cmd_checked(f"set server {backend}/{server} addr {ip} port {port}")
|
||||
haproxy_cmd_checked(f"set server {backend}/{server} state ready")
|
||||
results.append(f"{server} → {ip}:{port}")
|
||||
|
||||
# Save to persistent config
|
||||
@@ -942,6 +975,9 @@ def haproxy_add_servers(domain: str, servers: str) -> str:
|
||||
if not server_list:
|
||||
return "Error: servers array is empty"
|
||||
|
||||
if len(server_list) > MAX_BULK_SERVERS:
|
||||
return f"Error: Cannot add more than {MAX_BULK_SERVERS} servers at once"
|
||||
|
||||
# Validate all servers first before adding any
|
||||
validated_servers = []
|
||||
validation_errors = []
|
||||
@@ -1014,8 +1050,8 @@ def haproxy_add_servers(domain: str, servers: str) -> str:
|
||||
try:
|
||||
for suffix, port in get_server_suffixes(http_port):
|
||||
server = f"{server_prefix}{suffix}_{slot}"
|
||||
haproxy_cmd(f"set server {backend}/{server} addr {ip} port {port}")
|
||||
haproxy_cmd(f"set server {backend}/{server} state ready")
|
||||
haproxy_cmd_checked(f"set server {backend}/{server} addr {ip} port {port}")
|
||||
haproxy_cmd_checked(f"set server {backend}/{server} state ready")
|
||||
|
||||
# Save to persistent config
|
||||
add_server_to_config(domain, slot, ip, http_port)
|
||||
@@ -1063,8 +1099,8 @@ def haproxy_remove_server(domain: str, slot: int) -> str:
|
||||
|
||||
# HTTP only - single server per slot
|
||||
server = f"{server_prefix}_{slot}"
|
||||
haproxy_cmd(f"set server {backend}/{server} state maint")
|
||||
haproxy_cmd(f"set server {backend}/{server} addr 0.0.0.0 port 0")
|
||||
haproxy_cmd_checked(f"set server {backend}/{server} state maint")
|
||||
haproxy_cmd_checked(f"set server {backend}/{server} addr 0.0.0.0 port 0")
|
||||
|
||||
# Remove from persistent config
|
||||
remove_server_from_config(domain, slot)
|
||||
@@ -1323,8 +1359,8 @@ def haproxy_set_server_state(backend: str, server: str, state: str) -> str:
|
||||
if state not in ["ready", "drain", "maint"]:
|
||||
return "Error: state must be 'ready', 'drain', or 'maint'"
|
||||
try:
|
||||
result = haproxy_cmd(f"set server {backend}/{server} state {state}")
|
||||
return result if result else f"Server {backend}/{server} set to {state}"
|
||||
haproxy_cmd_checked(f"set server {backend}/{server} state {state}")
|
||||
return f"Server {backend}/{server} set to {state}"
|
||||
except HaproxyError as e:
|
||||
return f"Error: {e}"
|
||||
|
||||
@@ -1392,8 +1428,8 @@ def haproxy_set_server_weight(backend: str, server: str, weight: int) -> str:
|
||||
if not (0 <= weight <= 256):
|
||||
return "Error: weight must be between 0 and 256"
|
||||
try:
|
||||
result = haproxy_cmd(f"set server {backend}/{server} weight {weight}")
|
||||
return result if result else f"Server {backend}/{server} weight set to {weight}"
|
||||
haproxy_cmd_checked(f"set server {backend}/{server} weight {weight}")
|
||||
return f"Server {backend}/{server} weight set to {weight}"
|
||||
except HaproxyError as e:
|
||||
return f"Error: {e}"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user