Skip to content

RDKBNETWOR-76 : Add WireGuard VPN manager with IPv4/IPv6 support - #5

Open
sameerunnisa9 wants to merge 2 commits into
rdkcentral:mainfrom
sameerunnisa9:RdkVpnManager_Upstream
Open

sameerunnisa9 wants to merge 2 commits into
rdkcentral:mainfrom
sameerunnisa9:RdkVpnManager_Upstream

Conversation

@sameerunnisa9

Copy link
Copy Markdown

WireGuard VPN manager with IPv4/IPv6 support

Reason for change: Introduce RdkVpnManager to manage WireGuard tunnels over
TR-181 (X_RDK.Wireguard), including interface bring-up/tear-down, peer
configuration, ListenPort, and IPv4/IPv6 address and AllowedIPs handling via
vpn_config.sh and syscfg (wireguard_enabled, wireguard_local_ipv4,
wireguard_local_ipv6, wireguard_subnet).
Test Procedure:

  1. Enable WireGuard via TR-181 and verify wg0.conf is created with IPv4 and IPv6 Address, ListenPort, and PrivateKey.
  2. Create a peer tunnel and confirm PublicKey, Endpoint, and IPv4/IPv6 AllowedIPs are written correctly.
  3. Bring the tunnel UP/DOWN and verify syscfg wireguard_enabled and firewall-restart are updated.
  4. Confirm public key retrieval and tunnel status through the data model.

Testing Done : Results are captured in RDKBNETWOR-76
Risks: None.

Copilot AI lite review requested due to automatic review settings August 3, 2026 11:44
@sameerunnisa9

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@rdkcmf-jenkins

Copy link
Copy Markdown
Contributor

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 7 files pending identification.

  • Protex Server Path: /home/blackduck/github/RdkVpnManager/5/rdkcentral/RdkVpnManager

  • Commit: 47c5c8b

Report detail: gist'

Copilot AI left a comment

Copy link
Copy Markdown

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 introduces a new RdkVpnManager component that manages WireGuard tunnels via a TR-181 data model, including syscfg persistence and a helper shell script for bringing up/down interfaces and syncing peers.

Changes:

  • Adds a new vpnmanager CCSP component (ssp_* glue) that registers a TR-181 data model and connects to the CCSP message bus.
  • Implements TR-181 middle-layer DML + backend manager for WireGuard interface and peer/tunnel configuration (IPv4/IPv6, ports, keys).
  • Adds packaging/build integration, systemd service unit, XML data model, and vpn_config.sh script.

Reviewed changes

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

Show a summary per file
File Description
source/VpnManager/ssp_messagebus_interface.h Declares message bus engagement/teardown hooks.
source/VpnManager/ssp_messagebus_interface.c Implements CCSP message bus init/register logic and callbacks.
source/VpnManager/ssp_main.c Adds daemon entrypoint, signal handling, and bus/component startup.
source/VpnManager/ssp_internal.h Defines component IDs/paths and internal interfaces.
source/VpnManager/ssp_global.h Provides global CCSP/ANSC includes for the component.
source/VpnManager/ssp_action.c Creates/engages/cancels CCSP controller + data model registration.
source/VpnManager/Makefile.am Builds the vpnmanager binary and links middle-layer library.
source/TR-181/middle_layer_src/vpn_manager_internal.h Internal WireGuard DM object and helper macros/constants.
source/TR-181/middle_layer_src/vpn_manager_internal.c WireGuard DM object creation/init/remove and public key retrieval.
source/TR-181/middle_layer_src/vpn_manager_dml.h Declares TR-181 DML getters/setters/commit/rollback for WireGuard.
source/TR-181/middle_layer_src/vpn_manager_dml.c Implements TR-181 DML logic and commit/apply behavior.
source/TR-181/middle_layer_src/vpn_manager_dml_apis.h Declares backend “CosaDml_*” APIs and helpers used by DML.
source/TR-181/middle_layer_src/vpn_manager_dml_apis.c Implements syscfg-backed persistence, wg show status, config generation, and peer sync.
source/TR-181/middle_layer_src/plugin_main.h Plugin version/export definitions for TR-181 integration.
source/TR-181/middle_layer_src/plugin_main.c Registers DML functions and initializes backend manager via plugin API.
source/TR-181/middle_layer_src/plugin_main_apis.h Defines backend manager + WireGuard data structures and enums.
source/TR-181/middle_layer_src/plugin_main_apis.c Implements backend manager create/init/remove lifecycle.
source/TR-181/middle_layer_src/Makefile.am Builds middle-layer static library for the component.
source/TR-181/Makefile.am Adds TR-181 subdir build wiring.
source/Makefile.am Adds TR-181 and VpnManager subdirs to build.
Makefile.am Top-level automake wiring to build source/.
files/vpn_config.sh Shell helper to generate wg0.conf, bring interface up/down, and sync peers.
configure.ac Autotools configuration to generate Makefiles for new subdirs.
config/RdkVpnManager.xml TR-181 data model XML definition for X_RDK_Wireguard and Tunnel table.
config/RdkVPNManager.service Systemd unit to start the vpnmanager daemon.
Suppressed comments (2)

source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:99

  • VpnDmlInitialize() is declared to return ANSC_STATUS but falls off the end without returning a value, which is a build error with -Werror=return-type.
        {
             pMyObject->WireguardPort = atoi(string);
        }	
    }
}

files/vpn_config.sh:99

  • Same issue as above: this logs the config file contents (including secrets) and also uses WG_CONFIG_FILE without $. Redact secrets before logging or avoid logging the config entirely.
    echo "*********************************" >> $WG_DEBUG_FILE
    cat WG_CONFIG_FILE >> $WG_DEBUG_FILE
    echo "*********************************" >> $WG_DEBUG_FILE

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

Comment thread source/TR-181/middle_layer_src/vpn_manager_dml_apis.c
Comment thread source/TR-181/middle_layer_src/vpn_manager_dml_apis.c
Comment thread source/TR-181/middle_layer_src/vpn_manager_dml_apis.c
Comment thread source/TR-181/middle_layer_src/vpn_manager_internal.c
Comment thread source/TR-181/middle_layer_src/plugin_main.c
Comment thread source/VpnManager/ssp_action.c
Comment thread source/VpnManager/ssp_action.c
Comment thread source/VpnManager/ssp_main.c
Comment thread source/VpnManager/ssp_main.c
Comment thread files/vpn_config.sh

@mhughesacn mhughesacn left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi @sameerunnisa9 : Please will you change the header line in these new files:
"If not stated otherwise in this file or this component's Licenses.txt file"
change "Licenses.txt" to "LICENSE" as the older form is no longer used.
The files should scan clean on the update.
Thank you
MartinH, RDK CMF Compliance Team

Reason for change: Introduce RdkVpnManager to manage WireGuard tunnels over
                                  TR-181 (X_RDK.Wireguard), including interface bring-up/tear-down, peer
                                  configuration, ListenPort, and IPv4/IPv6 address and AllowedIPs handling via
                                  vpn_config.sh and syscfg (wireguard_enabled, wireguard_local_ipv4,
                                  wireguard_local_ipv6, wireguard_subnet).
Test Procedure:
1. Enable WireGuard via TR-181 and verify wg0.conf is created with IPv4 and IPv6 Address, ListenPort, and PrivateKey.
2. Create a peer tunnel and confirm PublicKey, Endpoint, and IPv4/IPv6 AllowedIPs are written correctly.
3. Bring the tunnel UP/DOWN and verify syscfg wireguard_enabled and firewall-restart are updated.
4. Confirm public key retrieval and tunnel status through the data model.

Testing Done : Results are captured in RDKBNETWOR-76
Risks: None.

Signed-off-by: Sameerunnisa S <sameerunnisa.s@telekom-digital.com>
Copilot AI review requested due to automatic review settings August 5, 2026 05:10
@sameerunnisa9
sameerunnisa9 force-pushed the RdkVpnManager_Upstream branch from 47c5c8b to 694636c Compare August 5, 2026 05:10

Copilot AI left a comment

Copy link
Copy Markdown

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 25 out of 25 changed files in this pull request and generated 13 comments.

Suppressed comments (7)

source/TR-181/middle_layer_src/vpn_manager_dml.c:807

  • ins is declared but never used, which will fail compilation under -Wall -Werror (-Wunused-variable).
    }

    ULONG ins = pWireGuardTu->InstanceNumber;

    pWireGuardTu->Enable = FALSE;

source/VpnManager/ssp_main.c:60

  • On setsid() failure, daemonize() exits with status 0, which can mask startup failures. This should exit non-zero.
    if (setsid() < 0) 
    {
        CcspTraceInfo(("Error demonizing (setsid)! %d - %s\n", errno, strerror(errno)));
        exit(0);
    }

files/vpn_config.sh:61

  • In the UP command, this cat uses the literal filename WG_CONFIG_FILE instead of the $WG_CONFIG_FILE variable, so the debug log won’t show the actual config (and may fail if no such file exists).
elif [ "$COMMAND" = "UP" ]; then
    echo "*********************************" >> $WG_DEBUG_FILE 
    cat WG_CONFIG_FILE >> $WG_DEBUG_FILE
    echo "*********************************" >> $WG_DEBUG_FILE

files/vpn_config.sh:99

  • In the SYNC command, this cat uses the literal filename WG_CONFIG_FILE instead of the $WG_CONFIG_FILE variable, so the debug log won’t show the actual config (and may fail if no such file exists).
elif [ "$COMMAND" = "SYNC" ]; then
    echo "*********************************" >> $WG_DEBUG_FILE
    cat WG_CONFIG_FILE >> $WG_DEBUG_FILE
    echo "*********************************" >> $WG_DEBUG_FILE

source/VpnManager/ssp_messagebus_interface.c:75

  • If component_id or path is NULL, the function logs an error but continues and passes NULL into CCSP_Message_Bus_Init/Register_Path, which can crash or mis-register the component. This should fail fast and return an error status.
    if ( ! component_id || ! path )
    {
        CcspTraceError((" !!! ssp_Mbi_MessageBusEngage: component_id or path is NULL !!!\n"));
    }

source/VpnManager/ssp_main.c:51

  • On fork failure, daemonize() exits with status 0, which makes service managers treat startup as successful even though daemonization failed. This should exit non-zero.

This issue also appears on line 56 of the same file.

        case -1:{
            // Error
            CcspTraceInfo(("Error daemonizing (fork)! %d - %s\n", errno, strerror(
                            errno)));
		
            exit(0);
            break;}

source/VpnManager/ssp_main.c:274

  • SIGKILL cannot be caught or handled; registering a handler for it is ineffective and can confuse maintenance/troubleshooting.
    signal(SIGSEGV, sig_handler);
    signal(SIGBUS, sig_handler);
    signal(SIGKILL, sig_handler);
    signal(SIGFPE, sig_handler);
    signal(SIGILL, sig_handler);

Comment thread files/vpn_config.sh
Comment thread source/TR-181/middle_layer_src/vpn_manager_internal.c
Comment thread source/TR-181/middle_layer_src/vpn_manager_internal.c
Comment thread source/TR-181/middle_layer_src/vpn_manager_dml_apis.c
Comment thread source/TR-181/middle_layer_src/vpn_manager_dml_apis.c
Comment thread source/VpnManager/ssp_action.c
Comment thread source/VpnManager/ssp_main.c
Comment thread source/VpnManager/ssp_main.c
Comment thread source/VpnManager/ssp_main.c
Comment thread source/TR-181/middle_layer_src/vpn_manager_dml_apis.c
Copilot AI review requested due to automatic review settings September 2, 2026 12:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

There are multiple confirmed build-breaking issues (missing returns and -Werror-triggering constructs/format strings) plus functional/script bugs and a sensitive-key logging issue that should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (14)

source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:99

  • VpnDmlInitialize() has return type ANSC_STATUS but currently falls off the end without returning a value, which is undefined behavior and will typically fail the build with -Werror.
        if (0 == syscfg_get(NULL, "Wireguard_Port", string, sizeof(string)))
        {
             pMyObject->WireguardPort = atoi(string);
        }	
    }
}

source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:117

  • Assignment inside the if-condition (fp = popen(...)) will trigger -Wall warnings (often treated as errors via -Werror). Split the assignment from the condition to avoid build breaks.
    if (fp = popen("wg show", "r"))
    {

source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:456

  • sprintf(buf, "%d", val) is the wrong format for ULONG and can fail the build under -Wformat/-Werror. Prefer snprintf with %lu.
ANSC_STATUS
CosaDml_WireGuardTunnelSetRemotePort(ULONG tuIns, ULONG val)
{
    char syscfg_var[MAX_SIZE +1]={0};
    char buf[BUF_SIZE]={0};

    sprintf(buf, "%d", val);
    snprintf(syscfg_var, sizeof(syscfg_var),WIREGUARDTU_PARAM_REMPORT, tuIns);

    syscfg_set(NULL,syscfg_var,buf);

source/TR-181/middle_layer_src/vpn_manager_dml.c:305

  • sprintf(buf, "%d", uValue) uses the wrong format for ULONG and may fail compilation under -Wformat/-Werror. Use snprintf with %lu.
        {
            pMyObject->WireguardPort = uValue;
	    sprintf(buf, "%d", uValue);
	    syscfg_set(NULL, "Wireguard_Port", buf);
            syscfg_commit();

source/TR-181/middle_layer_src/vpn_manager_internal.c:42

  • WireGuard_GetPublicKey(): fgets() return value isn’t checked and keylen can be 0, making pubKey[keylen-1] an underflow. The function also doesn’t return an ANSC_STATUS on success, which is undefined behavior and can break builds.
    fgets(pubKey,64,fPtr);
    keylen = strlen(pubKey);

    if(pubKey[keylen-1] == '\n')
        pubKey[keylen-1] = '\0';

source/TR-181/middle_layer_src/plugin_main.c:308

  • Assignment in the if-condition (tmpSubsystemPrefix = ...) will typically trigger -Wall warnings and can break the build under -Werror. Split assignment from condition.
        if ( tmpSubsystemPrefix = g_GetSubsystemPrefix(g_pDslhDmlAgent) )
        {
            AnscCopyString(g_SubSysPrefix_Irep, tmpSubsystemPrefix);
        }

source/TR-181/middle_layer_src/vpn_manager_dml_apis.h:35

  • CosaDml_WireGuardGetStatus() is declared twice in this header, which is redundant and can confuse maintenance. Keep a single declaration.
ANSC_STATUS
CosaDml_WireGuardGetStatus(DML_VPN_IF_CFG_STATUS *st);

source/VpnManager/ssp_action.c:162

  • ssp_engage() returns ANSC_STATUS_SUCCESS even if RegisterCcspDataModel fails, which hides initialization failures from callers and supervision logic.
    }

    return ANSC_STATUS_SUCCESS;
}

source/VpnManager/ssp_messagebus_interface.c:75

  • ssp_Mbi_MessageBusEngage() logs when component_id/path are NULL but continues anyway, which will likely crash in CCSP_Message_Bus_Init / CCSP_Message_Bus_Register_Path. Return failure immediately on invalid inputs.
    if ( ! component_id || ! path )
    {
        CcspTraceError((" !!! ssp_Mbi_MessageBusEngage: component_id or path is NULL !!!\n"));
    }

source/VpnManager/ssp_messagebus_interface.c:95

  • bus_handle is a handle/pointer type; logging it with %x is undefined and will warn/fail under -Wformat/-Werror. Use %p with a (void*) cast.
    CcspTraceInfo(("INFO: bus_handle: 0x%8x \n", bus_handle));

source/VpnManager/ssp_main.c:353

  • SIGKILL cannot be caught/handled; registering a handler for it is ineffective and may hide that shutdown cleanup isn’t happening as expected. Remove the SIGKILL handler registration.
    signal(SIGSEGV, sig_handler);
    signal(SIGBUS, sig_handler);
    signal(SIGKILL, sig_handler);
    signal(SIGFPE, sig_handler);

files/vpn_config.sh:61

  • The script uses cat WG_CONFIG_FILE (missing $), so it won’t actually dump the wg0.conf contents to the debug log.
elif [ "$COMMAND" = "UP" ]; then
    echo "*********************************" >> $WG_DEBUG_FILE 
    cat WG_CONFIG_FILE >> $WG_DEBUG_FILE
    echo "*********************************" >> $WG_DEBUG_FILE

files/vpn_config.sh:86

  • In create_tun, the Endpoint line is appended to the debug log instead of wg0.conf, so peers won’t have an Endpoint configured.
    if [ -z "$3" ] || [ -z "$4" ]; then
	    echo "No end point or remote needed as its configure as server" >> $WG_DEBUG_FILE
    else
	    echo "Endpoint = $3:$4" >> $WG_DEBUG_FILE
    fi

files/vpn_config.sh:92

  • The script writes the pre-shared key value into $WG_DEBUG_FILE ("PSK $7"), which is sensitive material and should not be logged.
	echo "PresharedKey = $7" >> $WG_CONFIG_FILE
        echo "PSK $7" >> $WG_DEBUG_FILE
    else
        echo "NO PSK $7" >> $WG_DEBUG_FILE
	wg-quick down wg0; wg-quick up wg0
  • Files reviewed: 25/25 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment on lines +289 to +301
v_secure_system(VPN_CONFIG_SCRIPT " enable %s %d %s %ld", pMyObject->LocalIP, netMastToCIDR(pMyObject->Subnet), pMyObject->LocalIPv6, pMyObject->WireguardPort);
for (i = 0; i < 5; i++)
{
PDML_VPN_TUN_CFG pTunnel = &(wireGuard->WireGuardTu[i]);

if (!pTunnel->Enable)
continue;

CcspTraceInfo(("%s %d - Creating Tunnel through the script. \n", __FUNCTION__, __LINE__));
v_secure_system(VPN_CONFIG_SCRIPT " create_tun %s %s %ld %s %s %s",
pTunnel->PeerPublicKey, pTunnel->RemoteEndPoint,
pTunnel->RemotePort,pTunnel->RemoteIP,pTunnel->RemoteIPv6,
pTunnel->PSKEnable ? pTunnel->PreSharedKey:"\0");
Comment thread files/vpn_config.sh
wg genkey | tee $WG_PRIV_KEY_FILE | wg pubkey | tee $WG_PUB_KEY_FILE
echo "Key file not found....Generating it." >> $WG_DEBUG_FILE
fi
PrivKey=$(<$WG_PRIV_KEY_FILE)
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.

4 participants