From bd70e1cf4c77d9e17537420c7eafb1e89bf2a998 Mon Sep 17 00:00:00 2001 From: Matt Gilman Date: Tue, 30 Jun 2026 13:31:26 -0400 Subject: [PATCH] NIFI-16059: validate connector postMessage origin against the host origin instead of the entity-controlled configurationUrl (CWE-346) --- .../connector-message-host.service.spec.ts | 107 +++++++++--------- .../service/connector-message-host.service.ts | 39 ++++--- .../connector-configure.component.spec.ts | 41 ++++++- .../connector-configure.component.ts | 8 +- .../connector-detail.component.spec.ts | 1 - .../connector-detail.component.ts | 1 - 6 files changed, 116 insertions(+), 81 deletions(-) diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/service/connector-message-host.service.spec.ts b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/service/connector-message-host.service.spec.ts index 91d7dee335a6..6758a45c0d6f 100644 --- a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/service/connector-message-host.service.spec.ts +++ b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/service/connector-message-host.service.spec.ts @@ -22,7 +22,8 @@ import { CONNECTOR_MESSAGE_NAMESPACE } from '@nifi/shared'; import { ConnectorMessageHost, ConnectorMessageHostOptions } from './connector-message-host.service'; describe('ConnectorMessageHost', () => { - const TRUSTED_ORIGIN = 'http://localhost:4200'; + // The host trusts only the application's own origin; tests dispatch from it. + const TRUSTED_ORIGIN = window.location.origin; function createMockDestroyRef(): DestroyRef { const callbacks: Array<() => void> = []; @@ -49,7 +50,7 @@ describe('ConnectorMessageHost', () => { }).compileComponents(); const service = TestBed.inject(ConnectorMessageHost); - const router = TestBed.inject(Router) as { navigate: ReturnType }; + const router = TestBed.inject(Router) as unknown as { navigate: ReturnType }; const destroyRef = createMockDestroyRef(); @@ -58,8 +59,7 @@ describe('ConnectorMessageHost', () => { function createDefaultOptions(destroyRef: DestroyRef): ConnectorMessageHostOptions { return { - destroyRef, - expectedOrigin: TRUSTED_ORIGIN + destroyRef }; } @@ -145,7 +145,7 @@ describe('ConnectorMessageHost', () => { expect(navigate).not.toHaveBeenCalled(); }); - it('should reject messages with empty origin when expectedOrigin is set', async () => { + it('should reject messages with empty origin', async () => { const { service, navigate, destroyRef } = await setup(); service.startListening(createDefaultOptions(destroyRef)); @@ -252,7 +252,7 @@ describe('ConnectorMessageHost', () => { expect(onConnectorUiReady.mock.calls[0][0]).toBeInstanceOf(MessageEvent); }); - it('should not invoke onConnectorUiReady when origin does not match expectedOrigin', async () => { + it('should not invoke onConnectorUiReady when origin does not match the trusted origin', async () => { const onConnectorUiReady = vi.fn(); const { service, destroyRef } = await setup(); @@ -350,23 +350,43 @@ describe('ConnectorMessageHost', () => { }); }); - describe('extractOrigin', () => { - it('should extract origin from a valid URL', () => { - expect(ConnectorMessageHost.extractOrigin('https://connector-ui.example.com/config?id=123')).toBe( - 'https://connector-ui.example.com' + describe('origin decoupling (FLOW-11982)', () => { + it('should drop messages whose origin matches an attacker-controlled configurationUrl', async () => { + const { service, navigate, destroyRef } = await setup(); + service.startListening(createDefaultOptions(destroyRef)); + + // Simulate a connector entity whose configurationUrl points at an + // attacker origin. Even though that URL would load the iframe, the + // host must not trust messages claiming to come from it. + const attackerConfigurationUrl = 'https://attacker.example.com/custom-config'; + const attackerOrigin = new URL(attackerConfigurationUrl).origin; + + dispatchMessageEvent( + { + namespace: CONNECTOR_MESSAGE_NAMESPACE, + type: 'navigate-to-connector-listing', + payload: { connectorId: 'c1' } + }, + attackerOrigin ); - }); - it('should extract origin including port', () => { - expect(ConnectorMessageHost.extractOrigin('http://localhost:4200/wizard')).toBe('http://localhost:4200'); + expect(navigate).not.toHaveBeenCalled(); }); - it('should return empty string for invalid URL', () => { - expect(ConnectorMessageHost.extractOrigin('not-a-url')).toBe(''); - }); + it('should accept messages from the application origin', async () => { + const { service, navigate, destroyRef } = await setup(); + service.startListening(createDefaultOptions(destroyRef)); - it('should return empty string for empty string', () => { - expect(ConnectorMessageHost.extractOrigin('')).toBe(''); + dispatchMessageEvent( + { + namespace: CONNECTOR_MESSAGE_NAMESPACE, + type: 'navigate-to-connector-listing', + payload: { connectorId: 'c1' } + }, + window.location.origin + ); + + expect(navigate).toHaveBeenCalledWith(['/connectors', 'c1']); }); }); @@ -395,47 +415,28 @@ describe('ConnectorMessageHost', () => { it('should tear down previous listener when startListening is called again', async () => { const { service, navigate, destroyRef } = await setup(); - const firstOrigin = 'http://first-origin.example.com'; - const secondOrigin = 'http://second-origin.example.com'; + // Start first listener (simulates initial route activation) + service.startListening(createDefaultOptions(destroyRef)); - service.startListening({ - destroyRef, - expectedOrigin: firstOrigin + dispatchMessageEvent({ + namespace: CONNECTOR_MESSAGE_NAMESPACE, + type: 'navigate-to-connector-listing', + payload: { connectorId: 'c1' } }); - - dispatchMessageEvent( - { - namespace: CONNECTOR_MESSAGE_NAMESPACE, - type: 'navigate-to-connector-listing', - payload: { connectorId: 'c1' } - }, - firstOrigin - ); expect(navigate).toHaveBeenCalledTimes(1); - service.startListening({ - destroyRef, - expectedOrigin: secondOrigin - }); + // Start second listener (simulates route param change). The previous + // subscription must be torn down so handlers do not accumulate. + service.startListening(createDefaultOptions(destroyRef)); - dispatchMessageEvent( - { - namespace: CONNECTOR_MESSAGE_NAMESPACE, - type: 'navigate-to-connector-listing', - payload: { connectorId: 'c2' } - }, - firstOrigin - ); - expect(navigate).toHaveBeenCalledTimes(1); + dispatchMessageEvent({ + namespace: CONNECTOR_MESSAGE_NAMESPACE, + type: 'navigate-to-connector-listing', + payload: { connectorId: 'c2' } + }); - dispatchMessageEvent( - { - namespace: CONNECTOR_MESSAGE_NAMESPACE, - type: 'navigate-to-connector-listing', - payload: { connectorId: 'c3' } - }, - secondOrigin - ); + // Exactly one additional handler invocation, proving the old + // subscription was replaced rather than duplicated. expect(navigate).toHaveBeenCalledTimes(2); }); diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/service/connector-message-host.service.ts b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/service/connector-message-host.service.ts index 67ad465804b1..ca08f5386416 100644 --- a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/service/connector-message-host.service.ts +++ b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/service/connector-message-host.service.ts @@ -16,6 +16,7 @@ */ import { DestroyRef, Injectable, inject } from '@angular/core'; +import { DOCUMENT } from '@angular/common'; import { Router } from '@angular/router'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { Subscription, fromEvent } from 'rxjs'; @@ -26,12 +27,6 @@ export interface ConnectorMessageHostOptions { /** Angular DestroyRef for automatic subscription cleanup. */ destroyRef: DestroyRef; - /** - * The expected origin of the iframe (e.g. `'https://connector-ui.example.com'`). - * Messages whose `event.origin` does not match are silently dropped. - */ - expectedOrigin: string; - /** * Optional lazy getter for the HTMLIFrameElement hosting the custom UI. * When provided, the host verifies that `event.source` matches the @@ -68,20 +63,22 @@ export interface ConnectorMessageHostOptions { }) export class ConnectorMessageHost { private router = inject(Router); + private readonly document = inject(DOCUMENT); private activeSubscription: Subscription | null = null; /** - * Extract the origin from a URL string for use with `expectedOrigin`. - * Returns an empty string when the URL cannot be parsed, which will - * cause all origin checks to fail-closed (no messages accepted). + * The only origin trusted for inbound and outbound connector messages. + * + * Connector custom UIs are always served by the same NiFi web server that + * serves this application, so the iframe is same-origin with the parent. + * Trusting the application's own origin -- rather than an origin derived + * from the entity-controlled `configurationUrl` / `detailsUrl` -- prevents + * a connector entity from attesting to its own message origin (CWE-346). + * + * The origin is immutable for the application's lifetime, so it is captured + * once at construction rather than recomputed on every access. */ - static extractOrigin(url: string): string { - try { - return new URL(url).origin; - } catch { - return ''; - } - } + readonly trustedOrigin = this.document.location.origin; /** * Begin listening for postMessage events. @@ -91,7 +88,7 @@ export class ConnectorMessageHost { * accumulating duplicate subscriptions. * * Only messages that pass all of the following checks are processed: - * 1. `event.origin` matches `expectedOrigin` + * 1. `event.origin` matches the application's own origin (`trustedOrigin`) * 2. `event.source` matches the iframe's `contentWindow` (when `iframeElement` is provided) * 3. `isConnectorMessage()` type guard passes (namespace + type validation) * @@ -131,7 +128,13 @@ export class ConnectorMessageHost { * Validate that the MessageEvent originates from the expected iframe. */ private isAllowedSource(event: MessageEvent, options: ConnectorMessageHostOptions): boolean { - if (event.origin !== options.expectedOrigin) { + // Origin must match the application's own origin exactly. Connector + // custom UIs are served same-origin by the backend (see + // ConnectorResource#buildCustomUiUrl), so a mismatch means the message + // is not from our embedded UI and is dropped (fail-closed). A custom UI + // whose configurationUrl/detailsUrl were ever cross-origin would render + // but be unable to communicate. + if (event.origin !== this.trustedOrigin) { return false; } diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-configure/connector-configure.component.spec.ts b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-configure/connector-configure.component.spec.ts index b47326d9430f..882a30edfb8f 100644 --- a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-configure/connector-configure.component.spec.ts +++ b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-configure/connector-configure.component.spec.ts @@ -19,7 +19,13 @@ import { ComponentFixture, TestBed } from '@angular/core/testing'; import { ConnectorConfigure } from './connector-configure.component'; import { MockStore, provideMockStore } from '@ngrx/store/testing'; import { DomSanitizer } from '@angular/platform-browser'; -import { ConnectorConfigurationService, ConnectorEntity, ConnectorWizard, SystemTokensService } from '@nifi/shared'; +import { + CONNECTOR_MESSAGE_NAMESPACE, + ConnectorConfigurationService, + ConnectorEntity, + ConnectorWizard, + SystemTokensService +} from '@nifi/shared'; import { MockComponent } from 'ng-mocks'; import { Navigation } from '../../../../ui/common/navigation/navigation.component'; import { of, throwError } from 'rxjs'; @@ -155,7 +161,8 @@ describe('ConnectorConfigure', () => { const mockConnectorMessageHost = { startListening: vi.fn(), - stopListening: vi.fn() + stopListening: vi.fn(), + trustedOrigin: window.location.origin }; const mockClusterConnectionService = { @@ -497,7 +504,6 @@ describe('ConnectorConfigure', () => { expect(connectorMessageHost.startListening).toHaveBeenCalledTimes(1); expect(connectorMessageHost.startListening).toHaveBeenCalledWith( expect.objectContaining({ - expectedOrigin: 'http://localhost:4200', iframeElement: expect.any(Function) }) ); @@ -545,5 +551,34 @@ describe('ConnectorConfigure', () => { expect(connectorMessageHost.stopListening).toHaveBeenCalled(); }); + + it('should post the disconnected-node-acknowledgment to the host trusted origin, not the entity configurationUrl origin', () => { + connectorConfigurationService.getConnector.mockReturnValue(of(mockConnectorWithCustomUrl)); + fixture = TestBed.createComponent(ConnectorConfigure); + component = fixture.componentInstance; + component.ngOnInit(); + + // Stub the iframe so the outbound postMessage has a target window. + const postMessageSpy = vi.fn(); + (component as unknown as { iframeRef: () => unknown }).iframeRef = () => ({ + nativeElement: { contentWindow: { postMessage: postMessageSpy } } + }); + + ( + component as unknown as { postDisconnectedNodeAcknowledgmentToChild(): void } + ).postDisconnectedNodeAcknowledgmentToChild(); + + expect(postMessageSpy).toHaveBeenCalledTimes(1); + const [message, targetOrigin] = postMessageSpy.mock.calls[0]; + // targetOrigin must be the application's own origin, never an origin + // derived from the entity-controlled configurationUrl. + expect(targetOrigin).toBe(window.location.origin); + expect(message).toEqual( + expect.objectContaining({ + namespace: CONNECTOR_MESSAGE_NAMESPACE, + type: 'disconnected-node-acknowledgment' + }) + ); + }); }); }); diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-configure/connector-configure.component.ts b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-configure/connector-configure.component.ts index 576943586910..cd9b33b8b384 100644 --- a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-configure/connector-configure.component.ts +++ b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-configure/connector-configure.component.ts @@ -163,7 +163,6 @@ export class ConnectorConfigure implements OnInit { this.connectorMessageHost.startListening({ destroyRef: this.destroyRef, - expectedOrigin: ConnectorMessageHost.extractOrigin(connector.component.configurationUrl), iframeElement: () => this.iframeRef()?.nativeElement, onConnectorUiReady: () => { this.childConnectorUiReady = true; @@ -180,10 +179,9 @@ export class ConnectorConfigure implements OnInit { if (!iframe?.contentWindow || !configurationUrl) { return; } - const targetOrigin = ConnectorMessageHost.extractOrigin(configurationUrl); - if (!targetOrigin) { - return; - } + // Target the application's own origin (where the same-origin custom UI + // is served), not an origin derived from the entity-controlled URL. + const targetOrigin = this.connectorMessageHost.trustedOrigin; const message: ParentToConnectorMessage = { namespace: CONNECTOR_MESSAGE_NAMESPACE, type: 'disconnected-node-acknowledgment', diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-detail/connector-detail.component.spec.ts b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-detail/connector-detail.component.spec.ts index 9b2a7852824b..21339ef229c3 100644 --- a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-detail/connector-detail.component.spec.ts +++ b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-detail/connector-detail.component.spec.ts @@ -455,7 +455,6 @@ describe('ConnectorDetail', () => { expect(connectorMessageHost.startListening).toHaveBeenCalledTimes(1); expect(connectorMessageHost.startListening).toHaveBeenCalledWith( expect.objectContaining({ - expectedOrigin: 'http://localhost:4200', iframeElement: expect.any(Function) }) ); diff --git a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-detail/connector-detail.component.ts b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-detail/connector-detail.component.ts index 89ac1e1de69b..96d29e7a7631 100644 --- a/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-detail/connector-detail.component.ts +++ b/nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/ui/connector-detail/connector-detail.component.ts @@ -108,7 +108,6 @@ export class ConnectorDetail implements OnInit { this.connectorMessageHost.startListening({ destroyRef: this.destroyRef, - expectedOrigin: ConnectorMessageHost.extractOrigin(connector.component.detailsUrl), iframeElement: () => this.iframeRef()?.nativeElement }); }