From c5ed3728825e54e188558744b5685fa957eff3e0 Mon Sep 17 00:00:00 2001 From: Ann Pokora Date: Mon, 1 Nov 2021 17:19:45 -0700 Subject: [PATCH 1/4] [CLI] MACsec support --- config/macsec.py | 168 ++++++++++++++++++++++++++ config/main.py | 2 + scripts/macsecshow | 233 ++++++++++++++++++++++++++++++++++++ setup.py | 1 + show/macsec.py | 44 +++++++ show/main.py | 2 + tests/config_macsec_test.py | 130 ++++++++++++++++++++ 7 files changed, 580 insertions(+) create mode 100644 config/macsec.py create mode 100755 scripts/macsecshow create mode 100644 show/macsec.py create mode 100644 tests/config_macsec_test.py diff --git a/config/macsec.py b/config/macsec.py new file mode 100644 index 0000000000..4401668516 --- /dev/null +++ b/config/macsec.py @@ -0,0 +1,168 @@ +import click +import utilities_common.cli as clicommon + +# +# 'macsec' group ('config macsec ...') +# +@click.group(cls=clicommon.AbbreviationGroup, name='macsec') +def macsec(): + """MACsec-related configuration tasks""" + pass + + +# +# 'port' group ('config macsec port ...') +# +@macsec.group(cls=clicommon.AbbreviationGroup, name='port') +def macsec_port(): + """Enable MACsec or disable MACsec on the specified port""" + pass + +# +# 'add' command ('config macsec port add ...') +# +@macsec_port.command('add') +@click.argument('port', metavar='', required=True) +@click.argument('profile', metavar='', required=True) +@clicommon.pass_db +def add_port(db, port, profile): + """ + Add MACsec port + """ + ctx = click.get_current_context() + + if clicommon.get_interface_naming_mode() == "alias": + alias = port + iface_alias_converter = clicommon.InterfaceAliasConverter(db) + port = iface_alias_converter.alias_to_name(alias) + if port is None: + ctx.fail("cannot find port name for alias {}".format(alias)) + + profile_entry = db.cfgdb.get_entry('MACSEC_PROFILE', profile) + if len(profile_entry) == 0: + ctx.fail("profile {} doesn't exist".format(profile)) + + db.cfgdb.set_entry("PORT", port, {'macsec': profile}) + + +# +# 'del' command ('config port del ...') +# +@macsec_port.command('del') +@click.argument('port', metavar='', required=True) +@clicommon.pass_db +def del_port(db, port): + """ + Delete MACsec port + """ + ctx = click.get_current_context() + + if clicommon.get_interface_naming_mode() == "alias": + alias = port + iface_alias_converter = clicommon.InterfaceAliasConverter(db) + port = iface_alias_converter.alias_to_name(alias) + if port is None: + ctx.fail("cannot find port name for alias {}".format(alias)) + + db.cfgdb.set_entry("PORT", port, {'macsec': ""}) + + +# +# 'profile' group ('config macsec profile ...') +# +@macsec.group(cls=clicommon.AbbreviationGroup, name='profile') +def macsec_profile(): + pass + + +def is_hexstring(hexstring: str): + try: + int(hexstring, 16) + return True + except ValueError: + return False + + +# +# 'add' command ('config macsec profile add ...') +# +@macsec_profile.command('add') +@click.argument('profile', metavar='', required=True) +@click.option('--priority', metavar='', required=False, default=255, show_default=True, type=click.IntRange(0, 255), help="For Key server election. In 0-255 range with 0 being the highest priority.") +@click.option('--cipher_suite', metavar='', 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='', required=True, type=str, help="Primary Connectivity Association Key.") +@click.option('--primary_ckn', metavar='', required=True, type=str, help="Primary CAK Name.") +@click.option('--policy', metavar='', 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='', required=False, default=False, show_default=True, is_flag=True, help="Whether enable replay protect.") +@click.option('--replay_window', metavar='', 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.") +@click.option('--send_sci/--no_send_sci', metavar='', required=False, default=True, show_default=True, is_flag=True, help="Whether send SCI.") +@click.option('--rekey_period', metavar='', required=False, default=0, show_default=True, type=click.IntRange(min=0), help="The period of proactively refresh (Unit second).") +@clicommon.pass_db +def add_profile(db, profile, priority, cipher_suite, primary_cak, primary_ckn, policy, enable_replay_protect, replay_window, send_sci, rekey_period): + """ + Add MACsec profile + """ + ctx = click.get_current_context() + + profile_entry = db.cfgdb.get_entry('MACSEC_PROFILE', profile) + if not len(profile_entry) == 0: + ctx.fail("{} already exists".format(profile)) + + profile_table = {} + + profile_table["priority"] = priority + + profile_table["cipher_suite"] = cipher_suite + + if "128" in cipher_suite: + if len(primary_cak) != 32: + ctx.fail("Expect the length of CAK is 32, but got {}".format(len(primary_cak))) + elif "256" in cipher_suite: + if len(primary_cak) != 64: + ctx.fail("Expect the length of CAK is 64, but got {}".format(len(primary_cak))) + if not is_hexstring(primary_cak): + ctx.fail("Expect the primary_cak is valid hex string") + if not is_hexstring(primary_ckn): + ctx.fail("Expect the primary_ckn is valid hex string") + profile_table["primary_cak"] = primary_cak + profile_table["primary_ckn"] = primary_ckn + + profile_table["policy"] = policy + + if enable_replay_protect and replay_window > 0: + profile_table["enable_replay_protect"] = enable_replay_protect + profile_table["replay_window"] = replay_window + + profile_table["send_sci"] = send_sci + + if rekey_period > 0: + profile_table["replay_period"] = rekey_period + + for k, v in profile_table.items(): + if isinstance(v, bool): + if v: + profile_table[k] = "1" + else: + profile_table[k] = "0" + else: + profile_table[k] = str(v) + db.cfgdb.set_entry("MACSEC_PROFILE", profile, profile_table) + + +# +# 'del' command ('config macsec profile del ...') +# +@macsec_profile.command('del') +@click.argument('profile', metavar='', required=True) +@clicommon.pass_db +def del_profile(db, profile): + """ + Delete MACsec profile + """ + ctx = click.get_current_context() + + profile_entry = db.cfgdb.get_entry('MACSEC_PROFILE', profile) + if len(profile_entry) == 0: + ctx.fail("{} doesn't exist".format(profile)) + + db.cfgdb.set_entry("MACSEC_PROFILE", profile, None) diff --git a/config/main.py b/config/main.py index 20d5c1c290..2c1ea37bd8 100644 --- a/config/main.py +++ b/config/main.py @@ -37,6 +37,7 @@ from . import feature from . import kdump from . import kube +from . import macsec from . import muxcable from . import nat from . import vlan @@ -1033,6 +1034,7 @@ def config(ctx): config.add_command(feature.feature) config.add_command(kdump.kdump) config.add_command(kube.kubernetes) +config.add_command(macsec.macsec) config.add_command(muxcable.muxcable) config.add_command(nat.nat) config.add_command(vlan.vlan) diff --git a/scripts/macsecshow b/scripts/macsecshow new file mode 100755 index 0000000000..b211915b4c --- /dev/null +++ b/scripts/macsecshow @@ -0,0 +1,233 @@ +#!/usr/bin/env python3 + +##################################################################### +# +# macsecshow is a tool for summarizing MACsec details. +# +##################################################################### + +import json +import os +import re +import swsssdk +import subprocess +import sys + +import click +from collections import namedtuple +from utilities_common.netstat import STATUS_NA + +MACSEC_PORT_TABLE_PREFIX = "MACSEC_PORT_TABLE:" + +COUNTER_TABLE_PREFIX = "COUNTERS:" +COUNTERS_MACSEC_FLOW_TX_NAME_MAP = "COUNTERS_MACSEC_FLOW_TX_NAME_MAP" +COUNTERS_MACSEC_FLOW_RX_NAME_MAP = "COUNTERS_MACSEC_FLOW_RX_NAME_MAP" +COUNTERS_MACSEC_SA_TX_NAME_MAP = "COUNTERS_MACSEC_SA_TX_NAME_MAP" +COUNTERS_MACSEC_SA_RX_NAME_MAP = "COUNTERS_MACSEC_SA_RX_NAME_MAP" + +FlowStats = namedtuple("FlowStats", "ctl, untagged, tx_too_long, rx_no_tag,\ + rx_bad_tag, rx_no_sci, rx_unk_sci, rx_overrun,") + +flow_stat_names = ( + 'SAI_MACSEC_FLOW_STAT_CONTROL_PKTS', + 'SAI_MACSEC_FLOW_STAT_PKTS_UNTAGGED', + 'SAI_MACSEC_FLOW_STAT_OUT_PKTS_TOO_LONG', + 'SAI_MACSEC_FLOW_STAT_IN_PKTS_NO_TAG', + 'SAI_MACSEC_FLOW_STAT_IN_PKTS_BAD_TAG', + 'SAI_MACSEC_FLOW_STAT_IN_PKTS_NO_SCI', + 'SAI_MACSEC_FLOW_STAT_IN_PKTS_UNKNOWN_SCI', + 'SAI_MACSEC_FLOW_STAT_IN_PKTS_OVERRUN' +) + +SaStats = namedtuple("SaStats", "b_enc, b_prot, tx_p_enc, tx_p_prot, rx_p_ok,\ + rx_p_unchecked, rx_p_delayed, rx_p_late, rx_p_invalid,\ + rx_p_not_valid, rx_p_not_using_sa, rx_p_unused_sa") + +sa_stat_names = ( + 'SAI_MACSEC_SA_STAT_OCTETS_ENCRYPTED', + 'SAI_MACSEC_SA_STAT_OCTETS_PROTECTED', + 'SAI_MACSEC_SA_STAT_OUT_PKTS_ENCRYPTED', + 'SAI_MACSEC_SA_STAT_OUT_PKTS_PROTECTED', + 'SAI_MACSEC_SA_STAT_IN_PKTS_OK', + 'SAI_MACSEC_SA_STAT_IN_PKTS_UNCHECKED', + 'SAI_MACSEC_SA_STAT_IN_PKTS_DELAYED', + 'SAI_MACSEC_SA_STAT_IN_PKTS_LATE', + 'SAI_MACSEC_SA_STAT_IN_PKTS_INVALID', + 'SAI_MACSEC_SA_STAT_IN_PKTS_NOT_VALID', + 'SAI_MACSEC_SA_STAT_IN_PKTS_NOT_USING_SA', + 'SAI_MACSEC_SA_STAT_IN_PKTS_UNUSED_SA' +) + +def get_macsec_port_list(appl_db, intf_name): + """ + Get the macsec port names from database. + """ + if intf_name is None: + port_keys = appl_db.keys(appl_db.APPL_DB, MACSEC_PORT_TABLE_PREFIX+"*") + else: + port_keys = appl_db.keys(appl_db.APPL_DB, MACSEC_PORT_TABLE_PREFIX+"%s" % intf_name) + if port_keys is None: + return None + macsec_ports = [key[len(MACSEC_PORT_TABLE_PREFIX):] for key in port_keys] + return macsec_ports + +class MacsecConn(object): + def display_connections(self, intf_name): + """ + Get MACsec info from wpa_cli. + """ + macsec_cmd = 'docker exec -it macsec wpa_cli -g /var/run/' + intf_name + ' IFNAME=' + intf_name + ' macsec' + click.echo("Interface: {} ".format(intf_name)) + p = subprocess.Popen(macsec_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) + (output, err) = p.communicate() + rc = p.wait() + if rc == 0: + click.echo(output) + else: + click.echo("Error retrieving MACsec connections: {} ".format(err)) + + def __init__(self, intf_name): + # setup db connection + self.db = swsssdk.SonicV2Connector(host='127.0.0.1') + self.db.connect(self.db.APPL_DB) + self.ports = get_macsec_port_list(self.db, intf_name) + if self.ports is None: + return + for port in self.ports: + self.display_connections(port) + +class MacsecStat(object): + def get_flow_stats(self, table_id): + """ + Get the stats from specific MACsec Flow. + """ + fields = [STATUS_NA] * len(flow_stat_names) + for pos, stat_name in enumerate(flow_stat_names): + full_table_id = COUNTER_TABLE_PREFIX + table_id + stat_data = self.db.get(self.db.COUNTERS_DB, full_table_id, stat_name) + if stat_data: + fields[pos] = str(stat_data) + + flow_stats = FlowStats._make(fields) + return flow_stats + + def get_sa_stats(self, table_id): + """ + Get the stats from specific MACsec SA. + """ + fields = ['0'] * len(sa_stat_names) + for pos, stat_name in enumerate(sa_stat_names): + full_table_id = COUNTER_TABLE_PREFIX + table_id + stat_data = self.db.get(self.db.COUNTERS_DB, full_table_id, stat_name) + if stat_data: + fields[pos] = str(stat_data) + + sa_stats = SaStats._make(fields) + return sa_stats + + def get_sa_id(self, port, sa_map): + """ + Get the MACsec SA from the port. + """ + for sa_name in sa_map: + if port == sa_name.split(':')[0]: + return sa_map[sa_name] + + return None + + def display_txsa_statistics(self, port): + """ + Get MACsec stats from COUNTERS_DB. + """ + txsa_id = self.get_sa_id(port, self.txsa_map) + + body = """ TX: + Encrypted packets: %10s + Encrypted bytes: %10s + Protected packets: %10s + Protected bytes: %10s""" + + if txsa_id: + txsa_stats = self.get_sa_stats(txsa_id) + body = body % (txsa_stats.tx_p_enc, txsa_stats.b_enc, + txsa_stats.tx_p_prot, txsa_stats.b_prot) + else: + body = body % ('0', '0', '0', '0') + + click.echo(body) + + def display_rxsa_statistics(self, port): + """ + Get MACsec stats from COUNTERS_DB. + """ + rxsa_id = self.get_sa_id(port, self.rxsa_map) + + body = """ RX: + OK packets: %10s + Encrypted bytes: %10s + Protected bytes: %10s + Unchecked packets: %10s + Delayed packets: %10s + Late packets: %10s + Invalid packets: %10s + Not valid packets: %10s + Not using SA packets: %10s + Unused SA packets: %10s""" + + if rxsa_id: + rxsa_stats = self.get_sa_stats(rxsa_id) + body = body % (rxsa_stats.rx_p_ok, rxsa_stats.b_enc, rxsa_stats.b_prot, + rxsa_stats.rx_p_unchecked, rxsa_stats.rx_p_delayed, + rxsa_stats.rx_p_late, rxsa_stats.rx_p_invalid, + rxsa_stats.rx_p_not_valid, rxsa_stats.rx_p_not_using_sa, + rxsa_stats.rx_p_unused_sa) + else: + body = body % ('0', '0', '0', '0', '0', + '0', '0', '0', '0', '0') + + click.echo(body) + + def __init__(self, intf_name): + # setup db connection + self.db = swsssdk.SonicV2Connector(host='127.0.0.1') + self.db.connect(self.db.APPL_DB) + self.db.connect(self.db.COUNTERS_DB) + self.ports = get_macsec_port_list(self.db, intf_name) + if self.ports is None: + return + + self.txflow_map = self.db.get_all(self.db.COUNTERS_DB, COUNTERS_MACSEC_FLOW_TX_NAME_MAP); + self.rxflow_map = self.db.get_all(self.db.COUNTERS_DB, COUNTERS_MACSEC_FLOW_RX_NAME_MAP); + self.txsa_map = self.db.get_all(self.db.COUNTERS_DB, COUNTERS_MACSEC_SA_TX_NAME_MAP); + self.rxsa_map = self.db.get_all(self.db.COUNTERS_DB, COUNTERS_MACSEC_SA_RX_NAME_MAP); + + for port in self.ports: + click.echo("Interface: {} ".format(port)) + self.display_txsa_statistics(port) + self.display_rxsa_statistics(port) + click.echo("") + +def main(args): + if os.geteuid() != 0: + exit("This utility must be run as root") + + if len(args) == 0: + click.echo("No valid arguments provided") + return + + command = args[0] + if command != "connections" and command != "statistics": + click.echo("No valid command provided") + return + + intf_name = args[1] if len(args) == 2 else None + + if command == "connections": + macsec_conn = MacsecConn(intf_name) + elif command == "statistics": + macsec_stat = MacsecStat(intf_name) + + sys.exit(0) + +if __name__ == "__main__": + main(sys.argv[1:]) diff --git a/setup.py b/setup.py index 30e8c7bfd4..0976406abf 100644 --- a/setup.py +++ b/setup.py @@ -104,6 +104,7 @@ 'scripts/ipintutil', 'scripts/lldpshow', 'scripts/log_ssd_health', + 'scripts/macsecshow', 'scripts/mellanox_buffer_migrator.py', 'scripts/mmuconfig', 'scripts/natclear', diff --git a/show/macsec.py b/show/macsec.py new file mode 100644 index 0000000000..adfb5be197 --- /dev/null +++ b/show/macsec.py @@ -0,0 +1,44 @@ +import click +import utilities_common.cli as clicommon + +# +# 'macsec' group ("show macsec ...") +# +@click.group(cls=clicommon.AliasedGroup) +def macsec(): + """Show MACsec information""" + pass + +@macsec.command() +@click.argument('interfacename', required=False) +@click.option('--verbose', is_flag=True, help="Enable verbose output") +def connections(interfacename, verbose): + """Show MACsec connections""" + + cmd = "macsecshow connections" + + if interfacename is not None: + if clicommon.get_interface_naming_mode() == "alias": + iface_alias_converter = clicommon.InterfaceAliasConverter(db) + interfacename = iface_alias_converter.alias_to_name(interfacename) + + cmd += " {}".format(interfacename) + + clicommon.run_command(cmd, display_cmd=verbose) + +@macsec.command() +@click.argument('interfacename', required=False) +@click.option('--verbose', is_flag=True, help="Enable verbose output") +def statistics(interfacename, verbose): + """Show MACsec statistics""" + + cmd = "macsecshow statistics" + + if interfacename is not None: + if clicommon.get_interface_naming_mode() == "alias": + iface_alias_converter = clicommon.InterfaceAliasConverter(db) + interfacename = iface_alias_converter.alias_to_name(interfacename) + + cmd += " {}".format(interfacename) + + clicommon.run_command(cmd, display_cmd=verbose) diff --git a/show/main.py b/show/main.py index f4998218f2..8eb5050eb4 100755 --- a/show/main.py +++ b/show/main.py @@ -44,6 +44,7 @@ from . import interfaces from . import kdump from . import kube +from . import macsec from . import muxcable from . import nat from . import platform @@ -182,6 +183,7 @@ def cli(ctx): cli.add_command(interfaces.interfaces) cli.add_command(kdump.kdump) cli.add_command(kube.kubernetes) +cli.add_command(macsec.macsec) cli.add_command(muxcable.muxcable) cli.add_command(nat.nat) cli.add_command(platform.platform) diff --git a/tests/config_macsec_test.py b/tests/config_macsec_test.py new file mode 100644 index 0000000000..f78608607e --- /dev/null +++ b/tests/config_macsec_test.py @@ -0,0 +1,130 @@ +from unittest import mock + +from utilities_common.db import Db + +from click.testing import CliRunner +import config.main as config + + +def test_macsec_default_profile(): + runner = CliRunner() + db = Db() + + profile_name = "test" + primary_cak = "01234567890123456789012345678912" + primary_ckn = "01234567890123456789012345678912" + result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["add"], [profile_name, "--primary_cak=" + primary_cak,"--primary_ckn=" + primary_ckn], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + profile_table = db.cfgdb.get_entry("MACSEC_PROFILE", profile_name) + assert profile_table + assert profile_table["priority"] == "255" + assert profile_table["cipher_suite"] == "GCM-AES-128" + assert profile_table["primary_cak"] == primary_cak + assert profile_table["primary_ckn"] == primary_ckn + assert profile_table["policy"] == "security" + 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 + assert "rekey_period" not in profile_table + + result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["del"], [profile_name], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + profile_table = db.cfgdb.get_entry("MACSEC_PROFILE", profile_name) + assert not profile_table + + +def test_macsec_valid_profile(): + runner = CliRunner() + db = Db() + + profile_name = "test" + profile_map = { + "primary_cak": "0123456789012345678901234567891201234567890123456789012345678912", + "primary_ckn": "01234567890123456789012345678912", + "priority": 64, + "cipher_suite": "GCM-AES-XPN-256", + "policy": "integrity_only", + "enable_replay_protect": None, + "replay_window": 100, + "no_send_sci": None, + "rekey_period": 30 * 60, + } + options = [profile_name] + for k, v in profile_map.items(): + options.append("--" + k) + if v is not None: + options[-1] += "=" + str(v) + + result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["add"], options, obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + profile_table = db.cfgdb.get_entry("MACSEC_PROFILE", profile_name) + assert profile_table + assert profile_table["priority"] == str(profile_map["priority"]) + assert profile_table["cipher_suite"] == profile_map["cipher_suite"] + assert profile_table["primary_cak"] == profile_map["primary_cak"] + assert profile_table["primary_ckn"] == profile_map["primary_ckn"] + assert profile_table["policy"] == profile_map["policy"] + if "enable_replay_protect" in profile_map: + assert "enable_replay_protect" in profile_table and profile_table["enable_replay_protect"] == "1" + assert profile_table["replay_window"] == str(profile_map["replay_window"]) + if "send_sci" in profile_map: + 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: + assert profile_table["replay_period"] == str(rekey_period) + + +def test_macsec_invalid_profile(): + runner = CliRunner() + db = Db() + + # Loss primary cak and primary ckn + result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["add"], ["test"], obj=db) + assert result.exit_code != 0 + + # Invalid primary cak + result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["add"], ["test", "--primary_cak=abcdfghjk90123456789012345678912","--primary_ckn=01234567890123456789012345678912", "--cipher_suite=GCM-AES-128"], obj=db) + assert result.exit_code != 0 + + # Invalid primary cak length + result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["add"], ["test", "--primary_cak=01234567890123456789012345678912","--primary_ckn=01234567890123456789012345678912", "--cipher_suite=GCM-AES-256"], obj=db) + assert result.exit_code != 0 + + +def test_macsec_port(): + runner = CliRunner() + db = Db() + + result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["add"], ["test", "--primary_cak=01234567890123456789012345678912","--primary_ckn=01234567890123456789012345678912"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + result = runner.invoke(config.config.commands["macsec"].commands["port"].commands["add"], ["Ethernet0", "test"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + port_table = db.cfgdb.get_entry("PORT", "Ethernet0") + assert port_table + assert port_table["macsec"] == "test" + + result = runner.invoke(config.config.commands["macsec"].commands["port"].commands["del"], ["Ethernet0"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + port_table = db.cfgdb.get_entry("PORT", "Ethernet0") + assert not port_table["macsec"] + + +def test_macsec_invalid_operation(): + runner = CliRunner() + db = Db() + + # Enable nonexisted profile + result = runner.invoke(config.config.commands["macsec"].commands["port"].commands["add"], ["Ethernet0", "test"], obj=db) + assert result.exit_code != 0 + + # Delete nonexisted profile + result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["del"], ["test"], obj=db) + assert result.exit_code != 0 + + result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["add"], ["test", "--primary_cak=01234567890123456789012345678912","--primary_ckn=01234567890123456789012345678912"], obj=db) + assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) + # Repeat add profile + result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["add"], ["test", "--primary_cak=01234567890123456789012345678912","--primary_ckn=01234567890123456789012345678912"], obj=db) + assert result.exit_code != 0 \ No newline at end of file From 22881184d1c349134fcca1184d525a69b3afb51c Mon Sep 17 00:00:00 2001 From: Ann Pokora Date: Sat, 6 Nov 2021 06:18:04 -0700 Subject: [PATCH 2/4] updates to address review comments --- config/macsec.py | 12 ++++++------ scripts/macsecshow | 6 ++---- tests/config_macsec_test.py | 7 +++---- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/config/macsec.py b/config/macsec.py index 4401668516..6aeaf382e7 100644 --- a/config/macsec.py +++ b/config/macsec.py @@ -27,7 +27,7 @@ def macsec_port(): @clicommon.pass_db def add_port(db, port, profile): """ - Add MACsec port + Add MACsec port """ ctx = click.get_current_context() @@ -46,7 +46,7 @@ def add_port(db, port, profile): # -# 'del' command ('config port del ...') +# 'del' command ('config macsec port del ...') # @macsec_port.command('del') @click.argument('port', metavar='', required=True) @@ -92,10 +92,10 @@ def is_hexstring(hexstring: str): @click.option('--cipher_suite', metavar='', 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='', required=True, type=str, help="Primary Connectivity Association Key.") @click.option('--primary_ckn', metavar='', required=True, type=str, help="Primary CAK Name.") -@click.option('--policy', metavar='', 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('--policy', metavar='', required=False, default="security", show_default=True, type=click.Choice(["integrity_only", "security"]), help="MACsec policy. INTEGRITY_ONLY: All traffic, except EAPOL, will be converted to MACsec packets without encryption. SECURITY: All traffic, except EAPOL, will be encrypted by SecY.") @click.option('--enable_replay_protect/--disable_replay_protect', metavar='', required=False, default=False, show_default=True, is_flag=True, help="Whether enable replay protect.") -@click.option('--replay_window', metavar='', 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.") -@click.option('--send_sci/--no_send_sci', metavar='', required=False, default=True, show_default=True, is_flag=True, help="Whether send SCI.") +@click.option('--replay_window', metavar='', 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 field works only if ENABLE_REPLAY_PROTECT is true.") +@click.option('--send_sci/--no_send_sci', metavar='', required=False, default=True, show_default=True, is_flag=True, help="Send SCI in SecTAG field of MACsec header.") @click.option('--rekey_period', metavar='', required=False, default=0, show_default=True, type=click.IntRange(min=0), help="The period of proactively refresh (Unit second).") @clicommon.pass_db def add_profile(db, profile, priority, cipher_suite, primary_cak, primary_ckn, policy, enable_replay_protect, replay_window, send_sci, rekey_period): @@ -136,7 +136,7 @@ def add_profile(db, profile, priority, cipher_suite, primary_cak, primary_ckn, p profile_table["send_sci"] = send_sci if rekey_period > 0: - profile_table["replay_period"] = rekey_period + profile_table["rekey_period"] = rekey_period for k, v in profile_table.items(): if isinstance(v, bool): diff --git a/scripts/macsecshow b/scripts/macsecshow index b211915b4c..3733f486fa 100755 --- a/scripts/macsecshow +++ b/scripts/macsecshow @@ -6,9 +6,7 @@ # ##################################################################### -import json import os -import re import swsssdk import subprocess import sys @@ -223,9 +221,9 @@ def main(args): intf_name = args[1] if len(args) == 2 else None if command == "connections": - macsec_conn = MacsecConn(intf_name) + MacsecConn(intf_name) elif command == "statistics": - macsec_stat = MacsecStat(intf_name) + MacsecStat(intf_name) sys.exit(0) diff --git a/tests/config_macsec_test.py b/tests/config_macsec_test.py index f78608607e..af3dd69309 100644 --- a/tests/config_macsec_test.py +++ b/tests/config_macsec_test.py @@ -25,7 +25,6 @@ def test_macsec_default_profile(): 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 assert "rekey_period" not in profile_table result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["del"], [profile_name], obj=db) @@ -72,8 +71,8 @@ def test_macsec_valid_profile(): 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: - assert profile_table["replay_period"] == str(rekey_period) + if "rekey_period" in profile_map: + assert profile_table["rekey_period"] == str(profile_map["rekey_period"]) def test_macsec_invalid_profile(): @@ -127,4 +126,4 @@ def test_macsec_invalid_operation(): assert result.exit_code == 0, "exit code: {}, Exception: {}, Traceback: {}".format(result.exit_code, result.exception, result.exc_info) # Repeat add profile result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["add"], ["test", "--primary_cak=01234567890123456789012345678912","--primary_ckn=01234567890123456789012345678912"], obj=db) - assert result.exit_code != 0 \ No newline at end of file + assert result.exit_code != 0 From 95aed5a5f6f3422e7966e99ced302fa2fce6a4e7 Mon Sep 17 00:00:00 2001 From: Ann Pokora Date: Mon, 8 Nov 2021 13:28:59 -0800 Subject: [PATCH 3/4] macsecmgrd expects true/false not 1/0 in MACSEC_PROFILE --- config/macsec.py | 4 ++-- tests/config_macsec_test.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/config/macsec.py b/config/macsec.py index 6aeaf382e7..bb6702dfc1 100644 --- a/config/macsec.py +++ b/config/macsec.py @@ -141,9 +141,9 @@ def add_profile(db, profile, priority, cipher_suite, primary_cak, primary_ckn, p for k, v in profile_table.items(): if isinstance(v, bool): if v: - profile_table[k] = "1" + profile_table[k] = "true" else: - profile_table[k] = "0" + profile_table[k] = "false" else: profile_table[k] = str(v) db.cfgdb.set_entry("MACSEC_PROFILE", profile, profile_table) diff --git a/tests/config_macsec_test.py b/tests/config_macsec_test.py index af3dd69309..d6bef71285 100644 --- a/tests/config_macsec_test.py +++ b/tests/config_macsec_test.py @@ -24,7 +24,7 @@ def test_macsec_default_profile(): assert profile_table["policy"] == "security" assert "enable_replay_protect" not in profile_table assert "replay_window" not in profile_table - assert profile_table["send_sci"] == "1" + assert profile_table["send_sci"] == "true" assert "rekey_period" not in profile_table result = runner.invoke(config.config.commands["macsec"].commands["profile"].commands["del"], [profile_name], obj=db) @@ -65,12 +65,12 @@ def test_macsec_valid_profile(): assert profile_table["primary_ckn"] == profile_map["primary_ckn"] assert profile_table["policy"] == profile_map["policy"] if "enable_replay_protect" in profile_map: - assert "enable_replay_protect" in profile_table and profile_table["enable_replay_protect"] == "1" + assert "enable_replay_protect" in profile_table and profile_table["enable_replay_protect"] == "true" assert profile_table["replay_window"] == str(profile_map["replay_window"]) if "send_sci" in profile_map: - assert profile_table["send_sci"] == "1" + assert profile_table["send_sci"] == "true" if "no_send_sci" in profile_map: - assert profile_table["send_sci"] == "0" + assert profile_table["send_sci"] == "false" if "rekey_period" in profile_map: assert profile_table["rekey_period"] == str(profile_map["rekey_period"]) From 7d05c5574ddeda0a6082aa2543e35b4386b5d116 Mon Sep 17 00:00:00 2001 From: Ann Pokora Date: Thu, 18 Nov 2021 08:48:46 -0800 Subject: [PATCH 4/4] fix for config_db PORT stanza issue --- config/macsec.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/macsec.py b/config/macsec.py index bb6702dfc1..0955a96e04 100644 --- a/config/macsec.py +++ b/config/macsec.py @@ -42,7 +42,7 @@ def add_port(db, port, profile): if len(profile_entry) == 0: ctx.fail("profile {} doesn't exist".format(profile)) - db.cfgdb.set_entry("PORT", port, {'macsec': profile}) + db.cfgdb.mod_entry("PORT", port, {'macsec': profile}) # @@ -64,7 +64,7 @@ def del_port(db, port): if port is None: ctx.fail("cannot find port name for alias {}".format(alias)) - db.cfgdb.set_entry("PORT", port, {'macsec': ""}) + db.cfgdb.mod_entry("PORT", port, {'macsec': ""}) #