Skip to content

Forward RFC 3326 Reason header on CANCEL/BYE between B2B legs#369

Merged
hecko merged 1 commit intomasterfrom
claude/review-pr-165-typescript-fzCvY
Apr 17, 2026
Merged

Forward RFC 3326 Reason header on CANCEL/BYE between B2B legs#369
hecko merged 1 commit intomasterfrom
claude/review-pr-165-typescript-fzCvY

Conversation

@hecko
Copy link
Copy Markdown
Contributor

@hecko hecko commented Apr 17, 2026

Summary

This change implements selective forwarding of RFC 3326 "Reason:" headers from inbound CANCEL requests to outbound CANCEL/BYE messages on peer B2B legs in the SBC. This allows call termination reasons to propagate across the SBC while maintaining security boundaries by filtering out trust-sensitive headers.

Key Changes

  • Added filterReasonHdr() helper function: Extracts only RFC 3326 "Reason:" headers from raw SIP header blocks, intentionally excluding other CANCEL headers (Via, CSeq, Route, Authorization, Privacy, P-A-I) that are either meaningless on different legs or would leak trust-boundary metadata.

  • Introduced CallLegTerminateEvent struct: A custom B2B event that extends B2BEvent to carry filtered CANCEL headers between CallLegs. Reuses the existing B2BTerminateLeg event_id so non-CallLeg receivers still terminate correctly without the extra header.

  • Added cancel_hdrs member variable: Stores filtered extension headers (only RFC 3326 "Reason:") captured from inbound CANCEL requests. Non-empty only for the duration of a single termination call chain.

  • Modified terminateOtherLeg(): Now relays CallLegTerminateEvent with filtered headers to peer legs instead of plain termination events when cancel headers are present.

  • Modified terminateNotConnectedLegs(): Propagates filtered CANCEL headers to all non-connected legs via CallLegTerminateEvent.

  • Enhanced onB2BEvent() handler: Added case for B2BTerminateLeg to detect CallLegTerminateEvent and apply the propagated headers when terminating the local leg.

  • Updated onCancel() handler: Captures and filters the "Reason:" header from inbound CANCEL requests into cancel_hdrs before processing call termination.

  • Modified terminateLeg(): When cancel_hdrs is populated, sends BYE with the filtered headers using SIP_FLAGS_VERBATIM flag instead of the default termination path.

Implementation Details

  • The filtering is conservative: only RFC 3326 "Reason:" headers are forwarded; all other headers are dropped to prevent leaking sensitive information across trust boundaries.
  • The cancel_hdrs variable is temporary and scoped to the termination call chain, being saved and restored to prevent unintended side effects.
  • The implementation maintains backward compatibility by falling back to standard termination when no filtered headers are present.

https://claude.ai/code/session_01PWHzum8cu3oJhct3A4YEdd

Re-implementation of PR #165 with the concerns raised in review
addressed:

- Filter to RFC 3326 "Reason:" only. Other CANCEL headers (Via, CSeq,
  Route, Authorization, Privacy, P-Asserted-Identity, etc.) are no
  longer forwarded verbatim onto the peer leg's BYE — they are either
  nonsensical there or leak trust-boundary metadata.
- Keep the implementation inside apps/sbc so it doesn't touch
  AmB2BSession / AmB2ABSession virtual signatures. External modules
  that subclass those base classes continue to override correctly
  (the original patch silently broke their overrides because
  `terminateLeg()` / `terminateOtherLeg()` gained a defaulted
  parameter and the old zero-arg overrides stopped overriding).
- Propagate the filtered header between CallLegs via an SBC-local
  B2BEvent subclass that reuses the existing B2BTerminateLeg event
  id, so non-CallLeg receivers still terminate correctly.
- Not configurable: enabled unconditionally for SBC CallLeg, matching
  the PR's original behaviour.

https://claude.ai/code/session_01PWHzum8cu3oJhct3A4YEdd
Copilot AI review requested due to automatic review settings April 17, 2026 06:39
@hecko hecko merged commit fecacc9 into master Apr 17, 2026
26 of 27 checks passed
@hecko hecko deleted the claude/review-pr-165-typescript-fzCvY branch April 17, 2026 06:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements selective propagation of RFC 3326 Reason: from inbound CANCEL to outbound CANCEL/BYE across SBC B2B legs, while avoiding forwarding trust-boundary-sensitive headers.

Changes:

  • Added a cancel_hdrs per-leg temporary buffer to carry filtered Reason: headers through a termination chain.
  • Introduced filterReasonHdr() and a CallLegTerminateEvent to transport the filtered headers between legs using the existing B2BTerminateLeg event id.
  • Updated termination and CANCEL handling to attach the filtered Reason: header(s) to generated CANCEL/BYE.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
apps/sbc/CallLeg.h Adds cancel_hdrs member to store filtered Reason: headers during termination propagation.
apps/sbc/CallLeg.cpp Implements Reason: extraction, propagates it via a custom terminate event, and uses it when generating CANCEL/BYE.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/sbc/CallLeg.cpp
AmSessionContainer::instance()->postEvent(i->id, new B2BEvent(B2BTerminateLeg));
B2BEvent *ev = cancel_hdrs.empty()
? new B2BEvent(B2BTerminateLeg)
: static_cast<B2BEvent*>(new CallLegTerminateEvent(cancel_hdrs));
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The static_cast<B2BEvent*> around new CallLegTerminateEvent(...) is redundant (implicit upcast already applies) and makes the ternary harder to read. Consider dropping the cast and keeping both branches symmetric for clarity.

Suggested change
: static_cast<B2BEvent*>(new CallLegTerminateEvent(cancel_hdrs));
: new CallLegTerminateEvent(cancel_hdrs);

Copilot uses AI. Check for mistakes.
Comment thread apps/sbc/CallLeg.cpp
Comment on lines +55 to +60
size_t colon = hdrs.find(':', pos);
if (colon != string::npos && colon < content_end
&& (colon - pos) == 6
&& strncasecmp(hdrs.c_str() + pos, "Reason", 6) == 0) {
out.append(hdrs, pos, content_end - pos);
out.append("\r\n");
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filterReasonHdr() only matches Reason when the ':' immediately follows the 6-character name (i.e., it rejects Reason : / Reason\t:). Elsewhere in the codebase header matching explicitly tolerates optional whitespace before ':' (see findHeader() in core/AmSipMsg.cpp), so this stricter parsing may cause valid-in-the-wild Reason headers to be dropped and not propagated. Consider loosening the match to allow OWS between the header name and ':' (or reuse the existing header-finding logic).

Suggested change
size_t colon = hdrs.find(':', pos);
if (colon != string::npos && colon < content_end
&& (colon - pos) == 6
&& strncasecmp(hdrs.c_str() + pos, "Reason", 6) == 0) {
out.append(hdrs, pos, content_end - pos);
out.append("\r\n");
if ((content_end - pos) >= 6
&& strncasecmp(hdrs.c_str() + pos, "Reason", 6) == 0) {
size_t colon = pos + 6;
while (colon < content_end &&
(hdrs[colon] == ' ' || hdrs[colon] == '\t')) {
++colon;
}
if (colon < content_end && hdrs[colon] == ':') {
out.append(hdrs, pos, content_end - pos);
out.append("\r\n");
}

Copilot uses AI. Check for mistakes.
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.

3 participants