From 3508b6c641e9f984d9612b820574fa52fce1ecb6 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Tue, 7 May 2019 09:55:48 +1000 Subject: [PATCH] Log warning when unlicensed realms are skipped (#41828) Because realms are configured at node startup, but license levels can change dynamically, it is possible to have a running node that has a particular realm type configured, but that realm is not permitted under the current license. In this case the realm is silently ignored during authentication. This commit adds a warning in the elasticsearch logs if authentication fails, and there are realms that have been skipped due to licensing. This message is not intended to imply that the realms could (or would) have successfully authenticated the user, but they may help reduce confusion about why authentication failed if the caller was expecting the authentication to be handled by a particular realm that is in fact unlicensed. Backport of: #41778 --- .../security/authc/AuthenticationService.java | 10 ++- .../xpack/security/authc/Realms.java | 57 ++++++++++---- .../xpack/security/authc/RealmsTests.java | 74 ++++++++++++++++++- 3 files changed, 123 insertions(+), 18 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java index b76d3a48035..1fe3ed67f73 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java @@ -14,6 +14,7 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ContextPreservingActionListener; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.cache.Cache; import org.elasticsearch.common.cache.CacheBuilder; import org.elasticsearch.common.collect.Tuple; @@ -35,6 +36,7 @@ import org.elasticsearch.xpack.core.security.authc.AuthenticationResult; import org.elasticsearch.xpack.core.security.authc.AuthenticationServiceField; import org.elasticsearch.xpack.core.security.authc.AuthenticationToken; import org.elasticsearch.xpack.core.security.authc.Realm; +import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.EmptyAuthorizationInfo; import org.elasticsearch.xpack.core.security.support.Exceptions; import org.elasticsearch.xpack.core.security.user.AnonymousUser; import org.elasticsearch.xpack.core.security.user.SystemUser; @@ -43,7 +45,6 @@ import org.elasticsearch.xpack.security.audit.AuditTrail; import org.elasticsearch.xpack.security.audit.AuditTrailService; import org.elasticsearch.xpack.security.audit.AuditUtil; import org.elasticsearch.xpack.security.authc.support.RealmUserLookup; -import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.EmptyAuthorizationInfo; import org.elasticsearch.xpack.security.support.SecurityIndexManager; import java.util.ArrayList; @@ -484,6 +485,13 @@ public class AuthenticationService { final String cause = tuple.v2() == null ? "" : " (Caused by " + tuple.v2() + ")"; logger.warn("Authentication to realm {} failed - {}{}", realm.name(), message, cause); }); + List unlicensedRealms = realms.getUnlicensedRealms(); + if (unlicensedRealms.isEmpty() == false) { + logger.warn("Authentication failed using realms [{}]." + + " Realms [{}] were skipped because they are not permitted on the current license", + Strings.collectionToCommaDelimitedString(defaultOrderedRealmList), + Strings.collectionToCommaDelimitedString(unlicensedRealms)); + } listener.onFailure(request.authenticationFailed(authenticationToken)); } else { threadContext.putTransient(AuthenticationResult.THREAD_CONTEXT_KEY, authenticationResult); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Realms.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Realms.java index 925654fae8b..39b981b42e3 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Realms.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Realms.java @@ -5,24 +5,8 @@ */ package org.elasticsearch.xpack.security.authc; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.Collectors; -import java.util.stream.Stream; -import java.util.stream.StreamSupport; - import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; - import org.elasticsearch.action.ActionListener; import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.common.settings.Settings; @@ -39,6 +23,21 @@ import org.elasticsearch.xpack.core.security.authc.file.FileRealmSettings; import org.elasticsearch.xpack.core.security.authc.kerberos.KerberosRealmSettings; import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; + /** * Serves as a realms registry (also responsible for ordering the realms appropriately) */ @@ -119,6 +118,32 @@ public class Realms implements Iterable { } } + /** + * Returns a list of realms that are configured, but are not permitted under the current license. + */ + public List getUnlicensedRealms() { + // If auth is not allowed, then everything is unlicensed + if (licenseState.isAuthAllowed() == false) { + return Collections.unmodifiableList(realms); + } + + AllowedRealmType allowedRealmType = licenseState.allowedRealmType(); + // If all realms are allowed, then nothing is unlicensed + if (allowedRealmType == AllowedRealmType.ALL) { + return Collections.emptyList(); + } + + final List allowedRealms = this.asList(); + // Shortcut for the typical case, all the configured realms are allowed + if (allowedRealms.equals(this.realms.size())) { + return Collections.emptyList(); + } + + // Otherwise, we return anything in "all realms" that is not in the allowed realm list + List unlicensed = realms.stream().filter(r -> allowedRealms.contains(r) == false).collect(Collectors.toList()); + return Collections.unmodifiableList(unlicensed); + } + public Stream stream() { return StreamSupport.stream(this.spliterator(), false); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/RealmsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/RealmsTests.java index c37d6913d1f..a26e05e5234 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/RealmsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/RealmsTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings; import org.elasticsearch.xpack.core.security.authc.file.FileRealmSettings; import org.elasticsearch.xpack.core.security.authc.kerberos.KerberosRealmSettings; import org.elasticsearch.xpack.core.security.authc.ldap.LdapRealmSettings; +import org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings; import org.elasticsearch.xpack.core.security.authc.saml.SamlRealmSettings; import org.elasticsearch.xpack.core.security.user.User; import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm; @@ -39,10 +40,13 @@ import java.util.TreeMap; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.iterableWithSize; import static org.hamcrest.Matchers.notNullValue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -104,6 +108,8 @@ public class RealmsTests extends ESTestCase { assertThat(realm.name(), equalTo("realm_" + index)); i++; } + + assertThat(realms.getUnlicensedRealms(), empty()); } public void testWithSettingsWhereDifferentRealmsHaveSameOrder() throws Exception { @@ -142,6 +148,8 @@ public class RealmsTests extends ESTestCase { assertThat(realm.type(), equalTo("type_" + nameToRealmId.get(expectedRealmName))); assertThat(realm.name(), equalTo(expectedRealmName)); } + + assertThat(realms.getUnlicensedRealms(), empty()); } public void testWithSettingsWithMultipleInternalRealmsOfSameType() throws Exception { @@ -175,6 +183,8 @@ public class RealmsTests extends ESTestCase { assertThat(realm.type(), equalTo(NativeRealmSettings.TYPE)); assertThat(realm.name(), equalTo("default_" + NativeRealmSettings.TYPE)); assertThat(iter.hasNext(), is(false)); + + assertThat(realms.getUnlicensedRealms(), empty()); } public void testUnlicensedWithOnlyCustomRealms() throws Exception { @@ -209,6 +219,8 @@ public class RealmsTests extends ESTestCase { i++; } + assertThat(realms.getUnlicensedRealms(), empty()); + when(licenseState.allowedRealmType()).thenReturn(AllowedRealmType.DEFAULT); iter = realms.iterator(); @@ -225,6 +237,18 @@ public class RealmsTests extends ESTestCase { assertThat(realm.name(), equalTo("default_" + NativeRealmSettings.TYPE)); assertThat(iter.hasNext(), is(false)); + assertThat(realms.getUnlicensedRealms(), iterableWithSize(randomRealmTypesCount)); + iter = realms.getUnlicensedRealms().iterator(); + i = 0; + while (iter.hasNext()) { + realm = iter.next(); + assertThat(realm.order(), equalTo(i)); + int index = orderToIndex.get(i); + assertThat(realm.type(), equalTo("type_" + index)); + assertThat(realm.name(), equalTo("realm_" + index)); + i++; + } + when(licenseState.allowedRealmType()).thenReturn(AllowedRealmType.NATIVE); iter = realms.iterator(); @@ -240,6 +264,18 @@ public class RealmsTests extends ESTestCase { assertThat(realm.type(), equalTo(NativeRealmSettings.TYPE)); assertThat(realm.name(), equalTo("default_" + NativeRealmSettings.TYPE)); assertThat(iter.hasNext(), is(false)); + + assertThat(realms.getUnlicensedRealms(), iterableWithSize(randomRealmTypesCount)); + iter = realms.getUnlicensedRealms().iterator(); + i = 0; + while (iter.hasNext()) { + realm = iter.next(); + assertThat(realm.order(), equalTo(i)); + int index = orderToIndex.get(i); + assertThat(realm.type(), equalTo("type_" + index)); + assertThat(realm.name(), equalTo("realm_" + index)); + i++; + } } public void testUnlicensedWithInternalRealms() throws Exception { @@ -266,6 +302,7 @@ public class RealmsTests extends ESTestCase { types.add(realm.type()); } assertThat(types, contains("ldap", "type_0")); + assertThat(realms.getUnlicensedRealms(), empty()); when(licenseState.allowedRealmType()).thenReturn(AllowedRealmType.DEFAULT); iter = realms.iterator(); @@ -280,6 +317,11 @@ public class RealmsTests extends ESTestCase { } assertThat(i, is(1)); + assertThat(realms.getUnlicensedRealms(), iterableWithSize(1)); + realm = realms.getUnlicensedRealms().get(0); + assertThat(realm.type(), equalTo("type_0")); + assertThat(realm.name(), equalTo("custom")); + when(licenseState.allowedRealmType()).thenReturn(AllowedRealmType.NATIVE); iter = realms.iterator(); assertThat(iter.hasNext(), is(true)); @@ -294,6 +336,14 @@ public class RealmsTests extends ESTestCase { assertThat(realm.type(), equalTo(NativeRealmSettings.TYPE)); assertThat(realm.name(), equalTo("default_" + NativeRealmSettings.TYPE)); assertThat(iter.hasNext(), is(false)); + + assertThat(realms.getUnlicensedRealms(), iterableWithSize(2)); + realm = realms.getUnlicensedRealms().get(0); + assertThat(realm.type(), equalTo("ldap")); + assertThat(realm.name(), equalTo("foo")); + realm = realms.getUnlicensedRealms().get(1); + assertThat(realm.type(), equalTo("type_0")); + assertThat(realm.name(), equalTo("custom")); } public void testUnlicensedWithNativeRealmSettings() throws Exception { @@ -317,6 +367,7 @@ public class RealmsTests extends ESTestCase { realm = iter.next(); assertThat(realm.type(), is(type)); assertThat(iter.hasNext(), is(false)); + assertThat(realms.getUnlicensedRealms(), empty()); when(licenseState.allowedRealmType()).thenReturn(AllowedRealmType.NATIVE); iter = realms.iterator(); @@ -327,10 +378,15 @@ public class RealmsTests extends ESTestCase { realm = iter.next(); assertThat(realm.type(), is(type)); assertThat(iter.hasNext(), is(false)); + + assertThat(realms.getUnlicensedRealms(), iterableWithSize(1)); + realm = realms.getUnlicensedRealms().get(0); + assertThat(realm.type(), equalTo("ldap")); + assertThat(realm.name(), equalTo("foo")); } public void testUnlicensedWithNonStandardRealms() throws Exception { - final String selectedRealmType = randomFrom(SamlRealmSettings.TYPE, KerberosRealmSettings.TYPE); + final String selectedRealmType = randomFrom(SamlRealmSettings.TYPE, KerberosRealmSettings.TYPE, OpenIdConnectRealmSettings.TYPE); factories.put(selectedRealmType, config -> new DummyRealm(selectedRealmType, config)); Settings.Builder builder = Settings.builder() .put("path.home", createTempDir()) @@ -346,6 +402,7 @@ public class RealmsTests extends ESTestCase { realm = iter.next(); assertThat(realm.type(), is(selectedRealmType)); assertThat(iter.hasNext(), is(false)); + assertThat(realms.getUnlicensedRealms(), empty()); when(licenseState.allowedRealmType()).thenReturn(AllowedRealmType.DEFAULT); iter = realms.iterator(); @@ -360,6 +417,11 @@ public class RealmsTests extends ESTestCase { assertThat(realm.type(), is(NativeRealmSettings.TYPE)); assertThat(iter.hasNext(), is(false)); + assertThat(realms.getUnlicensedRealms(), iterableWithSize(1)); + realm = realms.getUnlicensedRealms().get(0); + assertThat(realm.type(), equalTo(selectedRealmType)); + assertThat(realm.name(), equalTo("foo")); + when(licenseState.allowedRealmType()).thenReturn(AllowedRealmType.NATIVE); iter = realms.iterator(); assertThat(iter.hasNext(), is(true)); @@ -372,6 +434,11 @@ public class RealmsTests extends ESTestCase { realm = iter.next(); assertThat(realm.type(), is(NativeRealmSettings.TYPE)); assertThat(iter.hasNext(), is(false)); + + assertThat(realms.getUnlicensedRealms(), iterableWithSize(1)); + realm = realms.getUnlicensedRealms().get(0); + assertThat(realm.type(), equalTo(selectedRealmType)); + assertThat(realm.name(), equalTo("foo")); } public void testDisabledRealmsAreNotAdded() throws Exception { @@ -422,6 +489,11 @@ public class RealmsTests extends ESTestCase { } assertThat(count, equalTo(orderToIndex.size())); + assertThat(realms.getUnlicensedRealms(), empty()); + + // check that disabled realms are not included in unlicensed realms + when(licenseState.allowedRealmType()).thenReturn(AllowedRealmType.NATIVE); + assertThat(realms.getUnlicensedRealms(), hasSize(orderToIndex.size())); } public void testAuthcAuthzDisabled() throws Exception {