[macsec]: Add config/show commands for MACsec fallback key & MKA

[liamkearney-msft]

Add docker-macsec configuration and show commands for MKA operational state and safe primary/fallback CAK rotation.

Companion design and implementation:

• HLD: [doc] MACsec: add MKA operational state and key rotation HLD SONiC#2545
• Shared STATE_DB constants: [schema]: Add MACsec MKA state table names sonic-swss-common#1251
• MKA state publication and rollover actioning: [macsec]: Fallback cak and cak rollover  sonic-swss#4827
• Frozen WPA control/status dependency: MACsec: make SAK rollover safe across multiple peers sonic-wpa-supplicant#138

Dependency order: sonic-net/sonic-swss-common#1251 → sonic-net/sonic-swss#4827 → this PR. The WPA PR supplies the frozen control/status interface consumed by macsecmgrd.

Work item tracking

• Microsoft ADO (number only):

How I did it

• Added paired fallback CAK/CKN profile options and canonical hex validation.
• Validated the encoded CAK Type-7 salt index as decimal 00..52.
• Added config macsec profile update replacement selected by old CKN.
• Added an all-attached-port, all-or-nothing, namespace-local STATE_DB safety preflight before CONFIG_DB mutation.
• Interpreted the final WPA controlled-port fields explicitly: protected operation is authenticated=false, secured=true; authenticated=true denotes authenticated-only, unprotected controlled-port state.
• Kept STATE_DB health dimensions separate as last_updated, query_status, config_status, and optional redacted config_error; freshness is derived rather than stored.
• Applied a shared 60-second freshness threshold for configuration safety and display, accounting for the collector's 20-second namespace sweep and bounded per-port queries.
• Added show macsec --mka [interface] with multi-ASIC aggregation, natural interface ordering, namespace disambiguation, and strict secret-safe field allowlisting.
• Compact output presents Status and Age: age comes only from the last successful snapshot, while status preserves independent query, staleness, and configuration health.
• Detailed output derives one strict Controlled port mode (secured, authenticated-only, failed, inactive, inconsistent, or unknown) from the raw WPA state while retaining separate query/config/error and last-updated diagnostics.
• Preserved existing profile, dump, POST, FIPS, and non-MKA show behavior.

How to verify it

• Focused hermetic Click/pytest suite: 27 passed.
• python3 -m py_compile passed on all four changed Python files.
• git diff --check passed.

Coverage includes fallback validation, canonical CKN/CAK and salt-index rejection, duplicate CKN rejection, unattached and attached primary/fallback profile updates, the complete controlled-port preflight state matrix, atomic preflight failures, the 60-second config/show freshness boundary, independent query/config/stale status combinations, never-success age, strict malformed-state rendering, the detailed controlled-port mode state matrix and valid pre-election zero SCI rendering, compact/detail display, natural interface ordering, duplicate interface names across namespaces, namespace aggregation, and secret redaction.

Which release branch to backport (provide reason below if selected)

• 202305
• 202311
• 202405
• 202411
• 202505
• 202511
• 202512
• 202605
• 202608

Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO):
Failure type:

Tested branch

• master
• 202305
• 202311
• 202405
• 202411
• 202505
• 202511
• 202512
• 202605
• 202608
• N/A

Test result

master: focused docker-macsec CLI tests, 27 passed; Python compilation and diff checks passed. Physical image validation is tracked separately.

Description for the changelog

Add safe MACsec primary/fallback CAK rotation commands and namespace-aware MKA operational-state display.

Link to config_db schema for YANG module changes

N/A — this PR consumes the existing MACsec profile schema and adds CLI behavior.

A picture of a cute animal (not mandatory but encouraged)

[mssonicbld]

/azp run Azure.sonic-buildimage

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Adds CLI support in the MACsec config plugin to configure a fallback CAK/CKN when creating MACsec profiles, and to rotate (update) a profile’s primary or fallback key at runtime to support hitless CAK rotation workflows.

Changes:

• Add --fallback_cak/--fallback_ckn options to config macsec profile add, including validation and persistence in MACSEC_PROFILE.
• Introduce config macsec profile update to rotate either the primary or fallback key selected by --old_ckn.
• Extend CLI plugin tests to cover fallback configuration and key-rotation behaviors (including invalid input cases).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
dockers/docker-macsec/cli/config/plugins/macsec.py	Adds fallback CAK/CKN support, shared key validation helpers, and a new profile update command for key rotation.
dockers/docker-macsec/cli-plugin-tests/test_config_macsec.py	Adds test coverage for fallback key configuration and runtime key rotation, including negative test cases.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[mssonicbld]

/azp run Azure.sonic-buildimage

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🟢 Approval recommended

The feature implementation is coherent and has substantial test coverage; remaining feedback is limited to improving the specificity of newly-generic CLI error messages.

Review details

Suppressed comments (2)

dockers/docker-macsec/cli/config/plugins/macsec.py:128

• validate_cak() now emits generic failure messages ("Expect the CAK is valid hex string" / "Expect the length of CAK...") without indicating which CLI argument was invalid (primary_cak vs fallback_cak vs new_cak), which makes troubleshooting harder—especially now that multiple CAKs can be provided/rotated. Consider including the failing field name in the error (or passing it into validate_cak) so the CLI points to the exact option that needs fixing.

def validate_cak(ctx, cipher_suite, cak):
    length = expected_cak_length(cipher_suite)
    if length is not None and len(cak) != length:
        ctx.fail("Expect the length of CAK is {}, but got {}".format(length, len(cak)))
    if not is_hexstring(cak):
        ctx.fail("Expect the CAK is valid hex string")

dockers/docker-macsec/cli/config/plugins/macsec.py:134

• validate_ckn() now reports a generic "Expect the CKN is valid hex string" error, which doesn’t identify whether the invalid value came from --primary_ckn, --fallback_ckn, --old_ckn, or --new_ckn. Since the new 'update' flow introduces multiple CKN inputs, it would be clearer to include the specific argument name in the failure message (or pass a label into validate_ckn).

def validate_ckn(ctx, ckn):
    if not is_hexstring(ckn):
        ctx.fail("Expect the CKN is valid hex string")

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[saravanan-nexthop]

cc @senthil-nexthop for review

[mssonicbld]

/azp run Azure.sonic-buildimage

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🟡 Changes recommended

Unresolved runtime failure and validation and multi-ASIC display issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 4
• Review effort level: Lite

[mssonicbld]

/azp run Azure.sonic-buildimage

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🔵 Needs a closer look

Add fallback-path rotation tests and correct missing-value duration formatting.

Review details

Suppressed comments (2)

dockers/docker-macsec/cli/config/plugins/macsec.py:438

• The attached-port tests cover only the selected_role == "primary" path. This fallback branch has different safety semantics (the configured primary must be the live alternate), so add an attached-profile test that rotates old_ckn from the fallback and verifies both the successful case and a failed-primary-alternate preflight leave CONFIG_DB unchanged.

    elif normalized_old == normalized_fallback:
        selected_role = "fallback"
        other_ckn = normalized_primary

dockers/docker-macsec/cli/show/plugins/show_macsec.py:675

• _safe_uint returns - for a missing or malformed field, but this formatting appends ms unconditionally, so an incomplete session is rendered as - ms instead of the required missing-value marker -. Format the unit only when the sanitized value is valid.

                ("MKA hello time", "{} ms".format(
                    _safe_uint(session.get("mka_hello_time_ms"))
                )),

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mssonicbld]

/azp run Azure.sonic-buildimage

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🔵 Needs a closer look

Unresolved review findings remain in MKA rendering and attached-profile fallback-rotation coverage.

Review details

Suppressed comments (4)

dockers/docker-macsec/cli/config/plugins/macsec.py:437

• The attached-profile tests cover only the selected_role == "primary" path. The fallback-rotation branch here uses the opposite role predicate and must prove that the configured primary is active, has is_primary=true, and has a live peer before CONFIG_DB is changed; add an attached-profile test that rotates the fallback and asserts both the success case and rejection when that primary alternate is unsafe.

    elif normalized_old == normalized_fallback:
        selected_role = "fallback"

dockers/docker-macsec/cli/show/plugins/show_macsec.py:127

• int(value, 16) is not a strict hexadecimal check: it accepts signs and surrounding whitespace, so malformed values such as a fixed-width +... SCI/CKN are rendered instead of being mapped to -. Use an ASCII hex-character check here, as is_hexstring() does in the config plugin, before lowercasing the value.

    try:
        int(value, 16)
        return value.lower()
    except ValueError:
        return "-"

dockers/docker-macsec/cli/show/plugins/show_macsec.py:80

• When last_updated parses but query_status is missing or an unexpected value, this path falls through and reports a plain age such as 2s; only ok or the explicitly retained error state should produce a freshness age. That makes malformed state look fresh even though the query column renders -; return a conservative never/unknown freshness for unrecognized statuses.

def _freshness(last_updated, query_status, now=None):
    parsed = _parse_utc_timestamp(last_updated)
    if parsed is None:
        return "never", "never"

dockers/docker-macsec/cli/show/plugins/show_macsec.py:675

• A missing or invalid mka_hello_time_ms is rendered as - ms because the unit is appended unconditionally. The MKA detail contract uses - for missing fields, so this output incorrectly suggests a value with a unit; append ms only when _safe_uint returns a real counter.

                ("MKA hello time", "{} ms".format(
                    _safe_uint(session.get("mka_hello_time_ms"))
                )),

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[mssonicbld]

/azp run Azure.sonic-buildimage

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🟡 Changes recommended

Align freshness thresholds with the HLD and preserve documented MKA status distinctions.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 3
• Review effort level: Lite

[mssonicbld]

/azp run Azure.sonic-buildimage

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🟡 Changes recommended

The MKA display has one moderate correctness issue and one labeling issue to address.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

dockers/docker-macsec/cli/show/plugins/show_macsec.py:174

• key_server_sci=0000000000000000 is a valid normalized value before key-server election, but this helper converts it to -. That makes show macsec --mka lose the distinction between a legitimate pre-election state and an invalid/missing SCI; the allowlisted 16-hex value should be displayed as-is.

    if sci == "0000000000000000":
        return "-"

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[mssonicbld]

/azp run Azure.sonic-buildimage

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Copilot]

🟡 Changes recommended

Unresolved critical secret-redaction and moderate freshness-preflight issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

dockers/docker-macsec/cli/config/plugins/macsec.py:281

• now is captured once before iterating all attached ports, so a port checked late in a large/slow preflight can cross the 60-second freshness limit while still being accepted, and the subsequent CONFIG_DB write proceeds with stale safety data. Compute the reference time per port (as close as possible to that port's validation) rather than reusing the timestamp from the start of the whole sweep.

    now = datetime.datetime.now(datetime.timezone.utc)

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite