Skip to content

RDKB-67001: Resolve Firewall Configuration Failures - #440

Open
rirfha948 wants to merge 8 commits into
developfrom
topic/67001
Open

rirfha948 wants to merge 8 commits into
developfrom
topic/67001

Conversation

@rirfha948

@rirfha948 rirfha948 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Reason for change:

Implement general code improvements to enhance reliability, maintainability, and overall software quality.
Address identified issues and optimize existing implementation.
Test Procedure:

Verify the updated logic and functionality.
Validate that existing features continue to work as expected.
Perform regression testing to ensure no unintended impact on related functionality.
Review logs and system behavior for any anomalies after the update.
Risks: Low
Priority: P1
RDKB67001-logs.txt

Copilot AI balanced review requested due to automatic review settings October 8, 2026 12:33
@rirfha948
rirfha948 requested review from a team as code owners October 8, 2026 12:33
Comment thread source/firewall/firewall.c Fixed
@rdkcmf-jenkins

Copy link
Copy Markdown
Contributor

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/utopia/440/rdkb/components/opensource/ccsp/Utopia

  • Commit: 5dde06c

Report detail: gist'

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Invalid CmDiag values can still enter generated rulesets, and custom-port parental-control enforcement is regressed.

5 open findings
What changed in this PR

Adds firewall-rule validation and telemetry to prevent corrupt CmDiag interface data from invalidating generated rulesets.

Changes:

  • Validates CmDiag interface names for selected IPv4/IPv6 rules.
  • Adds telemetry reporting and linkage.
  • Removes parental-control custom-port rule insertion and initializes Wi-Fi buffers.
File Description
source/​firewall/​Makefile.am Links telemetry support.
source/​firewall/​firewall.h Declares the validation API.
source/​firewall/​firewall.c Adds validation, telemetry, and rule-generation changes.
source/​firewall/​firewall_priv.c Removes custom-port insertion helper.
source/​firewall/​firewall_ipv6.c Adds CmDiag validation to an IPv6 rule.
source/​firewall/​firewall_custom.h Removes the helper declaration.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread source/firewall/firewall.c Outdated
Comment thread source/firewall/firewall.c Outdated
Comment thread source/firewall/firewall.c Outdated
Comment thread source/firewall/firewall.c Outdated
Comment thread source/firewall/firewall_ipv6.c Outdated
Copilot AI balanced review requested due to automatic review settings October 8, 2026 12:39
@rdkcmf-jenkins

Copy link
Copy Markdown
Contributor

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

  • Protex Server Path: /home/blackduck/github/utopia/440/rdkb/components/opensource/ccsp/Utopia

  • Commit: cfd383c

Report detail: gist'

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Some configurations fail to compile, and corrupt interface data can still reach multiple generated rules.

5 open findings
2 resolved since last review

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

Comment thread source/firewall/firewall.c Outdated
Comment thread source/firewall/firewall.c Outdated
…tting applied

Reason for Change:
AC3, AC4
Implement general code improvements to enhance reliability, maintainability, and overall software quality.
Address identified issues and optimize existing implementation.
Test Procedure:

Verify the updated logic and functionality.
Validate that existing features continue to work as expected.
Perform regression testing to ensure no unintended impact on related functionality.
Review logs and system behavior for any anomalies after the update

Priority: P1
Risk :Medium
Copilot AI balanced review requested due to automatic review settings October 8, 2026 12:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

P1 multi-platform firewall behavior changes require targeted device and regression validation.

0 open findings

5 resolved since last review

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@rirfha948
rirfha948 requested review from rajkamal-cv and snayak002c and a balanced review from Copilot October 8, 2026 13:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The BCM-MIPS path can generate an invalid rule insertion index and cause iptables-restore to reject the ruleset.

1 open finding

🧠 Review effort: Balanced

Comment thread source/firewall/firewall.c Outdated
Copilot AI balanced review requested due to automatic review settings October 9, 2026 11:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new platform condition can emit an unsupported GWMETA rule on Raspberry Pi builds and cause firewall restoration to fail.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Comment thread source/firewall/firewall_priv.c Outdated
Copilot AI balanced review requested due to automatic review settings October 12, 2026 04:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread source/firewall/firewall.c Outdated
Comment thread source/firewall/firewall.c Outdated
Comment thread source/firewall/firewall.c Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 06:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Invalid bridge-mode configuration can still expose management ports, and the validator accepts invalid Linux interface names.

2 open findings
4 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Reject colons and special names in interface validation

source/​firewall/​firewall.c:2357

This validator still accepts : and the special names ./.., although Linux rejects them as network-interface names. A corrupt value such as lan0:bad therefore survives validation and is emitted into the restore file, so this change does not reliably prevent malformed cmdiag_ifname rules. Reject those cases here.

🧠 Review effort: Balanced

Comment thread source/firewall/firewall.c
Copilot AI balanced review requested due to automatic review settings October 13, 2026 06:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Rejected interface data can leave remote management ports exposed over IPv6 in bridge mode.

1 open finding
2 resolved since last review

🧠 Review effort: Balanced

Comment thread source/firewall/firewall.c Outdated
Copilot AI balanced review requested due to automatic review settings October 13, 2026 08:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The bridge-mode fallback can apply LAN access policy to WAN traffic, and empty configuration incorrectly generates corruption telemetry.

3 open findings

🧠 Review effort: Balanced

Comment thread source/firewall/firewall.c Outdated
Comment thread source/firewall/firewall.c Outdated
…revent SSH restriction bypass'

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 13, 2026 08:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Missing fail-closed fallbacks can expose management services when the CMDIAG interface is rejected.

0 open findings

3 resolved since last review

🧠 Review effort: Balanced

@rirfha948 rirfha948 changed the title RDKB:67001 -[Field]Multiple firewall rules error causing rules not getting applied RDKB-67001: [Field]Multiple firewall rules error causing rules not getting applied Oct 13, 2026
@rirfha948 rirfha948 changed the title RDKB-67001: [Field]Multiple firewall rules error causing rules not getting applied RDKB-67001: Resolve Firewall Configuration Failures Oct 13, 2026
Copilot AI balanced review requested due to automatic review settings October 14, 2026 03:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A duplicate function prevents compilation, and empty CMDIAG fallbacks can omit security restrictions.

1 open finding

🧠 Review effort: Balanced

Comment thread source/firewall/firewall.c
Copilot AI balanced review requested due to automatic review settings October 14, 2026 03:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Invalid interface handling can fail open and expose remote-management services.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

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