diff --git a/.changeset/brown-rings-smile.md b/.changeset/brown-rings-smile.md new file mode 100644 index 0000000000..ea8e4b1708 --- /dev/null +++ b/.changeset/brown-rings-smile.md @@ -0,0 +1,5 @@ +--- +'livekit-client': patch +--- + +When an SEI or AUD NALU precedes the slice, encrypt h264 frames from the correct offset. Those NALUs read as h265 slice types, so detection returned h265 and the unencrypted prefix ended inside the leading NALU. The slice header was encrypted and no error was raised. The frame bytes now take precedence over a caller-supplied codec. The sender cryptor is also seeded with the publish codec for h264 and h265. A fallback to VP8 handling now logs the first bytes of the frame. diff --git a/src/e2ee/E2eeManager.test.ts b/src/e2ee/E2eeManager.test.ts index b42b19fbc9..f0803c7645 100644 --- a/src/e2ee/E2eeManager.test.ts +++ b/src/e2ee/E2eeManager.test.ts @@ -1,6 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { LogLevel, getWorkerLogLevelListenerCount, setLogLevel, workerLogger } from '../logger'; import Room from '../room/Room'; +import { ParticipantEvent } from '../room/events'; +import { Track } from '../room/track/Track'; +import type { VideoCodec } from '../room/track/options'; import { E2EEManager } from './E2eeManager'; import { BaseKeyProvider } from './KeyProvider'; @@ -194,3 +197,43 @@ describe('E2EEManager GC cleanup', () => { }, ); }); + +describe('E2EEManager sender codec seeding', () => { + function publishVideo(videoCodec: VideoCodec | undefined) { + const { room, worker, manager } = makeManager(); + room.localParticipant.identity = 'me'; + manager.setup(room); + + const sender = { + createEncodedStreams: () => ({ + readable: new ReadableStream(), + writable: new WritableStream(), + }), + } as unknown as RTCRtpSender; + const track = { + isLocal: true, + kind: Track.Kind.Video, + mediaStreamID: 'ms1', + publishOptions: { videoCodec }, + }; + + room.localParticipant.emit(ParticipantEvent.LocalSenderCreated, sender, track as any); + manager.dispose(); + + return worker.postMessage.mock.calls.map(([m]) => m).find((m: any) => m?.kind === 'encode'); + } + + it('seeds the publish codec for h264', () => { + expect(publishVideo('h264')?.data.codec).toBe('h264'); + }); + + it('seeds the publish codec for h265', () => { + expect(publishVideo('h265')?.data.codec).toBe('h265'); + }); + + it('leaves vp8 unseeded so byte detection still decides', () => { + const encode = publishVideo('vp8'); + expect(encode).toBeDefined(); + expect(encode.data.codec).toBeUndefined(); + }); +}); diff --git a/src/e2ee/E2eeManager.ts b/src/e2ee/E2eeManager.ts index f9f6f7b7b6..e946aed122 100644 --- a/src/e2ee/E2eeManager.ts +++ b/src/e2ee/E2eeManager.ts @@ -620,11 +620,15 @@ export class E2EEManager if (!sender) this.log.warn('early return because sender is not ready'); return; } + const isVideo = isVideoTrack(track); + // Safari exposes no per-frame codec info until trackInfo arrives. Seed only h264/h265: + // a wrong vp8 seed would skip the NALU path, a wrong NALU seed only costs a logged fallback. + const publishedCodec = isVideo ? track.publishOptions?.videoCodec : undefined; this.handleSender( sender, track.mediaStreamID, - undefined, - isVideoTrack(track) + publishedCodec === 'h264' || publishedCodec === 'h265' ? publishedCodec : undefined, + isVideo ? (track.publishOptions?.frameMetadata ?? track.publishOptions?.packetTrailer) : undefined, ); diff --git a/src/e2ee/worker/FrameCryptor.test.ts b/src/e2ee/worker/FrameCryptor.test.ts index 72a71a5193..ccefd124ba 100644 --- a/src/e2ee/worker/FrameCryptor.test.ts +++ b/src/e2ee/worker/FrameCryptor.test.ts @@ -1,6 +1,8 @@ import { afterEach, describe, expect, it, vitest } from 'vitest'; import { appendPacketTrailer, extractPacketTrailer } from '../../frameMetadata/frameMetadata'; import type { FrameMetadataPublishOptions } from '../../frameMetadata/types'; +import { workerLogger } from '../../logger'; +import type { VideoCodec } from '../../room/track/options'; import type { NonSharedUint8Array } from '../../type-polyfills/non-shared-typed-arrays'; import { IV_LENGTH, KEY_PROVIDER_DEFAULTS } from '../constants'; import { CryptorEvent } from '../events'; @@ -71,12 +73,14 @@ function prepareParticipantTestEncoder( participantIdentity: string, partialKeyProviderOptions: Partial, packetTrailer?: FrameMetadataPublishOptions, + codec?: VideoCodec, ) { return prepareParticipantTest( 'encode', participantIdentity, partialKeyProviderOptions, packetTrailer, + codec, ); } @@ -85,6 +89,7 @@ function prepareParticipantTest( participantIdentity: string, partialKeyProviderOptions: Partial, packetTrailer?: FrameMetadataPublishOptions, + codec?: VideoCodec, ): { keys: ParticipantKeyHandler; cryptor: FrameCryptor; @@ -110,7 +115,7 @@ function prepareParticipantTest( new ReadableStream(input), new WritableStream(output), 'testTrack', - undefined, + codec, packetTrailer, ); @@ -306,83 +311,151 @@ describe('FrameCryptor', () => { ); expect(Array.from(decryptedData)).toEqual(plainTextData); }); - }); - - it('appends packet trailer after encryption', async () => { - vitest.useFakeTimers(); - const now = new Date('2025-04-10T12:00:00.123Z'); - vitest.setSystemTime(now); - try { - const { keys, input, output } = prepareParticipantTestEncoder( - participantIdentity, - {}, - { timestamp: true, frameId: true }, - ); - - await keys.setKey(await createKeyMaterialFromString('key1'), 1); - - const frame = mockRTCEncodedVideoFrame( - new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]), - ); - - input.write(frame); - await vitest.waitFor(() => expect(output.chunks).toHaveLength(1)); - - const extracted = extractPacketTrailer(frame.data); - expect(extracted.metadata?.userTimestamp).toBeGreaterThanOrEqual( - BigInt(now.getTime()) * BigInt(1000), - ); - expect(extracted.metadata?.frameId).toBe(1); - - const frameTrailer = new Uint8Array( - extracted.data.buffer.slice(extracted.data.byteLength - 2), - ); - expect(frameTrailer[0]).toEqual(IV_LENGTH); - expect(frameTrailer[1]).toEqual(1); - } finally { - vitest.useRealTimers(); - } - }); - - it('does not append packet trailer after encryption when no trailer features are enabled', async () => { - vitest.useFakeTimers(); - try { - const { keys, input, output } = prepareParticipantTestEncoder( - participantIdentity, - {}, - { timestamp: false, frameId: false }, - ); - - await keys.setKey(await createKeyMaterialFromString('key1'), 1); - - const frame = mockRTCEncodedVideoFrame( - new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]), - ); + it('logs the frame bytes once when NALU processing fails', async () => { + vitest.useFakeTimers(); + const warn = vitest.spyOn(workerLogger, 'warn'); + try { + const { keys, input, output } = prepareParticipantTestEncoder( + participantIdentity, + {}, + undefined, + 'h264', + ); + + await keys.setKey(await createKeyMaterialFromString('key1'), 1); + + // No start code at offset 0, so findNALUIndices throws 'byte stream contains + // leading data' and the cryptor falls back to VP8 handling. + input.write( + mockRTCEncodedVideoFrame(new Uint8Array([9, 9, 9, 9, 1, 2, 3, 4, 5, 6, 7, 8])), + ); + await vitest.waitFor(() => expect(output.chunks).toHaveLength(1)); + + expect(warn).toHaveBeenCalledWith( + 'NALU processing failed, falling back to VP8 handling', + expect.objectContaining({ + detectedCodec: 'h264', + frameType: 'key', + byteLength: 12, + firstBytes: expect.stringMatching(/^09 09 09 09 /), + }), + ); + + // same payload type, so the once-per-tuple guard must suppress the second log + input.write( + mockRTCEncodedVideoFrame(new Uint8Array([8, 8, 8, 8, 1, 2, 3, 4, 5, 6, 7, 8])), + ); + await vitest.waitFor(() => expect(output.chunks).toHaveLength(2)); + + expect(warn).toHaveBeenCalledTimes(1); + } finally { + warn.mockRestore(); + vitest.useRealTimers(); + } + }); - input.write(frame); - await vitest.waitFor(() => expect(output.chunks).toHaveLength(1)); + it('appends packet trailer after encryption', async () => { + vitest.useFakeTimers(); + const now = new Date('2025-04-10T12:00:00.123Z'); + vitest.setSystemTime(now); + try { + const { keys, input, output } = prepareParticipantTestEncoder( + participantIdentity, + {}, + { timestamp: true, frameId: true }, + ); + + await keys.setKey(await createKeyMaterialFromString('key1'), 1); + + const frame = mockRTCEncodedVideoFrame( + new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]), + ); + + input.write(frame); + await vitest.waitFor(() => expect(output.chunks).toHaveLength(1)); + + const extracted = extractPacketTrailer(frame.data); + expect(extracted.metadata?.userTimestamp).toBeGreaterThanOrEqual( + BigInt(now.getTime()) * BigInt(1000), + ); + expect(extracted.metadata?.frameId).toBe(1); + + const frameTrailer = new Uint8Array( + extracted.data.buffer.slice(extracted.data.byteLength - 2), + ); + expect(frameTrailer[0]).toEqual(IV_LENGTH); + expect(frameTrailer[1]).toEqual(1); + } finally { + vitest.useRealTimers(); + } + }); - const extracted = extractPacketTrailer(frame.data); - expect(extracted.metadata).toBeUndefined(); - expect(extracted.data.byteLength).toBe(frame.data.byteLength); + it('does not append packet trailer after encryption when no trailer features are enabled', async () => { + vitest.useFakeTimers(); + try { + const { keys, input, output } = prepareParticipantTestEncoder( + participantIdentity, + {}, + { timestamp: false, frameId: false }, + ); + + await keys.setKey(await createKeyMaterialFromString('key1'), 1); + + const frame = mockRTCEncodedVideoFrame( + new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]), + ); + + input.write(frame); + await vitest.waitFor(() => expect(output.chunks).toHaveLength(1)); + + const extracted = extractPacketTrailer(frame.data); + expect(extracted.metadata).toBeUndefined(); + expect(extracted.data.byteLength).toBe(frame.data.byteLength); + + const frameTrailer = new Uint8Array(frame.data.slice(frame.data.byteLength - 2)); + expect(frameTrailer[0]).toEqual(IV_LENGTH); + expect(frameTrailer[1]).toEqual(1); + } finally { + vitest.useRealTimers(); + } + }); - const frameTrailer = new Uint8Array(frame.data.slice(frame.data.byteLength - 2)); - expect(frameTrailer[0]).toEqual(IV_LENGTH); - expect(frameTrailer[1]).toEqual(1); - } finally { - vitest.useRealTimers(); - } - }); + it('appends only timestamp packet trailer metadata after encryption', async () => { + vitest.useFakeTimers(); + const now = new Date('2025-04-10T12:00:00.123Z'); + vitest.setSystemTime(now); + try { + const { keys, input, output } = prepareParticipantTestEncoder( + participantIdentity, + {}, + { timestamp: true }, + ); + + await keys.setKey(await createKeyMaterialFromString('key1'), 1); + + const frame = mockRTCEncodedVideoFrame( + new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]), + ); + + input.write(frame); + await vitest.waitFor(() => expect(output.chunks).toHaveLength(1)); + + const extracted = extractPacketTrailer(frame.data); + expect(frame.data.byteLength - extracted.data.byteLength).toBe(15); + expect(extracted.metadata?.userTimestamp).toBeGreaterThanOrEqual( + BigInt(now.getTime()) * BigInt(1000), + ); + expect(extracted.metadata?.frameId).toBe(0); + } finally { + vitest.useRealTimers(); + } + }); - it('appends only timestamp packet trailer metadata after encryption', async () => { - vitest.useFakeTimers(); - const now = new Date('2025-04-10T12:00:00.123Z'); - vitest.setSystemTime(now); - try { + it('appends only frame id packet trailer metadata after encryption', async () => { const { keys, input, output } = prepareParticipantTestEncoder( participantIdentity, {}, - { timestamp: true }, + { frameId: true }, ); await keys.setKey(await createKeyMaterialFromString('key1'), 1); @@ -395,37 +468,11 @@ describe('FrameCryptor', () => { await vitest.waitFor(() => expect(output.chunks).toHaveLength(1)); const extracted = extractPacketTrailer(frame.data); - expect(frame.data.byteLength - extracted.data.byteLength).toBe(15); - expect(extracted.metadata?.userTimestamp).toBeGreaterThanOrEqual( - BigInt(now.getTime()) * BigInt(1000), - ); - expect(extracted.metadata?.frameId).toBe(0); - } finally { - vitest.useRealTimers(); - } - }); - - it('appends only frame id packet trailer metadata after encryption', async () => { - const { keys, input, output } = prepareParticipantTestEncoder( - participantIdentity, - {}, - { frameId: true }, - ); - - await keys.setKey(await createKeyMaterialFromString('key1'), 1); - - const frame = mockRTCEncodedVideoFrame( - new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]), - ); - - input.write(frame); - await vitest.waitFor(() => expect(output.chunks).toHaveLength(1)); - - const extracted = extractPacketTrailer(frame.data); - expect(frame.data.byteLength - extracted.data.byteLength).toBe(11); - expect(extracted.metadata).toEqual({ - userTimestamp: BigInt(0), - frameId: 1, + expect(frame.data.byteLength - extracted.data.byteLength).toBe(11); + expect(extracted.metadata).toEqual({ + userTimestamp: BigInt(0), + frameId: 1, + }); }); }); }); diff --git a/src/e2ee/worker/FrameCryptor.ts b/src/e2ee/worker/FrameCryptor.ts index de5a5c8642..20d1f9d313 100644 --- a/src/e2ee/worker/FrameCryptor.ts +++ b/src/e2ee/worker/FrameCryptor.ts @@ -915,7 +915,7 @@ export class FrameCryptor extends BaseFrameCryptor { }; } } catch (e) { - this.logNALUFallbackOnce(fallbackKey, payloadType, e); + this.logNALUFallbackOnce(fallbackKey, frame, payloadType, detectedCodec, e); } // Fallback to VP8 handling @@ -928,7 +928,9 @@ export class FrameCryptor extends BaseFrameCryptor { */ private logNALUFallbackOnce( fallbackKey: string, + frame: RTCEncodedVideoFrame, payloadType: number | undefined, + detectedCodec: VideoCodec | undefined, error: unknown, ) { if (this.loggedNALUFallbacks.has(fallbackKey)) { @@ -938,6 +940,12 @@ export class FrameCryptor extends BaseFrameCryptor { workerLogger.warn('NALU processing failed, falling back to VP8 handling', { error, payloadType, + detectedCodec, + frameType: frame.type, + byteLength: frame.data.byteLength, + firstBytes: Array.from(new Uint8Array(frame.data, 0, Math.min(16, frame.data.byteLength))) + .map((b) => b.toString(16).padStart(2, '0')) + .join(' '), ...this.logContext, }); } diff --git a/src/e2ee/worker/naluUtils.test.ts b/src/e2ee/worker/naluUtils.test.ts index 383bf09469..dbed67cb82 100644 --- a/src/e2ee/worker/naluUtils.test.ts +++ b/src/e2ee/worker/naluUtils.test.ts @@ -1,5 +1,21 @@ import { describe, expect, it } from 'vitest'; -import { normalizeAnnexBStartCodes } from './naluUtils'; +import { normalizeAnnexBStartCodes, processNALUsForEncryption } from './naluUtils'; + +/** Annex-B frame: a 4-byte start code before every NALU. */ +function annexB(...nalus: number[][]) { + return Uint8Array.from(nalus.flatMap((nalu) => [0x00, 0x00, 0x00, 0x01, ...nalu])); +} + +// h264: 0x06 SEI, 0x65 IDR slice, 0x67 SPS. h265: 0x26 IDR_W_RADL slice. +// An h264 SEI byte (0x06) reads as h265 TSA_R, which is why the SEI leads here. +// findNALUIndices stops at stream.length - 3. A trailing NALU with fewer than 4 payload +// bytes is never indexed, so its fixture tests nothing. Interior NALUs are fine at any +// length, because the next start code ends them. +const h264KeyFrame = annexB([0x06, 0x11, 0x22], [0x65, 0x33, 0x44, 0x55, 0x66]); +const h265KeyFrame = annexB([0x26, 0x11, 0x22], [0x02, 0x33, 0x44, 0x55, 0x66]); +const noSliceFrame = annexB([0x67, 0x11, 0x22, 0x33, 0x44, 0x55]); +// 0x25 is a slice under both: h264 &0x1f = 5 IDR, h265 (b >> 1) & 0x3f = 18 BLA_W_RADL. +const ambiguousFrame = annexB([0x25, 0x11, 0x22, 0x33, 0x44, 0x55]); describe('normalizeAnnexBStartCodes', () => { it('rewrites 3-byte start codes to 4-byte start codes', () => { @@ -54,3 +70,54 @@ describe('normalizeAnnexBStartCodes', () => { expect(normalizeAnnexBStartCodes(input)).toBe(input); }); }); + +describe('processNALUsForEncryption', () => { + it('slices at the h264 slice NALU even when seeded with h265', () => { + // The whole point: the h265 seed must not win, or the offset lands at 6 (inside the + // SEI) and the slice header gets encrypted with no error raised. + expect(processNALUsForEncryption(h264KeyFrame, 'h265')).toEqual({ + unencryptedBytes: 13, + detectedCodec: 'h264', + requiresNALUProcessing: true, + }); + }); + + it('reads an h264 frame whose SEI precedes the slice as h264', () => { + expect(processNALUsForEncryption(h264KeyFrame)).toEqual({ + unencryptedBytes: 13, + detectedCodec: 'h264', + requiresNALUProcessing: true, + }); + }); + + it('still reads an h265 frame as h265', () => { + expect(processNALUsForEncryption(h265KeyFrame)).toEqual({ + unencryptedBytes: 6, + detectedCodec: 'h265', + requiresNALUProcessing: true, + }); + expect(processNALUsForEncryption(h265KeyFrame, 'h264')).toEqual({ + unencryptedBytes: 6, + detectedCodec: 'h265', + requiresNALUProcessing: true, + }); + }); + + it('resolves a byte that is a slice under both codecs as h264', () => { + // Only the h264-first ordering decides this one. Swap the two loops back and it reads h265. + expect(processNALUsForEncryption(ambiguousFrame)).toEqual({ + unencryptedBytes: 6, + detectedCodec: 'h264', + requiresNALUProcessing: true, + }); + }); + + it('throws with a seed, and returns quietly without one, when no slice NALU is present', () => { + expect(() => processNALUsForEncryption(noSliceFrame, 'h264')).toThrow('Could not find NALU'); + expect(processNALUsForEncryption(noSliceFrame)).toEqual({ + unencryptedBytes: 0, + detectedCodec: 'unknown', + requiresNALUProcessing: false, + }); + }); +}); diff --git a/src/e2ee/worker/naluUtils.ts b/src/e2ee/worker/naluUtils.ts index 922e1bf645..4466d8a72c 100644 --- a/src/e2ee/worker/naluUtils.ts +++ b/src/e2ee/worker/naluUtils.ts @@ -202,8 +202,14 @@ export interface NALUProcessingResult { * @returns Detected codec type */ function detectCodecFromNALUs(data: NonSharedUint8Array, naluIndices: number[]): DetectedCodec { + // h264 runs to completion first. Several non-slice h264 NALUs read as h265 slice types + // (AUD 0x09 -> STSA_N, SEI 0x06 -> TSA_R), so interleaving the two checks lets one of + // those outrank the real h264 slice that follows it. The reverse cannot happen: h265 + // header bytes are even at nuh_layer_id 0, so they never mask to an h264 slice type. for (const naluIndex of naluIndices) { if (isH264SliceNALU(parseH264NALUType(data[naluIndex]))) return 'h264'; + } + for (const naluIndex of naluIndices) { if (isH265SliceNALU(parseH265NALUType(data[naluIndex]))) return 'h265'; } return 'unknown'; @@ -306,7 +312,7 @@ function findNALUIndices(stream: NonSharedUint8Array): number[] { /** * Process NALU data for frame encryption, detecting codec and finding unencrypted bytes * @param data Frame data - * @param knownCodec Known codec from other sources (optional) + * @param knownCodec Codec from other sources, used only when the bytes are inconclusive (optional) * @returns NALU processing result */ export function processNALUsForEncryption( @@ -314,7 +320,16 @@ export function processNALUsForEncryption( knownCodec?: 'h264' | 'h265', ): NALUProcessingResult { const naluIndices = findNALUIndices(data); - const detectedCodec = knownCodec ?? detectCodecFromNALUs(data, naluIndices); + // The bytes outrank knownCodec, which can be the codec the client asked for rather than the + // one that was negotiated. Parsing the wrong one mis-slices without raising anything: the + // offset lands before the real slice header, so the header itself gets encrypted. + // That holds in one direction only. An h264 access unit carrying no slice NALU but carrying + // an AUD or SEI that reads as an h265 slice now slices there instead of throwing. No real + // WebRTC access unit can reach it, because every one of them carries a type 1 or 5 slice. + let detectedCodec = detectCodecFromNALUs(data, naluIndices); + if (detectedCodec === 'unknown') { + detectedCodec = knownCodec ?? 'unknown'; + } if (detectedCodec === 'unknown') { return { unencryptedBytes: 0, detectedCodec, requiresNALUProcessing: false };