fix app access regression when the app is on a leaf cluster

lib/web/apps.go

@@ -149,10 +149,17 @@ func (h *Handler) clusterAppsGet(w http.ResponseWriter, r *http.Request, p httpr
 type GetAppDetailsRequest ResolveAppParams
 
 type GetAppDetailsResponse struct {
-	// FQDN is application FQDN.
+	// FQDN is application FQDN. See utils.AssembleAppFQDN for details
+	// on how the FQDN is formed.
 	FQDN string `json:"fqdn"`
 	// RequiredAppFQDNs is a list of required app fqdn
 	RequiredAppFQDNs []string `json:"requiredAppFQDNs"`
+	// LocalClusterName is the name of the cluster that resolved the
+	// application, which may belong to another cluster.
+	LocalClusterName string `json:"localClusterName"`
+	// PublicAddress is the public address of the application, which
+	// may be different than the FQDN in some cases.
+	PublicAddress string `json:"publicAddress"`
 }
 
 // getAppDetails resolves the input params to a known application and returns
@@ -179,7 +186,9 @@ func (h *Handler) getAppDetails(w http.ResponseWriter, r *http.Request, p httpro
 	}
 
 	resp := &GetAppDetailsResponse{
-		FQDN: result.FQDN,
+		FQDN:             result.FQDN,
+		LocalClusterName: h.auth.clusterName,
+		PublicAddress:    result.App.GetPublicAddr(),
 	}
 
 	requiredAppNames := result.App.GetRequiredAppNames()

web/packages/teleport/src/AppLauncher/AppLauncher.test.tsx

@@ -85,6 +85,8 @@ describe('app launcher path is properly formed', () => {
     jest.spyOn(api, 'post').mockResolvedValue({});
     jest.spyOn(service, 'getAppDetails').mockResolvedValue({
       fqdn: 'grafana.localhost',
+      localClusterName: 'localhost',
+      publicAddress: 'grafana.localhost',
     });
     jest.spyOn(service, 'createAppSession').mockResolvedValue({
       cookieValue: 'cookie-value',
@@ -281,6 +283,8 @@ describe('fqdn is matched', () => {
     }) => {
       jest.spyOn(service, 'getAppDetails').mockResolvedValue({
         fqdn: returnedFqdn,
+        localClusterName: '',
+        publicAddress: '',
       });
       jest.spyOn(service, 'createAppSession');
 
@@ -309,6 +313,8 @@ describe('fqdn is matched', () => {
   test('not matching FQDN throws error', async () => {
     jest.spyOn(service, 'getAppDetails').mockResolvedValue({
       fqdn: 'different.fqdn',
+      localClusterName: 'test.teleport',
+      publicAddress: 'https://test-app.test.teleport',
     });
 
     render(
@@ -332,9 +338,94 @@ describe('fqdn is matched', () => {
     expect(window.location.replace).not.toHaveBeenCalled();
   });
 
+  test('not matching FQDN and random public addr from another cluster throws error', async () => {
+    jest.spyOn(service, 'getAppDetails').mockResolvedValue({
+      fqdn: 'test-app.root.teleport',
+      localClusterName: 'root',
+      publicAddress: 'malicious.teleporto',
+    });
+
+    render(
+      <Router
+        history={createMockHistory(
+          'test-app.leaf.teleport:443/leaf.teleport/test-app.leaf.teleport:443?state=ABC'
+        )}
+      >
+        <Route path={cfg.routes.appLauncher}>
+          <AppLauncher />
+        </Route>
+      </Router>
+    );
+
+    await screen.findByText(/access denied/i);
+    expect(
+      screen.getByText(
+        /failed to match applications with FQDN "test-app.leaf.teleport:443"/i
+      )
+    ).toBeInTheDocument();
+    expect(window.location.replace).not.toHaveBeenCalled();
+  });
+
+  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.teleport',
+      publicAddress: 'test-app.leaf.teleport',
+    });
+
+    render(
+      <Router
+        history={createMockHistory(
+          'malicious:443/leaf.teleport/test-app.leaf.teleport:443?state=ABC'
+        )}
+      >
+        <Route path={cfg.routes.appLauncher}>
+          <AppLauncher />
+        </Route>
+      </Router>
+    );
+
+    await screen.findByText(/access denied/i);
+    expect(
+      screen.getByText(
+        /failed to match applications with FQDN "malicious:443"/i
+      )
+    ).toBeInTheDocument();
+    expect(window.location.replace).not.toHaveBeenCalled();
+  });
+
+  test('matching FQDN from another cluster is allowed', async () => {
+    jest.spyOn(service, 'getAppDetails').mockResolvedValue({
+      fqdn: 'test-app.root.teleport',
+      localClusterName: 'root.teleport',
+      publicAddress: 'test-app.leaf.teleport',
+    });
+
+    render(
+      <Router
+        history={createMockHistory(
+          'test-app.leaf.teleport:443/leaf.teleport/test-app.leaf.teleport:443?state=ABC'
+        )}
+      >
+        <Route path={cfg.routes.appLauncher}>
+          <AppLauncher />
+        </Route>
+      </Router>
+    );
+
+    await waitFor(() => {
+      expect(service.createAppSession).toHaveBeenCalled();
+    });
+
+    await waitFor(() => expect(window.location.replace).toHaveBeenCalled());
+    expect(screen.queryByText(/access denied/i)).not.toBeInTheDocument();
+  });
+
   test('invalid URL when constructing a new URL with a malformed FQDN', async () => {
     jest.spyOn(service, 'getAppDetails').mockResolvedValue({
       fqdn: 'invalid.fqdn:3080:3090',
+      localClusterName: '',
+      publicAddress: '',
     });
 
     render(

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,15 +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)) {
-        throw Error(`Failed to match applications with FQDN "${params.fqdn}"`);
-      }
+      validateResolvedApp(
+        params,
+        resolvedApp.fqdn,
+        resolvedApp.localClusterName,
+        resolvedApp.publicAddress
+      );
 
       let path = '';
       if (queryParams.has('path')) {

web/packages/teleport/src/services/apps/apps.ts

@@ -78,13 +78,17 @@ const service = {
     return api.get(cfg.getAppDetailsUrl(params)).then(json => ({
       fqdn: json.fqdn,
       requiredAppFQDNs: json.requiredAppFQDNs,
+      localClusterName: json.localClusterName,
+      publicAddress: json.publicAddress,
     }));
   },
 };
 
 type AppDetails = {
   fqdn: string;
   requiredAppFQDNs?: string[];
+  localClusterName: string;
+  publicAddress: string;
 };
 
 export default service;