Skip to content

Commit 0ec9e41

Browse files
ext/soap: redact more sensitive headers
1 parent 7bcd245 commit 0ec9e41

20 files changed

Lines changed: 371 additions & 102 deletions

‎ext/soap/php_sdl.c‎

Lines changed: 166 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,75 @@ static int is_wsdl_element(xmlNodePtr node)
198198
return 1;
199199
}
200200

201+
// Which headers need to be redacted
202+
#define REDACT_NONE 0
203+
#define REDACT_AUTHORIZATION (1 << 0)
204+
#define REDACT_PROXY_AUTH (1 << 1)
205+
#define REDACT_COOKIE (1 << 2)
206+
207+
static uint32_t header_needs_redaction(zend_string *header) {
208+
uint32_t redaction = REDACT_NONE;
209+
/* The various loops are so that if the first use of "authorization" is in
210+
* some other header, we still redact the actual "authorization" header.
211+
*
212+
* We check if the character before the start of the header is a '\n' but
213+
* not if the character before that was a `\r`; per RFC9112 while the
214+
* separator is officially both (CRLF) recipients "MAY" recognize just a
215+
* '\n' (LF) as a line terminator. A recipient "MUST" treat a bare '\r' (CR)
216+
* as invalid or replace it, so a '\r' on its own does not start a new
217+
* header.
218+
*/
219+
char *auth_start = strstr(ZSTR_VAL(header), "authorization:");
220+
while (auth_start && *auth_start) {
221+
if (auth_start == ZSTR_VAL(header) || *(auth_start - 1) == '\n') {
222+
redaction |= REDACT_AUTHORIZATION;
223+
break;
224+
}
225+
auth_start = strstr(auth_start + 1, "authorization:");
226+
}
227+
228+
char *proxy_start = strstr(ZSTR_VAL(header), "proxy-authorization:");
229+
while (proxy_start && *proxy_start) {
230+
if (proxy_start == ZSTR_VAL(header) || *(proxy_start - 1) == '\n') {
231+
redaction |= REDACT_PROXY_AUTH;
232+
break;
233+
}
234+
proxy_start = strstr(proxy_start + 1, "proxy-authorization:");
235+
}
236+
237+
char *cookie_start = strstr(ZSTR_VAL(header), "cookie:");
238+
while (cookie_start && *cookie_start) {
239+
if (cookie_start == ZSTR_VAL(header) || *(cookie_start - 1) == '\n') {
240+
redaction |= REDACT_COOKIE;
241+
break;
242+
}
243+
cookie_start = strstr(cookie_start + 1, "cookie:");
244+
}
245+
return redaction;
246+
}
247+
248+
static bool redact_header(char *header_start, uint32_t to_redact) {
249+
/* When redacting, we don't remove the REDACT_* flag from to_redact, because
250+
* a header might have been specified multiple times and we want to remove
251+
* all of those uses. */
252+
if (to_redact & REDACT_AUTHORIZATION
253+
&& strncmp(header_start, "authorization:", strlen("authorization:")) == 0
254+
) {
255+
return true;
256+
}
257+
if (to_redact & REDACT_PROXY_AUTH
258+
&& strncmp(header_start, "proxy-authorization:", strlen("proxy-authorization:")) == 0
259+
) {
260+
return true;
261+
}
262+
if (to_redact & REDACT_COOKIE
263+
&& strncmp(header_start, "cookie:", strlen("cookie:")) == 0
264+
) {
265+
return true;
266+
}
267+
return false;
268+
}
269+
201270
void sdl_set_uri_credentials(sdlCtx *ctx, char *uri)
202271
{
203272
char *s;
@@ -264,26 +333,107 @@ void sdl_set_uri_credentials(sdlCtx *ctx, char *uri)
264333
return;
265334
}
266335

267-
if ((header = php_stream_context_get_option(ctx->context, "http", "header")) != NULL &&
268-
Z_TYPE_P(header) == IS_STRING
269-
) {
270-
/* TODO: should support header as an array, but this code path is untested */
271-
s = strstr(Z_STRVAL_P(header), "Authorization: Basic");
272-
if (s && (s == Z_STRVAL_P(header) || *(s-1) == '\n' || *(s-1) == '\r')) {
273-
char *rest = strstr(s, "\r\n");
274-
if (rest) {
275-
zval new_header;
336+
header = php_stream_context_get_option(ctx->context, "http", "header");
337+
if (header == NULL) {
338+
return;
339+
}
276340

277-
rest += 2;
278-
ZVAL_NEW_STR(&new_header, zend_string_alloc(Z_STRLEN_P(header) - (rest - s), 0));
279-
memcpy(Z_STRVAL(new_header), Z_STRVAL_P(header), s - Z_STRVAL_P(header));
280-
memcpy(Z_STRVAL(new_header) + (s - Z_STRVAL_P(header)), rest, Z_STRLEN_P(header) - (rest - Z_STRVAL_P(header)) + 1);
281-
ZVAL_COPY(&ctx->old_header, header);
282-
php_stream_context_set_option(ctx->context, "http", "header", &new_header);
283-
zval_ptr_dtor(&new_header);
341+
zend_string *flat_headers;
342+
if (Z_TYPE_P(header) == IS_STRING) {
343+
flat_headers = zend_string_copy(Z_STR_P(header));
344+
} else if (Z_TYPE_P(header) == IS_ARRAY) {
345+
smart_str buf = {0};
346+
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(header), zval *value) {
347+
if (Z_TYPE_P(value) == IS_STRING && Z_STRLEN_P(value)) {
348+
if (buf.s) {
349+
smart_str_appendl(&buf, "\r\n", 2);
350+
}
351+
smart_str_append(&buf, Z_STR_P(value));
352+
}
353+
} ZEND_HASH_FOREACH_END();
354+
flat_headers = smart_str_extract(&buf);
355+
} else {
356+
return;
357+
}
358+
zend_string *lc_headers = zend_string_tolower(flat_headers);
359+
// Fast path: no redaction needed
360+
uint32_t redaction = header_needs_redaction(lc_headers);
361+
if (redaction == REDACT_NONE) {
362+
zend_string_release(lc_headers);
363+
zend_string_release(flat_headers);
364+
return;
365+
}
366+
// At least one of the headers needs to be stripped. To avoid repeated
367+
// copying and case comparisons, we go through the headers one at a time.
368+
// Will be allocated on the first header that should be kept.
369+
zend_string *redacted = NULL;
370+
// Pointer to the current read location
371+
char *read_ptr = ZSTR_VAL(flat_headers);
372+
// Offset from the start of the read location
373+
size_t read_offset = 0;
374+
// Offset from the start of the write location
375+
size_t write_offset = 0;
376+
377+
while (true) {
378+
if (redact_header(ZSTR_VAL(lc_headers) + read_offset, redaction)) {
379+
/* Current header should be skipped
380+
* As with header_needs_redaction() we treat a '\n' as the end
381+
* of the header even if not preceded by '\r'. */
382+
char *header_end = memchr(read_ptr, '\n', ZSTR_LEN(flat_headers) - read_offset);
383+
if (header_end) {
384+
// Skip the current header and the \n
385+
size_t header_length = (header_end - read_ptr) + 1;
386+
read_offset += header_length;
387+
read_ptr += header_length;
388+
} else {
389+
// This is the end
390+
break;
391+
}
392+
} else {
393+
// Current header (from `read_offset`) should be kept
394+
if (redacted == NULL) {
395+
// This will over allocate the redacted header string but
396+
// that isn't a big deal, the headers should be fairly short
397+
redacted = zend_string_alloc(ZSTR_LEN(flat_headers), false);
398+
}
399+
char *header_end = memchr(read_ptr, '\n', ZSTR_LEN(flat_headers) - read_offset);
400+
if (header_end) {
401+
// Copy the current header and the \n
402+
size_t header_length = (header_end - read_ptr) + 1;
403+
memcpy(ZSTR_VAL(redacted) + write_offset, read_ptr, header_length);
404+
// Update offsets
405+
read_offset += header_length;
406+
read_ptr += header_length;
407+
write_offset += header_length;
408+
} else {
409+
// Copy until the end of the headers
410+
size_t header_length = ZSTR_LEN(lc_headers) - read_offset;
411+
memcpy(ZSTR_VAL(redacted) + write_offset, read_ptr, header_length);
412+
write_offset += header_length;
413+
break;
284414
}
285415
}
286416
}
417+
zend_string_release(lc_headers);
418+
zend_string_release(flat_headers);
419+
420+
ZVAL_COPY(&ctx->old_header, header);
421+
zval new_header;
422+
if (redacted == NULL) {
423+
// All headers were redacted
424+
ZVAL_EMPTY_STRING(&new_header);
425+
} else {
426+
// Only some headers were redacted
427+
// The zval_ptr_dtor call will ensure that the redacted headers
428+
// are freed;
429+
// Make sure that we don't read garbage
430+
ZSTR_VAL(redacted)[write_offset] = '\0';
431+
ZSTR_LEN(redacted) = write_offset;
432+
433+
ZVAL_STR(&new_header, redacted);
434+
}
435+
php_stream_context_set_option(ctx->context, "http", "header", &new_header);
436+
zval_ptr_dtor(&new_header);
287437
}
288438

289439
void sdl_restore_uri_credentials(sdlCtx *ctx)

‎ext/soap/tests/gh23686/header-just-lf-separator.phpt‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const LOGS_PATH = __DIR__ . "/header-just-lf-separator-logs.txt";
2323

2424
// Headers reach sdl_set_uri_credentials() as a string
2525
$context = stream_context_create([
26-
'http' => ['header' => "Authorization: Basic\nFoo: bar"],
26+
'http' => ['header' => "Authorization: Basic\nFoo: bar\nProxy-Authorization: Something\nBaz: Qux\nCookie: Tasty"],
2727
]);
2828

2929
echo "Headers are in a string:\n";
@@ -35,7 +35,7 @@ $context = stream_context_create([
3535
'http' => [
3636
// having a protocol here means that SOAP won't add it itself
3737
'protocol_version' => 1.1,
38-
'header' => "Authorization: Basic\nFoo: bar"
38+
'header' => "Authorization: Basic\nFoo: bar\nProxy-Authorization: Something\nBaz: Qux\nCookie: Tasty"
3939
],
4040
]);
4141

@@ -50,44 +50,48 @@ string(%d) "array (
5050
'Host' => 'localhost:%d',
5151
'Connection' => 'close',
5252
'Foo' => 'bar',
53+
'Baz' => 'Qux',
5354
)"
5455
string(%d) "array (
5556
'Host' => 'localhost:%d',
5657
'Connection' => 'close',
5758
'Foo' => 'bar',
59+
'Baz' => 'Qux',
5860
)"
5961
string(%d) "array (
6062
'Host' => 'localhost:%d',
6163
'Connection' => 'close',
6264
'Foo' => 'bar',
65+
'Baz' => 'Qux',
6366
)"
6467
string(%d) "array (
6568
'Host' => 'localhost:%d',
6669
'Connection' => 'close',
6770
'Foo' => 'bar',
71+
'Baz' => 'Qux',
6872
)"
6973
Headers are in an array:
7074
string(%d) "array (
7175
'Host' => 'localhost:%d',
7276
'Connection' => 'close',
73-
'Authorization' => 'Basic',
7477
'Foo' => 'bar',
78+
'Baz' => 'Qux',
7579
)"
7680
string(%d) "array (
7781
'Host' => 'localhost:%d',
7882
'Connection' => 'close',
79-
'Authorization' => 'Basic',
8083
'Foo' => 'bar',
84+
'Baz' => 'Qux',
8185
)"
8286
string(%d) "array (
8387
'Host' => 'localhost:%d',
8488
'Connection' => 'close',
85-
'Authorization' => 'Basic',
8689
'Foo' => 'bar',
90+
'Baz' => 'Qux',
8791
)"
8892
string(%d) "array (
8993
'Host' => 'localhost:%d',
9094
'Connection' => 'close',
91-
'Authorization' => 'Basic',
9295
'Foo' => 'bar',
96+
'Baz' => 'Qux',
9397
)"

‎ext/soap/tests/gh23686/import-authorization-basic-lowercase.phpt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,8 @@ check_headers_for_import(LOGS_PATH, $context);
3232
string(%d) "array (
3333
'Host' => 'localhost:%d',
3434
'Connection' => 'close',
35-
'authorization' => 'Basic foo bar',
3635
)"
3736
string(%d) "array (
3837
'Host' => 'localhost:%d',
3938
'Connection' => 'close',
40-
'authorization' => 'Basic foo bar',
4139
)"

‎ext/soap/tests/gh23686/import-authorization-bearer.phpt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,8 @@ check_headers_for_import(LOGS_PATH, $context);
3232
string(%d) "array (
3333
'Host' => 'localhost:%d',
3434
'Connection' => 'close',
35-
'Authorization' => 'Bearer secret-token',
3635
)"
3736
string(%d) "array (
3837
'Host' => 'localhost:%d',
3938
'Connection' => 'close',
40-
'Authorization' => 'Bearer secret-token',
4139
)"

‎ext/soap/tests/gh23686/import-authorization-digest.phpt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,8 @@ check_headers_for_import(LOGS_PATH, $context);
3232
string(%d) "array (
3333
'Host' => 'localhost:%d',
3434
'Connection' => 'close',
35-
'Authorization' => 'Digest secret-token',
3635
)"
3736
string(%d) "array (
3837
'Host' => 'localhost:%d',
3938
'Connection' => 'close',
40-
'Authorization' => 'Digest secret-token',
4139
)"

‎ext/soap/tests/gh23686/import-cookies.phpt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,8 @@ check_headers_for_import(LOGS_PATH, $context);
3232
string(%d) "array (
3333
'Host' => 'localhost:%d',
3434
'Connection' => 'close',
35-
'Cookie' => 'foo=bar',
3635
)"
3736
string(%d) "array (
3837
'Host' => 'localhost:%d',
3938
'Connection' => 'close',
40-
'Cookie' => 'foo=bar',
4139
)"

‎ext/soap/tests/gh23686/import-proxy-authorization.phpt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,8 @@ check_headers_for_import(LOGS_PATH, $context);
3232
string(%d) "array (
3333
'Host' => 'localhost:%d',
3434
'Connection' => 'close',
35-
'Proxy-Authorization' => 'FooBar',
3635
)"
3736
string(%d) "array (
3837
'Host' => 'localhost:%d',
3938
'Connection' => 'close',
40-
'Proxy-Authorization' => 'FooBar',
4139
)"

‎ext/soap/tests/gh23686/import-repeated-authorization.phpt‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
GH-23686: Regression tests for repeated authorization headers (which doesn't work) (`<import>`)
2+
GH-23686: Regression tests for repeated authorization headers (which previously didn't work) (`<import>`)
33
--EXTENSIONS--
44
soap
55
--INI--
@@ -32,13 +32,11 @@ check_headers_for_import(LOGS_PATH, $context, false);
3232

3333
?>
3434
--EXPECTF--
35-
SoapFault: SOAP-ERROR: Parsing WSDL: Couldn't load from 'http://localhost:%d/index.php' : failed to load external entity "http://localhost:%d/index.php"
36-
37-
bool(false)
38-
SoapFault: SOAP-ERROR: Parsing WSDL: Couldn't load from 'http://localhost:%d/index.php' : failed to load external entity "http://localhost:%d/index.php"
39-
40-
bool(false)
41-
Server exited with non-zero status: 1
42-
43-
Warning: file_get_contents(): Failed to open stream: No such file or directory in %s on line %d
44-
Server output:
35+
string(%d) "array (
36+
'Host' => 'localhost:%d',
37+
'Connection' => 'close',
38+
)"
39+
string(%d) "array (
40+
'Host' => 'localhost:%d',
41+
'Connection' => 'close',
42+
)"

‎ext/soap/tests/gh23686/import-repeated-cookies.phpt‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
GH-23686: Regression tests for repeated cookies headers (which doesn't work) (`<import>`)
2+
GH-23686: Regression tests for repeated cookies headers (which previously didn't work) (`<import>`)
33
--EXTENSIONS--
44
soap
55
--INI--
@@ -32,13 +32,11 @@ check_headers_for_import(LOGS_PATH, $context, false);
3232

3333
?>
3434
--EXPECTF--
35-
SoapFault: SOAP-ERROR: Parsing WSDL: Couldn't load from 'http://localhost:%d/index.php' : failed to load external entity "http://localhost:%d/index.php"
36-
37-
bool(false)
38-
SoapFault: SOAP-ERROR: Parsing WSDL: Couldn't load from 'http://localhost:%d/index.php' : failed to load external entity "http://localhost:%d/index.php"
39-
40-
bool(false)
41-
Server exited with non-zero status: 1
42-
43-
Warning: file_get_contents(): Failed to open stream: No such file or directory in %s on line %d
44-
Server output:
35+
string(%d) "array (
36+
'Host' => 'localhost:%d',
37+
'Connection' => 'close',
38+
)"
39+
string(%d) "array (
40+
'Host' => 'localhost:%d',
41+
'Connection' => 'close',
42+
)"

0 commit comments

Comments
 (0)