Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent Kerberos and PKI providers from initiating a new session for unauthenticated XHR/API requests. #82817

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,6 @@
/x-pack/test/ui_capabilities/ @elastic/kibana-security
/x-pack/test/encrypted_saved_objects_api_integration/ @elastic/kibana-security
/x-pack/test/functional/apps/security/ @elastic/kibana-security
/x-pack/test/kerberos_api_integration/ @elastic/kibana-security
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: just continue moving the remaining tests whenever I have chance.

/x-pack/test/oidc_api_integration/ @elastic/kibana-security
/x-pack/test/pki_api_integration/ @elastic/kibana-security
/x-pack/test/security_api_integration/ @elastic/kibana-security
/x-pack/test/security_functional/ @elastic/kibana-security
/x-pack/test/spaces_api_integration/ @elastic/kibana-security
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,16 @@ describe('KerberosAuthenticationProvider', () => {
expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
});

it('does not start SPNEGO for Ajax requests.', async () => {
const request = httpServerMock.createKibanaRequest({ headers: { 'kbn-xsrf': 'xsrf' } });
await expect(provider.authenticate(request)).resolves.toEqual(
AuthenticationResult.notHandled()
);

expect(mockOptions.client.asScoped).not.toHaveBeenCalled();
expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
});

it('succeeds if state contains a valid token.', async () => {
const user = mockAuthenticatedUser();
const request = httpServerMock.createKibanaRequest({ headers: {} });
Expand Down Expand Up @@ -442,9 +452,6 @@ describe('KerberosAuthenticationProvider', () => {
});

it('fails with `Negotiate` challenge if both access and refresh tokens from the state are expired and backend supports Kerberos.', async () => {
const request = httpServerMock.createKibanaRequest();
const tokenPair = { accessToken: 'expired-token', refreshToken: 'some-valid-refresh-token' };

const failureReason = LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(
new (errors.AuthenticationException as any)('Unauthorized', {
body: { error: { header: { 'WWW-Authenticate': 'Negotiate' } } },
Expand All @@ -456,37 +463,45 @@ describe('KerberosAuthenticationProvider', () => {

mockOptions.tokens.refresh.mockResolvedValue(null);

await expect(provider.authenticate(request, tokenPair)).resolves.toEqual(
const nonAjaxRequest = httpServerMock.createKibanaRequest();
const nonAjaxTokenPair = {
accessToken: 'expired-token',
refreshToken: 'some-valid-refresh-token',
};
await expect(provider.authenticate(nonAjaxRequest, nonAjaxTokenPair)).resolves.toEqual(
AuthenticationResult.failed(failureReason, {
authResponseHeaders: { 'WWW-Authenticate': 'Negotiate' },
})
);

expect(mockOptions.tokens.refresh).toHaveBeenCalledTimes(1);
expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(tokenPair.refreshToken);
});

it('does not re-start SPNEGO if both access and refresh tokens from the state are expired.', async () => {
const request = httpServerMock.createKibanaRequest({ routeAuthRequired: false });
const tokenPair = { accessToken: 'expired-token', refreshToken: 'some-valid-refresh-token' };

const failureReason = LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(
new (errors.AuthenticationException as any)('Unauthorized', {
body: { error: { header: { 'WWW-Authenticate': 'Negotiate' } } },
const ajaxRequest = httpServerMock.createKibanaRequest({ headers: { 'kbn-xsrf': 'xsrf' } });
const ajaxTokenPair = {
accessToken: 'expired-token',
refreshToken: 'ajax-some-valid-refresh-token',
};
await expect(provider.authenticate(ajaxRequest, ajaxTokenPair)).resolves.toEqual(
AuthenticationResult.failed(failureReason, {
authResponseHeaders: { 'WWW-Authenticate': 'Negotiate' },
})
);
const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient();
mockScopedClusterClient.callAsCurrentUser.mockRejectedValue(failureReason);
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);

mockOptions.tokens.refresh.mockResolvedValue(null);

await expect(provider.authenticate(request, tokenPair)).resolves.toEqual(
AuthenticationResult.notHandled()
const optionalAuthRequest = httpServerMock.createKibanaRequest({ routeAuthRequired: false });
const optionalAuthTokenPair = {
accessToken: 'expired-token',
refreshToken: 'optional-some-valid-refresh-token',
};
await expect(
provider.authenticate(optionalAuthRequest, optionalAuthTokenPair)
).resolves.toEqual(
AuthenticationResult.failed(failureReason, {
authResponseHeaders: { 'WWW-Authenticate': 'Negotiate' },
})
);

expect(mockOptions.tokens.refresh).toHaveBeenCalledTimes(1);
expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(tokenPair.refreshToken);
expect(mockOptions.tokens.refresh).toHaveBeenCalledTimes(3);
expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(nonAjaxTokenPair.refreshToken);
expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(ajaxTokenPair.refreshToken);
expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(optionalAuthTokenPair.refreshToken);
});
});

Expand Down
37 changes: 20 additions & 17 deletions x-pack/plugins/security/server/authentication/providers/kerberos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import { AuthenticationResult } from '../authentication_result';
import { DeauthenticationResult } from '../deauthentication_result';
import { HTTPAuthorizationHeader } from '../http_authentication';
import { canRedirectRequest } from '../can_redirect_request';
import { Tokens, TokenPair } from '../tokens';
import { BaseAuthenticationProvider } from './base';

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

/**
Expand Down Expand Up @@ -75,11 +77,8 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider {
return AuthenticationResult.notHandled();
}

let authenticationResult = authorizationHeader
Copy link
Member Author

@azasypkin azasypkin Nov 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I'm not sure why we weren't checking the canStartNewSession here before, but I cannot find any reason why we shouldn't do it now. And we also should give a higher priority to the current session as well.

? await this.authenticateWithNegotiateScheme(request)
: AuthenticationResult.notHandled();

if (state && authenticationResult.notHandled()) {
let authenticationResult = AuthenticationResult.notHandled();
if (state) {
authenticationResult = await this.authenticateViaState(request, state);
if (
authenticationResult.failed() &&
Expand All @@ -89,11 +88,15 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider {
}
}

// If we couldn't authenticate by means of all methods above, let's try to check if Elasticsearch can
// start authentication mechanism negotiation, otherwise just return authentication result we have.
return authenticationResult.notHandled() && canStartNewSession(request)
? await this.authenticateViaSPNEGO(request, state)
: authenticationResult;
if (!authenticationResult.notHandled() || !canStartNewSession(request)) {
return authenticationResult;
}

// If we couldn't authenticate by means of all methods above, let's check if we're already at the authentication
// mechanism negotiation stage, otherwise check with Elasticsearch if we can start it.
return authorizationHeader
? await this.authenticateWithNegotiateScheme(request)
: await this.authenticateViaSPNEGO(request, state);
}

/**
Expand Down Expand Up @@ -264,12 +267,12 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider {
return AuthenticationResult.failed(err);
}

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

try {
Expand Down
138 changes: 97 additions & 41 deletions x-pack/plugins/security/server/authentication/providers/pki.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,22 @@ describe('PKIAuthenticationProvider', () => {
expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
});

it('does not exchange peer certificate to access token for Ajax requests.', async () => {
const request = httpServerMock.createKibanaRequest({
headers: { 'kbn-xsrf': 'xsrf' },
socket: getMockSocket({
authorized: true,
peerCertificate: getMockPeerCertificate(['2A:7A:C2:DD', '3B:8B:D3:EE']),
}),
});
await expect(provider.authenticate(request)).resolves.toEqual(
AuthenticationResult.notHandled()
);

expect(mockOptions.client.asScoped).not.toHaveBeenCalled();
expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
});

it('fails with non-401 error if state is available, peer is authorized, but certificate is not available.', async () => {
const request = httpServerMock.createKibanaRequest({
socket: getMockSocket({ authorized: true }),
Expand Down Expand Up @@ -383,14 +399,7 @@ describe('PKIAuthenticationProvider', () => {
});

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

const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient();
mockScopedClusterClient.callAsCurrentUser
Expand All @@ -399,55 +408,102 @@ describe('PKIAuthenticationProvider', () => {
LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error())
)
// In response to a call with a new token.
.mockResolvedValueOnce(user) // In response to call with an expired token.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I'll remove this stupid setup in the scope of #80952 soon.

.mockRejectedValueOnce(
LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error())
)
// In response to a call with a new token.
.mockResolvedValueOnce(user) // In response to call with an expired token.
.mockRejectedValueOnce(
LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error())
)
// In response to a call with a new token.
.mockResolvedValueOnce(user);
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);
mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'access-token' });

await expect(provider.authenticate(request, state)).resolves.toEqual(
AuthenticationResult.succeeded(
{ ...user, authentication_provider: { type: 'pki', name: 'pki' } },
{
authHeaders: { authorization: 'Bearer access-token' },
state: { accessToken: 'access-token', peerCertificateFingerprint256: '2A:7A:C2:DD' },
}
)
const nonAjaxRequest = httpServerMock.createKibanaRequest({
socket: getMockSocket({
authorized: true,
peerCertificate: getMockPeerCertificate(['2A:7A:C2:DD', '3B:8B:D3:EE']),
}),
});
const nonAjaxState = {
accessToken: 'existing-token',
peerCertificateFingerprint256: '2A:7A:C2:DD',
};
await expect(provider.authenticate(nonAjaxRequest, nonAjaxState)).resolves.toEqual(
AuthenticationResult.succeeded(user, {
authHeaders: { authorization: 'Bearer access-token' },
state: { accessToken: 'access-token', peerCertificateFingerprint256: '2A:7A:C2:DD' },
})
);

expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1);
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.delegatePKI', {
body: {
x509_certificate_chain: [
'fingerprint:2A:7A:C2:DD:base64',
'fingerprint:3B:8B:D3:EE:base64',
],
},
const ajaxRequest = httpServerMock.createKibanaRequest({
headers: { 'kbn-xsrf': 'xsrf' },
socket: getMockSocket({
authorized: true,
peerCertificate: getMockPeerCertificate(['3A:7A:C2:DD', '3B:8B:D3:EE']),
}),
});
const ajaxState = {
accessToken: 'existing-token',
peerCertificateFingerprint256: '3A:7A:C2:DD',
};
await expect(provider.authenticate(ajaxRequest, ajaxState)).resolves.toEqual(
AuthenticationResult.succeeded(user, {
authHeaders: { authorization: 'Bearer access-token' },
state: { accessToken: 'access-token', peerCertificateFingerprint256: '3A:7A:C2:DD' },
})
);

expect(request.headers).not.toHaveProperty('authorization');
});

it('does not exchange peer certificate to a new access token even if existing token is expired and request does not require authentication.', async () => {
const request = httpServerMock.createKibanaRequest({
const optionalAuthRequest = httpServerMock.createKibanaRequest({
routeAuthRequired: false,
socket: getMockSocket({
authorized: true,
peerCertificate: getMockPeerCertificate(['2A:7A:C2:DD', '3B:8B:D3:EE']),
peerCertificate: getMockPeerCertificate(['4A:7A:C2:DD', '3B:8B:D3:EE']),
}),
});
const state = { accessToken: 'existing-token', peerCertificateFingerprint256: '2A:7A:C2:DD' };

const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient();
mockScopedClusterClient.callAsCurrentUser.mockRejectedValueOnce(
LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error())
const optionalAuthState = {
accessToken: 'existing-token',
peerCertificateFingerprint256: '4A:7A:C2:DD',
};
await expect(provider.authenticate(optionalAuthRequest, optionalAuthState)).resolves.toEqual(
AuthenticationResult.succeeded(user, {
authHeaders: { authorization: 'Bearer access-token' },
state: { accessToken: 'access-token', peerCertificateFingerprint256: '4A:7A:C2:DD' },
})
);
mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient);

await expect(provider.authenticate(request, state)).resolves.toEqual(
AuthenticationResult.notHandled()
);
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(3);
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.delegatePKI', {
body: {
x509_certificate_chain: [
'fingerprint:2A:7A:C2:DD:base64',
'fingerprint:3B:8B:D3:EE:base64',
],
},
});
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.delegatePKI', {
body: {
x509_certificate_chain: [
'fingerprint:3A:7A:C2:DD:base64',
'fingerprint:3B:8B:D3:EE:base64',
],
},
});
expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.delegatePKI', {
body: {
x509_certificate_chain: [
'fingerprint:4A:7A:C2:DD:base64',
'fingerprint:3B:8B:D3:EE:base64',
],
},
});

expect(mockOptions.client.callAsInternalUser).not.toHaveBeenCalled();
expect(request.headers).not.toHaveProperty('authorization');
expect(nonAjaxRequest.headers).not.toHaveProperty('authorization');
expect(ajaxRequest.headers).not.toHaveProperty('authorization');
expect(optionalAuthRequest.headers).not.toHaveProperty('authorization');
});

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