Skip to content

Commit

Permalink
Security: fix token bwc with pre 6.0.0-beta2 (#31254)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@jaymode
jaymode committed Jun 13, 2018
1 parent 20c860a commit d3ba009
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 57 deletions.
63 changes: 32 additions & 31 deletions ...ck/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.StringHelper;
Expand Down Expand Up @@ -1043,7 +1042,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 =
Expand Down Expand Up @@ -1371,16 +1372,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 {
Expand All @@ -1396,33 +1399,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);
}
});
}
}

Expand Down
81 changes: 60 additions & 21 deletions ...ugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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.SecurityLifecycleService;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
Expand All @@ -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;
Expand All @@ -91,6 +97,7 @@ public class TokenServiceTests extends ESTestCase {
private Client client;
private SecurityLifecycleService lifecycleService;
private ClusterService clusterService;
private boolean mixedCluster;
private Settings tokenServiceEnabledSettings = Settings.builder()
.put(XPackSettings.TOKEN_SERVICE_ENABLED_SETTING.getKey(), true).build();

Expand Down Expand Up @@ -141,6 +148,25 @@ public void setupClient() {
return null;
}).when(lifecycleService).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
Expand Down Expand Up @@ -173,7 +199,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)) {
Expand All @@ -184,13 +210,13 @@ 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 {
TokenService tokenService =
new TokenService(tokenServiceEnabledSettings, systemUTC(), client, lifecycleService, clusterService);
assumeFalse("internally managed keys do not work in a mixed cluster", mixedCluster);
TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, lifecycleService, clusterService);
Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null);
PlainActionFuture<Tuple<UserToken, String>> tokenFuture = new PlainActionFuture<>();
tokenService.createUserToken(authentication, authentication, tokenFuture, Collections.emptyMap());
Expand All @@ -205,15 +231,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<>();
Expand Down Expand Up @@ -242,8 +268,8 @@ private void rotateKeys(TokenService tokenService) {
}

public void testKeyExchange() throws Exception {
TokenService tokenService =
new TokenService(tokenServiceEnabledSettings, systemUTC(), client, lifecycleService, clusterService);
assumeFalse("internally managed keys do not work in a mixed cluster", mixedCluster);
TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, lifecycleService, clusterService);
int numRotations = 0;randomIntBetween(1, 5);
for (int i = 0; i < numRotations; i++) {
rotateKeys(tokenService);
Expand All @@ -264,7 +290,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);
Expand All @@ -275,13 +301,13 @@ 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 {
TokenService tokenService =
new TokenService(tokenServiceEnabledSettings, systemUTC(), client, lifecycleService, clusterService);
assumeFalse("internally managed keys do not work in a mixed cluster", mixedCluster);
TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, lifecycleService, clusterService);
Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null);
PlainActionFuture<Tuple<UserToken, String>> tokenFuture = new PlainActionFuture<>();
tokenService.createUserToken(authentication, authentication, tokenFuture, Collections.emptyMap());
Expand All @@ -296,7 +322,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);
Expand All @@ -310,7 +336,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<>();
Expand All @@ -336,7 +362,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());
}

}
Expand All @@ -358,7 +384,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)) {
Expand Down Expand Up @@ -459,7 +485,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);
Expand All @@ -469,7 +495,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)) {
Expand All @@ -478,7 +504,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)) {
Expand Down Expand Up @@ -574,7 +600,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(lifecycleService.isSecurityIndexAvailable()).thenReturn(false);
when(lifecycleService.isSecurityIndexExisting()).thenReturn(true);
Expand Down Expand Up @@ -606,6 +632,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, lifecycleService, clusterService);
Expand All @@ -616,7 +643,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) {
Expand All @@ -643,4 +670,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);
}
}
}
1 change: 0 additions & 1 deletion x-pack/qa/rolling-upgrade/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Loading

0 comments on commit d3ba009

Please sign in to comment.