From d897b295fbda3d751e7ed3fd24c993083d0875c0 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Thu, 9 Aug 2018 17:28:00 +1000 Subject: [PATCH 1/2] Prevent chains in authorization_realms If realm "A" delegates authoriaation to realm "B" then it is not permissible for realm "B" to also be using delegated authorization. A realm which is in the value for "authorization_realms" must handle its own authorization. --- .../DelegatedAuthorizationSupport.java | 34 ++++++++++++++----- .../DelegatedAuthorizationSupportTests.java | 18 +++++++++- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DelegatedAuthorizationSupport.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DelegatedAuthorizationSupport.java index 8b2bf87c9ad06..73b72edd06697 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DelegatedAuthorizationSupport.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DelegatedAuthorizationSupport.java @@ -10,6 +10,7 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.license.LicenseUtils; import org.elasticsearch.license.XPackLicenseState; @@ -17,10 +18,12 @@ import org.elasticsearch.xpack.core.security.authc.AuthenticationResult; import org.elasticsearch.xpack.core.security.authc.Realm; import org.elasticsearch.xpack.core.security.authc.RealmConfig; +import org.elasticsearch.xpack.core.security.authc.RealmSettings; import org.elasticsearch.xpack.core.security.authc.support.DelegatedAuthorizationSettings; import java.util.ArrayList; import java.util.List; +import java.util.Map; import static org.elasticsearch.common.Strings.collectionToDelimitedString; @@ -40,10 +43,11 @@ public class DelegatedAuthorizationSupport { /** * Resolves the {@link DelegatedAuthorizationSettings#AUTHZ_REALMS} setting from {@code config} and calls - * {@link #DelegatedAuthorizationSupport(Iterable, List, ThreadContext, XPackLicenseState)} + * {@link #DelegatedAuthorizationSupport(Iterable, List, Settings, ThreadContext, XPackLicenseState)} */ public DelegatedAuthorizationSupport(Iterable allRealms, RealmConfig config, XPackLicenseState licenseState) { - this(allRealms, DelegatedAuthorizationSettings.AUTHZ_REALMS.get(config.settings()), config.threadContext(), licenseState); + this(allRealms, DelegatedAuthorizationSettings.AUTHZ_REALMS.get(config.settings()), config.globalSettings(), config.threadContext(), + licenseState); } /** @@ -51,11 +55,13 @@ public DelegatedAuthorizationSupport(Iterable allRealms, RealmC * {@code allRealms}. * @throws IllegalArgumentException if one of the specified realms does not exist */ - protected DelegatedAuthorizationSupport(Iterable allRealms, List lookupRealms, ThreadContext threadContext, - XPackLicenseState licenseState) { - this.lookup = new RealmUserLookup(resolveRealms(allRealms, lookupRealms), threadContext); - this.logger = Loggers.getLogger(getClass()); - this.licenseState = licenseState; + protected DelegatedAuthorizationSupport(Iterable allRealms, List lookupRealms, Settings settings, + ThreadContext threadContext, XPackLicenseState licenseState) { + final List realms = resolveRealms(allRealms, lookupRealms); + checkRealms(realms, settings); + this.lookup = new RealmUserLookup(realms, threadContext); + this.logger = Loggers.getLogger(getClass()); + this.licenseState = licenseState; } /** @@ -108,13 +114,25 @@ private List resolveRealms(Iterable allRealms, List realms, Settings globalSettings) { + final Map settingsByRealm = RealmSettings.getRealmSettings(globalSettings); + for (Realm realm : realms) { + final Settings realmSettings = settingsByRealm.get(realm.name()); + if (realmSettings != null && DelegatedAuthorizationSettings.AUTHZ_REALMS.exists(realmSettings)) { + throw new IllegalArgumentException("cannot use realm [" + realm + + "] as an authorization realm - it is already delegating authorization to [" + + DelegatedAuthorizationSettings.AUTHZ_REALMS.get(realmSettings) + "]"); + } + } + } + private Realm findRealm(String name, Iterable allRealms) { for (Realm realm : allRealms) { if (name.equals(realm.name())) { return realm; } } - throw new IllegalArgumentException("configured authorizing realm [" + name + "] does not exist (or is not enabled)"); + throw new IllegalArgumentException("configured authorization realm [" + name + "] does not exist (or is not enabled)"); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/DelegatedAuthorizationSupportTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/DelegatedAuthorizationSupportTests.java index 2354e98b3f02f..d1ec5518cce53 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/DelegatedAuthorizationSupportTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/DelegatedAuthorizationSupportTests.java @@ -86,7 +86,23 @@ public void testMissingRealmInDelegationList() { final IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> new DelegatedAuthorizationSupport(realms, buildRealmConfig("r", settings), license) ); - assertThat(ex.getMessage(), equalTo("configured authorizing realm [no-such-realm] does not exist (or is not enabled)")); + assertThat(ex.getMessage(), equalTo("configured authorization realm [no-such-realm] does not exist (or is not enabled)")); + } + + public void testDelegationChainsAreRejected() { + final XPackLicenseState license = getLicenseState(true); + final Settings settings = Settings.builder() + .putList("authorization_realms", "lookup-1", "lookup-2", "lookup-3") + .build(); + globalSettings = Settings.builder() + .put(globalSettings) + .putList("xpack.security.authc.realms.lookup-2.authorization_realms", "lookup-1") + .build(); + final IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> + new DelegatedAuthorizationSupport(realms, buildRealmConfig("realm1", settings), license) + ); + assertThat(ex.getMessage(), + equalTo("cannot use realm [mock/lookup-2] as an authorization realm - it is already delegating authorization to [[lookup-1]]")); } public void testMatchInDelegationList() throws Exception { From 82eb1bea8b113761efe73fb245740721c3454498 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Fri, 10 Aug 2018 15:23:05 +1000 Subject: [PATCH 2/2] Address feedback --- .../support/DelegatedAuthorizationSupport.java | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DelegatedAuthorizationSupport.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DelegatedAuthorizationSupport.java index 73b72edd06697..cbf565555113e 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DelegatedAuthorizationSupport.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/support/DelegatedAuthorizationSupport.java @@ -57,9 +57,9 @@ public DelegatedAuthorizationSupport(Iterable allRealms, RealmC */ protected DelegatedAuthorizationSupport(Iterable allRealms, List lookupRealms, Settings settings, ThreadContext threadContext, XPackLicenseState licenseState) { - final List realms = resolveRealms(allRealms, lookupRealms); - checkRealms(realms, settings); - this.lookup = new RealmUserLookup(realms, threadContext); + final List resolvedLookupRealms = resolveRealms(allRealms, lookupRealms); + checkForRealmChains(resolvedLookupRealms, settings); + this.lookup = new RealmUserLookup(resolvedLookupRealms, threadContext); this.logger = Loggers.getLogger(getClass()); this.licenseState = licenseState; } @@ -114,9 +114,17 @@ private List resolveRealms(Iterable allRealms, List realms, Settings globalSettings) { + /** + * Checks for (and rejects) chains of delegation in the provided realms. + * A chain occurs when "realmA" delegates authorization to "realmB", and realmB also delegates authorization (to any realm). + * Since "realmB" does not handle its own authorization, it is not a valid target for delegated authorization. + * @param delegatedRealms The list of realms that are going to be used for authorization. If is an error if any of these realms are + * also configured to delegate their authorization. + * @throws IllegalArgumentException if a chain is detected + */ + private void checkForRealmChains(Iterable delegatedRealms, Settings globalSettings) { final Map settingsByRealm = RealmSettings.getRealmSettings(globalSettings); - for (Realm realm : realms) { + for (Realm realm : delegatedRealms) { final Settings realmSettings = settingsByRealm.get(realm.name()); if (realmSettings != null && DelegatedAuthorizationSettings.AUTHZ_REALMS.exists(realmSettings)) { throw new IllegalArgumentException("cannot use realm [" + realm +