config MACsec CLI #1895

Pterosaur · 2021-10-25T14:14:05Z

What I did

Added support for MACsec config

How I did it

Add macsec.py and register macsec command in config/main.py

How to verify it

Add MACsec profile

config macsec profile add [OPTIONS] <profile_name>

Delete MACsec profile

config macsec profile del [OPTIONS] <profile_name>

Add MACsec port

 config macsec port add [OPTIONS] <port_name> <profile_name>

Delete MACsec port

 config macsec port del [OPTIONS] <port_name>

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

admin@sonic:$ sudo config macsec profile add -h
Usage: config macsec profile add [OPTIONS] <profile_name>

  Add MACsec profile

Options:
  --priority <priority>           For Key server election. In 0-255 range with
                                  0 being the highest priority.  [default:
                                  255]
  --cipher_suite <cipher_suite>   The cipher suite for MACsec.  [default: GCM-
                                  AES-128]
  --primary_cak <primary_cak>     Primary Connectivity Association Key.
                                  [required]
  --primary_ckn <primary_cak>     Primary CAK Name.  [required]
  --policy <policy>               MACsec policy. INTEGRITY_ONLY: All traffics,
                                  except EAPOL, will be converted to MACsec
                                  packets without encryption.SECURITY: All
                                  traffics, except EAPOL, will be encrypted by
                                  SecY.  [default: security]
  --enable_replay_protect / --disable_replay_protect
                                  Whether enable replay protect.  [default:
                                  False]
  --replay_window <enable_replay_protect>
                                  Replay window size that is the number of
                                  packets that could be out of order. This
                                  filed works only if ENABLE_REPLAY_PROTECT is
                                  true.  [default: 0]
  --send_sci / --no_send_sci      Whether send SCI.  [default: True]
  --rekey_period <rekey_period>   The period of proactively refresh (Unit
                                  second).  [default: 0]
  -?, -h, --help                  Show this message and exit.

admin@sonic:$ sudo config macsec profile del -h

Usage: config macsec profile del [OPTIONS] <profile_name>

  Delete MACsec profile

Options:
  -h, -?, --help  Show this message and exit.

admin@sonic$ sudo config macsec port add -h
Usage: config macsec port add [OPTIONS] <port_name> <profile_name>

  Add MACsec port

Options:
  -h, -?, --help  Show this message and exit.

admin@sonic$ sudo config macsec port del -h
Usage: config macsec port del [OPTIONS] <port_name>

  Delete MACsec port

Options:
  -h, -?, --help  Show this message and exit.

lgtm-com · 2021-10-25T14:25:31Z

This pull request introduces 1 alert when merging 5eda315 into 8ea834b - view on LGTM.com

new alerts:

1 for Unused import

Signed-off-by: Ze Gan <[email protected]>

Pterosaur · 2021-10-26T08:08:11Z

/azp run

azure-pipelines · 2021-10-26T08:08:28Z

Azure Pipelines successfully started running 1 pipeline(s).

Pterosaur · 2021-10-27T06:56:24Z

/azp run

azure-pipelines · 2021-10-27T06:56:35Z

Azure Pipelines successfully started running 1 pipeline(s).

qbdwlr · 2021-11-05T14:55:01Z

config/macsec.py

+@click.option('--cipher_suite', metavar='<cipher_suite>', required=False, default="GCM-AES-128", show_default=True, type=click.Choice(["GCM-AES-128", "GCM-AES-256", "GCM-AES-XPN-128", "GCM-AES-XPN-256"]), help="The cipher suite for MACsec.")
+@click.option('--primary_cak', metavar='<primary_cak>', required=True, type=str, help="Primary Connectivity Association Key.")
+@click.option('--primary_ckn', metavar='<primary_cak>', required=True, type=str, help="Primary CAK Name.")
+@click.option('--policy', metavar='<policy>', required=False, default="security", show_default=True, type=click.Choice(["integrity_only", "security"]), help="MACsec policy. INTEGRITY_ONLY: All traffics, except EAPOL, will be converted to MACsec packets without encryption.SECURITY: All traffics, except EAPOL, will be encrypted by SecY.")


Minor typos: change traffics to traffic and add space between period and SECURITY.

qbdwlr · 2021-11-05T14:55:31Z

config/macsec.py

+@click.option('--primary_ckn', metavar='<primary_cak>', required=True, type=str, help="Primary CAK Name.")
+@click.option('--policy', metavar='<policy>', required=False, default="security", show_default=True, type=click.Choice(["integrity_only", "security"]), help="MACsec policy. INTEGRITY_ONLY: All traffics, except EAPOL, will be converted to MACsec packets without encryption.SECURITY: All traffics, except EAPOL, will be encrypted by SecY.")
+@click.option('--enable_replay_protect/--disable_replay_protect', metavar='<replay_protect>', required=False, default=False, show_default=True, is_flag=True, help="Whether enable replay protect.")
+@click.option('--replay_window', metavar='<enable_replay_protect>', required=False, default=0, show_default=True, type=click.IntRange(0, 2**32), help="Replay window size that is the number of packets that could be out of order. This filed works only if ENABLE_REPLAY_PROTECT is true.")


Minor typo: change filed to field.

qbdwlr · 2021-11-05T14:56:03Z

config/macsec.py

+    profile_table["send_sci"] = send_sci
+
+    if rekey_period > 0:
+        profile_table["replay_period"] = rekey_period


replay_period should be rekey_period

qbdwlr · 2021-11-05T14:57:43Z

tests/config_macsec_test.py

+    assert "enable_replay_protect" not in profile_table
+    assert "replay_window" not in profile_table
+    assert profile_table["send_sci"] == "1"
+    assert "replay_period" not in profile_table


I don't think there is any field called replay_period? There is only replay_window and rekey_period AFAIK?

qbdwlr · 2021-11-05T14:58:51Z

tests/config_macsec_test.py

+        assert profile_table["send_sci"] == "1"
+    if "no_send_sci" in profile_map:
+        assert profile_table["send_sci"] == "0"
+    if "replay_period" in profile_map:


AFAIK, replay_period should be rekey_period?

qbdwlr · 2021-11-06T13:32:21Z

I have implemented and tested the changes I suggested above... You can find the updated files if you want to copy the changes in #1727

Pterosaur · 2022-05-19T13:58:27Z

MACsec CLI implemented by PR: sonic-net/sonic-buildimage#9390

qbdwlr and others added 2 commits July 26, 2021 12:55

[CLI] macsec support

766aaf9

Merge branch 'macsecCLI' into macsec_cli

5eda315

Update macsec conf

52e8c93

Signed-off-by: Ze Gan <[email protected]>

Pterosaur force-pushed the macsec_cli branch from 4daba68 to 52e8c93 Compare October 25, 2021 17:47

Add unittest

91c7afa

Signed-off-by: Ze Gan <[email protected]>

Pterosaur marked this pull request as ready for review October 25, 2021 17:59

[CLI] MACsec support

c5ed372

qbdwlr reviewed Nov 5, 2021

View reviewed changes

updates to address review comments

2288118

Pterosaur marked this pull request as draft November 7, 2021 15:40

macsecmgrd expects true/false not 1/0 in MACSEC_PROFILE

95aed5a

qbdwlr mentioned this pull request Nov 9, 2021

[CLI] MACsec support #1727

Open

Merge remote-tracking branch 'ann_repo/macsecCLI' into macsec_cli

3f9d9de

Pterosaur closed this May 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config MACsec CLI #1895

config MACsec CLI #1895

Pterosaur commented Oct 25, 2021 •

edited

Loading

lgtm-com bot commented Oct 25, 2021

Pterosaur commented Oct 26, 2021

azure-pipelines bot commented Oct 26, 2021

Pterosaur commented Oct 27, 2021

azure-pipelines bot commented Oct 27, 2021

qbdwlr Nov 5, 2021

qbdwlr Nov 5, 2021

qbdwlr Nov 5, 2021

qbdwlr Nov 5, 2021

qbdwlr Nov 5, 2021

qbdwlr commented Nov 6, 2021

Pterosaur commented May 19, 2022

config MACsec CLI #1895

config MACsec CLI #1895

Conversation

Pterosaur commented Oct 25, 2021 • edited Loading

What I did

How I did it

How to verify it

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

lgtm-com bot commented Oct 25, 2021

Pterosaur commented Oct 26, 2021

azure-pipelines bot commented Oct 26, 2021

Pterosaur commented Oct 27, 2021

azure-pipelines bot commented Oct 27, 2021

qbdwlr Nov 5, 2021

Choose a reason for hiding this comment

qbdwlr Nov 5, 2021

Choose a reason for hiding this comment

qbdwlr Nov 5, 2021

Choose a reason for hiding this comment

qbdwlr Nov 5, 2021

Choose a reason for hiding this comment

qbdwlr Nov 5, 2021

Choose a reason for hiding this comment

qbdwlr commented Nov 6, 2021

Pterosaur commented May 19, 2022

Pterosaur commented Oct 25, 2021 •

edited

Loading