Skip to content

Commit baf51f7

Browse files
Fixing a problem with potential dirty read of a token document on token refresh (#64031) (#64178)
* Fixing a problem with potential dirty read of a token document on token refresh (#64031) * Fixing a problem with potential dirty read of a token document. Related to #59685 * Fixing a problem with potential dirty read of a token document. Adding CreateTokenResult to hold authentication object * Fixing a problem with potential dirty read of a token document. Adding CreateTokenResult to hold authentication object Co-authored-by: Elastic Machine <[email protected]> * Fixing tests failures Co-authored-by: Elastic Machine <[email protected]>
1 parent 0a9692f commit baf51f7

12 files changed

+116
-80
lines changed

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,10 @@ protected void doExecute(Task task, DelegatePkiAuthenticationRequest request,
8282
ActionListener.wrap(authentication -> {
8383
assert authentication != null : "authentication should never be null at this point";
8484
tokenService.createOAuth2Tokens(authentication, delegateeAuthentication, Collections.emptyMap(), false,
85-
ActionListener.wrap(tuple -> {
85+
ActionListener.wrap(tokenResult -> {
8686
final TimeValue expiresIn = tokenService.getExpirationDelay();
87-
listener.onResponse(new DelegatePkiAuthenticationResponse(tuple.v1(), expiresIn, authentication));
87+
listener.onResponse(new DelegatePkiAuthenticationResponse(tokenResult.getAccessToken(), expiresIn,
88+
authentication));
8889
}, listener::onFailure));
8990
}, e -> {
9091
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

+3-2
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,10 @@ protected void doExecute(Task task, OpenIdConnectAuthenticateRequest request,
7272
@SuppressWarnings("unchecked") final Map<String, Object> tokenMetadata = (Map<String, Object>) result.getMetadata()
7373
.get(OpenIdConnectRealm.CONTEXT_TOKEN_DATA);
7474
tokenService.createOAuth2Tokens(authentication, originatingAuthentication, tokenMetadata, true,
75-
ActionListener.wrap(tuple -> {
75+
ActionListener.wrap(tokenResult -> {
7676
final TimeValue expiresIn = tokenService.getExpirationDelay();
77-
listener.onResponse(new OpenIdConnectAuthenticateResponse(authentication, tuple.v1(), tuple.v2(), expiresIn));
77+
listener.onResponse(new OpenIdConnectAuthenticateResponse(authentication, tokenResult.getAccessToken(),
78+
tokenResult.getRefreshToken(), 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

+3-2
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,11 @@ protected void doExecute(Task task, SamlAuthenticateRequest request, ActionListe
6565
assert authentication != null : "authentication should never be null at this point";
6666
final Map<String, Object> tokenMeta = (Map<String, Object>) result.getMetadata().get(SamlRealm.CONTEXT_TOKEN_DATA);
6767
tokenService.createOAuth2Tokens(authentication, originatingAuthentication,
68-
tokenMeta, true, ActionListener.wrap(tuple -> {
68+
tokenMeta, true, ActionListener.wrap(tokenResult -> {
6969
final TimeValue expiresIn = tokenService.getExpirationDelay();
7070
listener.onResponse(
71-
new SamlAuthenticateResponse(authentication, tuple.v1(), tuple.v2(), expiresIn));
71+
new SamlAuthenticateResponse(authentication, tokenResult.getAccessToken(), tokenResult.getRefreshToken(),
72+
expiresIn));
7273
}, listener::onFailure));
7374
}, e -> {
7475
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

+4-3
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,12 @@ private void clearCredentialsFromRequest(GrantType grantType, CreateTokenRequest
131131
private void createToken(GrantType grantType, CreateTokenRequest request, Authentication authentication, Authentication originatingAuth,
132132
boolean includeRefreshToken, ActionListener<CreateTokenResponse> listener) {
133133
tokenService.createOAuth2Tokens(authentication, originatingAuth, Collections.emptyMap(), includeRefreshToken,
134-
ActionListener.wrap(tuple -> {
134+
ActionListener.wrap(tokenResult -> {
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(tokenResult.getAccessToken(),
138+
tokenService.getExpirationDelay(), scope, tokenResult.getRefreshToken(), base64AuthenticateResponse,
139+
authentication);
139140
listener.onResponse(response);
140141
}, listener::onFailure));
141142
}

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

+5-7
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;
@@ -31,13 +30,12 @@ public TransportRefreshTokenAction(TransportService transportService, ActionFilt
3130

3231
@Override
3332
protected void doExecute(Task task, CreateTokenRequest request, ActionListener<CreateTokenResponse> listener) {
34-
tokenService.refreshToken(request.getRefreshToken(), ActionListener.wrap(tuple -> {
33+
tokenService.refreshToken(request.getRefreshToken(), ActionListener.wrap(tokenResult -> {
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(tokenResult.getAccessToken(), tokenService.getExpirationDelay(), scope,
37+
tokenResult.getRefreshToken(), null, tokenResult.getAuthentication());
38+
listener.onResponse(response);
4139
}, listener::onFailure));
4240
}
4341
}

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

+50-17
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ public TokenService(Settings settings, Clock clock, Client client, XPackLicenseS
255255
* {@link #VERSION_TOKENS_INDEX_INTRODUCED} and to a specific security tokens index for later versions.
256256
*/
257257
public void createOAuth2Tokens(Authentication authentication, Authentication originatingClientAuth, Map<String, Object> metadata,
258-
boolean includeRefreshToken, ActionListener<Tuple<String, String>> listener) {
258+
boolean includeRefreshToken, ActionListener<CreateTokenResult> listener) {
259259
// the created token is compatible with the oldest node version in the cluster
260260
final Version tokenVersion = getTokenVersionCompatibility();
261261
// tokens moved to a separate index in newer versions
@@ -274,7 +274,7 @@ public void createOAuth2Tokens(Authentication authentication, Authentication ori
274274
//public for testing
275275
public void createOAuth2Tokens(String accessToken, String refreshToken, Authentication authentication,
276276
Authentication originatingClientAuth,
277-
Map<String, Object> metadata, ActionListener<Tuple<String, String>> listener) {
277+
Map<String, Object> metadata, ActionListener<CreateTokenResult> listener) {
278278
// the created token is compatible with the oldest node version in the cluster
279279
final Version tokenVersion = getTokenVersionCompatibility();
280280
// tokens moved to a separate index in newer versions
@@ -307,12 +307,13 @@ public void createOAuth2Tokens(String accessToken, String refreshToken, Authenti
307307
* @param authentication The authentication object representing the user for which the tokens are created
308308
* @param originatingClientAuth The authentication object representing the client that called the related API
309309
* @param metadata A map with metadata to be stored in the token document
310-
* @param listener The listener to call upon completion with a {@link Tuple} containing the
311-
* serialized access token and serialized refresh token as these will be returned to the client
310+
* @param listener The listener to call upon completion with a {@link CreateTokenResult} containing the
311+
* serialized access token, serialized refresh token and authentication for which the token is created
312+
* as these will be returned to the client
312313
*/
313314
private void createOAuth2Tokens(String accessToken, String refreshToken, Version tokenVersion, SecurityIndexManager tokensIndex,
314315
Authentication authentication, Authentication originatingClientAuth, Map<String, Object> metadata,
315-
ActionListener<Tuple<String, String>> listener) {
316+
ActionListener<CreateTokenResult> listener) {
316317
assert accessToken.length() == TOKEN_LENGTH : "We assume token ids have a fixed length for nodes of a certain version."
317318
+ " When changing the token length, be careful that the inferences about its length still hold.";
318319
ensureEnabled();
@@ -352,12 +353,13 @@ private void createOAuth2Tokens(String accessToken, String refreshToken, Version
352353
final String versionedRefreshToken = refreshToken != null
353354
? prependVersionAndEncodeRefreshToken(tokenVersion, refreshToken)
354355
: null;
355-
listener.onResponse(new Tuple<>(versionedAccessToken, versionedRefreshToken));
356+
listener.onResponse(new CreateTokenResult(versionedAccessToken, versionedRefreshToken,
357+
authentication));
356358
} else {
357359
// prior versions of the refresh token are not version-prepended, as nodes on those
358360
// versions don't expect it.
359361
// Such nodes might exist in a mixed cluster during a rolling upgrade.
360-
listener.onResponse(new Tuple<>(versionedAccessToken, refreshToken));
362+
listener.onResponse(new CreateTokenResult(versionedAccessToken, refreshToken,authentication));
361363
}
362364
} else {
363365
listener.onFailure(traceLog("create token",
@@ -860,10 +862,11 @@ private void indexInvalidation(Collection<String> tokenIds, SecurityIndexManager
860862
* Called by the transport action in order to start the process of refreshing a token.
861863
*
862864
* @param refreshToken The refresh token as provided by the client
863-
* @param listener The listener to call upon completion with a {@link Tuple} containing the
864-
* serialized access token and serialized refresh token as these will be returned to the client
865+
* @param listener The listener to call upon completion with a {@link CreateTokenResult} containing the
866+
* serialized access token, serialized refresh token and authentication for which the token is created
867+
* as these will be returned to the client
865868
*/
866-
public void refreshToken(String refreshToken, ActionListener<Tuple<String, String>> listener) {
869+
public void refreshToken(String refreshToken, ActionListener<CreateTokenResult> listener) {
867870
ensureEnabled();
868871
final Instant refreshRequested = clock.instant();
869872
final Iterator<TimeValue> backoff = DEFAULT_BACKOFF.iterator();
@@ -996,7 +999,7 @@ private void findTokenFromRefreshToken(String refreshToken, SecurityIndexManager
996999
*/
9971000
private void innerRefresh(String refreshToken, String tokenDocId, Map<String, Object> source, long seqNo, long primaryTerm,
9981001
Authentication clientAuth, Iterator<TimeValue> backoff, Instant refreshRequested,
999-
ActionListener<Tuple<String, String>> listener) {
1002+
ActionListener<CreateTokenResult> listener) {
10001003
logger.debug("Attempting to refresh token stored in token document [{}]", tokenDocId);
10011004
final Consumer<Exception> onFailure = ex -> listener.onFailure(traceLog("refresh token", tokenDocId, ex));
10021005
final Tuple<RefreshTokenStatus, Optional<ElasticsearchSecurityException>> checkRefreshResult;
@@ -1015,7 +1018,9 @@ private void innerRefresh(String refreshToken, String tokenDocId, Map<String, Ob
10151018
if (refreshTokenStatus.isRefreshed()) {
10161019
logger.debug("Token document [{}] was recently refreshed, when a new token document was generated. Reusing that result.",
10171020
tokenDocId);
1018-
decryptAndReturnSupersedingTokens(refreshToken, refreshTokenStatus, refreshedTokenIndex, listener);
1021+
final Tuple<UserToken, String> parsedTokens = parseTokensFromDocument(source, null);
1022+
Authentication authentication = parsedTokens.v1().getAuthentication();
1023+
decryptAndReturnSupersedingTokens(refreshToken, refreshTokenStatus, refreshedTokenIndex, authentication, listener);
10191024
} else {
10201025
final String newAccessTokenString = UUIDs.randomBase64UUID();
10211026
final String newRefreshTokenString = UUIDs.randomBase64UUID();
@@ -1127,11 +1132,13 @@ public void onFailure(Exception e) {
11271132
* @param refreshTokenStatus The {@link RefreshTokenStatus} containing information about the superseding tokens as retrieved from the
11281133
* index
11291134
* @param tokensIndex the manager for the index where the tokens are stored
1130-
* @param listener The listener to call upon completion with a {@link Tuple} containing the
1131-
* serialized access token and serialized refresh token as these will be returned to the client
1135+
* @param authentication The authentication object representing the user for which the tokens are created
1136+
* @param listener The listener to call upon completion with a {@link CreateTokenResult} containing the
1137+
* serialized access token, serialized refresh token and authentication for which the token is created
1138+
* as these will be returned to the client
11321139
*/
11331140
void decryptAndReturnSupersedingTokens(String refreshToken, RefreshTokenStatus refreshTokenStatus, SecurityIndexManager tokensIndex,
1134-
ActionListener<Tuple<String, String>> listener) {
1141+
Authentication authentication, ActionListener<CreateTokenResult> listener) {
11351142

11361143
final byte[] iv = Base64.getDecoder().decode(refreshTokenStatus.getIv());
11371144
final byte[] salt = Base64.getDecoder().decode(refreshTokenStatus.getSalt());
@@ -1167,8 +1174,10 @@ public void onResponse(GetResponse response) {
11671174
if (response.isExists()) {
11681175
try {
11691176
listener.onResponse(
1170-
new Tuple<>(prependVersionAndEncodeAccessToken(refreshTokenStatus.getVersion(), decryptedTokens[0]),
1171-
prependVersionAndEncodeRefreshToken(refreshTokenStatus.getVersion(), decryptedTokens[1])));
1177+
new CreateTokenResult(prependVersionAndEncodeAccessToken(refreshTokenStatus.getVersion(),
1178+
decryptedTokens[0]),
1179+
prependVersionAndEncodeRefreshToken(refreshTokenStatus.getVersion(), decryptedTokens[1]),
1180+
authentication));
11721181
} catch (GeneralSecurityException | IOException e) {
11731182
logger.warn("Could not format stored superseding token values", e);
11741183
onFailure.accept(invalidGrantException("could not refresh the requested token"));
@@ -1911,6 +1920,30 @@ boolean isExpirationInProgress() {
19111920
return expiredTokenRemover.isExpirationInProgress();
19121921
}
19131922

1923+
public static final class CreateTokenResult {
1924+
private final String accessToken;
1925+
private final String refreshToken;
1926+
private final Authentication authentication;
1927+
1928+
public CreateTokenResult(String accessToken, String refreshToken, Authentication authentication) {
1929+
this.accessToken = accessToken;
1930+
this.refreshToken = refreshToken;
1931+
this.authentication = authentication;
1932+
}
1933+
1934+
public String getAccessToken() {
1935+
return accessToken;
1936+
}
1937+
1938+
public String getRefreshToken() {
1939+
return refreshToken;
1940+
}
1941+
1942+
public Authentication getAuthentication() {
1943+
return authentication;
1944+
}
1945+
}
1946+
19141947
private class KeyComputingRunnable extends AbstractRunnable {
19151948

19161949
private final BytesKey decodedSalt;

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

+2-3
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.elasticsearch.client.Client;
2727
import org.elasticsearch.cluster.service.ClusterService;
2828
import org.elasticsearch.common.UUIDs;
29-
import org.elasticsearch.common.collect.Tuple;
3029
import org.elasticsearch.common.settings.Settings;
3130
import org.elasticsearch.common.util.concurrent.ThreadContext;
3231
import org.elasticsearch.env.Environment;
@@ -210,11 +209,11 @@ public void testLogoutInvalidatesTokens() throws Exception {
210209
final Authentication authentication = new Authentication(user, realmRef, null, null, Authentication.AuthenticationType.REALM,
211210
tokenMetadata);
212211

213-
final PlainActionFuture<Tuple<String, String>> future = new PlainActionFuture<>();
212+
final PlainActionFuture<TokenService.CreateTokenResult> future = new PlainActionFuture<>();
214213
final String userTokenId = UUIDs.randomBase64UUID();
215214
final String refreshToken = UUIDs.randomBase64UUID();
216215
tokenService.createOAuth2Tokens(userTokenId, refreshToken, authentication, authentication, tokenMetadata, future);
217-
final String accessToken = future.actionGet().v1();
216+
final String accessToken = future.actionGet().getAccessToken();
218217
mockGetTokenFromId(tokenService, userTokenId, authentication, false, client);
219218

220219
final OpenIdConnectLogoutRequest request = new OpenIdConnectLogoutRequest();

0 commit comments

Comments
 (0)