diff --git a/containers/squid/Dockerfile b/containers/squid/Dockerfile index 19ecff14..96251cf5 100644 --- a/containers/squid/Dockerfile +++ b/containers/squid/Dockerfile @@ -1,10 +1,9 @@ FROM ubuntu/squid:latest # Install additional tools for debugging, healthcheck, and SSL Bump -# gosu is used to drop from root to proxy user after permission setup # Retry logic handles transient 404s when Ubuntu archive supersedes package versions mid-build RUN set -eux; \ - PKGS="curl dnsutils gosu net-tools netcat-openbsd openssl squid-openssl"; \ + PKGS="curl dnsutils net-tools netcat-openbsd openssl squid-openssl"; \ apt-get update && \ apt-get install -y --only-upgrade gpgv && \ ( apt-get install -y --no-install-recommends $PKGS || \ @@ -24,5 +23,10 @@ RUN chmod +x /usr/local/bin/entrypoint.sh EXPOSE 3128 EXPOSE 3129 -# Use entrypoint to fix permissions before starting Squid +# Run as non-root proxy user for security hardening +# Permission setup is done at build time; mounted volumes must be pre-configured +# by the host (docker-manager.ts sets correct ownership before starting containers) +USER proxy + +# Use entrypoint to start Squid (runs as proxy user) ENTRYPOINT ["/usr/local/bin/entrypoint.sh"] diff --git a/containers/squid/entrypoint.sh b/containers/squid/entrypoint.sh index e8639f59..24f3e4ba 100644 --- a/containers/squid/entrypoint.sh +++ b/containers/squid/entrypoint.sh @@ -1,29 +1,13 @@ #!/bin/bash set -e -# Fix permissions on mounted log directory -# The directory is mounted from the host and may have wrong ownership -chown -R proxy:proxy /var/log/squid -chmod -R 755 /var/log/squid +# This entrypoint runs as the non-root 'proxy' user (set by USER in Dockerfile). +# All directory permissions are set at build time or by the host before container start. -# Fix permissions on SSL certificate database if SSL Bump is enabled -# The database is initialized on the host side by awf, but the permissions -# need to be fixed for the proxy user inside the container. +# Verify SSL certificate database permissions if SSL Bump is enabled if [ -d "/var/spool/squid_ssl_db" ]; then - echo "[squid-entrypoint] SSL Bump mode detected - fixing SSL database permissions..." - - # Fix ownership for Squid (runs as proxy user) - chown -R proxy:proxy /var/spool/squid_ssl_db - chmod -R 700 /var/spool/squid_ssl_db - - echo "[squid-entrypoint] SSL certificate database ready" + echo "[squid-entrypoint] SSL Bump mode detected - SSL database ready" fi -# Ensure Squid config directory and run directory are writable by proxy -chown -R proxy:proxy /etc/squid /var/run/squid /var/spool/squid 2>/dev/null || true - -# Ensure pid file is writable by proxy user (default: /run/squid.pid) -touch /run/squid.pid && chown proxy:proxy /run/squid.pid - -# Drop to proxy user and start Squid -exec gosu proxy squid -N -d 1 +# Start Squid directly (already running as proxy user via Dockerfile USER directive) +exec squid -N -d 1 diff --git a/src/chown-recursive.test.ts b/src/chown-recursive.test.ts new file mode 100644 index 00000000..bf7dc8b6 --- /dev/null +++ b/src/chown-recursive.test.ts @@ -0,0 +1,89 @@ +/** + * Isolated test for chownRecursive that mocks fs to test the traversal logic. + * This is in a separate file because the main ssl-bump.test.ts uses real fs operations, + * and Node.js fs.chownSync is non-configurable (can't be spied on or redefined). + */ + +import * as path from 'path'; + +// Mock fs before importing the module under test +const mockChownSync = jest.fn(); +const mockReaddirSync = jest.fn(); +jest.mock('fs', () => { + const actual = jest.requireActual('fs'); + return { + ...actual, + chownSync: (...args: unknown[]) => mockChownSync(...args), + readdirSync: (...args: unknown[]) => { + // Only intercept calls with { withFileTypes: true } (from chownRecursive) + if (args[1] && typeof args[1] === 'object' && 'withFileTypes' in args[1]) { + return mockReaddirSync(...args); + } + return actual.readdirSync(...args); + }, + }; +}); + +// Mock execa (required by ssl-bump module) +jest.mock('execa'); + +import { chownRecursive } from './ssl-bump'; + +describe('chownRecursive', () => { + beforeEach(() => { + mockChownSync.mockReset(); + mockReaddirSync.mockReset(); + }); + + it('should chown the directory itself', () => { + mockReaddirSync.mockReturnValue([]); + + chownRecursive('/some/dir', 13, 13); + + expect(mockChownSync).toHaveBeenCalledWith('/some/dir', 13, 13); + }); + + it('should chown files in the directory', () => { + mockReaddirSync.mockReturnValue([ + { name: 'file1.txt', isDirectory: () => false }, + { name: 'file2.txt', isDirectory: () => false }, + ]); + + chownRecursive('/some/dir', 13, 13); + + expect(mockChownSync).toHaveBeenCalledTimes(3); // dir + 2 files + expect(mockChownSync).toHaveBeenCalledWith('/some/dir', 13, 13); + expect(mockChownSync).toHaveBeenCalledWith(path.join('/some/dir', 'file1.txt'), 13, 13); + expect(mockChownSync).toHaveBeenCalledWith(path.join('/some/dir', 'file2.txt'), 13, 13); + }); + + it('should recursively chown subdirectories', () => { + // Root dir has a subdir and a file + mockReaddirSync + .mockReturnValueOnce([ + { name: 'subdir', isDirectory: () => true }, + { name: 'root-file.txt', isDirectory: () => false }, + ]) + // Subdir has one file + .mockReturnValueOnce([ + { name: 'sub-file.txt', isDirectory: () => false }, + ]); + + chownRecursive('/root', 13, 13); + + expect(mockChownSync).toHaveBeenCalledTimes(4); // root + subdir + root-file + sub-file + expect(mockChownSync).toHaveBeenCalledWith('/root', 13, 13); + expect(mockChownSync).toHaveBeenCalledWith(path.join('/root', 'subdir'), 13, 13); + expect(mockChownSync).toHaveBeenCalledWith(path.join('/root', 'root-file.txt'), 13, 13); + expect(mockChownSync).toHaveBeenCalledWith(path.join('/root', 'subdir', 'sub-file.txt'), 13, 13); + }); + + it('should handle empty directory', () => { + mockReaddirSync.mockReturnValue([]); + + chownRecursive('/empty', 1000, 1000); + + expect(mockChownSync).toHaveBeenCalledTimes(1); // just the dir itself + expect(mockChownSync).toHaveBeenCalledWith('/empty', 1000, 1000); + }); +}); diff --git a/src/docker-manager.ts b/src/docker-manager.ts index c65e7226..665023b7 100644 --- a/src/docker-manager.ts +++ b/src/docker-manager.ts @@ -1189,11 +1189,19 @@ export async function writeConfigs(config: WrapperConfig): Promise { // Otherwise, use workDir/squid-logs (will be moved to /tmp after cleanup) // Note: Squid runs as user 'proxy' (UID 13, GID 13 in ubuntu/squid image) // We need to make the directory writable by the proxy user + // Squid container runs as non-root 'proxy' user (UID 13, GID 13) + // Set ownership so proxy user can write logs without root privileges + const SQUID_PROXY_UID = 13; + const SQUID_PROXY_GID = 13; const squidLogsDir = config.proxyLogsDir || path.join(config.workDir, 'squid-logs'); if (!fs.existsSync(squidLogsDir)) { - fs.mkdirSync(squidLogsDir, { recursive: true, mode: 0o777 }); - // Explicitly set permissions to 0o777 (not affected by umask) - fs.chmodSync(squidLogsDir, 0o777); + fs.mkdirSync(squidLogsDir, { recursive: true, mode: 0o755 }); + try { + fs.chownSync(squidLogsDir, SQUID_PROXY_UID, SQUID_PROXY_GID); + } catch { + // Fallback to world-writable if chown fails (e.g., non-root context) + fs.chmodSync(squidLogsDir, 0o777); + } } logger.debug(`Squid logs directory created at: ${squidLogsDir}`); diff --git a/src/squid-config.ts b/src/squid-config.ts index c9ee3cae..6116ac78 100644 --- a/src/squid-config.ts +++ b/src/squid-config.ts @@ -506,6 +506,9 @@ ${sslBump ? '\n# SSL Bump mode enabled - HTTPS traffic will be intercepted for U # Disable pinger (ICMP) - requires NET_RAW capability which we don't have for security pinger_enable off +# PID file location - use proxy-owned directory since container runs as non-root +pid_filename /var/run/squid/squid.pid + # Custom log format with detailed connection information # Format: timestamp client_ip:port dest_domain dest_ip:port protocol method status decision url user_agent # Note: For CONNECT requests (HTTPS), the domain is in the URL field diff --git a/src/ssl-bump.test.ts b/src/ssl-bump.test.ts index 8a87b88a..bf96b1b4 100644 --- a/src/ssl-bump.test.ts +++ b/src/ssl-bump.test.ts @@ -2,7 +2,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import execa from 'execa'; -import { parseUrlPatterns, generateSessionCa, initSslDb, isOpenSslAvailable, secureWipeFile, cleanupSslKeyMaterial } from './ssl-bump'; +import { parseUrlPatterns, generateSessionCa, initSslDb, isOpenSslAvailable, secureWipeFile, cleanupSslKeyMaterial, chownRecursive } from './ssl-bump'; // Pattern constant for the safer URL character class (matches the implementation) const URL_CHAR_PATTERN = '[^\\s]*'; @@ -315,6 +315,47 @@ describe('SSL Bump', () => { fs.chmodSync(sslDbPath, 0o700); } }); + + it('should gracefully handle EPERM from chown (non-root)', async () => { + // initSslDb calls chownRecursive(sslDbPath, 13, 13) internally. + // When not running as root, chownSync throws EPERM which is caught. + // This test verifies the EPERM path completes successfully. + const result = await initSslDb(tempDir); + expect(result).toBe(path.join(tempDir, 'ssl_db')); + // Verify the ssl_db was fully created despite chown failure + expect(fs.existsSync(path.join(result, 'certs'))).toBe(true); + expect(fs.existsSync(path.join(result, 'index.txt'))).toBe(true); + expect(fs.existsSync(path.join(result, 'size'))).toBe(true); + }); + + }); + + describe('chownRecursive', () => { + let tempDir: string; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'chown-test-')); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + it('should attempt to chown a directory and its contents', () => { + // Create directory structure + const subDir = path.join(tempDir, 'subdir'); + fs.mkdirSync(subDir); + fs.writeFileSync(path.join(tempDir, 'file1.txt'), 'test'); + fs.writeFileSync(path.join(subDir, 'file2.txt'), 'test'); + + // chownRecursive will throw EPERM when not root, but it should + // attempt to chown the root directory first + expect(() => chownRecursive(tempDir, 13, 13)).toThrow(/EPERM/); + }); + + it('should throw for non-existent directory', () => { + expect(() => chownRecursive('/tmp/nonexistent-chown-test', 13, 13)).toThrow(); + }); }); describe('isOpenSslAvailable', () => { diff --git a/src/ssl-bump.ts b/src/ssl-bump.ts index 62ecb367..a7324851 100644 --- a/src/ssl-bump.ts +++ b/src/ssl-bump.ts @@ -18,6 +18,21 @@ import * as crypto from 'crypto'; import execa from 'execa'; import { logger } from './logger'; +/** + * Recursively chown a directory and its contents + */ +export function chownRecursive(dirPath: string, uid: number, gid: number): void { + fs.chownSync(dirPath, uid, gid); + for (const entry of fs.readdirSync(dirPath, { withFileTypes: true })) { + const fullPath = path.join(dirPath, entry.name); + if (entry.isDirectory()) { + chownRecursive(fullPath, uid, gid); + } else { + fs.chownSync(fullPath, uid, gid); + } + } +} + /** * Configuration for SSL Bump CA generation */ @@ -273,6 +288,15 @@ export async function initSslDb(workDir: string): Promise { if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err; } + // Chown to proxy user (uid=13, gid=13) so the non-root Squid container can access it + // Gracefully skip if not running as root (e.g., in unit tests) + try { + chownRecursive(sslDbPath, 13, 13); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code !== 'EPERM') throw err; + logger.debug('Skipping SSL db chown (not running as root)'); + } + logger.debug(`SSL certificate database initialized at: ${sslDbPath}`); return sslDbPath; }