From d6eb2c82613243410637580636d030a7eda39da4 Mon Sep 17 00:00:00 2001 From: jaymode Date: Tue, 25 Aug 2015 11:19:07 -0400 Subject: [PATCH] do not copy the authorization header from rest requests Currently we copy the authorization header from every rest request to the action request. This is not necessary because the user associated with each request is copied into the context and then if the request leaves the node, the user will be serialized into a string and attached as a header. This commit removes the copying of the authorization header as it is not necessary and by not copying it, we limit the amount of copies we make of this sensitive information. Original commit: elastic/x-pack-elasticsearch@4e5ba4b4aaf989af8646fff7b7255b3ccc5b5f14 --- .../activedirectory/ActiveDirectoryRealm.java | 5 ++--- .../shield/authc/esusers/ESUsersRealm.java | 4 +--- .../shield/authc/ldap/LdapRealm.java | 5 ++--- .../authc/ldap/support/AbstractLdapRealm.java | 5 ++--- .../authc/support/UsernamePasswordRealm.java | 6 +----- .../shield/authc/esusers/ESUsersRealmTests.java | 16 ++++------------ .../shield/authc/ldap/LdapRealmTests.java | 9 --------- 7 files changed, 12 insertions(+), 38 deletions(-) diff --git a/shield/src/main/java/org/elasticsearch/shield/authc/activedirectory/ActiveDirectoryRealm.java b/shield/src/main/java/org/elasticsearch/shield/authc/activedirectory/ActiveDirectoryRealm.java index 50d2f800265..f3fe939dc09 100644 --- a/shield/src/main/java/org/elasticsearch/shield/authc/activedirectory/ActiveDirectoryRealm.java +++ b/shield/src/main/java/org/elasticsearch/shield/authc/activedirectory/ActiveDirectoryRealm.java @@ -6,7 +6,6 @@ package org.elasticsearch.shield.authc.activedirectory; import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.rest.RestController; import org.elasticsearch.shield.ShieldSettingsFilter; import org.elasticsearch.shield.authc.RealmConfig; import org.elasticsearch.shield.authc.ldap.support.AbstractLdapRealm; @@ -34,8 +33,8 @@ public class ActiveDirectoryRealm extends AbstractLdapRealm { private final ClientSSLService clientSSLService; @Inject - public Factory(ResourceWatcherService watcherService, RestController restController, ClientSSLService clientSSLService) { - super(ActiveDirectoryRealm.TYPE, restController); + public Factory(ResourceWatcherService watcherService, ClientSSLService clientSSLService) { + super(ActiveDirectoryRealm.TYPE); this.watcherService = watcherService; this.clientSSLService = clientSSLService; } diff --git a/shield/src/main/java/org/elasticsearch/shield/authc/esusers/ESUsersRealm.java b/shield/src/main/java/org/elasticsearch/shield/authc/esusers/ESUsersRealm.java index 3225933125e..07cd43feaaa 100644 --- a/shield/src/main/java/org/elasticsearch/shield/authc/esusers/ESUsersRealm.java +++ b/shield/src/main/java/org/elasticsearch/shield/authc/esusers/ESUsersRealm.java @@ -8,7 +8,6 @@ package org.elasticsearch.shield.authc.esusers; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; -import org.elasticsearch.rest.RestController; import org.elasticsearch.shield.User; import org.elasticsearch.shield.authc.Realm; import org.elasticsearch.shield.authc.RealmConfig; @@ -59,12 +58,11 @@ public class ESUsersRealm extends CachingUsernamePasswordRealm { private final ResourceWatcherService watcherService; @Inject - public Factory(Settings settings, Environment env, ResourceWatcherService watcherService, RestController restController) { + public Factory(Settings settings, Environment env, ResourceWatcherService watcherService) { super(TYPE, true); this.settings = settings; this.env = env; this.watcherService = watcherService; - restController.registerRelevantHeaders(UsernamePasswordToken.BASIC_AUTH_HEADER); } @Override diff --git a/shield/src/main/java/org/elasticsearch/shield/authc/ldap/LdapRealm.java b/shield/src/main/java/org/elasticsearch/shield/authc/ldap/LdapRealm.java index f66e8011492..36d38445060 100644 --- a/shield/src/main/java/org/elasticsearch/shield/authc/ldap/LdapRealm.java +++ b/shield/src/main/java/org/elasticsearch/shield/authc/ldap/LdapRealm.java @@ -8,7 +8,6 @@ package org.elasticsearch.shield.authc.ldap; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.rest.RestController; import org.elasticsearch.shield.ShieldSettingsFilter; import org.elasticsearch.shield.authc.RealmConfig; import org.elasticsearch.shield.authc.ldap.support.AbstractLdapRealm; @@ -36,8 +35,8 @@ public class LdapRealm extends AbstractLdapRealm { private final ClientSSLService clientSSLService; @Inject - public Factory(ResourceWatcherService watcherService, RestController restController, ClientSSLService clientSSLService) { - super(TYPE, restController); + public Factory(ResourceWatcherService watcherService, ClientSSLService clientSSLService) { + super(TYPE); this.watcherService = watcherService; this.clientSSLService = clientSSLService; } diff --git a/shield/src/main/java/org/elasticsearch/shield/authc/ldap/support/AbstractLdapRealm.java b/shield/src/main/java/org/elasticsearch/shield/authc/ldap/support/AbstractLdapRealm.java index 23430c5c328..4bf8b1b275b 100644 --- a/shield/src/main/java/org/elasticsearch/shield/authc/ldap/support/AbstractLdapRealm.java +++ b/shield/src/main/java/org/elasticsearch/shield/authc/ldap/support/AbstractLdapRealm.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.shield.authc.ldap.support; -import org.elasticsearch.rest.RestController; import org.elasticsearch.shield.User; import org.elasticsearch.shield.authc.RealmConfig; import org.elasticsearch.shield.authc.support.*; @@ -64,8 +63,8 @@ public abstract class AbstractLdapRealm extends CachingUsernamePasswordRealm { public static abstract class Factory extends UsernamePasswordRealm.Factory { - public Factory(String type, RestController restController) { - super(type, false, restController); + public Factory(String type) { + super(type, false); } /** diff --git a/shield/src/main/java/org/elasticsearch/shield/authc/support/UsernamePasswordRealm.java b/shield/src/main/java/org/elasticsearch/shield/authc/support/UsernamePasswordRealm.java index 2d13b424776..2da565fadc7 100644 --- a/shield/src/main/java/org/elasticsearch/shield/authc/support/UsernamePasswordRealm.java +++ b/shield/src/main/java/org/elasticsearch/shield/authc/support/UsernamePasswordRealm.java @@ -5,15 +5,12 @@ */ package org.elasticsearch.shield.authc.support; -import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.shield.authc.AuthenticationToken; import org.elasticsearch.shield.authc.Realm; import org.elasticsearch.shield.authc.RealmConfig; import org.elasticsearch.transport.TransportMessage; -import static org.elasticsearch.shield.authc.support.UsernamePasswordToken.BASIC_AUTH_HEADER; - /** * */ @@ -39,9 +36,8 @@ public abstract class UsernamePasswordRealm extends Realm public static abstract class Factory extends Realm.Factory { - protected Factory(String type, boolean internal, RestController restController) { + protected Factory(String type, boolean internal) { super(type, internal); - restController.registerRelevantHeaders(BASIC_AUTH_HEADER); } } } diff --git a/shield/src/test/java/org/elasticsearch/shield/authc/esusers/ESUsersRealmTests.java b/shield/src/test/java/org/elasticsearch/shield/authc/esusers/ESUsersRealmTests.java index 2168d8f1fe5..0ec75f38138 100644 --- a/shield/src/test/java/org/elasticsearch/shield/authc/esusers/ESUsersRealmTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/authc/esusers/ESUsersRealmTests.java @@ -43,7 +43,6 @@ import static org.mockito.Mockito.*; */ public class ESUsersRealmTests extends ESTestCase { - private RestController restController; private Client client; private AdminClient adminClient; private FileUserPasswdStore userPasswdStore; @@ -54,18 +53,11 @@ public class ESUsersRealmTests extends ESTestCase { public void init() throws Exception { client = mock(Client.class); adminClient = mock(AdminClient.class); - restController = mock(RestController.class); userPasswdStore = mock(FileUserPasswdStore.class); userRolesStore = mock(FileUserRolesStore.class); globalSettings = Settings.builder().put("path.home", createTempDir()).build(); } - @Test - public void testRestHeaderRegistration() { - new ESUsersRealm.Factory(Settings.EMPTY, mock(Environment.class), mock(ResourceWatcherService.class), restController); - verify(restController).registerRelevantHeaders(UsernamePasswordToken.BASIC_AUTH_HEADER); - } - @Test public void testAuthenticate() throws Exception { when(userPasswdStore.verifyPassword("user1", SecuredStringTests.build("test123"))).thenReturn(true); @@ -134,11 +126,11 @@ public class ESUsersRealmTests extends ESTestCase { } @Test @SuppressWarnings("unchecked") - public void testRestHeadersAreCopied() throws Exception { + public void testAuthorizationHeaderIsNotCopied() throws Exception { + RestController restController = mock(RestController.class); RealmConfig config = new RealmConfig("esusers-test", Settings.EMPTY, globalSettings); - // the required header will be registered only if ESUsersRealm is actually used. new ESUsersRealm(config, new UserPasswdStore(config), new UserRolesStore(config)); - when(restController.relevantHeaders()).thenReturn(ImmutableSet.of(UsernamePasswordToken.BASIC_AUTH_HEADER)); + when(restController.relevantHeaders()).thenReturn(ImmutableSet.of()); when(client.admin()).thenReturn(adminClient); when(client.settings()).thenReturn(Settings.EMPTY); when(client.headers()).thenReturn(Headers.EMPTY); @@ -163,7 +155,7 @@ public class ESUsersRealmTests extends ESTestCase { when(restRequest.header(UsernamePasswordToken.BASIC_AUTH_HEADER)).thenReturn("foobar"); RestChannel channel = mock(RestChannel.class); handler.handleRequest(restRequest, channel); - assertThat((String) request.getHeader(UsernamePasswordToken.BASIC_AUTH_HEADER), Matchers.equalTo("foobar")); + assertThat(request.getHeader(UsernamePasswordToken.BASIC_AUTH_HEADER), is(nullValue())); } static class UserPasswdStore extends FileUserPasswdStore { diff --git a/shield/src/test/java/org/elasticsearch/shield/authc/ldap/LdapRealmTests.java b/shield/src/test/java/org/elasticsearch/shield/authc/ldap/LdapRealmTests.java index c66559dd558..48f7bab0fb7 100644 --- a/shield/src/test/java/org/elasticsearch/shield/authc/ldap/LdapRealmTests.java +++ b/shield/src/test/java/org/elasticsearch/shield/authc/ldap/LdapRealmTests.java @@ -6,7 +6,6 @@ package org.elasticsearch.shield.authc.ldap; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.rest.RestController; import org.elasticsearch.shield.User; import org.elasticsearch.shield.authc.RealmConfig; import org.elasticsearch.shield.authc.ldap.support.LdapSearchScope; @@ -36,14 +35,12 @@ public class LdapRealmTests extends LdapTestCase { public static final String VALID_USERNAME = "Thomas Masterman Hardy"; public static final String PASSWORD = "pass"; - private RestController restController; private ThreadPool threadPool; private ResourceWatcherService resourceWatcherService; private Settings globalSettings; @Before public void init() throws Exception { - restController = mock(RestController.class); threadPool = new ThreadPool("ldap realm tests"); resourceWatcherService = new ResourceWatcherService(Settings.EMPTY, threadPool); globalSettings = Settings.builder().put("path.home", createTempDir()).build(); @@ -55,12 +52,6 @@ public class LdapRealmTests extends LdapTestCase { terminate(threadPool); } - @Test - public void testRestHeaderRegistration() { - new LdapRealm.Factory(resourceWatcherService, restController, null); - verify(restController).registerRelevantHeaders(UsernamePasswordToken.BASIC_AUTH_HEADER); - } - @Test public void testAuthenticate_SubTreeGroupSearch() throws Exception { String groupSearchBase = "o=sevenSeas";