diff --git a/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx b/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx index 50f854b0db69e..b1293b94ccb3a 100644 --- a/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx +++ b/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx @@ -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'; @@ -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=', @@ -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( + + + + + + ); + + 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( + + + + + + ); + + 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( - + ); - 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( - + ); - 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], + }); +} diff --git a/web/packages/teleport/src/AppLauncher/AppLauncher.tsx b/web/packages/teleport/src/AppLauncher/AppLauncher.tsx index c767b56e64d9f..74fbb8bf262e4 100644 --- a/web/packages/teleport/src/AppLauncher/AppLauncher.tsx +++ b/web/packages/teleport/src/AppLauncher/AppLauncher.tsx @@ -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 = ''; @@ -138,8 +144,30 @@ export function AppLauncherAccessDenied(props: AppLauncherAccessDeniedProps) { return ; } +// 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 @@ -195,3 +223,7 @@ function initiateNewAuthExchange({ window.location.replace(url.toString()); } + +function throwFailedToParseUrlError(err: TypeError) { + throw Error(`Failed to parse URL: ${err.message}`); +}