Skip to content

Conversation

@paolovisintin
Copy link
Collaborator

@paolovisintin paolovisintin commented Dec 18, 2025

This pull request introduces several improvements and fixes related to SIP proxy networking, firewall configuration, and developer tooling. The main focus is on enhancing Kamailio's handling of internal and external networks, opening new SIP-related ports, and providing new scripts and ignore rules for development convenience.

Kamailio SIP Proxy Improvements:

  • Enhanced the SET_SOCKET route in kamailio.cfg to more robustly determine and set the correct network socket for SIP messages, including explicit handling for INTERNAL_NETWORK (service network for Asterisk) and LOCALNETWORKS, with improved logging for debugging. [1] [2] [3] [4]
  • Modified NAT scenario handling in bootstrap.sh to add listeners for new SIP ports (6060/udp, 6060/tcp, 6061/tls) when behind NAT.
  • Added support for the SERVICE_IP variable in the Kamailio configuration template for better separation of service and private networks.

Firewall and Network Configuration:

  • Updated the firewall module to open additional SIP-related ports (6060/tcp, 6060/udp, 6061/tcp) alongside the standard SIP and RTP ports to support new proxy scenarios.

Developer Tooling and Scripts:

  • Added a new script baresipCall.sh to automate SIP call testing between different network scenarios (LAN, WAN, reverse), including colored output and progress visualization.
  • Improved the rsync.sh script by fixing the remote path for syncing project files to use /home/$2/ns8-nethvoice-proxy/
  • Updated ignore files to exclude SpecStory auto-save files from indexing and git, and to ignore project identity and explanation files in .specstory/. [1] [2]

Copy link
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

This PR adds support for NAT configurations with custom ports 6060/6061 and introduces a LOCAL_SOURCE_SUBNETS feature to handle SIP clients from local subnets differently than external clients, avoiding the use of advertised public IP for internal clients.

Key changes:

  • Added custom TCP/UDP port 6060 and TLS port 6061 for SIP traffic
  • Implemented LOCAL_SOURCE_SUBNETS functionality to selectively use private IP for clients in specified local subnets
  • Added firewall rules, configuration validation, and comprehensive documentation

Reviewed changes

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

Show a summary per file
File Description
imageroot/actions/create-module/20firewall Opens custom ports 6060 (TCP/UDP) and 6061 (TCP) in firewall
modules/kamailio/bootstrap.sh Adds listen directives for custom ports 6060/6061 and initializes LOCAL_SOURCE_SUBNETS environment variable
modules/kamailio/config/kamailio.cfg Implements CHECK_LOCAL_SOURCE route to selectively use private IP for requests from configured local subnets
modules/kamailio/config/template.kamailio-local.cfg Adds LOCAL_SOURCE_SUBNETS configuration variable
imageroot/actions/configure-module/20configure Handles LOCAL_SOURCE_SUBNETS configuration from API input
imageroot/actions/get-configuration/20read Returns LOCAL_SOURCE_SUBNETS in configuration output
imageroot/actions/configure-module/validate-input.json Adds validation schema for local_source_subnets array with CIDR pattern
tests/10_actions/15_configure_local_source_subnets.robot Adds automated tests for LOCAL_SOURCE_SUBNETS configuration
examples/test-local-source-subnets.sh Provides test script for manual verification of LOCAL_SOURCE_SUBNETS
examples/configure-with-local-subnets.json Sample configuration with local_source_subnets
docs/LOCAL_SOURCE_SUBNETS.md Technical documentation for LOCAL_SOURCE_SUBNETS feature
README_LOCAL_SOURCE_SUBNETS_IT.md Italian user guide for LOCAL_SOURCE_SUBNETS
CHANGELOG.md Documents the new LOCAL_SOURCE_SUBNETS feature

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

@Amygos Amygos force-pushed the 157-under-nat-localnet branch from 4e2b18c to 33786ee Compare January 26, 2026 08:55
Paolo added 6 commits January 26, 2026 10:36
Open custom ports:
  * 6060 udp/tcp: SIP
  * 6061 tcp: SIPS
Enable port 6060 and 6061 in the Kamailio bootstrap script.
Update SET_SOCKET route to handle LOCALNETWORKS and INTERNAL_NETWORK
more accurately. This forces PRIVATE_IP for LAN SIP traffic and uses
SERVICE_IP for service network destinations (e.g., Asterisk). Fixes NAT
connectivity issues for clients behind local subnets by ensuring
advertised address matches network scope.
Add a script for making SIP calls using the baresip application.
The script includes functions for printing banners, scenarios, usage,
steps, success messages, and information messages. It also includes
functions for starting the caller and callee, initiating the call,
displaying progress, and cleaning up after the call. The script
supports LAN, WAN reverse call scenarios.
Updated the rsync destination path in.sh script to correctly sync files
the specified directory on the server.
Added .cursorindexingignore to exclude SpecStory auto-save files from
indexing and allow explicit context inclusion via @ references. Also
added .specstory/.gitignore to ignore SpecStory project identity and
explanation files.
@Amygos Amygos force-pushed the 157-under-nat-localnet branch from 33786ee to 93fc0b7 Compare January 26, 2026 10:26
Wrap TT157 xlog statements in conditional debug checks to reduce
unnecessary log noise in production.
Copy link
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

Copilot reviewed 8 out of 9 changed files in this pull request and generated 10 comments.


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

@@ -5,7 +5,7 @@ then
if [[ $REPLY =~ ^[Yy]$ ]]
then
cd ..
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

The removal of the double slash in the remote path (from ://home/ to :/home/) is correct. However, note that this script assumes the remote directory structure already exists at /home/$2/ns8-nethvoice-proxy/. If the directory doesn't exist, rsync will fail. Consider adding documentation or a check to ensure the target directory exists.

Suggested change
cd ..
cd ..
ssh "$1" "sudo mkdir -p /home/$2/ns8-nethvoice-proxy" || { echo; echo "Failed to create remote directory /home/$2/ns8-nethvoice-proxy on $1"; exit 1; }

Copilot uses AI. Check for mistakes.
Comment on lines +179 to +186
ssh -f root@$CALLEE_HOST "baresip" 2>/dev/null
sleep 3
success "Extension ${CALLEE_EXT} ready (auto-answer)"

# Start caller and dial
MODE_UPPER=$(echo "$MODE" | tr '[:lower:]' '[:upper:]')
step "Dialing ${CALLEE_EXT} from ${CALLER_EXT} (${MODE_UPPER})..."
ssh root@$CALLER_HOST "baresip -e '/dial ${CALLEE_EXT}' > /tmp/baresip.log 2>&1 &" 2>/dev/null &
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

SSH commands lack proper error handling. If SSH connection fails (e.g., host unreachable, authentication failure), the script will continue executing and may produce misleading output. Consider adding error checks after critical SSH commands to ensure the test setup is valid before proceeding.

Suggested change
ssh -f root@$CALLEE_HOST "baresip" 2>/dev/null
sleep 3
success "Extension ${CALLEE_EXT} ready (auto-answer)"
# Start caller and dial
MODE_UPPER=$(echo "$MODE" | tr '[:lower:]' '[:upper:]')
step "Dialing ${CALLEE_EXT} from ${CALLER_EXT} (${MODE_UPPER})..."
ssh root@$CALLER_HOST "baresip -e '/dial ${CALLEE_EXT}' > /tmp/baresip.log 2>&1 &" 2>/dev/null &
if ! ssh -f root@"$CALLEE_HOST" "baresip"; then
echo -e "${RED}Failed to start auto-answer on ${CALLEE_EXT} via SSH to ${CALLEE_HOST}.${NC}"
exit 1
fi
sleep 3
success "Extension ${CALLEE_EXT} ready (auto-answer)"
# Start caller and dial
MODE_UPPER=$(echo "$MODE" | tr '[:lower:]' '[:upper:]')
step "Dialing ${CALLEE_EXT} from ${CALLER_EXT} (${MODE_UPPER})..."
if ! ssh root@"$CALLER_HOST" "baresip -e '/dial ${CALLEE_EXT}' > /tmp/baresip.log 2>&1 &"; then
echo -e "${RED}Failed to initiate call from ${CALLER_EXT} via SSH to ${CALLER_HOST}.${NC}"
exit 1
fi

Copilot uses AI. Check for mistakes.
Comment on lines +1106 to +1118
if (is_in_subnet($var(destination_ip), INTERNAL_NETWORK)) {
if ($shv(debug) == 1) xlog('L_WARN', "[TT157] - $ci $rm-$cs - SET_SOCKET: MATCH! $var(destination_ip) IS in INTERNAL_NETWORK \n");
if ($shv(debug) == 1) xlog('L_WARN', "[DEV] - $ci $rm-$cs - destination ip is in INTERNAL_NETWORK, forcing socket to SERVICE_IP \n");
# Force using the service network interface
$fs = SERVICE_IP + ":5060";
if ($shv(debug) == 1) xlog('L_WARN', "[TT157] - $ci $rm-$cs - SET_SOCKET: forced $fs \n");
# replace external in rtpengine_conf with internal
$var(rtpengine_conf) = $(var(rtpengine_conf){s.replace,external,internal});
if ($shv(debug) == 1) xlog('L_WARN', "[DEV] - $ci $rm-$cs - forced socket: $fs, rtpengine_conf: $var(rtpengine_conf) \n");
} else {
if ($shv(debug) == 1) xlog('L_WARN', "[TT157] - $ci $rm-$cs - SET_SOCKET: NO MATCH! $var(destination_ip) NOT in INTERNAL_NETWORK \n");
if ($shv(debug) == 1) xlog('L_WARN', "[DEV] - $ci $rm-$cs - destination ip $var(destination_ip) is NOT in INTERNAL_NETWORK \n");
}
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

The INTERNAL_NETWORK check at line 1106 can override the LOCALNETWORKS socket setting from line 1092. If a destination IP is in both LOCALNETWORKS and INTERNAL_NETWORK, the socket will first be set to PRIVATE_IP:6060 (line 1092), but then immediately overridden to SERVICE_IP:5060 (line 1110). Consider restructuring this logic with else-if to ensure only one socket configuration is applied, or document why both checks should run sequentially.

Copilot uses AI. Check for mistakes.
Comment on lines +1071 to +1075
if ($du != $null && $du != 0 && $du != "") {
$var(destination_ip) = $(du{uri.host});
} else {
$var(destination_ip) = $(ru{uri.host}); # fetch the destination ip from ru
}
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

The new logic to extract destination_ip from $du when present (lines 1071-1075) is a good improvement. However, the comparison $du != 0 is checking if $du is the numeric value 0, which may not be meaningful for a URI string. Consider simplifying to just check $du != $null && $du != "" for clarity.

Copilot uses AI. Check for mistakes.
"5060-5061/udp",
"6060/tcp",
"6060/udp",
"6061/tcp",
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

The firewall configuration opens port 6061/tcp but not 6061/udp, while port 5061 is opened for both TCP and UDP (lines 13-14). The listener configuration in bootstrap.sh uses TLS on port 6061 (line 39), which typically only needs TCP. However, for consistency with how port 5061 is configured, consider whether 6061/udp should also be opened, or document why the asymmetry is intentional.

Copilot uses AI. Check for mistakes.
if ($shv(debug) == 1) xlog('L_WARN', "[DEV] - $ci $rm-$cs - destination ip is in LOCALNETWORKS \n");
$var(from_socket) = PRIVATE_IP;
set_advertised_address(PRIVATE_IP);
$fs = PRIVATE_IP + ":6060";
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

The hardcoded port ":6060" is used here for LOCALNETWORKS traffic, but this differs from the standard port :5060 used elsewhere. While this appears intentional based on the PR description, there's an inconsistency: the listener setup in bootstrap.sh adds port 6060 only for NAT scenarios, but this code will try to use it regardless of NAT configuration. This could cause issues when not behind NAT.

Suggested change
$fs = PRIVATE_IP + ":6060";
$fs = PRIVATE_IP;

Copilot uses AI. Check for mistakes.
# this route set the from socket accoding also to LOCALNETWORKS
# route[SET_SOCKET]
# this route sets the from socket according to LOCALNETWORKS and INTERNAL_NETWORK
# - LOCALNETWORKS: uses PRIVATE_IP for advertised address
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

The comment says "uses PRIVATE_IP for advertised address" but the actual implementation now sets the socket to PRIVATE_IP:6060 instead of using set_advertised_address(). The comment should be updated to accurately reflect that the socket is being set to a specific IP:port combination, not just using an advertised address.

Suggested change
# - LOCALNETWORKS: uses PRIVATE_IP for advertised address
# - LOCALNETWORKS: sets the socket to PRIVATE_IP:6060 (specific IP:port)

Copilot uses AI. Check for mistakes.
route(DLGURI);
if ( is_method("ACK") ) {
# ACK is forwarded statelessly
# Ensure correct socket is used (especially for INTERNAL_NETWORK)
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

Adding route(SET_SOCKET) before route(NATMANAGE) for ACK messages is a good enhancement to ensure the correct socket is used. However, the comment "especially for INTERNAL_NETWORK" suggests this is primarily for internal network handling, but SET_SOCKET also modifies socket settings for LOCALNETWORKS. Ensure this change doesn't cause issues for ACK messages in other scenarios by verifying the socket selection logic works correctly for all network types.

Suggested change
# Ensure correct socket is used (especially for INTERNAL_NETWORK)
# Ensure correct socket is used for ACKs across all relevant network types

Copilot uses AI. Check for mistakes.
@Amygos Amygos force-pushed the 157-under-nat-localnet branch from e9c345c to 76a48a1 Compare January 27, 2026 14:11
Introduce helper to generate IPv4 rich rules mapping SIP ports 5060/5061
to alternate targets when proxy is behind NAT. Ensure previous rules are
removed before applying updates, preventing stale firewall entries. Also
store multiple local networks in environment, improving flexibility for
multi-subnet deployments.
Include portsadm in org.nethserver.authorizations label to avoid 403
Forbidden error when running configure-module task. This acts as a
temporary workaround for failing rich rules update.
@Amygos Amygos force-pushed the 157-under-nat-localnet branch from e7aefa7 to 6f22ae7 Compare January 28, 2026 14:36
Ensure any port forwarding rich rules created during module
configuration are cleaned up when destroying the module, but only in
scenarios where BEHIND_NAT is true and a public IP was present.
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