TACACSPLUS_PASSKEY_ENCRYPTION support Part - I

.azure-pipelines/pre-commit-check.yml

@@ -0,0 +1,35 @@
+steps:
+- checkout: self
+  clean: true
+  displayName: 'checkout sonic-utilities repo'
+
+- script: |
+    set -x
+    sudo pip install pre-commit
+    pre-commit install-hooks
+  displayName: 'Prepare pre-commit check'
+
+- script: |
+    # Run pre-commit check and capture the output
+    out=`pre-commit run --color never --from-ref HEAD^ --to-ref HEAD 2>&1`
+    RC=$?
+    if [[ $RC -ne 0 ]]; then
+      echo -e "The [pre-commit](http://pre-commit.com/) check detected issues in the files touched by this pull request.\n\
+    The pre-commit check is a mandatory check, please fix detected issues.\n\
+    \n\
+    To run the pre-commit checks locally, you can follow below steps:\n\
+      1. Ensure that default python is python3.\n\
+      2. Ensure that the 'pre-commit' package is installed:\n\
+         sudo pip install pre-commit\n\
+      3. Go to repository root folder\n\
+      4. Install the pre-commit hooks:\n\
+         pre-commit install\n\
+      5. Use pre-commit to check staged file:\n\
+         pre-commit\n\
+      6. Alternatively, you can check committed files using:\n\
+         pre-commit run --from-ref <commit_id> --to-ref <commit_id>\n"
+    fi
+    echo "Pre-commit check results:"
+    echo "$out"
+    exit $RC
+  displayName: 'Run pre-commit check'

.github/workflows/semgrep.yml

@@ -18,4 +18,4 @@ jobs:
       - uses: actions/checkout@v3
       - run: semgrep ci
         env:
-           SEMGREP_RULES: p/default
+           SEMGREP_RULES: "p/default r/python.lang.security.audit.dangerous-system-call-audit.dangerous-system-call-audit"

.gitignore

@@ -15,6 +15,10 @@ dist/
 coverage.xml
 htmlcov/
 
+# Dev tools
+.vscode/
+.idea/
+
 # Ignores for sonic-utilities-data
 sonic-utilities-data/debian/*
 !sonic-utilities-data/debian/changelog

.pre-commit-config.yaml

@@ -0,0 +1,9 @@
+# See https://pre-commit.com for more information
+# See https://pre-commit.com/hooks.html for more hooks
+repos:
+-   repo: https://github.com/PyCQA/flake8
+    rev: 4.0.1
+    hooks:
+    -   id: flake8
+        entry: bash -c 'git diff HEAD^ HEAD -U0 -- "$@" | flake8 --diff "$@"' --
+        args: ["--max-line-length=120"]

README.md

@@ -51,7 +51,7 @@ A convenient alternative is to let the SONiC build system configure a build envi
 
 2. Build the sonic-utilities Python wheel package inside the Bullseye slave container, and tell the build system to keep the container alive when finished
     ```
-    make NOSTRETCH=1 NOBUSTER=1 KEEP_SLAVE_ON=yes target/python-wheels/bullseye/sonic_utilities-1.2-py3-none-any.whl
+    make -f Makefile.work BLDENV=bookworm KEEP_SLAVE_ON=yes target/python-wheels/bookworm/sonic_utilities-1.2-py3-none-any.whl
     ```
 
 3. When the build finishes, your prompt will change to indicate you are inside the slave container. Change into the `src/sonic-utilities/` directory
@@ -66,13 +66,20 @@ A convenient alternative is to let the SONiC build system configure a build envi
 ```
 python3 setup.py bdist_wheel
 ```
+Note: This command by default will not update the wheel package in target/. To specify the destination location of wheel package, use "-d" option.
 
 #### To run unit tests
 
 ```
 python3 setup.py test
 ```
 
+#### To install the package on a SONiC machine
+```
+sudo pip uninstall sonic-utilities
+sudo pip install YOUR_WHEEL_PACKAGE
+```
+Note: Don't use "--force-reinstall".
 
 ### sonic-utilities-data
 

acl_loader/main.py

@@ -73,8 +73,10 @@ class AclLoader(object):
     ACL_TABLE = "ACL_TABLE"
     ACL_RULE = "ACL_RULE"
     CFG_ACL_TABLE = "ACL_TABLE"
+    APPL_ACL_TABLE = "ACL_TABLE_TABLE"
     STATE_ACL_TABLE = "ACL_TABLE_TABLE"
     CFG_ACL_RULE = "ACL_RULE"
+    APPL_ACL_RULE = "ACL_RULE_TABLE"
     STATE_ACL_RULE = "ACL_RULE_TABLE"
     ACL_TABLE_TYPE_MIRROR = "MIRROR"
     ACL_TABLE_TYPE_CTRLPLANE = "CTRLPLANE"
@@ -135,6 +137,8 @@ def __init__(self):
         self.configdb.connect()
         self.statedb = SonicV2Connector(host="127.0.0.1")
         self.statedb.connect(self.statedb.STATE_DB)
+        self.appldb = SonicV2Connector(host="127.0.0.1")
+        self.appldb.connect(self.statedb.APPL_DB)
 
         # For multi-npu architecture we will have both global and per front asic namespace.
         # Global namespace will be used for Control plane ACL which are via IPTables.
@@ -165,8 +169,8 @@ def __init__(self):
         self.read_rules_info()
         self.read_sessions_info()
         self.read_policers_info()
-        self.acl_table_status = self.read_acl_object_status_info(self.CFG_ACL_TABLE, self.STATE_ACL_TABLE)
-        self.acl_rule_status = self.read_acl_object_status_info(self.CFG_ACL_RULE, self.STATE_ACL_RULE)
+        self.acl_table_status = self.read_acl_object_status_info(self.tables_db_info.keys(), self.STATE_ACL_TABLE)
+        self.acl_rule_status = self.read_acl_object_status_info(self.rules_db_info.keys(), self.STATE_ACL_RULE)
 
     def read_tables_info(self):
         """
@@ -199,16 +203,51 @@ def read_tables_info(self):
                         self.tables_db_info[table]['ports'] += entry.get(
                             'ports', [])
 
+        if self.per_npu_configdb:
+            # Note: Ability to read table information from APPL_DB is not yet supported for masic devices
+            return
+
+        appl_db_keys = self.appldb.keys(self.appldb.APPL_DB, "{}:*".format(self.APPL_ACL_TABLE))
+        if not appl_db_keys:
+            return
+
+        for app_acl_tbl in appl_db_keys:
+            key = app_acl_tbl.split(":")[-1]
+            if key in self.tables_db_info:
+                # Shouldn't be hit, table is either programmed to APPL or CONFIG DB
+                continue
+            self.tables_db_info[key] = dict()
+            for f, v in self.appldb.get_all(self.appldb.APPL_DB, app_acl_tbl).items():
+                if f.lower() == "ports":
+                    v = v.split(",")
+                self.tables_db_info[key][f.lower()] = v
+
     def get_tables_db_info(self):
         return self.tables_db_info
 
     def read_rules_info(self):
         """
-        Read ACL_RULE table from configuration database
+        Read ACL_RULE table from CFG_DB and APPL_DB database
         :return:
         """
         self.rules_db_info = self.configdb.get_table(self.ACL_RULE)
 
+        if self.per_npu_configdb:
+            # Note: Ability to read table information from APPL_DB is not yet supported for masic devices
+            return
+
+        # Read rule information from APPL_DB
+        appl_db_keys = self.appldb.keys(self.appldb.APPL_DB, "{}:*".format(self.APPL_ACL_RULE))
+        if not appl_db_keys:
+            return
+
+        for app_acl_rule in appl_db_keys:
+            _, tid, rid = app_acl_rule.split(":")
+            if (tid, rid) in self.rules_db_info:
+                # Shouldn't be hit, table is either programmed to APPL or CONFIG DB
+                continue
+            self.rules_db_info[(tid, rid)] = self.appldb.get_all(self.appldb.APPL_DB, app_acl_rule)
+
     def get_rules_db_info(self):
         return self.rules_db_info
 
@@ -259,16 +298,10 @@ def read_sessions_info(self):
                 self.sessions_db_info[key]["status"] = state_db_info.get("status", "inactive") if state_db_info else "error"
                 self.sessions_db_info[key]["monitor_port"] = state_db_info.get("monitor_port", "") if state_db_info else ""
 
-    def read_acl_object_status_info(self, cfg_db_table_name, state_db_table_name):
+    def read_acl_object_status_info(self, keys, state_db_table_name):
         """
         Read ACL_TABLE status or ACL_RULE status from STATE_DB
         """
-        if self.per_npu_configdb:
-            namespace_configdb = list(self.per_npu_configdb.values())[0]
-            keys = namespace_configdb.get_table(cfg_db_table_name).keys()
-        else:
-            keys = self.configdb.get_table(cfg_db_table_name).keys()
-
         status = {}
         for key in keys:
             # For ACL_RULE, the key is (acl_table_name, acl_rule_name)
@@ -413,17 +446,17 @@ def parse_acl_json(filename):
                 raise AclLoaderException("Invalid input file %s" % filename)
         return yang_acl
 
-    def load_rules_from_file(self, filename):
+    def load_rules_from_file(self, filename, skip_action_validation=False):
         """
         Load file with ACL rules configuration in openconfig ACL format. Convert rules
         to Config DB schema.
         :param filename: File in openconfig ACL format
         :return:
         """
         self.yang_acl = AclLoader.parse_acl_json(filename)
-        self.convert_rules()
+        self.convert_rules(skip_action_validation)
 
-    def convert_action(self, table_name, rule_idx, rule):
+    def convert_action(self, table_name, rule_idx, rule, skip_validation=False):
         rule_props = {}
 
         if rule.actions.config.forwarding_action == "ACCEPT":
@@ -452,13 +485,13 @@ def convert_action(self, table_name, rule_idx, rule):
             raise AclLoaderException("Unknown rule action {} in table {}, rule {}".format(
                 rule.actions.config.forwarding_action, table_name, rule_idx))
 
-        if not self.validate_actions(table_name, rule_props):
+        if not self.validate_actions(table_name, rule_props, skip_validation):
             raise AclLoaderException("Rule action {} is not supported in table {}, rule {}".format(
                 rule.actions.config.forwarding_action, table_name, rule_idx))
 
         return rule_props
 
-    def validate_actions(self, table_name, action_props):
+    def validate_actions(self, table_name, action_props, skip_validation=False):
         if self.is_table_control_plane(table_name):
             return True
 
@@ -481,6 +514,11 @@ def validate_actions(self, table_name, action_props):
         else:
             aclcapability = self.statedb.get_all(self.statedb.STATE_DB, "{}|{}".format(self.ACL_STAGE_CAPABILITY_TABLE, stage.upper()))
             switchcapability = self.statedb.get_all(self.statedb.STATE_DB, "{}|switch".format(self.SWITCH_CAPABILITY_TABLE))
+        # In the load_minigraph path, it's possible that the STATE_DB entry haven't pop up because orchagent is stopped
+        # before loading acl.json. So we skip the validation if any table is empty
+        if skip_validation and (not aclcapability or not switchcapability):
+            warning("Skipped action validation as capability table is not present in STATE_DB")
+            return True
         for action_key in dict(action_props):
             action_list_key = self.ACL_ACTIONS_CAPABILITY_FIELD
             if action_list_key not in aclcapability:
@@ -699,7 +737,7 @@ def validate_rule_fields(self, rule_props):
             if ("ICMPV6_TYPE" in rule_props or "ICMPV6_CODE" in rule_props) and protocol != 58:
                 raise AclLoaderException("IP_PROTOCOL={} is not ICMPV6, but ICMPV6 fields were provided".format(protocol))
 
-    def convert_rule_to_db_schema(self, table_name, rule):
+    def convert_rule_to_db_schema(self, table_name, rule, skip_action_validation=False):
         """
         Convert rules format from openconfig ACL to Config DB schema
         :param table_name: ACL table name to which rule belong
@@ -729,7 +767,7 @@ def convert_rule_to_db_schema(self, table_name, rule):
         elif self.is_table_l3(table_name):
             rule_props["ETHER_TYPE"] = str(self.ethertype_map["ETHERTYPE_IPV4"])
 
-        deep_update(rule_props, self.convert_action(table_name, rule_idx, rule))
+        deep_update(rule_props, self.convert_action(table_name, rule_idx, rule, skip_action_validation))
         deep_update(rule_props, self.convert_l2(table_name, rule_idx, rule))
         deep_update(rule_props, self.convert_ip(table_name, rule_idx, rule))
         deep_update(rule_props, self.convert_icmp(table_name, rule_idx, rule))
@@ -761,7 +799,7 @@ def deny_rule(self, table_name):
             return {}  # Don't add default deny rule if table is not [L3, L3V6]
         return rule_data
 
-    def convert_rules(self):
+    def convert_rules(self, skip_aciton_validation=False):
         """
         Convert rules in openconfig ACL format to Config DB schema
         :return:
@@ -780,7 +818,7 @@ def convert_rules(self):
             for acl_entry_name in acl_set.acl_entries.acl_entry:
                 acl_entry = acl_set.acl_entries.acl_entry[acl_entry_name]
                 try:
-                    rule = self.convert_rule_to_db_schema(table_name, acl_entry)
+                    rule = self.convert_rule_to_db_schema(table_name, acl_entry, skip_aciton_validation)
                     deep_update(self.rules_info, rule)
                 except AclLoaderException as ex:
                     error("Error processing rule %s: %s. Skipped." % (acl_entry_name, ex))
@@ -917,19 +955,19 @@ def show_table(self, table_name):
                 status = self.acl_table_status[key]['status']
             else:
                 status = 'N/A'
-            if val["type"] == AclLoader.ACL_TABLE_TYPE_CTRLPLANE:
+            if val.get("type", "N/A") == AclLoader.ACL_TABLE_TYPE_CTRLPLANE:
                 services = natsorted(val["services"])
-                data.append([key, val["type"], services[0], val["policy_desc"], stage, status])
+                data.append([key, val.get("type", "N/A"), services[0], val.get("policy_desc", ""), stage, status])
 
                 if len(services) > 1:
                     for service in services[1:]:
                         data.append(["", "", service, "", "", ""])
             else:
-                if not val["ports"]:
-                    data.append([key, val["type"], "", val["policy_desc"], stage, status])
+                if not val.get("ports", []):
+                    data.append([key, val["type"], "", val.get("policy_desc", ""), stage, status])
                 else:
                     ports = natsorted(val["ports"])
-                    data.append([key, val["type"], ports[0], val["policy_desc"], stage, status])
+                    data.append([key, val["type"], ports[0], val.get("policy_desc", ""), stage, status])
 
                     if len(ports) > 1:
                         for port in ports[1:]:
@@ -1149,8 +1187,9 @@ def update(ctx):
 @click.option('--session_name', type=click.STRING, required=False)
 @click.option('--mirror_stage', type=click.Choice(["ingress", "egress"]), default="ingress")
 @click.option('--max_priority', type=click.INT, required=False)
+@click.option('--skip_action_validation', is_flag=True, default=False, help="Skip action validation")
 @click.pass_context
-def full(ctx, filename, table_name, session_name, mirror_stage, max_priority):
+def full(ctx, filename, table_name, session_name, mirror_stage, max_priority, skip_action_validation):
     """
     Full update of ACL rules configuration.
     If a table_name is provided, the operation will be restricted in the specified table.
@@ -1168,7 +1207,7 @@ def full(ctx, filename, table_name, session_name, mirror_stage, max_priority):
     if max_priority:
         acl_loader.set_max_priority(max_priority)
 
-    acl_loader.load_rules_from_file(filename)
+    acl_loader.load_rules_from_file(filename, skip_action_validation)
     acl_loader.full_update()