core: fix User-Agent injection on SBC-relayed requests; add hide_user_agent option#540
Open
hecko wants to merge 7 commits into
Open
core: fix User-Agent injection on SBC-relayed requests; add hide_user_agent option#540hecko wants to merge 7 commits into
hecko wants to merge 7 commits into
Conversation
…_agent option SIP_FLAGS_VERBATIM suppressed AmConfig::Signature injection on all SBC outgoing requests (INVITE, re-INVITE, REGISTER relay, auth-retry REGISTER), making the configured signature invisible regardless of sems.conf settings. Replace the VERBATIM-gated injection with a presence check: inject the signature whenever the User-Agent (requests) or Server (replies) header is absent, regardless of the VERBATIM flag. This preserves verbatim B2BUA relay for upstream UAs that supply their own header while ensuring SEMS-originated requests (e.g. UACAuth retry after 401/407) carry the configured identity. Add hide_user_agent = yes to sems.conf to actively strip User-Agent from requests and Server from replies, eliminating software fingerprinting as an attack surface. RFC 3261 §20.41 and §20.35 both note that omitting these headers reduces vulnerability to targeted attacks. Fixes: #539 https://claude.ai/code/session_016Pg7MeJjEfiNfJNSSLFiPb
…n; unit tests
Problem
-------
The previous commit fixed User-Agent injection for SBC-relayed messages but
kept the old default behaviour (identity revealed when `signature` is set).
Disclosing the exact server version in User-Agent / Server headers is an
explicit security risk per RFC 3261 §20.41 and §20.35.
Changes
-------
* Default is now HIDDEN (SendUserAgent=false). User-Agent is stripped from
all outgoing requests and Server from all replies unless opted in.
* New config knob `send_user_agent=yes` opts in to forwarding the identity
header. Works in concert with the existing `use_default_signature` /
`signature` options, which now have no visible effect unless
`send_user_agent=yes` is also set.
* Startup WARNING is logged whenever `send_user_agent=yes` and a signature
string is configured, reminding operators of the fingerprinting exposure.
* Refactor: the injection/suppression logic is extracted into the static
helper `AmBasicSipDialog::applyIdentityHeader(hdrs, hdr_name)`. All four
former inline blocks (sendRequest in AmBasicSipDialog and AmSipDialog, and
the two reply paths) are replaced by a single call to this helper, keeping
the policy in one auditable location.
* B2BUA transparency preserved: when `send_user_agent=yes` and the upstream
UAC already supplied a User-Agent, SEMS does not overwrite it.
Documentation
-------------
* sems.conf.sample: rewrote the identity-header section with full description
of `send_user_agent`, its interaction with `signature` /
`use_default_signature`, and the security rationale. The previously active
`use_default_signature=yes` line is now commented out.
Unit tests
----------
* core/tests/test_ua_header.{h,cpp}: 12 FCT tests covering:
- Default hidden mode strips forwarded UA even when signature configured
- send_user_agent=yes injects signature only when header is absent
- Existing upstream UA is preserved (B2BUA transparency)
- Symmetric behaviour for Server header on replies
- Regression test for issue #539 auth-retry REGISTER scenario
https://claude.ai/code/session_016Pg7MeJjEfiNfJNSSLFiPb
…lear) AmMimeBody has no clear() method. When the SDP part is present but empty, drop it via body.deletePart(SIP_APPLICATION_SDP), which is the correct API for removing a sub-part by content-type. This pre-existing compile error (introduced in commit 7c5374c on master) was blocking every CI build across all platforms. https://claude.ai/code/session_016Pg7MeJjEfiNfJNSSLFiPb
The second parameter of MD5Update was declared as 'unsigned char *' but callers (UACAuth, AmUtils, AmConfigReader) pass 'const unsigned char *' data. On macOS/clang this is a hard error. Add 'const' to the input parameter of MD5Update, MD5Transform, and Decode throughout md5.h/md5.cpp — none of these functions modify the input buffer, so the qualifier is correct and the change is safe. https://claude.ai/code/session_016Pg7MeJjEfiNfJNSSLFiPb
Removes the push trigger from all three workflows so CI only runs when a PR is opened or updated, not on every branch commit. https://claude.ai/code/session_016Pg7MeJjEfiNfJNSSLFiPb
The UA header injection logic previously lived in AmBasicSipDialog as a static method (applyIdentityHeader) keyed on global AmConfig::SendUserAgent. This is architecturally wrong: endpoint apps (conference, voicemail) have different identity needs than a B2BUA relay, and global config cannot express per-profile policy. Core changes (minimal): - Revert AmConfig: remove SendUserAgent; restore original Signature-only config and sems.conf.sample (use_default_signature=yes active by default) - Replace the three inline injection sites in AmBasicSipDialog::sendRequest(), reply(), and AmSipDialog::send_200_ack() with calls to a new virtual hook AmBasicSipEventHandler::onApplyIdentityHeader(hdrs, hdr_name, flags) - Default implementation of the hook reproduces the original SEMS behaviour: inject AmConfig::Signature when absent and SIP_FLAGS_VERBATIM is not set; all non-SBC sessions are unaffected - reply_error() remains inline (static method, no handler available) - Remove test_ua_header.cpp/h from core (tested the removed static method) SBC changes: - SBCCallProfile gains send_user_agent bool (default: false); parsed from the call profile config file; a WARN is emitted when enabled with a configured Signature (RFC 3261 §20.41/§20.35 fingerprinting risk) - SBCCallLeg overrides onApplyIdentityHeader: when send_user_agent=false (default), strip any forwarded User-Agent/Server so software identity is not disclosed on the relay path; when true, inject Signature only if the header is absent (preserves upstream phone's UA for B2BUA transparency) - transparent.sbcprofile.conf documents the new send_user_agent option This fixes issue #539 (auth-retry REGISTER losing User-Agent) on the SBC path: onApplyIdentityHeader is called after onSendRequest, so the SBC's injection decision is made after auth credentials are appended and is independent of SIP_FLAGS_VERBATIM. https://claude.ai/code/session_016Pg7MeJjEfiNfJNSSLFiPb
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to change how SIP identity headers are handled, primarily around SBC relay behavior, so SEMS can inject its configured signature when appropriate while also offering a way to suppress User-Agent/Server disclosure. It also includes a couple of unrelated cleanup/build changes.
Changes:
- Refactors request/reply identity-header injection through a new dialog event hook and wires SBC to apply a profile-based User-Agent/Server policy.
- Adds SBC profile documentation and config parsing for
send_user_agent, plus a warning about identity disclosure. - Includes small cleanup changes in MD5 signatures, SDP body handling, gitignore, and GitHub workflow triggers.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
core/md5.h |
Makes MD5 input pointer const-correct in the public declaration. |
core/md5.cpp |
Propagates const-correctness through MD5 implementation helpers. |
core/etc/sems.conf.sample |
Minor formatting cleanup in signature-related sample config comments. |
core/AmSipDialog.cpp |
Routes ACK User-Agent handling through the new identity-header hook. |
core/AmConfig.h |
Clarifies the meaning of the configured signature string. |
core/AmConfig.cpp |
Renames nearby comment to describe signature-based identity handling. |
core/AmBasicSipDialog.h |
Adds the new onApplyIdentityHeader extension point and its documentation. |
core/AmBasicSipDialog.cpp |
Implements the default identity-header hook and uses it for requests/replies. |
core/AmB2BSession.cpp |
Removes empty SDP parts instead of clearing them in place. |
apps/sbc/etc/transparent.sbcprofile.conf |
Documents the new SBC profile option controlling User-Agent/Server behavior. |
apps/sbc/SBCCallProfile.h |
Adds send_user_agent to the SBC call profile. |
apps/sbc/SBCCallProfile.cpp |
Reads send_user_agent from config and logs a disclosure warning. |
apps/sbc/SBCCallLeg.h |
Declares SBC override for identity-header policy. |
apps/sbc/SBCCallLeg.cpp |
Implements SBC-specific stripping/injection of User-Agent/Server headers. |
.gitignore |
Ignores build_check/. |
.github/workflows/macos_build.yml |
Removes push-triggered execution from macOS CI workflow. |
.github/workflows/freebsd_build.yml |
Removes push-triggered execution from FreeBSD CI workflow. |
.github/workflows/build_test.yml |
Removes push-triggered execution from packaging/hardening CI workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (AmConfig::Signature.length()) | ||
| reply.hdrs += SIP_HDR_COLSP(SIP_HDR_SERVER) + AmConfig::Signature + CRLF; | ||
| } | ||
| if (hdl) hdl->onApplyIdentityHeader(reply.hdrs, SIP_HDR_SERVER, flags); |
Comment on lines
+79
to
+90
| ## User-Agent / Server identity header policy | ||
| # | ||
| # send_user_agent=yes | ||
| # Inject the server identity string (configured via use_default_signature or | ||
| # signature in sems.conf) as a User-Agent header on outgoing requests and a | ||
| # Server header on outgoing replies. If the upstream UAC already provided a | ||
| # User-Agent, it is forwarded as-is; the signature is only added when the | ||
| # header is absent. | ||
| # Default: no (suppress both headers to prevent software-version disclosure; | ||
| # see RFC 3261 §20.41 and §20.35). | ||
| # | ||
| #send_user_agent=yes |
| if (send_user_agent && !AmConfig::Signature.empty()) | ||
| WARN("SBC profile '%s': send_user_agent=yes will disclose server identity " | ||
| "'%s' in User-Agent/Server headers on all outgoing SBC messages. " | ||
| "This may expose the server to targeted attacks (RFC 3261 SS20.41/20.35).\n", |
Comment on lines
+458
to
+462
| * The default implementation reproduces the original SEMS behaviour: inject | ||
| * AmConfig::Signature when the header is absent and SIP_FLAGS_VERBATIM is not | ||
| * set. Subclasses (e.g. the SBC) may override to enforce a different policy | ||
| * such as stripping forwarded identity headers or controlling injection via a | ||
| * per-call-profile option. |
| const char* hdr_name, | ||
| int flags) | ||
| { | ||
| if (!(flags & SIP_FLAGS_VERBATIM) && AmConfig::Signature.length()) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SIP_FLAGS_VERBATIM suppressed AmConfig::Signature injection on all SBC
outgoing requests (INVITE, re-INVITE, REGISTER relay, auth-retry REGISTER),
making the configured signature invisible regardless of sems.conf settings.
Replace the VERBATIM-gated injection with a presence check: inject the
signature whenever the User-Agent (requests) or Server (replies) header is
absent, regardless of the VERBATIM flag. This preserves verbatim B2BUA
relay for upstream UAs that supply their own header while ensuring
SEMS-originated requests (e.g. UACAuth retry after 401/407) carry the
configured identity.
Add hide_user_agent = yes to sems.conf to actively strip User-Agent from
requests and Server from replies, eliminating software fingerprinting as
an attack surface. RFC 3261 §20.41 and §20.35 both note that omitting
these headers reduces vulnerability to targeted attacks.
Fixes: #539
https://claude.ai/code/session_016Pg7MeJjEfiNfJNSSLFiPb