From c447b9b503dab6bad1bd8a5e912e664afe9a6b70 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 18 Aug 2026 13:28:43 +1000 Subject: [PATCH 01/10] video: allow five streams per entry, not three An aircraft can carry more than three cameras, and one port carries one stream, so the cap was the limit on how many a single entry could proxy. The three fields that hold per-slot state -- video_ports, video_flags and video_rtmp_path -- all sit in the middle of the record, so none of them could simply grow: every field after them would shift, every record already on disk would be misparsed, and an older binary would read garbage. The append-only contract at the top of keydb.h exists to make that unnecessary, so slots 3 and 4 are carried in new fields appended after reserved[], and accessors join the two halves. A record written before they existed zero-extends into them, which reads as two unused slots, so nothing needs converting and the live database keeps working. video_flags could not be widened for the same reason, and it was already full: three slot bytes plus the entry-wide byte is exactly 32 bits. A fourth slot byte at shift 24 would have landed on the entry options -- which is what happened first time round, and is why there is now a test that sets slots 3 and 4 to 0xFF and checks the audio flag survives. The record grows 344 -> 456 bytes. Also fixes video_port_count() tripping over a short list, which callers that build a KeyEntry by hand were relying on not happening. The README's video section had been pasted in three times: the edit that added it replaced on "## Building", which matches three headings. Only one copy remains. --- README.md | 108 +------------------------- keydb.h | 104 ++++++++++++++++++++++--- keydb_lib.py | 103 ++++++++++++++++++------ supportproxy.cpp | 20 +++-- tests/test_video_ports.py | 30 +++---- tests/test_video_schema.py | 77 +++++++++++++++++- tests/webadmin/test_log_routes.py | 2 +- tests/webadmin/test_video_ui.py | 34 ++++---- video.cpp | 9 ++- videoview.cpp | 2 +- webadmin/__init__.py | 6 ++ webadmin/forms.py | 35 ++++++++- webadmin/logs.py | 4 +- webadmin/templates/_video_fields.html | 2 +- 14 files changed, 344 insertions(+), 192 deletions(-) diff --git a/README.md b/README.md index 624c991..93c145a 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ Video is deliberately independent of the MAVLink session: it survives a telemetry dropout, and with a publish password it needs no MAVLink at all. -**Ports.** Up to three per entry, allocated by an admin from the web UI +**Ports.** Up to five per entry, allocated by an admin from the web UI (suggested from 40001). Each carries one stream, on TCP and UDP. **These are public listening ports and must be open in the firewall.** @@ -122,59 +122,6 @@ source venv/bin/activate pip install pymavlink ``` -### Video - -Optional, off unless an entry has it enabled. A user points a camera at -one of their entry's video ports and any number of ground stations can -watch, with the same NAT traversal and per-entry credentials the MAVLink -side already provides. Recordings land beside the tlogs under -`logs///` and are covered by the same retention. - -Video is deliberately independent of the MAVLink session: it survives a -telemetry dropout, and with a publish password it needs no MAVLink at -all. - -**Ports.** Up to three per entry, allocated by an admin from the web UI -(suggested from 40001). Each carries one stream, on TCP and UDP. -**These are public listening ports and must be open in the firewall.** - -**Publishing.** - -| Transport | Credential | -|---|---| -| MPEG-TS over UDP | none possible — see below | -| RTSP | `?pw=` on the request URI | -| RTMP | `?pw=` on the stream key, e.g. `FPV?pw=secret` | - -Plain MPEG-TS over UDP has nowhere to carry a password, so it is -admitted on the MAVLink-session path only: a publisher is accepted when -a MAVLink session for the entry was seen from the same address within -the grace window. On a non-bidi entry *any* datagram latches the user -side, so a scanner between flights can become the authorised address and -the aircraft's video is then refused until the grace expires. **Entries -used for video should set `bidi_sign` or a publish password.** - -**Watching.** In the browser from the web UI, or outside it with the -`ffplay`/`vlc` command the page offers. The browser player needs H.264: -Chrome and Firefox will not decode HEVC in Media Source Extensions on -desktop Linux, and nothing here transcodes. - -**Disk.** Video has its own budget, separate from telemetry, so a busy -camera can never evict a user's tlogs. Set it per entry in the web UI; -a free-space floor stops recording before the disk fills. - -**Log rotation.** The daemon's own log is not rotated by default. -Install the supplied config once, as root: - -```bash -sudo install -m 644 scripts/supportproxy.logrotate \ - /etc/logrotate.d/supportproxy -``` - -It uses `copytruncate`, which is required rather than preferred when the -daemon's stdout is a file systemd holds open — see the comments in that -file. - ## Building SupportProxy ```bash @@ -317,59 +264,6 @@ netstat -ln | grep ":1000[0-9]" SupportProxy can also be run using Docker for easier deployment and management. -### Video - -Optional, off unless an entry has it enabled. A user points a camera at -one of their entry's video ports and any number of ground stations can -watch, with the same NAT traversal and per-entry credentials the MAVLink -side already provides. Recordings land beside the tlogs under -`logs///` and are covered by the same retention. - -Video is deliberately independent of the MAVLink session: it survives a -telemetry dropout, and with a publish password it needs no MAVLink at -all. - -**Ports.** Up to three per entry, allocated by an admin from the web UI -(suggested from 40001). Each carries one stream, on TCP and UDP. -**These are public listening ports and must be open in the firewall.** - -**Publishing.** - -| Transport | Credential | -|---|---| -| MPEG-TS over UDP | none possible — see below | -| RTSP | `?pw=` on the request URI | -| RTMP | `?pw=` on the stream key, e.g. `FPV?pw=secret` | - -Plain MPEG-TS over UDP has nowhere to carry a password, so it is -admitted on the MAVLink-session path only: a publisher is accepted when -a MAVLink session for the entry was seen from the same address within -the grace window. On a non-bidi entry *any* datagram latches the user -side, so a scanner between flights can become the authorised address and -the aircraft's video is then refused until the grace expires. **Entries -used for video should set `bidi_sign` or a publish password.** - -**Watching.** In the browser from the web UI, or outside it with the -`ffplay`/`vlc` command the page offers. The browser player needs H.264: -Chrome and Firefox will not decode HEVC in Media Source Extensions on -desktop Linux, and nothing here transcodes. - -**Disk.** Video has its own budget, separate from telemetry, so a busy -camera can never evict a user's tlogs. Set it per entry in the web UI; -a free-space floor stops recording before the disk fills. - -**Log rotation.** The daemon's own log is not rotated by default. -Install the supplied config once, as root: - -```bash -sudo install -m 644 scripts/supportproxy.logrotate \ - /etc/logrotate.d/supportproxy -``` - -It uses `copytruncate`, which is required rather than preferred when the -daemon's stdout is a file systemd holds open — see the comments in that -file. - ## Building the Docker Image ```bash diff --git a/keydb.h b/keydb.h index 0b12579..e67e96d 100644 --- a/keydb.h +++ b/keydb.h @@ -36,7 +36,20 @@ #define KEY_FLAG_USE_TZ (1u << 4) // name logs with tz_offset_hours; else server local #define KEY_FLAG_VIDEO (1u << 5) // video proxying enabled for this entry -#define KEY_MAX_VIDEO_PORTS 3 +#define KEY_MAX_VIDEO_PORTS 5 + +/* + How many slots the original layout carried inline. + + Slots 0..2 live in video_ports/video_flags/video_rtmp_path where they + always did; 3 and 4 are in fields appended after reserved[]. The split + is not elegant, but those three are middle fields: growing any of them + shifts every field after it, so every record already on disk would be + misparsed and an older binary would read garbage. The append-only + contract at the top of this file is what makes that unnecessary, and + the accessors below keep the seam out of callers' way. + */ +#define KEY_VIDEO_PORTS_INLINE 3 /* KeyEntry.video_flags: one byte of options per video slot, plus a @@ -48,6 +61,10 @@ bits 8-15 slot 1 bits 16-23 slot 2 bits 24-31 entry-wide + + That is the whole word, which is why slots 3 and 4 have their own + video_flags_hi (byte 0 = slot 3, byte 1 = slot 4). Widening this field + was not an option: it sits in the middle of the record. */ #define VIDEO_SLOT_BITS 8 #define VIDEO_SLOT_SHIFT(slot) ((slot) * VIDEO_SLOT_BITS) @@ -64,12 +81,28 @@ // from an aircraft, and dropping it // keeps the muxed TS video-only. -static inline uint32_t video_slot_opts(uint32_t video_flags, unsigned slot) +/* + Per-slot options, from whichever word holds the slot. Callers that + keep their own copy of the two words (supportproxy's listen_port) use + this directly; everything else uses the KeyEntry overload below. + */ +static inline uint32_t video_slot_opts_split(uint32_t lo, uint32_t hi, + unsigned slot) { if (slot >= KEY_MAX_VIDEO_PORTS) { return 0; } - return (video_flags >> VIDEO_SLOT_SHIFT(slot)) & 0xFFu; + if (slot < KEY_VIDEO_PORTS_INLINE) { + return (lo >> VIDEO_SLOT_SHIFT(slot)) & 0xFFu; + } + return (hi >> VIDEO_SLOT_SHIFT(slot - KEY_VIDEO_PORTS_INLINE)) & 0xFFu; +} + +static inline uint32_t video_slot_opts_set(uint32_t word, unsigned index, + uint32_t opts) +{ + const uint32_t mask = 0xFFu << VIDEO_SLOT_SHIFT(index); + return (word & ~mask) | ((opts & 0xFFu) << VIDEO_SLOT_SHIFT(index)); } static inline uint32_t video_entry_opts(uint32_t video_flags) @@ -95,7 +128,7 @@ struct KeyEntry { float log_retention_days; // tlog + bin; 0.0 = forever; fractional values allowed for tests uint32_t fc_sysid; // 0 = match any; otherwise only monitor packets from this MAVLink sysid (binlog reboot detection) float tz_offset_hours; // log naming: GMT offset in hours (fractional allowed), used only when KEY_FLAG_USE_TZ is set - uint32_t video_ports[KEY_MAX_VIDEO_PORTS]; // 0 = slot unused + uint32_t video_ports[KEY_VIDEO_PORTS_INLINE]; // 0 = slot unused uint32_t video_flags; // VIDEO_SLOT_* / VIDEO_OPT_*, see above uint8_t video_viewer_key[32]; // sha256(viewer password); all-zero = open uint8_t video_publish_key[32]; // sha256(publish password); all-zero = the @@ -112,24 +145,71 @@ struct KeyEntry { parsed here now (videortmp.cpp), so the app and stream are read off the wire. Set, only that path is admitted on the slot. */ - char video_rtmp_path[KEY_MAX_VIDEO_PORTS][32]; + char video_rtmp_path[KEY_VIDEO_PORTS_INLINE][32]; uint32_t reserved[12]; + + /* + Slots 3 and 4. Appended rather than grown into the arrays above, + which are middle fields -- see KEY_VIDEO_PORTS_INLINE. A record + written before these existed zero-extends, which reads as two + unused slots, so nothing needs converting. + */ + uint32_t video_ports_hi[KEY_MAX_VIDEO_PORTS - KEY_VIDEO_PORTS_INLINE]; + uint32_t video_flags_hi; // byte 0 = slot 3, byte 1 = slot 4 + char video_rtmp_path_hi[KEY_MAX_VIDEO_PORTS - KEY_VIDEO_PORTS_INLINE][32]; + uint32_t reserved2[9]; }; +/* Unified views over the split. Slot indices are 0..KEY_MAX_VIDEO_PORTS-1. */ +static inline uint32_t video_port_of(const struct KeyEntry &ke, unsigned slot) +{ + if (slot < KEY_VIDEO_PORTS_INLINE) { + return ke.video_ports[slot]; + } + if (slot < KEY_MAX_VIDEO_PORTS) { + return ke.video_ports_hi[slot - KEY_VIDEO_PORTS_INLINE]; + } + return 0; +} + +static inline const char *video_rtmp_path_of(const struct KeyEntry &ke, + unsigned slot) +{ + if (slot < KEY_VIDEO_PORTS_INLINE) { + return ke.video_rtmp_path[slot]; + } + if (slot < KEY_MAX_VIDEO_PORTS) { + return ke.video_rtmp_path_hi[slot - KEY_VIDEO_PORTS_INLINE]; + } + return ""; +} + +static inline size_t video_rtmp_path_size(void) +{ + return sizeof(((struct KeyEntry *)nullptr)->video_rtmp_path[0]); +} + +static inline uint32_t video_slot_opts_of(const struct KeyEntry &ke, + unsigned slot) +{ + return video_slot_opts_split(ke.video_flags, ke.video_flags_hi, slot); +} + /* The on-disk layout is an ABI shared with keydb_lib.py's PACK_FORMAT - (" 248 -> 344 as video fields were added. That is + The record grew 168 -> 248 -> 344 -> 456 as video fields were added, + the last step to carry video slots 3 and 4. That is allowed by the append-only contract at the top of this file: readers zero-extend a short record and writers preserve any tail they don't understand, so old and new binaries interoperate in both directions. */ static_assert(sizeof(int) == 4, "KeyEntry ABI assumes 32-bit int"); static_assert(sizeof(float) == 4, "KeyEntry ABI assumes 32-bit float"); -static_assert(sizeof(struct KeyEntry) == 344, "KeyEntry size changed"); +static_assert(sizeof(struct KeyEntry) == 456, "KeyEntry size changed"); static_assert(offsetof(struct KeyEntry, magic) == 0, "KeyEntry layout"); static_assert(offsetof(struct KeyEntry, timestamp) == 8, "KeyEntry layout"); static_assert(offsetof(struct KeyEntry, secret_key) == 16, "KeyEntry layout"); @@ -148,13 +228,19 @@ static_assert(offsetof(struct KeyEntry, video_viewer_key) == 128, "KeyEntry layo static_assert(offsetof(struct KeyEntry, video_publish_key) == 160, "KeyEntry layout"); static_assert(offsetof(struct KeyEntry, video_quota_mb) == 192, "KeyEntry layout"); static_assert(offsetof(struct KeyEntry, video_mav_grace_s) == 196, "KeyEntry layout"); +// Slots 3 and 4 start after every field the 344-byte record had, so an +// old record zero-extends into them and an old writer preserves them. +static_assert(offsetof(struct KeyEntry, video_ports_hi) == 344, "KeyEntry layout"); +static_assert(offsetof(struct KeyEntry, video_flags_hi) == 352, "KeyEntry layout"); +static_assert(offsetof(struct KeyEntry, video_rtmp_path_hi) == 356, "KeyEntry layout"); +static_assert(offsetof(struct KeyEntry, reserved2) == 420, "KeyEntry layout"); static_assert(offsetof(struct KeyEntry, video_rtmp_path) == 200, "KeyEntry layout"); static_assert(sizeof(((struct KeyEntry *)nullptr)->video_rtmp_path) == 96, "KeyEntry layout"); static_assert(offsetof(struct KeyEntry, reserved) == 296, "KeyEntry layout"); // No implicit tail padding, so appending a field trips the size assert. -static_assert(offsetof(struct KeyEntry, reserved) + 12*sizeof(uint32_t) +static_assert(offsetof(struct KeyEntry, reserved2) + 9*sizeof(uint32_t) == sizeof(struct KeyEntry), "KeyEntry must have no tail padding"); // KEYENTRY_MIN_SIZE is the pre-flags layout: everything through name[]. static_assert(KEYENTRY_MIN_SIZE == offsetof(struct KeyEntry, flags), diff --git a/keydb_lib.py b/keydb_lib.py index b09c150..2d4226b 100644 --- a/keydb_lib.py +++ b/keydb_lib.py @@ -37,12 +37,12 @@ # server-local naming (the flag, not the value, decides whether the offset # is used), needing no conversion. KEYENTRY_MIN_SIZE = 96 -PACK_FORMAT = "> (slot * VIDEO_SLOT_BITS)) & 0xFF + return (video_flags >> (index * VIDEO_SLOT_BITS)) & 0xFF -def video_set_slot_opts(video_flags, slot, opts): - """Return video_flags with slot's option byte replaced.""" - if not 0 <= slot < MAX_VIDEO_PORTS: - raise ValueError("slot out of range: %r" % (slot,)) - shift = slot * VIDEO_SLOT_BITS +def video_set_slot_opts(video_flags, index, opts): + """Return the word with the option byte at `index` replaced.""" + if not 0 <= index < VIDEO_SLOTS_PER_WORD: + raise ValueError("slot index out of range: %r" % (index,)) + shift = index * VIDEO_SLOT_BITS return (video_flags & ~(0xFF << shift)) | ((opts & 0xFF) << shift) @@ -200,12 +217,14 @@ def __init__(self, port2): self.tz_offset_hours = 0.0 self.video_ports = [0] * MAX_VIDEO_PORTS self.video_flags = 0 + self.video_flags_hi = 0 self.video_viewer_key = bytearray(32) self.video_publish_key = bytearray(32) self.video_quota_mb = 0 self.video_mav_grace_s = 0 self.video_rtmp_path = [''] * MAX_VIDEO_PORTS self.reserved = [0] * RESERVED_WORDS + self.reserved2 = [0] * RESERVED2_WORDS self.port2 = port2 # opaque trailing bytes from a record written by a future schema self._tail = b'' @@ -214,6 +233,8 @@ def pack(self): name = self.name.encode('UTF-8').ljust(32, b'\x00')[:32] reserved = list(self.reserved) + [0] * (RESERVED_WORDS - len(self.reserved)) vports = list(self.video_ports) + [0] * (MAX_VIDEO_PORTS - len(self.video_ports)) + reserved2 = (list(self.reserved2) + + [0] * (RESERVED2_WORDS - len(self.reserved2))) body = struct.pack(PACK_FORMAT, self.magic, self.timestamp, bytes(self.secret_key), self.port1, self.connections, self.count1, @@ -221,15 +242,21 @@ def pack(self): self.log_retention_days, self.fc_sysid, self.tz_offset_hours, - *vports[:MAX_VIDEO_PORTS], - self.video_flags, + *vports[:VIDEO_PORTS_INLINE], + self.video_flags & 0xFFFFFFFF, bytes(self.video_viewer_key), bytes(self.video_publish_key), self.video_quota_mb, self.video_mav_grace_s, *[self._rtmp_bytes(i) - for i in range(MAX_VIDEO_PORTS)], - *reserved[:RESERVED_WORDS]) + for i in range(VIDEO_PORTS_INLINE)], + *reserved[:RESERVED_WORDS], + *vports[VIDEO_PORTS_INLINE:MAX_VIDEO_PORTS], + self.video_flags_hi & 0xFFFFFFFF, + *[self._rtmp_bytes(i) + for i in range(VIDEO_PORTS_INLINE, + MAX_VIDEO_PORTS)], + *reserved2[:RESERVED2_WORDS]) return body + self._tail def unpack(self, data): @@ -248,16 +275,27 @@ def unpack(self, data): self.flags, self.log_retention_days, self.fc_sysid, self.tz_offset_hours) = unpacked[:12] n = 12 - self.video_ports = list(unpacked[n:n + MAX_VIDEO_PORTS]) - n += MAX_VIDEO_PORTS + self.video_ports = list(unpacked[n:n + VIDEO_PORTS_INLINE]) + n += VIDEO_PORTS_INLINE (self.video_flags, viewer_key, publish_key, self.video_quota_mb, self.video_mav_grace_s) = unpacked[n:n + 5] n += 5 self.video_rtmp_path = [ b.decode('utf-8', errors='ignore').rstrip('\0') - for b in unpacked[n:n + MAX_VIDEO_PORTS]] - n += MAX_VIDEO_PORTS + for b in unpacked[n:n + VIDEO_PORTS_INLINE]] + n += VIDEO_PORTS_INLINE self.reserved = list(unpacked[n:n + RESERVED_WORDS]) + n += RESERVED_WORDS + n_hi = MAX_VIDEO_PORTS - VIDEO_PORTS_INLINE + self.video_ports += list(unpacked[n:n + n_hi]) + n += n_hi + self.video_flags_hi = unpacked[n] + n += 1 + self.video_rtmp_path += [ + b.decode('utf-8', errors='ignore').rstrip('\0') + for b in unpacked[n:n + n_hi]] + n += n_hi + self.reserved2 = list(unpacked[n:n + RESERVED2_WORDS]) self.video_viewer_key = bytearray(viewer_key) self.video_publish_key = bytearray(publish_key) self.secret_key = bytearray(secret_key) @@ -314,9 +352,11 @@ def video_port_count(self): still accounts for every port it owns. Never 0: an entry with no ports yet is presented as wanting one. """ + # Tolerate a short list: callers build KeyEntry objects by hand + # and the slot count has grown before. highest = 0 - for slot in range(MAX_VIDEO_PORTS): - if self.video_ports[slot]: + for slot, port in enumerate(self.video_ports[:MAX_VIDEO_PORTS]): + if port: highest = slot + 1 return highest or 1 @@ -353,10 +393,23 @@ def set_rtmp_path(self, slot, path): self.video_rtmp_path = paths[:MAX_VIDEO_PORTS] def slot_opts(self, slot): - return video_slot_opts(self.video_flags, slot) + """Options for one slot, from whichever word holds it.""" + if not 0 <= slot < MAX_VIDEO_PORTS: + return 0 + if slot < VIDEO_PORTS_INLINE: + return video_slot_opts(self.video_flags, slot) + return video_slot_opts(self.video_flags_hi, + slot - VIDEO_PORTS_INLINE) def set_slot_opts(self, slot, opts): - self.video_flags = video_set_slot_opts(self.video_flags, slot, opts) + if not 0 <= slot < MAX_VIDEO_PORTS: + raise CLIError("slot must be 0..%d" % (MAX_VIDEO_PORTS - 1)) + if slot < VIDEO_PORTS_INLINE: + self.video_flags = video_set_slot_opts(self.video_flags, slot, + opts) + else: + self.video_flags_hi = video_set_slot_opts( + self.video_flags_hi, slot - VIDEO_PORTS_INLINE, opts) def slot_opt_names(self, slot): opts = self.slot_opts(slot) diff --git a/supportproxy.cpp b/supportproxy.cpp index 3613bdc..64f907a 100644 --- a/supportproxy.cpp +++ b/supportproxy.cpp @@ -90,6 +90,7 @@ struct listen_port { // can't be re-forked in a tight loop uint32_t video_ports[KEY_MAX_VIDEO_PORTS]; uint32_t video_flags; + uint32_t video_flags_hi; // slots past KEY_VIDEO_PORTS_INLINE uint32_t flags; uint8_t fc_sysid; // 0 = match any; otherwise the FC's MAVLink // sysid for binlog reboot detection @@ -155,12 +156,12 @@ static void close_sockets(struct listen_port *p); */ static bool video_cfg_differs(const struct listen_port *p, uint32_t flags, const uint32_t *video_ports, - uint32_t video_flags) + uint32_t video_flags, uint32_t video_flags_hi) { if ((p->flags & KEY_FLAG_VIDEO) != (flags & KEY_FLAG_VIDEO)) { return true; } - if (p->video_flags != video_flags) { + if (p->video_flags != video_flags || p->video_flags_hi != video_flags_hi) { return true; } for (int i = 0; i < KEY_MAX_VIDEO_PORTS; i++) { @@ -183,12 +184,13 @@ static void video_stop_child(struct listen_port *p, const char *why) static void upsert_port(int port1, int port2, uint32_t flags, uint8_t fc_sysid, float tz_offset_hours, const uint32_t *video_ports, - uint32_t video_flags) + uint32_t video_flags, uint32_t video_flags_hi) { for (auto *p = ports; p; p=p->next) { if (p->port2 == port2) { p->seen = true; - if (video_cfg_differs(p, flags, video_ports, video_flags)) { + if (video_cfg_differs(p, flags, video_ports, video_flags, + video_flags_hi)) { // Ports/enable/slot options changed: the running child // still binds the old set, so stop it and let // check_children() re-fork with the new config. @@ -196,6 +198,7 @@ static void upsert_port(int port1, int port2, uint32_t flags, uint8_t fc_sysid, } memcpy(p->video_ports, video_ports, sizeof(p->video_ports)); p->video_flags = video_flags; + p->video_flags_hi = video_flags_hi; if (p->removed) { // came back: re-add as a fresh listener printf("[%d] re-added (port1=%d)\n", port2, port1); @@ -244,6 +247,7 @@ static void upsert_port(int port1, int port2, uint32_t flags, uint8_t fc_sysid, p->video_respawn_after = 0; memcpy(p->video_ports, video_ports, sizeof(p->video_ports)); p->video_flags = video_flags; + p->video_flags_hi = video_flags_hi; p->flags = flags; p->fc_sysid = fc_sysid; p->tz_offset_hours = tz_offset_hours; @@ -269,8 +273,14 @@ static int handle_record(struct tdb_context *db, TDB_DATA key, TDB_DATA data, vo // KeyEntry.fc_sysid is uint32 for forward compat; the wire value is // a MAVLink sysid (0..255), so truncate to uint8 once it crosses the // C++/binlog boundary. The CLI / web UI already cap at 255. + // The slots are split across two field groups on disk; hand + // upsert_port one flat array so nothing downstream has to know. + uint32_t vports[KEY_MAX_VIDEO_PORTS]; + for (unsigned i = 0; i < KEY_MAX_VIDEO_PORTS; i++) { + vports[i] = video_port_of(k, i); + } upsert_port(k.port1, port2, k.flags, uint8_t(k.fc_sysid), - k.tz_offset_hours, k.video_ports, k.video_flags); + k.tz_offset_hours, vports, k.video_flags, k.video_flags_hi); return 0; } diff --git a/tests/test_video_ports.py b/tests/test_video_ports.py index 1ce4c91..f415287 100644 --- a/tests/test_video_ports.py +++ b/tests/test_video_ports.py @@ -47,9 +47,9 @@ def _ports(d, port2=PORT2): def test_set_and_clear_video_ports(db): keydb_lib.set_video_ports(db, PORT2, [21001, 21002]) - assert _ports(db) == [21001, 21002, 0] + assert _ports(db) == [21001, 21002, 0, 0, 0] keydb_lib.set_video_ports(db, PORT2, []) - assert _ports(db) == [0, 0, 0] + assert _ports(db) == [0, 0, 0, 0, 0] @pytest.mark.parametrize('bad,msg', [ @@ -60,14 +60,14 @@ def test_set_and_clear_video_ports(db): ([22000, 22000], 'listed twice'), # duplicate in one call ([80000], 'out of range'), # above the port range ([80], 'out of range'), # below VIDEO_PORT_MIN - ([1, 2, 3, 4], 'at most 3'), # too many + ([1] * (keydb_lib.MAX_VIDEO_PORTS + 1), 'at most'), # too many ]) def test_video_port_collisions_rejected(db, bad, msg): with pytest.raises(CLIError) as ei: keydb_lib.set_video_ports(db, PORT2, bad) assert msg in str(ei.value) # a rejected call must not have partially applied - assert _ports(db) == [0, 0, 0] + assert _ports(db) == [0, 0, 0, 0, 0] def test_video_port_blocks_a_later_add(db): @@ -82,10 +82,10 @@ def test_video_port_can_be_reassigned_to_itself(db): """Re-setting the same ports must not collide with the entry's own.""" keydb_lib.set_video_ports(db, PORT2, [21001, 21002]) keydb_lib.set_video_ports(db, PORT2, [21001, 21002]) - assert _ports(db) == [21001, 21002, 0] + assert _ports(db) == [21001, 21002, 0, 0, 0] # and reordering is fine keydb_lib.set_video_ports(db, PORT2, [21002, 21001]) - assert _ports(db) == [21002, 21001, 0] + assert _ports(db) == [21002, 21001, 0, 0, 0] def test_ports_in_use_excludes_named_entry(db): @@ -236,11 +236,11 @@ def test_starts_at_the_base(self, db): ke = keydb_lib.add_entry(db, 10001, 10002, 'a', 'p') got = keydb_lib.suggest_video_ports(db, ke, 1) assert got[0] == keydb_lib.VIDEO_PORT_BASE - assert got[1:] == [0, 0] + assert got[1:] == [0] * (keydb_lib.MAX_VIDEO_PORTS - 1) def test_consecutive_within_one_entry(self, db): ke = keydb_lib.add_entry(db, 10001, 10002, 'a', 'p') - assert keydb_lib.suggest_video_ports(db, ke, 3) == [ + assert keydb_lib.suggest_video_ports(db, ke, 3)[:3] == [ keydb_lib.VIDEO_PORT_BASE, keydb_lib.VIDEO_PORT_BASE + 1, keydb_lib.VIDEO_PORT_BASE + 2] @@ -248,16 +248,16 @@ def test_consecutive_within_one_entry(self, db): def test_skips_ports_another_entry_holds(self, db): base = keydb_lib.VIDEO_PORT_BASE other = keydb_lib.add_entry(db, 10001, 10002, 'a', 'p') - other.video_ports = [base, base + 2, 0] + other.video_ports = [base, base + 2, 0, 0, 0] other.store(db) ke = keydb_lib.add_entry(db, 10003, 10004, 'b', 'p') - assert keydb_lib.suggest_video_ports(db, ke, 2) == [base + 1, - base + 3, 0] + assert keydb_lib.suggest_video_ports(db, ke, 2)[:3] == [ + base + 1, base + 3, 0] def test_skips_port1_and_port2(self, db): base = keydb_lib.VIDEO_PORT_BASE ke = keydb_lib.add_entry(db, base, base + 1, 'a', 'p') - assert keydb_lib.suggest_video_ports(db, ke, 1) == [base + 2, 0, 0] + assert keydb_lib.suggest_video_ports(db, ke, 1) == [base + 2] + [0] * (keydb_lib.MAX_VIDEO_PORTS - 1) def test_keeps_an_already_allocated_port(self, db): """An entry that is already streaming on a port must not be @@ -266,14 +266,14 @@ def test_keeps_an_already_allocated_port(self, db): ke = keydb_lib.add_entry(db, 10001, 10002, 'a', 'p') ke.video_ports = [50000, 0, 0] got = keydb_lib.suggest_video_ports(db, ke, 2, keep=ke.video_ports) - assert got == [50000, base, 0] + assert got == [50000, base, 0, 0, 0] def test_kept_port_is_not_reused_for_another_slot(self, db): base = keydb_lib.VIDEO_PORT_BASE ke = keydb_lib.add_entry(db, 10001, 10002, 'a', 'p') - ke.video_ports = [0, base, 0] + ke.video_ports = [0, base, 0, 0, 0] got = keydb_lib.suggest_video_ports(db, ke, 2, keep=ke.video_ports) - assert got == [base + 1, base, 0] + assert got[:3] == [base + 1, base, 0] assert len(set(p for p in got if p)) == 2 def test_suggestions_validate(self, db): diff --git a/tests/test_video_schema.py b/tests/test_video_schema.py index c4ae28e..57712e4 100644 --- a/tests/test_video_schema.py +++ b/tests/test_video_schema.py @@ -44,11 +44,11 @@ def test_keyentry_video_roundtrip(): e.video_mav_grace_s = 90 blob = e.pack() - assert len(blob) == 344 + assert len(blob) == 456 d = keydb_lib.KeyEntry(0) d.unpack(blob) - assert d.video_ports == [20001, 20002, 0] + assert d.video_ports == [20001, 20002, 0, 0, 0] assert d.active_video_ports() == [(0, 20001), (1, 20002)] assert set(d.slot_opt_names(0)) == {'srt', 'record'} assert set(d.slot_opt_names(1)) == {'raw_tcp'} @@ -135,7 +135,7 @@ def test_prevideo_keyentry_zero_extends(): assert not d.video_publish_pass_set() assert d.mav_grace_seconds() == keydb_lib.VIDEO_MAV_GRACE_DEFAULT_S # and re-packing upgrades it in place without losing anything - assert len(d.pack()) == 344 + assert len(d.pack()) == keydb_lib.KEYENTRY_CURRENT_SIZE def test_keyentry_future_tail_preserved(): @@ -200,3 +200,74 @@ def test_video_conn_index_range_is_disjoint(): last_sub = pub + conntdb_lib.VIDEO_CONN_STRIDE - 1 next_pub = pub + conntdb_lib.VIDEO_CONN_STRIDE assert last_sub < next_pub + + +def test_five_slots_round_trip(): + """All five slots survive a pack/unpack, including the split. + + Slots 0-2 live in the fields the 344-byte record had; 3 and 4 are in + fields appended after reserved[]. Nothing outside keydb_lib should + be able to tell. + """ + e = keydb_lib.KeyEntry(4242) + e.video_ports = [40001, 40002, 40003, 40004, 40005] + e.video_rtmp_path = ['a/1', 'b/2', 'c/3', 'd/4', 'e/5'] + for slot in range(keydb_lib.MAX_VIDEO_PORTS): + e.set_slot_opts(slot, keydb_lib.VIDEO_SLOT_RECORD) + e.video_flags = keydb_lib.video_set_entry_opts( + e.video_flags, keydb_lib.VIDEO_OPT_AUDIO) + + d = keydb_lib.KeyEntry(4242) + d.unpack(e.pack()) + assert d.video_ports == [40001, 40002, 40003, 40004, 40005] + assert d.video_rtmp_path == ['a/1', 'b/2', 'c/3', 'd/4', 'e/5'] + for slot in range(keydb_lib.MAX_VIDEO_PORTS): + assert d.slot_opts(slot) & keydb_lib.VIDEO_SLOT_RECORD, slot + assert keydb_lib.video_entry_opts(d.video_flags) \ + & keydb_lib.VIDEO_OPT_AUDIO + + +def test_slot_three_does_not_clobber_the_entry_options(): + """The low flags word is full: three slot bytes plus the entry byte. + + A fourth slot byte at shift 24 would land exactly on the entry-wide + options, which is why slots past the third have their own word. + """ + e = keydb_lib.KeyEntry(4243) + e.video_flags = keydb_lib.video_set_entry_opts( + e.video_flags, keydb_lib.VIDEO_OPT_AUDIO) + e.set_slot_opts(3, 0xFF) + e.set_slot_opts(4, 0xFF) + assert keydb_lib.video_entry_opts(e.video_flags) \ + & keydb_lib.VIDEO_OPT_AUDIO, 'entry options lost' + + d = keydb_lib.KeyEntry(4243) + d.unpack(e.pack()) + assert keydb_lib.video_entry_opts(d.video_flags) \ + & keydb_lib.VIDEO_OPT_AUDIO + assert d.slot_opts(3) == 0xFF and d.slot_opts(4) == 0xFF + + +def test_three_slot_record_still_reads(): + """A record written before slots 3-4 existed must be untouched. + + The three inline fields stay exactly where they were, so an existing + database keeps working and an older binary can still read what a + newer one writes. + """ + e = keydb_lib.KeyEntry(4244) + e.video_ports = [40001, 40002, 40003, 0, 0] + e.video_rtmp_path = ['x/1', 'y/2', 'z/3', '', ''] + e.set_slot_opts(1, keydb_lib.VIDEO_SLOT_RECORD) + e.video_flags = keydb_lib.video_set_entry_opts( + e.video_flags, keydb_lib.VIDEO_OPT_AUDIO) + old = e.pack()[:344] # truncated, as an old writer would + assert len(old) == 344 + + d = keydb_lib.KeyEntry(4244) + d.unpack(old) + assert d.video_ports == [40001, 40002, 40003, 0, 0] + assert d.video_rtmp_path == ['x/1', 'y/2', 'z/3', '', ''] + assert d.slot_opts(1) & keydb_lib.VIDEO_SLOT_RECORD + assert keydb_lib.video_entry_opts(d.video_flags) \ + & keydb_lib.VIDEO_OPT_AUDIO diff --git a/tests/webadmin/test_log_routes.py b/tests/webadmin/test_log_routes.py index 9629651..7153aec 100644 --- a/tests/webadmin/test_log_routes.py +++ b/tests/webadmin/test_log_routes.py @@ -587,7 +587,7 @@ def test_segments_sort_with_the_collision_suffix(self, client, logs_dir): 'video segments not in natural collision order' @pytest.mark.parametrize('bad', [ - '2026_08_01_10:00:00.v4.ts', # slot out of range + '2026_08_01_10:00:00.v6.ts', # slot out of range '2026_08_01_10:00:00.ts', # no slot '2026_08_01_10:00:00.v1.tsx', # not a segment 'evil.ts', diff --git a/tests/webadmin/test_video_ui.py b/tests/webadmin/test_video_ui.py index b2422b1..19a0795 100644 --- a/tests/webadmin/test_video_ui.py +++ b/tests/webadmin/test_video_ui.py @@ -70,7 +70,7 @@ def test_owner_cannot_set_ports_by_posting_them(self, client, keydb_path): _owner_post(client, video_enabled='y', video_port_1=str(VPORT_A), video_quota_mb='9999') ke = fetch_entry(keydb_path, ALICE_PORT2) - assert ke.video_ports == [0, 0, 0] + assert ke.video_ports == [0, 0, 0, 0, 0] assert ke.video_quota_mb == 0 def test_owner_grace_out_of_range_rejected(self, client, keydb_path): @@ -136,7 +136,7 @@ def test_admin_can_allocate_ports(self, client, keydb_path): video_quota_mb='4096') assert resp.status_code == 302 ke = fetch_entry(keydb_path, ALICE_PORT2) - assert ke.video_ports == [VPORT_A, VPORT_B, 0] + assert ke.video_ports == [VPORT_A, VPORT_B, 0, 0, 0] assert ke.video_quota_mb == 4096 assert ke.active_video_ports() == [(0, VPORT_A), (1, VPORT_B)] @@ -147,14 +147,14 @@ def test_admin_video_port_collision_is_refused(self, client, keydb_path): video_port_1=str(BOB_PORT1)) assert resp.status_code == 302 ke = fetch_entry(keydb_path, ALICE_PORT2) - assert ke.video_ports == [0, 0, 0], 'collision must not be stored' + assert ke.video_ports == [0, 0, 0, 0, 0], 'collision must not be stored' def test_admin_video_port_cannot_take_own_port2(self, client, keydb_path): login_as(client, BOB_PORT1, BOB_PASS) resp = _admin_post(client, ALICE_PORT2, video_enabled='y', video_port_1=str(ALICE_PORT2)) assert resp.status_code == 302 - assert fetch_entry(keydb_path, ALICE_PORT2).video_ports == [0, 0, 0] + assert fetch_entry(keydb_path, ALICE_PORT2).video_ports == [0, 0, 0, 0, 0] def test_admin_can_clear_ports(self, client, keydb_path): login_as(client, BOB_PORT1, BOB_PASS) @@ -162,7 +162,7 @@ def test_admin_can_clear_ports(self, client, keydb_path): video_port_1=str(VPORT_A)) assert fetch_entry(keydb_path, ALICE_PORT2).video_ports[0] == VPORT_A _admin_post(client, ALICE_PORT2, video_enabled='y', video_port_1='') - assert fetch_entry(keydb_path, ALICE_PORT2).video_ports == [0, 0, 0] + assert fetch_entry(keydb_path, ALICE_PORT2).video_ports == [0, 0, 0, 0, 0] def test_allocated_port_blocks_a_later_port1_change(self, client, keydb_path): @@ -282,14 +282,16 @@ def test_owner_has_no_selector(self, client, keydb_path): def test_unused_slots_ship_hidden(self, client, keydb_path): """No-JS and first paint: an entry with one port must not show - three rows even before the script runs.""" + every slot's row before the script runs.""" login_as(client, BOB_PORT1, BOB_PASS) html = client.get('/admin/%d' % ALICE_PORT2).get_data(as_text=True) rows = re.findall(r'
]*)>', html) - assert len(rows) == 3 - assert {int(s): ('hidden' in a) for s, a in rows} == { - 1: False, 2: True, 3: True} + assert len(rows) == keydb_lib.MAX_VIDEO_PORTS + # Slot 1 shown, every other slot hidden. + expected = {n: (n != 1) + for n in range(1, keydb_lib.MAX_VIDEO_PORTS + 1)} + assert {int(s): ('hidden' in a) for s, a in rows} == expected def test_ports_default_to_the_base(self, client, keydb_path): login_as(client, BOB_PORT1, BOB_PASS) @@ -317,7 +319,7 @@ def test_saving_a_disabled_entry_allocates_nothing(self, client, video_port_2='40002', video_port_3='40003') assert resp.status_code == 302 ke = fetch_entry(keydb_path, ALICE_PORT2) - assert ke.video_ports == [0, 0, 0] + assert ke.video_ports == [0, 0, 0, 0, 0] assert not ke.video_enabled() def test_count_bounds_what_is_stored(self, client, keydb_path): @@ -329,7 +331,7 @@ def test_count_bounds_what_is_stored(self, client, keydb_path): video_port_2=str(VPORT_B)) assert resp.status_code == 302 ke = fetch_entry(keydb_path, ALICE_PORT2) - assert ke.video_ports == [VPORT_A, 0, 0] + assert ke.video_ports == [VPORT_A, 0, 0, 0, 0] assert ke.video_port_count() == 1 def test_count_two_stores_two(self, client, keydb_path): @@ -339,7 +341,7 @@ def test_count_two_stores_two(self, client, keydb_path): video_port_2=str(VPORT_B)) assert resp.status_code == 302 ke = fetch_entry(keydb_path, ALICE_PORT2) - assert ke.video_ports == [VPORT_A, VPORT_B, 0] + assert ke.video_ports == [VPORT_A, VPORT_B, 0, 0, 0] assert ke.video_port_count() == 2 def test_a_submission_without_the_count_keeps_every_slot(self, client, @@ -351,7 +353,7 @@ def test_a_submission_without_the_count_keeps_every_slot(self, client, video_port_2=str(VPORT_B)) assert resp.status_code == 302 ke = fetch_entry(keydb_path, ALICE_PORT2) - assert ke.video_ports == [VPORT_A, VPORT_B, 0] + assert ke.video_ports == [VPORT_A, VPORT_B, 0, 0, 0] def test_allocated_ports_are_not_renumbered_on_reopen(self, client, keydb_path): @@ -372,8 +374,10 @@ def test_count_reflects_what_is_allocated_on_reopen(self, client, html = client.get('/admin/%d' % ALICE_PORT2).get_data(as_text=True) rows = re.findall(r'
]*)>', html) - assert {int(s): ('hidden' in a) for s, a in rows} == { - 1: False, 2: False, 3: True} + # Two allocated: slots 1 and 2 shown, the rest hidden. + expected = {n: (n > 2) + for n in range(1, keydb_lib.MAX_VIDEO_PORTS + 1)} + assert {int(s): ('hidden' in a) for s, a in rows} == expected def client_logged_in_as_admin(client): diff --git a/video.cpp b/video.cpp index cb09a28..f18301c 100644 --- a/video.cpp +++ b/video.cpp @@ -267,7 +267,7 @@ int VideoChild::bind_slots(void) { int first_err = 0; for (int i = 0; i < KEY_MAX_VIDEO_PORTS; i++) { - const uint32_t port = ke_.video_ports[i]; + const uint32_t port = video_port_of(ke_, unsigned(i)); if (port == 0) { continue; } @@ -465,7 +465,7 @@ void VideoChild::handle_udp(Slot &s, int idx) s.ring.init(video_ring_bytes()); s.scanner = TSScanner(); s.had_anchor = false; - s.recording = (video_slot_opts(ke_.video_flags, unsigned(idx)) + s.recording = (video_slot_opts_of(ke_, unsigned(idx)) & VIDEO_SLOT_RECORD) != 0; if (s.recording) { s.rec.configure(uint32_t(port2_), idx, @@ -557,7 +557,7 @@ void VideoChild::latch_publisher(Slot &s, int idx, time_t now) s.ring.init(video_ring_bytes()); s.scanner = TSScanner(); s.had_anchor = false; - s.recording = (video_slot_opts(ke_.video_flags, unsigned(idx)) + s.recording = (video_slot_opts_of(ke_, unsigned(idx)) & VIDEO_SLOT_RECORD) != 0; if (s.recording) { s.rec.configure(uint32_t(port2_), idx, @@ -825,7 +825,8 @@ bool VideoChild::promote_pending(Slot &s, int idx, PendingRtmp &p, expect. Left blank the slot takes whatever is published. */ char want[sizeof(ke_.video_rtmp_path[0]) + 1] {}; - memcpy(want, ke_.video_rtmp_path[idx], sizeof(ke_.video_rtmp_path[idx])); + memcpy(want, video_rtmp_path_of(ke_, unsigned(idx)), + sizeof(ke_.video_rtmp_path[0])); want[sizeof(want) - 1] = '\0'; if (want[0] != '\0' && r.path() != want) { printf("[%d] video slot %d RTMP publisher refused: published %s, " diff --git a/videoview.cpp b/videoview.cpp index caa615e..a178224 100644 --- a/videoview.cpp +++ b/videoview.cpp @@ -295,7 +295,7 @@ bool VideoViewer::detect_timeout(const struct KeyEntry &ke, int slot, // credential, so this is only offered when the slot allows it and // no viewer password is set. kind_ = VVK_RAW; - const uint32_t opts = video_slot_opts(ke.video_flags, unsigned(slot)); + const uint32_t opts = video_slot_opts_of(ke, unsigned(slot)); if ((opts & VIDEO_SLOT_RAW_TCP) == 0) { drop_reason_ = "raw-TCP viewers not enabled on this slot"; return false; diff --git a/webadmin/__init__.py b/webadmin/__init__.py index 499ab2f..4cc3544 100644 --- a/webadmin/__init__.py +++ b/webadmin/__init__.py @@ -32,6 +32,8 @@ import os import subprocess +import keydb_lib + from flask import Flask, redirect, url_for from flask_wtf.csrf import CSRFProtect from werkzeug.middleware.proxy_fix import ProxyFix @@ -118,6 +120,10 @@ def create_app(test_config=None): app.register_blueprint(video_bp) app.register_blueprint(system_bp) + # The video templates loop over slots; keep the count in one place + # rather than repeating it in every route that renders them. + app.jinja_env.globals['max_video_slots'] = keydb_lib.MAX_VIDEO_PORTS + @app.route('/') def index(): from .auth import is_admin, current_owner diff --git a/webadmin/forms.py b/webadmin/forms.py index 291e538..7890b6a 100644 --- a/webadmin/forms.py +++ b/webadmin/forms.py @@ -6,6 +6,8 @@ short enough to scan while the detail stays one hover away. Put the explanation in `description`, not in the label. """ +import keydb_lib + from flask_wtf import FlaskForm from wtforms import (BooleanField, FloatField, IntegerField, PasswordField, SelectField, StringField, SubmitField) @@ -137,6 +139,14 @@ class _VideoOwnerFields: 'Slot 3 RTMP path', description='As above, for the third slot.', validators=[Optional(), Length(max=31)]) + video_rtmp_4 = StringField( + 'Slot 4 RTMP path', + description='As above, for the fourth slot.', + validators=[Optional(), Length(max=31)]) + video_rtmp_5 = StringField( + 'Slot 5 RTMP path', + description='As above, for the fifth slot.', + validators=[Optional(), Length(max=31)]) video_srt_1 = BooleanField('Slot 1: UDP side speaks SRT (else MPEG-TS)') video_record_1 = BooleanField('Slot 1: record to disk') video_rawtcp_1 = BooleanField('Slot 1: allow raw-TCP viewers (no password)') @@ -146,18 +156,25 @@ class _VideoOwnerFields: video_srt_3 = BooleanField('Slot 3: UDP side speaks SRT (else MPEG-TS)') video_record_3 = BooleanField('Slot 3: record to disk') video_rawtcp_3 = BooleanField('Slot 3: allow raw-TCP viewers (no password)') + video_srt_4 = BooleanField('Slot 4: UDP side speaks SRT (else MPEG-TS)') + video_record_4 = BooleanField('Slot 4: record to disk') + video_rawtcp_4 = BooleanField('Slot 4: allow raw-TCP viewers (no password)') + video_srt_5 = BooleanField('Slot 5: UDP side speaks SRT (else MPEG-TS)') + video_record_5 = BooleanField('Slot 5: record to disk') + video_rawtcp_5 = BooleanField('Slot 5: allow raw-TCP viewers (no password)') class _VideoAdminFields(_VideoOwnerFields): """Adds the port allocation and the disk budget.""" - # How many of the three slots this entry uses. Most entries want one - # camera, so showing three sets of ports and per-slot options by - # default is noise; this drives which slots the page shows at all. + # How many slots this entry uses. Most entries want one camera, so + # showing every set of ports and per-slot options by default is + # noise; this drives which slots the page shows at all. video_port_count = SelectField( 'Number of video ports', description='One port carries one stream, so allocate one per ' 'camera. Slots you do not use are hidden.', - choices=[(n, str(n)) for n in range(1, 4)], + choices=[(n, str(n)) + for n in range(1, keydb_lib.MAX_VIDEO_PORTS + 1)], coerce=int, default=1) video_port_1 = IntegerField( 'Video port 1', @@ -176,6 +193,16 @@ class _VideoAdminFields(_VideoOwnerFields): description='Third camera. One port carries exactly one stream.', validators=[Optional(), NumberRange(min=VIDEO_PORT_MIN, max=VIDEO_PORT_MAX)]) + video_port_4 = IntegerField( + 'Video port 4', + description='Fourth camera. One port carries exactly one stream.', + validators=[Optional(), NumberRange(min=VIDEO_PORT_MIN, + max=VIDEO_PORT_MAX)]) + video_port_5 = IntegerField( + 'Video port 5', + description='Fifth camera. One port carries exactly one stream.', + validators=[Optional(), NumberRange(min=VIDEO_PORT_MIN, + max=VIDEO_PORT_MAX)]) video_quota_mb = IntegerField( 'Video disk budget (MB)', description='Oldest recordings are deleted once this entry\'s ' diff --git a/webadmin/logs.py b/webadmin/logs.py index 51f2759..39099a2 100644 --- a/webadmin/logs.py +++ b/webadmin/logs.py @@ -41,7 +41,7 @@ # and the legacy sessionN names so old logs stay browsable. SESSION_RE = re.compile( r'^(session\d+|\d{4}_\d{2}_\d{2}_\d{2}:\d{2}:\d{2}(-\d+)?)' - r'\.(tlog|bin|v[1-3]\.ts)$') + r'\.(tlog|bin|v[1-5]\.ts)$') # Natural-sort key: treat embedded digit runs as numbers so that # session10.tlog sorts AFTER session2.tlog (not between session1 and @@ -68,7 +68,7 @@ # new file types streamable. VIDEO_NAME_RE = re.compile( r'^(session\d+|\d{4}_\d{2}_\d{2}_\d{2}:\d{2}:\d{2}(-\d+)?)' - r'\.v[1-3]\.ts$') + r'\.v[1-5]\.ts$') def _natural_key(name): diff --git a/webadmin/templates/_video_fields.html b/webadmin/templates/_video_fields.html index 7ee3223..f49555c 100644 --- a/webadmin/templates/_video_fields.html +++ b/webadmin/templates/_video_fields.html @@ -23,7 +23,7 @@

Video

{{ row(form.video_port_count) }} {% endif %} -{% for slot in [1, 2, 3] %} +{% for slot in range(1, max_video_slots + 1) %} {% set p = entry.video_ports[slot - 1] %} {# Slots past the chosen count are rendered but hidden, so the browser still submits their options and lowering then raising the count does From ab8b1b053d1ae1a8f55cf73371bb1c9c43a7c450 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 19 Aug 2026 10:31:04 +1000 Subject: [PATCH 02/10] keydb: add a per-slot VIDEO_SLOT_SESSION_OK flag Bit 3 of each slot's option byte, which was free, so no record growth and no migration -- an existing record reads it clear, which is the current behaviour. It marks a slot whose publisher may be admitted by the entry's MAVLink session even though a publish password is set. Some publishers cannot present one: a camera speaking RTMP from its own firmware has nowhere to put a credential unless its stream-key field tolerates a query, and plain MPEG-TS over UDP never does. Without this an entry faced an all-or-nothing choice between a password and those streams. --- keydb.h | 18 ++++++++++++++++++ keydb_lib.py | 5 +++++ 2 files changed, 23 insertions(+) diff --git a/keydb.h b/keydb.h index e67e96d..1bcc8e6 100644 --- a/keydb.h +++ b/keydb.h @@ -73,6 +73,24 @@ #define VIDEO_SLOT_SRT (1u << 0) // UDP side speaks SRT, not plain MPEG-TS #define VIDEO_SLOT_RECORD (1u << 1) // write .ts segments under logs/ #define VIDEO_SLOT_RAW_TCP (1u << 2) // allow raw-TCP viewers (no credential) +/* + Accept a publisher on this slot that its MAVLink session authorises, + even though the entry has a publish password. + + Some publishers cannot present one: a camera speaking RTMP straight + out of its own firmware has nowhere to put a credential unless its + stream-key field tolerates a query, and plain MPEG-TS over UDP never + does. Without this the choice was all-or-nothing per entry -- set a + password and those streams stop, or leave it off and every stream is + admitted on its source address. + + Opt-in, and per slot, so an entry that already has a password keeps + password-only publishing everywhere until someone deliberately widens + one slot. The fallback applies only when no credential was offered at + all: a wrong password is still a wrong password, so a typo does not + quietly succeed on the strength of the address. + */ +#define VIDEO_SLOT_SESSION_OK (1u << 3) // entry-wide bits, stored in the top byte #define VIDEO_OPT_SHIFT 24 diff --git a/keydb_lib.py b/keydb_lib.py index 2d4226b..2db8e78 100644 --- a/keydb_lib.py +++ b/keydb_lib.py @@ -81,11 +81,16 @@ VIDEO_SLOT_SRT = 1 << 0 # UDP side speaks SRT, not plain MPEG-TS VIDEO_SLOT_RECORD = 1 << 1 # write .ts segments under logs/ VIDEO_SLOT_RAW_TCP = 1 << 2 # allow raw-TCP viewers (no credential) +# Accept a publisher this slot's MAVLink session authorises even when the +# entry has a publish password. For streams that cannot carry one -- a +# camera's own RTMP, plain MPEG-TS over UDP. Opt-in, per slot. +VIDEO_SLOT_SESSION_OK = 1 << 3 VIDEO_SLOT_FLAG_NAMES = { "srt": VIDEO_SLOT_SRT, "record": VIDEO_SLOT_RECORD, "raw_tcp": VIDEO_SLOT_RAW_TCP, + "session_ok": VIDEO_SLOT_SESSION_OK, } VIDEO_OPT_SHIFT = 24 From 7dfe9ba19f53e976cffb1dc454c7db96782830b0 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 19 Aug 2026 10:31:04 +1000 Subject: [PATCH 03/10] video: admit a flagged slot on the MAVLink session despite a password admit() gains the slot's session_ok bit. With it set and no credential offered, admission falls through to the MAVLink-session path instead of refusing; without it, nothing changes. The fallback deliberately does not apply to a credential that was offered and is wrong. Downgrading that to address matching would turn a clear rejection into a silent weakening, so a typo cannot succeed on the strength of the source address. Five tests, of which two guard the behaviour being preserved: an unflagged slot still refuses a session-only publisher, and a wrong password is still refused on a flagged one. Verified RED by forcing the bit false -- the three that assert the new path fail, the two guards still pass. --- README.md | 17 ++++++++ tests/test_video_rtsp.py | 89 +++++++++++++++++++++++++++++++++++++++- video.cpp | 18 +++++--- videoauth.cpp | 37 ++++++++++------- videoauth.h | 15 +++++-- 5 files changed, 151 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 93c145a..fdf9409 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,23 @@ side, so a scanner between flights can become the authorised address and the aircraft's video is then refused until the grace expires. **Entries used for video should set `bidi_sign` or a publish password.** +A publish password normally *replaces* the MAVLink-session check rather +than adding to it — an operator sets one precisely so address matching +is not the gate. Some publishers cannot present one at all, though: a +camera speaking RTMP from its own firmware has nowhere to put it unless +its stream-key field tolerates a query, and plain UDP never does. The +per-slot **MAVLink publish** option lets one slot fall back to the +session check while the rest of the entry stays password-only: + +```bash +./keydb.py videoflag 11024 0 session_ok +``` + +It is opt-in and per slot, so enabling it on the camera's slot does not +weaken the others. A password that *is* supplied and is wrong is still +refused — the fallback applies only when none was offered, so a typo +cannot quietly succeed on the strength of the address. + **Watching.** In the browser from the web UI, or outside it with the `ffplay`/`vlc` command the page offers. The browser player needs H.264: Chrome and Firefox will not decode HEVC in Media Source Extensions on diff --git a/tests/test_video_rtsp.py b/tests/test_video_rtsp.py index 855ae31..4e174ab 100644 --- a/tests/test_video_rtsp.py +++ b/tests/test_video_rtsp.py @@ -57,7 +57,7 @@ def clip(tmp_path_factory): def _workdir(tmp_path, record=True, publish_pass=None, - rtmp_path=None): + rtmp_path=None, session_ok=False): p = tmp_path / 'work' p.mkdir() db = keydb_lib.init_db(str(p / 'keys.tdb')) @@ -71,6 +71,8 @@ def _workdir(tmp_path, record=True, publish_pass=None, keydb_lib.set_video_publish_pass(db, PORT_ENG, publish_pass) if rtmp_path: keydb_lib.set_video_rtmp_path(db, PORT_ENG, 0, rtmp_path) + if session_ok: + keydb_lib.set_video_slot_flag(db, PORT_ENG, 0, 'session_ok') db.transaction_prepare_commit() db.transaction_commit() db.close() @@ -227,7 +229,7 @@ def session(tmp_path): def _start(**kw): made['s'] = RtspSession(_workdir(tmp_path, **{ k: v for k, v in kw.items() - if k in ('record', 'publish_pass')}), + if k in ('record', 'publish_pass', 'session_ok')}), with_mav=kw.get('with_mav', True)) return made['s'] @@ -434,6 +436,89 @@ def test_password_replaces_the_mavlink_check(self, session, clip): assert 'RTSP publisher' not in s.proxy.log +@pytest.mark.integration +class TestSessionOkSlot: + """VIDEO_SLOT_SESSION_OK: one slot opts back out of password-only. + + A camera speaking RTMP out of its own firmware, and plain MPEG-TS + over UDP, have nowhere to put a credential. Without this the entry + faced an all-or-nothing choice: set a password and those streams + stop, or leave it off and every slot is admitted on its source + address. The flag is per slot and opt-in, so the streams that can + present a password still have to. + """ + + def test_udp_is_admitted_by_the_session_on_a_flagged_slot(self, session): + import socket + sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + import tsgen + s = session(with_mav=True, publish_pass='pubsecret', session_ok=True) + g = tsgen.TSGen() + sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + try: + for dg in g.datagrams(g.stream(60, gop=10, psi_every=20)): + sock.sendto(dg, ('127.0.0.1', VPORT)) + time.sleep(0.003) + finally: + sock.close() + assert s.proxy.wait_for(r'join=ready', timeout=25), s.proxy.log + assert 'cannot carry one' not in s.proxy.log + + def test_an_unflagged_slot_still_refuses_the_same_publisher(self, session): + """The property the design turns on: a MAVLink session does not + become a way past a publish password unless a slot says so.""" + import socket + sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + import tsgen + s = session(with_mav=True, publish_pass='pubsecret', session_ok=False) + g = tsgen.TSGen() + sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + try: + for dg in g.datagrams(g.stream(60, gop=10, psi_every=20)): + sock.sendto(dg, ('127.0.0.1', VPORT)) + time.sleep(0.003) + finally: + sock.close() + assert s.proxy.wait_for(r'cannot carry one', timeout=20), s.proxy.log + assert 'join=ready' not in s.proxy.log + + def test_a_wrong_password_is_still_refused_on_a_flagged_slot(self, + session, clip): + """The fallback is for a publisher that offered nothing. A + credential that was offered and is wrong must not be silently + downgraded to address matching, or a typo would look like it + worked.""" + s = session(with_mav=True, publish_pass='pubsecret', session_ok=True) + s.pub = _publish_with('?pw=wrong', clip) + assert s.proxy.wait_for(r'wrong publish password', timeout=25), \ + s.proxy.log + assert 'RTSP publisher' not in s.proxy.log + + def test_offering_none_falls_back_on_a_flagged_slot(self, session, clip): + s = session(with_mav=True, publish_pass='pubsecret', session_ok=True) + s.pub = _publish_with('', clip) + assert s.proxy.wait_for(r'RTSP publisher', timeout=25), s.proxy.log + assert 'none was supplied' not in s.proxy.log + + def test_the_flag_is_not_a_blanket_bypass(self, session): + """With no MAVLink session anywhere it refuses, and says why -- + the flag redirects to path B, it does not skip admission.""" + import socket + sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + import tsgen + s = session(with_mav=False, publish_pass='pubsecret', session_ok=True) + g = tsgen.TSGen() + sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + try: + for dg in g.datagrams(g.stream(30, gop=10, psi_every=20)): + sock.sendto(dg, ('127.0.0.1', VPORT)) + time.sleep(0.003) + finally: + sock.close() + assert s.proxy.wait_for(r'no MAVLink session', timeout=20), s.proxy.log + assert 'join=ready' not in s.proxy.log + + class TestRtspPublisherRestart: """Restarting an RTSP publisher, which is the case that mattered. diff --git a/video.cpp b/video.cpp index f18301c..9746341 100644 --- a/video.cpp +++ b/video.cpp @@ -211,6 +211,13 @@ class VideoChild { void handle_tcp(Slot &s, int idx); void tick(time_t now); void write_conn_rows(time_t now); + // Does this slot fall back to the MAVLink session when a publish + // password is set but the publisher offered none? + bool session_ok(int idx) const { + return (video_slot_opts_of(ke_, unsigned(idx)) + & VIDEO_SLOT_SESSION_OK) != 0; + } + void log_reject(Slot &s, int idx, uint32_t ip_be, video_admit_t r, time_t now); void ingest(Slot &s, int idx, const uint8_t *buf, size_t n); @@ -429,10 +436,11 @@ void VideoChild::handle_udp(Slot &s, int idx) // Plain MPEG-TS over UDP carries no credential, so path A can't // apply here; admit() falls through to the MAVLink-session check - // unless a publish password is set, in which case UDP can't satisfy - // it and the datagram is refused. + // unless a publish password is set and the slot is not flagged + // session_ok, in which case UDP can't satisfy it and the datagram + // is refused. video_admit_t r = auth_.admit(ke_, uint32_t(from.sin_addr.s_addr), - nullptr, now); + nullptr, session_ok(idx), now); if (r != VIDEO_ADMIT_OK) { log_reject(s, idx, uint32_t(from.sin_addr.s_addr), r, now); return; @@ -650,7 +658,7 @@ void VideoChild::handle_rtsp(Slot &s, int idx, int fd, // RTSP connection is a publisher (we do not parse enough to tell // them apart -- see videortsp.h). const video_admit_t r = auth_.admit(ke_, uint32_t(from.sin_addr.s_addr), - pw.c_str(), now); + pw.c_str(), session_ok(idx), now); if (r != VIDEO_ADMIT_OK) { log_reject(s, idx, uint32_t(from.sin_addr.s_addr), r, now); close(fd); @@ -812,7 +820,7 @@ bool VideoChild::promote_pending(Slot &s, int idx, PendingRtmp &p, RtmpSession &r = *p.sess; const video_admit_t a = auth_.admit(ke_, p.ip_be, r.password().c_str(), - now); + session_ok(idx), now); if (a != VIDEO_ADMIT_OK) { log_reject(s, idx, p.ip_be, a, now); r.reject_publish("NetStream.Publish.Denied", video_admit_str(a)); diff --git a/videoauth.cpp b/videoauth.cpp index b101960..221a861 100644 --- a/videoauth.cpp +++ b/videoauth.cpp @@ -103,7 +103,8 @@ video_admit_t VideoAuth::check_session(const struct KeyEntry &ke, } video_admit_t VideoAuth::admit(const struct KeyEntry &ke, uint32_t peer_ip_be, - const char *password, time_t now) + const char *password, bool session_ok, + time_t now) { // Path A: a publish password, when set, is sufficient on its own. bool have_pw = false; @@ -112,22 +113,30 @@ video_admit_t VideoAuth::admit(const struct KeyEntry &ke, uint32_t peer_ip_be, } if (have_pw) { /* - The password replaces the MAVLink-session check rather than - adding to it: an operator sets one precisely because they do - not want address matching to be the gate. - - Say so clearly when the transport had no way to present it. - "wrong publish password" is true but useless to someone whose - udpsink never sent one. + A credential that was offered is judged on its own merits, and + a wrong one is fatal even on a session_ok slot: falling back + on a typo would turn a clear rejection into a silent downgrade + to address matching. */ - if (password == nullptr) { - return VIDEO_ADMIT_NO_CREDENTIAL; + if (password != nullptr && *password != '\0') { + return video_password_matches(ke.video_publish_key, password) + ? VIDEO_ADMIT_OK : VIDEO_ADMIT_BAD_PASSWORD; } - if (*password == '\0') { - return VIDEO_ADMIT_MISSING_PASSWORD; + /* + None offered. By default the password replaces the + MAVLink-session check rather than adding to it: an operator + sets one precisely because they do not want address matching + to be the gate. VIDEO_SLOT_SESSION_OK opts one slot back out, + for a publisher with nowhere to put a credential. + + Distinguish the two cases when refusing -- "wrong publish + password" is true but useless to someone whose udpsink never + sent one. + */ + if (!session_ok) { + return password == nullptr ? VIDEO_ADMIT_NO_CREDENTIAL + : VIDEO_ADMIT_MISSING_PASSWORD; } - return video_password_matches(ke.video_publish_key, password) - ? VIDEO_ADMIT_OK : VIDEO_ADMIT_BAD_PASSWORD; } // Path B: match a recent MAVLink session. diff --git a/videoauth.h b/videoauth.h index 6e06d34..fb48544 100644 --- a/videoauth.h +++ b/videoauth.h @@ -6,9 +6,11 @@ A. publish password -- accepted standalone, no MAVLink needed. For CGNAT, split-egress (video on a second link) and video-only use. - B. MAVLink session -- with no publish password set, a publisher is - accepted when a MAVLink session for this entry was seen from the - same IPv4 address within the entry's grace window. The grace is + B. MAVLink session -- with no publish password set (or on a slot + flagged VIDEO_SLOT_SESSION_OK, where one is set but the publisher + offered none), a publisher is accepted when a MAVLink session for + this entry was seen from the same IPv4 address within the entry's + grace window. The grace is what lets video ride through a telemetry dropout instead of being revoked by a link flap. On a bidi entry the session must also have been signature-validated. @@ -63,9 +65,14 @@ class VideoAuth { nullptr the transport cannot carry a credential at all (UDP) "" it could, but none was supplied (RTSP with no ?pw=) "..." a credential to check + + `session_ok` is the slot's VIDEO_SLOT_SESSION_OK bit: when set, a + publisher that offered no credential falls back to path B even + though the entry has a publish password. A credential that was + offered and is wrong is still refused. */ video_admit_t admit(const struct KeyEntry &ke, uint32_t peer_ip_be, - const char *password, time_t now); + const char *password, bool session_ok, time_t now); // Force the next lookup to re-read, e.g. after a config change. void invalidate(void) { fetched_at_ = 0; } From 0cc3553de5f3cf2e42513095910bb1656a41627c Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 19 Aug 2026 10:31:04 +1000 Subject: [PATCH 04/10] webadmin: per-slot "MAVLink publish" checkbox Rendered in the existing per-slot options row and documented in the template beside it, as the other three are. The forms.py tooltip check exempts the per-slot booleans, so the guard that they really are documented where they are rendered is extended to cover it. --- tests/webadmin/test_tooltips.py | 13 +++++++------ webadmin/forms.py | 10 ++++++++++ webadmin/templates/_video_fields.html | 8 ++++++++ webadmin/videoform.py | 1 + 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/tests/webadmin/test_tooltips.py b/tests/webadmin/test_tooltips.py index 4db2416..53e5621 100644 --- a/tests/webadmin/test_tooltips.py +++ b/tests/webadmin/test_tooltips.py @@ -16,7 +16,7 @@ # Fields that legitimately have no `description`. # # The per-slot video booleans are documented by hand in -# _video_fields.html instead: three slots x three options would mean nine +# _video_fields.html instead: five slots x four options would mean twenty # near-identical strings in forms.py, and the row is rendered by hand # there anyway. _NO_DESCRIPTION_NEEDED = {'submit', 'csrf_token'} @@ -30,7 +30,7 @@ def _documented(form_cls): name = field.name if name in _NO_DESCRIPTION_NEEDED: continue - if re.match(r'^video_(srt|record|rawtcp)_\d$', name): + if re.match(r'^video_(srt|record|rawtcp|sessok)_\d$', name): continue need.add(name) if field.description: @@ -120,13 +120,14 @@ def test_reachable_without_a_pointer(self, client, keydb_path): def test_video_slot_options_are_documented_in_the_template( self, client, keydb_path): - """These three are exempt from the forms.py check, so make sure - they really are documented where they are rendered.""" + """These are exempt from the forms.py check, so make sure they + really are documented where they are rendered.""" login_as(client, BOB_PORT1, BOB_PASS) html = client.get('/admin/%d' % ALICE_PORT2).get_data(as_text=True) assert 'cannot share a port' in html # SRT assert 'timestamped .ts segments' in html # record assert 'ffplay tcp://' in html # raw TCP viewers + assert 'nowhere to put a credential' in html # MAVLink publish def test_description_is_escaped(self, app): """Descriptions are trusted text today, but they are rendered @@ -165,8 +166,8 @@ def test_tip_rules_are_present(self, client): assert ':focus' in css, 'tooltip must open on keyboard focus' def test_row_selector_is_direct_child_only(self, client): - """The per-slot video row holds three separately documented - checkboxes; a descendant selector would throw all three + """The per-slot video row holds several separately documented + checkboxes; a descendant selector would throw all of their tooltips up at once when the row is hovered.""" css = _css_rules(client) assert '.field:hover > .tip' in css diff --git a/webadmin/forms.py b/webadmin/forms.py index 7890b6a..fa49b9b 100644 --- a/webadmin/forms.py +++ b/webadmin/forms.py @@ -150,18 +150,28 @@ class _VideoOwnerFields: video_srt_1 = BooleanField('Slot 1: UDP side speaks SRT (else MPEG-TS)') video_record_1 = BooleanField('Slot 1: record to disk') video_rawtcp_1 = BooleanField('Slot 1: allow raw-TCP viewers (no password)') + video_sessok_1 = BooleanField( + 'Slot 1: publish with no password when the MAVLink session matches') video_srt_2 = BooleanField('Slot 2: UDP side speaks SRT (else MPEG-TS)') video_record_2 = BooleanField('Slot 2: record to disk') video_rawtcp_2 = BooleanField('Slot 2: allow raw-TCP viewers (no password)') + video_sessok_2 = BooleanField( + 'Slot 2: publish with no password when the MAVLink session matches') video_srt_3 = BooleanField('Slot 3: UDP side speaks SRT (else MPEG-TS)') video_record_3 = BooleanField('Slot 3: record to disk') video_rawtcp_3 = BooleanField('Slot 3: allow raw-TCP viewers (no password)') + video_sessok_3 = BooleanField( + 'Slot 3: publish with no password when the MAVLink session matches') video_srt_4 = BooleanField('Slot 4: UDP side speaks SRT (else MPEG-TS)') video_record_4 = BooleanField('Slot 4: record to disk') video_rawtcp_4 = BooleanField('Slot 4: allow raw-TCP viewers (no password)') + video_sessok_4 = BooleanField( + 'Slot 4: publish with no password when the MAVLink session matches') video_srt_5 = BooleanField('Slot 5: UDP side speaks SRT (else MPEG-TS)') video_record_5 = BooleanField('Slot 5: record to disk') video_rawtcp_5 = BooleanField('Slot 5: allow raw-TCP viewers (no password)') + video_sessok_5 = BooleanField( + 'Slot 5: publish with no password when the MAVLink session matches') class _VideoAdminFields(_VideoOwnerFields): diff --git a/webadmin/templates/_video_fields.html b/webadmin/templates/_video_fields.html index f49555c..3e4696b 100644 --- a/webadmin/templates/_video_fields.html +++ b/webadmin/templates/_video_fields.html @@ -72,6 +72,14 @@

Video

nowhere in a raw stream to carry a password, so this is offered only when no viewer password is set. + {{ form['video_sessok_' ~ slot]() }} MAVLink publish + + Accept a publisher on this slot that the entry's MAVLink session + authorises, even though a publish password is set. For a camera + that speaks RTMP from its own firmware, or plain MPEG-TS over + UDP, with nowhere to put a credential. A password that is given + and is wrong is still refused. + RTMP {{ form['video_rtmp_' ~ slot](size=16, placeholder='app/stream') }} diff --git a/webadmin/videoform.py b/webadmin/videoform.py index 430bfdc..8092b88 100644 --- a/webadmin/videoform.py +++ b/webadmin/videoform.py @@ -11,6 +11,7 @@ ('video_srt_%d', keydb_lib.VIDEO_SLOT_SRT), ('video_record_%d', keydb_lib.VIDEO_SLOT_RECORD), ('video_rawtcp_%d', keydb_lib.VIDEO_SLOT_RAW_TCP), + ('video_sessok_%d', keydb_lib.VIDEO_SLOT_SESSION_OK), ) From 2175f444b2661e9a74398490bdffa40ec14f0052 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 19 Aug 2026 11:23:38 +1000 Subject: [PATCH 05/10] webadmin: drop the tooltip from the login passphrase field Pasting into it with the tooltip showing wedges Chrome's renderer: the tab stops responding to input entirely, and it does not recover. The text it carried moves into the blurb above the form, which already introduced the field, so nothing is lost. The forms.py "every option is documented" guard exempts the field and records why, so it cannot be quietly reinstated. --- tests/webadmin/test_tooltips.py | 6 +++++- webadmin/forms.py | 6 ++++-- webadmin/templates/login.html | 3 ++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/webadmin/test_tooltips.py b/tests/webadmin/test_tooltips.py index 53e5621..f93e8d2 100644 --- a/tests/webadmin/test_tooltips.py +++ b/tests/webadmin/test_tooltips.py @@ -19,7 +19,11 @@ # _video_fields.html instead: five slots x four options would mean twenty # near-identical strings in forms.py, and the row is rendered by hand # there anyway. -_NO_DESCRIPTION_NEEDED = {'submit', 'csrf_token'} +# 'passphrase' is the login password field. A tooltip on it froze +# Chrome's renderer on paste -- the tab stopped accepting input at all -- +# so it deliberately carries no description and its text moved into the +# blurb above the form. +_NO_DESCRIPTION_NEEDED = {'submit', 'csrf_token', 'passphrase'} def _documented(form_cls): diff --git a/webadmin/forms.py b/webadmin/forms.py index fa49b9b..cd46e9e 100644 --- a/webadmin/forms.py +++ b/webadmin/forms.py @@ -229,10 +229,12 @@ class LoginForm(FlaskForm): 'you get the engineer view; either way you reach the ' 'same entry.', validators=[DataRequired(), NumberRange(min=1, max=65535)]) + # No description, so no tooltip: pasting into this field with the + # tooltip up wedges Chrome's renderer hard enough that the tab stops + # responding to input entirely. The text it carried is in the blurb + # above the form instead, where it costs nothing. passphrase = PasswordField( 'Passphrase', - description='The shared MAVLink passphrase for this entry -- the ' - 'same one set with keydb.py, not a separate web login.', validators=[DataRequired(), Length(min=1, max=256)], render_kw=_CURRENT_PW_KW) submit = SubmitField('Log in') diff --git a/webadmin/templates/login.html b/webadmin/templates/login.html index 68babad..b675658 100644 --- a/webadmin/templates/login.html +++ b/webadmin/templates/login.html @@ -4,7 +4,8 @@ {% block content %}

Log in

Enter the user-side port (port1) or the engineer-side port (port2) - for your entry, plus the passphrase you set with keydb.py.

+ for your entry, plus the shared MAVLink passphrase you set with + keydb.py — not a separate web login.

{{ form.csrf_token }} {{ row(form.port, autofocus=True) }} From 348b1c5ec740768ded51dc11fe41342a04a74c90 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 19 Aug 2026 11:31:11 +1000 Subject: [PATCH 06/10] webadmin: no hover tooltip on any password field A tooltip over a password field wedges Chrome's renderer: the tab stops accepting input and does not recover. Reproduced on the login form and removed there; this covers the rest, which share the markup and so presumably share the fault. The help itself is worth keeping -- the publish-password text explains that it replaces the address check, and the new-passphrase one that blank means unchanged -- so the macro renders a password field's description in flow as .field-hint rather than dropping it. The dotted underline that advertises a tooltip goes with it, and aria-describedby is now emitted only when there is something to point at. Two guards: no password input on any page carries a .tip, and the text still reaches the page. Both verified RED. --- tests/webadmin/test_tooltips.py | 31 +++++++++++++++++++++++++++++++ webadmin/static/style.css | 11 +++++++++++ webadmin/templates/_macros.html | 20 +++++++++++++++----- 3 files changed, 57 insertions(+), 5 deletions(-) diff --git a/tests/webadmin/test_tooltips.py b/tests/webadmin/test_tooltips.py index f93e8d2..042b576 100644 --- a/tests/webadmin/test_tooltips.py +++ b/tests/webadmin/test_tooltips.py @@ -133,6 +133,37 @@ def test_video_slot_options_are_documented_in_the_template( assert 'ffplay tcp://' in html # raw TCP viewers assert 'nowhere to put a credential' in html # MAVLink publish + def test_no_password_field_gets_a_hover_tooltip(self, client, + keydb_path): + """A hover tooltip over a password field wedges Chrome's + renderer when you paste into it -- the tab stops accepting + input at all and does not recover. Their help renders in flow + as .field-hint instead, so this must hold on every page.""" + login_as(client, BOB_PORT1, BOB_PASS) + pages = ['/admin/%d' % ALICE_PORT2, '/admin/'] + client.get('/logout') + pages.append('/login') + seen_any = False + for url in pages: + if url != '/login': + login_as(client, BOB_PORT1, BOB_PASS) + html = client.get(url).get_data(as_text=True) + for pw_id in re.findall( + r']*type="password"[^>]*id="([^"]+)"', html) + \ + re.findall(r']*id="([^"]+)"[^>]*type="password"', + html): + seen_any = True + assert '{{ field.description }} + {%- if field.type == 'PasswordField' -%} + {#- A hover tooltip over a password field wedges Chrome's renderer + when you paste into it -- the tab stops taking input at all. + The text is worth keeping, so it renders in flow instead. -#} + {{ field.description }} + {%- else -%} + {{ field.description }} + {%- endif -%} {%- endif -%} {%- endmacro %} {# Label-then-input, for text/number/password/select fields. #} {% macro row(field) -%} -
- {{ field.label }}: {{ field(aria_describedby=field.id ~ '-tip', **kwargs) }}{{ tip(field) }} +
+ {# Only point at the help element when there is one to point at. #} + {{ field.label }}: {{ field(aria_describedby=field.id ~ '-tip', **kwargs) + if field.description else field(**kwargs) }}{{ tip(field) }}
{%- endmacro %} {# Checkbox-then-label, for BooleanFields. #} {% macro check(field) -%} -
- {{ field(aria_describedby=field.id ~ '-tip', **kwargs) }} {{ field.label }}{{ tip(field) }} +
+ {{ field(aria_describedby=field.id ~ '-tip', **kwargs) + if field.description else field(**kwargs) }} {{ field.label }}{{ tip(field) }}
{%- endmacro %} From a5c1e736f9beb26763db71d1babfa2aba1d75b30 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 19 Aug 2026 14:37:34 +1000 Subject: [PATCH 07/10] video: ignore unsupported RTMP data streams --- tests/test_video_rtsp.py | 38 ++++++++++++++++++++++++++++++++++++++ videortsp.cpp | 23 ++++++++++++++++++++++- 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/tests/test_video_rtsp.py b/tests/test_video_rtsp.py index 4e174ab..4549e0c 100644 --- a/tests/test_video_rtsp.py +++ b/tests/test_video_rtsp.py @@ -823,6 +823,44 @@ def test_media_pipelined_with_publish_is_kept(self, tmp_path, clip): pub.close() s.stop() + def test_repeated_script_tags_do_not_kill_the_backend(self, tmp_path, + clip): + """A publisher may re-send onMetaData throughout the stream. + + gstreamer's flvmux does exactly that rather than emitting it + once at the head, and ffmpeg's FLV demuxer surfaces the repeats + as a second, data stream. The backend mapped every input stream + into MPEG-TS, which has no encoder for that one, so ffmpeg died + on "Error selecting an encoder" before writing a byte: the slot + sat at 0 KiB with the backend apparently running. ffmpeg's own + FLV has no such stream, which is why publishing with ffmpeg + worked and the stock gst-launch pipeline never produced a frame. + """ + meta = (rtmp_client._amf_str('@setDataFrame') + + rtmp_client._amf_str('onMetaData') + + rtmp_client._amf_obj({'width': 1280.0, 'height': 720.0, + 'videocodecid': 7.0})) + tags = self._flv_of(clip, tmp_path) + wd = _workdir(tmp_path, record=True) + s = RtspSession(wd) + pub = None + try: + pub = rtmp_client.RtmpPublisher('127.0.0.1', VPORT) + pub.handshake() + pub.connect() + pub.publish(first_tags=tags[:1]) + for i, (ttype, ts, body) in enumerate(tags[1:]): + pub.send_tag(ttype, ts, body) + # Interleaved the way flvmux does, not just at the head. + if i % 5 == 0: + pub.send_tag(18, ts, meta) + time.sleep(0.004) + assert s.proxy.wait_for(r'join=ready', timeout=40), s.proxy.log + finally: + if pub: + pub.close() + s.stop() + def test_a_split_chunk_does_not_inflate_timestamps(self, tmp_path, clip): """A chunk header split from its payload must not double its delta. diff --git a/videortsp.cpp b/videortsp.cpp index 248c2de..661f215 100644 --- a/videortsp.cpp +++ b/videortsp.cpp @@ -235,8 +235,29 @@ bool RtspBackend::start(int port2, int slot, bool want_audio, } argv[n++] = "-i"; argv[n++] = url; + /* + Map the streams we can actually carry, not everything. + + "-map 0" took whatever the publisher offered, and a stream + MPEG-TS has no encoder for kills the whole output: ffmpeg + fails with "Error selecting an encoder" before writing a byte, + so the slot sits at 0 KiB with the backend apparently running. + + gstreamer walks straight into this. flvmux re-sends + onMetaData throughout the stream rather than once at the head, + and ffmpeg's FLV demuxer surfaces that as a second, data + stream -- so the stock gst-launch RTMP pipeline never + produced a frame here, while ffmpeg publishing the same video + worked, because its own FLV carries no such stream. + + "0:a?" is optional: no audio track is not an error. + */ argv[n++] = "-map"; - argv[n++] = "0"; + argv[n++] = "0:v:0"; + if (want_audio) { + argv[n++] = "-map"; + argv[n++] = "0:a?"; + } argv[n++] = "-c:v"; argv[n++] = "copy"; if (vbsf != nullptr && vbsf[0] != '\0') { From 1174f36b03aba0d1a7ec82d667e5555101c396bf Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 25 Aug 2026 09:00:03 +1000 Subject: [PATCH 08/10] webadmin: add configurable log access --- README.md | 15 ++++ keydb.h | 2 + keydb_lib.py | 32 ++++++++ tests/webadmin/test_log_routes.py | 117 +++++++++++++++++++++++++++++ webadmin/auth.py | 10 +++ webadmin/forms.py | 17 +++++ webadmin/logs.py | 80 +++++++++++++------- webadmin/routes_admin.py | 2 + webadmin/routes_owner.py | 2 + webadmin/templates/admin_edit.html | 3 +- webadmin/templates/admin_logs.html | 10 ++- webadmin/templates/owner.html | 8 +- 12 files changed, 268 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index fdf9409..7490527 100644 --- a/README.md +++ b/README.md @@ -414,6 +414,21 @@ proxy restart. - Everyone else gets the self-service UI (rename their own entry, rotate their own passphrase, reset their signing timestamp). +### Log access + +Each entry's edit page has a **Log access** setting: + +- **Private** (the default): only the entry owner and server admins can read + its logs. +- **Login Required**: any user with a valid SupportProxy login can browse and + download them. +- **Public**: anyone with the `/admin/logs//...` URL can browse and + download them without logging in. + +Shared access is read-only. Deleting individual recordings or a whole day +continues to require either the entry owner's `/me/logs/` view or a server +admin. Log responses remain non-cacheable even when the entry is Public. + ### Install dependencies ```bash diff --git a/keydb.h b/keydb.h index 1bcc8e6..b86e2b9 100644 --- a/keydb.h +++ b/keydb.h @@ -35,6 +35,8 @@ #define KEY_FLAG_BINLOG (1u << 3) // record ArduPilot bin logs over MAVLink #define KEY_FLAG_USE_TZ (1u << 4) // name logs with tz_offset_hours; else server local #define KEY_FLAG_VIDEO (1u << 5) // video proxying enabled for this entry +#define KEY_FLAG_LOG_LOGIN (1u << 6) // any authenticated web user may read logs +#define KEY_FLAG_LOG_PUBLIC (1u << 7) // anyone with the URL may read logs #define KEY_MAX_VIDEO_PORTS 5 diff --git a/keydb_lib.py b/keydb_lib.py index 2db8e78..b216a9d 100644 --- a/keydb_lib.py +++ b/keydb_lib.py @@ -51,6 +51,18 @@ FLAG_BINLOG = 1 << 3 # record ArduPilot bin logs over MAVLink FLAG_USE_TZ = 1 << 4 # name logs with tz_offset_hours; else server local FLAG_VIDEO = 1 << 5 # video proxying enabled for this entry +FLAG_LOG_LOGIN = 1 << 6 # any authenticated web user may read logs +FLAG_LOG_PUBLIC = 1 << 7 # anyone with the URL may read logs + +# Per-entry web log access. The two flag bits deliberately encode the wider +# policies independently: if an old/new CLI combination ever sets both, +# Public wins rather than unexpectedly making a shared URL private. +LOG_ACCESS_PRIVATE = 0 +LOG_ACCESS_LOGIN_REQUIRED = 1 +LOG_ACCESS_PUBLIC = 2 +LOG_ACCESS_CHOICES = (LOG_ACCESS_PRIVATE, LOG_ACCESS_LOGIN_REQUIRED, + LOG_ACCESS_PUBLIC) +LOG_ACCESS_MASK = FLAG_LOG_LOGIN | FLAG_LOG_PUBLIC FLAG_NAMES = { "admin": FLAG_ADMIN, @@ -59,6 +71,8 @@ "binlog": FLAG_BINLOG, "use_tz": FLAG_USE_TZ, "video": FLAG_VIDEO, + "log_login": FLAG_LOG_LOGIN, + "log_public": FLAG_LOG_PUBLIC, } DEFAULT_LOG_RETENTION_DAYS = 7.0 @@ -338,6 +352,24 @@ def passphrase_matches(self, passphrase): def is_admin(self): return bool(self.flags & FLAG_ADMIN) + def log_access(self): + """Who may read this entry's logs through the web UI.""" + if self.flags & FLAG_LOG_PUBLIC: + return LOG_ACCESS_PUBLIC + if self.flags & FLAG_LOG_LOGIN: + return LOG_ACCESS_LOGIN_REQUIRED + return LOG_ACCESS_PRIVATE + + def set_log_access(self, access): + """Set the web log access policy while preserving unrelated flags.""" + if access not in LOG_ACCESS_CHOICES: + raise ValueError("invalid log access policy: %r" % (access,)) + self.flags &= ~LOG_ACCESS_MASK + if access == LOG_ACCESS_LOGIN_REQUIRED: + self.flags |= FLAG_LOG_LOGIN + elif access == LOG_ACCESS_PUBLIC: + self.flags |= FLAG_LOG_PUBLIC + # --- video --------------------------------------------------------- def video_enabled(self): diff --git a/tests/webadmin/test_log_routes.py b/tests/webadmin/test_log_routes.py index 7153aec..6773274 100644 --- a/tests/webadmin/test_log_routes.py +++ b/tests/webadmin/test_log_routes.py @@ -39,11 +39,41 @@ def seed_session(logs_root, port2, date, session_name, content=b'TLOGDATA'): return f +def set_log_access(keydb_path, port2, access): + db = keydb_lib.open_db(keydb_path) + db.transaction_start() + ke = keydb_lib.KeyEntry(port2) + assert ke.fetch(db) + ke.set_log_access(access) + ke.store(db) + db.transaction_prepare_commit() + db.transaction_commit() + db.close() + + # --------------------------------------------------------------------------- # form: enable / disable / retention validation # --------------------------------------------------------------------------- class TestOwnerTlogForm: + def test_owner_can_make_logs_public(self, client, keydb_path): + login_as(client, ALICE_PORT1, ALICE_PASS) + resp = client.post('/me/', data={ + 'name': 'alice', + 'log_access': str(keydb_lib.LOG_ACCESS_PUBLIC), + 'submit': 'Save', + }) + assert resp.status_code == 302 + assert (fetch_entry(keydb_path, ALICE_PORT2).log_access() + == keydb_lib.LOG_ACCESS_PUBLIC) + + def test_owner_form_offers_all_log_access_choices(self, client): + login_as(client, ALICE_PORT1, ALICE_PASS) + body = client.get('/me/').get_data(as_text=True) + assert '>Private<' in body + assert '>Login Required<' in body + assert '>Public<' in body + def test_owner_enable_default_retention(self, client, keydb_path): login_as(client, ALICE_PORT1, ALICE_PASS) resp = client.post('/me/', data={ @@ -153,6 +183,18 @@ def test_owner_disable_keeps_retention(self, client, keydb_path): class TestAdminTlogForm: + def test_admin_can_require_login_for_logs(self, client, keydb_path): + login_as(client, BOB_PORT1, BOB_PASS) + resp = client.post('/admin/' + str(ALICE_PORT2), data={ + 'name': 'alice', + 'port1': str(ALICE_PORT1), + 'log_access': str(keydb_lib.LOG_ACCESS_LOGIN_REQUIRED), + 'submit': 'Save', + }) + assert resp.status_code == 302 + assert (fetch_entry(keydb_path, ALICE_PORT2).log_access() + == keydb_lib.LOG_ACCESS_LOGIN_REQUIRED) + def test_admin_can_set_high_retention(self, client, keydb_path): login_as(client, BOB_PORT1, BOB_PASS) # bob_admin edits alice's entry @@ -504,6 +546,81 @@ def test_admin_404_for_unknown_port2(self, client): assert r.status_code == 404 +class TestSharedLogAccess: + def test_public_listing_and_download_need_no_login(self, client, + keydb_path, + logs_dir): + seed_session(logs_dir, ALICE_PORT2, '2026-08-25', + 'session1.tlog', content=b'PUBLIC_LOG') + set_log_access(keydb_path, ALICE_PORT2, + keydb_lib.LOG_ACCESS_PUBLIC) + + base = '/admin/logs/%d/2026-08-25/' % ALICE_PORT2 + listing = client.get(base) + assert listing.status_code == 200 + assert b'session1.tlog' in listing.data + assert b'Read-only log access' in listing.data + assert b'edit entry' not in listing.data + assert b'/delete' not in listing.data + + download = client.get(base + 'session1.tlog') + assert download.status_code == 200 + assert download.data.endswith(b'PUBLIC_LOG') + + def test_public_video_playback_routes_need_no_login(self, client, + keydb_path, + logs_dir): + name = '2026_08_25_10:00:00.v1.ts' + seed_session(logs_dir, ALICE_PORT2, '2026-08-25', name, + content=b'\x47PUBLIC_VIDEO') + set_log_access(keydb_path, ALICE_PORT2, + keydb_lib.LOG_ACCESS_PUBLIC) + base = '/admin/logs/%d/2026-08-25/%s' % (ALICE_PORT2, name) + + assert client.get(base + '/watch').status_code == 200 + stream = client.get(base + '/stream') + assert stream.status_code == 200 + assert stream.data == b'\x47PUBLIC_VIDEO' + + def test_login_required_redirects_anonymous_reader(self, client, + keydb_path): + set_log_access(keydb_path, BOB_PORT2, + keydb_lib.LOG_ACCESS_LOGIN_REQUIRED) + url = '/admin/logs/%d/' % BOB_PORT2 + r = client.get(url, follow_redirects=False) + assert r.status_code == 302 + assert '/login' in r.location + assert 'next=' in r.location + + def test_login_required_accepts_any_valid_login_read_only(self, client, + keydb_path, + logs_dir): + seed_session(logs_dir, BOB_PORT2, '2026-08-25', + 'session2.bin', content=b'LOGIN_LOG') + set_log_access(keydb_path, BOB_PORT2, + keydb_lib.LOG_ACCESS_LOGIN_REQUIRED) + login_as(client, ALICE_PORT1, ALICE_PASS) + + base = '/admin/logs/%d/2026-08-25/' % BOB_PORT2 + listing = client.get(base) + assert listing.status_code == 200 + assert b'session2.bin' in listing.data + assert b'Read-only log access' in listing.data + assert b'/delete' not in listing.data + assert client.get(base + 'session2.bin').data.endswith(b'LOGIN_LOG') + + def test_shared_access_never_grants_delete(self, client, keydb_path, + logs_dir): + path = seed_session(logs_dir, ALICE_PORT2, '2026-08-25', + 'session1.tlog') + set_log_access(keydb_path, ALICE_PORT2, + keydb_lib.LOG_ACCESS_PUBLIC) + r = client.post('/admin/logs/%d/2026-08-25/session1.tlog/delete' + % ALICE_PORT2) + assert r.status_code == 403 + assert path.exists() + + class TestPathSafety: @pytest.mark.parametrize('bad', [ '../etc', # date with traversal diff --git a/webadmin/auth.py b/webadmin/auth.py index 5c35bdc..1938c93 100644 --- a/webadmin/auth.py +++ b/webadmin/auth.py @@ -49,6 +49,16 @@ def _refresh_role(): return ke +def current_entry(): + """Return the logged-in user's live entry, or None. + + Unlike current_owner(), this revalidates the session against keys.tdb and + refreshes its admin role. Routes with optional authentication use this + when anonymous access may also be valid. + """ + return _refresh_role() + + def is_admin(): """For *display* only (templates). Authorisation paths must call require_admin so the role is re-validated from keys.tdb.""" diff --git a/webadmin/forms.py b/webadmin/forms.py index cd46e9e..289e5e9 100644 --- a/webadmin/forms.py +++ b/webadmin/forms.py @@ -56,6 +56,10 @@ 'this. 0 keeps them forever. Fractional days are allowed. ' 'Video recordings are covered by their own disk budget, ' 'not by this.') +_D_LOG_ACCESS = ('Controls read-only access to this entry\'s log browser and ' + 'downloads. Private allows only this entry\'s owner and ' + 'server admins. Login Required also allows any user with a ' + 'SupportProxy login. Public allows anyone with the URL.') _D_SYSID = ('Restricts flight-controller reboot detection -- which is what ' 'starts a new .bin file -- to packets from this MAVLink system ' 'ID. 0 accepts any sysid, which is usually right unless several ' @@ -74,6 +78,17 @@ 'must use the new value, so tell them before you save.') +def _log_access_field(): + return SelectField( + 'Log access', description=_D_LOG_ACCESS, + choices=[ + (keydb_lib.LOG_ACCESS_PRIVATE, 'Private'), + (keydb_lib.LOG_ACCESS_LOGIN_REQUIRED, 'Login Required'), + (keydb_lib.LOG_ACCESS_PUBLIC, 'Public'), + ], + coerce=int, default=keydb_lib.LOG_ACCESS_PRIVATE) + + class _VideoOwnerFields: """Video settings an owner may change. Ports are admin-only.""" video_enabled = BooleanField( @@ -272,6 +287,7 @@ class OwnerEditForm(FlaskForm, _VideoOwnerFields): 'admin for longer.' % OWNER_MAX_LOG_RETENTION_DAYS, validators=[Optional(), NumberRange(min=0.0, max=OWNER_MAX_LOG_RETENTION_DAYS)]) + log_access = _log_access_field() fc_sysid = IntegerField( 'Flight-controller MAVLink sysid (0 = any)', description=_D_SYSID, validators=[Optional(), NumberRange(min=0, max=255)]) @@ -325,6 +341,7 @@ class AdminEditForm(FlaskForm, _VideoAdminFields): 'Log retention (days, 0 = keep forever)', description=_D_RETENTION, validators=[Optional(), NumberRange(min=0.0, max=ADMIN_MAX_LOG_RETENTION_DAYS)]) + log_access = _log_access_field() fc_sysid = IntegerField( 'Flight-controller MAVLink sysid (0 = any)', description=_D_SYSID, validators=[Optional(), NumberRange(min=0, max=255)]) diff --git a/webadmin/logs.py b/webadmin/logs.py index 39099a2..8b613f8 100644 --- a/webadmin/logs.py +++ b/webadmin/logs.py @@ -11,12 +11,14 @@ Two parallel views, sharing the listing/download helpers below: - * admin: /admin/logs//[] — admin can browse any entry + * shared: /admin/logs//[] — admin access, plus the entry's + configured Private / Login Required / Public read policy * owner: /me/logs/[] — owner can browse only their own -The blueprints differ only in which port2 they resolve and which auth -decorator they use. +Only admins can mutate logs through the shared namespace; widened entry +access is always read-only. """ +import functools import os import re import shutil @@ -24,12 +26,12 @@ import subprocess import time -from flask import (Blueprint, Response, abort, current_app, flash, redirect, - render_template, send_from_directory, url_for) +from flask import (Blueprint, Response, abort, current_app, flash, g, redirect, + render_template, request, send_from_directory, url_for) import keydb_lib -from .auth import current_owner, require_admin, require_login +from .auth import current_entry, current_owner, require_admin, require_login from .forms import DeleteLogForm from .db import tdb_readonly @@ -272,6 +274,37 @@ def _entry_label(port2): return ke +def require_log_read(view): + """Allow an admin, or a reader admitted by the entry's log policy. + + These are the existing /admin/logs// URLs so shared links remain + stable. Only GET views use this decorator; deletion stays behind + require_admin regardless of the configured read policy. + """ + @functools.wraps(view) + def wrapper(port2, *args, **kwargs): + entry = _entry_label(port2) + if entry is None: + abort(404) + + viewer = current_entry() + can_manage = viewer is not None and viewer.is_admin() + access = entry.log_access() + if access == keydb_lib.LOG_ACCESS_PRIVATE and not can_manage: + abort(403) + if (access == keydb_lib.LOG_ACCESS_LOGIN_REQUIRED + and viewer is None): + return redirect(url_for('auth.login', next=request.path)) + + # Avoid a second database read in each listing/playback view and give + # the template an explicit capability rather than trusting session + # presentation state for security-sensitive controls. + g.log_entry = entry + g.can_manage_logs = can_manage + return view(port2, *args, **kwargs) + return wrapper + + def _list_dates(port2): """All date subdirs under logs//, newest first. @@ -450,48 +483,43 @@ def _send_session_file(port2, date, session_name): @admin_bp.route('//', methods=['GET']) -@require_admin +@require_log_read def admin_dates(port2): - entry = _entry_label(port2) - if entry is None: - abort(404) return render_template('admin_logs.html', - entry=entry, dates=_list_dates(port2), + entry=g.log_entry, dates=_list_dates(port2), date=None, sessions=None, - del_form=DeleteLogForm()) + can_manage=g.can_manage_logs, + del_form=(DeleteLogForm() + if g.can_manage_logs else None)) @admin_bp.route('///', methods=['GET']) -@require_admin +@require_log_read def admin_sessions(port2, date): _safe_date(date) - entry = _entry_label(port2) - if entry is None: - abort(404) return render_template('admin_logs.html', - entry=entry, dates=_list_dates(port2), + entry=g.log_entry, dates=_list_dates(port2), date=date, sessions=_list_sessions(port2, date), - del_form=DeleteLogForm()) + can_manage=g.can_manage_logs, + del_form=(DeleteLogForm() + if g.can_manage_logs else None)) @admin_bp.route('///', methods=['GET']) -@require_admin +@require_log_read def admin_download(port2, date, session_name): return _send_session_file(port2, date, session_name) @admin_bp.route('////watch', methods=['GET']) -@require_admin +@require_log_read def admin_watch(port2, date, session_name): _safe_date(date) _safe_session(session_name) if not _is_video(session_name): abort(404) - entry = _entry_label(port2) - if entry is None: - abort(404) return render_template( - 'log_play.html', entry=entry, date=date, name=session_name, + 'log_play.html', entry=g.log_entry, date=date, name=session_name, stream_url=url_for('admin_logs.admin_stream', port2=port2, date=date, session_name=session_name), mp4_url=url_for('admin_logs.admin_play_mp4', port2=port2, @@ -504,14 +532,14 @@ def admin_watch(port2, date, session_name): @admin_bp.route('////stream', methods=['GET']) -@require_admin +@require_log_read def admin_stream(port2, date, session_name): return _send_session_inline(port2, date, session_name) @admin_bp.route('////play.mp4', methods=['GET']) -@require_admin +@require_log_read def admin_play_mp4(port2, date, session_name): return _remux_response(port2, date, session_name) diff --git a/webadmin/routes_admin.py b/webadmin/routes_admin.py index a326457..98816d0 100644 --- a/webadmin/routes_admin.py +++ b/webadmin/routes_admin.py @@ -195,6 +195,7 @@ def edit(port2): ke.flags &= ~keydb_lib.FLAG_BINLOG if form.log_retention_days.data is not None: ke.log_retention_days = float(form.log_retention_days.data) + ke.set_log_access(form.log_access.data) # First-enable default for either recording flag. just_enabled = ((form.tlog_enabled.data and not was_tlog) or (form.binlog_enabled.data and not was_binlog)) @@ -236,6 +237,7 @@ def edit(port2): form.tlog_enabled.data = bool(ke.flags & keydb_lib.FLAG_TLOG) form.binlog_enabled.data = bool(ke.flags & keydb_lib.FLAG_BINLOG) form.log_retention_days.data = ke.log_retention_days + form.log_access.data = ke.log_access() form.fc_sysid.data = ke.fc_sysid form.tz_offset_hours.data = ke.tz_offset_hours form.use_tz.data = bool(ke.flags & keydb_lib.FLAG_USE_TZ) diff --git a/webadmin/routes_owner.py b/webadmin/routes_owner.py index 5727894..1a40b3c 100644 --- a/webadmin/routes_owner.py +++ b/webadmin/routes_owner.py @@ -69,6 +69,7 @@ def me(): ke.flags &= ~keydb_lib.FLAG_BINLOG if form.log_retention_days.data is not None: ke.log_retention_days = float(form.log_retention_days.data) + ke.set_log_access(form.log_access.data) # First-enable default: when either recording flag flips # from off to on and retention is still "keep forever", # seed 7 days so freshly-toggled flags have a reasonable @@ -111,6 +112,7 @@ def me(): form.tlog_enabled.data = bool(ke.flags & keydb_lib.FLAG_TLOG) form.binlog_enabled.data = bool(ke.flags & keydb_lib.FLAG_BINLOG) form.log_retention_days.data = ke.log_retention_days + form.log_access.data = ke.log_access() form.fc_sysid.data = ke.fc_sysid form.tz_offset_hours.data = ke.tz_offset_hours form.use_tz.data = bool(ke.flags & keydb_lib.FLAG_USE_TZ) diff --git a/webadmin/templates/admin_edit.html b/webadmin/templates/admin_edit.html index 908805a..d0aa62f 100644 --- a/webadmin/templates/admin_edit.html +++ b/webadmin/templates/admin_edit.html @@ -20,10 +20,11 @@

Edit entry {{ entry.port1 }}/{{ entry.port2 }}

{{ check(form.tlog_enabled) }} {{ check(form.binlog_enabled) }} {{ row(form.log_retention_days) }} + {{ row(form.log_access) }} {{ row(form.fc_sysid) }} {{ check(form.use_tz) }} {{ row(form.tz_offset_hours) }} -
browse logs →
+
browse / share logs →
{% include "_video_fields.html" %} {{ check(form.reset_timestamp) }}
{{ form.submit() }}
diff --git a/webadmin/templates/admin_logs.html b/webadmin/templates/admin_logs.html index 004288b..a1eb059 100644 --- a/webadmin/templates/admin_logs.html +++ b/webadmin/templates/admin_logs.html @@ -6,11 +6,15 @@

Logs for {{ entry.port1 }}/{{ entry.port2 }} '{{ entry.name }}'

+ {% if can_manage %} edit entry · all entries {% if entry.video_enabled() and entry.active_video_ports() %} · watch live video → {% endif %} + {% else %} + Read-only log access + {% endif %}

Dates

@@ -39,13 +43,13 @@

Sessions on {{ date }}

{{ icon('download') }} - {{ del_form.csrf_token }} - + {% endif %} {% if s.is_video %} {{ icon('play') }} @@ -57,6 +61,7 @@

Sessions on {{ date }}

{% endfor %} +{% if can_manage %}
@@ -64,4 +69,5 @@

Sessions on {{ date }}

{% endif %} +{% endif %} {% endblock %} diff --git a/webadmin/templates/owner.html b/webadmin/templates/owner.html index c2ac101..a4f10ad 100644 --- a/webadmin/templates/owner.html +++ b/webadmin/templates/owner.html @@ -55,10 +55,16 @@

My entry

{{ check(form.tlog_enabled) }} {{ check(form.binlog_enabled) }} {{ row(form.log_retention_days) }} + {{ row(form.log_access) }} {{ row(form.fc_sysid) }} {{ check(form.use_tz) }} {{ row(form.tz_offset_hours) }} -
browse logs →
+
+ browse logs → + {% if entry.log_access() %} + · shared read-only link → + {% endif %} +
{% include "_video_fields.html" %} {{ check(form.reset_timestamp) }}
{{ form.submit() }}
From a9ab58d57120c5dfb378ef0ffb1a6edfda27b8a8 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 25 Aug 2026 18:13:15 +1000 Subject: [PATCH 09/10] video: harden publish session fallback Preserve the distinction between absent and supplied publish credentials across RTSP and RTMP parsing, and export signed MAVLink authentication state for bidi session fallback. Update stale five-slot schema documentation and add regression coverage. --- httpreq.cpp | 50 +++++++++++++++++++------------ httpreq.h | 9 ++++++ keydb_lib.py | 15 +++++----- supportproxy.cpp | 1 + tests/test_keydb_log.py | 16 +++++----- tests/test_video_child.py | 34 +++++++++++++++++++++ tests/test_video_rtsp.py | 62 +++++++++++++++++++++++++++++++++++++++ video.cpp | 50 +++++++++++++++++++------------ videoauth.cpp | 20 ++++++++----- videoauth.h | 19 +++++++----- videortmp.cpp | 20 ++++++++----- videortmp.h | 2 ++ videoview.cpp | 14 ++++++++- 13 files changed, 235 insertions(+), 77 deletions(-) diff --git a/httpreq.cpp b/httpreq.cpp index e844208..9580d22 100644 --- a/httpreq.cpp +++ b/httpreq.cpp @@ -112,24 +112,9 @@ std::string HttpRequest::header(const char *name) const std::string HttpRequest::query(const char *name) const { - const std::string want = name; - size_t pos = 0; - while (pos <= query_.size()) { - size_t amp = query_.find('&', pos); - if (amp == std::string::npos) { - amp = query_.size(); - } - const std::string kv = query_.substr(pos, amp - pos); - const size_t eq = kv.find('='); - if (eq != std::string::npos && kv.compare(0, eq, want) == 0) { - return http_url_decode(kv.substr(eq + 1)); - } - if (amp == query_.size()) { - break; - } - pos = amp + 1; - } - return ""; + std::string value; + (void)http_query_value(target_, name, value); + return value; } std::string http_url_decode(const std::string &s) @@ -152,6 +137,35 @@ std::string http_url_decode(const std::string &s) return o; } +bool http_query_value(const std::string &target, const char *name, + std::string &value) +{ + value.clear(); + const size_t q = target.find('?'); + if (q == std::string::npos) { + return false; + } + const std::string want = name; + size_t pos = q + 1; + while (pos <= target.size()) { + size_t amp = target.find('&', pos); + if (amp == std::string::npos) { + amp = target.size(); + } + const std::string kv = target.substr(pos, amp - pos); + const size_t eq = kv.find('='); + if (eq != std::string::npos && kv.compare(0, eq, want) == 0) { + value = http_url_decode(kv.substr(eq + 1)); + return true; // first value wins; duplicates cannot erase it + } + if (amp == target.size()) { + break; + } + pos = amp + 1; + } + return false; +} + std::string http_basic_password(const std::string &authorization) { const std::string prefix = "Basic "; diff --git a/httpreq.h b/httpreq.h index 712b506..44424d0 100644 --- a/httpreq.h +++ b/httpreq.h @@ -55,6 +55,15 @@ class HttpRequest { // escapes are left as-is rather than silently dropped. std::string http_url_decode(const std::string &s); +/* + Fetch the first named query parameter from a request target. The boolean + distinguishes an absent parameter from one explicitly supplied with an + empty value; callers making access-control decisions must not collapse the + two. `value` is percent-decoded when present. + */ +bool http_query_value(const std::string &target, const char *name, + std::string &value); + /* A request target with credential query values replaced. diff --git a/keydb_lib.py b/keydb_lib.py index b216a9d..1007cef 100644 --- a/keydb_lib.py +++ b/keydb_lib.py @@ -24,13 +24,12 @@ # Pre-flags layout was 96 bytes. Anything smaller is invalid; anything bigger # is acceptable (extra trailing bytes belong to a newer schema we ignore). # -# The current C++ struct ends with the video fields -- ports, flags, the -# viewer and publish keys, the quota, the MAVLink grace window, the per-slot -# RTMP paths -- and `uint32_t reserved[12]`. All are 4-byte aligned and slot in -# cleanly after the existing fields, so the struct is 344 bytes with no -# trailing pad. When a future field is added, claim -# another `reserved[]` slot (renumber: shrink reserved by 1, add a named field) -# so the on-disk byte layout stays compatible — the zero-init paths in +# The current C++ struct carries three video slots inline, then the original +# `uint32_t reserved[12]`, followed by the two appended slots and +# `uint32_t reserved2[9]`. All are 4-byte aligned, so the struct is 456 bytes +# with no trailing pad. When a future field is added, claim another trailing +# `reserved2[]` slot (renumber: shrink reserved2 by 1, add a named field) so +# the on-disk byte layout stays compatible — the zero-init paths in # db_load_key (C++) and unpack() (Python) handle older records transparently. # tz_offset_hours took a slot that was previously a zeroed reserved word, so # older records read back as 0.0 with the KEY_FLAG_USE_TZ bit clear — i.e. @@ -837,7 +836,7 @@ def validate_video_ports(db, ke, ports): def set_video_ports(db, port2, ports): - """Set this entry's video ports. `ports` is a list of up to 3 ints; + """Set this entry's video ports. `ports` is a list of up to 5 ints; 0 (or a short list) leaves the remaining slots unused.""" ke = KeyEntry(port2) if not ke.fetch(db): diff --git a/supportproxy.cpp b/supportproxy.cpp index 64f907a..bc987f1 100644 --- a/supportproxy.cpp +++ b/supportproxy.cpp @@ -1097,6 +1097,7 @@ static void main_loop(struct listen_port *p) e.transport = mav1_is_tcp ? CONN_TRANSPORT_TCP : CONN_TRANSPORT_UDP; } e.is_user = 1; + e.authenticated = mav1.is_authenticated() ? 1 : 0; if (drop_mask & 1u) { e.flags |= CONN_FLAG_DROP_REQUESTED; } diff --git a/tests/test_keydb_log.py b/tests/test_keydb_log.py index 76eceab..a54fbf1 100644 --- a/tests/test_keydb_log.py +++ b/tests/test_keydb_log.py @@ -20,14 +20,14 @@ KEYDB_PY = os.path.join(_REPO_ROOT, 'keydb.py') -def test_pack_format_size_is_248(): - """The on-disk record is 248 bytes after appending the video fields. +def test_pack_format_size_is_456(): + """The on-disk record is 456 bytes after appending five-slot video. keydb.h carries a matching static_assert, so this catches either side drifting from the other. """ - assert struct.calcsize(keydb_lib.PACK_FORMAT) == 344 - assert keydb_lib.KEYENTRY_CURRENT_SIZE == 344 + assert struct.calcsize(keydb_lib.PACK_FORMAT) == 456 + assert keydb_lib.KEYENTRY_CURRENT_SIZE == 456 def test_pack_unpack_roundtrip(): @@ -38,7 +38,7 @@ def test_pack_unpack_roundtrip(): e.flags = keydb_lib.FLAG_TLOG | keydb_lib.FLAG_ADMIN e.log_retention_days = 0.0001 data = e.pack() - assert len(data) == 344 + assert len(data) == 456 e2 = keydb_lib.KeyEntry(0) e2.unpack(data) @@ -81,9 +81,9 @@ def test_legacy_104byte_record_zero_extends(): assert decoded.video_flags == 0 assert not decoded.video_viewer_pass_set() - # Re-pack: should emit the full 248-byte modern layout. + # Re-pack: should emit the full 456-byte modern layout. re = decoded.pack() - assert len(re) == 344 + assert len(re) == 456 def test_forward_compat_tail_is_preserved(): @@ -102,7 +102,7 @@ def test_forward_compat_tail_is_preserved(): assert decoded._tail == extra re = decoded.pack() assert re.endswith(extra) - assert len(re) == 344 + len(extra) + assert len(re) == 456 + len(extra) def test_flag_names_includes_tlog(): diff --git a/tests/test_video_child.py b/tests/test_video_child.py index 3a649aa..fc1849b 100644 --- a/tests/test_video_child.py +++ b/tests/test_video_child.py @@ -54,6 +54,8 @@ def _make_workdir(tmp_path, flags=('video',), vports=(VPORT,), **kw): keydb_lib.set_video_ports(db, PORT_ENG, list(vports)) if kw.get('publish_pass'): keydb_lib.set_video_publish_pass(db, PORT_ENG, kw['publish_pass']) + if kw.get('session_ok'): + keydb_lib.set_video_slot_flag(db, PORT_ENG, 0, 'session_ok') if kw.get('grace') is not None: keydb_lib.set_video_grace(db, PORT_ENG, kw['grace']) db.transaction_prepare_commit() @@ -450,6 +452,38 @@ def test_bidi_entry_requires_authenticated_session(self, proxy): finally: mav.stop() + def test_bidi_signed_session_authorises_flagged_password_slot(self, proxy): + """The signed state exported to connections.tdb is what lets the + independent video child use session fallback on a bidi entry.""" + p = proxy(flags=('video', 'bidi_sign'), publish_pass='pubpw', + session_ok=True) + assert p.wait_for(r'video slot 0 listening'), p.log + mav = _Mav(signed=True) + try: + assert p.wait_for(r'have UDP conn1'), p.log + deadline = time.time() + 12 + authenticated = False + conn_path = conntdb_lib.conn_path_for( + str(p.workdir / 'keys.tdb')) + while time.time() < deadline: + rows = conntdb_lib.list_active(conn_path, max_age_s=60) + users = [r for r in rows if r.conn_index == 0] + if users and users[0].authenticated: + authenticated = True + break + time.sleep(0.3) + assert authenticated, 'signed state was not exported:\n%s' % p.log + + for _ in range(10): + _send_ts(VPORT, n=2) + if re.search(r'video slot 0 publisher', p.log): + break + time.sleep(0.5) + assert re.search(r'video slot 0 publisher', p.log), p.log + assert 'not signature-validated' not in p.log + finally: + mav.stop() + @pytest.mark.integration class TestVideoConnRows: diff --git a/tests/test_video_rtsp.py b/tests/test_video_rtsp.py index 4549e0c..32308d0 100644 --- a/tests/test_video_rtsp.py +++ b/tests/test_video_rtsp.py @@ -494,6 +494,40 @@ def test_a_wrong_password_is_still_refused_on_a_flagged_slot(self, s.proxy.log assert 'RTSP publisher' not in s.proxy.log + @pytest.mark.parametrize('target', [ + '/cam?mode=x&pw=wrong', + '/cam?pw=%00wrong', + ]) + def test_malformed_or_nonfirst_password_never_falls_back(self, session, + target): + """Presence is independent of decoded value and query position.""" + s = session(with_mav=True, publish_pass='pubsecret', session_ok=True) + sock = socket.create_connection(('127.0.0.1', VPORT), 5) + try: + req = ('OPTIONS rtsp://127.0.0.1:%d%s RTSP/1.0\r\n' + 'CSeq: 1\r\n\r\n' % (VPORT, target)).encode() + sock.sendall(req) + assert s.proxy.wait_for(r'wrong publish password', timeout=10), \ + s.proxy.log + assert 'RTSP publisher' not in s.proxy.log + finally: + sock.close() + + def test_fragmented_request_line_waits_for_the_credential(self, session): + """Do not authorise from a TCP prefix before ?pw= has arrived.""" + s = session(with_mav=True, publish_pass='pubsecret', session_ok=True) + sock = socket.create_connection(('127.0.0.1', VPORT), 5) + try: + sock.sendall(('OPTIONS rtsp://127.0.0.1:%d/cam' % VPORT).encode()) + time.sleep(1.0) + assert 'RTSP publisher' not in s.proxy.log, s.proxy.log + sock.sendall(b'?pw=wrong RTSP/1.0\r\nCSeq: 1\r\n\r\n') + assert s.proxy.wait_for(r'wrong publish password', timeout=10), \ + s.proxy.log + assert 'RTSP publisher' not in s.proxy.log + finally: + sock.close() + def test_offering_none_falls_back_on_a_flagged_slot(self, session, clip): s = session(with_mav=True, publish_pass='pubsecret', session_ok=True) s.pub = _publish_with('', clip) @@ -1028,6 +1062,34 @@ def test_publish_password_refuses_when_absent(self, tmp_path, clip): finally: s.stop() + @pytest.mark.parametrize('stream', [ + 'FPV?pw=wrong&pw=', + 'FPV?pw=%00wrong', + ]) + def test_supplied_rtmp_password_cannot_become_absent(self, tmp_path, + stream): + """Duplicates and decoded NULs remain supplied wrong credentials. + + This uses the MAVLink fallback so the pre-fix collapse to an empty + C string would be observable as successful publishing. + """ + wd = _workdir(tmp_path, publish_pass='secret', session_ok=True) + s = RtspSession(wd, with_mav=True) + pub = None + try: + pub = rtmp_client.RtmpPublisher( + '127.0.0.1', VPORT, app='PhoenixFPV', stream=stream) + pub.handshake() + pub.connect() + pub.publish() + assert s.proxy.wait_for(r'wrong publish password', timeout=15), \ + s.proxy.log + assert 'RTMP publishing' not in s.proxy.log + finally: + if pub: + pub.close() + s.stop() + def test_no_orphan_backend_after_the_rtmp_publisher_leaves( self, tmp_path, clip): s = RtspSession(_workdir(tmp_path, rtmp_path='PhoenixFPV/FPV')) diff --git a/video.cpp b/video.cpp index 9746341..f6e0baf 100644 --- a/video.cpp +++ b/video.cpp @@ -440,7 +440,7 @@ void VideoChild::handle_udp(Slot &s, int idx) // session_ok, in which case UDP can't satisfy it and the datagram // is refused. video_admit_t r = auth_.admit(ke_, uint32_t(from.sin_addr.s_addr), - nullptr, session_ok(idx), now); + nullptr, false, session_ok(idx), now); if (r != VIDEO_ADMIT_OK) { log_reject(s, idx, uint32_t(from.sin_addr.s_addr), r, now); return; @@ -636,29 +636,40 @@ void VideoChild::handle_rtsp(Slot &s, int idx, int fd, configured on an aircraft rather than typed into a browser, so it does not end up in history or a Referer header. */ - uint8_t line[512] {}; + uint8_t line[2048] {}; std::string pw; - const ssize_t ln = ::recv(fd, line, sizeof(line) - 1, MSG_PEEK); - if (ln > 0) { - const std::string req(reinterpret_cast(line), size_t(ln)); - const size_t eol = req.find('\r'); - const std::string first = req.substr(0, eol == std::string::npos - ? req.size() : eol); - const size_t q = first.find("?pw="); - if (q != std::string::npos) { - size_t end = first.find_first_of(" &", q + 4); - if (end == std::string::npos) { - end = first.size(); - } - pw = http_url_decode(first.substr(q + 4, end - (q + 4))); - } + bool pw_present = false; + const ssize_t ln = ::recv(fd, line, sizeof(line), MSG_PEEK); + if (ln <= 0) { + close(fd); // fail closed if the peek changed under us + return; + } + const std::string req(reinterpret_cast(line), size_t(ln)); + const size_t eol = req.find('\n'); + if (eol == std::string::npos) { + close(fd); // detect phase promised a complete line + return; + } + std::string first = req.substr(0, eol); + if (!first.empty() && first.back() == '\r') { + first.pop_back(); } + const size_t sp1 = first.find(' '); + const size_t sp2 = sp1 == std::string::npos + ? std::string::npos : first.find(' ', sp1 + 1); + if (sp1 == std::string::npos || sp2 == std::string::npos) { + close(fd); + return; + } + const std::string target = first.substr(sp1 + 1, sp2 - sp1 - 1); + pw_present = http_query_value(target, "pw", pw); // Publishers are authorised; viewers are not, and on this port an // RTSP connection is a publisher (we do not parse enough to tell // them apart -- see videortsp.h). const video_admit_t r = auth_.admit(ke_, uint32_t(from.sin_addr.s_addr), - pw.c_str(), session_ok(idx), now); + pw_present ? &pw : nullptr, true, + session_ok(idx), now); if (r != VIDEO_ADMIT_OK) { log_reject(s, idx, uint32_t(from.sin_addr.s_addr), r, now); close(fd); @@ -819,8 +830,9 @@ bool VideoChild::promote_pending(Slot &s, int idx, PendingRtmp &p, { RtmpSession &r = *p.sess; - const video_admit_t a = auth_.admit(ke_, p.ip_be, r.password().c_str(), - session_ok(idx), now); + const video_admit_t a = auth_.admit( + ke_, p.ip_be, r.password_present() ? &r.password() : nullptr, true, + session_ok(idx), now); if (a != VIDEO_ADMIT_OK) { log_reject(s, idx, p.ip_be, a, now); r.reject_publish("NetStream.Publish.Denied", video_admit_str(a)); diff --git a/videoauth.cpp b/videoauth.cpp index 221a861..8073260 100644 --- a/videoauth.cpp +++ b/videoauth.cpp @@ -38,18 +38,21 @@ const char *video_admit_str(video_admit_t r) return "unknown"; } -bool video_password_matches(const uint8_t stored[32], const char *candidate) +bool video_password_matches(const uint8_t stored[32], + const std::string &candidate) { // An all-zero key is the "unset" sentinel, never a hash to match. bool any = false; for (int i = 0; i < 32; i++) { any |= stored[i] != 0; } - if (!any || candidate == nullptr || *candidate == '\0') { + if (!any || candidate.empty() + || candidate.find('\0') != std::string::npos) { return false; } uint8_t want[SHA256_DIGEST_LENGTH]; - SHA256((const unsigned char *)candidate, strlen(candidate), want); + SHA256(reinterpret_cast(candidate.data()), + candidate.size(), want); return CRYPTO_memcmp(want, stored, sizeof(want)) == 0; } @@ -103,7 +106,8 @@ video_admit_t VideoAuth::check_session(const struct KeyEntry &ke, } video_admit_t VideoAuth::admit(const struct KeyEntry &ke, uint32_t peer_ip_be, - const char *password, bool session_ok, + const std::string *password, + bool credential_capable, bool session_ok, time_t now) { // Path A: a publish password, when set, is sufficient on its own. @@ -118,8 +122,8 @@ video_admit_t VideoAuth::admit(const struct KeyEntry &ke, uint32_t peer_ip_be, on a typo would turn a clear rejection into a silent downgrade to address matching. */ - if (password != nullptr && *password != '\0') { - return video_password_matches(ke.video_publish_key, password) + if (password != nullptr) { + return video_password_matches(ke.video_publish_key, *password) ? VIDEO_ADMIT_OK : VIDEO_ADMIT_BAD_PASSWORD; } /* @@ -134,8 +138,8 @@ video_admit_t VideoAuth::admit(const struct KeyEntry &ke, uint32_t peer_ip_be, sent one. */ if (!session_ok) { - return password == nullptr ? VIDEO_ADMIT_NO_CREDENTIAL - : VIDEO_ADMIT_MISSING_PASSWORD; + return credential_capable ? VIDEO_ADMIT_MISSING_PASSWORD + : VIDEO_ADMIT_NO_CREDENTIAL; } } diff --git a/videoauth.h b/videoauth.h index fb48544..57bb5df 100644 --- a/videoauth.h +++ b/videoauth.h @@ -26,6 +26,8 @@ #include #include +#include + #include "keydb.h" enum video_admit_t { @@ -60,11 +62,12 @@ class VideoAuth { /* Decide whether `peer_ip_be` may publish. - `password` distinguishes three cases, and the difference is what - the operator sees in the log: - nullptr the transport cannot carry a credential at all (UDP) - "" it could, but none was supplied (RTSP with no ?pw=) - "..." a credential to check + `password` is non-null only when a credential was explicitly supplied. + It is length-aware, so an empty or embedded-NUL value is still an + offered (wrong) credential rather than silently becoming absent. + `credential_capable` distinguishes a transport that supplied none + (RTSP/RTMP) from one that cannot carry one at all (UDP), which is what + the operator sees in the rejection log. `session_ok` is the slot's VIDEO_SLOT_SESSION_OK bit: when set, a publisher that offered no credential falls back to path B even @@ -72,7 +75,8 @@ class VideoAuth { offered and is wrong is still refused. */ video_admit_t admit(const struct KeyEntry &ke, uint32_t peer_ip_be, - const char *password, bool session_ok, time_t now); + const std::string *password, bool credential_capable, + bool session_ok, time_t now); // Force the next lookup to re-read, e.g. after a config change. void invalidate(void) { fetched_at_ = 0; } @@ -107,7 +111,8 @@ class VideoAuth { // Constant-time compare of a candidate password against a stored // sha256. Returns false when the stored key is all-zero (unset). -bool video_password_matches(const uint8_t stored[32], const char *candidate); +bool video_password_matches(const uint8_t stored[32], + const std::string &candidate); /* Verify a short-lived viewer token minted by the web admin. diff --git a/videortmp.cpp b/videortmp.cpp index 4cc9577..6b81e91 100644 --- a/videortmp.cpp +++ b/videortmp.cpp @@ -275,11 +275,11 @@ bool amf_object_strings(const uint8_t *p, size_t n, size_t &i, credential. Cameras put the stream key in a single field, so a query on the stream name is the only place RTMP has to carry one. */ -void split_credential(std::string &name, std::string &pw) +bool split_credential(std::string &name, std::string &pw) { const size_t q = name.find_first_of("?&"); if (q == std::string::npos) { - return; + return false; } const std::string query = name.substr(q + 1); name.resize(q); @@ -295,10 +295,12 @@ void split_credential(std::string &name, std::string &pw) const std::string k = kv.substr(0, eq); if (k == "pw" || k == "password" || k == "key") { pw = http_url_decode(kv.substr(eq + 1)); + return true; // first credential wins; a duplicate cannot erase it } } at = end + 1; } + return false; } } // namespace @@ -735,12 +737,14 @@ bool RtmpSession::on_command(ChunkStream &c, const uint8_t *p, size_t n) connected_ = true; std::string tc_url; amf_object_strings(p, n, i, "app", app_, "tcUrl", tc_url); - split_credential(app_, password_); - if (password_.empty() && !tc_url.empty()) { + password_present_ = split_credential(app_, password_); + if (!password_present_ && !tc_url.empty()) { std::string ignored = tc_url; std::string pw; - split_credential(ignored, pw); - password_ = pw; + if (split_credential(ignored, pw)) { + password_ = pw; + password_present_ = true; + } } // Window Ack Size, Set Peer Bandwidth, Stream Begin, chunk size. const uint8_t win[4] = { 0x00, 0x26, 0x25, 0xa0 }; @@ -855,9 +859,9 @@ bool RtmpSession::on_command(ChunkStream &c, const uint8_t *p, size_t n) } stream_ = name; std::string pw; - split_credential(stream_, pw); - if (!pw.empty()) { + if (split_credential(stream_, pw)) { password_ = pw; + password_present_ = true; } publish_txn_ = txn; publish_sid_ = c.sid != 0 ? c.sid : 1; diff --git a/videortmp.h b/videortmp.h index 7b0488c..e12ed6c 100644 --- a/videortmp.h +++ b/videortmp.h @@ -150,6 +150,7 @@ class RtmpSession { const std::string &app(void) const { return app_; } const std::string &stream(void) const { return stream_; } const std::string &password(void) const { return password_; } + bool password_present(void) const { return password_present_; } // "app/stream", for matching against a configured path. std::string path(void) const; @@ -202,6 +203,7 @@ class RtmpSession { std::string app_; std::string stream_; std::string password_; + bool password_present_ = false; double publish_txn_ = 0; uint32_t publish_sid_ = 1; bool publishing_ = false; diff --git a/videoview.cpp b/videoview.cpp index a178224..76feb93 100644 --- a/videoview.cpp +++ b/videoview.cpp @@ -24,7 +24,7 @@ bool video_viewer_authorised(const struct KeyEntry &ke, if (!has_pw) { return true; // open viewing } - return video_password_matches(ke.video_viewer_key, password.c_str()); + return video_password_matches(ke.video_viewer_key, password); } void VideoViewer::start(int fd, int port2, uint32_t peer_ip_be, @@ -203,6 +203,18 @@ bool VideoViewer::on_readable(const struct KeyEntry &ke, int slot, for (const char *m : rtsp_methods) { const size_t len = strlen(m); if (size_t(n) >= len && memcmp(buf, m, len) == 0) { + // The request target carries the publish credential. Do not hand + // the socket off while that target can still be split across TCP + // segments: an early decision would turn a supplied password into + // "absent" and permit the session fallback. The peek is bounded, + // so a line that fills it without ending is malformed. + if (memchr(buf, '\n', size_t(n)) == nullptr) { + if (size_t(n) == sizeof(buf)) { + drop_reason_ = "RTSP request line too long"; + return false; + } + return true; + } kind_ = VVK_RTSP; return true; // the child takes the socket from here } From 42209cf0c7d7060e2d7e3fcc84f67c3dc56f3fb9 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Wed, 26 Aug 2026 21:45:37 +1000 Subject: [PATCH 10/10] video: close remaining credential downgrade paths Reject duplicate RTMP connect properties, preserve the first credential across RTMP setup commands, and continue validating credentials on every RTSP request. Bound RTSP framing, reject ambiguous content lengths, and cover the reported downgrade cases with integration tests. --- httpreq.cpp | 30 +++++++++ httpreq.h | 4 ++ tests/rtmp_client.py | 17 +++-- tests/test_video_rtsp.py | 70 ++++++++++++++++++++ video.cpp | 140 ++++++++++++++++++++++++++++++++++++++- videortmp.cpp | 29 ++++++-- 6 files changed, 281 insertions(+), 9 deletions(-) diff --git a/httpreq.cpp b/httpreq.cpp index 9580d22..92f80b6 100644 --- a/httpreq.cpp +++ b/httpreq.cpp @@ -110,6 +110,36 @@ std::string HttpRequest::header(const char *name) const return ""; } +size_t HttpRequest::header_count(const char *name) const +{ + const std::string want = lower(name); + size_t count = 0; + size_t pos = 0; + while (pos < headers_.size()) { + size_t eol = headers_.find('\n', pos); + if (eol == std::string::npos) { + eol = headers_.size(); + } + std::string line = headers_.substr(pos, eol - pos); + if (!line.empty() && line.back() == '\r') { + line.pop_back(); + } + const size_t colon = line.find(':'); + if (colon != std::string::npos) { + std::string field = line.substr(0, colon); + const size_t end = field.find_last_not_of(" \t"); + if (end != std::string::npos) { + field.resize(end + 1); + } + if (lower(field) == want) { + count++; + } + } + pos = eol + 1; + } + return count; +} + std::string HttpRequest::query(const char *name) const { std::string value; diff --git a/httpreq.h b/httpreq.h index 44424d0..9f9c640 100644 --- a/httpreq.h +++ b/httpreq.h @@ -32,6 +32,10 @@ class HttpRequest { // Header lookup, case-insensitive. Empty string when absent. std::string header(const char *name) const; + // Number of occurrences of a header, case-insensitive. Security + // parsers use this to reject ambiguous framing fields. + size_t header_count(const char *name) const; + // Query parameter from the request target. Empty when absent. std::string query(const char *name) const; diff --git a/tests/rtmp_client.py b/tests/rtmp_client.py index cc1f928..0165bfc 100644 --- a/tests/rtmp_client.py +++ b/tests/rtmp_client.py @@ -29,7 +29,8 @@ def _amf_null(): def _amf_obj(d): out = b'\x03' - for k, v in d.items(): + items = d.items() if hasattr(d, 'items') else d + for k, v in items: kb = k.encode() out += struct.pack('>H', len(kb)) + kb out += _amf_str(v) if isinstance(v, str) else _amf_num(v) @@ -117,7 +118,7 @@ def handshake(self): self.s.sendall(got[1:1537]) # C2 echoes S1 return got - def connect(self, password=None): + def connect(self, password=None, properties=None): stream = self.stream if password: stream = '%s?pw=%s' % (stream, password) @@ -127,9 +128,11 @@ def connect(self, password=None): # told otherwise. self.s.sendall(self._chunk(2, 1, 0, 0, self.out_chunk.to_bytes(4, 'big'))) + if properties is None: + properties = {'app': self.app, 'tcUrl': tc, + 'flashVer': 'test'} self.s.sendall(self._command( - _amf_str('connect') + _amf_num(1) + - _amf_obj({'app': self.app, 'tcUrl': tc, 'flashVer': 'test'}))) + _amf_str('connect') + _amf_num(1) + _amf_obj(properties))) self._drain() self.s.sendall(self._command( _amf_str('createStream') + _amf_num(2) + _amf_null())) @@ -138,6 +141,12 @@ def connect(self, password=None): _amf_str('publish') + _amf_num(3) + _amf_null() + _amf_str(stream) + _amf_str('live'))) + def fcpublish(self, stream): + self.s.sendall(self._command( + _amf_str('FCPublish') + _amf_num(0) + _amf_null() + + _amf_str(stream))) + self._drain() + def publish(self, first_tags=(), pipeline=False): """Send publish. With pipeline, media rides the same write. diff --git a/tests/test_video_rtsp.py b/tests/test_video_rtsp.py index 32308d0..14cabd2 100644 --- a/tests/test_video_rtsp.py +++ b/tests/test_video_rtsp.py @@ -388,6 +388,9 @@ class TestPublishPassword: """ def test_accepted_with_no_mavlink_session_at_all(self, session, clip): + # ffmpeg later resolves the SDP control URI as + # ?pw=pubsecret/streamid=0. Reaching join=ready therefore also + # covers the guard's exact-password-plus-control-path handling. s = session(with_mav=False, publish_pass='pubsecret') s.pub = _publish_with('?pw=pubsecret', clip) assert s.proxy.wait_for(r'RTSP publisher', timeout=25), s.proxy.log @@ -528,6 +531,43 @@ def test_fragmented_request_line_waits_for_the_credential(self, session): finally: sock.close() + def test_wrong_password_on_later_rtsp_request_is_refused(self, session): + """Session fallback on OPTIONS must not hide a credential later.""" + s = session(with_mav=True, publish_pass='pubsecret', session_ok=True) + sock = socket.create_connection(('127.0.0.1', VPORT), 5) + sock.settimeout(10) + try: + sock.sendall( + ('OPTIONS rtsp://127.0.0.1:%d/cam RTSP/1.0\r\n' + 'CSeq: 1\r\n\r\n' % VPORT).encode()) + assert b'RTSP/1.0 200' in sock.recv(4096) + sock.sendall( + ('ANNOUNCE rtsp://127.0.0.1:%d/cam?pw=wrong RTSP/1.0\r\n' + 'CSeq: 2\r\nContent-Length: 0\r\n\r\n' % VPORT).encode()) + assert s.proxy.wait_for(r'wrong publish password', timeout=10), \ + s.proxy.log + finally: + sock.close() + + def test_ambiguous_rtsp_body_length_is_refused(self, session): + """A framing disagreement must not hide a later credential.""" + s = session(with_mav=True, publish_pass='pubsecret', session_ok=True) + sock = socket.create_connection(('127.0.0.1', VPORT), 5) + sock.settimeout(10) + try: + sock.sendall( + ('OPTIONS rtsp://127.0.0.1:%d/cam RTSP/1.0\r\n' + 'CSeq: 1\r\n\r\n' % VPORT).encode()) + assert b'RTSP/1.0 200' in sock.recv(4096) + sock.sendall( + b'ANNOUNCE rtsp://127.0.0.1/cam RTSP/1.0\r\n' + b'CSeq: 2\r\nContent-Length: 64\r\n' + b'Content-Length: 0\r\n\r\n') + assert s.proxy.wait_for(r'RTSP publisher gone', timeout=10), \ + s.proxy.log + finally: + sock.close() + def test_offering_none_falls_back_on_a_flagged_slot(self, session, clip): s = session(with_mav=True, publish_pass='pubsecret', session_ok=True) s.pub = _publish_with('', clip) @@ -1090,6 +1130,36 @@ def test_supplied_rtmp_password_cannot_become_absent(self, tmp_path, pub.close() s.stop() + @pytest.mark.parametrize('source', ['duplicate_app', 'fcpublish']) + def test_earlier_rtmp_password_cannot_be_erased(self, tmp_path, source): + """Every pre-publish credential source preserves explicit presence.""" + wd = _workdir(tmp_path, publish_pass='secret', session_ok=True) + s = RtspSession(wd, with_mav=True) + pub = None + try: + pub = rtmp_client.RtmpPublisher( + '127.0.0.1', VPORT, app='PhoenixFPV', stream='FPV') + pub.handshake() + if source == 'duplicate_app': + pub.connect(properties=[ + ('app', 'PhoenixFPV?pw=wrong'), + ('app', 'PhoenixFPV'), + ('tcUrl', 'rtmp://127.0.0.1/PhoenixFPV'), + ]) + assert s.proxy.wait_for(r'duplicate connect property', + timeout=15), s.proxy.log + else: + pub.connect() + pub.fcpublish('FPV?pw=wrong') + pub.publish() + assert s.proxy.wait_for(r'wrong publish password', + timeout=15), s.proxy.log + assert 'RTMP publishing' not in s.proxy.log + finally: + if pub: + pub.close() + s.stop() + def test_no_orphan_backend_after_the_rtmp_publisher_leaves( self, tmp_path, clip): s = RtspSession(_workdir(tmp_path, rtmp_path='PhoenixFPV/FPV')) diff --git a/video.cpp b/video.cpp index f6e0baf..07081bb 100644 --- a/video.cpp +++ b/video.cpp @@ -9,6 +9,7 @@ */ #include "video.h" +#include #include #include #include @@ -116,6 +117,19 @@ struct SpliceQueue { // instead of this process buffering without limit. #define SPLICE_QUEUE_MAX (1u * 1024 * 1024) +/* + RTSP is relayed byte-for-byte, but admission credentials can occur on + any request URI in the session. Hold only the current request header + long enough to inspect its request line. Bodies and interleaved RTP are + counted and streamed without interpretation. + */ +struct RtspRequestGuard { + std::vector buffered; + size_t opaque_left = 0; + + void clear(void) { buffered.clear(); opaque_left = 0; } +}; + /* One RTMP handshake that has not published yet. @@ -155,6 +169,7 @@ struct Slot { int rtsp_client_fd = -1; SpliceQueue to_backend; // bytes read from the client, owed to ffmpeg SpliceQueue to_client; // and the other way + RtspRequestGuard rtsp_guard; /* The RTMP session that owns the slot, once one has published and been admitted. Null until then. @@ -227,6 +242,8 @@ class VideoChild { splice_proto_t proto); void close_rtsp(Slot &s, int idx, const char *why); bool pump_rtsp(Slot &s, int idx, int fd, time_t now); + bool guard_rtsp_requests(Slot &s, int idx, const uint8_t *buf, size_t n, + time_t now); bool pump_rtmp(Slot &s, int idx, int fd, time_t now); bool rtmp_start_backend(Slot &s, int idx); bool rtmp_drain_owner(Slot &s, int idx, bool alive); @@ -691,6 +708,7 @@ void VideoChild::handle_rtsp(Slot &s, int idx, int fd, } fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK); s.rtsp_client_fd = fd; + s.rtsp_guard.clear(); s.pub_ip_be = uint32_t(from.sin_addr.s_addr); s.pub_port_be = from.sin_port; latch_publisher(s, idx, now); @@ -734,6 +752,7 @@ void VideoChild::close_rtsp(Slot &s, int idx, const char *why) s.rtmp.reset(); s.to_backend.clear(); s.to_client.clear(); + s.rtsp_guard.clear(); s.rtsp.stop(); s.rec.close_segment(); s.recording = false; @@ -778,6 +797,119 @@ bool VideoChild::splice_flush(int to_fd, SpliceQueue &q) return true; } +/* + Filter the client-to-backend half of an RTSP splice. The first request + was already checked before the backend started, but a client can put a + credential on a later ANNOUNCE. Without continued inspection an absent + OPTIONS could take session fallback and a later wrong password would be + silently ignored. + */ +bool VideoChild::guard_rtsp_requests(Slot &s, int idx, const uint8_t *buf, + size_t n, time_t now) +{ + RtspRequestGuard &g = s.rtsp_guard; + g.buffered.insert(g.buffered.end(), buf, buf + n); + + while (!g.buffered.empty()) { + if (g.opaque_left > 0) { + const size_t take = std::min(g.opaque_left, g.buffered.size()); + s.to_backend.buf.insert(s.to_backend.buf.end(), + g.buffered.begin(), + g.buffered.begin() + long(take)); + g.buffered.erase(g.buffered.begin(), + g.buffered.begin() + long(take)); + g.opaque_left -= take; + continue; + } + + // Interleaved RTP/RTCP: '$', channel, 16-bit big-endian length. + if (g.buffered[0] == '$') { + if (g.buffered.size() < 4) { + return true; + } + g.opaque_left = (size_t(g.buffered[2]) << 8) | g.buffered[3]; + s.to_backend.buf.insert(s.to_backend.buf.end(), + g.buffered.begin(), + g.buffered.begin() + 4); + g.buffered.erase(g.buffered.begin(), g.buffered.begin() + 4); + continue; + } + + size_t header_len = 0; + for (size_t i = 0; i + 1 < g.buffered.size(); i++) { + if (g.buffered[i] == '\n' && g.buffered[i + 1] == '\n') { + header_len = i + 2; + break; + } + if (i + 3 < g.buffered.size() + && g.buffered[i] == '\r' && g.buffered[i + 1] == '\n' + && g.buffered[i + 2] == '\r' + && g.buffered[i + 3] == '\n') { + header_len = i + 4; + break; + } + } + if (header_len == 0) { + return g.buffered.size() <= HTTP_MAX_REQUEST; + } + + HttpRequest req; + if (req.feed(g.buffered.data(), header_len) != 1) { + return false; + } + std::string pw; + if (http_query_value(req.target(), "pw", pw)) { + bool have_pw = false; + for (uint8_t b : ke_.video_publish_key) { + have_pw |= b != 0; + } + bool matches = video_password_matches(ke_.video_publish_key, pw); + /* + ffmpeg resolves an SDP control path after the whole source + URI, producing e.g. ?pw=secret/streamid=0. URI syntax makes + that suffix part of the query value. Accept it only when a + slash-delimited prefix is itself the exact password; the + peer still has to know the configured credential. + */ + const size_t slash = pw.rfind('/'); + if (!matches && slash != std::string::npos) { + matches = video_password_matches( + ke_.video_publish_key, pw.substr(0, slash)); + } + if (have_pw && !matches) { + log_reject(s, idx, s.pub_ip_be, VIDEO_ADMIT_BAD_PASSWORD, + now); + return false; + } + } + + const size_t content_length_count = + req.header_count("Content-Length"); + const std::string content_length = req.header("Content-Length"); + if (content_length_count > 1 + || (content_length_count == 1 && content_length.empty())) { + return false; + } + if (!content_length.empty()) { + char *end = nullptr; + errno = 0; + const unsigned long long body = + strtoull(content_length.c_str(), &end, 10); + if (errno != 0 || end == content_length.c_str() || *end != '\0' + || body > 16u * 1024u * 1024u) { + return false; + } + g.opaque_left = size_t(body); + } + s.to_backend.buf.insert(s.to_backend.buf.end(), + g.buffered.begin(), + g.buffered.begin() + long(header_len)); + g.buffered.erase(g.buffered.begin(), + g.buffered.begin() + long(header_len)); + } + return true; +} + // EPOLLOUT only while something is queued. Armed unconditionally it // would make epoll_wait return immediately for ever on an idle splice, // which is the same idle-viewer CPU burn measured earlier. @@ -1156,7 +1288,13 @@ bool VideoChild::pump_rtsp(Slot &s, int idx, int fd, time_t now) return false; } if (n > 0) { - out.buf.insert(out.buf.end(), buf, buf + n); + if (fd == s.rtsp_client_fd) { + if (!guard_rtsp_requests(s, idx, buf, size_t(n), now)) { + return false; + } + } else { + out.buf.insert(out.buf.end(), buf, buf + n); + } if (!splice_flush(to_fd, out)) { return false; } diff --git a/videortmp.cpp b/videortmp.cpp index 6b81e91..c04bce9 100644 --- a/videortmp.cpp +++ b/videortmp.cpp @@ -225,6 +225,8 @@ bool amf_object_strings(const uint8_t *p, size_t n, size_t &i, const char *k1, std::string &v1, const char *k2, std::string &v2) { + bool have_v1 = false; + bool have_v2 = false; if (i >= n) { return false; } @@ -253,12 +255,22 @@ bool amf_object_strings(const uint8_t *p, size_t n, size_t &i, } const std::string key(reinterpret_cast(p + i), klen); i += klen; + const bool is_v1 = key == k1; + const bool is_v2 = key == k2; + if ((is_v1 && have_v1) || (is_v2 && have_v2)) { + // AMF objects are maps. Reject duplicate security-relevant + // properties rather than letting a later value erase a + // credential carried by the first one. + return false; + } + have_v1 |= is_v1; + have_v2 |= is_v2; std::string sv; const size_t save = i; if (i < n && p[i] == AMF_STRING && amf_read_string(p, n, i, sv)) { - if (key == k1) { + if (is_v1) { v1 = sv; - } else if (key == k2) { + } else if (is_v2) { v2 = sv; } continue; @@ -736,7 +748,9 @@ bool RtmpSession::on_command(ChunkStream &c, const uint8_t *p, size_t n) } connected_ = true; std::string tc_url; - amf_object_strings(p, n, i, "app", app_, "tcUrl", tc_url); + if (!amf_object_strings(p, n, i, "app", app_, "tcUrl", tc_url)) { + return fail("malformed or duplicate connect property"); + } password_present_ = split_credential(app_, password_); if (!password_present_ && !tc_url.empty()) { std::string ignored = tc_url; @@ -813,6 +827,13 @@ bool RtmpSession::on_command(ChunkStream &c, const uint8_t *p, size_t n) size_t j = i; amf_skip(p, n, j); // command object, usually null amf_read_string(p, n, j, name); + std::string pw; + if (split_credential(name, pw) && !password_present_) { + // The first credential offered anywhere in the RTMP setup wins. + // A later command without one cannot turn it back into "absent". + password_ = pw; + password_present_ = true; + } /* The response ffmpeg gets wrong: it writes the command name and stops. A camera that waits for the status object here simply @@ -859,7 +880,7 @@ bool RtmpSession::on_command(ChunkStream &c, const uint8_t *p, size_t n) } stream_ = name; std::string pw; - if (split_credential(stream_, pw)) { + if (split_credential(stream_, pw) && !password_present_) { password_ = pw; password_present_ = true; }