diff --git a/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx b/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx index 0c6296b81e34e..7059de6852a2e 100644 --- a/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx +++ b/web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx @@ -86,7 +86,7 @@ describe('app launcher path is properly formed', () => { jest.spyOn(service, 'getAppDetails').mockResolvedValue({ fqdn: 'grafana.localhost', localClusterName: 'localhost', - publicAddress: 'grafana.localhost' + publicAddress: 'grafana.localhost', }); jest.spyOn(service, 'createAppSession').mockResolvedValue({ cookieValue: 'cookie-value', @@ -284,7 +284,7 @@ describe('fqdn is matched', () => { jest.spyOn(service, 'getAppDetails').mockResolvedValue({ fqdn: returnedFqdn, localClusterName: '', - publicAddress:'', + publicAddress: '', }); jest.spyOn(service, 'createAppSession'); @@ -313,8 +313,8 @@ describe('fqdn is matched', () => { test('not matching FQDN throws error', async () => { jest.spyOn(service, 'getAppDetails').mockResolvedValue({ fqdn: 'different.fqdn', - localClusterName: 'test', - publicAddress:'https://test-app.test.teleport', + localClusterName: 'test.teleport', + publicAddress: 'https://test-app.test.teleport', }); render( @@ -342,7 +342,7 @@ describe('fqdn is matched', () => { jest.spyOn(service, 'getAppDetails').mockResolvedValue({ fqdn: 'test-app.root.teleport', localClusterName: 'root', - publicAddress:'malicious.teleporto', + publicAddress: 'malicious.teleporto', }); render( @@ -369,8 +369,8 @@ describe('fqdn is matched', () => { test('not matching FQDN and public address from another cluster throws error', async () => { jest.spyOn(service, 'getAppDetails').mockResolvedValue({ fqdn: 'test-app.root.teleport', - localClusterName: 'root', - publicAddress:'test-app.leaf.teleport', + localClusterName: 'root.teleport', + publicAddress: 'test-app.leaf.teleport', }); render( @@ -398,7 +398,7 @@ describe('fqdn is matched', () => { jest.spyOn(service, 'getAppDetails').mockResolvedValue({ fqdn: 'test-app.root.teleport', localClusterName: 'root.teleport', - publicAddress:'test-app.leaf.teleport', + publicAddress: 'test-app.leaf.teleport', }); render( @@ -425,7 +425,7 @@ describe('fqdn is matched', () => { jest.spyOn(service, 'getAppDetails').mockResolvedValue({ fqdn: 'invalid.fqdn:3080:3090', localClusterName: '', - publicAddress:'', + publicAddress: '', }); render( diff --git a/web/packages/teleport/src/AppLauncher/AppLauncher.tsx b/web/packages/teleport/src/AppLauncher/AppLauncher.tsx index 5714733170f6f..58420124e08f3 100644 --- a/web/packages/teleport/src/AppLauncher/AppLauncher.tsx +++ b/web/packages/teleport/src/AppLauncher/AppLauncher.tsx @@ -37,6 +37,43 @@ export function AppLauncher() { const queryParams = new URLSearchParams(search); const isRedirectFlow = queryParams.get('required-apps'); + function validateResolvedApp( + params: UrlLauncherParams, + fqdn: string, + localClusterName: string, + publicAddress: string + ) { + // 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(fqdn) == prepareFqdn(params.fqdn)) { + return; + } + + // The resolved FQDN didn't match what we expect, and the app + // belongs to the same cluster that resolved it, this FQDN is not + // legitimate. + if (params.clusterId == localClusterName) { + throw Error(`Failed to match applications with FQDN "${params.fqdn}"`); + } + + // If the resolved FQDN doesn't match what we expect, check the + // expected FQDN against the returned public address only if the + // local cluster that resolved the application is different than + // the cluster that the application resides in. This won't allow + // arbitrary addresses through because the resolved public address + // which isn't attacker controlled is matched against. If the app's + // cluster differs from the cluster that resolved it the FQDN will be + // built with the resolved cluster's name, making this check necessary + // to allow access to apps in leaf clusters. + if (prepareFqdn(publicAddress) !== prepareFqdn(params.fqdn)) { + throw Error(`Failed to match applications with FQDN "${params.fqdn}"`); + } + } + const createAppSession = useCallback(async (params: UrlLauncherParams) => { let fqdn = params.fqdn; const port = location.port ? `:${location.port}` : ''; @@ -56,29 +93,12 @@ export function AppLauncher() { publicAddr: params.publicAddr, arn: params.arn, }); - - // 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)) { - // If the resolved FQDN doesn't match what we expect, check the - // expected FQDN from the returned public address only if the - // local cluster that resolved the application is different than - // the cluster that the application resides in. This won't allow - // arbitrary addresses through because the resolved public address - // which isn't attacker controlled is matched against. - if ( - params.clusterId !== resolvedApp.localClusterName && - prepareFqdn(resolvedApp.publicAddress) !== prepareFqdn(params.fqdn) - ) { - throw Error( - `Failed to match applications with FQDN "${params.fqdn}"` - ); - } - } + validateResolvedApp( + params, + resolvedApp.fqdn, + resolvedApp.localClusterName, + resolvedApp.publicAddress + ); let path = ''; if (queryParams.has('path')) {