Skip to content

Commit a97c757

Browse files
Mossakaclaude
andauthored
fix(squid): run Squid container as non-root proxy user (#1271)
* fix(security): run Squid container as non-root proxy user Add USER directive to Squid Dockerfile so the container runs as the non-root 'proxy' user (uid=13) from the start, reducing the impact of potential container escapes or Squid vulnerabilities. Changes: - Dockerfile: Add USER proxy, remove gosu dependency - entrypoint.sh: Remove root-only chown operations and gosu exec - docker-manager.ts: Set squid log dir ownership to proxy uid - ssl-bump.ts: Chown SSL db to proxy user after initialization - squid-config.ts: Add pid_filename to proxy-owned directory Fixes #250 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add coverage for chownRecursive and SSL db chown in ssl-bump Export chownRecursive function and add isolated tests with mocked fs to cover directory traversal logic. Add tests for EPERM graceful handling in initSslDb. Fixes coverage regression in ssl-bump.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent fa8d80b commit a97c757

File tree

7 files changed

+182
-29
lines changed

7 files changed

+182
-29
lines changed

containers/squid/Dockerfile

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
FROM ubuntu/squid:latest
22

33
# Install additional tools for debugging, healthcheck, and SSL Bump
4-
# gosu is used to drop from root to proxy user after permission setup
54
# Retry logic handles transient 404s when Ubuntu archive supersedes package versions mid-build
65
RUN set -eux; \
7-
PKGS="curl dnsutils gosu net-tools netcat-openbsd openssl squid-openssl"; \
6+
PKGS="curl dnsutils net-tools netcat-openbsd openssl squid-openssl"; \
87
apt-get update && \
98
apt-get install -y --only-upgrade gpgv && \
109
( apt-get install -y --no-install-recommends $PKGS || \
@@ -24,5 +23,10 @@ RUN chmod +x /usr/local/bin/entrypoint.sh
2423
EXPOSE 3128
2524
EXPOSE 3129
2625

27-
# Use entrypoint to fix permissions before starting Squid
26+
# Run as non-root proxy user for security hardening
27+
# Permission setup is done at build time; mounted volumes must be pre-configured
28+
# by the host (docker-manager.ts sets correct ownership before starting containers)
29+
USER proxy
30+
31+
# Use entrypoint to start Squid (runs as proxy user)
2832
ENTRYPOINT ["/usr/local/bin/entrypoint.sh"]

containers/squid/entrypoint.sh

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,13 @@
11
#!/bin/bash
22
set -e
33

4-
# Fix permissions on mounted log directory
5-
# The directory is mounted from the host and may have wrong ownership
6-
chown -R proxy:proxy /var/log/squid
7-
chmod -R 755 /var/log/squid
4+
# This entrypoint runs as the non-root 'proxy' user (set by USER in Dockerfile).
5+
# All directory permissions are set at build time or by the host before container start.
86

9-
# Fix permissions on SSL certificate database if SSL Bump is enabled
10-
# The database is initialized on the host side by awf, but the permissions
11-
# need to be fixed for the proxy user inside the container.
7+
# Verify SSL certificate database permissions if SSL Bump is enabled
128
if [ -d "/var/spool/squid_ssl_db" ]; then
13-
echo "[squid-entrypoint] SSL Bump mode detected - fixing SSL database permissions..."
14-
15-
# Fix ownership for Squid (runs as proxy user)
16-
chown -R proxy:proxy /var/spool/squid_ssl_db
17-
chmod -R 700 /var/spool/squid_ssl_db
18-
19-
echo "[squid-entrypoint] SSL certificate database ready"
9+
echo "[squid-entrypoint] SSL Bump mode detected - SSL database ready"
2010
fi
2111

22-
# Ensure Squid config directory and run directory are writable by proxy
23-
chown -R proxy:proxy /etc/squid /var/run/squid /var/spool/squid 2>/dev/null || true
24-
25-
# Ensure pid file is writable by proxy user (default: /run/squid.pid)
26-
touch /run/squid.pid && chown proxy:proxy /run/squid.pid
27-
28-
# Drop to proxy user and start Squid
29-
exec gosu proxy squid -N -d 1
12+
# Start Squid directly (already running as proxy user via Dockerfile USER directive)
13+
exec squid -N -d 1

src/chown-recursive.test.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/**
2+
* Isolated test for chownRecursive that mocks fs to test the traversal logic.
3+
* This is in a separate file because the main ssl-bump.test.ts uses real fs operations,
4+
* and Node.js fs.chownSync is non-configurable (can't be spied on or redefined).
5+
*/
6+
7+
import * as path from 'path';
8+
9+
// Mock fs before importing the module under test
10+
const mockChownSync = jest.fn();
11+
const mockReaddirSync = jest.fn();
12+
jest.mock('fs', () => {
13+
const actual = jest.requireActual('fs');
14+
return {
15+
...actual,
16+
chownSync: (...args: unknown[]) => mockChownSync(...args),
17+
readdirSync: (...args: unknown[]) => {
18+
// Only intercept calls with { withFileTypes: true } (from chownRecursive)
19+
if (args[1] && typeof args[1] === 'object' && 'withFileTypes' in args[1]) {
20+
return mockReaddirSync(...args);
21+
}
22+
return actual.readdirSync(...args);
23+
},
24+
};
25+
});
26+
27+
// Mock execa (required by ssl-bump module)
28+
jest.mock('execa');
29+
30+
import { chownRecursive } from './ssl-bump';
31+
32+
describe('chownRecursive', () => {
33+
beforeEach(() => {
34+
mockChownSync.mockReset();
35+
mockReaddirSync.mockReset();
36+
});
37+
38+
it('should chown the directory itself', () => {
39+
mockReaddirSync.mockReturnValue([]);
40+
41+
chownRecursive('/some/dir', 13, 13);
42+
43+
expect(mockChownSync).toHaveBeenCalledWith('/some/dir', 13, 13);
44+
});
45+
46+
it('should chown files in the directory', () => {
47+
mockReaddirSync.mockReturnValue([
48+
{ name: 'file1.txt', isDirectory: () => false },
49+
{ name: 'file2.txt', isDirectory: () => false },
50+
]);
51+
52+
chownRecursive('/some/dir', 13, 13);
53+
54+
expect(mockChownSync).toHaveBeenCalledTimes(3); // dir + 2 files
55+
expect(mockChownSync).toHaveBeenCalledWith('/some/dir', 13, 13);
56+
expect(mockChownSync).toHaveBeenCalledWith(path.join('/some/dir', 'file1.txt'), 13, 13);
57+
expect(mockChownSync).toHaveBeenCalledWith(path.join('/some/dir', 'file2.txt'), 13, 13);
58+
});
59+
60+
it('should recursively chown subdirectories', () => {
61+
// Root dir has a subdir and a file
62+
mockReaddirSync
63+
.mockReturnValueOnce([
64+
{ name: 'subdir', isDirectory: () => true },
65+
{ name: 'root-file.txt', isDirectory: () => false },
66+
])
67+
// Subdir has one file
68+
.mockReturnValueOnce([
69+
{ name: 'sub-file.txt', isDirectory: () => false },
70+
]);
71+
72+
chownRecursive('/root', 13, 13);
73+
74+
expect(mockChownSync).toHaveBeenCalledTimes(4); // root + subdir + root-file + sub-file
75+
expect(mockChownSync).toHaveBeenCalledWith('/root', 13, 13);
76+
expect(mockChownSync).toHaveBeenCalledWith(path.join('/root', 'subdir'), 13, 13);
77+
expect(mockChownSync).toHaveBeenCalledWith(path.join('/root', 'root-file.txt'), 13, 13);
78+
expect(mockChownSync).toHaveBeenCalledWith(path.join('/root', 'subdir', 'sub-file.txt'), 13, 13);
79+
});
80+
81+
it('should handle empty directory', () => {
82+
mockReaddirSync.mockReturnValue([]);
83+
84+
chownRecursive('/empty', 1000, 1000);
85+
86+
expect(mockChownSync).toHaveBeenCalledTimes(1); // just the dir itself
87+
expect(mockChownSync).toHaveBeenCalledWith('/empty', 1000, 1000);
88+
});
89+
});

src/docker-manager.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,11 +1189,19 @@ export async function writeConfigs(config: WrapperConfig): Promise<void> {
11891189
// Otherwise, use workDir/squid-logs (will be moved to /tmp after cleanup)
11901190
// Note: Squid runs as user 'proxy' (UID 13, GID 13 in ubuntu/squid image)
11911191
// We need to make the directory writable by the proxy user
1192+
// Squid container runs as non-root 'proxy' user (UID 13, GID 13)
1193+
// Set ownership so proxy user can write logs without root privileges
1194+
const SQUID_PROXY_UID = 13;
1195+
const SQUID_PROXY_GID = 13;
11921196
const squidLogsDir = config.proxyLogsDir || path.join(config.workDir, 'squid-logs');
11931197
if (!fs.existsSync(squidLogsDir)) {
1194-
fs.mkdirSync(squidLogsDir, { recursive: true, mode: 0o777 });
1195-
// Explicitly set permissions to 0o777 (not affected by umask)
1196-
fs.chmodSync(squidLogsDir, 0o777);
1198+
fs.mkdirSync(squidLogsDir, { recursive: true, mode: 0o755 });
1199+
try {
1200+
fs.chownSync(squidLogsDir, SQUID_PROXY_UID, SQUID_PROXY_GID);
1201+
} catch {
1202+
// Fallback to world-writable if chown fails (e.g., non-root context)
1203+
fs.chmodSync(squidLogsDir, 0o777);
1204+
}
11971205
}
11981206
logger.debug(`Squid logs directory created at: ${squidLogsDir}`);
11991207

src/squid-config.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,9 @@ ${sslBump ? '\n# SSL Bump mode enabled - HTTPS traffic will be intercepted for U
506506
# Disable pinger (ICMP) - requires NET_RAW capability which we don't have for security
507507
pinger_enable off
508508
509+
# PID file location - use proxy-owned directory since container runs as non-root
510+
pid_filename /var/run/squid/squid.pid
511+
509512
# Custom log format with detailed connection information
510513
# Format: timestamp client_ip:port dest_domain dest_ip:port protocol method status decision url user_agent
511514
# Note: For CONNECT requests (HTTPS), the domain is in the URL field

src/ssl-bump.test.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as fs from 'fs';
22
import * as path from 'path';
33
import * as os from 'os';
44
import execa from 'execa';
5-
import { parseUrlPatterns, generateSessionCa, initSslDb, isOpenSslAvailable, secureWipeFile, cleanupSslKeyMaterial } from './ssl-bump';
5+
import { parseUrlPatterns, generateSessionCa, initSslDb, isOpenSslAvailable, secureWipeFile, cleanupSslKeyMaterial, chownRecursive } from './ssl-bump';
66

77
// Pattern constant for the safer URL character class (matches the implementation)
88
const URL_CHAR_PATTERN = '[^\\s]*';
@@ -315,6 +315,47 @@ describe('SSL Bump', () => {
315315
fs.chmodSync(sslDbPath, 0o700);
316316
}
317317
});
318+
319+
it('should gracefully handle EPERM from chown (non-root)', async () => {
320+
// initSslDb calls chownRecursive(sslDbPath, 13, 13) internally.
321+
// When not running as root, chownSync throws EPERM which is caught.
322+
// This test verifies the EPERM path completes successfully.
323+
const result = await initSslDb(tempDir);
324+
expect(result).toBe(path.join(tempDir, 'ssl_db'));
325+
// Verify the ssl_db was fully created despite chown failure
326+
expect(fs.existsSync(path.join(result, 'certs'))).toBe(true);
327+
expect(fs.existsSync(path.join(result, 'index.txt'))).toBe(true);
328+
expect(fs.existsSync(path.join(result, 'size'))).toBe(true);
329+
});
330+
331+
});
332+
333+
describe('chownRecursive', () => {
334+
let tempDir: string;
335+
336+
beforeEach(() => {
337+
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'chown-test-'));
338+
});
339+
340+
afterEach(() => {
341+
fs.rmSync(tempDir, { recursive: true, force: true });
342+
});
343+
344+
it('should attempt to chown a directory and its contents', () => {
345+
// Create directory structure
346+
const subDir = path.join(tempDir, 'subdir');
347+
fs.mkdirSync(subDir);
348+
fs.writeFileSync(path.join(tempDir, 'file1.txt'), 'test');
349+
fs.writeFileSync(path.join(subDir, 'file2.txt'), 'test');
350+
351+
// chownRecursive will throw EPERM when not root, but it should
352+
// attempt to chown the root directory first
353+
expect(() => chownRecursive(tempDir, 13, 13)).toThrow(/EPERM/);
354+
});
355+
356+
it('should throw for non-existent directory', () => {
357+
expect(() => chownRecursive('/tmp/nonexistent-chown-test', 13, 13)).toThrow();
358+
});
318359
});
319360

320361
describe('isOpenSslAvailable', () => {

src/ssl-bump.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,21 @@ import * as crypto from 'crypto';
1818
import execa from 'execa';
1919
import { logger } from './logger';
2020

21+
/**
22+
* Recursively chown a directory and its contents
23+
*/
24+
export function chownRecursive(dirPath: string, uid: number, gid: number): void {
25+
fs.chownSync(dirPath, uid, gid);
26+
for (const entry of fs.readdirSync(dirPath, { withFileTypes: true })) {
27+
const fullPath = path.join(dirPath, entry.name);
28+
if (entry.isDirectory()) {
29+
chownRecursive(fullPath, uid, gid);
30+
} else {
31+
fs.chownSync(fullPath, uid, gid);
32+
}
33+
}
34+
}
35+
2136
/**
2237
* Configuration for SSL Bump CA generation
2338
*/
@@ -273,6 +288,15 @@ export async function initSslDb(workDir: string): Promise<string> {
273288
if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err;
274289
}
275290

291+
// Chown to proxy user (uid=13, gid=13) so the non-root Squid container can access it
292+
// Gracefully skip if not running as root (e.g., in unit tests)
293+
try {
294+
chownRecursive(sslDbPath, 13, 13);
295+
} catch (err: unknown) {
296+
if ((err as NodeJS.ErrnoException).code !== 'EPERM') throw err;
297+
logger.debug('Skipping SSL db chown (not running as root)');
298+
}
299+
276300
logger.debug(`SSL certificate database initialized at: ${sslDbPath}`);
277301
return sslDbPath;
278302
}

0 commit comments

Comments
 (0)