RDKCOM-5616: RDKBDEV-3470, RDKBACCL-1853: SSH and WebUI not working - Ethernet Configurable WAN I…

[anatar818]

RDKBACCL-1853 SSH and WebUI not working - Ethernet Configurable WAN Interface Integration

Reason for change: Verify configurable wan interface in BPI R4 (ethagent functionality)
Test Procedure: Build and flash the image ,Validate wan functionality for the customized interface name
Risks: None
Priority : P2

Dependant PRs
rdkcentral/ethernet-agent#67
rdkcentral/provisioning-and-management#312

[Copilot]

Pull request overview

This PR aims to restore SSH/WebUI accessibility when the WAN interface name is configurable (e.g., on BPI R4), by ensuring firewall logic uses the correct WAN interface name when generating iptables rules.

Changes:

• When FEATURE_RDKB_CONFIGURABLE_WAN_INTERFACE is enabled, initialize current_wan_ifname from syscfg (wan_physical_ifname) if sysevent doesn’t provide it.
• When ETH WAN is enabled, generate the SSH filter iptables rule using current_wan_ifname (when set) under the configurable-WAN feature flag.

---

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[AkhilaReddyK7]

Builds generated at https://gerrit.teamccp.com/#/c/962410/

[pradeeptakdas]

Based on @manigandanj input wanInterface buf size changed from BUFLEN_64 to 20 bytes in source/firewall/firewall.c to fix the build issue.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

source/firewall/firewall.c:2390

• In the configurable-WAN branch, if both current_wan_ifname and default_wan_ifname are empty and wan_physical_ifname is not set, the code leaves current_wan_ifname/ecm_wan_ifname empty. That can generate invalid iptables rules elsewhere (e.g., "-i -p tcp ..."). The non-configurable path avoids this by falling back to "erouter0"; the configurable path should keep an equivalent fallback and can simplify by always deriving ecm_wan_ifname from the final current_wan_ifname.

    if ('\0' == current_wan_ifname[0]) {
	  char wanInterface[20] = {'\0'};	
      syscfg_get(NULL, "wan_physical_ifname", wanInterface, sizeof(wanInterface));
	  if(wanInterface[0] != '\0'){     
        snprintf(current_wan_ifname, sizeof(current_wan_ifname), "%s", wanInterface);

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

source/firewall/firewall.c:2399

• In the configurable-WAN path, when wan_physical_ifname is empty you copy default_wan_ifname into ecm_wan_ifname using sizeof(current_wan_ifname). ecm_wan_ifname is a smaller buffer (20 bytes), so this passes an incorrect destmax to strcpy_s and can lead to overflow/constraint violations. Also, the legacy fallback to "erouter0" when default_wan_ifname is empty is skipped under this feature flag, leaving WAN ifnames empty.

      else{
		 safec_rc=strcpy_s(current_wan_ifname, sizeof(current_wan_ifname),default_wan_ifname); 
		 ERR_CHK(safec_rc);
		 safec_rc=strcpy_s(ecm_wan_ifname, sizeof(ecm_wan_ifname),default_wan_ifname);  
		 ERR_CHK(safec_rc);

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

source/firewall/firewall.c:2400

• Under FEATURE_RDKB_CONFIGURABLE_WAN_INTERFACE, when sysevent "current_wan_ifname" is empty and syscfg "wan_physical_ifname" is also empty, this falls back to copying default_wan_ifname even when it is empty. This removes the previous (non-feature) fallback to "erouter0" and can leave current_wan_ifname/ecm_wan_ifname empty, which later generates invalid iptables rules (e.g., "-i " with no interface).

		 safec_rc=strcpy_s(current_wan_ifname, sizeof(current_wan_ifname),default_wan_ifname); 
		 ERR_CHK(safec_rc);
		 safec_rc=strcpy_s(ecm_wan_ifname, sizeof(ecm_wan_ifname),default_wan_ifname);  
		 ERR_CHK(safec_rc);
	  }	  

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[Copilot]

🟡 Changes recommended

The configurable-WAN path changes how ecm_wan_ifname is sourced and can have security/behavioral impact (broad firewall rules tied to ecm_wan_ifname), plus it can leave current_wan_ifname empty in cases previously handled by fallback logic.

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

Review details

Suppressed comments (1)

source/firewall/firewall.c:2391

• Under FEATURE_RDKB_CONFIGURABLE_WAN_INTERFACE this block overwrites ecm_wan_ifname with the configured WAN interface (wan_physical_ifname/default/current). ecm_wan_ifname is used elsewhere as the CM diagnostics / internal interface (e.g., defaults set $$ecm_wan_ifname=wan0 in source/scripts/init/defaults/system_defaults_arm:890-894), and later firewall rules add broad INPUT accepts for ecm_wan_ifname; copying the external WAN interface here can unintentionally widen exposure. Also, unlike the non-feature path, this code can leave current_wan_ifname empty when both wan_physical_ifname and wan_ifname are unset, which can generate invalid iptables rules.

    if ('\0' == current_wan_ifname[0]) {
	  char wanInterface[20] = {'\0'};	
      syscfg_get(NULL, "wan_physical_ifname", wanInterface, sizeof(wanInterface));
	  if(wanInterface[0] != '\0'){   
	     safec_rc=strcpy_s(current_wan_ifname, sizeof(current_wan_ifname),wanInterface);

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The updated configurable-WAN code path contains a malformed snprintf that will fail compilation.

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

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The change is narrowly scoped and feature-flagged, with only a minor maintainability nit identified.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

source/firewall/firewall.c:2389

• Avoid hardcoding the interface-name buffer size (20) here; using IFNAMSIZ (already used elsewhere in the codebase for interface names) prevents future drift and makes the intended constraint explicit.

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

It introduces a broad behavior change for how ecm_wan_ifname is sourced under the feature flag, which may affect many WAN-facing firewall rules beyond SSH/WebUI and needs confirmation/adjustment before approval.

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

Review details

Suppressed comments (1)

source/firewall/firewall.c:2518

• With FEATURE_RDKB_CONFIGURABLE_WAN_INTERFACE enabled, the syscfg key ecm_wan_ifname is ignored and ecm_wan_ifname is derived earlier from current_wan_ifname / wan_physical_ifname. Since ecm_wan_ifname is used broadly for WAN-facing firewall rules (not just SSH/WebUI), please confirm this behavior change is intended for all those rules; if not, keep ecm_wan_ifname sourced from syscfg and instead apply the configurable interface only to the specific management rules that need it.

#ifndef FEATURE_RDKB_CONFIGURABLE_WAN_INTERFACE
   syscfg_get(NULL, "ecm_wan_ifname", ecm_wan_ifname, sizeof(ecm_wan_ifname));
#endif

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite