From be98a12cd040785fe77b4bceb379fef3c73ee827 Mon Sep 17 00:00:00 2001
From: Jason Tedor <jason@tedor.me>
Date: Tue, 16 Jul 2019 16:12:36 +0900
Subject: [PATCH] Do not swallow I/O exception getting authentication (#44398)

When getting authentication info from the thread context, it might be
that we encounter an I/O exception. Today we swallow this exception and
return a null authentication info to the caller. Yet, this could be
hiding bugs or errors. This commits adjusts this behavior so that we no
longer swallow the exception.
---
 .../xpack/core/security/SecurityContext.java        |  7 +++----
 .../xpack/security/SecurityContextTests.java        | 13 +++++++++++++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java
index 0da07a52996..e1922ca87ea 100644
--- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java
+++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java
@@ -5,8 +5,8 @@
  */
 package org.elasticsearch.xpack.core.security;
 
-import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.elasticsearch.Version;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
@@ -17,6 +17,7 @@ import org.elasticsearch.xpack.core.security.authc.Authentication.Authentication
 import org.elasticsearch.xpack.core.security.user.User;
 
 import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.util.Collections;
 import java.util.Objects;
 import java.util.function.Consumer;
@@ -53,10 +54,8 @@ public class SecurityContext {
         try {
             return Authentication.readFromContext(threadContext);
         } catch (IOException e) {
-            // TODO: this seems bogus, the only way to get an ioexception here is from a corrupt or tampered
-            // auth header, which should be be audited?
             logger.error("failed to read authentication", e);
-            return null;
+            throw new UncheckedIOException(e);
         }
     }
 
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityContextTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityContextTests.java
index 8fa280b68de..fd0c4d8f459 100644
--- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityContextTests.java
+++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityContextTests.java
@@ -15,13 +15,18 @@ import org.elasticsearch.xpack.core.security.SecurityContext;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
 import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType;
 import org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef;
+import org.elasticsearch.xpack.core.security.authc.AuthenticationField;
 import org.elasticsearch.xpack.core.security.user.SystemUser;
 import org.elasticsearch.xpack.core.security.user.User;
 import org.junit.Before;
 
+import java.io.EOFException;
 import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.util.concurrent.atomic.AtomicReference;
 
+import static org.hamcrest.Matchers.instanceOf;
+
 public class SecurityContextTests extends ESTestCase {
 
     private Settings settings;
@@ -51,6 +56,14 @@ public class SecurityContextTests extends ESTestCase {
         assertEquals(user, securityContext.getUser());
     }
 
+    public void testGetAuthenticationDoesNotSwallowIOException() {
+        threadContext.putHeader(AuthenticationField.AUTHENTICATION_KEY, ""); // an intentionally corrupt header
+        final SecurityContext securityContext = new SecurityContext(Settings.EMPTY, threadContext);
+        final UncheckedIOException e = expectThrows(UncheckedIOException.class, securityContext::getAuthentication);
+        assertNotNull(e.getCause());
+        assertThat(e.getCause(), instanceOf(EOFException.class));
+    }
+
     public void testSetUser() {
         final User user = new User("test");
         assertNull(securityContext.getAuthentication());