diff --git a/README.md b/README.md index db5ebac..3503ab1 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,32 @@ The following headers are always redacted and cannot be un-redacted: `Authorizat ], ``` +### `audit_http_client_redact_params` + +Additional query parameter names whose values are logged as `[redacted]` in the `uri` field. Matching is case-insensitive; the parameter name and the rest of the URI stay unchanged. Default: `[]`. + +The following parameters are always redacted and cannot be un-redacted: `access_token`, `api_key`, `apikey`, `auth`, `client_secret`, `password`, `secret`, `signature`, `token`, `X-Amz-Credential`, `X-Amz-Security-Token`, `X-Amz-Signature`. + +```php +'audit_http_client_redact_params' => [ + 'session_key', +], +``` + +### `audit_http_client_redact_path_patterns` + +Additional PCRE patterns whose matches in the URI **path** are logged as `[redacted]` — for secrets that live in the path instead of the query string. Default: `[]`. + +The following pattern is always applied and cannot be removed: `#(?<=/)private-[^/]+#` — it covers Google's secret iCal addresses (`…/ical//private-/basic.ics`), where the hash is the only protection of the calendar. + +```php +'audit_http_client_redact_path_patterns' => [ + '#(?<=/)key-[0-9a-f]+#', +], +``` + +Invalid patterns are skipped. To skip logging for a host entirely, use [`audit_http_client_exclude_domains`](#audit_http_client_exclude_domains) instead. + ## Installation This app is not yet in the App Store. It is built with [ncmake](https://github.com/ernolf/ncmake). To build and install it from source — release tarball, `make rsync` or `make cp` — see the [installation guide](https://github.com/ernolf/ncmake/blob/main/doc/INSTALL.md). diff --git a/lib/Http/Client/LoggingClientService.php b/lib/Http/Client/LoggingClientService.php index 723d128..150e15e 100644 --- a/lib/Http/Client/LoggingClientService.php +++ b/lib/Http/Client/LoggingClientService.php @@ -54,6 +54,8 @@ public function newClient(?callable $handler = null): IClient { $this->config->getSystemValueString('audit_http_client_format', 'both'), (array)$this->config->getSystemValue('audit_http_client_exclude_domains', []), (array)$this->config->getSystemValue('audit_http_client_redact_headers', []), + (array)$this->config->getSystemValue('audit_http_client_redact_params', []), + (array)$this->config->getSystemValue('audit_http_client_redact_path_patterns', []), $this->request->getId(), ), 'admin_audit_http_client' diff --git a/lib/Http/Client/Middleware/HttpClientLoggerMiddleware.php b/lib/Http/Client/Middleware/HttpClientLoggerMiddleware.php index f25a029..ccdfb1a 100644 --- a/lib/Http/Client/Middleware/HttpClientLoggerMiddleware.php +++ b/lib/Http/Client/Middleware/HttpClientLoggerMiddleware.php @@ -29,12 +29,42 @@ class HttpClientLoggerMiddleware { 'x-auth-token', ]; + /** + * Query parameter values that would leak credentials to disk when the URI + * is logged; always logged as [redacted], with no opt-out. + */ + private const DEFAULT_REDACT_PARAMS = [ + 'access_token', + 'api_key', + 'apikey', + 'auth', + 'client_secret', + 'password', + 'secret', + 'signature', + 'token', + 'x-amz-credential', + 'x-amz-security-token', + 'x-amz-signature', + ]; + + /** + * PCRE patterns whose matches in the URI path are logged as [redacted]. + * Google marks its secret iCal URLs with a "private-" path segment; + * anyone holding such a URL can read the calendar. + */ + private const DEFAULT_REDACT_PATH_PATTERNS = [ + '#(?<=/)private-[^/]+#', + ]; + private LoggerInterface $logger; private string $logBaseDir; private int $logLevel; private string $logFormat; private array $excludeDomains; private array $redactHeaders; + private array $redactParams; + private array $redactPathPatterns; private string $serverReqId; public function __construct( @@ -44,6 +74,8 @@ public function __construct( string $logFormat = 'both', array $excludeDomains = [], array $redactHeaders = [], + array $redactParams = [], + array $redactPathPatterns = [], string $serverReqId = '', ) { $this->logger = $logger; @@ -53,13 +85,28 @@ public function __construct( $this->excludeDomains = $excludeDomains; $this->serverReqId = $serverReqId; - $this->redactHeaders = self::DEFAULT_REDACT_HEADERS; - foreach ($redactHeaders as $name) { + $this->redactHeaders = $this->mergeRedactList(self::DEFAULT_REDACT_HEADERS, $redactHeaders); + $this->redactParams = $this->mergeRedactList(self::DEFAULT_REDACT_PARAMS, $redactParams); + + // Patterns are regexes: merge verbatim, without case normalization. + $this->redactPathPatterns = self::DEFAULT_REDACT_PATH_PATTERNS; + foreach ($redactPathPatterns as $pattern) { + $pattern = (string)$pattern; + if ($pattern !== '' && !in_array($pattern, $this->redactPathPatterns, true)) { + $this->redactPathPatterns[] = $pattern; + } + } + } + + private function mergeRedactList(array $defaults, array $extra): array { + $merged = $defaults; + foreach ($extra as $name) { $name = strtolower(trim((string)$name)); - if ($name !== '' && !in_array($name, $this->redactHeaders, true)) { - $this->redactHeaders[] = $name; + if ($name !== '' && !in_array($name, $merged, true)) { + $merged[] = $name; } } + return $merged; } public function __invoke(callable $handler): callable { @@ -110,7 +157,7 @@ function (ResponseInterface $response) use ($request, $reqHeaders, $reqId) { 'reqId' => $reqId, 'time' => date('c'), 'method' => $request->getMethod(), - 'uri' => (string)$request->getUri(), + 'uri' => $this->redactUri((string)$request->getUri()), 'status' => $response->getStatusCode(), 'http' => 'HTTP/' . $response->getProtocolVersion(), 'requestHeaders' => $this->compactHeaders($reqHeaders), @@ -175,13 +222,16 @@ function (ResponseInterface $response) use ($request, $reqHeaders, $reqId) { }, function ($reason) use ($request, $reqHeaders, $reqId) { try { + $uri = $this->redactUri((string)$request->getUri()); + $reqCompact = $this->compactHeaders($reqHeaders); + $entry = [ 'reqId' => $reqId, 'time' => date('c'), 'method' => $request->getMethod(), - 'uri' => (string)$request->getUri(), + 'uri' => $uri, 'error' => is_object($reason) ? get_class($reason) : (string)$reason, - 'requestHeaders' => $this->compactHeaders($reqHeaders), + 'requestHeaders' => $reqCompact, 'event' => 'error', ]; @@ -190,13 +240,13 @@ function ($reason) use ($request, $reqHeaders, $reqId) { $reqId, date('c'), $request->getMethod(), - (string)$request->getUri(), + $uri, is_object($reason) ? get_class($reason) : (string)$reason ); $metaForPaths = [ - 'uri' => (string)$request->getUri(), - 'requestHeaders' => $this->compactHeaders($reqHeaders), + 'uri' => $uri, + 'requestHeaders' => $reqCompact, ]; [$jsonFile, $plainFile] = LogPathHelper::getPathsFromMeta($metaForPaths, $this->logBaseDir, $reqId); @@ -346,6 +396,53 @@ private function isExcluded(string $host): bool { return false; } + /** + * Redacts credentials from a URI before it is logged: values of + * credential-bearing query parameters become [redacted] (names keep their + * original spelling, matching is case-insensitive, parameters without a + * value and the fragment are left untouched), and path portions matching + * the configured patterns are replaced likewise. Invalid patterns are + * skipped. + */ + private function redactUri(string $uri): string { + $qPos = strpos($uri, '?'); + $base = $qPos === false ? $uri : substr($uri, 0, $qPos); + + foreach ($this->redactPathPatterns as $pattern) { + $replaced = @preg_replace($pattern, '[redacted]', $base); + if (is_string($replaced)) { + $base = $replaced; + } + } + + if ($qPos === false) { + return $base; + } + + $query = substr($uri, $qPos + 1); + + $fragment = ''; + $fPos = strpos($query, '#'); + if ($fPos !== false) { + $fragment = substr($query, $fPos); + $query = substr($query, 0, $fPos); + } + + $pairs = explode('&', $query); + foreach ($pairs as $i => $pair) { + $eq = strpos($pair, '='); + if ($eq === false) { + continue; + } + $name = substr($pair, 0, $eq); + if (in_array(strtolower(rawurldecode($name)), $this->redactParams, true)) { + $pairs[$i] = $name . '=[redacted]'; + } + } + + return $base . '?' . implode('&', $pairs) . $fragment; + } + private function normalizeHeaders(array $hdrs): array { $out = []; foreach ($hdrs as $k => $v) { diff --git a/tests/unit/Http/Client/Middleware/HttpClientLoggerMiddlewareTest.php b/tests/unit/Http/Client/Middleware/HttpClientLoggerMiddlewareTest.php index d659839..fe0d6e3 100644 --- a/tests/unit/Http/Client/Middleware/HttpClientLoggerMiddlewareTest.php +++ b/tests/unit/Http/Client/Middleware/HttpClientLoggerMiddlewareTest.php @@ -25,6 +25,8 @@ private function middleware( int $logLevel = 0, array $excludeDomains = [], array $redactHeaders = [], + array $redactParams = [], + array $redactPathPatterns = [], string $serverReqId = '', ): HttpClientLoggerMiddleware { return new HttpClientLoggerMiddleware( @@ -34,6 +36,8 @@ private function middleware( 'both', $excludeDomains, $redactHeaders, + $redactParams, + $redactPathPatterns, $serverReqId, ); } @@ -113,7 +117,7 @@ public function testStatsEntryIsClearedWhenResponseIsNotLogged(): void { } public function testGeneratedRequestIdCarriesServerRequestIdPrefix(): void { - $mw = $this->middleware(2, [], [], 'server-req'); + $mw = $this->middleware(2, [], [], [], [], 'server-req'); $seen = null; $handler = $mw(function (Request $request, array $options) use (&$seen) { @@ -261,6 +265,91 @@ public function testCompactHeadersRedactsConfiguredExtraHeaders(): void { ); } + public function testRedactUriRedactsDefaultParamsCaseInsensitively(): void { + $mw = $this->middleware(); + $this->assertSame( + 'https://api.example.com/v1/data?access_token=[redacted]&page=2', + $this->invokePrivate($mw, 'redactUri', ['https://api.example.com/v1/data?access_token=geheim&page=2']), + ); + $this->assertSame( + 'https://api.example.com/v1/data?Access_Token=[redacted]', + $this->invokePrivate($mw, 'redactUri', ['https://api.example.com/v1/data?Access_Token=geheim']), + ); + } + + public function testRedactUriRedactsConfiguredExtraParams(): void { + $mw = $this->middleware(0, [], [], ['session_key']); + $this->assertSame( + 'https://x.test/p?session_key=[redacted]&token=[redacted]', + $this->invokePrivate($mw, 'redactUri', ['https://x.test/p?session_key=abc&token=xyz']), + ); + } + + public function testRedactUriLeavesValuelessParamsAndFragment(): void { + $mw = $this->middleware(); + $this->assertSame( + 'https://x.test/p?token&flag=1#frag', + $this->invokePrivate($mw, 'redactUri', ['https://x.test/p?token&flag=1#frag']), + ); + } + + public function testRedactUriRedactsPrivatePathSegments(): void { + $mw = $this->middleware(); + $this->assertSame( + 'https://calendar.google.com/calendar/ical/user%40googlemail.com/[redacted]/basic.ics', + $this->invokePrivate($mw, 'redactUri', [ + 'https://calendar.google.com/calendar/ical/user%40googlemail.com/private-0123456789abcdef0123456789abcdef/basic.ics', + ]), + ); + } + + public function testRedactUriAppliesConfiguredPathPatterns(): void { + $mw = $this->middleware(0, [], [], [], ['#(?<=/)key-[0-9a-f]+#']); + $this->assertSame( + 'https://x.test/feed/[redacted]/data.xml', + $this->invokePrivate($mw, 'redactUri', ['https://x.test/feed/key-abc123/data.xml']), + ); + } + + public function testRedactUriSkipsInvalidPathPatterns(): void { + $mw = $this->middleware(0, [], [], [], ['#[unclosed']); + $this->assertSame( + 'https://x.test/feed/data.xml', + $this->invokePrivate($mw, 'redactUri', ['https://x.test/feed/data.xml']), + ); + } + + public function testRedactUriWithoutQueryIsUnchanged(): void { + $mw = $this->middleware(); + $this->assertSame( + 'https://x.test/plain/path', + $this->invokePrivate($mw, 'redactUri', ['https://x.test/plain/path']), + ); + } + + public function testLoggedUriIsRedacted(): void { + $dir = sys_get_temp_dir() . '/aahc-uri-' . bin2hex(random_bytes(4)); + $mw = new HttpClientLoggerMiddleware(new NullLogger(), $dir); + + $handler = $mw(function (Request $request, array $options) { + return new FulfilledPromise(new Response(204)); + }); + + try { + $handler(new Request('GET', 'https://example.com/hook?token=geheim&id=7'), [])->wait(); + + $lines = file($dir . '/example.com.json', FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES); + $this->assertNotFalse($lines); + $entry = json_decode($lines[0], true, 512, JSON_THROW_ON_ERROR); + $this->assertSame('https://example.com/hook?token=[redacted]&id=7', $entry['uri']); + } finally { + foreach (glob($dir . '/*') ?: [] as $file) { + @unlink($file); + } + @rmdir($dir); + } + } + public function testCompactHeadersCastsNumericLengths(): void { $mw = $this->middleware(); $this->assertSame(