From 2950cd8e01742c3c94c566d827680ba1bd60dafc Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Mon, 8 Feb 2021 12:44:51 -0500 Subject: [PATCH] Make it more difficult to accidentally log sensitive headers (#6551) --- okhttp/src/main/kotlin/okhttp3/Headers.kt | 29 ++++++++--- .../src/main/kotlin/okhttp3/internal/Util.kt | 9 +++- okhttp/src/test/java/okhttp3/HeadersTest.java | 49 +++++++++++++++++++ 3 files changed, 80 insertions(+), 7 deletions(-) diff --git a/okhttp/src/main/kotlin/okhttp3/Headers.kt b/okhttp/src/main/kotlin/okhttp3/Headers.kt index db1f36edaddd..cd5e9a3aa2d8 100644 --- a/okhttp/src/main/kotlin/okhttp3/Headers.kt +++ b/okhttp/src/main/kotlin/okhttp3/Headers.kt @@ -28,6 +28,7 @@ import okhttp3.Headers.Builder import okhttp3.internal.format import okhttp3.internal.http.toHttpDateOrNull import okhttp3.internal.http.toHttpDateString +import okhttp3.internal.isSensitiveHeader import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement /** @@ -180,12 +181,27 @@ class Headers private constructor( override fun hashCode(): Int = namesAndValues.contentHashCode() + /** + * Returns header names and values. The names and values are separated by `: ` and each pair is + * followed by a newline character `\n`. + * + * Since OkHttp 5 this redacts these sensitive headers: + * + * * `Authorization` + * * `Cookie` + * * `Proxy-Authorization` + * * `Set-Cookie` + * + * To get all headers as a human-readable string use `toMultimap().toString()`. + */ override fun toString(): String { return buildString { for (i in 0 until size) { - append(name(i)) + val name = name(i) + val value = value(i) + append(name) append(": ") - append(value(i)) + append(if (isSensitiveHeader(name)) "██" else value) append("\n") } } @@ -370,7 +386,7 @@ class Headers private constructor( } // Check for malformed headers. - for (i in 0 until namesAndValues.size step 2) { + for (i in namesAndValues.indices step 2) { val name = namesAndValues[i] val value = namesAndValues[i + 1] checkName(name) @@ -420,7 +436,7 @@ class Headers private constructor( private fun checkName(name: String) { require(name.isNotEmpty()) { "name is empty" } - for (i in 0 until name.length) { + for (i in name.indices) { val c = name[i] require(c in '\u0021'..'\u007e') { format("Unexpected char %#04x at %d in header name: %s", c.toInt(), i, name) @@ -429,10 +445,11 @@ class Headers private constructor( } private fun checkValue(value: String, name: String) { - for (i in 0 until value.length) { + for (i in value.indices) { val c = value[i] require(c == '\t' || c in '\u0020'..'\u007e') { - format("Unexpected char %#04x at %d in %s value: %s", c.toInt(), i, name, value) + format("Unexpected char %#04x at %d in %s value", c.toInt(), i, name) + + (if (isSensitiveHeader(name)) "" else ": $value") } } } diff --git a/okhttp/src/main/kotlin/okhttp3/internal/Util.kt b/okhttp/src/main/kotlin/okhttp3/internal/Util.kt index 1383feae6599..a2d055ec0999 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/Util.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/Util.kt @@ -38,7 +38,6 @@ import java.util.concurrent.ThreadFactory import java.util.concurrent.TimeUnit import kotlin.text.Charsets.UTF_32BE import kotlin.text.Charsets.UTF_32LE -import okhttp3.Call import okhttp3.EventListener import okhttp3.Headers import okhttp3.Headers.Companion.headersOf @@ -248,6 +247,14 @@ fun String.canParseAsIpAddress(): Boolean { return VERIFY_AS_IP_ADDRESS.matches(this) } +/** Returns true if we should void putting this this header in an exception or toString(). */ +fun isSensitiveHeader(name: String): Boolean { + return name.equals("Authorization", ignoreCase = true) || + name.equals("Cookie", ignoreCase = true) || + name.equals("Proxy-Authorization", ignoreCase = true) || + name.equals("Set-Cookie", ignoreCase = true) +} + /** Returns a [Locale.US] formatted [String]. */ fun format(format: String, vararg args: Any): String { return String.format(Locale.US, format, *args) diff --git a/okhttp/src/test/java/okhttp3/HeadersTest.java b/okhttp/src/test/java/okhttp3/HeadersTest.java index cbb86ae8c570..f60b1961e4f7 100644 --- a/okhttp/src/test/java/okhttp3/HeadersTest.java +++ b/okhttp/src/test/java/okhttp3/HeadersTest.java @@ -380,6 +380,37 @@ public final class HeadersTest { } } + @Test public void sensitiveHeadersNotIncludedInExceptions() { + try { + new Headers.Builder().add("Authorization", "valué1"); + fail("Should have complained about invalid name"); + } catch (IllegalArgumentException expected) { + assertThat(expected.getMessage()).isEqualTo( + "Unexpected char 0xe9 at 4 in Authorization value"); + } + try { + new Headers.Builder().add("Cookie", "valué1"); + fail("Should have complained about invalid name"); + } catch (IllegalArgumentException expected) { + assertThat(expected.getMessage()).isEqualTo( + "Unexpected char 0xe9 at 4 in Cookie value"); + } + try { + new Headers.Builder().add("Proxy-Authorization", "valué1"); + fail("Should have complained about invalid name"); + } catch (IllegalArgumentException expected) { + assertThat(expected.getMessage()).isEqualTo( + "Unexpected char 0xe9 at 4 in Proxy-Authorization value"); + } + try { + new Headers.Builder().add("Set-Cookie", "valué1"); + fail("Should have complained about invalid name"); + } catch (IllegalArgumentException expected) { + assertThat(expected.getMessage()).isEqualTo( + "Unexpected char 0xe9 at 4 in Set-Cookie value"); + } + } + @Test public void headersEquals() { Headers headers1 = new Headers.Builder() .add("Connection", "close") @@ -414,6 +445,24 @@ public final class HeadersTest { assertThat(headers.toString()).isEqualTo("A: a\nB: bb\n"); } + @Test public void headersToStringRedactsSensitiveHeaders() { + Headers headers = new Headers.Builder() + .add("content-length", "99") + .add("authorization", "peanutbutter") + .add("proxy-authorization", "chocolate") + .add("cookie", "drink=coffee") + .add("set-cookie", "accessory=sugar") + .add("user-agent", "OkHttp") + .build(); + assertThat(headers.toString()).isEqualTo("" + + "content-length: 99\n" + + "authorization: ██\n" + + "proxy-authorization: ██\n" + + "cookie: ██\n" + + "set-cookie: ██\n" + + "user-agent: OkHttp\n"); + } + @Test public void headersAddAll() { Headers sourceHeaders = new Headers.Builder() .add("A", "aa")