Skip to content

Commit eef6ef0

Browse files
Copilotsipsorcery
andauthored
Add WebRTC Nostr Signalling prototype example (#1606)
* Add WebRTCNostrSignalling prototype with Nostr-based WebRTC signalling Agent-Logs-Url: https://github.com/sipsorcery-org/sipsorcery/sessions/5a7ca99d-f3c3-4fb4-a3c3-829056df172c Co-authored-by: sipsorcery <197660+sipsorcery@users.noreply.github.com> * Address code review feedback - improve async error handling and documentation Agent-Logs-Url: https://github.com/sipsorcery-org/sipsorcery/sessions/5a7ca99d-f3c3-4fb4-a3c3-829056df172c Co-authored-by: sipsorcery <197660+sipsorcery@users.noreply.github.com> * Fix Nostr relay connection - call ListenForMessages and improve diagnostics Agent-Logs-Url: https://github.com/sipsorcery-org/sipsorcery/sessions/cd6b7917-80e5-4d4b-ac30-c990926222ec Co-authored-by: sipsorcery <197660+sipsorcery@users.noreply.github.com> * Fix Nostr connection - remove duplicate ListenForMessages call and improve diagnostics Agent-Logs-Url: https://github.com/sipsorcery-org/sipsorcery/sessions/7d6ebf91-78fe-40e5-ba1b-4c2c582ff560 Co-authored-by: sipsorcery <197660+sipsorcery@users.noreply.github.com> * Change Nostr relay from broken wss://nostr.net to working wss://nos.lol Agent-Logs-Url: https://github.com/sipsorcery-org/sipsorcery/sessions/3e314f7f-d83f-4f0b-82b4-2b15f26defce Co-authored-by: sipsorcery <197660+sipsorcery@users.noreply.github.com> * Handle encrypted/non-JSON Nostr events gracefully Agent-Logs-Url: https://github.com/sipsorcery-org/sipsorcery/sessions/01b9b14e-55c3-45c4-9856-d9ba116186cd Co-authored-by: sipsorcery <197660+sipsorcery@users.noreply.github.com> * WebRTCNostrSignalling: fix Nostr signalling so the offer reaches the answerer The PR's offerer was logging "invalid: bad event id" from the relay on every publish, plus a misleading "sent and acknowledged" right after, plus drowning in unrelated NIP-46 traffic on the kind it had picked. This commit fixes the architectural issues underneath all of that. Five changes: 1. Persistent Nostr keypair per process (was: a fresh random keypair per published event). The previous code generated a new ECPrivKey inside SendSignalMessage, so every offer / answer / ICE candidate from the same peer arrived from a different pubkey. 2. Peer ID is now the FULL hex Nostr public key (64 chars), with an 8-char short prefix shown for at-a-glance use in logs. The previous random Guid-derived 8-char string had no relationship to any Nostr identity, so subscriptions couldn't be filtered server- side and routing was buried inside the JSON content. 3. Each published event now carries a ["p", remotePubkey] tag addressing the recipient -- the Nostr-native routing primitive. 4. Subscription is now {kinds:[KIND], "#p":[localPubkey]} so the relay only forwards events whose author has tagged our pubkey. Removes the firehose of unrelated traffic from other apps using the same kind. Side effect: outgoing events have a non-empty tags array, which avoids an id-preimage / publish-JSON canonical- isation mismatch in some NNostr.Client versions that surfaces as the relay's "invalid: bad event id" rejection. 5. Signalling kind changed from 24133 -> 25555. 24133 is NIP-46 NostrConnect which is busy on public relays AND is encrypted by spec, so subscribing to it pulls a stream of base64 blobs. Plus: SendSignalMessage now subscribes to nostrClient.OkReceived BEFORE publishing the event, then waits for the OK frame and surfaces the relay's reject message as a real exception. Previously SendEventsAndWaitUntilReceived returned on any OK frame regardless of the success bit, so the offerer logged "Nostr event sent and acknowledged" on the same event the relay had just rejected with "invalid: bad event id". API-level cross-check done against the NNostr.Client master source: ECPrivKey.CreateXOnlyPubKey().ToHex() for the pubkey, NostrEventTag .TagIdentifier / .Data for the "p" tag, NostrSubscriptionFilter .ReferencedPublicKeys for the "#p" filter, INostrClient.OkReceived for the relay-NACK detection. * WebRTCNostrSignalling: hex-encode content to dodge NNostr.Client id-canonicalisation bug Even after PR fix #1 (persistent keypair, "p" tag, "#p" filter, kind=25555, OkReceived check), the offerer was still getting "OK ... false ... invalid: bad event id" from the relay on every publish. Root cause is a bug inside NNostr.Client itself: * The id-preimage path in NostrExtensions.ToIdPreimage uses StringEscaperJsonConverter.JavaScriptStringEncode for the content field. That encoder does NOT escape <, >, ', &, +, or non-ASCII -- it only escapes the JSON-mandatory chars (\b, \t, \n, \f, \r, \", \\, plus 0x00..0x1F as \uXXXX). * The publish path uses StringEscaperJsonConverter.Write, which just delegates to writer.WriteStringValue(value) -- which goes through System.Text.Json's default JavaScriptEncoder.Default. That encoder DOES escape <, >, ', &, +, `, and all non-ASCII as \uXXXX. For any content that contains one of those characters -- and a real WebRTC SDP is full of them ("+" in ice-pwd, "+" in fingerprint, etc.) -- the id NNostr hashes locally differs from the id the relay re-computes from the JSON we publish. The relay returns "invalid: bad event id" and the event is discarded. Bug exists on master too, not just the v0.0.37 we're consuming, so upgrading the package wouldn't fix it. Workaround at our layer: hex-encode the inner JSON before putting it on the wire so the content is purely [0-9a-f]. Both encoders pass hex through unchanged, and ids match. Receiver hex-decodes before deserialising; non-hex content from foreign traffic on the same kind is skipped silently (replaces the previous "starts with '{'" sniffing). The right long-term fix is upstream -- either change the lib's JsonConverter Write to use JavaScriptStringEncode so the two paths agree, or change ToIdPreimage to use System.Text.Json with the default encoder. Either direction works; both have to agree. * WebRTCNostrSignalling: buffer remote ICE candidates + raise diagnostic log levels Symptoms after the canonicalisation fix landed: the offerer publishes the offer successfully (relay returns OK true), the answerer receives it and sends back the Answer + trickled ICE candidates over Nostr, but the offerer's ICE checks against the answerer never start. Probable cause: Nostr does not preserve ordering across separate events, so a trickled IceCandidate frequently arrives at the offerer BEFORE the Answer SDP. The previous code called peerConnection.addIceCandidate immediately on receipt regardless of whether setRemoteDescription had been applied, so any pre-Answer candidate was effectively discarded. Fix: * Buffer remote ICE candidates in pendingRemoteCandidates while remoteDescriptionApplied == false. * On the offerer side, after setRemoteDescription(answer) succeeds, set remoteDescriptionApplied = true and drain the buffer. * On the answerer side, do the same after setRemoteDescription(offer) succeeds (covers the symmetric case where the offerer's trickled candidate races ahead of the offer). Plus: promote the OnNostrEventsReceived entry log + the per-message processing logs from Debug to Information so the next test run shows decisively whether the Answer is being processed and whether the remote description is being applied. The existing per-state-change logs on the peer connection were already at Information. Tested-by: code review only -- the sandbox can't fit the full SIPSorcery + WebRTCNostrSignalling build in its working set. * WebRTCNostrSignalling: bypass NNostr.Client's Verify() to receive peer events Symptom: even after the publish-side hex-encode workaround landed and the relay started accepting our offers (OK true), the offerer's OnNostrEventsReceived handler never fired for the answerer's reply. Raw EVENT messages on the "webrtc-signal" subscription arrived (logged via MessageReceived), but the typed EventsReceived event for that subscription was silent. Root cause: NostrClient.HandleIncomingMessage: case "event": var evt = json[2].Deserialize<NostrEvent>(); if (evt?.Verify() is true) { EventsReceived?.Invoke(this, (subscriptionId, new[] {evt})); } break; The if-Verify gate silently drops the event when Verify() returns false. NostrExtensions.Verify rebuilds the id-preimage using NNostr's own JavaScriptStringEncode (which does NOT escape <, >, ', &, +, or non-ASCII), SHA-256s it, and requires the result to equal evt.Id. But evt.Id was set by the relay from the JSON the answerer published through System.Text.Json's WriteStringValue (which DOES escape those chars). Same canonicalisation bug as the publish side, just running in the opposite direction. Our hex-encode publish-side workaround makes our outgoing events round-trip through Verify (because hex contains no chars the two encoders disagree on). But a PEER's event still has whatever id the relay computed from the published JSON, and our copy of NNostr.Client re-runs Verify on receive and silently drops it. Workaround: hook into MessageReceived (which fires for every raw relay message), parse EVENT messages for our subscription ourselves, and call OnNostrEventsReceived directly -- bypassing the typed EventsReceived event and its Verify gate. Acceptable for a prototype because: * The relay-side "#p" filter already restricts what gets sent to us to events tagged for our pubkey. * The application-level TargetPeerId check inside OnNostrEventsReceived rejects anything that slipped through. * We're using a fresh per-process keypair, so impersonation is the only attack and at this prototype stage there's no recipient to impersonate to. Real fix is upstream in NNostr.Client -- their JsonConverter.Write needs to use JavaScriptStringEncode so the publish path matches the preimage path (or vice versa). * WebRTCNostrSignalling: fix CS0026 in MessageReceived bypass Compile error on line 267 from the previous commit -- the bypass handler is a lambda inside a static method, so `this` is invalid. Pass the lambda's `sender` parameter through instead. Lambda was registered with `(sender, message) =>`, so `sender` is the NostrClient instance the event fired on. * WebRTCNostrSignalling: rewrite nostr-webrtc.html to interop with the C# console app The old browser page would not have interoperated with the C# code for several independent reasons; rewriting it as a single document fixes all of them in one step: * Relay URL: wss://nostr.net (broken) -> wss://nos.lol (matches C#). * Kind: 24133 (NIP-46 NostrConnect, busy + encrypted by spec) -> 25555 (matches C#). * Identity: previously a SHA-256 hash of a random 32 bytes was used as the "pubkey". That isn't a real secp256k1 pubkey, so the relay accepted nothing the page sent (the page also tried to publish with sig = "0".repeat(128)). Now uses nostr-tools' generateSecretKey + getPublicKey + finalizeEvent for real BIP-340 Schnorr signing. * Peer ID: previously a Math.random() short string with no relationship to the Nostr pubkey. Now derived from the full secp256k1 pubkey, so the peer's identity AND its routing address are the same value (matches C# behaviour). * Routing: previously no "p" tag and a {kinds:[X]} subscription that pulled every event of that kind from the entire relay. Now adds ["p", remotePubkey] to outgoing events and subscribes with {kinds:[X], "#p":[localPubkey]} so the relay only forwards events tagged for us. * Content encoding: hex-encodes the inner JSON before publishing and hex-decodes on receive. Matches the C# workaround that dodges the NNostr.Client canonicalisation bug. (JS's JSON.stringify wouldn't have the bug on its own, but it has to interop bit-exact with what C# now sends.) * JSON shape: PascalCase property names + integer Type enum values (Offer=0, Answer=1, IceCandidate=2) to match what System.Text.Json on the C# side produces by default. * Receive-side parity: ICE candidates that arrive before the remote description is set are buffered and drained on success, just like the C# side does, so out-of-order trickled candidates don't get dropped. * OK frame check: "false" responses are surfaced explicitly in the log with the relay's reject text rather than being silently swallowed. The page is a single self-contained HTML file with one esm.sh import of nostr-tools 2.7.2 -- works without a build step, just open in a browser. * WebRTCNostrSignalling: switch C# tracks to SendOnly to fix browser-as-offerer flow When the browser is the offerer it adds its transceivers with direction = "recvonly" (the browser is just rendering remote media, it doesn't have a camera/mic to source). The C# answerer was adding its tracks with MediaStreamStatusEnum.SendRecv, so the answer SDP came back with direction = "sendrecv". The browser rejected it with: InvalidAccessError: Failed to execute 'setRemoteDescription' on 'RTCPeerConnection': Failed to set remote answer sdp: Incompatible send direction Per RFC 3264 / RFC 8829 a recvonly offer must be answered with sendonly or inactive -- sendrecv widens the contract and is invalid. The C# side only ever sources media in this example (test pattern video + music audio); it has no rendering sink. SendOnly is the accurate description and produces a compatible answer SDP for a recvonly offer AND a self-consistent offer SDP when the C# side initiates. * Add nostr webrtc demo to sln. * Cosmetic fixes. * WebRTCNostrSignalling: detach media event handlers before close to silence "SendRtpRaw on closed session" spam When the remote peer closes the connection (now that #1607 makes DTLS close_notify actually tear down the RTCPeerConnection), there's a brief race window between the connection transitioning to closed and the media sources stopping. AudioExtrasSource produces a packet every ~20 ms and typically has a handful in flight; each one fires OnAudioSourceEncodedSample -> pc.SendAudio -> RTPSession.SendRtpRaw, which logs: [WRN] SendRtpRaw was called for a audio packet on a closed RTP session. Detach the OnVideoSourceEncodedSample / OnAudioSourceEncodedSample handlers BEFORE awaiting CloseVideo / CloseAudio so any in-flight samples stop racing past the close boundary. Video happens to be quieter (33 ms frame interval at 30 fps) so it almost never trips the warning, but unsubscribe both for symmetry. * RTP ICE: replace OS DNS fallback with real mDNS via Makaretu.Dns.Multicast (#1608) * RTP ICE: replace OS DNS fallback with real mDNS via Makaretu.Dns.Multicast Resolves the longstanding issue where Chrome-published .local ICE candidates almost never resolve on Windows, leaving WebRTC browser-vs-SIPSorcery sessions stuck in ICE checking with: [ERR] Error resolving mDNS hostname <guid>.local System.Net.Sockets.SocketException (11001): No such host is known. Chrome publishes ICE candidates as "<guid>.local" mDNS names by default (privacy mitigation -- chrome://flags/#enable-webrtc-hide-local-ips-with-mdns). RtpIceChannel.ResolveMdnsName previously fell back to Dns.GetHostAddressesAsync, which delegates to the OS resolver. On Windows the OS resolver does NOT reliably do mDNS -- whether it works at all depends on whether Windows' built-in mDNS resolver (Win10 1803+) or Bonjour is enabled, whether the multicast packet round-trips through the firewall in time, and which adapters are bound. End result: it "almost always fails", just as the original 2020 author comment ("Hopefully support is coming to .Net Core soon") anticipated. Fix: add Makaretu.Dns.Multicast 0.27.0 (a small RFC 6762 mDNS implementation over UDP/5353 multicast) and a new internal MdnsResolver helper that: * Lazily starts a single shared MulticastService per process so the multicast group binding doesn't churn on every query. * Sends an A and an AAAA query for the hostname. * Collects answers via the AnswerReceived event (filtering by name and accepting both A and AAAA records). * Returns whatever arrives within a 2-second timeout (configurable via MdnsResolver.DefaultTimeout); empty array on timeout. * Logs and falls through gracefully if MulticastService.Start() fails (e.g. blocked by firewall) -- the existing OnIceCandidateError event still fires for the candidate. The user-supplied MdnsGetAddresses / MdnsResolve hooks on RtpIceChannel still take precedence; this only changes the no-hook fallback. Targets: netstandard2.0+ which Makaretu.Dns.Multicast supports natively. Tested-by: code review against https://github.com/richardschneider/net-mdns/blob/master/src/MulticastService.cs The sandbox can't fit a full SIPSorcery build alongside the SDK. * MdnsResolver: use LogFactory.CreateLogger like the rest of SIPSorcery Compile error: Log.Logger doesn't exist in this codebase. Other ICE files (RtpIceChannel, IceServer) use LogFactory.CreateLogger<T>() -- match that pattern. * MdnsResolver: drop 'static' from class declaration (CS0718) LogFactory.CreateLogger<T>() requires T to be a non-static type. Switch to 'internal sealed class' with a private constructor; all members stay static. Matches the pattern in NetServices, Crypto and StorageTypesConverter elsewhere in SIPSorcery.Sys. * Bumped nnostr package version. * wip nostr signalling example. * Hard code c# demo app and corresponding browser page private nostr keys for demo purposes. * Added nip44 encryption to nostr signalling message exchanges. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sipsorcery <197660+sipsorcery@users.noreply.github.com> Co-authored-by: Claude (Opus 4.7) <aaron.clauson@gmail.com> Co-authored-by: Aaron Clauson <aaron@sipsorcery.com>
1 parent 3a544cf commit eef6ef0

6 files changed

Lines changed: 1883 additions & 387 deletions

File tree

SIPSorcery.slnx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@
107107
<Project Path="examples/WebRTCExamples/WebRTCGetStartedLibvpx/WebRTCGetStartedLibvpx.csproj" />
108108
<Project Path="examples/WebRTCExamples/WebRTCMp4Source/WebRTCMp4Source.csproj" />
109109
<Project Path="examples/WebRTCExamples/WebRTCNoSignalling/WebRTCNoSignalling.csproj" />
110+
<Project Path="examples/WebRTCExamples/WebRTCNostrSignalling/WebRTCNostrSignalling.csproj" />
110111
<Project Path="examples/WebRTCExamples/WebRTCOpenGL/WebRTCOpenGL.csproj" />
111112
<Project Path="examples/WebRTCExamples/WebRTCOpenGLSource/WebRTCOpenGLSource.csproj" />
112113
<Project Path="examples/WebRTCExamples/WebRTCReceiveAudio/WebRTCReceiveAudio.csproj" />
@@ -138,6 +139,7 @@
138139
<Project Path="src/SIPSorceryMedia.FFmpeg/SIPSorceryMedia.FFmpeg.csproj" />
139140
<Project Path="src/SIPSorceryMedia.Windows/SIPSorceryMedia.Windows.csproj" />
140141
</Folder>
142+
<Project Path="../NNostr/NNostr.Client/NNostr.Client.csproj" />
141143
<Properties Name="TestCaseManagementSettings" Scope="PostLoad">
142144
<Property Name="CategoryFile" Value="SIPSorcery-Core.vsmdi" />
143145
</Properties>

0 commit comments

Comments
 (0)