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

Add server side private IP blocking for data source endpoints validation #3912

Merged
merged 1 commit into from
Apr 24, 2023
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: 3 additions & 0 deletions .lycheeexclude
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ https://opensearch.org/redirect
http://www.opensearch.org/painlessDocs
https://www.hostedgraphite.com/
https://connectionurl.com
http://169.254.169.254/latest/meta-data/

# External urls
https://www.zeek.org/
Expand Down Expand Up @@ -117,3 +118,5 @@ http://www.creedthoughts.gov
https://media-for-the-masses.theacademyofperformingartsandscience.org/
https://yarnpkg.com/latest.msi
https://forum.opensearch.org/
https://facebook.github.io/jest/
https://facebook.github.io/jest/docs/cli.html
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [CVE-2023-25653] Bump node-jose to 2.2.0 ([#3445](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3445))
- [CVE-2023-26486][cve-2023-26487] Bump vega from 5.22.1 to 5.23.0 ([#3533](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3533))
- [CVE-2023-0842] Bump xml2js from 0.4.23 to 0.5.0 ([#3842](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3842))
- [Multi DataSource] Add private IP blocking validation on server side([#3912](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3912))

### 📈 Features/Enhancements

Expand Down
27 changes: 26 additions & 1 deletion config/opensearch_dashboards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -238,5 +238,30 @@
#data_source.encryption.wrappingKeyNamespace: 'changeme'
#data_source.encryption.wrappingKey: [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]

#data_source.endpointDeniedIPs: [
# '127.0.0.0/8',
kristenTian marked this conversation as resolved.
Show resolved Hide resolved
# '::1/128',
# '169.254.0.0/16',
# 'fe80::/10',
# '10.0.0.0/8',
# '172.16.0.0/12',
# '192.168.0.0/16',
# 'fc00::/7',
# '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',
# ]

# Set the value of this setting to false to hide the help menu link to the OpenSearch Dashboards user survey
# opensearchDashboards.survey.url: "https://survey.opensearch.org"
# opensearchDashboards.survey.url: "https://survey.opensearch.org"
1 change: 1 addition & 0 deletions src/plugins/data_source/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const configSchema = schema.object({
enabled: schema.boolean({ defaultValue: false }),
appender: fileAppenderSchema,
}),
endpointDeniedIPs: schema.maybe(schema.arrayOf(schema.string())),
});

export type DataSourcePluginConfigType = TypeOf<typeof configSchema>;
3 changes: 2 additions & 1 deletion src/plugins/data_source/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ export class DataSourcePlugin implements Plugin<DataSourcePluginSetup, DataSourc

const dataSourceSavedObjectsClientWrapper = new DataSourceSavedObjectsClientWrapper(
cryptographyServiceSetup,
this.logger.get('data-source-saved-objects-client-wrapper-factory')
this.logger.get('data-source-saved-objects-client-wrapper-factory'),
config.endpointDeniedIPs
);

// Add data source saved objects client wrapper factory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,13 @@ import {
UsernamePasswordTypedContent,
} from '../../common/data_sources';
import { EncryptionContext, CryptographyServiceSetup } from '../cryptography_service';
import { isValidURL } from '../util/endpoint_validator';

/**
* Describes the Credential Saved Objects Client Wrapper class,
* which contains the factory used to create Saved Objects Client Wrapper instances
*/
export class DataSourceSavedObjectsClientWrapper {
constructor(private cryptography: CryptographyServiceSetup, private logger: Logger) {}

/**
* Describes the factory used to create instances of Saved Objects Client Wrappers
* for data source specific operations such as credentials encryption
Expand Down Expand Up @@ -138,14 +137,11 @@ export class DataSourceSavedObjectsClientWrapper {
};
};

private isValidUrl(endpoint: string) {
try {
const url = new URL(endpoint);
return Boolean(url) && (url.protocol === 'http:' || url.protocol === 'https:');
} catch (e) {
return false;
}
}
constructor(
private cryptography: CryptographyServiceSetup,
private logger: Logger,
private endpointBlockedIps?: string[]
) {}

private async validateAndEncryptAttributes<T = unknown>(attributes: T) {
this.validateAttributes(attributes);
Expand Down Expand Up @@ -254,8 +250,10 @@ export class DataSourceSavedObjectsClientWrapper {
);
}

if (!this.isValidUrl(endpoint)) {
throw SavedObjectsErrorHelpers.createBadRequestError('"endpoint" attribute is not valid');
if (!isValidURL(endpoint, this.endpointBlockedIps)) {
throw SavedObjectsErrorHelpers.createBadRequestError(
'"endpoint" attribute is not valid or allowed'
);
}

if (!auth) {
Expand Down
34 changes: 34 additions & 0 deletions src/plugins/data_source/server/util/endpoint_validator.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import * as validator from './endpoint_validator';

describe('endpoint_validator', function () {
it('Url1 that should be blocked should return false', function () {
expect(validator.isValidURL('http://127.0.0.1', ['127.0.0.0/8'])).toEqual(false);
});

it('Url2 that is invalid should return false', function () {
expect(validator.isValidURL('www.test.com', [])).toEqual(false);
});

it('Url3 that is invalid should return false', function () {
expect(validator.isValidURL('ftp://www.test.com', [])).toEqual(false);
});

it('Url4 that should be blocked should return false', function () {
expect(
validator.isValidURL('http://169.254.169.254/latest/meta-data/', ['169.254.0.0/16'])
).toEqual(false);
});

it('Url5 that should not be blocked should return true', function () {
expect(validator.isValidURL('https://www.opensearch.org', ['127.0.0.0/8'])).toEqual(true);
});

kristenTian marked this conversation as resolved.
Show resolved Hide resolved
it('Url6 that should not be blocked should return true when null IPs', function () {
expect(validator.isValidURL('https://www.opensearch.org')).toEqual(true);
});
});
59 changes: 59 additions & 0 deletions src/plugins/data_source/server/util/endpoint_validator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import dns from 'dns-sync';
import IPCIDR from 'ip-cidr';

export function isValidURL(endpoint: string, deniedIPs?: string[]) {
// Check the format of URL, URL has be in the format as
// scheme://server/path/resource otherwise an TypeError
// would be thrown.
let url;
try {
url = new URL(endpoint);
} catch (err) {
return false;
}

if (!(Boolean(url) && (url.protocol === 'http:' || url.protocol === 'https:'))) {
return false;
}

const ip = getIpAddress(url);
if (!ip) {
return false;
}

// IP CIDR check if a specific IP address fall in the
// range of an IP address block
for (const deniedIP of deniedIPs ?? []) {
Copy link
Member

Choose a reason for hiding this comment

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

Assuming that opensearch and another http service are deployed on 5601 and 1025, attackers can still attack / visit the 1025 port by using dashboard as a "jump server"? Maybe we should add an allowlist with port validation?

Copy link
Member

Choose a reason for hiding this comment

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

@SuZhou-Joe Can you make a follow-up issue for tracking?

const cidr = new IPCIDR(deniedIP);
if (cidr.contains(ip)) {
return false;
}
}
return true;
}

/**
* Resolve hostname to IP address
* @param {object} urlObject
* @returns {string} configuredIP
* or null if it cannot be resolve
* According to RFC, all IPv6 IP address needs to be in []
* such as [::1].
* So if we detect a IPv6 address, we remove brackets.
*/
function getIpAddress(urlObject: URL) {
const hostname = urlObject.hostname;
const configuredIP = dns.resolve(hostname);
if (configuredIP) {
return configuredIP;
}
if (hostname.startsWith('[') && hostname.endsWith(']')) {
return hostname.substr(1).slice(0, -1);
}
return null;
}