ISS-3102 FIPS POST validation for MACsec control plane crypto.#3907
ISS-3102 FIPS POST validation for MACsec control plane crypto.#3907vikram-nexthop wants to merge 16 commits intosonic-net:masterfrom
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…failure. - update FIPS_MACSEC_POST_TABLE even if "wpa_supplicant -F" command fails due to timeout.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…ck to confirm FIPS mode.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
gentle reminder to complete the review. |
|
@vikram-nexthop we already stop the configs in macsecmgrd in PR : #3836 , macsecmgrd. Please share more details on this PR |
|
@judyjoseph PR #3836 handles the data plane POST (SAI/hardware crypto engines), while this PR addresses control plane POST (wpa_supplicant crypto module for MKA protocol). Both are separate cryptographic modules and therefore require independent FIPS validation. Complete MACsec FIPS compliance requires both POST validations to pass. So, both POST validations must pass before enabling MACsec configuration processing. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| SWSS_LOG_NOTICE("starting main loop"); | ||
| while (!received_sigterm) | ||
| { | ||
| if (!isCpPostStateReady) |
There was a problem hiding this comment.
Not seeing anyone updating this variable isCpPostStateReady before checking ? This API getCpCryptoFipsPostStatus() is just called once I see ?
There was a problem hiding this comment.
The getCpCryptoFipsPostStatus() fetches the control plane POST status synchronously, and therefore invoked only once.
| std::string state = getMacsecPostState(&stateDb); | ||
| if (state == "pass" || state == "disabled") | ||
| { | ||
| SWSS_LOG_NOTICE("FIPS MACSec POST ready: state %s", state.c_str()); |
There was a problem hiding this comment.
Can we do this check isCpPostStateReady() here. i.e check CpPost if dataplanePost is pass ? that way we need not check for fipsEnabled explicitly
We have removed the file open and check for fips here in orcahgent/main.cpp and replaced with an option "https://github.com/nexthop-ai/sonic-swss/blob/a3df4d5452fa30e52ac8205ae8b95d5bf3cf7b84/orchagent/main.cpp#L107"
There was a problem hiding this comment.
ok, will look into this.
There was a problem hiding this comment.
Can we do this check isCpPostStateReady() here. i.e check CpPost if dataplanePost is pass ? that way we need not check for fipsEnabled explicitly
We have removed the file open and check for fips here in orcahgent/main.cpp and replaced with an option "https://github.com/nexthop-ai/sonic-swss/blob/a3df4d5452fa30e52ac8205ae8b95d5bf3cf7b84/orchagent/main.cpp#L107"
I have the same feeling that we may be able to do those FIPS ready check in macsecmgrd. In this case there's no need to make code change into wpa_supplicant which is a third party open source.
There was a problem hiding this comment.
I think @judyjoseph's comment is to invoke isCpPostStateReady() after SAI/switch post ststus becomes available (within the if statement, line no 227).
Regarding you suggestion to move FIPS ready check to macsecmgrd, I have some concerns. The crypto libraries (OpenSSL,etc) are linked to the wpa_supplicant. Macsecmgrd doesn't use any crypto libraries as such.
The migration would require the following changes, as per my understanding,
- change macsecmgrd to link/include all the crypto libraries.
- since the wpa_supplicant supports multiple backend crypto libraries, keep track of the list of supported libraries and ensure that macsecmgrd validates the same library that is used by the wpa_supplicant.
The decision to make changes in the wpa_supplicant (which is directly connected to the crypto module) was based on these factors.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
closing this PR, since control plane POST check is not required. Refer to sonic-net/SONiC#2083 for more details. |
4 participants
What I did
Why I did it
MACsec control plane FIPS POST validation is required to ensure the cryptographic backend has passed self-tests before enabling MACsec operations. This is to ensure MACsec control plane fails securely if crypto backend POST validation fails in FIPS environments.
How I verified it
Enabled MACsec service with and without SymCrypt FIPS provider.
Details if related
related PR: sonic-net/sonic-wpa-supplicant#99