Skip to content

Commit

Permalink
[Patch] Graphite SSRF patch (#392)
Browse files Browse the repository at this point in the history
* ssrf patch

Signed-off-by: Anan Zhuang <[email protected]>

* revise based on PR comments

Signed-off-by: Anan Zhuang <[email protected]>

* revise unit test and comments

Signed-off-by: Anan Zhuang <[email protected]>

* fix lint issue

Signed-off-by: Anan Zhuang <[email protected]>

* add helper

Signed-off-by: Anan Zhuang <[email protected]>

* fix bug

Signed-off-by: Anan Zhuang <[email protected]>

* fix comments

Signed-off-by: Anan Zhuang <[email protected]>

* fix frontend display. helper shouldnot show in visualize

Signed-off-by: Anan Zhuang <[email protected]>
  • Loading branch information
ananzh authored and kavilla committed Jun 4, 2021
1 parent c749ab7 commit 16694e2
Show file tree
Hide file tree
Showing 13 changed files with 512 additions and 24 deletions.
35 changes: 35 additions & 0 deletions config/opensearch_dashboards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,38 @@
# Specifies locale to be used for all localizable strings, dates and number formats.
# Supported languages are the following: English - en , by default , Chinese - zh-CN .
#i18n.locale: "en"

# Set the allowlist to check input graphite Url. Allowlist is the default check list.
#vis_type_timeline.graphiteAllowedUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite']

# Set the blocklist to check input graphite Url. Blocklist is an IP list.
# Below is an example for reference
# vis_type_timeline.graphiteBlockedIPs: [
# //Loopback
# '127.0.0.0/8',
# '::1/128',
# //Link-local Address for IPv6
# 'fe80::/10',
# //Private IP address for IPv4
# '10.0.0.0/8',
# '172.16.0.0/12',
# '192.168.0.0/16',
# //Unique local address (ULA)
# 'fc00::/7',
# //Reserved IP address
# '0.0.0.0/8',
# '100.64.0.0/10',
# '192.0.0.0/24',
# '192.0.2.0/24',
# '198.18.0.0/15',
# '192.88.99.0/24',
# '198.51.100.0/24',
# '203.0.113.0/24',
# '224.0.0.0/4',
# '240.0.0.0/4',
# '255.255.255.255/32',
# '::/128',
# '2001:db8::/32',
# 'ff00::/8',
# ]
#vis_type_timeline.graphiteBlockedIPs: []
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@
"cypress-promise": "^1.1.0",
"deep-freeze-strict": "^1.1.1",
"del": "^5.1.0",
"dns-sync": "^0.2.1",
"elastic-apm-node": "^3.7.0",
"elasticsearch": "^16.7.0",
"execa": "^4.0.2",
Expand All @@ -169,6 +170,7 @@
"https-proxy-agent": "^5.0.0",
"inert": "^5.1.0",
"inline-style": "^2.0.0",
"ip-cidr": "^2.1.0",
"joi": "^13.5.2",
"js-yaml": "^3.14.0",
"json-stable-stringify": "^1.0.1",
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/vis_type_timeline/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ export const configSchema = schema.object(
{
enabled: schema.boolean({ defaultValue: true }),
ui: schema.object({ enabled: schema.boolean({ defaultValue: false }) }),
graphiteUrls: schema.maybe(schema.arrayOf(schema.string())),
graphiteAllowedUrls: schema.maybe(schema.arrayOf(schema.string())),
graphiteBlockedIPs: schema.maybe(schema.arrayOf(schema.string())),
},
// This option should be removed as soon as we entirely migrate config from legacy Timeline plugin.
{ unknowns: 'allow' }
Expand Down
1 change: 1 addition & 0 deletions src/plugins/vis_type_timeline/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export const config: PluginConfigDescriptor<ConfigSchema> = {
renameFromRoot('timeline_vis.enabled', 'vis_type_timeline.enabled'),
renameFromRoot('timeline.enabled', 'vis_type_timeline.enabled'),
renameFromRoot('timeline.graphiteUrls', 'vis_type_timeline.graphiteUrls'),
renameFromRoot('vis_type_timeline.graphiteUrls', 'vis_type_timeline.graphiteAllowedUrls'),
renameFromRoot('timeline.ui.enabled', 'vis_type_timeline.ui.enabled', true),
],
};
Expand Down
14 changes: 10 additions & 4 deletions src/plugins/vis_type_timeline/server/lib/config_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ import { configSchema } from '../../config';

export class ConfigManager {
private opensearchShardTimeout: number = 0;
private graphiteUrls: string[] = [];
private graphiteAllowedUrls: string[] = [];
private graphiteBlockedIPs: string[] = [];

constructor(config: PluginInitializerContext['config']) {
config.create<TypeOf<typeof configSchema>>().subscribe((configUpdate) => {
this.graphiteUrls = configUpdate.graphiteUrls || [];
this.graphiteAllowedUrls = configUpdate.graphiteAllowedUrls || [];
this.graphiteBlockedIPs = configUpdate.graphiteBlockedIPs || [];
});

config.legacy.globalConfig$.subscribe((configUpdate) => {
Expand All @@ -52,7 +54,11 @@ export class ConfigManager {
return this.opensearchShardTimeout;
}

getGraphiteUrls() {
return this.graphiteUrls;
getGraphiteAllowedUrls() {
return this.graphiteAllowedUrls;
}

getGraphiteBlockedIPs() {
return this.graphiteBlockedIPs;
}
}
10 changes: 5 additions & 5 deletions src/plugins/vis_type_timeline/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,14 @@ export class Plugin {
description:
'The URL should be in the form of https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite',
}),
value: config.graphiteUrls && config.graphiteUrls.length ? config.graphiteUrls[0] : null,
value:
config.graphiteAllowedUrls && config.graphiteAllowedUrls.length
? config.graphiteAllowedUrls[0]
: null,
description: i18n.translate('timeline.uiSettings.graphiteURLDescription', {
defaultMessage:
'{experimentalLabel} The <a href="https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite" target="_blank" rel="noopener">URL</a> of your graphite host',
defaultMessage: '{experimentalLabel} The URL of your graphite host',
values: { experimentalLabel: `<em>[${experimentalLabel}]</em>` },
}),
type: 'select',
options: config.graphiteUrls || [],
category: ['timeline'],
schema: schema.nullable(schema.string()),
},
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/vis_type_timeline/server/routes/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ export function runRoute(
settings: _.defaults(uiSettings, timelineDefaults), // Just in case they delete some setting.
getFunction,
getStartServices: core.getStartServices,
allowedGraphiteUrls: configManager.getGraphiteUrls(),
allowedGraphiteUrls: configManager.getGraphiteAllowedUrls(),
blockedGraphiteIPs: configManager.getGraphiteBlockedIPs(),
opensearchShardTimeout: configManager.getOpenSearchShardTimeout(),
savedObjectsClient: context.core.savedObjects.client,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export default function () {

opensearchShardTimeout: moment.duration(30000),
allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'],
blockedGraphiteIPs: [],
});

tlConfig.time = {
Expand All @@ -67,6 +68,10 @@ export default function () {

tlConfig.settings = timelineDefaults();

tlConfig.allowedGraphiteUrls = timelineDefaults();

tlConfig.blockedGraphiteIPs = timelineDefaults();

tlConfig.setTargetSeries();

return tlConfig;
Expand Down
30 changes: 23 additions & 7 deletions src/plugins/vis_type_timeline/server/series_functions/graphite.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ import _ from 'lodash';
import fetch from 'node-fetch';
import moment from 'moment';
import Datasource from '../lib/classes/datasource';
import { isValidConfig } from './helpers/graphite_helper';

const MISS_CHECKLIST_MESSAGE = `Please configure on the opensearch_dashboards.yml file.
You can always enable the default allowlist configuration.`;

const INVALID_URL_MESSAGE = `The Graphite URL provided by you is invalid.
Please update your config from OpenSearch Dashboards's Advanced Setting.`;

export default new Datasource('graphite', {
args: [
Expand All @@ -59,19 +66,28 @@ export default new Datasource('graphite', {
min: moment(tlConfig.time.from).format('HH:mm[_]YYYYMMDD'),
max: moment(tlConfig.time.to).format('HH:mm[_]YYYYMMDD'),
};

const allowedUrls = tlConfig.allowedGraphiteUrls;
const blockedIPs = tlConfig.blockedGraphiteIPs;
const configuredUrl = tlConfig.settings['timeline:graphite.url'];
if (!allowedUrls.includes(configuredUrl)) {

if (allowedUrls.length === 0 && blockedIPs.length === 0) {
throw new Error(
i18n.translate('timeline.help.functions.missCheckGraphiteConfig', {
defaultMessage: MISS_CHECKLIST_MESSAGE,
})
);
}

if (!isValidConfig(blockedIPs, allowedUrls, configuredUrl)) {
throw new Error(
i18n.translate('timeline.help.functions.notAllowedGraphiteUrl', {
defaultMessage: `This graphite URL is not configured on the opensearch_dashbpards.yml file.
Please configure your graphite server list in the opensearch_dashbpards.yml file under 'timeline.graphiteUrls' and
select one from OpenSearch Dashboards's Advanced Settings`,
i18n.translate('timeline.help.functions.invalidGraphiteConfig', {
defaultMessage: INVALID_URL_MESSAGE,
})
);
}

const URL =
const GRAPHITE_URL =
tlConfig.settings['timeline:graphite.url'] +
'/render/' +
'?format=json' +
Expand All @@ -82,7 +98,7 @@ export default new Datasource('graphite', {
'&target=' +
config.metric;

return fetch(URL)
return fetch(GRAPHITE_URL, { redirect: 'error' })
.then(function (resp) {
return resp.json();
})
Expand Down
142 changes: 138 additions & 4 deletions src/plugins/vis_type_timeline/server/series_functions/graphite.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,16 @@ const expect = require('chai').expect;

import fn from './graphite';

jest.mock('node-fetch', () => () => {
const MISS_CHECKLIST_MESSAGE = `Please configure on the opensearch_dashboards.yml file.
You can always enable the default allowlist configuration.`;

const INVALID_URL_MESSAGE = `The Graphite URL provided by you is invalid.
Please update your config from OpenSearch Dashboards's Advanced Setting.`;

jest.mock('node-fetch', () => (url) => {
if (url.includes('redirect')) {
return Promise.reject(new Error('maximum redirect reached at: ' + url));
}
return Promise.resolve({
json: function () {
return [
Expand All @@ -56,21 +65,146 @@ import invoke from './helpers/invoke_series_fn.js';

describe('graphite', function () {
it('should wrap the graphite response up in a seriesList', function () {
return invoke(fn, []).then(function (result) {
return invoke(fn, [], {
allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'],
blockedGraphiteIPs: [],
}).then(function (result) {
expect(result.output.list[0].data[0][1]).to.eql(3);
expect(result.output.list[0].data[1][1]).to.eql(14);
});
});

it('should convert the seconds to milliseconds', function () {
return invoke(fn, []).then(function (result) {
return invoke(fn, [], {
allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'],
blockedGraphiteIPs: [],
}).then(function (result) {
expect(result.output.list[0].data[1][0]).to.eql(2000 * 1000);
});
});

it('should set the label to that of the graphite target', function () {
return invoke(fn, []).then(function (result) {
return invoke(fn, [], {
allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'],
blockedGraphiteIPs: [],
}).then(function (result) {
expect(result.output.list[0].label).to.eql('__beer__');
});
});

it('should return error message if both allowlist and blocklist are disabled', function () {
return invoke(fn, [], {
settings: { 'timeline:graphite.url': 'http://127.0.0.1' },
allowedGraphiteUrls: [],
blockedGraphiteIPs: [],
}).catch((e) => {
expect(e.message).to.eql(MISS_CHECKLIST_MESSAGE);
});
});

it('setting with matched allowlist url should return result', function () {
return invoke(fn, [], {
settings: {
'timeline:graphite.url': 'https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite',
},
allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'],
blockedGraphiteIPs: [],
}).then((result) => {
expect(result.output.list.length).to.eql(1);
});
});

it('setting with unmatched allowlist url should return error message', function () {
return invoke(fn, [], {
settings: { 'timeline:graphite.url': 'http://127.0.0.1' },
allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'],
blockedGraphiteIPs: [],
}).catch((e) => {
expect(e.message).to.eql(INVALID_URL_MESSAGE);
});
});

it('setting with matched blocklist url should return error message', function () {
return invoke(fn, [], {
settings: { 'timeline:graphite.url': 'http://127.0.0.1' },
allowedGraphiteUrls: [],
blockedGraphiteIPs: ['127.0.0.0/8'],
}).catch((e) => {
expect(e.message).to.eql(INVALID_URL_MESSAGE);
});
});

it('setting with matched blocklist localhost should return error message', function () {
return invoke(fn, [], {
settings: { 'timeline:graphite.url': 'http://localhost' },
allowedGraphiteUrls: [],
blockedGraphiteIPs: ['127.0.0.0/8'],
}).catch((e) => {
expect(e.message).to.eql(INVALID_URL_MESSAGE);
});
});

it('setting with unmatched blocklist https url should return result', function () {
return invoke(fn, [], {
settings: { 'timeline:graphite.url': 'https://www.opensearch.org/' },
allowedGraphiteUrls: [],
blockedGraphiteIPs: ['127.0.0.0/8'],
}).then((result) => {
expect(result.output.list.length).to.eql(1);
});
});

it('setting with unmatched blocklist ftp url should return result', function () {
return invoke(fn, [], {
settings: { 'timeline:graphite.url': 'ftp://www.opensearch.org' },
allowedGraphiteUrls: [],
blockedGraphiteIPs: ['127.0.0.0/8'],
}).then((result) => {
expect(result.output.list.length).to.eql(1);
});
});

it('setting with invalid url should return error message', function () {
return invoke(fn, [], {
settings: { 'timeline:graphite.url': 'www.opensearch.org' },
allowedGraphiteUrls: [],
blockedGraphiteIPs: ['127.0.0.0/8'],
}).catch((e) => {
expect(e.message).to.eql(INVALID_URL_MESSAGE);
});
});

it('setting with redirection error message', function () {
return invoke(fn, [], {
settings: { 'timeline:graphite.url': 'https://www.opensearch.org/redirect' },
allowedGraphiteUrls: [],
blockedGraphiteIPs: ['127.0.0.0/8'],
}).catch((e) => {
expect(e.message).to.includes('maximum redirect reached');
});
});

it('with both allowlist and blocklist, setting not in blocklist but in allowlist should return result', function () {
return invoke(fn, [], {
settings: {
'timeline:graphite.url': 'https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite',
},
allowedGraphiteUrls: ['https://www.hostedgraphite.com/UID/ACCESS_KEY/graphite'],
blockedGraphiteIPs: ['127.0.0.0/8'],
}).then((result) => {
expect(result.output.list.length).to.eql(1);
});
});

it('with conflict allowlist and blocklist, setting in blocklist and in allowlist should return error message', function () {
return invoke(fn, [], {
settings: {
'timeline:graphite.url': 'http://127.0.0.1',
},
allowedGraphiteUrls: ['http://127.0.0.1'],
blockedGraphiteIPs: ['127.0.0.0/8'],
}).catch((e) => {
expect(e.message).to.eql(INVALID_URL_MESSAGE);
});
});
});
Loading

0 comments on commit 16694e2

Please sign in to comment.