openvibers/openrestream Commits

OpenRe: fixes from the pre-launch security review, and openre.stream launches (cutover B0 (a)) (#25)

OP OpenVibers committed 9 Oct 2026 · a7f2b2be35 · 10 files, +95 −17

- DNS rebinding: a restream output connects to the address its host was checked against. rtmp:// and srt:// carry
  the address, and RTMP keeps the name in -rtmp_tcurl. rtmps:// keeps the name, because TLS needs it and a rebound
  name only gets a ClientHello. The destination test probes the checked address too, so it is no port-scan oracle.
- Sign-in's next refuses control characters and backslashes ("/<TAB>/evil.com" left the site).
- The per-person write limits cover the signed-in pages' form posts (new stream, test, rotation), not only /api/v1.
  nginx limits location / too.
- X-Frame-Options DENY and frame-ancestors 'none': a framed form carries the session and its CSRF token.
- test/launch-hardening.test.js. The README's launch rule records the review and the launch.

Changes

ddiff --git a/README.md b/README.md
iindex 8f37cf6..467ba12 100644
---- a/README.md
++++ b/README.md
@@@ -2,7 +2,7 @@
> Ingest and restream: stream definitions, keys, sessions, transport workers, outputs and output health.
--**Status:** alpha (roadmap Wave 7). Tested end to end with real RTMP. Deployed internally on `openvibe-ovh` since 2026-09-23 (API and coordinator on 127.0.0.1:4500; RTMP ingest public on port 1936 at `ingest.openre.stream` since the cutover runbook's phase A on 2026-09-23; Live wired with `OPENRE_URL` and an `openre.session.*` subscription; one rehearsal broadcast passed), but not launched: no broadcaster uses it, since every Live slot is still ingested by Live until its per-slot cutover (`docs/cutover.md` phase B). The domain keeps its placeholder page on OpenVibe.Sites until the launch rule below is met.
++**Status:** alpha (roadmap Wave 7). Tested end to end with real RTMP. Deployed internally on `openvibe-ovh` since 2026-09-23 (API and coordinator on 127.0.0.1:4500; RTMP ingest public on port 1936 at `ingest.openre.stream` since the cutover runbook's phase A on 2026-09-23; Live wired with `OPENRE_URL` and an `openre.session.*` subscription; one rehearsal broadcast passed). **openre.stream serves its own site since 2026-10-09** (plan T11, cutover step B0 (a)): sign in with OpenVibe, create a stream with its RTMP key, and send it to other channels. No Live slot is ingested here yet: every slot is still ingested by Live until its per-slot cutover (`docs/cutover.md` phase B).
**Domain:** `openre.stream` (UI + API), `ingest.openre.stream` (RTMP, DNS only)
**Plan:** OpenVibe End-to-End Realignment & Implementation Plan, revision 3 (20 Sep 2026), §10, §10.5, §15.10; ADR-009 (binding), ADR-004, ADR-006, ADR-007.
**License:** AGPL-3.0 (same as every OpenVibe service).
@@@ -276,8 +276,10 @@ JavaScript; real persistence and end-to-end workflows; capability and event regi
sitemap/robots/feed behaviour; acceptance tests proving the advertised functionality. Today the
runtime, identity integration, server-rendered routes, persistence, capability registration (contracts v0.16.0),
the migration strategy and the acceptance tests are in place, and the service is deployed
--internally with its public ingest port open. An independent security review and the first slot
--cutover are not.
++internally with its public ingest port open. The independent security review was done on 2026-10-09: its
++fixes pin each restream output and destination test to the address that was checked (DNS rebinding), refuse
++control characters in sign-in's `next`, rate-limit the signed-in pages' posts and deny framing
++(`test/launch-hardening.test.js`). The site launched the same day; the first slot cutover is still to come.
---
ddiff --git a/deploy/nginx/openre.stream.conf b/deploy/nginx/openre.stream.conf
iindex 756479a..4ab998c 100644
---- a/deploy/nginx/openre.stream.conf
++++ b/deploy/nginx/openre.stream.conf
@@@ -58,6 +58,7 @@ server {
add_header X-Content-Type-Options nosniff always;
add_header Referrer-Policy strict-origin-when-cross-origin always;
add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always;
++ add_header X-Frame-Options DENY always;
# Live playback: no buffering, long reads. Viewers reconnect across an API deploy; the
# ingest (a separate worker process) is never affected.
@@@ -94,6 +95,7 @@ server {
location = /metrics { return 404; }
location / {
++ limit_req zone=openre_api burst=40 nodelay;
proxy_pass http://127.0.0.1:4500;
proxy_http_version 1.1;
proxy_set_header Host $host;
ddiff --git a/server/api/actor-limits.js b/server/api/actor-limits.js
iindex fe6d77b..2a11956 100644
---- a/server/api/actor-limits.js
++++ b/server/api/actor-limits.js
@@@ -1,5 +1,5 @@
/**
-- * Per-actor limits on the ingest API's writes (roadmap WS-R task 4; openvibe-sdk/limits).
++ * Per-actor limits on the ingest API's writes and the signed-in pages' form posts (roadmap WS-R task 4; openvibe-sdk/limits).
*
* A signed-in person (req.caller.kind === 'user') is counted by subject. Service callers are not: Live manages every
* streamer's ingest with its service token, so counting svc:live as one caller would refuse the whole site; their
@@@ -14,10 +14,14 @@ const { createActorLimiter, createValkeyLimitStore } = require('openvibe-sdk/lim
const num = (v, d) => { const n = parseInt(v, 10); return Number.isFinite(n) && n > 0 ? n : d; };
--/** [name, method regex, path regex (relative to /api/v1), { minute, hour }]; the first match wins. */
++/**
++ * [name, method regex, path regex, { minute, hour }]; the first match wins. The paths are the API's (relative to /api/v1)
++ * and the signed-in pages' (the same limiter guards both, so one person has one budget): the page rotates a key at
++ * /streams/:id/rotate.
++ */
const ROUTES = [
// A rotation invalidates the key every encoder uses.
-- ['openre.key.rotate', /^POST$/, /^\/streams\/[^/]+\/keys\/rotate$/, { minute: 5, hour: 20 }],
++ ['openre.key.rotate', /^POST$/, /^\/streams\/[^/]+\/(keys\/)?rotate$/, { minute: 5, hour: 20 }],
// A test opens a connection to someone else's server.
['openre.destination.test', /^POST$/, /^\/destinations\/[^/]+\/test$/, { minute: 10, hour: 100 }],
['openre.stream.create', /^POST$/, /^\/streams\/?$/, { minute: 10, hour: 100 }],
ddiff --git a/server/app.js b/server/app.js
iindex c5994d9..c9c210c 100644
---- a/server/app.js
++++ b/server/app.js
@@@ -29,6 +29,9 @@ function createApp({ rt, auth, keys, log = console, fetchImpl }) {
app.use((req, res, next) => {
res.setHeader('X-Content-Type-Options', 'nosniff');
res.setHeader('Referrer-Policy', 'strict-origin-when-cross-origin');
++ // No page is meant to be framed: a framed form carries the session and its CSRF token (clickjacking).
++ res.setHeader('X-Frame-Options', 'DENY');
++ res.setHeader('Content-Security-Policy', "frame-ancestors 'none'");
next();
});
app.use(cookieParser());
@@@ -152,7 +155,7 @@ function createApp({ rt, auth, keys, log = console, fetchImpl }) {
app.use('/auth', createSsoRoutes({ config, auth, fetchImpl }));
// Crawl and machine-readability artifacts (robots.txt, sitemap.xml, llms.txt): openvibe-shared/seo.
app.use('/', createDiscoveryRoutes({ config }));
-- app.use('/', createUiRouter({ rt, auth }));
++ app.use('/', createUiRouter({ rt, auth, limits: actorLimits }));
app.use((req, res) => contracts.http.sendProblem(res, 404, 'openre.not_found', { detail: `no route ${req.method} ${req.path}`, ctx: req.ov }));
// eslint-disable-next-line no-unused-vars
ddiff --git a/server/auth/sso.js b/server/auth/sso.js
iindex c6a5d3d..5bd645b 100644
---- a/server/auth/sso.js
++++ b/server/auth/sso.js
@@@ -21,7 +21,9 @@ const STATE = 'ov_oauth_state';
const NEXT = 'ov_oauth_next';
function sanitizeNext(next) {
-- if (typeof next === 'string' && /^\/(?!\/|\\)/.test(next)) return next;
++ // Browsers drop tabs and newlines and read \ as /, so "/<TAB>/evil.com" or "/\\evil.com" would leave the site.
++ if (typeof next === 'string' && /[\u0000-\u001f\u007f\\]/.test(next)) return '/';
++ if (typeof next === 'string' && /^\/(?!\/)/.test(next)) return next;
return '/';
}
ddiff --git a/server/destination-test.js b/server/destination-test.js
iindex 6064c1e..bf0d905 100644
---- a/server/destination-test.js
++++ b/server/destination-test.js
@@@ -32,7 +32,10 @@ async function testDestination(dest, { allowPrivate = false, lookup, probe = tcp
checks.push({ check: 'connect', ok: true, detail: 'SRT is UDP; the receiver is only reached when an output runs' });
} else {
const port = Number(v.url.port) || (v.url.protocol === 'rtmps:' ? 443 : 1935);
-- const p = await probe(v.host, port);
++ // Connect to the address that was just checked, never the name again: a name with a zero TTL could answer
++ // a public address to the check and a loopback one to the connect (DNS rebinding, a port-scan oracle).
++ const target = r.addresses[0] || v.host;
++ const p = await probe(target, port);
checks.push({ check: 'connect', ok: p.ok, detail: p.ok ? `TCP ${v.host}:${port} answered in ${p.ms} ms` : p.error });
}
return { ok: checks.every(c => c.ok), checks };
ddiff --git a/server/ui/routes.js b/server/ui/routes.js
iindex 3671516..28eda6a 100644
---- a/server/ui/routes.js
++++ b/server/ui/routes.js
@@@ -27,11 +27,13 @@ const pill = (state) => `<span class="pill ${esc(state)}">${esc(state)}</span>`;
function csrfFor(token) { return crypto.createHash('sha256').update(`openre-csrf:${token}`).digest('hex').slice(0, 32); }
--function createUiRouter({ rt, auth }) {
++function createUiRouter({ rt, auth, limits = null }) {
const { store, config } = rt;
const router = express.Router();
router.use(express.urlencoded({ extended: false, limit: '64kb' }));
router.use(auth.middleware({ services: false }));
++ // The API's per-person limits count these form posts too (a test, a new stream, a rotation): one budget per person.
++ if (limits) router.use(limits);
const page = (req, res, o, status = 200) => res.status(status).type('html').send(renderPage({ ...o, user: req.caller.kind === 'user' ? req.caller : null, config }));
const csrfField = (req) => `<input type="hidden" name="_csrf" value="${esc(csrfFor(req.caller.token))}">`;
ddiff --git a/test/launch-hardening.test.js b/test/launch-hardening.test.js
nnew file mode 100644
iindex 0000000..236d956
---- /dev/null
++++ b/test/launch-hardening.test.js
@@@ -0,0 +1,38 @@
++'use strict';
++/**
++ * Pre-launch review fixes (2026-10-09):
++ * - a restream output connects to the address its host was checked against (DNS rebinding): rtmp:// and srt:// carry
++ * the address, RTMP keeps the name in -rtmp_tcurl; rtmps:// keeps the name (TLS needs it);
++ * - a destination test connects to the checked address, never the name again;
++ * - sign-in's next refuses control characters and backslashes (browsers drop tabs and read \ as /).
++ */
++const assert = require('assert');
++const { pinDestUrl } = require('../workers/restream/ffmpeg-args');
++const { testDestination } = require('../server/destination-test');
++const { sanitizeNext } = require('../server/auth/sso');
++
++(async () => {
++ assert.deepStrictEqual(pinDestUrl('rtmp://live.example.com/app/KEY123', '93.184.216.34'),
++ { url: 'rtmp://93.184.216.34/app/KEY123', extra: ['-rtmp_tcurl', 'rtmp://live.example.com:1935/app'] });
++ assert.deepStrictEqual(pinDestUrl('rtmp://live.example.com:1940/live2/k', '2606:2800:220:1::1'),
++ { url: 'rtmp://[2606:2800:220:1::1]:1940/live2/k', extra: ['-rtmp_tcurl', 'rtmp://live.example.com:1940/live2'] });
++ assert.deepStrictEqual(pinDestUrl('srt://srt.example.com:9000?streamid=k&mode=caller', '93.184.216.34'),
++ { url: 'srt://93.184.216.34:9000?streamid=k&mode=caller', extra: [] });
++ assert.deepStrictEqual(pinDestUrl('rtmps://live.twitch.tv/app/k', '93.184.216.34'), { url: 'rtmps://live.twitch.tv/app/k', extra: [] }, 'rtmps keeps its name');
++ assert.deepStrictEqual(pinDestUrl('rtmp://live.example.com/app/k', undefined), { url: 'rtmp://live.example.com/app/k', extra: [] }, 'nothing to pin (private hosts allowed)');
++
++ // The test probe gets the address the lookup answered, not the name a second lookup could rebind.
++ const probed = [];
++ const r = await testDestination({ server_url: 'rtmp://rebind.example.com/app', has_stream_key: true }, {
++ lookup: async () => [{ address: '93.184.216.34', family: 4 }],
++ probe: async (host, port) => { probed.push(`${host}:${port}`); return { ok: true, ms: 1 }; },
++ });
++ assert.ok(r.ok, JSON.stringify(r));
++ assert.deepStrictEqual(probed, ['93.184.216.34:1935']);
++
++ assert.strictEqual(sanitizeNext('/streams/std_1'), '/streams/std_1');
++ for (const bad of ['/\t/evil.com', '/\n/evil.com', '/\\evil.com', '/a\\b', '//evil.com', 'https://evil.com', '', null]) {
++ assert.strictEqual(sanitizeNext(bad), '/', `${JSON.stringify(bad)} stays on the site`);
++ }
++ console.log('launch hardening: outputs and tests pinned to the checked address; next stays on the site');
++})().catch((err) => { console.error(err); process.exit(1); });
ddiff --git a/workers/restream/ffmpeg-args.js b/workers/restream/ffmpeg-args.js
iindex 66fd441..5aa69fe 100644
---- a/workers/restream/ffmpeg-args.js
++++ b/workers/restream/ffmpeg-args.js
@@@ -50,6 +50,26 @@ function buildSrtUrl(base, dest) {
return u.toString();
}
++/**
++ * Pin an output to the address its host was checked against (DNS rebinding guard): ffmpeg would otherwise resolve the
++ * name again when it connects, and a name with a zero TTL could answer a loopback or private address then. rtmp:// and
++ * srt:// carry no name past the connect, so the host becomes the address; RTMP keeps the name in its tcUrl
++ * (`-rtmp_tcurl`), as a server that routes by it expects. rtmps:// is left as it is: TLS needs the name (SNI), and
++ * a rebound name only ever receives a TLS ClientHello. Returns { url, extra } (extra goes before the output URL).
++ */
++function pinDestUrl(destUrl, address) {
++ const s = String(destUrl || '');
++ const m = /^(rtmp|srt):\/\/(\[[^\]]+\]|[^/:?#]+)(:\d+)?([/?].*)?$/i.exec(s);
++ if (!address || !m) return { url: s, extra: [] };
++ const [, proto, , port, rest = ''] = m;
++ const host = address.includes(':') ? `[${address}]` : address;
++ const url = `${proto}://${host}${port || ''}${rest}`;
++ if (proto.toLowerCase() !== 'rtmp') return { url, extra: [] };
++ // ffmpeg's own tcUrl is proto://host:port/app; the app is the path before the stream key.
++ const app = rest.slice(1, Math.max(rest.lastIndexOf('/'), 0));
++ return { url, extra: ['-rtmp_tcurl', `rtmp://${m[2]}${port || ':1935'}/${app}`] };
++}
++
function outputArgs(destUrl) {
const common = ['-muxdelay', '0', '-muxpreload', '0', '-flush_packets', '1', '-max_muxing_queue_size', '4096'];
if (isSrtUrl(destUrl)) return [...common, '-f', 'mpegts', '-mpegts_flags', '+resend_headers', destUrl];
@@@ -190,5 +210,5 @@ function redactUrl(value) {
module.exports = {
QUALITY_PRESETS, PLATFORM_DEFAULT_PRESET, ENCODER_PRESETS, SRT_DEFAULT_LATENCY_MS,
-- isSrtUrl, buildDestUrl, buildSrtUrl, outputArgs, rtmpCopyArgs, jsmpegArgs, webrtcArgs, rtpInputArgs, withProgress, resolvePreset, customOverrides, encodingArgs, friendlyError, redactUrl, redactText,
++ isSrtUrl, buildDestUrl, buildSrtUrl, pinDestUrl, outputArgs, rtmpCopyArgs, jsmpegArgs, webrtcArgs, rtpInputArgs, withProgress, resolvePreset, customOverrides, encodingArgs, friendlyError, redactUrl, redactText,
};
ddiff --git a/workers/restream/output-runner.js b/workers/restream/output-runner.js
iindex d53d493..095f55e 100644
---- a/workers/restream/output-runner.js
++++ b/workers/restream/output-runner.js
@@@ -11,7 +11,7 @@ const fs = require('fs');
const os = require('os');
const path = require('path');
const dgram = require('dgram');
--const { rtmpCopyArgs, jsmpegArgs, webrtcArgs, withProgress, buildDestUrl, resolvePreset, customOverrides, friendlyError, redactUrl, redactText } = require('./ffmpeg-args');
++const { rtmpCopyArgs, jsmpegArgs, webrtcArgs, withProgress, buildDestUrl, pinDestUrl, resolvePreset, customOverrides, friendlyError, redactUrl, redactText } = require('./ffmpeg-args');
const { validateDestinationUrl, checkResolvedHost } = require('../../server/destination-url');
/** A free even UDP port (the WebRTC source's ffmpeg takes it and its +1 for RTCP). */
@@@ -96,7 +96,7 @@ class OutputRunner {
// No source yet (the producer has not appeared): retry with the usual backoff.
if (!prep.ok) return this.scheduleRestart(0, prep.error);
}
-- this.spawn(destUrl);
++ this.spawn(destUrl, resolved.addresses[0]);
}
/**
@@@ -130,12 +130,14 @@ class OutputRunner {
if (this.sdpPath) { try { fs.unlinkSync(this.sdpPath); } catch { /* gone */ } this.sdpPath = null; }
}
-- spawn(destUrl) {
++ spawn(destUrl, address = null) {
++ const pinned = pinDestUrl(destUrl, address);
const args = withProgress(this.source === 'jsmpeg'
-- ? jsmpegArgs(this.inputUrl, destUrl, this.preset || resolvePreset({}), this.overrides ? { overrides: this.overrides } : {})
++ ? jsmpegArgs(this.inputUrl, pinned.url, this.preset || resolvePreset({}), this.overrides ? { overrides: this.overrides } : {})
: this.source === 'webrtc'
-- ? webrtcArgs(this.sdpPath, destUrl, this.preset || resolvePreset({}), { hasAudio: this.hasAudio, overrides: this.overrides || {} })
-- : rtmpCopyArgs(this.inputUrl, destUrl));
++ ? webrtcArgs(this.sdpPath, pinned.url, this.preset || resolvePreset({}), { hasAudio: this.hasAudio, overrides: this.overrides || {} })
++ : rtmpCopyArgs(this.inputUrl, pinned.url));
++ if (pinned.extra.length) args.splice(args.lastIndexOf(pinned.url), 0, ...pinned.extra);
const rtmps = destUrl.startsWith('rtmps://');
const bin = rtmps && this.o.ffmpegOpenSslPath && fs.existsSync(this.o.ffmpegOpenSslPath) ? this.o.ffmpegOpenSslPath : this.o.ffmpegPath;
this.note('info', `starting ffmpeg → ${redactUrl(destUrl)}${bin !== this.o.ffmpegPath ? ' (openssl build)' : ''}`);