Added CACL entry for Redfish

[shreyansh-nexthop]

Why I did it

The Redfish docker (docker-sonic-redfish) binds host port 0.0.0.0:443:18080 (per the docker-sonic-redfish rules in sonic-buildimage), so any IP that can reach the BMC management interface on TCP/443 can hit the Redfish API. caclmgrd only emits iptables rules for services that are registered in its ACL_SERVICES dict. Without an entry for REDFISH, any operator-defined ACL_TABLE referencing it is silently dropped i.e. the framework has no way to translate the operator's intent into iptables rules. This patch adds that registration and gates it so it only takes effect on platforms where Redfish actually runs.

How I did it

1. Registration: Added a REDFISH entry (tcp/443) to ACL_SERVICES in scripts/caclmgrd, mirroring the existing SSH/SNMP/NTP entries.
2. Platform gating: REDFISH only ships on BMC platforms, so to keep a stray services=["REDFISH"] template from programming tcp/443 rules elsewhere, REDFISH handling is gated on the FEATURE.redfish state:
   • A RedfishAllowed flag, checked in get_acl_rules_and_translate_to_iptables_commands - REDFISH services are skipped (with a warning) when the feature isn't enabled.
   • A FEATURE table subscriber flips the flag on runtime config feature state redfish enabled/disabled and re-walks so iptables converges immediately.
   • The flag is seeded from CONFIG_DB in init, before the initial iptables walk, so a caclmgrd restart doesn't leave tcp/443 momentarily ungated.
     Net: REDFISH rules are programmed iff FEATURE.redfish.state == "enabled". On non-BMC platforms a stray REDFISH template is a no-op (warn + ignore).
3. Tests: Unit tests covering the walker for IPv4 + IPv6, in both enabled (rules emitted) and disabled (rules skipped) states.

How to verify it

1. Confirm the entry shipped:

   sudo grep -A4 '"REDFISH"' /usr/local/bin/caclmgrd

2. Confirm the feature is enabled (rules are only programmed when it is):

   sonic-db-cli CONFIG_DB hget "FEATURE|redfish" state    # expect: enabled

3. Create the CTRLPLANE table (acl-loader manages rules, not tables), then load rules via acl-loader:

   # Table (one-time; acl-loader does not create CTRLPLANE tables)
   redis-cli -n 4 HSET "ACL_TABLE|REDFISH_ACL" \
       policy_desc REDFISH_ACL type CTRLPLANE stage ingress services@ REDFISH
   
   # Rules
   cat > /tmp/redfish_acl.json <<'EOF'
   {
       "acl": {
           "acl-sets": {
               "acl-set": {
                   "REDFISH-ACL": {
                       "config": { "name": "REDFISH-ACL" },
                       "acl-entries": {
                           "acl-entry": {
                               "1": {
                                   "config": { "sequence-id": 1 },
                                   "ip": { "config": { "source-ip-address": "<your-source-ip>/32" } },
                                   "actions": { "config": { "forwarding-action": "ACCEPT" } }
                               }
                           }
                       }
                   }
               }
           }
       }
   }
   EOF
   
   acl-loader update full /tmp/redfish_acl.json

   The acl-set name REDFISH-ACL maps to table REDFISH_ACL (uppercased, -→_). acl-loader auto-appends a DEFAULT_RULE (DROP), so the allowlist behaves as deny-by-default — no explicit deny rule needed. The destination port (443) comes from the ACL_SERVICES entry, so the rule only specifies the source IP.

4. Confirm caclmgrd translated them into iptables:

   show acl table REDFISH_ACL
   show acl rule REDFISH_ACL
   sudo iptables -L INPUT -n --line-numbers | grep ':443'

   You should see your ACCEPT for <your-source-ip> on dport 443 plus a trailing DROP.

Remove when done:

acl-loader delete REDFISH_ACL
redis-cli -n 4 DEL "ACL_TABLE|REDFISH_ACL"

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[judyjoseph]

LGTM, @shreyansh-nexthop can you add sonic-mgmt test case to cover this.
Also can you refer may be this PR : #9 to check tests using acl-loader for this new ACL type ?

[shreyansh-nexthop]

LGTM, @shreyansh-nexthop can you add sonic-mgmt test case to cover this. Also can you refer may be this PR : #9 to check tests using acl-loader for this new ACL type ?

Sure @judyjoseph, will add sonic-mgmt test cases. Thanks!

[ZhaohuiS]

@shreyansh-nexthop I have a question, why introduce a new ACL table, not use EXTERNAL_CLIENT table instead?

[shreyansh-nexthop]

EXTERNAL_CLIENT has no dst_ports, while for REDFISH I want a fixed dst_port of 443. A dedicated REDFISH entry auto-scopes every rule to 443 (rather than every operator-written rule needing a manual --dport 443 match), and matches the existing SSH/SNMP/NTP pattern where each protocol gets its own ACL_SERVICES entry.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ZhaohuiS]

@shreyansh-nexthop could you pls run sonic-mgmt cacl related case to see if need to update them or add new cases to cover this new scenario?

[shreyansh-nexthop]

@shreyansh-nexthop could you pls run sonic-mgmt cacl related case to see if need to update them or add new cases to cover this new scenario?

@ZhaohuiS, the sonic-mgmt CACL testcases needs modification. I have done the code changes internally, currently testing it. Will soon raise the PR for it.

[yxieca]

Nice, tightly-scoped change overall — caclmgrd test pattern is followed cleanly and the PR description clears the description-as-merge-gate (root cause + fix + verification). One blocker plus a couple of questions before merge.

Blocking

1. REDFISH is platform-specific but ACL_SERVICES registration is unconditional (scripts/caclmgrd:111-115)

All other entries in ACL_SERVICES (SSH, NTP, SNMP, EXTERNAL_CLIENT) describe services that exist on every SONiC platform — their dst_ports always correspond to a real listener. REDFISH is the first entry whose listener (bmcweb in docker-sonic-redfish) only ships on BMC-equipped platforms.

Today, on a non-BMC platform, an operator-defined ACL_TABLE|REDFISH_ACL with services=["REDFISH"] gets logged as "unrecognized service" and is silently dropped. After this PR, the same configuration on a non-BMC platform will:

• emit iptables -A INPUT -p tcp --dport 443 ... rules, and
• once num_ctrl_plane_acl_rules > 0, append the default-deny tail of the INPUT chain.

If anything else happens to listen on tcp/443 on that platform (future HTTPS REST, gRPC-TLS, NetConf-over-TLS, telemetry-over-HTTPS, a third-party agent), the operator's "Redfish ACL" template silently governs that traffic instead — or worse, locks them out. This is exactly the kind of fleet-template-meets-mixed-SKU foot-gun the framework should refuse to load itself into.

Please gate the REDFISH handling on platform/feature presence. Two reasonable shapes:

• Feature-gated dispatch: in get_acl_rules_and_translate_to_iptables_commands (or at ACL_SERVICES init), skip/warn for REDFISH unless FEATURE.redfish is enabled — analogous to how bfdAllowed / VxlanAllowed already gate other code paths in this same file.
• Conditional registration: only insert the REDFISH entry into ACL_SERVICES when the Redfish FEATURE / docker is present at daemon init.

Either makes services=["REDFISH"] on a non-BMC platform behave the same as it does today (warn + ignore), so a shared CTRLPLANE template across mixed SKUs doesn't accidentally program iptables on the wrong boxes.

Please clarify

2. Companion sonic-buildimage PR? (PR description)

The body references "the docker-sonic-redfish rules in sonic-buildimage" that bind 0.0.0.0:443:18080. Without that change, this entry is dormant; with it, the BMC-platform gating story above depends on how docker-sonic-redfish is FEATURE-gated. Please link the buildimage PR so we can verify the two changes ship together and the FEATURE table is set up the way the gating in (1) would check.

3. Why a SONiC switch CACL for a BMC-hosted service?

A one-line addition to the description ("on integrated chassis the BMC mgmt IP is reachable via the switch CPU mgmt path; this lets operators filter inbound 443 at the switch") would prevent reviewer confusion and make the platform-gating assumption explicit.

Non-blocking nits

• scripts/caclmgrd:111-115 — multi_asic_ns_to_host_fwd: False is the only False in the visible neighborhood. A one-line comment naming the assumption (Redfish container binds the host net on 0.0.0.0:443:18080, no per-namespace forward needed) would save a future reader from chasing the buildimage docker config.
• tests/caclmgrd/test_redfish_acl_vectors.py:65-76 — IPv6 vector covers ACCEPT only; the IPv4 vector covers ACCEPT + DROP. Adding a v6 DROP row exercises the ip6tables ... -j DROP translation path symmetrically.

Reviewed by AI agent on behalf of Ying.

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[shreyansh-nexthop]

Nice, tightly-scoped change overall — caclmgrd test pattern is followed cleanly and the PR description clears the description-as-merge-gate (root cause + fix + verification). One blocker plus a couple of questions before merge.

Blocking

1. REDFISH is platform-specific but ACL_SERVICES registration is unconditional (scripts/caclmgrd:111-115)

All other entries in ACL_SERVICES (SSH, NTP, SNMP, EXTERNAL_CLIENT) describe services that exist on every SONiC platform — their dst_ports always correspond to a real listener. REDFISH is the first entry whose listener (bmcweb in docker-sonic-redfish) only ships on BMC-equipped platforms.

Today, on a non-BMC platform, an operator-defined ACL_TABLE|REDFISH_ACL with services=["REDFISH"] gets logged as "unrecognized service" and is silently dropped. After this PR, the same configuration on a non-BMC platform will:

• emit iptables -A INPUT -p tcp --dport 443 ... rules, and
• once num_ctrl_plane_acl_rules > 0, append the default-deny tail of the INPUT chain.

If anything else happens to listen on tcp/443 on that platform (future HTTPS REST, gRPC-TLS, NetConf-over-TLS, telemetry-over-HTTPS, a third-party agent), the operator's "Redfish ACL" template silently governs that traffic instead — or worse, locks them out. This is exactly the kind of fleet-template-meets-mixed-SKU foot-gun the framework should refuse to load itself into.

Please gate the REDFISH handling on platform/feature presence. Two reasonable shapes:

• Feature-gated dispatch: in get_acl_rules_and_translate_to_iptables_commands (or at ACL_SERVICES init), skip/warn for REDFISH unless FEATURE.redfish is enabled — analogous to how bfdAllowed / VxlanAllowed already gate other code paths in this same file.
• Conditional registration: only insert the REDFISH entry into ACL_SERVICES when the Redfish FEATURE / docker is present at daemon init.

Either makes services=["REDFISH"] on a non-BMC platform behave the same as it does today (warn + ignore), so a shared CTRLPLANE template across mixed SKUs doesn't accidentally program iptables on the wrong boxes.

Please clarify

2. Companion sonic-buildimage PR? (PR description)

The body references "the docker-sonic-redfish rules in sonic-buildimage" that bind 0.0.0.0:443:18080. Without that change, this entry is dormant; with it, the BMC-platform gating story above depends on how docker-sonic-redfish is FEATURE-gated. Please link the buildimage PR so we can verify the two changes ship together and the FEATURE table is set up the way the gating in (1) would check.

3. Why a SONiC switch CACL for a BMC-hosted service?

A one-line addition to the description ("on integrated chassis the BMC mgmt IP is reachable via the switch CPU mgmt path; this lets operators filter inbound 443 at the switch") would prevent reviewer confusion and make the platform-gating assumption explicit.

Non-blocking nits

• scripts/caclmgrd:111-115 — multi_asic_ns_to_host_fwd: False is the only False in the visible neighborhood. A one-line comment naming the assumption (Redfish container binds the host net on 0.0.0.0:443:18080, no per-namespace forward needed) would save a future reader from chasing the buildimage docker config.
• tests/caclmgrd/test_redfish_acl_vectors.py:65-76 — IPv6 vector covers ACCEPT only; the IPv4 vector covers ACCEPT + DROP. Adding a v6 DROP row exercises the ip6tables ... -j DROP translation path symmetrically.

Reviewed by AI agent on behalf of Ying.

Thanks @yxieca, addressed in the latest revision:

(1) Blocking (platform gating): REDFISH is now gated on FEATURE.redfish state — RedfishAllowed flag checked in the walker, flipped by a FEATURE table subscriber. On non-BMC platforms a services=["REDFISH"] template is a no-op (warn + ignore), same as today's "unrecognized service" path. Details in the updated PR description.

(2) Companion buildimage PR: sonic-net/sonic-buildimage#27349 — adds Redfish as a feature and creates the FEATURE.redfish row.

(3) Why a CACL for a BMC service: the BMC runs its own SONiC instance, so caclmgrd and the Redfish docker are co-located on the BMC — caclmgrd filters inbound 443 on the BMC directly, same as it does for SSH/SNMP/NTP.

Nits: added the v6 DROP vector

[yxieca]

Round-2 review on 7fb3607: the cross-platform CACL concern from the prior review is properly addressed via FEATURE.redfish gating (init-time seed + walker skip + subscriber re-walk), with v4/v6 enabled+disabled test coverage and direct handler-branch tests. Nice work.

Reviewed by AI agent on behalf of Ying.