diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java b/plugin/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java index b30f8a5cead..8a9140baa76 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java @@ -13,15 +13,10 @@ import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.admin.indices.close.CloseIndexAction; import org.elasticsearch.action.admin.indices.delete.DeleteIndexAction; import org.elasticsearch.action.admin.indices.open.OpenIndexAction; -import org.elasticsearch.action.search.ClearScrollRequest; -import org.elasticsearch.action.search.SearchResponse; -import org.elasticsearch.action.search.SearchScrollRequest; import org.elasticsearch.action.support.ActionFilter; import org.elasticsearch.action.support.ActionFilterChain; import org.elasticsearch.action.support.ContextPreservingActionListener; import org.elasticsearch.action.support.DestructiveOperations; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; @@ -34,27 +29,18 @@ import org.elasticsearch.xpack.XPackPlugin; import org.elasticsearch.xpack.security.SecurityContext; import org.elasticsearch.xpack.security.action.SecurityActionMapper; import org.elasticsearch.xpack.security.action.interceptor.RequestInterceptor; -import org.elasticsearch.xpack.security.audit.AuditTrail; -import org.elasticsearch.xpack.security.audit.AuditTrailService; import org.elasticsearch.xpack.security.authc.Authentication; import org.elasticsearch.xpack.security.authc.AuthenticationService; import org.elasticsearch.xpack.security.authz.AuthorizationService; import org.elasticsearch.xpack.security.authz.AuthorizationUtils; import org.elasticsearch.xpack.security.authz.privilege.HealthAndStatsPrivilege; -import org.elasticsearch.xpack.security.crypto.CryptoService; import org.elasticsearch.xpack.security.support.Automatons; import org.elasticsearch.xpack.security.user.SystemUser; import org.elasticsearch.xpack.security.user.User; import java.io.IOException; -import java.io.UncheckedIOException; -import java.util.ArrayList; -import java.util.List; import java.util.Set; import java.util.function.Predicate; -import java.util.function.Supplier; - -import static org.elasticsearch.xpack.security.support.Exceptions.authorizationError; public class SecurityActionFilter extends AbstractComponent implements ActionFilter { @@ -63,33 +49,25 @@ public class SecurityActionFilter extends AbstractComponent implements ActionFil private final AuthenticationService authcService; private final AuthorizationService authzService; - private final CryptoService cryptoService; - private final AuditTrail auditTrail; private final SecurityActionMapper actionMapper = new SecurityActionMapper(); private final Set requestInterceptors; private final XPackLicenseState licenseState; private final ThreadContext threadContext; private final SecurityContext securityContext; private final DestructiveOperations destructiveOperations; - private final ClusterService clusterService; @Inject public SecurityActionFilter(Settings settings, AuthenticationService authcService, AuthorizationService authzService, - CryptoService cryptoService, AuditTrailService auditTrail, XPackLicenseState licenseState, - Set requestInterceptors, ThreadPool threadPool, - SecurityContext securityContext, DestructiveOperations destructiveOperations, - ClusterService clusterService) { + XPackLicenseState licenseState, Set requestInterceptors, ThreadPool threadPool, + SecurityContext securityContext, DestructiveOperations destructiveOperations) { super(settings); this.authcService = authcService; this.authzService = authzService; - this.cryptoService = cryptoService; - this.auditTrail = auditTrail; this.licenseState = licenseState; this.requestInterceptors = requestInterceptors; this.threadContext = threadPool.getThreadContext(); this.securityContext = securityContext; this.destructiveOperations = destructiveOperations; - this.clusterService = clusterService; } @Override @@ -108,17 +86,10 @@ public class SecurityActionFilter extends AbstractComponent implements ActionFil if (licenseState.isAuthAllowed()) { final boolean useSystemUser = AuthorizationUtils.shouldReplaceUserWithSystem(threadContext, action); - final Supplier toRestore = threadContext.newRestorableContext(true); - final ActionListener signingListener = new ContextPreservingActionListener<>(toRestore, - ActionListener.wrap(r -> { - try { - listener.onResponse(sign(r)); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }, listener::onFailure)); - ActionListener authenticatedListener = - ActionListener.wrap((aVoid) -> chain.proceed(task, action, request, signingListener), signingListener::onFailure); + final ActionListener contextPreservingListener = + ContextPreservingActionListener.wrapPreservingContext(listener, threadContext); + ActionListener authenticatedListener = ActionListener.wrap( + (aVoid) -> chain.proceed(task, action, request, contextPreservingListener), contextPreservingListener::onFailure); try { if (useSystemUser) { securityContext.executeAsUser(SystemUser.INSTANCE, (original) -> { @@ -182,15 +153,14 @@ public class SecurityActionFilter extends AbstractComponent implements ActionFil (userRoles, runAsRoles) -> { authzService.authorize(authentication, securityAction, request, userRoles, runAsRoles); final User user = authentication.getUser(); - ActionRequest unsignedRequest = unsign(user, securityAction, request); /* * We use a separate concept for code that needs to be run after authentication and authorization that could * affect the running of the action. This is done to make it more clear of the state of the request. */ for (RequestInterceptor interceptor : requestInterceptors) { - if (interceptor.supports(unsignedRequest)) { - interceptor.intercept(unsignedRequest, user); + if (interceptor.supports(request)) { + interceptor.intercept(request, user); } } listener.onResponse(null); @@ -198,91 +168,4 @@ public class SecurityActionFilter extends AbstractComponent implements ActionFil asyncAuthorizer.authorize(authzService); } } - - ActionRequest unsign(User user, String action, final ActionRequest request) { - try { - // In order to provide backwards compatibility with previous versions that always signed scroll ids - // we sign the scroll requests and do not allow unsigned requests until all of the nodes in the cluster - // have been upgraded to a version that does not sign scroll ids and instead relies improved scroll - // authorization. It is important to note that older versions do not actually sign if the system key - // does not exist so we need to take that into account as well. - // TODO Remove any signing from master! - final ClusterState state = clusterService.state(); - final boolean signingRequired = state.nodes().getMinNodeVersion().before(Version.V_5_5_0) && - cryptoService.isSystemKeyPresent(); - if (request instanceof SearchScrollRequest) { - SearchScrollRequest scrollRequest = (SearchScrollRequest) request; - String scrollId = scrollRequest.scrollId(); - if (signingRequired) { - if (cryptoService.isSigned(scrollId)) { - scrollRequest.scrollId(cryptoService.unsignAndVerify(scrollId)); - } else { - logger.error("scroll id [{}] is not signed but is expected to be. nodes [{}], minimum node version [{}]", - scrollId, state.nodes(), state.nodes().getMinNodeVersion()); - // if we get a unsigned scroll request and not all nodes are up to date, then we cannot trust - // this scroll id and reject it - auditTrail.tamperedRequest(user, action, request); - throw authorizationError("invalid request"); - } - } else if (cryptoService.isSigned(scrollId)) { - // if signing isn't required we could still get a signed ID from an already running scroll or - // a node that hasn't received the current cluster state that shows signing isn't required - scrollRequest.scrollId(cryptoService.unsignAndVerify(scrollId)); - } - // else the scroll id is fine on the request so don't do anything - } else if (request instanceof ClearScrollRequest) { - ClearScrollRequest clearScrollRequest = (ClearScrollRequest) request; - final boolean isClearAllScrollRequest = clearScrollRequest.scrollIds().contains("_all"); - if (isClearAllScrollRequest == false) { - List signedIds = clearScrollRequest.scrollIds(); - List unsignedIds = new ArrayList<>(signedIds.size()); - for (String signedId : signedIds) { - if (signingRequired) { - if (cryptoService.isSigned(signedId)) { - unsignedIds.add(cryptoService.unsignAndVerify(signedId)); - } else { - logger.error("scroll id [{}] is not signed but is expected to be. nodes [{}], minimum node version [{}]", - signedId, state.nodes(), state.nodes().getMinNodeVersion()); - // if we get a unsigned scroll request and not all nodes are up to date, then we cannot trust - // this scroll id and reject it - auditTrail.tamperedRequest(user, action, request); - throw authorizationError("invalid request"); - } - } else if (cryptoService.isSigned(signedId)) { - // if signing isn't required we could still get a signed ID from an already running scroll or - // a node that hasn't received the current cluster state that shows signing isn't required - unsignedIds.add(cryptoService.unsignAndVerify(signedId)); - } else { - // the id is not signed and we allow unsigned requests so just add it - unsignedIds.add(signedId); - } - } - clearScrollRequest.scrollIds(unsignedIds); - } - } - } catch (IllegalArgumentException | IllegalStateException e) { - // this can happen when we decode invalid base64 or get a invalid scroll id - auditTrail.tamperedRequest(user, action, request); - throw authorizationError("invalid request. {}", e.getMessage()); - } - return request; - } - - private Response sign(Response response) throws IOException { - if (response instanceof SearchResponse) { - // In order to provide backwards compatibility with previous versions that always signed scroll ids - // we sign the scroll requests and do not allow unsigned requests until all of the nodes in the cluster - // have been upgraded to a version that supports unsigned scroll ids - final boolean sign = clusterService.state().nodes().getMinNodeVersion().before(Version.V_6_0_0_alpha2); - - if (sign) { - SearchResponse searchResponse = (SearchResponse) response; - String scrollId = searchResponse.getScrollId(); - if (scrollId != null && !cryptoService.isSigned(scrollId)) { - searchResponse.scrollId(cryptoService.sign(scrollId)); - } - } - } - return response; - } } diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/crypto/CryptoService.java b/plugin/src/main/java/org/elasticsearch/xpack/security/crypto/CryptoService.java index 1456f80e428..e0f6f40f5dd 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/crypto/CryptoService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/crypto/CryptoService.java @@ -9,13 +9,11 @@ import javax.crypto.BadPaddingException; import javax.crypto.Cipher; import javax.crypto.IllegalBlockSizeException; import javax.crypto.KeyGenerator; -import javax.crypto.Mac; import javax.crypto.SecretKey; import javax.crypto.spec.IvParameterSpec; import javax.crypto.spec.SecretKeySpec; import java.io.FileNotFoundException; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.security.MessageDigest; @@ -24,7 +22,6 @@ import java.security.SecureRandom; import java.util.Arrays; import java.util.Base64; import java.util.List; -import java.util.regex.Pattern; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.component.AbstractComponent; @@ -36,7 +33,6 @@ import org.elasticsearch.xpack.XPackPlugin; import org.elasticsearch.xpack.security.authc.support.CharArrays; import static org.elasticsearch.xpack.security.Security.setting; -import static org.elasticsearch.xpack.security.authc.support.CharArrays.constantTimeEquals; /** * Service that provides cryptographic methods based on a shared system key @@ -47,14 +43,11 @@ public class CryptoService extends AbstractComponent { public static final int KEY_SIZE = 1024; static final String FILE_NAME = "system_key"; - static final String HMAC_ALGO = "HmacSHA1"; static final String DEFAULT_ENCRYPTION_ALGORITHM = "AES/CTR/NoPadding"; static final String DEFAULT_KEY_ALGORITH = "AES"; static final String ENCRYPTED_TEXT_PREFIX = "::es_encrypted::"; static final int DEFAULT_KEY_LENGTH = 128; - private static final Pattern SIG_PATTERN = Pattern.compile("^\\$\\$[0-9]+\\$\\$[^\\$]*\\$\\$.+"); - private static final Setting SYSTEM_KEY_REQUIRED_SETTING = Setting.boolSetting(setting("system_key.required"), false, Property.NodeScope); private static final Setting ENCRYPTION_ALGO_SETTING = @@ -68,11 +61,9 @@ public class CryptoService extends AbstractComponent { private final String encryptionAlgorithm; private final int ivLength; /* - * Scroll ids are signed using the system key to authenticate them as we currently do not have a proper way to authorize these requests - * and the key is also used for encrypting sensitive data stored in watches. The encryption key is derived from the system key. + * The encryption key is derived from the system key. */ private final SecretKey encryptionKey; - private final SecretKey systemKey; public CryptoService(Settings settings, Environment env) throws IOException { super(settings); @@ -86,7 +77,7 @@ public class CryptoService extends AbstractComponent { } Path keyFile = resolveSystemKey(env); - systemKey = readSystemKey(keyFile, SYSTEM_KEY_REQUIRED_SETTING.get(settings)); + SecretKey systemKey = readSystemKey(keyFile, SYSTEM_KEY_REQUIRED_SETTING.get(settings)); try { encryptionKey = encryptionKey(systemKey, keyLength, keyAlgorithm); @@ -129,70 +120,6 @@ public class CryptoService extends AbstractComponent { return null; } - /** - * Signs the given text and returns the signed text (original text + signature) - * @param text the string to sign - */ - public String sign(String text) throws IOException { - if (systemKey != null) { - String sigStr = signInternal(text, systemKey); - return "$$" + sigStr.length() + "$$$$" + sigStr + text; - } else { - return text; - } - } - - /** - * Unsigns the given signed text, verifies the original text with the attached signature and if valid returns - * the unsigned (original) text. If signature verification fails a {@link IllegalArgumentException} is thrown. - * @param signedText the string to unsign and verify - */ - public String unsignAndVerify(String signedText) { - if (systemKey == null) { - return signedText; - } - - if (!signedText.startsWith("$$") || signedText.length() < 2) { - throw new IllegalArgumentException("tampered signed text"); - } - - // $$34$$$$sigtext - String[] pieces = signedText.split("\\$\\$"); - if (pieces.length != 4 || pieces[0].isEmpty() == false || pieces[2].isEmpty() == false) { - logger.debug("received signed text [{}] with [{}] parts", signedText, pieces.length); - throw new IllegalArgumentException("tampered signed text"); - } - String text; - String receivedSignature; - try { - int length = Integer.parseInt(pieces[1]); - receivedSignature = pieces[3].substring(0, length); - text = pieces[3].substring(length); - } catch (Exception e) { - logger.error("error occurred while parsing signed text", e); - throw new IllegalArgumentException("tampered signed text"); - } - - try { - String sig = signInternal(text, systemKey); - if (constantTimeEquals(sig, receivedSignature)) { - return text; - } - } catch (Exception e) { - logger.error("error occurred while verifying signed text", e); - throw new IllegalStateException("error while verifying the signed text"); - } - - throw new IllegalArgumentException("tampered signed text"); - } - - /** - * Checks whether the given text is signed. - */ - public boolean isSigned(String text) { - return SIG_PATTERN.matcher(text).matches(); - } - /** * Encrypts the provided char array and returns the encrypted values in a char array * @param chars the characters to encrypt @@ -255,14 +182,6 @@ public class CryptoService extends AbstractComponent { return this.encryptionKey != null; } - /** - * Flag for callers to determine if values will actually be signed or returned as is - * @return true if values will be signed - */ - public boolean isSystemKeyPresent() { - return this.systemKey != null; - } - private byte[] encryptInternal(byte[] bytes, SecretKey key) { byte[] iv = new byte[ivLength]; secureRandom.nextBytes(iv); @@ -297,22 +216,6 @@ public class CryptoService extends AbstractComponent { } } - static Mac createMac(SecretKey key) { - try { - Mac mac = HmacSHA1Provider.hmacSHA1(); - mac.init(key); - return mac; - } catch (Exception e) { - throw new ElasticsearchException("could not initialize mac", e); - } - } - - private static String signInternal(String text, SecretKey key) throws IOException { - Mac mac = createMac(key); - byte[] sig = mac.doFinal(text.getBytes(StandardCharsets.UTF_8)); - return Base64.getUrlEncoder().encodeToString(sig); - } - static Cipher cipher(int mode, String encryptionAlgorithm, SecretKey key, byte[] initializationVector) { try { @@ -346,28 +249,6 @@ public class CryptoService extends AbstractComponent { return new SecretKeySpec(truncatedDigest, algorithm); } - /** - * Provider class for the HmacSHA1 {@link Mac} that provides an optimization by using a thread local instead of calling - * Mac#getInstance and obtaining a lock (in the internals) - */ - private static class HmacSHA1Provider { - - private static final ThreadLocal MAC = ThreadLocal.withInitial(() -> { - try { - return Mac.getInstance(HMAC_ALGO); - } catch (NoSuchAlgorithmException e) { - throw new IllegalStateException("could not create Mac instance with algorithm [" + HMAC_ALGO + "]", e); - } - }); - - private static Mac hmacSHA1() { - Mac instance = MAC.get(); - instance.reset(); - return instance; - } - - } - public static void addSettings(List> settings) { settings.add(ENCRYPTION_KEY_LENGTH_SETTING); settings.add(ENCRYPTION_KEY_ALGO_SETTING); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilterTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilterTests.java index 055d770dda9..a8563fcc3e7 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilterTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilterTests.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.security.action.filter; import org.apache.lucene.util.SetOnce; -import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRequest; @@ -14,7 +13,6 @@ import org.elasticsearch.action.MockIndicesRequest; import org.elasticsearch.action.admin.indices.close.CloseIndexAction; import org.elasticsearch.action.admin.indices.delete.DeleteIndexAction; import org.elasticsearch.action.admin.indices.open.OpenIndexAction; -import org.elasticsearch.action.search.SearchScrollRequest; import org.elasticsearch.action.support.ActionFilterChain; import org.elasticsearch.action.support.ContextPreservingActionListener; import org.elasticsearch.action.support.DestructiveOperations; @@ -22,41 +20,33 @@ import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; -import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.license.XPackLicenseState; -import org.elasticsearch.rest.RestStatus; import org.elasticsearch.tasks.Task; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.security.SecurityContext; -import org.elasticsearch.xpack.security.audit.AuditTrailService; import org.elasticsearch.xpack.security.authc.Authentication; import org.elasticsearch.xpack.security.authc.Authentication.RealmRef; import org.elasticsearch.xpack.security.authc.AuthenticationService; import org.elasticsearch.xpack.security.authz.AuthorizationService; import org.elasticsearch.xpack.security.authz.permission.Role; -import org.elasticsearch.xpack.security.crypto.CryptoService; import org.elasticsearch.xpack.security.user.SystemUser; import org.elasticsearch.xpack.security.user.User; import org.junit.Before; -import org.mockito.ArgumentCaptor; import java.util.Collections; import java.util.HashSet; -import static org.hamcrest.Matchers.equalTo; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Matchers.isA; import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyZeroInteractions; @@ -65,20 +55,15 @@ import static org.mockito.Mockito.when; public class SecurityActionFilterTests extends ESTestCase { private AuthenticationService authcService; private AuthorizationService authzService; - private CryptoService cryptoService; - private AuditTrailService auditTrail; private XPackLicenseState licenseState; private SecurityActionFilter filter; private ThreadContext threadContext; - private ClusterService clusterService; private boolean failDestructiveOperations; @Before public void init() throws Exception { authcService = mock(AuthenticationService.class); authzService = mock(AuthorizationService.class); - cryptoService = mock(CryptoService.class); - auditTrail = mock(AuditTrailService.class); licenseState = mock(XPackLicenseState.class); when(licenseState.isAuthAllowed()).thenReturn(true); when(licenseState.isStatsAndHealthAllowed()).thenReturn(true); @@ -90,9 +75,7 @@ public class SecurityActionFilterTests extends ESTestCase { .put(DestructiveOperations.REQUIRES_NAME_SETTING.getKey(), failDestructiveOperations).build(); DestructiveOperations destructiveOperations = new DestructiveOperations(settings, new ClusterSettings(settings, Collections.singleton(DestructiveOperations.REQUIRES_NAME_SETTING))); - clusterService = mock(ClusterService.class); ClusterState state = mock(ClusterState.class); - when(clusterService.state()).thenReturn(state); DiscoveryNodes nodes = DiscoveryNodes.builder() .add(new DiscoveryNode("id1", new TransportAddress(TransportAddress.META_ADDRESS, randomIntBetween(49000, 65500)), Version.CURRENT)) @@ -102,8 +85,8 @@ public class SecurityActionFilterTests extends ESTestCase { when(state.nodes()).thenReturn(nodes); SecurityContext securityContext = new SecurityContext(settings, threadContext); - filter = new SecurityActionFilter(Settings.EMPTY, authcService, authzService, cryptoService, auditTrail, - licenseState, new HashSet<>(), threadPool, securityContext, destructiveOperations, clusterService); + filter = new SecurityActionFilter(Settings.EMPTY, authcService, authzService, + licenseState, new HashSet<>(), threadPool, securityContext, destructiveOperations); } public void testApply() throws Exception { @@ -126,7 +109,6 @@ public class SecurityActionFilterTests extends ESTestCase { callback.onResponse(empty); return Void.TYPE; }).when(authzService).roles(any(User.class), any(ActionListener.class)); - doReturn(request).when(spy(filter)).unsign(user, "_action", request); filter.apply(task, "_action", request, listener, chain); verify(authzService).authorize(authentication, "_action", request, empty, null); verify(chain).proceed(eq(task), eq("_action"), eq(request), isA(ContextPreservingActionListener.class)); @@ -155,7 +137,6 @@ public class SecurityActionFilterTests extends ESTestCase { callback.onResponse(empty); return Void.TYPE; }).when(authzService).roles(any(User.class), any(ActionListener.class)); - doReturn(request).when(spy(filter)).unsign(user, "_action", request); assertNull(threadContext.getTransient(Authentication.AUTHENTICATION_KEY)); filter.apply(task, "_action", request, listener, chain); @@ -189,11 +170,6 @@ public class SecurityActionFilterTests extends ESTestCase { callback.onResponse(threadContext.getTransient(Authentication.AUTHENTICATION_KEY)); return Void.TYPE; }).when(authcService).authenticate(eq(action), eq(request), eq(SystemUser.INSTANCE), any(ActionListener.class)); - doReturn(request).when(spy(filter)).unsign(user, action, request); - doAnswer((i) -> { - String text = (String) i.getArguments()[0]; - return text; - }).when(cryptoService).sign(any(String.class)); filter.apply(task, action, request, listener, chain); @@ -230,7 +206,6 @@ public class SecurityActionFilterTests extends ESTestCase { callback.onResponse(empty); return Void.TYPE; }).when(authzService).roles(any(User.class), any(ActionListener.class)); - doReturn(request).when(spy(filter)).unsign(user, action, request); filter.apply(task, action, request, listener, chain); if (failDestructiveOperations) { verify(listener).onFailure(isA(IllegalArgumentException.class)); @@ -268,139 +243,6 @@ public class SecurityActionFilterTests extends ESTestCase { verifyNoMoreInteractions(chain); } - public void testActionSignature() throws Exception { - SearchScrollRequest request = new SearchScrollRequest("signed_scroll_id"); - ActionListener listener = mock(ActionListener.class); - ActionFilterChain chain = mock(ActionFilterChain.class); - User user = mock(User.class); - Task task = mock(Task.class); - Authentication authentication = new Authentication(user, new RealmRef("test", "test", "foo"), null); - doAnswer((i) -> { - ActionListener callback = - (ActionListener) i.getArguments()[3]; - callback.onResponse(authentication); - return Void.TYPE; - }).when(authcService).authenticate(eq("_action"), eq(request), eq(SystemUser.INSTANCE), any(ActionListener.class)); - when(cryptoService.isSystemKeyPresent()).thenReturn(true); - when(cryptoService.isSigned("signed_scroll_id")).thenReturn(true); - when(cryptoService.unsignAndVerify("signed_scroll_id")).thenReturn("scroll_id"); - final Role empty = Role.EMPTY; - doAnswer((i) -> { - ActionListener callback = - (ActionListener) i.getArguments()[1]; - callback.onResponse(empty); - return Void.TYPE; - }).when(authzService).roles(any(User.class), any(ActionListener.class)); - filter.apply(task, "_action", request, listener, chain); - assertThat(request.scrollId(), equalTo("scroll_id")); - - verify(authzService).authorize(authentication, "_action", request, empty, null); - verify(chain).proceed(eq(task), eq("_action"), eq(request), isA(ContextPreservingActionListener.class)); - } - - public void testUnsignedWithOldVersionNode() throws Exception { - DiscoveryNodes nodes = DiscoveryNodes.builder(clusterService.state().nodes()) - .add(new DiscoveryNode("id3", - new TransportAddress(TransportAddress.META_ADDRESS, randomIntBetween(49000, 65500)), Version.V_5_4_0)) - .build(); - when(clusterService.state().nodes()).thenReturn(nodes); - SearchScrollRequest request = new SearchScrollRequest("unsigned"); - ActionListener listener = mock(ActionListener.class); - ActionFilterChain chain = mock(ActionFilterChain.class); - User user = mock(User.class); - Task task = mock(Task.class); - Authentication authentication = new Authentication(user, new RealmRef("test", "test", "foo"), null); - doAnswer((i) -> { - ActionListener callback = - (ActionListener) i.getArguments()[3]; - callback.onResponse(authentication); - return Void.TYPE; - }).when(authcService).authenticate(eq("_action"), eq(request), eq(SystemUser.INSTANCE), any(ActionListener.class)); - when(cryptoService.isSigned("unsigned")).thenReturn(false); - when(cryptoService.isSystemKeyPresent()).thenReturn(true); - final Role empty = Role.EMPTY; - doAnswer((i) -> { - ActionListener callback = - (ActionListener) i.getArguments()[1]; - callback.onResponse(empty); - return Void.TYPE; - }).when(authzService).roles(any(User.class), any(ActionListener.class)); - filter.apply(task, "_action", request, listener, chain); - - ArgumentCaptor captor = ArgumentCaptor.forClass(ElasticsearchSecurityException.class); - verify(listener).onFailure(captor.capture()); - ElasticsearchSecurityException e = captor.getValue(); - assertEquals("invalid request", e.getMessage()); - assertEquals(RestStatus.FORBIDDEN, e.status()); - verify(authzService).authorize(authentication, "_action", request, empty, null); - verifyZeroInteractions(chain); - } - - public void testUnsigned() throws Exception { - DiscoveryNodes nodes = DiscoveryNodes.builder() - .add(new DiscoveryNode("id1", new TransportAddress(TransportAddress.META_ADDRESS, randomIntBetween(49000, 65500)), - Version.V_6_0_0_alpha2)) - .add(new DiscoveryNode("id2", new TransportAddress(TransportAddress.META_ADDRESS, randomIntBetween(49000, 65500)), - Version.V_5_5_0)) - .build(); - when(clusterService.state().nodes()).thenReturn(nodes); - SearchScrollRequest request = new SearchScrollRequest("unsigned"); - ActionListener listener = mock(ActionListener.class); - ActionFilterChain chain = mock(ActionFilterChain.class); - User user = mock(User.class); - Task task = mock(Task.class); - Authentication authentication = new Authentication(user, new RealmRef("test", "test", "foo"), null); - doAnswer((i) -> { - ActionListener callback = - (ActionListener) i.getArguments()[3]; - callback.onResponse(authentication); - return Void.TYPE; - }).when(authcService).authenticate(eq("_action"), eq(request), eq(SystemUser.INSTANCE), any(ActionListener.class)); - when(cryptoService.isSigned("unsigned")).thenReturn(false); - when(cryptoService.isSystemKeyPresent()).thenReturn(randomBoolean()); - final Role empty = Role.EMPTY; - doAnswer((i) -> { - ActionListener callback = - (ActionListener) i.getArguments()[1]; - callback.onResponse(empty); - return Void.TYPE; - }).when(authzService).roles(any(User.class), any(ActionListener.class)); - filter.apply(task, "_action", request, listener, chain); - assertThat(request.scrollId(), equalTo("unsigned")); - - verify(authzService).authorize(authentication, "_action", request, empty, null); - verify(chain).proceed(eq(task), eq("_action"), eq(request), isA(ContextPreservingActionListener.class)); - } - - public void testActionSignatureError() throws Exception { - SearchScrollRequest request = new SearchScrollRequest("scroll_id"); - ActionListener listener = mock(ActionListener.class); - ActionFilterChain chain = mock(ActionFilterChain.class); - IllegalArgumentException sigException = new IllegalArgumentException("bad bad boy"); - User user = mock(User.class); - Task task = mock(Task.class); - Authentication authentication = new Authentication(user, new RealmRef("test", "test", "foo"), null); - doAnswer((i) -> { - ActionListener callback = - (ActionListener) i.getArguments()[3]; - callback.onResponse(authentication); - return Void.TYPE; - }).when(authcService).authenticate(eq("_action"), eq(request), eq(SystemUser.INSTANCE), any(ActionListener.class)); - when(cryptoService.isSystemKeyPresent()).thenReturn(true); - when(cryptoService.isSigned("scroll_id")).thenReturn(true); - doThrow(sigException).when(cryptoService).unsignAndVerify("scroll_id"); - doAnswer((i) -> { - ActionListener callback = - (ActionListener) i.getArguments()[1]; - callback.onResponse(Role.EMPTY); - return Void.TYPE; - }).when(authzService).roles(any(User.class), any(ActionListener.class)); - filter.apply(task, "_action", request, listener, chain); - verify(listener).onFailure(isA(ElasticsearchSecurityException.class)); - verify(auditTrail).tamperedRequest(user, "_action", request); - verifyNoMoreInteractions(chain); - } - public void testApplyUnlicensed() throws Exception { ActionRequest request = mock(ActionRequest.class); ActionListener listener = mock(ActionListener.class); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/crypto/CryptoServiceTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/crypto/CryptoServiceTests.java index 838ca1cb662..03b4e04b165 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/crypto/CryptoServiceTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/crypto/CryptoServiceTests.java @@ -20,7 +20,6 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; public class CryptoServiceTests extends ESTestCase { private Settings settings; @@ -42,95 +41,6 @@ public class CryptoServiceTests extends ESTestCase { env = new Environment(settings); } - public void testSigned() throws Exception { - CryptoService service = new CryptoService(settings, env); - String text = randomAlphaOfLength(10); - String signed = service.sign(text); - assertThat(service.isSigned(signed), is(true)); - } - - public void testSignAndUnsign() throws Exception { - CryptoService service = new CryptoService(settings, env); - String text = randomAlphaOfLength(10); - String signed = service.sign(text); - assertThat(text.equals(signed), is(false)); - String text2 = service.unsignAndVerify(signed); - assertThat(text, equalTo(text2)); - } - - public void testSignAndUnsignNoKeyFile() throws Exception { - Files.delete(keyFile); - CryptoService service = new CryptoService(Settings.EMPTY, env); - final String text = randomAlphaOfLength(10); - String signed = service.sign(text); - assertThat(text, equalTo(signed)); - String unsigned = service.unsignAndVerify(signed); - assertThat(unsigned, equalTo(text)); - } - - public void testTamperedSignature() throws Exception { - CryptoService service = new CryptoService(settings, env); - String text = randomAlphaOfLength(10); - String signed = service.sign(text); - int i = signed.indexOf("$$", 2); - int length = Integer.parseInt(signed.substring(2, i)); - String fakeSignature = randomAlphaOfLength(length); - String fakeSignedText = "$$" + length + "$$" + fakeSignature + signed.substring(i + 2 + length); - - try { - service.unsignAndVerify(fakeSignedText); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), is(equalTo("tampered signed text"))); - assertThat(e.getCause(), is(nullValue())); - } - } - - public void testTamperedSignatureOneChar() throws Exception { - CryptoService service = new CryptoService(settings, env); - String text = randomAlphaOfLength(10); - String signed = service.sign(text); - int i = signed.indexOf("$$", 2); - int length = Integer.parseInt(signed.substring(2, i)); - StringBuilder fakeSignature = new StringBuilder(signed.substring(i + 2, i + 2 + length)); - fakeSignature.setCharAt(randomIntBetween(0, fakeSignature.length() - 1), randomAlphaOfLength(1).charAt(0)); - - String fakeSignedText = "$$" + length + "$$" + fakeSignature.toString() + signed.substring(i + 2 + length); - - try { - service.unsignAndVerify(fakeSignedText); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), is(equalTo("tampered signed text"))); - assertThat(e.getCause(), is(nullValue())); - } - } - - public void testTamperedSignatureLength() throws Exception { - CryptoService service = new CryptoService(settings, env); - String text = randomAlphaOfLength(10); - String signed = service.sign(text); - int i = signed.indexOf("$$", 2); - int length = Integer.parseInt(signed.substring(2, i)); - String fakeSignature = randomAlphaOfLength(length); - - // Smaller sig length - String fakeSignedText = "$$" + randomIntBetween(0, length - 1) + "$$" + fakeSignature + signed.substring(i + 2 + length); - - try { - service.unsignAndVerify(fakeSignedText); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), is(equalTo("tampered signed text"))); - } - - // Larger sig length - fakeSignedText = "$$" + randomIntBetween(length + 1, Integer.MAX_VALUE) + "$$" + fakeSignature + signed.substring(i + 2 + length); - try { - service.unsignAndVerify(fakeSignedText); - } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), is(equalTo("tampered signed text"))); - assertThat(e.getCause(), is(nullValue())); - } - } - public void testEncryptionAndDecryptionChars() throws Exception { CryptoService service = new CryptoService(settings, env); assertThat(service.isEncryptionEnabled(), is(true));