Skip to content

Commit

Permalink
properly handle hosts ending in a 'dot', and account for IPv6 hosts
Browse files Browse the repository at this point in the history
  • Loading branch information
legrego committed Dec 9, 2020
1 parent f3dd0ba commit 85c1dfb
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 6 deletions.
72 changes: 68 additions & 4 deletions src/core/public/http/external_url_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,17 @@ const setupService = ({
serverBasePath: string;
policy: ExternalUrlConfig['policy'];
}) => {
const hashedPolicies = policy.map((entry) => ({
...entry,
host: entry.host ? new Sha256().update(entry.host, 'utf8').digest('hex') : undefined,
}));
const hashedPolicies = policy.map((entry) => {
// If the host contains a `[`, then it's likely an IPv6 address. Otherwise, append a `.` if it doesn't already contain one
const hostToHash =
entry.host && !entry.host.includes('[') && !entry.host.endsWith('.')
? `${entry.host}.`
: entry.host;
return {
...entry,
host: hostToHash ? new Sha256().update(hostToHash, 'utf8').digest('hex') : undefined,
};
});
const injectedMetadata = injectedMetadataServiceMock.createSetupContract();
injectedMetadata.getExternalUrlConfig.mockReturnValue({ policy: hashedPolicies });
injectedMetadata.getServerBasePath.mockReturnValue(serverBasePath);
Expand Down Expand Up @@ -332,6 +339,63 @@ describe('External Url Service', () => {
expect(result?.toString()).toEqual(urlCandidate);
});

it('allows external urls with a partially matching host and protocol in the allow list when the URL includes the root domain', () => {
const { setup } = setupService({
location,
serverBasePath,
policy: [
{
allow: true,
host: 'google.com',
protocol: 'https',
},
],
});
const urlCandidate = `https://www.google.com./foo?bar=baz`;
const result = setup.validateUrl(urlCandidate);

expect(result).toBeInstanceOf(URL);
expect(result?.toString()).toEqual(urlCandidate);
});

it('allows external urls with an IPv4 address', () => {
const { setup } = setupService({
location,
serverBasePath,
policy: [
{
allow: true,
host: '192.168.10.12',
protocol: 'https',
},
],
});
const urlCandidate = `https://192.168.10.12/foo?bar=baz`;
const result = setup.validateUrl(urlCandidate);

expect(result).toBeInstanceOf(URL);
expect(result?.toString()).toEqual(urlCandidate);
});

it('allows external urls with an IPv6 address', () => {
const { setup } = setupService({
location,
serverBasePath,
policy: [
{
allow: true,
host: '[2001:db8:85a3:8d3:1319:8a2e:370:7348]',
protocol: 'https',
},
],
});
const urlCandidate = `https://[2001:db8:85a3:8d3:1319:8a2e:370:7348]/foo?bar=baz`;
const result = setup.validateUrl(urlCandidate);

expect(result).toBeInstanceOf(URL);
expect(result?.toString()).toEqual(urlCandidate);
});

it('allows external urls that specify a locally addressable host', () => {
const { setup } = setupService({
location,
Expand Down
5 changes: 4 additions & 1 deletion src/core/public/http/external_url_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ function* getHostHashes(actualHost: string) {
}

const isHostMatch = (actualHost: string, ruleHostHash: string) => {
for (const hash of getHostHashes(actualHost)) {
// If the host contains a `[`, then it's likely an IPv6 address. Otherwise, append a `.` if it doesn't already contain one
const hostToHash =
!actualHost.includes('[') && !actualHost.endsWith('.') ? `${actualHost}.` : actualHost;
for (const hash of getHostHashes(hostToHash)) {
if (hash === ruleHostHash) {
return true;
}
Expand Down
7 changes: 6 additions & 1 deletion src/core/server/external_url/external_url_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,14 @@ export class ExternalUrlConfig implements IExternalUrlConfig {
constructor(rawConfig: IExternalUrlConfig) {
this.policy = rawConfig.policy.map((entry) => {
if (entry.host) {
// If the host contains a `[`, then it's likely an IPv6 address. Otherwise, append a `.` if it doesn't already contain one
const hostToHash =
entry.host && !entry.host.includes('[') && !entry.host.endsWith('.')
? `${entry.host}.`
: entry.host;
return {
...entry,
host: createSHA256Hash(entry.host),
host: createSHA256Hash(hostToHash),
};
}
return entry;
Expand Down

0 comments on commit 85c1dfb

Please sign in to comment.