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',
+      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',
+      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

@@ -56,14 +56,28 @@ 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}"`);
+        // 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}"`
+          );
+        }
       }
 
       let 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;