MACSec and IPSec FIPS Compliance by JaiOCP · Pull Request #2162 · opencomputeproject/SAI

JaiOCP · 2025-04-07T21:00:59Z

This PR brings support for running POST on an IPSec and MACSec engine mainly for FIPS compliance

Signed-off-by: JaiOCP <jai.kumar@broadcom.com>

kcudnik · 2025-04-08T11:47:39Z

+    /** Unknown */
+    SAI_MACSEC_POST_STATUS_UNKNOWN,
+
+    /** Pass */
+    SAI_MACSEC_POST_STATUS_PASS,
+
+    /** In Progress */
+    SAI_MACSEC_POST_STATUS_IN_PROGRESS,
+
+    /** Fail */
+    SAI_MACSEC_POST_STATUS_FAIL,


unnecessary comments, not introducing any meaningful information, please remove

judyjoseph · 2025-04-10T00:24:18Z

+FIPS-103 compliance requires that POST is executed on each port and only if POST completes with ‘success’, port must be enabled for admitting traffic, else port will remain in down state.
+
+There are two variations of MACSec/IPSec engine and ports
+1. Each MACSec/IPSec emgine serving ‘n’ number of ports (1::n)


typo "engine"

judyjoseph · 2025-04-10T04:45:55Z

+typedef enum _sai_switch_attr_t
+{
+    /**
+     * @brief Callback for completion status of all the MACSEC ports serviced by this MACSEC engine


does the notification callback gets registered at the sai switch level or at the macsec engine level

skeesara-nokia · 2025-04-10T16:33:21Z

Can we consider support for running POST - where it can be requested at the switch level - preferably also be able to request it as part of switch_create.

The most common use case for systems with FIPS is - they are running in a mode where FIPS is enabled or not enabled. It isn't typical for FIPS compliance to be required on some MACSEC ports and not on others. For this, the application implementation is simplified significantly, if it could request POST at initialization time and proceed on the knowledge of whether that was successful for all MACSEC engines in hardware or not.

If an implementation does not support requesting POST at this global/swich level - it would indicate that the attribute is not supported. In that case the application will need to implement code to request it whenever it creates individual MACSEC ports as described in this spec. That can be a more sophisticated application implementation that does not have be done right away.

kcudnik · 2025-04-11T07:40:23Z

please resolve conflicts

Signed-off-by: JaiOCP <jai.kumar@broadcom.com>

dipankar-nexthop · 2025-04-16T19:22:07Z

+#### 2.1.1 MACSec Engine
+Each MACSec engine is represented by a MACSec SAI object id. Single MACSec object id can be serving one or more MACSec ports.
+
+New attribute is introduced to trigger POST at the MACSec engine granularity.


Typically the entire system is FIPS certified and it requires POST before MACsec or IPsec is enabled. AFAIK, existing systems often use a simple implementation which invokes POST for all security engines in the system at the time of system initialization. For SAI, that would requires only static boolean controls (CREATE_ONLY attributes) SAI_SWITCH_ATTR_MACSEC_POST_ENABLED and SAI_SWITCH_ATTR_IPSEC_POST_ENABLED. With that approach, each ASIC vendor decides when to run POST within their system initialization sequence and removes the responsibility of correctly triggering POST from the NOS. Can we take this approach and remove all SAI attributes that trigger POST (SAI_IPSEC_ATTR_ENABLE_POST, SAI_MACSEC_ATTR_ENABLE_POST, SAI_SWITCH_ATTR_IPSEC_ENABLE_POST, SAI_SWITCH_ATTR_MACSEC_ENABLE_POST)?

This can work as long as there is no assumption that POST completes synchronously. We cannot guarantee that every implementation is able to implement a synchronous POST. We need to retain the notification API that the NOS can hook to, so it can act when POST completes.

Can you please confirm you agree with this @dipankar-nexthop ?

I agree POST is likely not going to complete synchronously and the notification API would be useful.

JaiOCP · 2025-04-17T15:08:28Z

+     * @type bool
+     * @flags READ_ONLY
+     */
+    SAI_SWITCH_ATTR_MACSEC_FIPS_COMPLIANT,


Remove FIPS_COMPLIANT attribute since query can be done ENABLE_POST to find out if switch support MACSEC and/or IPSEC POST

JaiOCP · 2025-04-17T15:23:48Z

+     * @flags CREATE_AND_SET
+     * @default false
+     */
+    SAI_SWITCH_ATTR_IPSEC_ENABLE_POST,


Change the flags CREATE_ONLY so as to suppress runtime change of this attribute

dipankar-nexthop · 2025-04-22T00:35:05Z

+
+FIPS-103 compliance requires that POST is executed on each MACSec/IPSec port and only if POST completes with ‘success’, the MACSec/IPSec port must be enabled for admitting traffic.
+
+There are two variations of MACSec/IPSec engine and ports


After 4/17/25 SAI meeting discussion, I went back and reexamined the definition of SAI_MACsec and SAI_IPsec objects. As per https://github.com/opencomputeproject/SAI/blob/master/doc/macsec-gearbox/SAI_MACsec_API_Proposal-v1.4.docx, each SAI ASIC is is expected to have only 2 instances of SAI_MACSEC - one for Rx (decryption) and another for Tx (encryption). The SAI IPsec documentation is less prescriptive, but AFAIR was based on the same principle. All MACsec/IPsec enabled ports of a SAI_SWITCH are associated to one instance of such objects. That is why there is no attribute for object count or associativity with ports.

Unless there is some important POST feature loss, the simplest would be to adopt this definiton and not increase the number of SAI_MACsec and SAI_IPsec objects. Otherwise we have to make signficant changes to ensure:

Compatibility with existing SAI drivers

Introduce the concept of multiple Security Engine per SAI_SWITCH and allow SAI_MACSEC and SAI_IPSEC instances per Security Engine

Explain additional POST features enabled by allowing multiple Security Engines.

Expose Security Engine count and port binding (typically each port is bound to one Security Engine)

kcudnik · 2025-04-22T04:10:00Z

please resolve conflicts

wumiaont · 2025-04-22T15:46:52Z

+    sai_status_t status = SAI_STATUS_SUCCESS;
+    sai_attr_capability_t post_capability={0}; 
+
+    // Query if MACSec POST is supported at Switch level


I have some concern here about how to trigger POST. As my understanding I need to take SAI_SWITCH_ATTR_MACSEC_ENABLE_POST as one of the attribute and then finally trigger sai_switch_api->create_switch() to create switch. the gSwitchId is the output of create_switch(). Before I pass in the SAI_SWITCH_ATTR_MACSEC_ENABLE_POST I need to check if the switch support POST or not. Which the query needs gSwitchId.
SAI_SWITCH_ATTR_MACSEC_ENABLE_POST will be CREATE_ONLY based on the comments below. Wondering how would I do the query before switch_create().

Perhaps we could unconditionally allow setting SAI_SWITCH_ATTR_MACSEC_ENABLE_POST and the switch could return a POST_NOT_SUPPORTED status?

That would work for us. In this case, also need to support SAI_SWITCH_ATTR_MACSEC(IPSEC)_POST_STATUS_NOTIFY etc as we will pass those attributes in.

We need to handle the case where FIPS module is present and not present. Running POST always will be not honoring the configuration. POST should run based on the NOS configuration.

JaiOCP · 2025-04-24T15:18:19Z

+     * @default false
+     */
+    SAI_MACSEC_ATTR_ENABLE_POST,
+


MACSec and IPSec object level POST enable is mainly for diagnostics purposes.
Add comments in documentation and attribute clarifying this.

kcudnik · 2025-04-25T07:56:07Z

please resolve conflicts

Signed-off-by: JaiOCP <jai.kumar@broadcom.com>

This is the 1st commit message: FIPS Compliance Signed-off-by: JaiOCP <jai.kumar@broadcom.com> FIPS Compliance at Switch and MACSec/IPSec object level Signed-off-by: JaiOCP <jai.kumar@broadcom.com>

…OP_PACKETS (opencomputeproject#2158) 2/ Add eni_trusted_vni entry 3/ Add global_trusted_vni entry

…ld not be created.. (opencomputeproject#2021) Signed-off-by: Kishore Gummadidala <kishorg@google.com> Co-authored-by: Kishore Gummadidala <kishorg@google.com>

Signed-off-by: Tejaswini Chadaga <tchadaga@microsoft.com>

Signed-off-by: kprasanth-marvell <kprasanth@marvell.com>

Signed-off-by: Longyin Huang <longhuan@cisco.com>

dipankar-nexthop · 2025-05-05T21:46:48Z

+FIPS-103 compliance requires that POST is executed on each MACSec/IPSec port and only if POST completes with ‘success’, the MACSec/IPSec port must be enabled for admitting traffic. 
+Note that this specification will provide POST at the MACSec/IPSec single logical instance level or at the switch level.
+
+As shown in Fig-1, there are two physical security engines and are represented by single logical instance in SAI. Enabling of POST happens at the logical intance level and in turn will trigger POST for all the physical instances present in a given hardware.


I find this confusing. There are 2 SAI_MACSEC objects (for ingress & egress) for all security engines. And there can be up to 2 SAI_IPSEC objects (for ingress & egress) for each security engine. Perhaps deleting this paragraph is best.

dipankar-nexthop · 2025-05-05T21:50:51Z

+Following sections describes the attributes for MACSec engine
+
+#### 2.1.1 MACSec Engine
+All physical MACSec engines are represented by a single MACSec SAI object id. Single SAI  MACSec object id can be serving one or more physical MACSec engines and ports.


Actually, all physical MACsec engines, together, are represented by 2 SAI_MACSEC objects(for ingress & egress).

dipankar-nexthop · 2025-05-05T22:02:03Z

+SAI_OBJECT_TYPE_MACSEC and SAI_OBJECT_TYPE_ IPSEC already exists
+SAI_MACSEC_ATTR_SUPPORTED_PORT_LIST and SAI_IPSEC_ATTR_SUPPORTED_PORT_LIST provides the list of MACSec/IPSec ports being service by an instance of MACSEC and IPSEC objects respectively. Each MACSec/IPSec port has a correspoding binding to the physical port SAI_MACSEC_PORT_ATTR_PORT_ID.
+```
+SAI_OBJECT_TYPE_SWITCH -> SAI_SWITCH_ATTR_MACSEC_OBJECT_LIST[x] -> SAI_MACSEC_ATTR_SUPPORTED_PORT_LIST[] -> SAI_MACSEC_PORT_ATTR_PORT_ID


Since the object relationship has not been changed, perhaps there is no need to mention this as enhancement.
While this description is correct, the reader misses one complexity - each of the 2 elements (for ingress & egress) of the MACSEC_OBJECT_LIST have SUPPORTED_PORT_LIST with identical membership.

dipankar-nexthop · 2025-05-05T22:05:41Z

+```
+SAI_OBJECT_TYPE_SWITCH -> SAI_SWITCH_ATTR_MACSEC_OBJECT_LIST[x] -> SAI_MACSEC_ATTR_SUPPORTED_PORT_LIST[] -> SAI_MACSEC_PORT_ATTR_PORT_ID
+
+SAI_OBJECT_TYPE_SWITCH -> SAI_SWITCH_ATTR_IPSEC_OBJECT_LIST[x] -> SAI_IPSEC_ATTR_SUPPORTED_PORT_LIST[] -> SAI_IPSEC_PORT_ATTR_PORT_ID


Similar comment as for MACSEC. However, IPsec can be a little more complex since it allows N security engines to serve N distinct subsets of ports. In that case, there would be be 2*N elements (for ingress and egress) of IPSEC_OBJECT_LIST.

dipankar-nexthop · 2025-05-06T17:07:09Z

+
+#### 2.1.2 MACSec Port
+New READ only attribute SAI_MACSEC_PORT_ATTR_POST_STATUS is introduced to read the status of POST for a given MACSec port.
+This attribute can be read after the POST is completed for the MACSec object hosting this port.


We should point out that this provides POST status separately for ingress and egress and both have to pass POST for a port to be usable for MACsec.

dipankar-nexthop · 2025-05-06T19:21:02Z

+   read(macsec_port_obj->SAI_MACSEC_PORT_ATTR_POST_STATUS)
+```
+**Step 5: **
+Set the POST enable flag in the macsec object to false. This is mainly for the hw where POST can be triggered runtime after the initialization.


This cannot be done for a CREATE_ONLY attribute.

dipankar-nexthop · 2025-05-06T19:25:57Z

    SAI_SWITCH_ATTR_PACKET_TRIM_DSCP_RESOLUTION_MODE,

+    /**
+     * @brief Callback for completion status of all the MACSEC ports serviced by this MACSEC engine


This should be "all MACsec capble ports of the switch", not related to a specific MACsec engine.

dipankar-nexthop · 2025-05-06T19:26:14Z

+    SAI_SWITCH_ATTR_MACSEC_POST_STATUS_NOTIFY,
+
+    /**
+     * @brief Callback for completion status of all the IPSEC ports serviced by this IPSEC engine


This should be "all IPsec capble ports of the switch", not related to a specific IPsec engine.

dipankar-nexthop · 2025-05-06T19:34:55Z

+
+If the engine is servicing a single port then this callback essentially becomes a per port callback.
+
+If the engine is servicing multiple ports then each port needs to be queried for its POST status using the READ only attribute of the port.


Please make this unconditional. A single instance of SAI_MACSEC object is expected to support all MACsec capable ports of a an SAI_SWITCH.

dipankar-nexthop · 2025-05-06T19:35:13Z

+#### 2.1.3 POST Completion Callback
+Single aggregate callback function is provided to return the status of POST status for the entire MACSec engine. 
+
+If the engine is servicing a single port then this callback essentially becomes a per port callback.


This is incorrect. See below.

JaiOCP · 2025-05-07T21:28:29Z

#2167 new PR

JaiOCP added 2 commits April 7, 2025 13:57

MACSec and IPSec FIPS Compliance

cce5032

Signed-off-by: JaiOCP <jai.kumar@broadcom.com>

MACSec and IPSec FIPS Compliance

b2289fd

Signed-off-by: JaiOCP <jai.kumar@broadcom.com>

kcudnik reviewed Apr 8, 2025

View reviewed changes

judyjoseph reviewed Apr 10, 2025

View reviewed changes

FIPS Compliance

3b90604

Signed-off-by: JaiOCP <jai.kumar@broadcom.com>

dipankar-nexthop reviewed Apr 16, 2025

View reviewed changes

JaiOCP commented Apr 17, 2025

View reviewed changes

tjchadaga added the reviewed PR is discussed in SAI Meeting label Apr 17, 2025

dipankar-nexthop reviewed Apr 22, 2025

View reviewed changes

wumiaont reviewed Apr 22, 2025

View reviewed changes

JaiOCP commented Apr 24, 2025

View reviewed changes

JaiOCP and others added 10 commits May 2, 2025 11:49

FIPS Compliance at Switch and MACSec/IPSec object level

468f600

Signed-off-by: JaiOCP <jai.kumar@broadcom.com>

This is a combination of 2 commits.

a2e8119

This is the 1st commit message: FIPS Compliance Signed-off-by: JaiOCP <jai.kumar@broadcom.com> FIPS Compliance at Switch and MACSec/IPSec object level Signed-off-by: JaiOCP <jai.kumar@broadcom.com>

Merge branch 'fips' of https://github.com/JaiOCP/SAI into fips

2589ab5

Merge branch 'master' into fips

b20dc69

[DASH] 1/ Add ENI mode VM/FNIC, counter ENI_TRUSTED_VNI_ENTRY_MISS_DR…

5363a5e

…OP_PACKETS (opencomputeproject#2158) 2/ Add eni_trusted_vni entry 3/ Add global_trusted_vni entry

Add a RIF attribute to specify if the corresponding My MAC entry shou…

d7e06e0

…ld not be created.. (opencomputeproject#2021) Signed-off-by: Kishore Gummadidala <kishorg@google.com> Co-authored-by: Kishore Gummadidala <kishorg@google.com>

Update workflow to use latest Ubuntu (opencomputeproject#2165)

dc21b69

Signed-off-by: Tejaswini Chadaga <tchadaga@microsoft.com>

FDB table size attribute description changes (opencomputeproject#2164)

aa1a4b7

Signed-off-by: kprasanth-marvell <kprasanth@marvell.com>

Add SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION (opencomputeproject#2156)

60f4b75

Signed-off-by: Longyin Huang <longhuan@cisco.com>

Merge branch 'fips' of https://github.com/JaiOCP/SAI into fips

e3c66fc

JaiOCP mentioned this pull request May 2, 2025

MACSec and IPSec FIPS Compliance #2167

Merged

dipankar-nexthop suggested changes May 6, 2025

View reviewed changes

JaiOCP closed this May 7, 2025

JaiOCP deleted the fips branch July 18, 2025 16:01


		FIPS-103 compliance requires that POST is executed on each MACSec/IPSec port and only if POST completes with ‘success’, the MACSec/IPSec port must be enabled for admitting traffic.

		There are two variations of MACSec/IPSec engine and ports


		If the engine is servicing a single port then this callback essentially becomes a per port callback.

		If the engine is servicing multiple ports then each port needs to be queried for its POST status using the READ only attribute of the port.

Conversation

JaiOCP commented Apr 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skeesara-nokia commented Apr 10, 2025

Uh oh!

kcudnik commented Apr 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kcudnik commented Apr 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kcudnik commented Apr 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JaiOCP commented May 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants