Skip to content

Fix: strip RTP padding before DAVE decrypt (RFC 3550 §5.1)#3263

Merged
quinchs merged 1 commit into
discord-net:devfrom
yury-opolev:fix/rtp-padding-stripping
Apr 22, 2026
Merged

Fix: strip RTP padding before DAVE decrypt (RFC 3550 §5.1)#3263
quinchs merged 1 commit into
discord-net:devfrom
yury-opolev:fix/rtp-padding-stripping

Conversation

@yury-opolev

Copy link
Copy Markdown
Contributor

Fixes #3262.

Summary

RTPReadStream.WriteAsync now honors the RTP P (padding) bit per
RFC 3550 §5.1. When the bit is set, the last octet of the packet is
the padding count; those trailing bytes are stripped before the
payload is handed to downstream streams.

Without this fix, any padded packet reaches DaveDecryptStream with
extra bytes appended after the DAVE trailer, and libdave rightly
rejects it — logging Failed to decrypt audio packet for {userId}: DecryptionFailure and silently losing the audio.

Root cause

The count - headerSize slice handed to _next.WriteAsync always
included any trailing padding the sender added. RTP spec:

If the padding bit is set, the packet contains one or more additional
padding octets at the end which are not part of the payload. The last
octet of the padding contains a count of how many padding octets
should be ignored, including itself.

Real Discord clients hit this on a steady background rate (~3%
observed in clean conditions), spiking when silence / DTX boundaries
trigger keepalive / pure-padding packets (saw 50+ such packets in a
single 100 ms burst).

Evidence

Same bot, same private voice channel, same speaker — three back-to-back
30-second runs against the spike diagnostics (extra logging added to
DaveDecryptStream — not in this PR):

Version Decrypt failures Attempts Rate
Stock 3.19.1 63 ~1,650 3.8%
+ RFC padding strip 52 ~2,710 1.9% (all empty-payload packets)
+ drop empty-payload 0 3,320 0.0%

Byte-level shape of pre-fix failures: variable-looking first four bytes
(ciphertext head) followed by long runs of the same byte at the end
(0E0E…0E, 0D0D…0D, FFFF…FF, 2020…20, 1010…10, C9C9…C9,
5555…55, etc.) — textbook RTP padding. Successes end in the DAVE
trailer 0D FA FA. Confirms the padding hypothesis at the byte level.

Changes

One file, ~25 lines. Self-contained.

Test plan

  • Manual: instrumented spike that logs every decrypt result before
    and after the patch. Failure rate went from 3.8% → 0.0% over
    ~3300 attempts on a single user turn. Transcription quality
    improved correspondingly (more audio reaching the app).
  • Build clean on dev branch: dotnet build src/Discord.Net.WebSocket/Discord.Net.WebSocket.csproj -c Release.
  • Would appreciate review from maintainers familiar with the DAVE
    pipeline — particularly whether the empty-payload drop is the
    right place for it, or whether it belongs somewhere earlier (e.g.
    in AudioClient before we even reach RTPReadStream).

Scope guardrails

  • No API changes.
  • No behavior change for packets that don't have the P bit set (the
    vast majority).
  • Pure-padding packets that used to produce a spurious log line are
    now dropped silently — an arguable semantic change, but in every
    practical case those packets carry no audio, and the prior behavior
    was noise.

When the P (padding) bit is set in the first RTP header byte, the last
octet of the packet is the padding count. Any trailing padding bytes
are not part of the payload and must be stripped before the payload is
handed to downstream streams — in particular the DAVE decryptor, which
otherwise fails AEAD verification because the trailing padding isn't
part of the ciphertext.

Observed in the wild: real Discord clients occasionally send padded
voice packets, sometimes in bursts of 50+ within ~100 ms (appears to
correlate with silence / DTX boundaries). Before this fix, those
packets cost ~3–15% decrypt-failure rate, generating
`Failed to decrypt audio packet for {userId}: DecryptionFailure`
noise and silently dropping real audio when the packet payload
followed the padded region.

End-to-end spike evidence (same bot, same channel, same speaker):

  Stock 3.19.1:           63 fails / 1650 attempts  (~3.8%)
  +  RFC padding strip:   52 fails / 2710 attempts  (~1.9%)  (all empty)
  +  drop empty-payload:   0 fails / 3320 attempts  (0.0%)

The zero-payload drop handles pure-padding keepalives / DTX markers
where the entire post-header content is padding; those never carry
audio so we skip the decryptor entirely instead of invoking it with
an empty buffer (which would spuriously log DecryptionFailure).

Fixes a significant fraction of the reports tracked as
"Voice receiving broken" / "Dave audio issues with multiple users".
@Misha-133 Misha-133 requested a review from quinchs April 20, 2026 08:47
@quinchs quinchs merged commit 1a843fb into discord-net:dev Apr 22, 2026
2 checks passed
This was referenced Jun 7, 2026
cgwhouse pushed a commit to cgwhouse/widen-bot that referenced this pull request Jun 9, 2026
Updated [Discord.Net](https://github.com/discord-net/Discord.Net) from
3.19.1 to 3.20.1.

<details>
<summary>Release notes</summary>

_Sourced from [Discord.Net's
releases](https://github.com/discord-net/Discord.Net/releases)._

## 3.20.1

## [3.20.1] - 2026-06-07
This release fixes a regression introduced in 3.20.0

### Fixed
- #​3276 Handle null VoiceChannel in SocketVoiceState constructor
(61ed916)

**Full Changelog**:
discord-net/Discord.Net@3.20.0...3.20.1

## 3.20.0

## [3.20.0] - 2026-06-06
This release brings support for checkboxes and checkbox/radio groups in
modals, and also covers the "new" message search endpoint.

### Breaking changes
- `SelectMenuOptionAttribute` from the Interaction Framework was renamed
to `EnumOptionAttribute`.

### Added
- #​3232 IF modal radio buttons, and checkboxes (c95fbf6)
- #​3268 add support for getting messages from a guild (with filters)
(31fed25)
- #​3255 add missing audit log action types (4476eea)
- #​3265 Add GET voice-state REST wrappers (13d83da)

### Fixed
- #​3258 propagate parent module attributes to child commands (cbc61d9)
- #​3263 strip RTP padding before DAVE decrypt (RFC 3550 В§5.1)
(1a843fb)
- #​3256 Add empty payload check (6527e71)
- #​3264 Fix reference to PreCompiledLambdas/UseCompiledLambda (763aa79)
- #​3271 fix for #​3269 (9abfbfd)
- #​3272 Fix default array converter in modals & add docs for
checkboxes/radio groups (527764c)

### Misc
- #​3254 user `global_name` description (05af64b)
- #​3257 feat(Core): add missing JSON error codes (4272ae1)
- #​3259 refactor(Core): rename JSON error code (504e1db)
- #​3261 Message call data timestamp nullability (5a328a0)
- #​3266 Add play audio sample (4d8b0bc)

## New Contributors
* @​Archivelit made their first contribution in
discord-net/Discord.Net#3256
* @​Sim-hu made their first contribution in
discord-net/Discord.Net#3255
* @​yury-opolev made their first contribution in
discord-net/Discord.Net#3263
* @​apartje made their first contribution in
discord-net/Discord.Net#3271

**Full Changelog**:
discord-net/Discord.Net@3.19.1...3.20.0

Commits viewable in [compare
view](discord-net/Discord.Net@3.19.1...3.20.1).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=Discord.Net&package-manager=nuget&previous-version=3.19.1&new-version=3.20.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: RTP padding bit not honored — ~3–15% false DecryptionFailure on DAVE voice

2 participants