github-actions[bot] commented on issue #14358:
URL: https://github.com/apache/cloudstack/issues/14358#issuecomment-6062952055
## ๐ฏ Triage report
Network ACL / firewall protocol validation
(`NetworkACLServiceImpl.validateProtocol()` and equivalents in
`FirewallManagerImpl`, `Ipv6ServiceImpl`, `RoutedIpv4ManagerImpl`) uses
`String.contains()` against the concatenated capability string
(`"tcp,udp,icmp,all"`) instead of checking membership in a parsed list. This
lets malformed protocol values like `cp`, `t`, or `,` pass validation, get
stored/listed via the API, but silently fail to be applied on the VR (agent
logs a warning and drops the rule). The result is a rule that appears to exist
but provides no actual firewalling โ a silent security-relevant functional gap.
### ๐ Assessment
| Dimension | Value | Reasoning |
|---|---|---|
| **Type** | type:bug | Validation logic incorrectly accepts invalid input,
leading to a rule that is silently never enforced. |
| **Component** | component:networking, component:virtual-router | Affects
Network ACL/firewall rule validation and VR-side ACL application. |
| **Severity** | Severity:Major | Not a crash, but silently fails to enforce
a security rule the user believes is active (allow/deny goes unenforced without
any visible error to the user). |
| **Labels** | type:bug, component:networking, component:virtual-router,
Severity:Major | See above. |
| **Coding agent** | Suitable | Root cause, affected files/methods, and the
fix approach (split the capability string and compare against trimmed list
entries) are precisely identified by the reporter; fix is self-contained and
localized. |
No similar or duplicate issues were found in the repository.
<details><summary>๐ก Notes and suggestions</summary>
- Fix should replace
`supportedProtocolsForAclRules.contains(protocol.toLowerCase())` with a check
against `Arrays.asList(supportedProtocolsForAclRules.split(","))` with each
entry trimmed, applied consistently across `NetworkACLServiceImpl`,
`FirewallManagerImpl`, `Ipv6ServiceImpl`, and `RoutedIpv4ManagerImpl`.
- Worth adding/augmenting unit tests for `validateProtocol()` with inputs
like `cp`, `dp,ic`, `,`, and `tcp,udp` to confirm they are now rejected.
- Since this silently drops security rules without any API/UI error,
consider whether existing stored rules with invalid protocols (created before
the fix) should be flagged or cleaned up โ out of scope for the code fix itself
but worth a mention in the PR description.
</details>
> Generated by [Daily Issue
Triage](https://github.com/apache/cloudstack/actions/runs/37785922422) ยท
sonnet50 79.4K ยท
[โท](https://github.com/search?q=repo%3Aapache%2Fcloudstack+%22gh-aw-workflow-call-id%3A+apache%2Fcloudstack%2Fdaily-issue-triage%22&type=issues)
>
<details>
<summary>Add this agentic workflows to your repo</summary>
To install this agentic workflow, run
```
gh aw add
githubnext/agentics/workflows/daily-issue-triage.md@d7c1dc4b72b00607a67caaffdcc216cb64379cf9
```
</details>
<!-- gh-aw-agentic-workflow: Daily Issue Triage, engine: copilot, version:
1.0.52, model: claude-sonnet-5, id: 37785922422, workflow_id:
daily-issue-triage, run:
https://github.com/apache/cloudstack/actions/runs/37785922422 -->
<!-- gh-aw-workflow-call-id: apache/cloudstack/daily-issue-triage -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
