fix(security): Implement Phase 1 critical security fixes
CORS: - Restrict CORS to DASHBOARD_URL environment variable - Default to production dashboard domain Authentication: - Add AuthUser requirement to all agent management endpoints - Add AuthUser requirement to all command endpoints - Add AuthUser requirement to all metrics endpoints - Add audit logging for command execution (user_id tracked) Agent Security: - Replace Unicode characters with ASCII markers [OK]/[ERROR]/[WARNING] - Add certificate pinning for update downloads (allowlist domains) - Fix insecure temp file creation (use /var/run/gururmm with 0700 perms) - Fix rollback script backgrounding (use setsid instead of literal &) Dashboard Security: - Move token storage from localStorage to sessionStorage - Add proper TypeScript types (remove 'any' from error handlers) - Centralize token management functions Legacy Agent: - Add -AllowInsecureTLS parameter (opt-in required) - Add Windows Event Log audit trail when insecure mode used - Update documentation with security warnings Closes: Phase 1 items in issue #1 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -457,14 +457,14 @@ WantedBy=multi-user.target
|
||||
anyhow::bail!("systemctl enable failed");
|
||||
}
|
||||
|
||||
println!("\n✓ GuruRMM Agent installed successfully!");
|
||||
println!("\n[OK] GuruRMM Agent installed successfully!");
|
||||
println!("\nInstalled files:");
|
||||
println!(" Binary: {}", binary_dest);
|
||||
println!(" Config: {}", config_dest);
|
||||
println!(" Service: {}", unit_file);
|
||||
|
||||
if config_needs_manual_edit {
|
||||
println!("\n⚠️ IMPORTANT: Edit {} with your server URL and API key!", config_dest);
|
||||
println!("\n[WARNING] IMPORTANT: Edit {} with your server URL and API key!", config_dest);
|
||||
println!("\nNext steps:");
|
||||
println!(" 1. Edit {} with your server URL and API key", config_dest);
|
||||
println!(" 2. Start the service: sudo systemctl start {}", SERVICE_NAME);
|
||||
@@ -475,9 +475,9 @@ WantedBy=multi-user.target
|
||||
.status();
|
||||
|
||||
if status.is_ok() && status.unwrap().success() {
|
||||
println!("✓ Service started successfully!");
|
||||
println!("[OK] Service started successfully!");
|
||||
} else {
|
||||
println!("⚠️ Failed to start service. Check logs: sudo journalctl -u {} -f", SERVICE_NAME);
|
||||
println!("[WARNING] Failed to start service. Check logs: sudo journalctl -u {} -f", SERVICE_NAME);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -556,7 +556,7 @@ async fn uninstall_systemd_service() -> Result<()> {
|
||||
.args(["daemon-reload"])
|
||||
.status();
|
||||
|
||||
println!("\n✓ GuruRMM Agent uninstalled successfully!");
|
||||
println!("\n[OK] GuruRMM Agent uninstalled successfully!");
|
||||
println!("\nNote: Config directory {} was preserved.", CONFIG_DIR);
|
||||
println!("Remove it manually if no longer needed: sudo rm -rf {}", CONFIG_DIR);
|
||||
|
||||
@@ -582,7 +582,7 @@ async fn start_service() -> Result<()> {
|
||||
.context("Failed to start service")?;
|
||||
|
||||
if status.success() {
|
||||
println!("** Service started successfully");
|
||||
println!("[OK] Service started successfully");
|
||||
println!("Check status: sudo systemctl status gururmm-agent");
|
||||
} else {
|
||||
anyhow::bail!("Failed to start service. Check: sudo journalctl -u gururmm-agent -n 50");
|
||||
@@ -616,7 +616,7 @@ async fn stop_service() -> Result<()> {
|
||||
.context("Failed to stop service")?;
|
||||
|
||||
if status.success() {
|
||||
println!("** Service stopped successfully");
|
||||
println!("[OK] Service stopped successfully");
|
||||
} else {
|
||||
anyhow::bail!("Failed to stop service");
|
||||
}
|
||||
|
||||
@@ -177,7 +177,36 @@ impl AgentUpdater {
|
||||
}
|
||||
|
||||
/// Download the new binary to a temp file
|
||||
///
|
||||
/// Security: Validates URL against allowed domains and requires HTTPS for external hosts
|
||||
async fn download_binary(&self, url: &str) -> Result<PathBuf> {
|
||||
// Validate URL is from trusted domain
|
||||
let allowed_domains = [
|
||||
"rmm-api.azcomputerguru.com",
|
||||
"downloads.azcomputerguru.com",
|
||||
"172.16.3.30", // Internal server
|
||||
];
|
||||
|
||||
let parsed_url = url::Url::parse(url)
|
||||
.context("Invalid download URL")?;
|
||||
|
||||
let host = parsed_url.host_str()
|
||||
.ok_or_else(|| anyhow::anyhow!("No host in download URL"))?;
|
||||
|
||||
if !allowed_domains.iter().any(|d| host == *d || host.ends_with(&format!(".{}", d))) {
|
||||
return Err(anyhow::anyhow!(
|
||||
"Download URL host '{}' not in allowed domains",
|
||||
host
|
||||
));
|
||||
}
|
||||
|
||||
// Require HTTPS (except for local/internal IPs)
|
||||
if parsed_url.scheme() != "https" && !host.starts_with("172.16.") && !host.starts_with("192.168.") {
|
||||
return Err(anyhow::anyhow!("Download URL must use HTTPS"));
|
||||
}
|
||||
|
||||
info!("[OK] URL validation passed: {}", url);
|
||||
|
||||
let response = self.http_client.get(url)
|
||||
.send()
|
||||
.await
|
||||
@@ -273,10 +302,26 @@ impl AgentUpdater {
|
||||
|
||||
#[cfg(unix)]
|
||||
async fn create_unix_rollback_watchdog(&self) -> Result<()> {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let backup_path = self.config.backup_path();
|
||||
let binary_path = &self.config.binary_path;
|
||||
let timeout = self.config.rollback_timeout_secs;
|
||||
|
||||
// Use secure directory instead of /tmp/ (world-writable)
|
||||
let script_dir = PathBuf::from("/var/run/gururmm");
|
||||
|
||||
// Create directory if needed with restricted permissions (owner only)
|
||||
if !script_dir.exists() {
|
||||
tokio::fs::create_dir_all(&script_dir).await
|
||||
.context("Failed to create secure script directory")?;
|
||||
std::fs::set_permissions(&script_dir, std::fs::Permissions::from_mode(0o700))
|
||||
.context("Failed to set script directory permissions")?;
|
||||
}
|
||||
|
||||
// Use UUID in filename to prevent predictable paths
|
||||
let script_path = script_dir.join(format!("rollback-{}.sh", Uuid::new_v4()));
|
||||
|
||||
let script = format!(r#"#!/bin/bash
|
||||
# GuruRMM Rollback Watchdog
|
||||
# Auto-generated - will be deleted after successful update
|
||||
@@ -284,49 +329,50 @@ impl AgentUpdater {
|
||||
BACKUP="{backup}"
|
||||
BINARY="{binary}"
|
||||
TIMEOUT={timeout}
|
||||
SCRIPT_PATH="{script}"
|
||||
|
||||
sleep $TIMEOUT
|
||||
|
||||
# Check if agent service is running
|
||||
if ! systemctl is-active --quiet gururmm-agent 2>/dev/null; then
|
||||
echo "Agent not running after update, rolling back..."
|
||||
echo "[WARNING] Agent not running after update, rolling back..."
|
||||
if [ -f "$BACKUP" ]; then
|
||||
cp "$BACKUP" "$BINARY"
|
||||
chmod +x "$BINARY"
|
||||
systemctl start gururmm-agent
|
||||
echo "Rollback completed"
|
||||
echo "[OK] Rollback completed"
|
||||
else
|
||||
echo "No backup file found!"
|
||||
echo "[ERROR] No backup file found!"
|
||||
fi
|
||||
fi
|
||||
|
||||
# Clean up this script
|
||||
rm -f /tmp/gururmm-rollback.sh
|
||||
rm -f "$SCRIPT_PATH"
|
||||
"#,
|
||||
backup = backup_path.display(),
|
||||
binary = binary_path.display(),
|
||||
timeout = timeout
|
||||
timeout = timeout,
|
||||
script = script_path.display()
|
||||
);
|
||||
|
||||
let script_path = PathBuf::from("/tmp/gururmm-rollback.sh");
|
||||
fs::write(&script_path, script).await?;
|
||||
fs::write(&script_path, script).await
|
||||
.context("Failed to write rollback script")?;
|
||||
|
||||
// Make executable and run in background
|
||||
tokio::process::Command::new("chmod")
|
||||
.arg("+x")
|
||||
.arg(&script_path)
|
||||
.status()
|
||||
.await?;
|
||||
// Set restrictive permissions (700 - owner only)
|
||||
std::fs::set_permissions(&script_path, std::fs::Permissions::from_mode(0o700))
|
||||
.context("Failed to set rollback script permissions")?;
|
||||
|
||||
// Spawn as detached background process
|
||||
tokio::process::Command::new("nohup")
|
||||
// Spawn as detached background process using setsid (not nohup with "&" literal arg)
|
||||
tokio::process::Command::new("setsid")
|
||||
.arg("bash")
|
||||
.arg(&script_path)
|
||||
.arg("&")
|
||||
.stdin(std::process::Stdio::null())
|
||||
.stdout(std::process::Stdio::null())
|
||||
.stderr(std::process::Stdio::null())
|
||||
.spawn()
|
||||
.context("Failed to spawn rollback watchdog")?;
|
||||
|
||||
info!("Rollback watchdog started (timeout: {}s)", timeout);
|
||||
info!("[OK] Rollback watchdog started (timeout: {}s)", timeout);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -524,12 +570,29 @@ Remove-Item -Path $MyInvocation.MyCommand.Path -Force
|
||||
pub async fn cancel_rollback_watchdog(&self) {
|
||||
#[cfg(unix)]
|
||||
{
|
||||
// Kill the watchdog script
|
||||
// Kill any running rollback watchdog scripts
|
||||
let _ = tokio::process::Command::new("pkill")
|
||||
.args(["-f", "gururmm-rollback.sh"])
|
||||
.args(["-f", "rollback-.*\\.sh"])
|
||||
.status()
|
||||
.await;
|
||||
let _ = fs::remove_file("/tmp/gururmm-rollback.sh").await;
|
||||
|
||||
// Clean up the secure script directory
|
||||
let script_dir = PathBuf::from("/var/run/gururmm");
|
||||
if script_dir.exists() {
|
||||
// Remove all rollback scripts in the directory
|
||||
if let Ok(mut entries) = tokio::fs::read_dir(&script_dir).await {
|
||||
while let Ok(Some(entry)) = entries.next_entry().await {
|
||||
let path = entry.path();
|
||||
if path.file_name()
|
||||
.and_then(|n| n.to_str())
|
||||
.map(|n| n.starts_with("rollback-"))
|
||||
.unwrap_or(false)
|
||||
{
|
||||
let _ = fs::remove_file(&path).await;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
|
||||
Reference in New Issue
Block a user