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

[Actions] Converted rejectUnauthorized config usages to verificationMode. #100179

Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9351d18
[Actions] Converted `rejectUnauthorized` config usages to `verificati…
YulNaumenko May 14, 2021
9a04843
Merge remote-tracking branch upstream/master
YulNaumenko May 14, 2021
ab10849
added new verificationMode config options for tls, proxy tls and cust…
YulNaumenko May 18, 2021
9ca7e2f
Merge remote-tracking branch upstream/master
YulNaumenko May 18, 2021
5e6f50f
added unit tests
YulNaumenko May 18, 2021
aede670
added unit tests
YulNaumenko May 18, 2021
b8b4a78
added kibana docker
YulNaumenko May 18, 2021
92135f2
Apply suggestions from code review
YulNaumenko May 18, 2021
b8f8a7e
Update alert-action-settings.asciidoc
YulNaumenko May 18, 2021
1e6aea4
Merge remote-tracking branch upstream/master
YulNaumenko May 19, 2021
ab8fb47
Apply suggestions from code review
YulNaumenko May 19, 2021
2574873
removed legacyRegectUnauthorized logic from getNodeTLSOptions
YulNaumenko May 19, 2021
3a4e08e
fixed typos
YulNaumenko May 19, 2021
de40bf2
added deprecations
YulNaumenko May 20, 2021
7e16add
Merge remote-tracking branch upstream/master
YulNaumenko May 20, 2021
7ff5a17
fixed doc links
YulNaumenko May 20, 2021
6c15bc3
fixed docs
YulNaumenko May 20, 2021
02269c1
Update x-pack/plugins/actions/server/builtin_action_types/lib/send_em…
YulNaumenko May 20, 2021
024a7db
[DOCS] Fixes build error
lcawl May 20, 2021
6368cca
Merge branch 'master' into actions-reject-unauth-migrate-to-verif-mode
kibanamachine May 24, 2021
1155f15
fixed deprecations to set custom message
YulNaumenko May 24, 2021
9315add
fixed doc
YulNaumenko May 24, 2021
393765e
changed to not throw exception on non existing verification mode
YulNaumenko May 26, 2021
0dfc20d
added tests
YulNaumenko May 26, 2021
42dd1db
fixed tests
YulNaumenko May 26, 2021
97ccc8a
Merge remote-tracking branch upstream/master
YulNaumenko May 26, 2021
6ac9f5e
fixed tests
YulNaumenko May 26, 2021
291385e
added integration tests for legacy rejectUnauthorized fale
YulNaumenko May 26, 2021
ce5f339
fixed tests
YulNaumenko May 27, 2021
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
26 changes: 23 additions & 3 deletions docs/settings/alert-action-settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ You can configure the following settings in the `kibana.yml` file.
xpack.actions.customHostSettings:
- url: smtp://mail.example.com:465
tls:
verificationMode: 'full'
certificateAuthoritiesFiles: [ 'one.crt' ]
certificateAuthoritiesData: |
-----BEGIN CERTIFICATE-----
Expand All @@ -79,7 +80,9 @@ xpack.actions.customHostSettings:
requireTLS: true
- url: https://webhook.example.com
tls:
// legacy
rejectUnauthorized: false
verificationMode: 'none'
--

[cols="2*<"]
Expand Down Expand Up @@ -114,11 +117,16 @@ xpack.actions.customHostSettings:
The options `smtp.ignoreTLS` and `smtp.requireTLS` can not both be set to true.

| `xpack.actions.customHostSettings[n]`
`.tls.rejectUnauthorized` {ess-icon}
`.tls.rejectUnauthorized` [Deprecated. Use `xpack.actions.customHostSettings.tls.verificationMode`] {ess-icon}
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved
| A boolean value indicating whether to bypass server certificate validation.
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved
Overrides the general `xpack.actions.rejectUnauthorized` configuration
for requests made for this hostname/port.

| `xpack.actions.customHostSettings[n]`
`.tls.verificationMode`
| Controls the verification of the server certificate that {hosted-ems} receives when making an outbound SSL/TLS connection to {es}. Valid values are "`full`", "`certificate`", and "`none`".
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved
Using "`full`" performs hostname verification, using "`certificate`" skips hostname verification, and using "`none`" skips verification entirely. *Default: `full`*. <<elasticsearch-ssl-verificationMode,Equivalent {kib} setting>>.
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved

| `xpack.actions.customHostSettings[n]`
`.tls.certificateAuthoritiesFiles`
| A file name or list of file names of PEM-encoded certificate files to use
Expand Down Expand Up @@ -155,15 +163,27 @@ xpack.actions.customHostSettings:
| Specifies HTTP headers for the proxy, if using a proxy for actions. Defaults to {}.

a|`xpack.actions.`
`proxyRejectUnauthorizedCertificates` {ess-icon}
`proxyRejectUnauthorizedCertificates` [Deprecated Use `xpack.actions.tls.proxyVerificationMode`.] {ess-icon}
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved
| Set to `false` to bypass certificate validation for the proxy, if using a proxy for actions. Defaults to `true`.
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved

| `xpack.actions.rejectUnauthorized` {ess-icon}
a|`xpack.actions[n]`
`.tls.proxyVerificationMode` {ess-icon}
| Controls the verification for the proxy server certificate that {hosted-ems} receives when making an outbound SSL/TLS connection to {es}. Valid values are "`full`", "`certificate`", and "`none`".
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved
Using "`full`" performs hostname verification, using "`certificate`" skips hostname verification, and using "`none`" skips verification entirely. *Default: `full`*. <<elasticsearch-ssl-verificationMode,Equivalent {kib} setting>>.
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved

| `xpack.actions.rejectUnauthorized` [Deprecated. Use `xpack.actions.tls.verificationMode`.] {ess-icon}
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved
| Set to `false` to bypass certificate validation for actions. Defaults to `true`. +
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved
+
As an alternative to setting `xpack.actions.rejectUnauthorized`, you can use the setting
`xpack.actions.customHostSettings` to set TLS options for specific servers.

a|`xpack.actions[n]`
`.tls.verificationMode` {ess-icon}
| Controls the verification for the server certificate that {hosted-ems} receives when making an outbound SSL/TLS connection to {es}. Valid values are "`full`", "`certificate`", and "`none`".
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved
Using "`full`" performs hostname verification, using "`certificate`" skips hostname verification, and using "`none`" skips verification entirely. *Default: `full`*. <<elasticsearch-ssl-verificationMode,Equivalent {kib} setting>>.
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved



| `xpack.actions.maxResponseContentLength` {ess-icon}
| Specifies the max number of bytes of the http response for requests to external resources. Defaults to 1000000 (1MB).

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ kibana_vars=(
xpack.actions.rejectUnauthorized
xpack.actions.maxResponseContentLength
xpack.actions.responseTimeout
xpack.actions.tls.verificationMode
xpack.actions.tls.proxyVerificationMode
xpack.alerts.healthCheck.interval
xpack.alerts.invalidateApiKeysTask.interval
xpack.alerts.invalidateApiKeysTask.removalDelay
Expand Down
8 changes: 6 additions & 2 deletions x-pack/plugins/actions/server/actions_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,8 @@ describe('create()', () => {
allowedHosts: ['*'],
preconfiguredAlertHistoryEsIndex: false,
preconfigured: {},
proxyRejectUnauthorizedCertificates: true,
rejectUnauthorized: true,
proxyRejectUnauthorizedCertificates: true, // legacy
rejectUnauthorized: true, // legacy
proxyBypassHosts: undefined,
proxyOnlyHosts: undefined,
maxResponseContentLength: new ByteSizeValue(1000000),
Expand All @@ -429,6 +429,10 @@ describe('create()', () => {
idleInterval: schema.duration().validate('1h'),
pageSize: 100,
},
tls: {
verificationMode: 'full',
proxyVerificationMode: 'full',
},
});

const localActionTypeRegistryParams = {
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/actions/server/actions_config.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ const createActionsConfigMock = () => {
ensureHostnameAllowed: jest.fn().mockReturnValue({}),
ensureUriAllowed: jest.fn().mockReturnValue({}),
ensureActionTypeEnabled: jest.fn().mockReturnValue({}),
isRejectUnauthorizedCertificatesEnabled: jest.fn().mockReturnValue(true),
getTLSSettings: jest.fn().mockReturnValue({
verificationMode: 'full',
}),
getProxySettings: jest.fn().mockReturnValue(undefined),
getResponseSettings: jest.fn().mockReturnValue({
maxContentLength: 1000000,
Expand Down
62 changes: 55 additions & 7 deletions x-pack/plugins/actions/server/actions_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ const defaultActionsConfig: ActionsConfig = {
enabledActionTypes: [],
preconfiguredAlertHistoryEsIndex: false,
preconfigured: {},
proxyRejectUnauthorizedCertificates: true,
rejectUnauthorized: true,
proxyRejectUnauthorizedCertificates: true, // legacy
rejectUnauthorized: true, // legacy
maxResponseContentLength: new ByteSizeValue(1000000),
responseTimeout: moment.duration(60000),
cleanupFailedExecutionsTask: {
Expand All @@ -37,6 +37,10 @@ const defaultActionsConfig: ActionsConfig = {
idleInterval: schema.duration().validate('1h'),
pageSize: 100,
},
tls: {
proxyVerificationMode: 'full',
verificationMode: 'full',
},
};

describe('ensureUriAllowed', () => {
Expand Down Expand Up @@ -305,22 +309,44 @@ describe('getProxySettings', () => {
expect(proxySettings?.proxyUrl).toBe(config.proxyUrl);
});

test('returns proxyRejectUnauthorizedCertificates', () => {
test('returns proper legacyRejectUnauthorized values, beased on the legacy config option proxyRejectUnauthorizedCertificates', () => {
const configTrue: ActionsConfig = {
...defaultActionsConfig,
proxyUrl: 'https://proxy.elastic.co',
proxyRejectUnauthorizedCertificates: true,
};
let proxySettings = getActionsConfigurationUtilities(configTrue).getProxySettings();
expect(proxySettings?.proxyRejectUnauthorizedCertificates).toBe(true);
expect(proxySettings?.proxyTLSSettings.legacyRejectUnauthorized).toBe(true);

const configFalse: ActionsConfig = {
...defaultActionsConfig,
proxyUrl: 'https://proxy.elastic.co',
proxyRejectUnauthorizedCertificates: false,
};
proxySettings = getActionsConfigurationUtilities(configFalse).getProxySettings();
expect(proxySettings?.proxyRejectUnauthorizedCertificates).toBe(false);
expect(proxySettings?.proxyTLSSettings.legacyRejectUnauthorized).toBe(false);
});

test('returns proper verificationMode value, based on the TLS proxy configuration', () => {
const configTrue: ActionsConfig = {
...defaultActionsConfig,
proxyUrl: 'https://proxy.elastic.co',
tls: {
proxyVerificationMode: 'full',
},
};
let proxySettings = getActionsConfigurationUtilities(configTrue).getProxySettings();
expect(proxySettings?.proxyTLSSettings.verificationMode).toBe('full');

const configFalse: ActionsConfig = {
...defaultActionsConfig,
proxyUrl: 'https://proxy.elastic.co',
tls: {
proxyVerificationMode: 'none',
},
};
proxySettings = getActionsConfigurationUtilities(configFalse).getProxySettings();
expect(proxySettings?.proxyTLSSettings.verificationMode).toBe('none');
});

test('returns proxy headers', () => {
Expand Down Expand Up @@ -406,13 +432,13 @@ describe('getProxySettings', () => {
{
url: 'https://elastic.co',
tls: {
rejectUnauthorized: true,
verificationMode: 'full',
},
},
{
url: 'smtp://elastic.co:123',
tls: {
rejectUnauthorized: false,
verificationMode: 'none',
},
smtp: {
ignoreTLS: true,
Expand All @@ -437,3 +463,25 @@ describe('getProxySettings', () => {
expect(chs).toEqual(undefined);
});
});

describe('getTLSSettings', () => {
test('returns proper verificationMode value, based on the TLS proxy configuration', () => {
const configTrue: ActionsConfig = {
...defaultActionsConfig,
tls: {
verificationMode: 'full',
},
};
let tlsSettings = getActionsConfigurationUtilities(configTrue).getTLSSettings();
expect(tlsSettings.verificationMode).toBe('full');

const configFalse: ActionsConfig = {
...defaultActionsConfig,
tls: {
verificationMode: 'none',
},
};
tlsSettings = getActionsConfigurationUtilities(configFalse).getTLSSettings();
expect(tlsSettings.verificationMode).toBe('none');
});
});
20 changes: 15 additions & 5 deletions x-pack/plugins/actions/server/actions_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { pipe } from 'fp-ts/lib/pipeable';
import { ActionsConfig, AllowedHosts, EnabledActionTypes, CustomHostSettings } from './config';
import { getCanonicalCustomHostUrl } from './lib/custom_host_settings';
import { ActionTypeDisabledError } from './lib';
import { ProxySettings, ResponseSettings } from './types';
import { ProxySettings, ResponseSettings, TLSSettings } from './types';

export { AllowedHosts, EnabledActionTypes } from './config';

Expand All @@ -30,7 +30,7 @@ export interface ActionsConfigurationUtilities {
ensureHostnameAllowed: (hostname: string) => void;
ensureUriAllowed: (uri: string) => void;
ensureActionTypeEnabled: (actionType: string) => void;
isRejectUnauthorizedCertificatesEnabled: () => boolean;
getTLSSettings: () => TLSSettings;
getProxySettings: () => undefined | ProxySettings;
getResponseSettings: () => ResponseSettings;
getCustomHostSettings: (targetUrl: string) => CustomHostSettings | undefined;
Expand Down Expand Up @@ -93,10 +93,20 @@ function getProxySettingsFromConfig(config: ActionsConfig): undefined | ProxySet
proxyBypassHosts: arrayAsSet(config.proxyBypassHosts),
proxyOnlyHosts: arrayAsSet(config.proxyOnlyHosts),
proxyHeaders: config.proxyHeaders,
proxyRejectUnauthorizedCertificates: config.proxyRejectUnauthorizedCertificates,
proxyTLSSettings: getTLSSettingsFromConfig(
config.tls?.proxyVerificationMode,
config.proxyRejectUnauthorizedCertificates
),
};
}

function getTLSSettingsFromConfig(
verificationMode?: 'none' | 'certificate' | 'full',
rejectUnauthorized?: boolean
): TLSSettings {
return { legacyRejectUnauthorized: rejectUnauthorized, verificationMode };
}
YulNaumenko marked this conversation as resolved.
Show resolved Hide resolved

function arrayAsSet<T>(arr: T[] | undefined): Set<T> | undefined {
if (!arr) return;
return new Set(arr);
Expand Down Expand Up @@ -142,8 +152,8 @@ export function getActionsConfigurationUtilities(
isActionTypeEnabled,
getProxySettings: () => getProxySettingsFromConfig(config),
getResponseSettings: () => getResponseSettingsFromConfig(config),
// returns the global rejectUnauthorized setting
isRejectUnauthorizedCertificatesEnabled: () => config.rejectUnauthorized,
getTLSSettings: () =>
getTLSSettingsFromConfig(config.tls?.verificationMode, config.rejectUnauthorized),
ensureUriAllowed(uri: string) {
if (!isUriAllowed(uri)) {
throw new Error(allowListErrorMessage(AllowListingField.URL, uri));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,9 @@ describe('execute()', () => {
"getCustomHostSettings": [MockFunction],
"getProxySettings": [MockFunction],
"getResponseSettings": [MockFunction],
"getTLSSettings": [MockFunction],
"isActionTypeEnabled": [MockFunction],
"isHostnameAllowed": [MockFunction],
"isRejectUnauthorizedCertificatesEnabled": [MockFunction],
"isUriAllowed": [MockFunction],
},
"content": Object {
Expand Down Expand Up @@ -346,9 +346,9 @@ describe('execute()', () => {
"getCustomHostSettings": [MockFunction],
"getProxySettings": [MockFunction],
"getResponseSettings": [MockFunction],
"getTLSSettings": [MockFunction],
"isActionTypeEnabled": [MockFunction],
"isHostnameAllowed": [MockFunction],
"isRejectUnauthorizedCertificatesEnabled": [MockFunction],
"isUriAllowed": [MockFunction],
},
"content": Object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { getCustomAgents } from './get_custom_agents';
const TestUrl = 'https://elastic.co/foo/bar/baz';

const logger = loggingSystemMock.create().get() as jest.Mocked<Logger>;
const configurationUtilities = actionsConfigMock.create();
let configurationUtilities = actionsConfigMock.create();
jest.mock('axios');
const axiosMock = (axios as unknown) as jest.Mock;

Expand All @@ -42,6 +42,7 @@ describe('request', () => {
headers: { 'content-type': 'application/json' },
data: { incidentId: '123' },
}));
configurationUtilities = actionsConfigMock.create();
configurationUtilities.getResponseSettings.mockReturnValue({
maxContentLength: 1000000,
timeout: 360000,
Expand Down Expand Up @@ -74,7 +75,9 @@ describe('request', () => {

test('it have been called with proper proxy agent for a valid url', async () => {
configurationUtilities.getProxySettings.mockReturnValue({
proxyRejectUnauthorizedCertificates: true,
proxyTLSSettings: {
verificationMode: 'full',
},
proxyUrl: 'https://localhost:1212',
proxyBypassHosts: undefined,
proxyOnlyHosts: undefined,
Expand Down Expand Up @@ -107,7 +110,9 @@ describe('request', () => {
test('it have been called with proper proxy agent for an invalid url', async () => {
configurationUtilities.getProxySettings.mockReturnValue({
proxyUrl: ':nope:',
proxyRejectUnauthorizedCertificates: false,
proxyTLSSettings: {
verificationMode: 'none',
},
proxyBypassHosts: undefined,
proxyOnlyHosts: undefined,
});
Expand Down Expand Up @@ -136,7 +141,9 @@ describe('request', () => {

test('it bypasses with proxyBypassHosts when expected', async () => {
configurationUtilities.getProxySettings.mockReturnValue({
proxyRejectUnauthorizedCertificates: true,
proxyTLSSettings: {
verificationMode: 'full',
},
proxyUrl: 'https://elastic.proxy.co',
proxyBypassHosts: new Set(['elastic.co']),
proxyOnlyHosts: undefined,
Expand All @@ -157,7 +164,9 @@ describe('request', () => {

test('it does not bypass with proxyBypassHosts when expected', async () => {
configurationUtilities.getProxySettings.mockReturnValue({
proxyRejectUnauthorizedCertificates: true,
proxyTLSSettings: {
verificationMode: 'full',
},
proxyUrl: 'https://elastic.proxy.co',
proxyBypassHosts: new Set(['not-elastic.co']),
proxyOnlyHosts: undefined,
Expand All @@ -178,7 +187,9 @@ describe('request', () => {

test('it proxies with proxyOnlyHosts when expected', async () => {
configurationUtilities.getProxySettings.mockReturnValue({
proxyRejectUnauthorizedCertificates: true,
proxyTLSSettings: {
verificationMode: 'full',
},
proxyUrl: 'https://elastic.proxy.co',
proxyBypassHosts: undefined,
proxyOnlyHosts: new Set(['elastic.co']),
Expand All @@ -199,7 +210,9 @@ describe('request', () => {

test('it does not proxy with proxyOnlyHosts when expected', async () => {
configurationUtilities.getProxySettings.mockReturnValue({
proxyRejectUnauthorizedCertificates: true,
proxyTLSSettings: {
verificationMode: 'full',
},
proxyUrl: 'https://elastic.proxy.co',
proxyBypassHosts: undefined,
proxyOnlyHosts: new Set(['not-elastic.co']),
Expand Down Expand Up @@ -252,6 +265,7 @@ describe('patch', () => {
status: 200,
headers: { 'content-type': 'application/json' },
}));
configurationUtilities = actionsConfigMock.create();
configurationUtilities.getResponseSettings.mockReturnValue({
maxContentLength: 1000000,
timeout: 360000,
Expand Down
Loading