Skip to content

[v14] fix regression that denied access to launch some Apps #45151

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.

Sign up for GitHub

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

Merged
capnspacehook merged 3 commits into from
Aug 6, 2024
Merged

[v14] fix regression that denied access to launch some Apps #45151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
827c560
fix regression that denied access to launch some Apps
Aug 2, 2024
7fb1cb0
Fix test, and wrap new URL with its own try catch block
kimlisa Aug 6, 2024
4f4bfd4
improve URL parsing fail error message
Aug 6, 2024
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
271 changes: 242 additions & 29 deletions web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import React from 'react';
import { render, waitFor } from 'design/utils/testing';
import { render, waitFor, screen } from 'design/utils/testing';
import { createMemoryHistory } from 'history';
import { Router } from 'react-router';

Expand All @@ -26,7 +26,11 @@ import service from 'teleport/services/apps';

import { AppLauncher } from './AppLauncher';

const testCases: { name: string; path: string; expectedPath: string }[] = [
const launcherPathTestCases: {
name: string;
path: string;
expectedPath: string;
}[] = [
{
name: 'no state and no path',
path: '?path=',
Expand Down Expand Up @@ -95,54 +99,263 @@ describe('app launcher path is properly formed', () => {
assignMock.mockClear();
});

test.each(testCases)('$name', async ({ path: query, expectedPath }) => {
const launcherPath = `/web/launch/grafana.localhost${query}`;
const mockHistory = createMemoryHistory({
initialEntries: [launcherPath],
test.each(launcherPathTestCases)(
'$name',
async ({ path: query, expectedPath }) => {
render(
<Router history={createMockHistory(`grafana.localhost${query}`)}>
<Route path={cfg.routes.appLauncher}>
<AppLauncher />
</Route>
</Router>
);

await waitFor(() =>
expect(window.location.replace).toHaveBeenCalledWith(
`https://grafana.localhost/${expectedPath}`
)
);
expect(screen.queryByText(/access denied/i)).not.toBeInTheDocument();
}
);
});

const appSessionTestCases: {
name: string;
path: string;
returnedFqdn: string;
expectedFqdn: string;
expectedPublicAddr: string;
expectedArn: string;
}[] = [
{
name: 'ARN URL',
path: 'test-app.test.teleport/test.teleport/test-app.test.teleport/arn:aws:iam::joe123:role%2FEC2FullAccess?state=ABC',
returnedFqdn: 'test-app.test.teleport',
expectedFqdn: 'test-app.test.teleport',
expectedPublicAddr: 'test-app.test.teleport',
expectedArn: 'arn:aws:iam::joe123:role/EC2FullAccess',
},
{
name: 'uppercase resolved FQDN',
path: 'test-app.test.teleport/test.teleport/test-app.test.teleport?state=ABC',
returnedFqdn: 'TEST-APP.test.teleport',
expectedFqdn: 'test-app.test.teleport',
expectedPublicAddr: 'test-app.test.teleport',
expectedArn: undefined,
},
{
name: 'uppercase public addr',
path: 'test-app.test.teleport/test.teleport/TEST-APP.test.teleport?state=ABC',
returnedFqdn: 'test-app.test.teleport',
expectedFqdn: 'test-app.test.teleport',
expectedPublicAddr: 'TEST-APP.test.teleport',
expectedArn: undefined,
},
{
name: 'uppercase FQDN',
path: 'TEST-APP.test.teleport/test.teleport/test-app.test.teleport?state=ABC',
returnedFqdn: 'test-app.test.teleport',
expectedFqdn: 'test-app.test.teleport',
expectedPublicAddr: 'test-app.test.teleport',
expectedArn: undefined,
},
{
name: 'uppercase resolved FQDN, public addr',
path: 'test-app.test.teleport/test.teleport/TEST-APP.test.teleport?state=ABC',
returnedFqdn: 'TEST-APP.test.teleport',
expectedFqdn: 'test-app.test.teleport',
expectedPublicAddr: 'TEST-APP.test.teleport',
expectedArn: undefined,
},
{
name: 'uppercase resolved FQDN,FQDN',
path: 'TEST-APP.test.teleport/test.teleport/test-app.test.teleport?state=ABC',
returnedFqdn: 'TEST-APP.test.teleport',
expectedFqdn: 'test-app.test.teleport',
expectedPublicAddr: 'test-app.test.teleport',
expectedArn: undefined,
},
{
name: 'uppercase public addr, FQDN',
path: 'TEST-APP.test.teleport/test.teleport/TEST-APP.test.teleport?state=ABC',
returnedFqdn: 'test-app.test.teleport',
expectedFqdn: 'test-app.test.teleport',
expectedPublicAddr: 'TEST-APP.test.teleport',
expectedArn: undefined,
},
{
name: 'uppercase FQDN, resolved FQDN, public addr',
path: 'TEST-APP.test.teleport/test.teleport/TEST-APP.test.teleport?state=ABC',
returnedFqdn: 'TEST-APP.test.teleport',
expectedFqdn: 'test-app.test.teleport',
expectedPublicAddr: 'TEST-APP.test.teleport',
expectedArn: undefined,
},
{
name: 'public addr with port',
path: 'test-app.test.teleport/test.teleport/test-app.test.teleport:443?state=ABC',
returnedFqdn: 'test-app.test.teleport',
expectedFqdn: 'test-app.test.teleport',
expectedPublicAddr: 'test-app.test.teleport',
expectedArn: undefined,
},
{
name: 'FQDN with port',
path: 'test-app.test.teleport:443/test.teleport/test-app.test.teleport?state=ABC',
returnedFqdn: 'test-app.test.teleport',
expectedFqdn: 'test-app.test.teleport:443',
expectedPublicAddr: 'test-app.test.teleport',
expectedArn: undefined,
},
{
name: 'resolved FQDN with port',
path: 'test-app.test.teleport/test.teleport/test-app.test.teleport?state=ABC',
returnedFqdn: 'test-app.test.teleport:443',
expectedFqdn: 'test-app.test.teleport',
expectedPublicAddr: 'test-app.test.teleport',
expectedArn: undefined,
},
{
name: 'FQDN, public addr with port',
path: 'test-app.test.teleport:443/test.teleport/test-app.test.teleport:443?state=ABC',
returnedFqdn: 'test-app.test.teleport',
expectedFqdn: 'test-app.test.teleport:443',
expectedPublicAddr: 'test-app.test.teleport',
expectedArn: undefined,
},
{
name: 'FQDN, resolved FQDN with port',
path: 'test-app.test.teleport:443/test.teleport/test-app.test.teleport?state=ABC',
returnedFqdn: 'test-app.test.teleport:443',
expectedFqdn: 'test-app.test.teleport:443',
expectedPublicAddr: 'test-app.test.teleport',
expectedArn: undefined,
},
{
name: 'public addr, resolved FQDN with port',
path: 'test-app.test.teleport/test.teleport/test-app.test.teleport:443?state=ABC',
returnedFqdn: 'test-app.test.teleport:443',
expectedFqdn: 'test-app.test.teleport',
expectedPublicAddr: 'test-app.test.teleport',
expectedArn: undefined,
},
{
name: 'FQDN, public addr, resolved FQDN with port',
path: 'test-app.test.teleport:443/test.teleport/test-app.test.teleport:443?state=ABC',
returnedFqdn: 'test-app.test.teleport:443',
expectedFqdn: 'test-app.test.teleport:443',
expectedPublicAddr: 'test-app.test.teleport',
expectedArn: undefined,
},
];

describe('fqdn is matched', () => {
const realLocation = window.location;
const assignMock = jest.fn();

beforeEach(() => {
global.fetch = jest.fn(() => Promise.resolve({})) as jest.Mock;
jest.spyOn(api, 'get').mockResolvedValue({});
jest.spyOn(api, 'post').mockResolvedValue({});

delete window.location;
window.location = { ...realLocation, replace: assignMock };
});

afterEach(() => {
window.location = realLocation;
assignMock.mockClear();
});

test.each(appSessionTestCases)(
'$name',
async ({
path,
returnedFqdn,
expectedFqdn,
expectedPublicAddr,
expectedArn,
}) => {
jest.spyOn(service, 'getAppFqdn').mockResolvedValue({
fqdn: returnedFqdn,
});
jest.spyOn(service, 'createAppSession');

render(
<Router history={createMockHistory(path)}>
<Route path={cfg.routes.appLauncher}>
<AppLauncher />
</Route>
</Router>
);

await waitFor(() => {
expect(service.createAppSession).toHaveBeenCalledWith({
fqdn: expectedFqdn,
clusterId: 'test.teleport',
publicAddr: expectedPublicAddr,
arn: expectedArn,
});
});

await waitFor(() => expect(window.location.replace).toHaveBeenCalled());
expect(screen.queryByText(/access denied/i)).not.toBeInTheDocument();
}
);

test('not matching FQDN throws error', async () => {
jest.spyOn(service, 'getAppFqdn').mockResolvedValue({
fqdn: 'different.fqdn',
});

render(
<Router history={mockHistory}>
<Router
history={createMockHistory(
'test-app.test.teleport:443/test.teleport/test-app.test.teleport:443?state=ABC'
)}
>
<Route path={cfg.routes.appLauncher}>
<AppLauncher />
</Route>
</Router>
);

await waitFor(() =>
expect(window.location.replace).toHaveBeenCalledWith(
`https://grafana.localhost/${expectedPath}`
await screen.findByText(/access denied/i);
expect(
screen.getByText(
/failed to match applications with FQDN "test-app.test.teleport:443"/i
)
);
).toBeInTheDocument();
expect(window.location.replace).not.toHaveBeenCalled();
});

test('arn is url decoded', async () => {
test('invalid URL when constructing a new URL with a malformed FQDN', async () => {
jest.spyOn(service, 'getAppFqdn').mockResolvedValue({
fqdn: 'test-app.test.teleport',
});
jest.spyOn(service, 'createAppSession');

const launcherPath =
'/web/launch/test-app.test.teleport/test.teleport/test-app.test.teleport/arn:aws:iam::joe123:role%2FEC2FullAccess?state=ABC';
const mockHistory = createMemoryHistory({
initialEntries: [launcherPath],
fqdn: 'invalid.fqdn:3080:3090',
});

render(
<Router history={mockHistory}>
<Router
history={createMockHistory(
'test-app.test.teleport:443/test.teleport/test-app.test.teleport:443?state=ABC'
)}
>
<Route path={cfg.routes.appLauncher}>
<AppLauncher />
</Route>
</Router>
);

await waitFor(() => {
expect(service.createAppSession).toHaveBeenCalledWith({
fqdn: 'test-app.test.teleport',
clusterId: 'test.teleport',
publicAddr: 'test-app.test.teleport',
arn: 'arn:aws:iam::joe123:role/EC2FullAccess',
});
});
await screen.findByText(/access denied/i);
expect(screen.getByText(/Failed to parse URL:/i)).toBeInTheDocument();
expect(window.location.replace).not.toHaveBeenCalled();
});
});

function createMockHistory(path: string) {
const launcherPath = `/web/launch/${path}`;
return createMemoryHistory({
initialEntries: [launcherPath],
});
}
38 changes: 35 additions & 3 deletions web/packages/teleport/src/AppLauncher/AppLauncher.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,14 @@ export function AppLauncher() {
publicAddr: params.publicAddr,
arn: params.arn,
});
if (resolvedApp.fqdn !== params.fqdn) {
throw Error(`Failed to match applications with FQDN ${params.fqdn}`);
// Because the ports are stripped from the FQDNs before they are
// compared, an attacker can pass a FQDN with a different port than
// what the app's public address is configured with and have Teleport
// redirect to the public address with an arbitrary port. But because
// the attacker can't control what domain is redirected to this has
// a low risk factor.
if (prepareFqdn(resolvedApp.fqdn) !== prepareFqdn(params.fqdn)) {
throw Error(`Failed to match applications with FQDN "${params.fqdn}"`);
}

let path = '';
Expand Down Expand Up @@ -138,8 +144,30 @@ export function AppLauncherAccessDenied(props: AppLauncherAccessDeniedProps) {
return <AccessDenied message={props.statusText} />;
}

// prepareFqdn removes the port from the FQDN if it has one and ensures
// the FQDN is lowercase. This is to prevent issues matching the
// resolved fqdn with the one that was passed. Apps generally aren't
// supposed to have a port in the public address but some integrations
// create apps that do. The FQDN is also lowercased to prevent
// issues with case sensitivity.
function prepareFqdn(fqdn: string) {
try {
const fqdnUrl = new URL('https://' + fqdn);
fqdnUrl.port = '';
// The returned FQDN will have a scheme added to it, but that's
// fine because we're just using it to compare the FQDNs.
return fqdnUrl.toString().toLowerCase();
} catch (err) {
throwFailedToParseUrlError(err);
}
}

function getXTeleportAuthUrl({ fqdn, port }: { fqdn: string; port: string }) {
return new URL(`https://${fqdn}${port}/x-teleport-auth`);
try {
return new URL(`https://${fqdn}${port}/x-teleport-auth`);
} catch (err) {
throwFailedToParseUrlError(err);
}
}

// initiateNewAuthExchange is the first step to gaining access to an
Expand Down Expand Up @@ -195,3 +223,7 @@ function initiateNewAuthExchange({

window.location.replace(url.toString());
}

function throwFailedToParseUrlError(err: TypeError) {
throw Error(`Failed to parse URL: ${err.message}`);
}