[macsec]: Fallback cak and cak rollover - #4827
Open
liamkearney-msft wants to merge 7 commits into
Open
Conversation
MACSEC_PROFILE already carried fallback_cak/fallback_ckn but macsecmgrd never read them. Wire them up so a port can run a standby CA alongside its primary, which wpa_supplicant keeps the port up on whenever the primary CA has no live peer. - configureMACsec() registers the fallback CA with 'macsec_add_mka ckn=<fallback_ckn> cak=<fallback_cak> fallback=1' once the primary network block is enabled. - loadProfile() validates into a local profile and commits it only once it passes, so a rejected update cannot leave key material behind. Both CAKs are decoded up front: decodeKey() is the only check on key length and reports a bad key by throwing, so deferring it to the apply path would retire the old CA before the new key is known to be good. A fallback CKN equal to the primary CKN is rejected; the YANG model rejects it too, this guards direct CONFIG_DB writes. - loadProfile() drives a change onto the ports already running the profile through hotUpdateProfile() instead of restarting their MKA sessions, and returns task_need_retry when that fails. - hotUpdateProfile() reconciles the fallback CA on a live port: add, remove, CKN change and CAK-only change. The participant is always recreated rather than updated in place, because a CA's key is fixed at creation. - MACsecProfile::update() clears the optional fallback fields before parsing them. The stored profile is mutated in place, so an operator HDEL of fallback_cak/fallback_ckn has to clear the old key rather than retain it. - MKASession records the key material actually applied to the port so a hot update diffs against that rather than against the previous profile, and a partially applied update is retried against what the port really has. Adds getMKAParticipants(), findParticipant(), addMKA() and delMKA() over the per-port wpa_supplicant control socket. addMKA() is idempotent and re-adds a CKN that is present holding the other role; delMKA() treats an absent CKN as success. CKNs are compared case-insensitively throughout, because wpa_supplicant reports lower-case hex while CONFIG_DB may hold either case. Single-CA behaviour is unchanged when no fallback is configured. Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
Support changing primary_ckn/primary_cak on a profile already applied to a port, without dropping traffic and without restarting the MKA session. A participant is keyed by CKN, so a new primary key means a new CA and the old one has to be retired. The hitless behaviour comes from the fallback CA already established on the port: wpa_supplicant rides it for as long as the port has no live primary. The rotation therefore retires the old primary first and only then adds the new key, as a real primary rather than staged in the fallback slot. The port holds at most two CAs at any instant and never stages a third. - A rotation is refused when no fallback CA is established. Without a second live CA there is nothing to carry traffic across the swap. This is the daemon-side guard for direct CONFIG_DB writes; the CLI rejects it at command time as well. - A failed delete of the old primary bails out instead of continuing on to reconcile the fallback, which would leave the port with no CA. - When the new primary CKN is the one the fallback was holding, the fallback baseline is cleared as the primary takes it over, so the reconcile that follows does not delete the CA that has just become the primary. - A CAK change with no CKN change cannot be rotated through, because the same CKN cannot hold two CAs. The live CA is left alone and the new key applies on the next wpa_supplicant restart, with a warning. Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
Add tests/mock_tests/macsecmgrd/macsecmgr_ut.cpp, covering the fallback CAK and primary rotation paths end to end through CONFIG_DB and doTask(). The tests drive the manager against a fake wpa_supplicant that models the control interface: macsec_add_mka rejects a duplicate CKN, macsec_mka_list replies with the exact output of ieee802_1x_kay_get_status(), and the primary CA is committed by enable_network from the staged network block. fork(), kill() and waitpid() are interposed so no supplicant is spawned. Covered: enabling with and without a fallback; primary rotation ordering and its refusal when no fallback is established; fallback add, removal, CKN change and CAK-only change; promotion of a fallback to primary; CKN case insensitivity in both the session diff and the participant lookup; and the rejection paths for a colliding fallback CKN, a malformed CAK and an incomplete profile. Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Collaborator
|
/azp run |
20 tasks
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends cfgmgr/macsecmgr to support fallback CAK/CKN configuration and hitless PSK rollover by applying profile changes to an active MACsec MKA session via wpa_cli runtime commands, rather than relying solely on restarts.
Changes:
- Track the key material currently applied per port and implement hot-update logic for primary/fallback CAK/CKN changes (including runtime MKA participant add/delete/list).
- Validate and apply MACsec profile updates more safely (pre-validate before committing and attempt per-port hot updates).
- Add dedicated
macsecmgrdmock unit tests and wire them into the mock test build/run.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/mock_tests/Makefile.am | Adds tests_macsecmgrd target to the mock test suite and build. |
| tests/mock_tests/macsecmgrd/macsecmgr_ut.cpp | New unit tests with a fake wpa_cli/MKA participant model to validate fallback + rollover behavior. |
| cfgmgr/macsecmgr.h | Extends MKASession to track applied key material and declares runtime MKA participant helpers + hot-update API. |
| cfgmgr/macsecmgr.cpp | Implements fallback handling on initial configure and adds runtime participant parsing/management plus hot-update reconciliation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+362
to
+366
| // The following fields are optional. Clear them first: update() mutates the | ||
| // stored profile in place, so an entry that no longer carries them (an | ||
| // operator HDEL of fallback_cak/fallback_ckn) must not retain the old key. | ||
| fallback_cak.clear(); | ||
| fallback_ckn.clear(); |
| { | ||
| SWSS_LOG_ENTER(); | ||
|
|
||
| const auto * present = findParticipant(getMKAParticipants(sock, port_name), ckn); |
Comment on lines
+1129
to
+1136
| if (findParticipant(getMKAParticipants(sock, port_name), ckn) == nullptr) | ||
| { | ||
| SWSS_LOG_NOTICE( | ||
| "MKA participant CKN '%s' not present on port '%s'; nothing to delete", | ||
| ckn.c_str(), | ||
| port_name.c_str()); | ||
| return true; | ||
| } |
A wpa_cli failure quotes the whole command line back in its exception, so logging that reason verbatim wrote the decoded CAK to syslog. decodeKey() likewise embedded the configured key in its own error, which reached the log whenever a malformed CAK was pushed into CONFIG_DB. Report the key length instead of the value, and scrub the CAK out of the wpa_cli reason before logging it. This also lets addMKA() report why a participant could not be added; it previously discarded the reason entirely to avoid the leak, leaving control socket failures impossible to diagnose from syslog alone. Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
The fallback CA was staged over the wpa_supplicant control socket with macsec_add_mka, after the network block had already been enabled. That put a synchronous control round trip on the enable path, after the point where the primary CA starts converging asynchronously, and made a failure there fatal to the whole port. On a 32 port line card this was observed twice: wpa_supplicant stopped answering its control interface for longer than the wpa_cli timeout, the fallback add was treated as a load failure, and the port was rolled back milliseconds after the primary had come up. The port was left bound to its profile in CONFIG_DB with no MACSEC_PORT_TABLE entry. wpa_supplicant accepts mka_cak_fallback and mka_ckn_fallback as network block parameters, so stage the fallback with the rest of the profile and let a single enable_network bring both CAs up. Rotation still needs the control socket, so macsec_add_mka is kept for hot updates only. Setting the fallback is best effort: a port that cannot be given a standby CA stays protected by its primary rather than being torn down. The session only records the fallback when it was actually applied, so a later rotation still sees a port with no standby and refuses, instead of retiring the live primary for a CA that was never installed. Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
enableMACsec returned whatever disableMACsec returned when a profile failed to load. The rollback normally succeeds, so a genuine load failure was reported to doTask as task_success. Report task_failed and keep the rollback's status for the rollback. Note this does not make the port self-heal: doTask erases anything that is not task_need_retry, so the port stays bound in CONFIG_DB with no STATE_DB entry until the next event on it. It does stop a failure from being logged and accounted as a success. Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Contributor
|
@senthil-nexthop , @saksarav-nokia please help review, thx |
getMKAParticipants() returns an empty list both when a port reports no
participants and when it could not be asked, and its two callers read
that as the former. A wpa_cli that times out is exactly the second case,
and it is the case seen in the field: wpa_supplicant is single threaded,
and a supplicant blocked elsewhere holds its control interface for the
full timeout.
delMKA() then reports the participant as already gone and returns
success, so a primary rotation retires nothing, believes it has, and adds
the replacement alongside a CAK that is still live. addMKA() reads the
same empty list as a port with nothing on it and skips the role check.
Return boost::none when the port cannot be queried, and treat a rejected
request the same way, so the two answers are no longer spelled alike.
Both callers now refuse rather than guess, which leaves the rotation
where it started with the old primary still carrying traffic.
addMKA() also held a pointer into the returned vector past the end of the
full expression that created it:
const auto * present = findParticipant(getMKAParticipants(...), ckn);
The temporary is destroyed at the semicolon, so present->fallback read
freed memory. A small allocation usually still holds the old bytes, which
is why this ran; under a different allocator it decides the role of a
participant from whatever replaced it. Hold the list in a named local
that outlives the pointer.
While here, reject a half configured fallback pair. The && short
circuited, so a CKN without a CAK never read the CKN and the fallback was
dropped instead of refused, leaving a port whose later rotation is denied
for having no standby the operator thought they had configured.
Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
Support for fallback cak/ckn (config db feilds already there)
Connect config db cak updates into add / del MKA calls in wpa_cli.
Allows for hot updating cak / ckn fields in macsec profiles,
Why I did it
To support fallback cak/ckn and hitless pre-shared key rollover.
How I verified it
Ran macsec suite + new tests against master
Details if related
Requires other repo changes.
wpa_supplicant: sonic-net/sonic-wpa-supplicant#123
buildimage: sonic-net/sonic-buildimage#29102