RDKBNETWOR-76 : WireguardPort and IPv4/IPv6 support for WireGuard… - #4
RDKBNETWOR-76 : WireguardPort and IPv4/IPv6 support for WireGuard…#4sameerunnisa9 wants to merge 2 commits into
Conversation
DESCRIPTION : Port WireGuard enhancements for tunnel name, listen/remote port handling,
and IPv6 local/remote addressing.
REASON FOR CHANGE : Align upstream RdkVpnManager with TunnelName, WireguardPort,
endpoint/port updates after handshake, and LocalIPv6/RemoteIPv6 support.
TESTING DONE : Results captured on RDKBNETWOR-76.
Signed-off-by: Sameerunnisa S <sameerunnisa.s@telekom-digital.com>
|
All contributors have signed the CLA ✍️ ✅ |
There was a problem hiding this comment.
Pull request overview
Adds TR-181/manager support for additional WireGuard configuration parameters (WireguardPort, TunnelName) and IPv4/IPv6 addressing, including updates to config generation and tunnel status handling.
Changes:
- Extend TR-181 data model + DML to support LocalIPv6, RemoteIPv6, TunnelName, and WireguardPort (get/set + persistence).
- Update WireGuard tunnel status collection to parse
wg show wg0output and persist endpoint/port updates. - Adjust
vpn_config.shand config generation to include IPv6 addresses and Wireguard listen port, and to pass IPv6 tunnel parameters.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| source/TR-181/middle_layer_src/vpn_manager_dml.h | Adds DML setter prototype for ulong parameters. |
| source/TR-181/middle_layer_src/vpn_manager_dml.c | Implements WireguardPort get/set and adds TunnelName/RemoteIPv6 handling in tunnel DML. |
| source/TR-181/middle_layer_src/vpn_manager_dml_apis.h | Adds new backend API prototypes (TunnelName/RemoteIPv6 + PSK generator). |
| source/TR-181/middle_layer_src/vpn_manager_dml_apis.c | Adds syscfg persistence for new fields, updates tunnel status parsing, updates script invocation arguments, and adds PSK generation helper. |
| source/TR-181/middle_layer_src/plugin_main.c | Registers WireGuard_SetParamUlongValue. |
| source/TR-181/middle_layer_src/plugin_main_apis.h | Extends datamodel structs/flags for TunnelName, RemoteIPv6, LocalIPv6, and WireguardPort. |
| files/vpn_config.sh | Updates wg0.conf generation for IPv6 addressing and Wireguard listen port; extends peer creation arguments. |
| config/RdkVpnManager.xml | Extends TR-181 schema for LocalIPv6, WireguardPort, TunnelName, and RemoteIPv6. |
Suppressed comments (2)
source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:615
- Potential buffer overflow:
strncpy(ip6, string, sizeof(string))uses the size of the localstringbuffer instead of the caller-providedip6buffer length (size). This can overwrite the destination whensizeis smaller thanSTRING_MAX_LEN(RemoteIPv6 is 64 bytes).
if (0 == syscfg_get(NULL,syscfg_var, string, sizeof(string)))
strncpy(ip6,string,sizeof(string));
if ((unsigned int)size > strlen(string))
snprintf(ip6, size, "%s", string);
source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:708
- Potential buffer overflow:
strncpy(pskKey, presharedKey, sizeof(presharedKey))copies up toSTRING_MAX_LENbytes intopskKey, but callers pass smaller buffers (e.g., 64 bytes). Use the providedsizeto bound the copy.
strncpy(pskKey,presharedKey,sizeof(presharedKey));
if ((unsigned int)size > strlen(presharedKey))
snprintf(pskKey, size, "%s", presharedKey);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [ -n "$6" ]; then | ||
| echo "PresharedKey = $6" >> $WG_CONFIG_FILE | ||
| echo "PSK $6" >> $WG_DEBUG_FILE | ||
| echo "AllowedIPs = $5/32,$6/128" >> $WG_CONFIG_FILE |
| if [ -z "$3" ] || [ -z "$4" ]; then | ||
| echo "No end point or remote needed as its configure as server" >> $WG_DEBUG_FILE | ||
| else | ||
| echo "NO PSK $6" >> $WG_DEBUG_FILE | ||
| echo "Endpoint = $3:$4" >> $WG_DEBUG_FILE | ||
| fi |
| if (0 == syscfg_get(NULL,syscfg_var, string, sizeof(string))) | ||
| strncpy(tunnelname,string,sizeof(string)); | ||
|
|
||
| if ((unsigned int)size > strlen(string)) | ||
| snprintf(tunnelname, size, "%s", string); |
| CcspTraceInfo(("%s %d - Enabling the WireGuard through the script. \n", __FUNCTION__, __LINE__)); | ||
| v_secure_system(VPN_CONFIG_SCRIPT " enable %s %d", pMyObject->LocalIP, netMastToCIDR(pMyObject->Subnet)); | ||
| 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++) |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Suppressed comments (14)
source/TR-181/middle_layer_src/vpn_manager_dml.c:689
- Tunnel validation still requires RemoteIP (IPv4) even when RemoteIPv6 is provided, which blocks IPv6-only tunnel configuration despite adding RemoteIPv6 support.
if (strlen(pWireGuardTu->RemoteIP) == 0)
{
AnscCopyString(pReturnParamName, "RemoteIP");
*puLength = AnscSizeOfString("RemoteIP");
CcspTraceError(("%s-RemoteIP is not valid for tunnel instance:%d \n",__FUNCTION__,ins));
source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:615
- CosaDml_WireGuardTunnelGetRemoteIPv6() copies into ip6 using sizeof(string) (64) instead of the caller-provided buffer size, which can overflow smaller buffers and may leave the destination unterminated.
if (0 == syscfg_get(NULL,syscfg_var, string, sizeof(string)))
strncpy(ip6,string,sizeof(string));
if ((unsigned int)size > strlen(string))
snprintf(ip6, size, "%s", string);
source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:708
- WireGuard_TunnelGeneratePskKey() copies into pskKey with strncpy(pskKey, ..., sizeof(presharedKey)), which ignores the destination buffer size and can overflow if the caller provided a smaller buffer.
strncpy(pskKey,presharedKey,sizeof(presharedKey));
if ((unsigned int)size > strlen(presharedKey))
snprintf(pskKey, size, "%s", presharedKey);
source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:290
- create_config_file() passes ULONG values to v_secure_system using "%ld" (signed long). This is the wrong format for ULONG and can print incorrect values on some platforms; use "%lu" and cast to unsigned long.
CcspTraceInfo(("%s %d - Enabling the WireGuard through the script. \n", __FUNCTION__, __LINE__));
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++)
files/vpn_config.sh:84
- The Endpoint line is being written to the debug log file instead of wg0.conf, so the peer config will miss Endpoint and the tunnel cannot connect in client mode. Also, if RemotePort is used to indicate server mode, treat "0" as server-mode (no endpoint) too.
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
source/TR-181/middle_layer_src/vpn_manager_dml.c:307
- WireGuard port is formatted with sprintf("%d", uValue) even though uValue is ULONG, and the write is unbounded (sprintf). This can truncate/misformat on platforms where ULONG != int. Also consider rejecting values outside valid UDP port range (0-65535).
if (AnscEqualString(ParamName, "WireguardPort", TRUE))
{
pMyObject->WireguardPort = uValue;
sprintf(buf, "%d", uValue);
syscfg_set(NULL, "Wireguard_Port", buf);
source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:99
- VpnDmlInitialize() returns ANSC_STATUS but falls off the end without returning a value, which is undefined behavior in C.
pMyObject->WireguardPort = atoi(string);
}
}
}
source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:695
- In WireGuard_TunnelGeneratePskKey(), presharedKey is overwritten with the literal command string and then used as the value if popen()/fgets() fail, which can end up storing "wg genpsk" into syscfg as the key.
snprintf(presharedKey,sizeof(presharedKey),"wg genpsk");
fd = popen(presharedKey, "r");
if (fd)
{
fgets(presharedKey,sizeof(presharedKey),fd);
source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:301
- create_config_file() passes RemotePort (ULONG) to v_secure_system using "%ld" (signed long). Use "%lu" with an unsigned long cast to avoid type/format mismatch.
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");
source/TR-181/middle_layer_src/vpn_manager_dml_apis.c:247
- CosaDml_WireGuardTunnelGetStatus() commits RemoteEndPoint/RemotePort back to syscfg even when endpoint parsing fails, which can overwrite existing configuration with empty strings. Only update syscfg when both remote_endpoint and port were successfully parsed.
snprintf(syscfg_endpoint, sizeof(syscfg_endpoint), WIREGUARDTU_PARAM_REMEP, tuIns);
syscfg_set(NULL,syscfg_endpoint,remote_endpoint);
snprintf(syscfg_remport, sizeof(syscfg_remport), WIREGUARDTU_PARAM_REMPORT, tuIns);
syscfg_set(NULL, syscfg_remport, port);
syscfg_commit();
files/vpn_config.sh:41
- When IPv6 address is not provided, this writes a trailing comma into the WireGuard Address field ("Address = v4/cidr,/64"), which makes wg0.conf invalid. Build the Address line so the IPv6 part is only appended when $4 is non-empty.
echo "Address = $2/$3,$4/64" >> $WG_CONFIG_FILE
if [ "" != "$5" ] && [ "0" != "$5" ]; then
echo "ListenPort = $5" >> $WG_CONFIG_FILE
files/vpn_config.sh:79
- AllowedIPs is always written as "$5/32,$6/128". If $6 is empty, the resulting ",/128" is invalid syntax. Append the IPv6 allowed IP only when $6 is provided.
echo "AllowedIPs = $5/32,$6/128" >> $WG_CONFIG_FILE
files/vpn_config.sh:91
- The create_tun path brings the interface down/up when no PSK is provided. This will disrupt existing tunnels and is not required for a no-PSK peer; it should just omit the PresharedKey line.
else
echo "NO PSK $7" >> $WG_DEBUG_FILE
wg-quick down wg0; wg-quick up wg0
fi
config/RdkVpnManager.xml:156
- RemoteIPv6 is declared as string(256) in the TR-181 schema, but the implementation stores it in a 64-byte buffer (COSA_DML_WIREGUARD_TUNNEL.RemoteIPv6[64]) and syscfg uses STRING_MAX_LEN=64. This mismatch can confuse consumers and lead to unexpected truncation.
<name>RemoteIPv6</name>
<type>string(256)</type>
<syntax>string</syntax>
<writable>true</writable>
| if (0 == syscfg_get(NULL,syscfg_var, string, sizeof(string))) | ||
| strncpy(tunnelname,string,sizeof(string)); | ||
|
|
||
| if ((unsigned int)size > strlen(string)) | ||
| snprintf(tunnelname, size, "%s", string); | ||
|
|
WireguardPort , TunnelName and IPv6 support for WireGuard VPN