Remove signing code completely from master (elastic/x-pack-elasticsearch#1719)

After improving the authorization of scroll requests and backporting to 5.x, we no longer need to
have any signing code in master. This commit removes it.

Original commit: elastic/x-pack-elasticsearch@8b65fd9338

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<RequestInterceptor> 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,
+                                XPackLicenseState licenseState, Set<RequestInterceptor> requestInterceptors, ThreadPool threadPool,
-                                Set<RequestInterceptor> requestInterceptors, ThreadPool threadPool,
+                                SecurityContext securityContext, DestructiveOperations destructiveOperations) {
-                                SecurityContext securityContext, DestructiveOperations destructiveOperations,
-                                ClusterService clusterService) {
         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<ThreadContext.StoredContext> toRestore = threadContext.newRestorableContext(true);
+            final ActionListener<ActionResponse> contextPreservingListener =
-            final ActionListener<ActionResponse> signingListener = new ContextPreservingActionListener<>(toRestore,
+                    ContextPreservingActionListener.wrapPreservingContext(listener, threadContext);
-                    ActionListener.wrap(r -> {
+            ActionListener<Void> authenticatedListener = ActionListener.wrap(
-                        try {
+                    (aVoid) -> chain.proceed(task, action, request, contextPreservingListener), contextPreservingListener::onFailure);
-                            listener.onResponse(sign(r));
-                        } catch (IOException e) {
-                            throw new UncheckedIOException(e);
-                        }
-                    }, listener::onFailure));
-            ActionListener<Void> authenticatedListener =
-                    ActionListener.wrap((aVoid) -> chain.proceed(task, action, request, signingListener), signingListener::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)) {
+                            if (interceptor.supports(request)) {
-                                interceptor.intercept(unsignedRequest, user);
+                                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<String> signedIds = clearScrollRequest.scrollIds();
-                    List<String> 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 extends ActionResponse> 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;
-    }
 }

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<Boolean> SYSTEM_KEY_REQUIRED_SETTING =
             Setting.boolSetting(setting("system_key.required"), false, Property.NodeScope);
     private static final Setting<String> 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
+     * The encryption key is derived from the system key.
-     * and the key is also used for encrypting sensitive data stored in watches. 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> 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<Setting<?>> settings) {
         settings.add(ENCRYPTION_KEY_LENGTH_SETTING);
         settings.add(ENCRYPTION_KEY_ALGO_SETTING);

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,
+        filter = new SecurityActionFilter(Settings.EMPTY, authcService, authzService,
-                        licenseState, new HashSet<>(), threadPool, securityContext, destructiveOperations, clusterService);
+                        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<ElasticsearchSecurityException> 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);

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));