Skip to content

Commit 45ddd69

Browse files
authored
Prevent Kerberos and PKI providers from initiating a new session for unauthenticated XHR/API requests. (#82817)
* Prevent Kerberos and PKI providers from initiating a new session for unauthenticated XHR requests. * Review#1: fix comment.
1 parent 55cf3bd commit 45ddd69

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+271
-290
lines changed

.github/CODEOWNERS

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,9 +254,6 @@
254254
/x-pack/test/ui_capabilities/ @elastic/kibana-security
255255
/x-pack/test/encrypted_saved_objects_api_integration/ @elastic/kibana-security
256256
/x-pack/test/functional/apps/security/ @elastic/kibana-security
257-
/x-pack/test/kerberos_api_integration/ @elastic/kibana-security
258-
/x-pack/test/oidc_api_integration/ @elastic/kibana-security
259-
/x-pack/test/pki_api_integration/ @elastic/kibana-security
260257
/x-pack/test/security_api_integration/ @elastic/kibana-security
261258
/x-pack/test/security_functional/ @elastic/kibana-security
262259
/x-pack/test/spaces_api_integration/ @elastic/kibana-security

x-pack/plugins/security/server/authentication/providers/kerberos.test.ts

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,16 @@ describe('KerberosAuthenticationProvider', () => {
346346
expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
347347
});
348348

349+
it('does not start SPNEGO for Ajax requests.', async () => {
350+
const request = httpServerMock.createKibanaRequest({ headers: { 'kbn-xsrf': 'xsrf' } });
351+
await expect(provider.authenticate(request)).resolves.toEqual(
352+
AuthenticationResult.notHandled()
353+
);
354+
355+
expect(mockOptions.client.asScoped).not.toHaveBeenCalled();
356+
expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
357+
});
358+
349359
it('succeeds if state contains a valid token.', async () => {
350360
const user = mockAuthenticatedUser();
351361
const request = httpServerMock.createKibanaRequest({ headers: {} });
@@ -442,9 +452,6 @@ describe('KerberosAuthenticationProvider', () => {
442452
});
443453

444454
it('fails with `Negotiate` challenge if both access and refresh tokens from the state are expired and backend supports Kerberos.', async () => {
445-
const request = httpServerMock.createKibanaRequest();
446-
const tokenPair = { accessToken: 'expired-token', refreshToken: 'some-valid-refresh-token' };
447-
448455
const failureReason = LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(
449456
new (errors.AuthenticationException as any)('Unauthorized', {
450457
body: { error: { header: { 'WWW-Authenticate': 'Negotiate' } } },
@@ -456,37 +463,45 @@ describe('KerberosAuthenticationProvider', () => {
456463

457464
mockOptions.tokens.refresh.mockResolvedValue(null);
458465

459-
await expect(provider.authenticate(request, tokenPair)).resolves.toEqual(
466+
const nonAjaxRequest = httpServerMock.createKibanaRequest();
467+
const nonAjaxTokenPair = {
468+
accessToken: 'expired-token',
469+
refreshToken: 'some-valid-refresh-token',
470+
};
471+
await expect(provider.authenticate(nonAjaxRequest, nonAjaxTokenPair)).resolves.toEqual(
460472
AuthenticationResult.failed(failureReason, {
461473
authResponseHeaders: { 'WWW-Authenticate': 'Negotiate' },
462474
})
463475
);
464476

465-
expect(mockOptions.tokens.refresh).toHaveBeenCalledTimes(1);
466-
expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(tokenPair.refreshToken);
467-
});
468-
469-
it('does not re-start SPNEGO if both access and refresh tokens from the state are expired.', async () => {
470-
const request = httpServerMock.createKibanaRequest({ routeAuthRequired: false });
471-
const tokenPair = { accessToken: 'expired-token', refreshToken: 'some-valid-refresh-token' };
472-
473-
const failureReason = LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(
474-
new (errors.AuthenticationException as any)('Unauthorized', {
475-
body: { error: { header: { 'WWW-Authenticate': 'Negotiate' } } },
477+
const ajaxRequest = httpServerMock.createKibanaRequest({ headers: { 'kbn-xsrf': 'xsrf' } });
478+
const ajaxTokenPair = {
479+
accessToken: 'expired-token',
480+
refreshToken: 'ajax-some-valid-refresh-token',
481+
};
482+
await expect(provider.authenticate(ajaxRequest, ajaxTokenPair)).resolves.toEqual(
483+
AuthenticationResult.failed(failureReason, {
484+
authResponseHeaders: { 'WWW-Authenticate': 'Negotiate' },
476485
})
477486
);
478-
const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient();
479-
mockScopedClusterClient.callAsCurrentUser.mockRejectedValue(failureReason);
480-
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
481487

482-
mockOptions.tokens.refresh.mockResolvedValue(null);
483-
484-
await expect(provider.authenticate(request, tokenPair)).resolves.toEqual(
485-
AuthenticationResult.notHandled()
488+
const optionalAuthRequest = httpServerMock.createKibanaRequest({ routeAuthRequired: false });
489+
const optionalAuthTokenPair = {
490+
accessToken: 'expired-token',
491+
refreshToken: 'optional-some-valid-refresh-token',
492+
};
493+
await expect(
494+
provider.authenticate(optionalAuthRequest, optionalAuthTokenPair)
495+
).resolves.toEqual(
496+
AuthenticationResult.failed(failureReason, {
497+
authResponseHeaders: { 'WWW-Authenticate': 'Negotiate' },
498+
})
486499
);
487500

488-
expect(mockOptions.tokens.refresh).toHaveBeenCalledTimes(1);
489-
expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(tokenPair.refreshToken);
501+
expect(mockOptions.tokens.refresh).toHaveBeenCalledTimes(3);
502+
expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(nonAjaxTokenPair.refreshToken);
503+
expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(ajaxTokenPair.refreshToken);
504+
expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(optionalAuthTokenPair.refreshToken);
490505
});
491506
});
492507

x-pack/plugins/security/server/authentication/providers/kerberos.ts

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
import { AuthenticationResult } from '../authentication_result';
1414
import { DeauthenticationResult } from '../deauthentication_result';
1515
import { HTTPAuthorizationHeader } from '../http_authentication';
16+
import { canRedirectRequest } from '../can_redirect_request';
1617
import { Tokens, TokenPair } from '../tokens';
1718
import { BaseAuthenticationProvider } from './base';
1819

@@ -32,8 +33,9 @@ const WWWAuthenticateHeaderName = 'WWW-Authenticate';
3233
* @param request Request instance.
3334
*/
3435
function canStartNewSession(request: KibanaRequest) {
35-
// We should try to establish new session only if request requires authentication.
36-
return request.route.options.authRequired === true;
36+
// We should try to establish new session only if request requires authentication and it's not an XHR request.
37+
// Technically we can authenticate XHR requests too, but we don't want these to create a new session unintentionally.
38+
return canRedirectRequest(request) && request.route.options.authRequired === true;
3739
}
3840

3941
/**
@@ -75,11 +77,8 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider {
7577
return AuthenticationResult.notHandled();
7678
}
7779

78-
let authenticationResult = authorizationHeader
79-
? await this.authenticateWithNegotiateScheme(request)
80-
: AuthenticationResult.notHandled();
81-
82-
if (state && authenticationResult.notHandled()) {
80+
let authenticationResult = AuthenticationResult.notHandled();
81+
if (state) {
8382
authenticationResult = await this.authenticateViaState(request, state);
8483
if (
8584
authenticationResult.failed() &&
@@ -89,11 +88,15 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider {
8988
}
9089
}
9190

92-
// If we couldn't authenticate by means of all methods above, let's try to check if Elasticsearch can
93-
// start authentication mechanism negotiation, otherwise just return authentication result we have.
94-
return authenticationResult.notHandled() && canStartNewSession(request)
95-
? await this.authenticateViaSPNEGO(request, state)
96-
: authenticationResult;
91+
if (!authenticationResult.notHandled() || !canStartNewSession(request)) {
92+
return authenticationResult;
93+
}
94+
95+
// If we couldn't authenticate by means of all methods above, let's check if we're already at the authentication
96+
// mechanism negotiation stage, otherwise check with Elasticsearch if we can start it.
97+
return authorizationHeader
98+
? await this.authenticateWithNegotiateScheme(request)
99+
: await this.authenticateViaSPNEGO(request, state);
97100
}
98101

99102
/**
@@ -264,12 +267,12 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider {
264267
return AuthenticationResult.failed(err);
265268
}
266269

267-
// If refresh token is no longer valid, then we should clear session and renegotiate using SPNEGO.
270+
// If refresh token is no longer valid, let's try to renegotiate new tokens using SPNEGO. We
271+
// allow this because expired underlying token is an implementation detail and Kibana user
272+
// facing session is still valid.
268273
if (refreshedTokenPair === null) {
269-
this.logger.debug('Both access and refresh tokens are expired.');
270-
return canStartNewSession(request)
271-
? this.authenticateViaSPNEGO(request, state)
272-
: AuthenticationResult.notHandled();
274+
this.logger.debug('Both access and refresh tokens are expired. Re-authenticating...');
275+
return this.authenticateViaSPNEGO(request, state);
273276
}
274277

275278
try {

x-pack/plugins/security/server/authentication/providers/pki.test.ts

Lines changed: 97 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,22 @@ describe('PKIAuthenticationProvider', () => {
295295
expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
296296
});
297297

298+
it('does not exchange peer certificate to access token for Ajax requests.', async () => {
299+
const request = httpServerMock.createKibanaRequest({
300+
headers: { 'kbn-xsrf': 'xsrf' },
301+
socket: getMockSocket({
302+
authorized: true,
303+
peerCertificate: getMockPeerCertificate(['2A:7A:C2:DD', '3B:8B:D3:EE']),
304+
}),
305+
});
306+
await expect(provider.authenticate(request)).resolves.toEqual(
307+
AuthenticationResult.notHandled()
308+
);
309+
310+
expect(mockOptions.client.asScoped).not.toHaveBeenCalled();
311+
expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
312+
});
313+
298314
it('fails with non-401 error if state is available, peer is authorized, but certificate is not available.', async () => {
299315
const request = httpServerMock.createKibanaRequest({
300316
socket: getMockSocket({ authorized: true }),
@@ -383,14 +399,7 @@ describe('PKIAuthenticationProvider', () => {
383399
});
384400

385401
it('gets a new access token even if existing token is expired.', async () => {
386-
const user = mockAuthenticatedUser();
387-
const request = httpServerMock.createKibanaRequest({
388-
socket: getMockSocket({
389-
authorized: true,
390-
peerCertificate: getMockPeerCertificate(['2A:7A:C2:DD', '3B:8B:D3:EE']),
391-
}),
392-
});
393-
const state = { accessToken: 'existing-token', peerCertificateFingerprint256: '2A:7A:C2:DD' };
402+
const user = mockAuthenticatedUser({ authentication_provider: { type: 'pki', name: 'pki' } });
394403

395404
const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient();
396405
mockScopedClusterClient.callAsCurrentUser
@@ -399,55 +408,102 @@ describe('PKIAuthenticationProvider', () => {
399408
LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error())
400409
)
401410
// In response to a call with a new token.
411+
.mockResolvedValueOnce(user) // In response to call with an expired token.
412+
.mockRejectedValueOnce(
413+
LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error())
414+
)
415+
// In response to a call with a new token.
416+
.mockResolvedValueOnce(user) // In response to call with an expired token.
417+
.mockRejectedValueOnce(
418+
LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error())
419+
)
420+
// In response to a call with a new token.
402421
.mockResolvedValueOnce(user);
403422
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
404423
mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'access-token' });
405424

406-
await expect(provider.authenticate(request, state)).resolves.toEqual(
407-
AuthenticationResult.succeeded(
408-
{ ...user, authentication_provider: { type: 'pki', name: 'pki' } },
409-
{
410-
authHeaders: { authorization: 'Bearer access-token' },
411-
state: { accessToken: 'access-token', peerCertificateFingerprint256: '2A:7A:C2:DD' },
412-
}
413-
)
425+
const nonAjaxRequest = httpServerMock.createKibanaRequest({
426+
socket: getMockSocket({
427+
authorized: true,
428+
peerCertificate: getMockPeerCertificate(['2A:7A:C2:DD', '3B:8B:D3:EE']),
429+
}),
430+
});
431+
const nonAjaxState = {
432+
accessToken: 'existing-token',
433+
peerCertificateFingerprint256: '2A:7A:C2:DD',
434+
};
435+
await expect(provider.authenticate(nonAjaxRequest, nonAjaxState)).resolves.toEqual(
436+
AuthenticationResult.succeeded(user, {
437+
authHeaders: { authorization: 'Bearer access-token' },
438+
state: { accessToken: 'access-token', peerCertificateFingerprint256: '2A:7A:C2:DD' },
439+
})
414440
);
415441

416-
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1);
417-
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.delegatePKI', {
418-
body: {
419-
x509_certificate_chain: [
420-
'fingerprint:2A:7A:C2:DD:base64',
421-
'fingerprint:3B:8B:D3:EE:base64',
422-
],
423-
},
442+
const ajaxRequest = httpServerMock.createKibanaRequest({
443+
headers: { 'kbn-xsrf': 'xsrf' },
444+
socket: getMockSocket({
445+
authorized: true,
446+
peerCertificate: getMockPeerCertificate(['3A:7A:C2:DD', '3B:8B:D3:EE']),
447+
}),
424448
});
449+
const ajaxState = {
450+
accessToken: 'existing-token',
451+
peerCertificateFingerprint256: '3A:7A:C2:DD',
452+
};
453+
await expect(provider.authenticate(ajaxRequest, ajaxState)).resolves.toEqual(
454+
AuthenticationResult.succeeded(user, {
455+
authHeaders: { authorization: 'Bearer access-token' },
456+
state: { accessToken: 'access-token', peerCertificateFingerprint256: '3A:7A:C2:DD' },
457+
})
458+
);
425459

426-
expect(request.headers).not.toHaveProperty('authorization');
427-
});
428-
429-
it('does not exchange peer certificate to a new access token even if existing token is expired and request does not require authentication.', async () => {
430-
const request = httpServerMock.createKibanaRequest({
460+
const optionalAuthRequest = httpServerMock.createKibanaRequest({
431461
routeAuthRequired: false,
432462
socket: getMockSocket({
433463
authorized: true,
434-
peerCertificate: getMockPeerCertificate(['2A:7A:C2:DD', '3B:8B:D3:EE']),
464+
peerCertificate: getMockPeerCertificate(['4A:7A:C2:DD', '3B:8B:D3:EE']),
435465
}),
436466
});
437-
const state = { accessToken: 'existing-token', peerCertificateFingerprint256: '2A:7A:C2:DD' };
438-
439-
const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient();
440-
mockScopedClusterClient.callAsCurrentUser.mockRejectedValueOnce(
441-
LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error())
467+
const optionalAuthState = {
468+
accessToken: 'existing-token',
469+
peerCertificateFingerprint256: '4A:7A:C2:DD',
470+
};
471+
await expect(provider.authenticate(optionalAuthRequest, optionalAuthState)).resolves.toEqual(
472+
AuthenticationResult.succeeded(user, {
473+
authHeaders: { authorization: 'Bearer access-token' },
474+
state: { accessToken: 'access-token', peerCertificateFingerprint256: '4A:7A:C2:DD' },
475+
})
442476
);
443-
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
444477

445-
await expect(provider.authenticate(request, state)).resolves.toEqual(
446-
AuthenticationResult.notHandled()
447-
);
478+
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(3);
479+
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.delegatePKI', {
480+
body: {
481+
x509_certificate_chain: [
482+
'fingerprint:2A:7A:C2:DD:base64',
483+
'fingerprint:3B:8B:D3:EE:base64',
484+
],
485+
},
486+
});
487+
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.delegatePKI', {
488+
body: {
489+
x509_certificate_chain: [
490+
'fingerprint:3A:7A:C2:DD:base64',
491+
'fingerprint:3B:8B:D3:EE:base64',
492+
],
493+
},
494+
});
495+
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.delegatePKI', {
496+
body: {
497+
x509_certificate_chain: [
498+
'fingerprint:4A:7A:C2:DD:base64',
499+
'fingerprint:3B:8B:D3:EE:base64',
500+
],
501+
},
502+
});
448503

449-
expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
450-
expect(request.headers).not.toHaveProperty('authorization');
504+
expect(nonAjaxRequest.headers).not.toHaveProperty('authorization');
505+
expect(ajaxRequest.headers).not.toHaveProperty('authorization');
506+
expect(optionalAuthRequest.headers).not.toHaveProperty('authorization');
451507
});
452508

453509
it('fails with 401 if existing token is expired, but certificate is not present.', async () => {

0 commit comments

Comments
 (0)