Skip to content

Commit 2c3e3c1

Browse files
BigPandaTooBigPandaToo
committed
Fixing a problem with potential dirty read of a token document.
Related to elastic#59685
1 parent ff736f0 commit 2c3e3c1

12 files changed

+60
-59
lines changed

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportDelegatePkiAuthenticationAction.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ protected void doExecute(Task task, DelegatePkiAuthenticationRequest request,
8484
tokenService.createOAuth2Tokens(authentication, delegateeAuthentication, Map.of(), false,
8585
ActionListener.wrap(tuple -> {
8686
final TimeValue expiresIn = tokenService.getExpirationDelay();
87-
listener.onResponse(new DelegatePkiAuthenticationResponse(tuple.v1(), expiresIn, authentication));
87+
listener.onResponse(new DelegatePkiAuthenticationResponse(tuple.v1().v1(), expiresIn, authentication));
8888
}, listener::onFailure));
8989
}, e -> {
9090
logger.debug((Supplier<?>) () -> new ParameterizedMessage("Delegated x509Token [{}] could not be authenticated",

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/oidc/TransportOpenIdConnectAuthenticateAction.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ protected void doExecute(Task task, OpenIdConnectAuthenticateRequest request,
7474
tokenService.createOAuth2Tokens(authentication, originatingAuthentication, tokenMetadata, true,
7575
ActionListener.wrap(tuple -> {
7676
final TimeValue expiresIn = tokenService.getExpirationDelay();
77-
listener.onResponse(new OpenIdConnectAuthenticateResponse(authentication, tuple.v1(), tuple.v2(), expiresIn));
77+
listener.onResponse(new OpenIdConnectAuthenticateResponse(authentication, tuple.v1().v1(), tuple.v1().v2(),
78+
expiresIn));
7879
}, listener::onFailure));
7980
}, e -> {
8081
logger.debug(() -> new ParameterizedMessage("OpenIDConnectToken [{}] could not be authenticated", token), e);

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlAuthenticateAction.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ protected void doExecute(Task task, SamlAuthenticateRequest request, ActionListe
6868
tokenMeta, true, ActionListener.wrap(tuple -> {
6969
final TimeValue expiresIn = tokenService.getExpirationDelay();
7070
listener.onResponse(
71-
new SamlAuthenticateResponse(authentication, tuple.v1(), tuple.v2(), expiresIn));
71+
new SamlAuthenticateResponse(authentication, tuple.v1().v1(), tuple.v1().v2(), expiresIn));
7272
}, listener::onFailure));
7373
}, e -> {
7474
logger.debug(() -> new ParameterizedMessage("SamlToken [{}] could not be authenticated", saml), e);

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/token/TransportCreateTokenAction.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ private void createToken(GrantType grantType, CreateTokenRequest request, Authen
134134
ActionListener.wrap(tuple -> {
135135
final String scope = getResponseScopeValue(request.getScope());
136136
final String base64AuthenticateResponse = (grantType == GrantType.KERBEROS) ? extractOutToken() : null;
137-
final CreateTokenResponse response = new CreateTokenResponse(tuple.v1(), tokenService.getExpirationDelay(), scope,
138-
tuple.v2(), base64AuthenticateResponse, authentication);
137+
final CreateTokenResponse response = new CreateTokenResponse(tuple.v1().v1(), tokenService.getExpirationDelay(), scope,
138+
tuple.v1().v2(), base64AuthenticateResponse, authentication);
139139
listener.onResponse(response);
140140
}, listener::onFailure));
141141
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/token/TransportRefreshTokenAction.java

+3-6
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import org.elasticsearch.action.support.ActionFilters;
1010
import org.elasticsearch.action.support.HandledTransportAction;
1111
import org.elasticsearch.common.inject.Inject;
12-
import org.elasticsearch.common.settings.SecureString;
1312
import org.elasticsearch.tasks.Task;
1413
import org.elasticsearch.transport.TransportService;
1514
import org.elasticsearch.xpack.core.security.action.token.CreateTokenRequest;
@@ -33,11 +32,9 @@ public TransportRefreshTokenAction(TransportService transportService, ActionFilt
3332
protected void doExecute(Task task, CreateTokenRequest request, ActionListener<CreateTokenResponse> listener) {
3433
tokenService.refreshToken(request.getRefreshToken(), ActionListener.wrap(tuple -> {
3534
final String scope = getResponseScopeValue(request.getScope());
36-
tokenService.authenticateToken(new SecureString(tuple.v1()), ActionListener.wrap(authentication -> {
37-
listener.onResponse(new CreateTokenResponse(tuple.v1(), tokenService.getExpirationDelay(), scope, tuple.v2(), null,
38-
authentication));
39-
},
40-
listener::onFailure));
35+
final CreateTokenResponse response =
36+
new CreateTokenResponse(tuple.v1().v1(), tokenService.getExpirationDelay(), scope, tuple.v1().v2(), null, tuple.v2());
37+
listener.onResponse(response);
4138
}, listener::onFailure));
4239
}
4340
}

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

+15-12
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ public TokenService(Settings settings, Clock clock, Client client, XPackLicenseS
254254
* {@link #VERSION_TOKENS_INDEX_INTRODUCED} and to a specific security tokens index for later versions.
255255
*/
256256
public void createOAuth2Tokens(Authentication authentication, Authentication originatingClientAuth, Map<String, Object> metadata,
257-
boolean includeRefreshToken, ActionListener<Tuple<String, String>> listener) {
257+
boolean includeRefreshToken, ActionListener<Tuple<Tuple<String, String>, Authentication>> listener) {
258258
// the created token is compatible with the oldest node version in the cluster
259259
final Version tokenVersion = getTokenVersionCompatibility();
260260
// tokens moved to a separate index in newer versions
@@ -273,7 +273,7 @@ public void createOAuth2Tokens(Authentication authentication, Authentication ori
273273
//public for testing
274274
public void createOAuth2Tokens(String accessToken, String refreshToken, Authentication authentication,
275275
Authentication originatingClientAuth,
276-
Map<String, Object> metadata, ActionListener<Tuple<String, String>> listener) {
276+
Map<String, Object> metadata, ActionListener<Tuple<Tuple<String, String>, Authentication>> listener) {
277277
// the created token is compatible with the oldest node version in the cluster
278278
final Version tokenVersion = getTokenVersionCompatibility();
279279
// tokens moved to a separate index in newer versions
@@ -311,7 +311,7 @@ public void createOAuth2Tokens(String accessToken, String refreshToken, Authenti
311311
*/
312312
private void createOAuth2Tokens(String accessToken, String refreshToken, Version tokenVersion, SecurityIndexManager tokensIndex,
313313
Authentication authentication, Authentication originatingClientAuth, Map<String, Object> metadata,
314-
ActionListener<Tuple<String, String>> listener) {
314+
ActionListener<Tuple<Tuple<String, String>, Authentication>> listener) {
315315
assert accessToken.length() == TOKEN_LENGTH : "We assume token ids have a fixed length for nodes of a certain version."
316316
+ " When changing the token length, be careful that the inferences about its length still hold.";
317317
ensureEnabled();
@@ -351,12 +351,13 @@ private void createOAuth2Tokens(String accessToken, String refreshToken, Version
351351
final String versionedRefreshToken = refreshToken != null
352352
? prependVersionAndEncodeRefreshToken(tokenVersion, refreshToken)
353353
: null;
354-
listener.onResponse(new Tuple<>(versionedAccessToken, versionedRefreshToken));
354+
listener.onResponse(new Tuple<>(new Tuple<>(versionedAccessToken, versionedRefreshToken),
355+
tokenAuth));
355356
} else {
356357
// prior versions of the refresh token are not version-prepended, as nodes on those
357358
// versions don't expect it.
358359
// Such nodes might exist in a mixed cluster during a rolling upgrade.
359-
listener.onResponse(new Tuple<>(versionedAccessToken, refreshToken));
360+
listener.onResponse(new Tuple<>(new Tuple<>(versionedAccessToken, refreshToken),tokenAuth));
360361
}
361362
} else {
362363
listener.onFailure(traceLog("create token",
@@ -862,7 +863,7 @@ private void indexInvalidation(Collection<String> tokenIds, SecurityIndexManager
862863
* @param listener The listener to call upon completion with a {@link Tuple} containing the
863864
* serialized access token and serialized refresh token as these will be returned to the client
864865
*/
865-
public void refreshToken(String refreshToken, ActionListener<Tuple<String, String>> listener) {
866+
public void refreshToken(String refreshToken, ActionListener<Tuple<Tuple<String, String>, Authentication>> listener) {
866867
ensureEnabled();
867868
final Instant refreshRequested = clock.instant();
868869
final Iterator<TimeValue> backoff = DEFAULT_BACKOFF.iterator();
@@ -995,7 +996,7 @@ private void findTokenFromRefreshToken(String refreshToken, SecurityIndexManager
995996
*/
996997
private void innerRefresh(String refreshToken, String tokenDocId, Map<String, Object> source, long seqNo, long primaryTerm,
997998
Authentication clientAuth, Iterator<TimeValue> backoff, Instant refreshRequested,
998-
ActionListener<Tuple<String, String>> listener) {
999+
ActionListener<Tuple<Tuple<String, String>, Authentication>> listener) {
9991000
logger.debug("Attempting to refresh token stored in token document [{}]", tokenDocId);
10001001
final Consumer<Exception> onFailure = ex -> listener.onFailure(traceLog("refresh token", tokenDocId, ex));
10011002
final Tuple<RefreshTokenStatus, Optional<ElasticsearchSecurityException>> checkRefreshResult;
@@ -1126,11 +1127,12 @@ public void onFailure(Exception e) {
11261127
* @param refreshTokenStatus The {@link RefreshTokenStatus} containing information about the superseding tokens as retrieved from the
11271128
* index
11281129
* @param tokensIndex the manager for the index where the tokens are stored
1129-
* @param listener The listener to call upon completion with a {@link Tuple} containing the
1130-
* serialized access token and serialized refresh token as these will be returned to the client
1130+
* @param listener The listener to call upon completion with a {@link Tuple} containing the Tuple of
1131+
* serialized access token and serialized refresh token and Authentication object as these will be returned
1132+
* to the client
11311133
*/
11321134
void decryptAndReturnSupersedingTokens(String refreshToken, RefreshTokenStatus refreshTokenStatus, SecurityIndexManager tokensIndex,
1133-
ActionListener<Tuple<String, String>> listener) {
1135+
ActionListener<Tuple<Tuple<String, String>, Authentication>> listener) {
11341136

11351137
final byte[] iv = Base64.getDecoder().decode(refreshTokenStatus.getIv());
11361138
final byte[] salt = Base64.getDecoder().decode(refreshTokenStatus.getSalt());
@@ -1166,8 +1168,9 @@ public void onResponse(GetResponse response) {
11661168
if (response.isExists()) {
11671169
try {
11681170
listener.onResponse(
1169-
new Tuple<>(prependVersionAndEncodeAccessToken(refreshTokenStatus.getVersion(), decryptedTokens[0]),
1170-
prependVersionAndEncodeRefreshToken(refreshTokenStatus.getVersion(), decryptedTokens[1])));
1171+
new Tuple<>(new Tuple<>(prependVersionAndEncodeAccessToken(refreshTokenStatus.getVersion(),
1172+
decryptedTokens[0]),
1173+
prependVersionAndEncodeRefreshToken(refreshTokenStatus.getVersion(), decryptedTokens[1])), null));
11711174
} catch (GeneralSecurityException | IOException e) {
11721175
logger.warn("Could not format stored superseding token values", e);
11731176
onFailure.accept(invalidGrantException("could not refresh the requested token"));

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/oidc/TransportOpenIdConnectLogoutActionTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,11 @@ public void testLogoutInvalidatesTokens() throws Exception {
203203
final Authentication authentication = new Authentication(user, realmRef, null, null, Authentication.AuthenticationType.REALM,
204204
tokenMetadata);
205205

206-
final PlainActionFuture<Tuple<String, String>> future = new PlainActionFuture<>();
206+
final PlainActionFuture<Tuple<Tuple<String, String>, Authentication>> future = new PlainActionFuture<>();
207207
final String userTokenId = UUIDs.randomBase64UUID();
208208
final String refreshToken = UUIDs.randomBase64UUID();
209209
tokenService.createOAuth2Tokens(userTokenId, refreshToken, authentication, authentication, tokenMetadata, future);
210-
final String accessToken = future.actionGet().v1();
210+
final String accessToken = future.actionGet().v1().v1();
211211
mockGetTokenFromId(tokenService, userTokenId, authentication, false, client);
212212

213213
final OpenIdConnectLogoutRequest request = new OpenIdConnectLogoutRequest();

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlInvalidateSessionActionTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,9 @@ private Tuple<String, String> storeToken(String userTokenId, String refreshToken
376376
Authentication authentication = new Authentication(new User("bob"),
377377
new RealmRef("native", NativeRealmSettings.TYPE, "node01"), null);
378378
final Map<String, Object> metadata = samlRealm.createTokenMetadata(nameId, session);
379-
final PlainActionFuture<Tuple<String, String>> future = new PlainActionFuture<>();
379+
final PlainActionFuture<Tuple<Tuple<String, String>, Authentication>> future = new PlainActionFuture<>();
380380
tokenService.createOAuth2Tokens(userTokenId, refreshToken, authentication, authentication, metadata, future);
381-
return future.actionGet();
381+
return future.actionGet().v1();
382382
}
383383

384384
private Tuple<String, String> storeToken(SamlNameId nameId, String session) {

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/saml/TransportSamlLogoutActionTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -246,11 +246,11 @@ public void testLogoutInvalidatesToken() throws Exception {
246246
tokenMetadata);
247247

248248

249-
final PlainActionFuture<Tuple<String, String>> future = new PlainActionFuture<>();
249+
final PlainActionFuture<Tuple<Tuple<String, String>, Authentication>> future = new PlainActionFuture<>();
250250
final String userTokenId = UUIDs.randomBase64UUID();
251251
final String refreshToken = UUIDs.randomBase64UUID();
252252
tokenService.createOAuth2Tokens(userTokenId, refreshToken, authentication, authentication, tokenMetadata, future);
253-
final String accessToken = future.actionGet().v1();
253+
final String accessToken = future.actionGet().v1().v1();
254254
mockGetTokenFromId(tokenService, userTokenId, authentication, false, client);
255255

256256
final SamlLogoutRequest request = new SamlLogoutRequest();

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -1305,14 +1305,14 @@ public void testAuthenticateWithToken() throws Exception {
13051305
User user = new User("_username", "r1");
13061306
final AtomicBoolean completed = new AtomicBoolean(false);
13071307
final Authentication expected = new Authentication(user, new RealmRef("realm", "custom", "node"), null);
1308-
PlainActionFuture<Tuple<String, String>> tokenFuture = new PlainActionFuture<>();
1308+
PlainActionFuture<Tuple<Tuple<String, String>, Authentication>> tokenFuture = new PlainActionFuture<>();
13091309
final String userTokenId = UUIDs.randomBase64UUID();
13101310
final String refreshToken = UUIDs.randomBase64UUID();
13111311
try (ThreadContext.StoredContext ctx = threadContext.stashContext()) {
13121312
Authentication originatingAuth = new Authentication(new User("creator"), new RealmRef("test", "test", "test"), null);
13131313
tokenService.createOAuth2Tokens(userTokenId, refreshToken, expected, originatingAuth, Collections.emptyMap(), tokenFuture);
13141314
}
1315-
String token = tokenFuture.get().v1();
1315+
String token = tokenFuture.get().v1().v1();
13161316
when(client.prepareMultiGet()).thenReturn(new MultiGetRequestBuilder(client, MultiGetAction.INSTANCE));
13171317
mockGetTokenFromId(tokenService, userTokenId, expected, false, client);
13181318
when(securityIndex.freeze()).thenReturn(securityIndex);
@@ -1393,14 +1393,14 @@ public void testExpiredToken() throws Exception {
13931393
when(securityIndex.indexExists()).thenReturn(true);
13941394
User user = new User("_username", "r1");
13951395
final Authentication expected = new Authentication(user, new RealmRef("realm", "custom", "node"), null);
1396-
PlainActionFuture<Tuple<String, String>> tokenFuture = new PlainActionFuture<>();
1396+
PlainActionFuture<Tuple<Tuple<String, String>, Authentication>> tokenFuture = new PlainActionFuture<>();
13971397
final String userTokenId = UUIDs.randomBase64UUID();
13981398
final String refreshToken = UUIDs.randomBase64UUID();
13991399
try (ThreadContext.StoredContext ctx = threadContext.stashContext()) {
14001400
Authentication originatingAuth = new Authentication(new User("creator"), new RealmRef("test", "test", "test"), null);
14011401
tokenService.createOAuth2Tokens(userTokenId, refreshToken, expected, originatingAuth, Collections.emptyMap(), tokenFuture);
14021402
}
1403-
String token = tokenFuture.get().v1();
1403+
String token = tokenFuture.get().v1().v1();
14041404
mockGetTokenFromId(tokenService, userTokenId, expected, true, client);
14051405
doAnswer(invocationOnMock -> {
14061406
((Runnable) invocationOnMock.getArguments()[1]).run();

0 commit comments

Comments
 (0)