Security: fix token bwc with pre 6.0.0-beta2 (#31254)

This commit fixes a backwards compatibility bug in the token service
that causes token decoding to fail when there is a pre 6.0.0-beta2 node
in the cluster. The token encoding is actually the culprit as a version
check is missing around the serialization of the key hash bytes. This
value was added in 6.0.0-beta2 and cannot be sent to nodes that do not
know about this value. The version check has been added and the token
service unit tests have been enhanced to randomly run with some 5.6.x
nodes in the cluster service.

Additionally, a small change was made to the way we check to see if the
token metadata needs to be installed. Previously we would pass the
metadata to the install method and check that the token metadata is
null. This null check is now done prior to checking if the metadata can
be installed.

Relates #30743
Closes #31195

Author: jaymode

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java

@@ -9,7 +9,6 @@
 import org.apache.lucene.util.BytesRef;
 import org.apache.lucene.util.BytesRefBuilder;
 import org.elasticsearch.cluster.ClusterStateUpdateTask;
-import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.common.Priority;
 import org.elasticsearch.core.internal.io.IOUtils;
 import org.apache.lucene.util.UnicodeUtil;
@@ -1041,7 +1040,9 @@ public String getUserTokenString(UserToken userToken) throws IOException, Genera
             KeyAndCache keyAndCache = keyCache.activeKeyCache;
             Version.writeVersion(userToken.getVersion(), out);
             out.writeByteArray(keyAndCache.getSalt().bytes);
-            out.writeByteArray(keyAndCache.getKeyHash().bytes);
+            if (userToken.getVersion().onOrAfter(Version.V_6_0_0_beta2)) {
+                out.writeByteArray(keyAndCache.getKeyHash().bytes);
+            }
             final byte[] initializationVector = getNewInitializationVector();
             out.writeByteArray(initializationVector);
             try (CipherOutputStream encryptedOutput =
@@ -1369,16 +1370,18 @@ private void initialize(ClusterService clusterService) {
                 return;
             }
 
+            TokenMetaData custom = event.state().custom(TokenMetaData.TYPE);
             if (state.nodes().isLocalNodeElectedMaster()) {
-                if (XPackPlugin.isReadyForXPackCustomMetadata(state)) {
-                    installTokenMetadata(state.metaData());
-                } else {
-                    logger.debug("cannot add token metadata to cluster as the following nodes might not understand the metadata: {}",
-                        () -> XPackPlugin.nodesNotReadyForXPackCustomMetadata(state));
+                if (custom == null) {
+                    if (XPackPlugin.isReadyForXPackCustomMetadata(state)) {
+                        installTokenMetadata();
+                    } else {
+                        logger.debug("cannot add token metadata to cluster as the following nodes might not understand the metadata: {}",
+                            () -> XPackPlugin.nodesNotReadyForXPackCustomMetadata(state));
+                    }
                 }
             }
 
-            TokenMetaData custom = event.state().custom(TokenMetaData.TYPE);
             if (custom != null && custom.equals(getTokenMetaData()) == false) {
                 logger.info("refresh keys");
                 try {
@@ -1394,33 +1397,31 @@ private void initialize(ClusterService clusterService) {
     // to prevent too many cluster state update tasks to be queued for doing the same update
     private final AtomicBoolean installTokenMetadataInProgress = new AtomicBoolean(false);
 
-    private void installTokenMetadata(MetaData metaData) {
-        if (metaData.custom(TokenMetaData.TYPE) == null) {
-            if (installTokenMetadataInProgress.compareAndSet(false, true)) {
-                clusterService.submitStateUpdateTask("install-token-metadata", new ClusterStateUpdateTask(Priority.URGENT) {
-                    @Override
-                    public ClusterState execute(ClusterState currentState) {
-                        XPackPlugin.checkReadyForXPackCustomMetadata(currentState);
+    private void installTokenMetadata() {
+        if (installTokenMetadataInProgress.compareAndSet(false, true)) {
+            clusterService.submitStateUpdateTask("install-token-metadata", new ClusterStateUpdateTask(Priority.URGENT) {
+                @Override
+                public ClusterState execute(ClusterState currentState) {
+                    XPackPlugin.checkReadyForXPackCustomMetadata(currentState);
 
-                        if (currentState.custom(TokenMetaData.TYPE) == null) {
-                            return ClusterState.builder(currentState).putCustom(TokenMetaData.TYPE, getTokenMetaData()).build();
-                        } else {
-                            return currentState;
-                        }
+                    if (currentState.custom(TokenMetaData.TYPE) == null) {
+                        return ClusterState.builder(currentState).putCustom(TokenMetaData.TYPE, getTokenMetaData()).build();
+                    } else {
+                        return currentState;
                     }
+                }
 
-                    @Override
-                    public void onFailure(String source, Exception e) {
-                        installTokenMetadataInProgress.set(false);
-                        logger.error("unable to install token metadata", e);
-                    }
+                @Override
+                public void onFailure(String source, Exception e) {
+                    installTokenMetadataInProgress.set(false);
+                    logger.error("unable to install token metadata", e);
+                }
 
-                    @Override
-                    public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) {
-                        installTokenMetadataInProgress.set(false);
-                    }
-                });
-            }
+                @Override
+                public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) {
+                    installTokenMetadataInProgress.set(false);
+                }
+            });
         }
     }
 

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java

@@ -26,6 +26,9 @@
 import org.elasticsearch.action.update.UpdateAction;
 import org.elasticsearch.action.update.UpdateRequestBuilder;
 import org.elasticsearch.client.Client;
+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.MockSecureSettings;
 import org.elasticsearch.common.Strings;
@@ -44,6 +47,7 @@
 import org.elasticsearch.test.ClusterServiceUtils;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.EqualsHashCodeTestUtils;
+import org.elasticsearch.test.VersionUtils;
 import org.elasticsearch.threadpool.FixedExecutorBuilder;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.xpack.core.XPackSettings;
@@ -53,6 +57,7 @@
 import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.core.watcher.watch.ClockMock;
 import org.elasticsearch.xpack.security.support.SecurityIndexManager;
+import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
@@ -66,6 +71,7 @@
 import java.time.temporal.ChronoUnit;
 import java.util.Base64;
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.ExecutionException;
@@ -91,6 +97,7 @@ public class TokenServiceTests extends ESTestCase {
     private Client client;
     private SecurityIndexManager securityIndex;
     private ClusterService clusterService;
+    private boolean mixedCluster;
     private Settings tokenServiceEnabledSettings = Settings.builder()
         .put(XPackSettings.TOKEN_SERVICE_ENABLED_SETTING.getKey(), true).build();
 
@@ -141,6 +148,25 @@ public void setupClient() {
             return null;
         }).when(securityIndex).prepareIndexIfNeededThenExecute(any(Consumer.class), any(Runnable.class));
         this.clusterService = ClusterServiceUtils.createClusterService(threadPool);
+        this.mixedCluster = randomBoolean();
+        if (mixedCluster) {
+            Version version = VersionUtils.randomVersionBetween(random(), Version.V_5_6_0, Version.V_5_6_10);
+            logger.info("adding a node with version [{}] to the cluster service", version);
+            ClusterState updatedState = ClusterState.builder(clusterService.state())
+                .nodes(DiscoveryNodes.builder(clusterService.state().nodes())
+                    .add(new DiscoveryNode("56node", ESTestCase.buildNewFakeTransportAddress(), Collections.emptyMap(),
+                        EnumSet.allOf(DiscoveryNode.Role.class), version))
+                    .build())
+                .build();
+            ClusterServiceUtils.setState(clusterService, updatedState);
+        }
+    }
+
+    @After
+    public void stopClusterService() {
+        if (clusterService != null) {
+            clusterService.close();
+        }
     }
 
     @BeforeClass
@@ -172,7 +198,7 @@ public void testAttachAndGetToken() throws Exception {
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             tokenService.getAndValidateToken(requestContext, future);
             UserToken serialized = future.get();
-            assertEquals(authentication, serialized.getAuthentication());
+            assertAuthenticationEquals(authentication, serialized.getAuthentication());
         }
 
         try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) {
@@ -183,11 +209,12 @@ public void testAttachAndGetToken() throws Exception {
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             anotherService.getAndValidateToken(requestContext, future);
             UserToken fromOtherService = future.get();
-            assertEquals(authentication, fromOtherService.getAuthentication());
+            assertAuthenticationEquals(authentication, fromOtherService.getAuthentication());
         }
     }
 
     public void testRotateKey() throws Exception {
+        assumeFalse("internally managed keys do not work in a mixed cluster", mixedCluster);
         TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, securityIndex, clusterService);
         Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null);
         PlainActionFuture<Tuple<UserToken, String>> tokenFuture = new PlainActionFuture<>();
@@ -203,15 +230,15 @@ public void testRotateKey() throws Exception {
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             tokenService.getAndValidateToken(requestContext, future);
             UserToken serialized = future.get();
-            assertEquals(authentication, serialized.getAuthentication());
+            assertAuthenticationEquals(authentication, serialized.getAuthentication());
         }
         rotateKeys(tokenService);
 
         try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) {
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             tokenService.getAndValidateToken(requestContext, future);
             UserToken serialized = future.get();
-            assertEquals(authentication, serialized.getAuthentication());
+            assertAuthenticationEquals(authentication, serialized.getAuthentication());
         }
 
         PlainActionFuture<Tuple<UserToken, String>> newTokenFuture = new PlainActionFuture<>();
@@ -240,6 +267,7 @@ private void rotateKeys(TokenService tokenService) {
     }
 
     public void testKeyExchange() throws Exception {
+        assumeFalse("internally managed keys do not work in a mixed cluster", mixedCluster);
         TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, securityIndex, clusterService);
         int numRotations = 0;randomIntBetween(1, 5);
         for (int i = 0; i < numRotations; i++) {
@@ -261,7 +289,7 @@ public void testKeyExchange() throws Exception {
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             otherTokenService.getAndValidateToken(requestContext, future);
             UserToken serialized = future.get();
-            assertEquals(authentication, serialized.getAuthentication());
+            assertAuthenticationEquals(authentication, serialized.getAuthentication());
         }
 
         rotateKeys(tokenService);
@@ -272,11 +300,12 @@ public void testKeyExchange() throws Exception {
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             otherTokenService.getAndValidateToken(requestContext, future);
             UserToken serialized = future.get();
-            assertEquals(authentication, serialized.getAuthentication());
+            assertAuthenticationEquals(authentication, serialized.getAuthentication());
         }
     }
 
     public void testPruneKeys() throws Exception {
+        assumeFalse("internally managed keys do not work in a mixed cluster", mixedCluster);
         TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, securityIndex, clusterService);
         Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null);
         PlainActionFuture<Tuple<UserToken, String>> tokenFuture = new PlainActionFuture<>();
@@ -292,7 +321,7 @@ public void testPruneKeys() throws Exception {
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             tokenService.getAndValidateToken(requestContext, future);
             UserToken serialized = future.get();
-            assertEquals(authentication, serialized.getAuthentication());
+            assertAuthenticationEquals(authentication, serialized.getAuthentication());
         }
         TokenMetaData metaData = tokenService.pruneKeys(randomIntBetween(0, 100));
         tokenService.refreshMetaData(metaData);
@@ -306,7 +335,7 @@ public void testPruneKeys() throws Exception {
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             tokenService.getAndValidateToken(requestContext, future);
             UserToken serialized = future.get();
-            assertEquals(authentication, serialized.getAuthentication());
+            assertAuthenticationEquals(authentication, serialized.getAuthentication());
         }
 
         PlainActionFuture<Tuple<UserToken, String>> newTokenFuture = new PlainActionFuture<>();
@@ -332,7 +361,7 @@ public void testPruneKeys() throws Exception {
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             tokenService.getAndValidateToken(requestContext, future);
             UserToken serialized = future.get();
-            assertEquals(authentication, serialized.getAuthentication());
+            assertAuthenticationEquals(authentication, serialized.getAuthentication());
         }
 
     }
@@ -353,7 +382,7 @@ public void testPassphraseWorks() throws Exception {
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             tokenService.getAndValidateToken(requestContext, future);
             UserToken serialized = future.get();
-            assertEquals(authentication, serialized.getAuthentication());
+            assertAuthenticationEquals(authentication, serialized.getAuthentication());
         }
 
         try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) {
@@ -454,7 +483,7 @@ public void testTokenExpiry() throws Exception {
             // the clock is still frozen, so the cookie should be valid
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             tokenService.getAndValidateToken(requestContext, future);
-            assertEquals(authentication, future.get().getAuthentication());
+            assertAuthenticationEquals(authentication, future.get().getAuthentication());
         }
 
         final TimeValue defaultExpiration = TokenService.TOKEN_EXPIRATION.get(Settings.EMPTY);
@@ -464,7 +493,7 @@ public void testTokenExpiry() throws Exception {
             clock.fastForwardSeconds(fastForwardAmount);
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             tokenService.getAndValidateToken(requestContext, future);
-            assertEquals(authentication, future.get().getAuthentication());
+            assertAuthenticationEquals(authentication, future.get().getAuthentication());
         }
 
         try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) {
@@ -473,7 +502,7 @@ public void testTokenExpiry() throws Exception {
             clock.rewind(TimeValue.timeValueNanos(clock.instant().getNano())); // trim off nanoseconds since don't store them in the index
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             tokenService.getAndValidateToken(requestContext, future);
-            assertEquals(authentication, future.get().getAuthentication());
+            assertAuthenticationEquals(authentication, future.get().getAuthentication());
         }
 
         try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) {
@@ -569,7 +598,7 @@ public void testIndexNotAvailable() throws Exception {
             PlainActionFuture<UserToken> future = new PlainActionFuture<>();
             tokenService.getAndValidateToken(requestContext, future);
             UserToken serialized = future.get();
-            assertEquals(authentication, serialized.getAuthentication());
+            assertAuthenticationEquals(authentication, serialized.getAuthentication());
 
             when(securityIndex.isAvailable()).thenReturn(false);
             when(securityIndex.indexExists()).thenReturn(true);
@@ -601,6 +630,7 @@ public void testDecodePre6xToken() throws GeneralSecurityException, ExecutionExc
         assertWarnings("[xpack.security.authc.token.passphrase] setting was deprecated in Elasticsearch and will be removed in a future" +
                 " release! See the breaking changes documentation for the next major version.");
     }
+
     public void testGetAuthenticationWorksWithExpiredToken() throws Exception {
         TokenService tokenService =
                 new TokenService(tokenServiceEnabledSettings, Clock.systemUTC(), client, securityIndex, clusterService);
@@ -611,7 +641,7 @@ public void testGetAuthenticationWorksWithExpiredToken() throws Exception {
         PlainActionFuture<Tuple<Authentication, Map<String, Object>>> authFuture = new PlainActionFuture<>();
         tokenService.getAuthenticationAndMetaData(userTokenString, authFuture);
         Authentication retrievedAuth = authFuture.actionGet().v1();
-        assertEquals(authentication, retrievedAuth);
+        assertAuthenticationEquals(authentication, retrievedAuth);
     }
 
     private void mockGetTokenFromId(UserToken userToken) {
@@ -638,4 +668,16 @@ public static void mockGetTokenFromId(UserToken userToken, Client client) {
             return Void.TYPE;
         }).when(client).get(any(GetRequest.class), any(ActionListener.class));
     }
+
+    private void assertAuthenticationEquals(Authentication expected, Authentication actual) {
+        if (mixedCluster) {
+            assertNotNull(expected);
+            assertNotNull(actual);
+            assertEquals(expected.getUser(), actual.getUser());
+            assertEquals(expected.getAuthenticatedBy(), actual.getAuthenticatedBy());
+            assertEquals(expected.getLookedUpBy(), actual.getLookedUpBy());
+        } else {
+            assertEquals(expected, actual);
+        }
+    }
 }

x-pack/qa/rolling-upgrade/build.gradle

@@ -146,7 +146,6 @@ subprojects {
       if (version.onOrAfter('6.0.0') == false) {
         // this is needed since in 5.6 we don't bootstrap the token service if there is no explicit initial password
         keystoreSetting 'xpack.security.authc.token.passphrase', 'xpack_token_passphrase'
-        setting 'xpack.security.authc.token.enabled', 'true'
       }
       dependsOn copyTestNodeKeystore
       extraConfigFile 'testnode.jks', new File(outputDir + '/testnode.jks')