Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions server/threads/threadServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,13 @@ function onSocket(listener, options) {
setPortServerMap(options.securePort, { protocol_name: 'TLS', name: getComponentName() });
const SNICallback = createTLSSelector('server', options.mtls);
const tlsConfig = env.get('tls');
socketServer = createSecureSocketServer(
// Own const, NOT the shared `socketServer` binding: a caller registering both ports (MQTT's
// port + securePort) reaches the plain-TCP branch below, which reassigns `socketServer` to
// the 1883 server. The writeMetadata closure below outlives this function, so capturing the
// mutable binding made every secure-port metadata write read `secureContexts` off the plain
// TCP server (undefined) and publish an empty `certificates:` list — the #1998 bug that let
// an SNI-routing proxy (Symphony) fall back to the node certificate on 8883.
const secureSocketServer = createSecureSocketServer(
{
rejectUnauthorized: Boolean(options.mtls?.required),
requestCert: Boolean(options.mtls),
Expand All @@ -508,14 +514,15 @@ function onSocket(listener, options) {
},
listener
);
SNICallback.initialize(socketServer);
socketServer = secureSocketServer;
SNICallback.initialize(secureSocketServer);
// Only opt out of reusePort on macOS, which doesn't reliably support SO_REUSEPORT on all
// socket types (ENOTSUP). Everywhere else, sharing the port lets every worker accept
// connections for this listener (e.g. MQTT), matching how HTTP servers are bound; without
// it only the first worker to bind serves the port and every sibling's listen() fails with
// a silently-swallowed EADDRINUSE.
if (process.platform === 'darwin') socketServer.noReusePort = true;
SERVERS[options.securePort] = socketServer;
if (process.platform === 'darwin') secureSocketServer.noReusePort = true;
SERVERS[options.securePort] = secureSocketServer;

// Create a corresponding Unix Domain Socket mirror for the secure socket
if (env.get(terms.CONFIG_PARAMS.TLS_UNIXDOMAINSOCKETS)) {
Expand All @@ -539,9 +546,9 @@ function onSocket(listener, options) {
SERVERS[udsPath] = udsServer;
httpComponent.registerUdsCleanupPaths(udsPath, yamlPath);

const writeMetadata = () => httpComponent.writeUdsMetadata(yamlPath, options.securePort, socketServer);
const writeMetadata = () => httpComponent.writeUdsMetadata(yamlPath, options.securePort, secureSocketServer);
SNICallback.ready.then(writeMetadata);
socketServer.secureContextsListeners.push(writeMetadata);
secureSocketServer.secureContextsListeners.push(writeMetadata);
}
}
if (options.port) {
Expand Down
81 changes: 81 additions & 0 deletions unitTests/apiTests/mqtt-test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,87 @@ describe('test MQTT connections and commands', function () {
}
});

it('secure-port UDS metadata carries the certificates when the same socket() call also registers a plain TCP port', async function () {
// Regression for #1998's surviving symptom: MQTT registers `{ port, securePort }` in ONE
// server.socket() call. onSocket built the TLS server into the function-scoped `socketServer`
// binding, the metadata-write closure captured that binding, and the plain-TCP branch then
// reassigned it to the port-only server — so every secure-port metadata write read
// `secureContexts` off the TCP server (undefined) and published an EMPTY `certificates:`
// list. A fronting SNI proxy (Symphony on 8883) then served the node certificate for every
// custom-domain SNI, fleet-wide, deterministically. The selector itself was always healthy,
// which is why selector-level tests (which drive createTLSSelector with a pseudo-server and
// never go through onSocket with both ports) missed it four review rounds in a row — this
// test goes through the real wiring and asserts the artifact a proxy actually consumes.
this.timeout(10000);
const { existsSync, readFileSync: readFile, readdirSync, unlinkSync } = await import('node:fs');
const { join } = await import('node:path');
const preexistingServers = { ...SERVERS };
const preexistingPortServer = new Map([...portServer.entries()].map(([key, servers]) => [key, [...servers]]));
const preexistingUds = env_get('tls_unixDomainSockets');
setProperty('tls_unixDomainSockets', true);
const securePort = 28887;
const socketsDir = join(environmentManager.getHdbBasePath(), 'sockets');
try {
// Clear any yaml a crashed prior run left behind — a stale populated file would false-pass
// the poll below even if the current write regressed.
if (existsSync(socketsDir)) {
for (const name of readdirSync(socketsDir).filter((n) => n.includes(`-${securePort}.`))) {
try {
unlinkSync(join(socketsDir, name));
} catch {}
}
}
// Note: like the sibling socket tests above, this leaves the selector's liveReload
// registration in keys.ts's module-global rebuild set — onSocket exposes no teardown.
global.server.socket(() => {}, { port: 21887, securePort });
// The yaml is written when the TLS selector's `.ready` resolves (or on a later rebuild);
// poll for a yaml for our securePort that actually carries certificates.
const deadline = Date.now() + 8000;
let yamlPath;
let content = '';
while (Date.now() < deadline) {
const candidates = existsSync(socketsDir)
? readdirSync(socketsDir).filter((name) => name.endsWith(`-${securePort}.yaml`))
: [];
if (candidates.length > 0) {
yamlPath = join(socketsDir, candidates[0]);
content = readFile(yamlPath, 'utf8');
if (content.includes('BEGIN CERTIFICATE')) break;
}
await delay(100);
}
assert.ok(yamlPath, `a <worker>-${securePort}.yaml should be written to ${socketsDir}`);
assert.ok(
content.includes('BEGIN CERTIFICATE'),
'the secure-port UDS metadata must carry the certificate list even when the same socket() call ' +
'also registered a plain TCP port — an empty `certificates:` list here is exactly what makes an ' +
`SNI-routing proxy fall back to the node certificate (got:\n${content.slice(0, 200)})`
);
} finally {
setProperty('tls_unixDomainSockets', preexistingUds);
// Close the servers this call created (TLS 28887, TCP 21887, and the UDS mirror) before
// dropping them from the registry, so they can't exert timing pressure on later tests in
// this mocha process even if something bound them.
for (const key of Object.keys(SERVERS)) {
if (preexistingServers[key]) continue;
try {
SERVERS[key]?.close?.(() => {});
} catch {}
}
for (const name of existsSync(socketsDir)
? readdirSync(socketsDir).filter((n) => n.includes(`-${securePort}.`))
: []) {
try {
unlinkSync(join(socketsDir, name));
} catch {}
}
for (const key of Object.keys(SERVERS)) delete SERVERS[key];
Object.assign(SERVERS, preexistingServers);
portServer.clear();
for (const [key, servers] of preexistingPortServer) portServer.set(key, servers);
}
});

after(() => {
clientV4?.end();
clientV5?.end();
Expand Down