Browse Source

Update authc failure headers on license change (#61734)

DefaultAuthenticationFailureHandler determines the set of response headers based on the active realms at startup.
However, the Realms list may be empty if the node thinks it has a TRIAL or BASIC license at startup (because we default security off in those cases).
The handler is never updated if the license state changes.

The fix is to add a call-back for license change to update the AuthenticationFailureHandler's Headers to reflect the new license property.
Resolves: #56318
Lyudmila Fokina 5 years ago
parent
commit
183533734b

+ 10 - 1
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandler.java

@@ -27,7 +27,7 @@ import static org.elasticsearch.xpack.core.security.support.Exceptions.authentic
  * response headers like 'WWW-Authenticate'
  */
 public class DefaultAuthenticationFailureHandler implements AuthenticationFailureHandler {
-    private final Map<String, List<String>> defaultFailureResponseHeaders;
+    private volatile Map<String, List<String>> defaultFailureResponseHeaders;
 
     /**
      * Constructs default authentication failure handler with provided default
@@ -55,6 +55,15 @@ public class DefaultAuthenticationFailureHandler implements AuthenticationFailur
         }
     }
 
+    /**
+     * This method is called when failureResponseHeaders need to be set (at startup) or updated (if license state changes)
+     *
+     * @param failureResponseHeaders the Map of failure response headers to be set
+     */
+    public void setHeaders(Map<String, List<String>> failureResponseHeaders){
+        defaultFailureResponseHeaders = failureResponseHeaders;
+    }
+
     /**
      * For given 'WWW-Authenticate' header value returns the priority based on
      * the auth-scheme. Lower number denotes more secure and preferred

+ 30 - 22
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

@@ -542,32 +542,40 @@ public class Security extends Plugin implements SystemIndexPlugin, IngestPlugin,
         }
         if (failureHandler == null) {
             logger.debug("Using default authentication failure handler");
-            final Map<String, List<String>> defaultFailureResponseHeaders = new HashMap<>();
-            realms.asList().stream().forEach((realm) -> {
-                Map<String, List<String>> realmFailureHeaders = realm.getAuthenticationFailureHeaders();
-                realmFailureHeaders.entrySet().stream().forEach((e) -> {
-                    String key = e.getKey();
-                    e.getValue().stream()
-                            .filter(v -> defaultFailureResponseHeaders.computeIfAbsent(key, x -> new ArrayList<>()).contains(v) == false)
-                            .forEach(v -> defaultFailureResponseHeaders.get(key).add(v));
+            Supplier<Map<String, List<String>>> headersSupplier = () -> {
+                final Map<String, List<String>> defaultFailureResponseHeaders = new HashMap<>();
+                realms.asList().stream().forEach((realm) -> {
+                    Map<String, List<String>> realmFailureHeaders = realm.getAuthenticationFailureHeaders();
+                    realmFailureHeaders.entrySet().stream().forEach((e) -> {
+                        String key = e.getKey();
+                        e.getValue().stream()
+                                .filter(v -> defaultFailureResponseHeaders.computeIfAbsent(key, x -> new ArrayList<>()).contains(v)
+                                    == false)
+                                .forEach(v -> defaultFailureResponseHeaders.get(key).add(v));
+                    });
                 });
-            });
 
-            if (TokenService.isTokenServiceEnabled(settings)) {
-                String bearerScheme = "Bearer realm=\"" + XPackField.SECURITY + "\"";
-                if (defaultFailureResponseHeaders.computeIfAbsent("WWW-Authenticate", x -> new ArrayList<>())
-                        .contains(bearerScheme) == false) {
-                    defaultFailureResponseHeaders.get("WWW-Authenticate").add(bearerScheme);
+                if (TokenService.isTokenServiceEnabled(settings)) {
+                    String bearerScheme = "Bearer realm=\"" + XPackField.SECURITY + "\"";
+                    if (defaultFailureResponseHeaders.computeIfAbsent("WWW-Authenticate", x -> new ArrayList<>())
+                            .contains(bearerScheme) == false) {
+                        defaultFailureResponseHeaders.get("WWW-Authenticate").add(bearerScheme);
+                    }
                 }
-            }
-            if (API_KEY_SERVICE_ENABLED_SETTING.get(settings)) {
-                final String apiKeyScheme = "ApiKey";
-                if (defaultFailureResponseHeaders.computeIfAbsent("WWW-Authenticate", x -> new ArrayList<>())
-                    .contains(apiKeyScheme) == false) {
-                    defaultFailureResponseHeaders.get("WWW-Authenticate").add(apiKeyScheme);
+                if (API_KEY_SERVICE_ENABLED_SETTING.get(settings)) {
+                    final String apiKeyScheme = "ApiKey";
+                    if (defaultFailureResponseHeaders.computeIfAbsent("WWW-Authenticate", x -> new ArrayList<>())
+                        .contains(apiKeyScheme) == false) {
+                        defaultFailureResponseHeaders.get("WWW-Authenticate").add(apiKeyScheme);
+                    }
                 }
-            }
-            failureHandler = new DefaultAuthenticationFailureHandler(defaultFailureResponseHeaders);
+                return defaultFailureResponseHeaders;
+            };
+            DefaultAuthenticationFailureHandler finalDefaultFailureHandler = new DefaultAuthenticationFailureHandler(headersSupplier.get());
+            failureHandler = finalDefaultFailureHandler;
+            getLicenseState().addListener(() -> {
+                finalDefaultFailureHandler.setHeaders(headersSupplier.get());
+            });
         } else {
             logger.debug("Using authentication failure handler from extension [" + extensionName + "]");
         }

+ 69 - 8
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java

@@ -5,7 +5,9 @@
  */
 package org.elasticsearch.xpack.security;
 
+import org.elasticsearch.ElasticsearchSecurityException;
 import org.elasticsearch.Version;
+import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterState;
@@ -28,11 +30,14 @@ import org.elasticsearch.license.License;
 import org.elasticsearch.license.TestUtils;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.plugins.MapperPlugin;
+import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.VersionUtils;
+import org.elasticsearch.test.rest.FakeRestRequest;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.watcher.ResourceWatcherService;
+import org.elasticsearch.xpack.core.XPackField;
 import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.security.SecurityExtension;
 import org.elasticsearch.xpack.core.security.SecurityField;
@@ -48,6 +53,7 @@ import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissionsDe
 import org.elasticsearch.xpack.core.ssl.SSLService;
 import org.elasticsearch.xpack.security.audit.AuditTrailService;
 import org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail;
+import org.elasticsearch.xpack.security.authc.AuthenticationService;
 import org.elasticsearch.xpack.security.authc.Realms;
 import org.hamcrest.Matchers;
 import org.junit.After;
@@ -60,6 +66,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BiConsumer;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -72,6 +79,8 @@ import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.notNullValue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -94,14 +103,7 @@ public class SecurityTests extends ESTestCase {
         }
     }
 
-    private Collection<Object> createComponents(Settings testSettings, SecurityExtension... extensions) throws Exception {
-        if (security != null) {
-            throw new IllegalStateException("Security object already exists (" + security + ")");
-        }
-        Settings settings = Settings.builder()
-            .put("xpack.security.enabled", true)
-            .put(testSettings)
-            .put("path.home", createTempDir()).build();
+    private Collection<Object> createComponentsUtil(Settings settings, SecurityExtension... extensions) throws Exception {
         Environment env = TestEnvironment.newEnvironment(settings);
         licenseState = new TestUtils.UpdatableLicenseState(settings);
         SSLService sslService = new SSLService(env);
@@ -133,6 +135,28 @@ public class SecurityTests extends ESTestCase {
             xContentRegistry(), env, new IndexNameExpressionResolver());
     }
 
+    private Collection<Object> createComponentsWithSecurityNotExplicitlyEnabled(Settings testSettings, SecurityExtension... extensions)
+        throws Exception {
+        if (security != null) {
+            throw new IllegalStateException("Security object already exists (" + security + ")");
+        }
+        Settings settings = Settings.builder()
+            .put(testSettings)
+            .put("path.home", createTempDir()).build();
+        return createComponentsUtil(settings, extensions);
+    }
+
+    private Collection<Object> createComponents(Settings testSettings, SecurityExtension... extensions) throws Exception {
+        if (security != null) {
+            throw new IllegalStateException("Security object already exists (" + security + ")");
+        }
+        Settings settings = Settings.builder()
+            .put("xpack.security.enabled", true)
+            .put(testSettings)
+            .put("path.home", createTempDir()).build();
+        return createComponentsUtil(settings, extensions);
+    }
+
     private static <T> T findComponent(Class<T> type, Collection<Object> components) {
         for (Object obj : components) {
             if (type.isInstance(obj)) {
@@ -462,4 +486,41 @@ public class SecurityTests extends ESTestCase {
         Security.validateForFips(settings);
         // no exception thrown
     }
+
+    public void testLicenseUpdateFailureHandlerUpdate() throws Exception {
+        Settings settings = Settings.builder().
+            put("xpack.security.authc.api_key.enabled", "true").
+            build();
+        Collection<Object> components = createComponentsWithSecurityNotExplicitlyEnabled(settings);
+        AuthenticationService service = findComponent(AuthenticationService.class, components);
+        assertNotNull(service);
+        RestRequest request = new FakeRestRequest();
+        final AtomicBoolean completed = new AtomicBoolean(false);
+        service.authenticate(request, ActionListener.wrap(result -> {
+            assertTrue(completed.compareAndSet(false, true));
+        }, this::logAndFail));
+        assertTrue(completed.compareAndSet(true, false));
+        threadContext.stashContext();
+        licenseState.update(
+            randomFrom(License.OperationMode.GOLD, License.OperationMode.ENTERPRISE, License.OperationMode.PLATINUM),
+            true, null);
+        service.authenticate(request, ActionListener.wrap(result -> {
+            assertTrue(completed.compareAndSet(false, true));
+        }, this::VerifyBasicAuthenticationHeader));
+        if(completed.get()){
+            fail("authentication succeeded but it shouldn't");
+        }
+    }
+
+    private void logAndFail(Exception e) {
+        logger.error("unexpected exception", e);
+        fail("unexpected exception " + e.getMessage());
+    }
+
+    private void VerifyBasicAuthenticationHeader(Exception e) {
+        assertThat(e, instanceOf(ElasticsearchSecurityException.class));
+        assertThat(((ElasticsearchSecurityException) e).getHeader("WWW-Authenticate"), notNullValue());
+        assertThat(((ElasticsearchSecurityException) e).getHeader("WWW-Authenticate"),
+            hasItem("Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\""));
+    }
 }