caclmgrd interface rules patch 1 by gupurush · Pull Request #197 · sonic-net/sonic-host-services

gupurush · 2024-12-27T03:27:35Z

Added management port exclusion (!, -i, eth0) for data plane protocols in iptable rules. Data plane protocols like BGP, BFD, and VXLAN are restricted to data interfaces only, while management protocols (NTP, SNMP, SSH) retain management port access. This enforces proper traffic segregation between management and data plane.

What is the motivation for this PR?
To enhance security by properly segregating management and data plane traffic through iptable rules in caclmgrd.

How did you do it?
Added exclude_mgmt_port rule (!, -i, eth0)
Applied exclusion to data plane protocols (BGP, BFD, VXLAN)
Preserved management port access for management services (NTP, SNMP, SSH)
Implemented conditional exclusion for other ACL rules based on service type

mssonicbld · 2024-12-27T03:27:37Z

/azp run

linux-foundation-easycla · 2024-12-27T03:27:38Z

The committers listed above are authorized under a signed CLA.

✅ login: gupurush / name: Nanma Purushotam (c0ca6d9, d131a47, 61461b5)

azure-pipelines · 2024-12-27T03:27:46Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2024-12-27T03:32:10Z

/azp run

azure-pipelines · 2024-12-27T03:32:18Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-01-02T20:49:13Z

/azp run

azure-pipelines · 2025-01-02T20:49:21Z

Azure Pipelines successfully started running 1 pipeline(s).

bingwang-ms · 2025-01-31T16:51:54Z

@ZhaohuiS Could you help review?

ZhaohuiS · 2025-02-05T05:34:04Z

scripts/caclmgrd

+                            if (dst_port not in self.ACL_SERVICES["NTP"]["dst_ports"] and
+                                dst_port not in self.ACL_SERVICES["SNMP"]["dst_ports"] and
+                                dst_port not in self.ACL_SERVICES["SSH"]["dst_ports"]):
+                                rule_cmd = self.exclude_mgmt_port(rule_cmd)


For EXTERNAL_CLIENT, there is no DST_PORT setting for it, with your change, does it mean it will block restapi or gnmi traffic from eth0?

@qiluo-msft @prsunny could you please review this change? It will prohibit traffic from eth0 except NTP,SNMP and SSH.

"EXTERNAL_CLIENT": { "ip_protocols": ["tcp"], "multi_asic_ns_to_host_fwd":True },

@ZhaohuiS We can add the protocols in ACL_SERVICES that has to go through eth0, could we have the defined list of protocols?

As I know, EXTERNAL_CLIENT is designed to be used for restapi and gnmi, and the port is not defined in ACL_SERVICES , we can define flexible ports for them in acl.json.

agree with @ZhaohuiS , this can break existing scenarios.

Removed check so EXTERNAL_CLIENT is not affected; Updated tests. Please help review

@gupurush could you please also update test case, otherwise submodule advance PR test will fail,

https://github.com/sonic-net/sonic-mgmt/blob/master/tests/cacl/test_cacl_application.py

The best solution should be:

skip test_cacl_application from PR test

imagebuild PR get merged

update test case

add test_cacl_application back for PR test

@gupurush could you please update test_cacl_application.py to support your change in this PR? And provide the test evidence here, just want to make sure the image code change could be verified and no regression will be introduced. Thanks.

@gupurush please update the test_cacl_application and enable it for PR test.

mssonicbld · 2025-02-25T22:22:43Z

/azp run

azure-pipelines · 2025-02-25T22:22:51Z

Azure Pipelines successfully started running 1 pipeline(s).

Description of PR Summary: Fixes # (issue) sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. Disable test_cacl_application in PR test temporarily for merging submodule PR. > pytest_assert(len(missing_iptables_rules) == 0, "Missing expected iptables rules: {}" .format(repr(missing_iptables_rules))) E Failed: Missing expected iptables rules: {'-A INPUT -p tcp -m tcp --dport 179 -j ACCEPT'} For any changes in caclmgrd, if it will fail test_cacl_application case, need to follow the steps: skip test_cacl_application from PR test imagebuild PR get merged update test case add test_cacl_application back for PR test Type of change Bug fix Testbed and Framework(new/improvement) New Test case Skipped for non-supported platforms Test case improvement Back port request 202205 202305 202311 202405 202411 202505 Approach What is the motivation for this PR? sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. How did you do it? Disable test_cacl_application in PR test temporarily for merging submodule PR. How did you verify/test it? Retrigger PR test for sonic-host-services submodule advance PR. Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>

Description of PR Summary: Fixes # (issue) sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. Disable test_cacl_application in PR test temporarily for merging submodule PR. > pytest_assert(len(missing_iptables_rules) == 0, "Missing expected iptables rules: {}" .format(repr(missing_iptables_rules))) E Failed: Missing expected iptables rules: {'-A INPUT -p tcp -m tcp --dport 179 -j ACCEPT'} For any changes in caclmgrd, if it will fail test_cacl_application case, need to follow the steps: skip test_cacl_application from PR test imagebuild PR get merged update test case add test_cacl_application back for PR test Type of change Bug fix Testbed and Framework(new/improvement) New Test case Skipped for non-supported platforms Test case improvement Back port request 202205 202305 202311 202405 202411 202505 Approach What is the motivation for this PR? sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. How did you do it? Disable test_cacl_application in PR test temporarily for merging submodule PR. How did you verify/test it? Retrigger PR test for sonic-host-services submodule advance PR. Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com> Signed-off-by: Aharon Malkin <amalkin@nvidia.com>

Description of PR Summary: Fixes # (issue) sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. Disable test_cacl_application in PR test temporarily for merging submodule PR. > pytest_assert(len(missing_iptables_rules) == 0, "Missing expected iptables rules: {}" .format(repr(missing_iptables_rules))) E Failed: Missing expected iptables rules: {'-A INPUT -p tcp -m tcp --dport 179 -j ACCEPT'} For any changes in caclmgrd, if it will fail test_cacl_application case, need to follow the steps: skip test_cacl_application from PR test imagebuild PR get merged update test case add test_cacl_application back for PR test Type of change Bug fix Testbed and Framework(new/improvement) New Test case Skipped for non-supported platforms Test case improvement Back port request 202205 202305 202311 202405 202411 202505 Approach What is the motivation for this PR? sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. How did you do it? Disable test_cacl_application in PR test temporarily for merging submodule PR. How did you verify/test it? Retrigger PR test for sonic-host-services submodule advance PR. Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com> Signed-off-by: Guy Shemesh <gshemesh@nvidia.com>

Description of PR Summary: Fixes # (issue) sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. Disable test_cacl_application in PR test temporarily for merging submodule PR. > pytest_assert(len(missing_iptables_rules) == 0, "Missing expected iptables rules: {}" .format(repr(missing_iptables_rules))) E Failed: Missing expected iptables rules: {'-A INPUT -p tcp -m tcp --dport 179 -j ACCEPT'} For any changes in caclmgrd, if it will fail test_cacl_application case, need to follow the steps: skip test_cacl_application from PR test imagebuild PR get merged update test case add test_cacl_application back for PR test Type of change Bug fix Testbed and Framework(new/improvement) New Test case Skipped for non-supported platforms Test case improvement Back port request 202205 202305 202311 202405 202411 202505 Approach What is the motivation for this PR? sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. How did you do it? Disable test_cacl_application in PR test temporarily for merging submodule PR. How did you verify/test it? Retrigger PR test for sonic-host-services submodule advance PR. Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>

Description of PR Summary: Fixes # (issue) sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. Disable test_cacl_application in PR test temporarily for merging submodule PR. > pytest_assert(len(missing_iptables_rules) == 0, "Missing expected iptables rules: {}" .format(repr(missing_iptables_rules))) E Failed: Missing expected iptables rules: {'-A INPUT -p tcp -m tcp --dport 179 -j ACCEPT'} For any changes in caclmgrd, if it will fail test_cacl_application case, need to follow the steps: skip test_cacl_application from PR test imagebuild PR get merged update test case add test_cacl_application back for PR test Type of change Bug fix Testbed and Framework(new/improvement) New Test case Skipped for non-supported platforms Test case improvement Back port request 202205 202305 202311 202405 202411 202505 Approach What is the motivation for this PR? sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. How did you do it? Disable test_cacl_application in PR test temporarily for merging submodule PR. How did you verify/test it? Retrigger PR test for sonic-host-services submodule advance PR. Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com> Signed-off-by: Guy Shemesh <gshemesh@nvidia.com>

Description of PR Summary: Fixes # (issue) sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. Disable test_cacl_application in PR test temporarily for merging submodule PR. > pytest_assert(len(missing_iptables_rules) == 0, "Missing expected iptables rules: {}" .format(repr(missing_iptables_rules))) E Failed: Missing expected iptables rules: {'-A INPUT -p tcp -m tcp --dport 179 -j ACCEPT'} For any changes in caclmgrd, if it will fail test_cacl_application case, need to follow the steps: skip test_cacl_application from PR test imagebuild PR get merged update test case add test_cacl_application back for PR test Type of change Bug fix Testbed and Framework(new/improvement) New Test case Skipped for non-supported platforms Test case improvement Back port request 202205 202305 202311 202405 202411 202505 Approach What is the motivation for this PR? sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. How did you do it? Disable test_cacl_application in PR test temporarily for merging submodule PR. How did you verify/test it? Retrigger PR test for sonic-host-services submodule advance PR. Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com> Signed-off-by: Yael Tzur <ytzur@nvidia.com>

Description of PR Summary: Fixes # (issue) sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. Disable test_cacl_application in PR test temporarily for merging submodule PR. > pytest_assert(len(missing_iptables_rules) == 0, "Missing expected iptables rules: {}" .format(repr(missing_iptables_rules))) E Failed: Missing expected iptables rules: {'-A INPUT -p tcp -m tcp --dport 179 -j ACCEPT'} For any changes in caclmgrd, if it will fail test_cacl_application case, need to follow the steps: skip test_cacl_application from PR test imagebuild PR get merged update test case add test_cacl_application back for PR test Type of change Bug fix Testbed and Framework(new/improvement) New Test case Skipped for non-supported platforms Test case improvement Back port request 202205 202305 202311 202405 202411 202505 Approach What is the motivation for this PR? sonic-net/sonic-host-services#197 added a new change for caclmgrd, PR test for submodule advanced PR of sonic-host-services failed. How did you do it? Disable test_cacl_application in PR test temporarily for merging submodule PR. How did you verify/test it? Retrigger PR test for sonic-host-services submodule advance PR. Signed-off-by: Zhaohui Sun <zhaohuisun@microsoft.com>

gupurush force-pushed the caclmgrd-interface-rules-patch-1 branch from 91e84f7 to 38f32c7 Compare December 27, 2024 03:32

gupurush added 2 commits January 2, 2025 12:48

patch: iptables rules for eth0 interface filtering

c0ca6d9

patch add iptables rules for eth0 interface filtering

d131a47

gupurush force-pushed the caclmgrd-interface-rules-patch-1 branch from 38f32c7 to d131a47 Compare January 2, 2025 20:49

gupurush marked this pull request as ready for review January 7, 2025 20:08

bingwang-ms assigned ZhaohuiS Jan 31, 2025

ZhaohuiS reviewed Feb 5, 2025

View reviewed changes

Removed eth0 check for external client

61461b5

gupurush requested review from ZhaohuiS and prsunny February 25, 2025 22:38

ZhaohuiS approved these changes Feb 26, 2025

View reviewed changes

gupurush mentioned this pull request Feb 27, 2025

Skipping test temporarily for caclmgrd updates sonic-net/sonic-mgmt#17231

Closed

qiluo-msft merged commit 6006e05 into sonic-net:master Mar 26, 2025
5 checks passed

This was referenced Jun 4, 2025

[submodule][master] Update submodule sonic-host-services to the latest HEAD automatically sonic-net/sonic-buildimage#22160

Merged

Skip test_cacl_application for PR test temporarily sonic-net/sonic-mgmt#18785

Merged

mssonicbld mentioned this pull request Jun 4, 2025

[action] [PR:18785] Skip test_cacl_application for PR test temporarily sonic-net/sonic-mgmt#18791

Merged

11 tasks

gupurush mentioned this pull request Jun 5, 2025

Update iptables to block BGP (TCP 179) traffic on eth0 sonic-net/sonic-mgmt#18834

Merged

11 tasks

gupurush mentioned this pull request Jul 2, 2025

Update iptables to block BGP (TCP 179) traffic on eth0 - 202505 sonic-net/sonic-mgmt#19351

Merged

11 tasks

Conversation

gupurush commented Dec 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Dec 27, 2024

Uh oh!

linux-foundation-easycla bot commented Dec 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azure-pipelines bot commented Dec 27, 2024

Uh oh!

mssonicbld commented Dec 27, 2024

Uh oh!

azure-pipelines bot commented Dec 27, 2024

Uh oh!

mssonicbld commented Jan 2, 2025

Uh oh!

azure-pipelines bot commented Jan 2, 2025

Uh oh!

bingwang-ms commented Jan 31, 2025

Uh oh!

ZhaohuiS Feb 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gupurush Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

ZhaohuiS Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

prsunny Feb 19, 2025

Choose a reason for hiding this comment

Uh oh!

gupurush Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

ZhaohuiS Feb 26, 2025

Choose a reason for hiding this comment

Uh oh!

ZhaohuiS Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

ZhaohuiS Mar 28, 2025

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Feb 25, 2025

Uh oh!

azure-pipelines bot commented Feb 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

gupurush commented Dec 27, 2024 •

edited

Loading

linux-foundation-easycla bot commented Dec 27, 2024 •

edited

Loading

ZhaohuiS Feb 5, 2025 •

edited

Loading