Skip to content

Commit 45140ef

Browse files
jaymodejaymode
authored
Use consistent view of realms for authentication (#38815)
This change updates the authentication service to use a consistent view of the realms based on the license state at the start of authentication. Without this, the license can change during authentication of a request and it will result in a failure if the realm that extracted the token is no longer in the realm list. This manifests in some tests as an authentication failure that should never really happen; one example would be the test framework's transport client user should always have a succesful authentication but in the LicensingTests this can fail and will show up as a NoNodeAvailableException. Additionally, the licensing tests have been updated to ensure that there is consistency when changing the license. The license is changed by modifying the internal xpack license state on each node, which has no protection against be changed by some pending cluster action. The methods to disable and enable now ensure we have a green cluster and that the cluster is consistent before returning. Closes #30301
1 parent c8224e3 commit 45140ef

File tree

9 files changed

+145
-73
lines changed

9 files changed

+145
-73
lines changed

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,15 @@ it to the action without an associated user (not via REST or transport - this is
152152
*/
153153
final String securityAction = actionMapper.action(action, request);
154154
authcService.authenticate(securityAction, request, SystemUser.INSTANCE,
155-
ActionListener.wrap((authc) -> authorizeRequest(authc, securityAction, request, listener), listener::onFailure));
155+
ActionListener.wrap((authc) -> {
156+
if (authc != null) {
157+
authorizeRequest(authc, securityAction, request, listener);
158+
} else if (licenseState.isAuthAllowed() == false) {
159+
listener.onResponse(null);
160+
} else {
161+
listener.onFailure(new IllegalStateException("no authentication present but auth is allowed"));
162+
}
163+
}, listener::onFailure));
156164
}
157165

158166
private <Request extends ActionRequest> void authorizeRequest(Authentication authentication, String securityAction, Request request,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ protected void doExecute(Task task, SamlAuthenticateRequest request, ActionListe
5959
listener.onFailure(new IllegalStateException("Cannot find AuthenticationResult on thread context"));
6060
return;
6161
}
62+
assert authentication != null : "authentication should never be null at this point";
6263
final Map<String, Object> tokenMeta = (Map<String, Object>) result.getMetadata().get(SamlRealm.CONTEXT_TOKEN_DATA);
6364
tokenService.createUserToken(authentication, originatingAuthentication,
6465
ActionListener.wrap(tuple -> {

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,11 @@ private void authenticateAndCreateToken(CreateTokenRequest request, ActionListen
7272
authenticationService.authenticate(CreateTokenAction.NAME, request, authToken,
7373
ActionListener.wrap(authentication -> {
7474
request.getPassword().close();
75-
createToken(request, authentication, originatingAuthentication, true, listener);
75+
if (authentication != null) {
76+
createToken(request, authentication, originatingAuthentication, true, listener);
77+
} else {
78+
listener.onFailure(new UnsupportedOperationException("cannot create token if authentication is not allowed"));
79+
}
7680
}, e -> {
7781
// clear the request password
7882
request.getPassword().close();

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

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,9 @@ class Authenticator {
196196

197197
private final AuditableRequest request;
198198
private final User fallbackUser;
199-
199+
private final List<Realm> defaultOrderedRealmList;
200200
private final ActionListener<Authentication> listener;
201+
201202
private RealmRef authenticatedBy = null;
202203
private RealmRef lookedupBy = null;
203204
private AuthenticationToken authenticationToken = null;
@@ -215,6 +216,7 @@ class Authenticator {
215216
private Authenticator(AuditableRequest auditableRequest, User fallbackUser, ActionListener<Authentication> listener) {
216217
this.request = auditableRequest;
217218
this.fallbackUser = fallbackUser;
219+
this.defaultOrderedRealmList = realms.asList();
218220
this.listener = listener;
219221
}
220222

@@ -233,27 +235,33 @@ private Authenticator(AuditableRequest auditableRequest, User fallbackUser, Acti
233235
* </ol>
234236
*/
235237
private void authenticateAsync() {
236-
lookForExistingAuthentication((authentication) -> {
237-
if (authentication != null) {
238-
listener.onResponse(authentication);
239-
} else {
240-
tokenService.getAndValidateToken(threadContext, ActionListener.wrap(userToken -> {
241-
if (userToken != null) {
242-
writeAuthToContext(userToken.getAuthentication());
243-
} else {
244-
checkForApiKey();
245-
}
246-
}, e -> {
247-
if (e instanceof ElasticsearchSecurityException &&
238+
if (defaultOrderedRealmList.isEmpty()) {
239+
// this happens when the license state changes between the call to authenticate and the actual invocation
240+
// to get the realm list
241+
listener.onResponse(null);
242+
} else {
243+
lookForExistingAuthentication((authentication) -> {
244+
if (authentication != null) {
245+
listener.onResponse(authentication);
246+
} else {
247+
tokenService.getAndValidateToken(threadContext, ActionListener.wrap(userToken -> {
248+
if (userToken != null) {
249+
writeAuthToContext(userToken.getAuthentication());
250+
} else {
251+
checkForApiKey();
252+
}
253+
}, e -> {
254+
if (e instanceof ElasticsearchSecurityException &&
248255
tokenService.isExpiredTokenException((ElasticsearchSecurityException) e) == false) {
249-
// intentionally ignore the returned exception; we call this primarily
250-
// for the auditing as we already have a purpose built exception
251-
request.tamperedRequest();
252-
}
253-
listener.onFailure(e);
254-
}));
255-
}
256-
});
256+
// intentionally ignore the returned exception; we call this primarily
257+
// for the auditing as we already have a purpose built exception
258+
request.tamperedRequest();
259+
}
260+
listener.onFailure(e);
261+
}));
262+
}
263+
});
264+
}
257265
}
258266

259267
private void checkForApiKey() {
@@ -320,7 +328,7 @@ void extractToken(Consumer<AuthenticationToken> consumer) {
320328
if (authenticationToken != null) {
321329
action = () -> consumer.accept(authenticationToken);
322330
} else {
323-
for (Realm realm : realms) {
331+
for (Realm realm : defaultOrderedRealmList) {
324332
final AuthenticationToken token = realm.token(threadContext);
325333
if (token != null) {
326334
action = () -> consumer.accept(token);
@@ -388,6 +396,7 @@ private void consumeToken(AuthenticationToken token) {
388396
userListener.onResponse(null);
389397
}
390398
};
399+
391400
final IteratingActionListener<User, Realm> authenticatingListener =
392401
new IteratingActionListener<>(ContextPreservingActionListener.wrapPreservingContext(ActionListener.wrap(
393402
(user) -> consumeUser(user, messages),
@@ -402,24 +411,24 @@ private void consumeToken(AuthenticationToken token) {
402411
}
403412

404413
private List<Realm> getRealmList(String principal) {
405-
final List<Realm> defaultOrderedRealms = realms.asList();
414+
final List<Realm> orderedRealmList = this.defaultOrderedRealmList;
406415
if (lastSuccessfulAuthCache != null) {
407416
final Realm lastSuccess = lastSuccessfulAuthCache.get(principal);
408417
if (lastSuccess != null) {
409-
final int index = defaultOrderedRealms.indexOf(lastSuccess);
418+
final int index = orderedRealmList.indexOf(lastSuccess);
410419
if (index > 0) {
411-
final List<Realm> smartOrder = new ArrayList<>(defaultOrderedRealms.size());
420+
final List<Realm> smartOrder = new ArrayList<>(orderedRealmList.size());
412421
smartOrder.add(lastSuccess);
413-
for (int i = 1; i < defaultOrderedRealms.size(); i++) {
422+
for (int i = 1; i < orderedRealmList.size(); i++) {
414423
if (i != index) {
415-
smartOrder.add(defaultOrderedRealms.get(i));
424+
smartOrder.add(orderedRealmList.get(i));
416425
}
417426
}
418427
return Collections.unmodifiableList(smartOrder);
419428
}
420429
}
421430
}
422-
return defaultOrderedRealms;
431+
return orderedRealmList;
423432
}
424433

425434
/**

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,12 +188,12 @@ protected Map<String, ServerTransportFilter> initializeProfileFilters(Destructiv
188188
case "client":
189189
profileFilters.put(entry.getKey(), new ServerTransportFilter.ClientProfile(authcService, authzService,
190190
threadPool.getThreadContext(), extractClientCert, destructiveOperations, reservedRealmEnabled,
191-
securityContext));
191+
securityContext, licenseState));
192192
break;
193193
case "node":
194194
profileFilters.put(entry.getKey(), new ServerTransportFilter.NodeProfile(authcService, authzService,
195195
threadPool.getThreadContext(), extractClientCert, destructiveOperations, reservedRealmEnabled,
196-
securityContext));
196+
securityContext, licenseState));
197197
break;
198198
default:
199199
throw new IllegalStateException("unknown profile type: " + type);

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/ServerTransportFilter.java‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.elasticsearch.action.admin.indices.open.OpenIndexAction;
1616
import org.elasticsearch.action.support.DestructiveOperations;
1717
import org.elasticsearch.common.util.concurrent.ThreadContext;
18+
import org.elasticsearch.license.XPackLicenseState;
1819
import org.elasticsearch.transport.TaskTransportChannel;
1920
import org.elasticsearch.transport.TcpChannel;
2021
import org.elasticsearch.transport.TcpTransportChannel;
@@ -66,17 +67,19 @@ class NodeProfile implements ServerTransportFilter {
6667
private final DestructiveOperations destructiveOperations;
6768
private final boolean reservedRealmEnabled;
6869
private final SecurityContext securityContext;
70+
private final XPackLicenseState licenseState;
6971

7072
NodeProfile(AuthenticationService authcService, AuthorizationService authzService,
7173
ThreadContext threadContext, boolean extractClientCert, DestructiveOperations destructiveOperations,
72-
boolean reservedRealmEnabled, SecurityContext securityContext) {
74+
boolean reservedRealmEnabled, SecurityContext securityContext, XPackLicenseState licenseState) {
7375
this.authcService = authcService;
7476
this.authzService = authzService;
7577
this.threadContext = threadContext;
7678
this.extractClientCert = extractClientCert;
7779
this.destructiveOperations = destructiveOperations;
7880
this.reservedRealmEnabled = reservedRealmEnabled;
7981
this.securityContext = securityContext;
82+
this.licenseState = licenseState;
8083
}
8184

8285
@Override
@@ -116,14 +119,20 @@ requests from all the nodes are attached with a user (either a serialize
116119

117120
final Version version = transportChannel.getVersion();
118121
authcService.authenticate(securityAction, request, (User)null, ActionListener.wrap((authentication) -> {
119-
if (securityAction.equals(TransportService.HANDSHAKE_ACTION_NAME) &&
120-
SystemUser.is(authentication.getUser()) == false) {
121-
securityContext.executeAsUser(SystemUser.INSTANCE, (ctx) -> {
122-
final Authentication replaced = Authentication.getAuthentication(threadContext);
123-
authzService.authorize(replaced, securityAction, request, listener);
124-
}, version);
122+
if (authentication != null) {
123+
if (securityAction.equals(TransportService.HANDSHAKE_ACTION_NAME) &&
124+
SystemUser.is(authentication.getUser()) == false) {
125+
securityContext.executeAsUser(SystemUser.INSTANCE, (ctx) -> {
126+
final Authentication replaced = Authentication.getAuthentication(threadContext);
127+
authzService.authorize(replaced, securityAction, request, listener);
128+
}, version);
129+
} else {
130+
authzService.authorize(authentication, securityAction, request, listener);
131+
}
132+
} else if (licenseState.isAuthAllowed() == false) {
133+
listener.onResponse(null);
125134
} else {
126-
authzService.authorize(authentication, securityAction, request, listener);
135+
listener.onFailure(new IllegalStateException("no authentication present but auth is allowed"));
127136
}
128137
}, listener::onFailure));
129138
}
@@ -139,9 +148,9 @@ class ClientProfile extends NodeProfile {
139148

140149
ClientProfile(AuthenticationService authcService, AuthorizationService authzService,
141150
ThreadContext threadContext, boolean extractClientCert, DestructiveOperations destructiveOperations,
142-
boolean reservedRealmEnabled, SecurityContext securityContext) {
151+
boolean reservedRealmEnabled, SecurityContext securityContext, XPackLicenseState licenseState) {
143152
super(authcService, authzService, threadContext, extractClientCert, destructiveOperations, reservedRealmEnabled,
144-
securityContext);
153+
securityContext, licenseState);
145154
}
146155

147156
@Override

0 commit comments

Comments
 (0)