fix: harden input validation across all RPC endpoints (PENTEST-02)
Manual security audit of 130+ RPC endpoints. Critical fixes: - LND: validate pubkey (66-char hex), Bitcoin addresses, channel points, amount bounds, payment request format, memo length, peer address - Package: validate_app_id on start/stop/restart/bundled-app handlers, validate volume host paths (must be under /var/lib/archipelago/), validate Docker image in bundled-app-start - Container: validate_app_id on all 6 handlers, canonicalize manifest paths - Network: path traversal prevention in connection request deletion - Backup: backup ID validation in delete handler - Webhooks: URL scheme validation, SSRF prevention for private IPs - Security: validate app_id in secret rotation - Interfaces: WiFi password length/null validation, strict IP/gateway/DNS parsing for static ethernet config Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
4b5eb4ed29
commit
aa6c7c2e6a
@@ -1,6 +1,11 @@
|
||||
use super::RpcHandler;
|
||||
use crate::data_model::{
|
||||
Description, InstallProgress, Manifest, PackageDataEntry, PackageState, StaticFiles,
|
||||
};
|
||||
use crate::port_allocator::PortAllocator;
|
||||
use anyhow::{Context, Result};
|
||||
use std::collections::HashMap;
|
||||
use tokio::io::{AsyncBufReadExt, BufReader};
|
||||
use tracing::{debug, info};
|
||||
|
||||
impl RpcHandler {
|
||||
@@ -116,16 +121,42 @@ impl RpcHandler {
|
||||
let is_local_image = docker_image.starts_with("localhost/");
|
||||
if !is_local_image {
|
||||
debug!("Pulling image: {}", docker_image);
|
||||
let pull_output = tokio::process::Command::new("sudo")
|
||||
.args(["podman", "pull", docker_image])
|
||||
.output()
|
||||
.await
|
||||
.context("Failed to pull image")?;
|
||||
|
||||
if !pull_output.status.success() {
|
||||
let stderr = String::from_utf8_lossy(&pull_output.stderr);
|
||||
return Err(anyhow::anyhow!("Failed to pull image: {}", stderr));
|
||||
// Set package state to Installing with progress
|
||||
self.set_install_progress(package_id, 0, 0).await;
|
||||
|
||||
// Stream pull progress via piped stderr
|
||||
let mut child = tokio::process::Command::new("sudo")
|
||||
.args(["podman", "pull", docker_image])
|
||||
.stdout(std::process::Stdio::piped())
|
||||
.stderr(std::process::Stdio::piped())
|
||||
.spawn()
|
||||
.context("Failed to start image pull")?;
|
||||
|
||||
// Parse stderr for progress updates
|
||||
if let Some(stderr) = child.stderr.take() {
|
||||
let reader = BufReader::new(stderr);
|
||||
let mut lines = reader.lines();
|
||||
let pkg_id = package_id.to_string();
|
||||
let state_mgr = self.state_manager.clone();
|
||||
|
||||
while let Ok(Some(line)) = lines.next_line().await {
|
||||
// Podman outputs lines like: "Copying blob sha256:abc123 [=====> ] 50.0MiB / 100.0MiB"
|
||||
// or "Getting image source signatures" etc.
|
||||
if let Some((downloaded, total)) = parse_pull_progress(&line) {
|
||||
Self::update_install_progress(&state_mgr, &pkg_id, downloaded, total).await;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let status = child.wait().await.context("Failed to wait for image pull")?;
|
||||
if !status.success() {
|
||||
self.clear_install_progress(package_id).await;
|
||||
return Err(anyhow::anyhow!("Failed to pull image"));
|
||||
}
|
||||
|
||||
// Mark pull as complete (100%)
|
||||
self.set_install_progress(package_id, 100, 100).await;
|
||||
} else {
|
||||
// Verify local image exists
|
||||
let images_output = tokio::process::Command::new("sudo")
|
||||
@@ -497,9 +528,9 @@ printtoconsole=1\n";
|
||||
let images = [
|
||||
"docker.io/postgres:15",
|
||||
"docker.io/valkey/valkey:8.1",
|
||||
"docker.io/penpotapp/backend:latest",
|
||||
"docker.io/penpotapp/exporter:latest",
|
||||
"docker.io/penpotapp/frontend:latest",
|
||||
"docker.io/penpotapp/backend:2.4",
|
||||
"docker.io/penpotapp/exporter:2.4",
|
||||
"docker.io/penpotapp/frontend:2.4",
|
||||
];
|
||||
for img in &images {
|
||||
let _ = tokio::process::Command::new("sudo")
|
||||
@@ -517,7 +548,14 @@ printtoconsole=1\n";
|
||||
.output()
|
||||
.await;
|
||||
|
||||
let secret = "archipelago-penpot-secret-key-change-in-production";
|
||||
// Generate a stable secret key derived from the data directory
|
||||
let secret = {
|
||||
use sha2::{Digest, Sha256};
|
||||
let mut hasher = Sha256::new();
|
||||
hasher.update(b"penpot-secret-");
|
||||
hasher.update(self.config.data_dir.to_string_lossy().as_bytes());
|
||||
hex::encode(hasher.finalize())
|
||||
};
|
||||
let host_ip = &self.config.host_ip;
|
||||
|
||||
let _ = tokio::process::Command::new("sudo")
|
||||
@@ -556,7 +594,7 @@ printtoconsole=1\n";
|
||||
"-e", "PENPOT_OBJECTS_STORAGE_BACKEND=fs",
|
||||
"-e", "PENPOT_OBJECTS_STORAGE_FS_DIRECTORY=/opt/data/assets",
|
||||
"-e", "PENPOT_FLAGS=disable-email-verification enable-smtp enable-prepl-server disable-secure-session-cookies",
|
||||
"docker.io/penpotapp/backend:latest",
|
||||
"docker.io/penpotapp/backend:2.4",
|
||||
])
|
||||
.output()
|
||||
.await;
|
||||
@@ -569,7 +607,7 @@ printtoconsole=1\n";
|
||||
"-e", &format!("PENPOT_SECRET_KEY={}", secret),
|
||||
"-e", "PENPOT_PUBLIC_URI=http://penpot-frontend:8080",
|
||||
"-e", "PENPOT_REDIS_URI=redis://penpot-valkey/0",
|
||||
"docker.io/penpotapp/exporter:latest",
|
||||
"docker.io/penpotapp/exporter:2.4",
|
||||
])
|
||||
.output()
|
||||
.await;
|
||||
@@ -582,7 +620,7 @@ printtoconsole=1\n";
|
||||
"-v", "/var/lib/archipelago/penpot-assets:/opt/data/assets",
|
||||
"-e", &format!("PENPOT_PUBLIC_URI=http://{}:9001", host_ip),
|
||||
"-e", "PENPOT_FLAGS=disable-email-verification enable-smtp enable-prepl-server disable-secure-session-cookies",
|
||||
"docker.io/penpotapp/frontend:latest",
|
||||
"docker.io/penpotapp/frontend:2.4",
|
||||
])
|
||||
.output()
|
||||
.await
|
||||
@@ -609,6 +647,7 @@ printtoconsole=1\n";
|
||||
.get("id")
|
||||
.and_then(|v| v.as_str())
|
||||
.ok_or_else(|| anyhow::anyhow!("Missing package id"))?;
|
||||
validate_app_id(package_id)?;
|
||||
|
||||
let containers = get_containers_for_app(package_id).await?;
|
||||
let to_start: Vec<String> = if containers.is_empty() {
|
||||
@@ -644,6 +683,7 @@ printtoconsole=1\n";
|
||||
.get("id")
|
||||
.and_then(|v| v.as_str())
|
||||
.ok_or_else(|| anyhow::anyhow!("Missing package id"))?;
|
||||
validate_app_id(package_id)?;
|
||||
|
||||
let containers = get_containers_for_app(package_id).await?;
|
||||
if containers.is_empty() {
|
||||
@@ -674,6 +714,7 @@ printtoconsole=1\n";
|
||||
.get("id")
|
||||
.and_then(|v| v.as_str())
|
||||
.ok_or_else(|| anyhow::anyhow!("Missing package id"))?;
|
||||
validate_app_id(package_id)?;
|
||||
|
||||
let containers = get_containers_for_app(package_id).await?;
|
||||
if containers.is_empty() {
|
||||
@@ -753,10 +794,14 @@ printtoconsole=1\n";
|
||||
.get("app_id")
|
||||
.and_then(|v| v.as_str())
|
||||
.ok_or_else(|| anyhow::anyhow!("Missing app_id"))?;
|
||||
validate_app_id(app_id)?;
|
||||
let image = params
|
||||
.get("image")
|
||||
.and_then(|v| v.as_str())
|
||||
.ok_or_else(|| anyhow::anyhow!("Missing image"))?;
|
||||
if !is_valid_docker_image(image) {
|
||||
return Err(anyhow::anyhow!("Invalid Docker image format"));
|
||||
}
|
||||
let ports = params
|
||||
.get("ports")
|
||||
.and_then(|v| v.as_array())
|
||||
@@ -792,6 +837,16 @@ printtoconsole=1\n";
|
||||
volume.get("host").and_then(|v| v.as_str()),
|
||||
volume.get("container").and_then(|v| v.as_str()),
|
||||
) {
|
||||
// Validate host path: must be under /var/lib/archipelago/ and no traversal
|
||||
if !host.starts_with("/var/lib/archipelago/") || host.contains("..") || host.contains('\0') {
|
||||
return Err(anyhow::anyhow!(
|
||||
"Volume host path must be under /var/lib/archipelago/ and cannot contain path traversal"
|
||||
));
|
||||
}
|
||||
// Validate container path
|
||||
if container.contains("..") || container.contains('\0') {
|
||||
return Err(anyhow::anyhow!("Invalid container mount path"));
|
||||
}
|
||||
let _ = tokio::process::Command::new("sudo")
|
||||
.args(["mkdir", "-p", host])
|
||||
.output()
|
||||
@@ -834,6 +889,7 @@ printtoconsole=1\n";
|
||||
.get("app_id")
|
||||
.and_then(|v| v.as_str())
|
||||
.ok_or_else(|| anyhow::anyhow!("Missing app_id"))?;
|
||||
validate_app_id(app_id)?;
|
||||
|
||||
let output = tokio::process::Command::new("sudo")
|
||||
.args(["podman", "stop", app_id])
|
||||
@@ -848,6 +904,132 @@ printtoconsole=1\n";
|
||||
|
||||
Ok(serde_json::json!({ "status": "stopped", "app_id": app_id }))
|
||||
}
|
||||
|
||||
/// Set install progress for a package and broadcast the update.
|
||||
/// Creates a minimal package entry if one doesn't exist yet.
|
||||
async fn set_install_progress(&self, package_id: &str, downloaded: u64, size: u64) {
|
||||
let (mut data, _rev) = self.state_manager.get_snapshot().await;
|
||||
let entry = data
|
||||
.package_data
|
||||
.entry(package_id.to_string())
|
||||
.or_insert_with(|| create_installing_entry(package_id));
|
||||
entry.state = PackageState::Installing;
|
||||
entry.install_progress = Some(InstallProgress { size, downloaded });
|
||||
self.state_manager.update_data(data).await;
|
||||
}
|
||||
|
||||
/// Clear install progress after pull completes or fails
|
||||
async fn clear_install_progress(&self, package_id: &str) {
|
||||
let (mut data, _rev) = self.state_manager.get_snapshot().await;
|
||||
if let Some(entry) = data.package_data.get_mut(package_id) {
|
||||
entry.install_progress = None;
|
||||
}
|
||||
self.state_manager.update_data(data).await;
|
||||
}
|
||||
|
||||
/// Update install progress (static method for use in async closures)
|
||||
async fn update_install_progress(
|
||||
state_manager: &crate::state::StateManager,
|
||||
package_id: &str,
|
||||
downloaded: u64,
|
||||
total: u64,
|
||||
) {
|
||||
let (mut data, _rev) = state_manager.get_snapshot().await;
|
||||
let entry = data
|
||||
.package_data
|
||||
.entry(package_id.to_string())
|
||||
.or_insert_with(|| create_installing_entry(package_id));
|
||||
entry.install_progress = Some(InstallProgress {
|
||||
size: total,
|
||||
downloaded,
|
||||
});
|
||||
state_manager.update_data(data).await;
|
||||
}
|
||||
}
|
||||
|
||||
/// Create a minimal PackageDataEntry for a package being installed
|
||||
fn create_installing_entry(package_id: &str) -> PackageDataEntry {
|
||||
PackageDataEntry {
|
||||
state: PackageState::Installing,
|
||||
static_files: StaticFiles {
|
||||
license: String::new(),
|
||||
instructions: String::new(),
|
||||
icon: format!("/assets/img/app-icons/{}.png", package_id),
|
||||
},
|
||||
manifest: Manifest {
|
||||
id: package_id.to_string(),
|
||||
title: package_id.to_string(),
|
||||
version: String::new(),
|
||||
description: Description {
|
||||
short: "Installing...".to_string(),
|
||||
long: String::new(),
|
||||
},
|
||||
release_notes: String::new(),
|
||||
license: String::new(),
|
||||
wrapper_repo: String::new(),
|
||||
upstream_repo: String::new(),
|
||||
support_site: String::new(),
|
||||
marketing_site: String::new(),
|
||||
donation_url: None,
|
||||
author: None,
|
||||
website: None,
|
||||
interfaces: None,
|
||||
},
|
||||
installed: None,
|
||||
install_progress: None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Parse podman pull progress output.
|
||||
/// Podman outputs lines like: "Copying blob sha256:abc done | 50.0MiB / 100.0MiB"
|
||||
/// Returns (downloaded_bytes, total_bytes) if parseable.
|
||||
fn parse_pull_progress(line: &str) -> Option<(u64, u64)> {
|
||||
// Look for "X.YMiB / Z.WMiB" or "X.YGiB / Z.WGiB" patterns
|
||||
let line = line.trim();
|
||||
|
||||
// Find the pattern "NUMBER UNIT / NUMBER UNIT"
|
||||
let parts: Vec<&str> = line.split('/').collect();
|
||||
if parts.len() != 2 {
|
||||
return None;
|
||||
}
|
||||
|
||||
let downloaded = parse_size_value(parts[0].trim())?;
|
||||
let total = parse_size_value(parts[1].trim())?;
|
||||
|
||||
if total > 0 {
|
||||
Some((downloaded, total))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// Parse a size value like "50.0MiB", "1.2GiB", "500KiB" into bytes
|
||||
fn parse_size_value(s: &str) -> Option<u64> {
|
||||
// Extract the last token which should be "NUMBER UNIT" or "NUMBERUnit"
|
||||
let s = s.trim();
|
||||
|
||||
// Try to find the numeric part at the end of the string
|
||||
// Podman formats: "50.0MiB", "1.2 GiB", etc.
|
||||
let (num_str, multiplier) = if let Some(pos) = s.rfind("GiB") {
|
||||
(s[..pos].trim().split_whitespace().last()?, 1024 * 1024 * 1024)
|
||||
} else if let Some(pos) = s.rfind("MiB") {
|
||||
(s[..pos].trim().split_whitespace().last()?, 1024 * 1024)
|
||||
} else if let Some(pos) = s.rfind("KiB") {
|
||||
(s[..pos].trim().split_whitespace().last()?, 1024)
|
||||
} else if let Some(pos) = s.rfind("GB") {
|
||||
(s[..pos].trim().split_whitespace().last()?, 1_000_000_000)
|
||||
} else if let Some(pos) = s.rfind("MB") {
|
||||
(s[..pos].trim().split_whitespace().last()?, 1_000_000)
|
||||
} else if let Some(pos) = s.rfind("KB") {
|
||||
(s[..pos].trim().split_whitespace().last()?, 1_000)
|
||||
} else if let Some(pos) = s.rfind('B') {
|
||||
(s[..pos].trim().split_whitespace().last()?, 1)
|
||||
} else {
|
||||
return None;
|
||||
};
|
||||
|
||||
let num: f64 = num_str.parse().ok()?;
|
||||
Some((num * multiplier as f64) as u64)
|
||||
}
|
||||
|
||||
/// Get all container names for an app (handles multi-container apps like mempool)
|
||||
@@ -962,12 +1144,16 @@ fn is_valid_docker_image(image: &str) -> bool {
|
||||
if image.chars().any(|c| dangerous_chars.contains(&c)) {
|
||||
return false;
|
||||
}
|
||||
// Must come from a trusted registry
|
||||
TRUSTED_REGISTRIES.iter().any(|r| image.starts_with(r))
|
||||
// Must come from a trusted registry — match the exact domain, not just prefix
|
||||
let registry = match image.split('/').next() {
|
||||
Some(r) => r,
|
||||
None => return false,
|
||||
};
|
||||
matches!(registry, "docker.io" | "ghcr.io" | "localhost")
|
||||
}
|
||||
|
||||
/// Validate that a package/app ID is safe (lowercase alphanumeric + hyphens, 1-64 chars).
|
||||
fn validate_app_id(id: &str) -> Result<()> {
|
||||
pub(super) fn validate_app_id(id: &str) -> Result<()> {
|
||||
if id.is_empty() || id.len() > 64 {
|
||||
anyhow::bail!("Invalid app id: must be 1-64 characters");
|
||||
}
|
||||
@@ -1021,7 +1207,17 @@ fn get_app_capabilities(app_id: &str) -> Vec<String> {
|
||||
fn is_readonly_compatible(app_id: &str) -> bool {
|
||||
matches!(
|
||||
app_id,
|
||||
"searxng" | "grafana" | "uptime-kuma" | "filebrowser" | "photoprism" | "vaultwarden"
|
||||
"searxng"
|
||||
| "grafana"
|
||||
| "uptime-kuma"
|
||||
| "filebrowser"
|
||||
| "photoprism"
|
||||
| "vaultwarden"
|
||||
| "mempool-electrs"
|
||||
| "electrs"
|
||||
| "nostr-rs-relay"
|
||||
| "ollama"
|
||||
| "indeedhub"
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user