okhttp: Exception for bad header can leak an Authorization secret

If there’s an illegal character in a header value, an IllegalArgumentException is thrown whose message includes the full header value. This is particularly bad if the header name is Authorization or Proxy-Authorization, which strongly suggests that the value is sensitive information.

Just because it contains a non-printable character that can’t possibly work in HTTP doesn’t mean it doesn’t also contain sensitive information: consider a scenario where an application reads a credential from a configuration file or an environment variable, but fails to trim a newline that was accidentally included by whatever logic provided the value. It’s the same reason why one wouldn’t want to print an “invalid password” message that included the password in cleartext: because the invalid one might be just a slight mistyping of the valid one.

java.lang.IllegalArgumentException: Unexpected char 0x0a at 20 in Authorization value: very-secret-password\n

This is especially risky since it is an unchecked exception. It’s common in Java for applications to have a fallback catch block to intercept any unchecked exceptions that have propagated out of lower-level code, and since those presumably represent some kind of unexpected condition that wasn’t accounted for, a typical response is to log them.

I would strongly suggest that these exceptions never include the header value, but only the name (and maybe, as you’re also doing now, what the illegal character was); that (plus the stacktrace) is likely to be enough to tell the developer where the defect is. But at the very least I would suppress the value if the header is Authorization or Proxy-Authorization. I realize you’re not literally logging the value yourself, but an unchecked exception is like a flare gun— one has to assume that it might be visible to someone other than the person you’re having a conversation with.

To reproduce:

package com.launchdarkly.eventsource;

import okhttp3.*;
import org.junit.Test;
import static org.hamcrest.MatcherAssert.*;
import static org.hamcrest.Matchers.*;

public class OkhttpHeaderExceptionTest {
  @Test
  public void invalidHeaderValueIsCapturedInException() throws Exception {
    String password = "very-secret-password";
    String badValue = password + "\n";
    
    try {
      Request req = new Request.Builder().url("http://github.com/path/doesnt/matter")
          .header("Authorization", badValue)
          .build();
    } catch (IllegalArgumentException e) {
      assertThat(e.getMessage(), not(containsString(password)));
    }
  }
}