TC-SC-7.1: Add test for unique discriminators (project-chip#34407)

* TC-SC-5.1: Add test for unique discriminators

see
CHIP-Specifications/connectedhomeip-spec#9117

Test:
please see test_testing for unit tests.

* Restyled by autopep8

* Restyled by isort

* rename test to TC-SC-7.1

TC-SC-5.1 exists, it's just in a different section.

* linter

* make setup codes empty lists by default

* fix basic comp

* fix qr code in ci

* Fixup manual codes

* Restyled by autopep8

* linter

---------

Co-authored-by: Restyled.io <commits@restyled.io>

Author: cecille

.github/workflows/tests.yaml

@@ -576,6 +576,7 @@ jobs:
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --load-from-env /tmp/test_env.yaml --script src/python_testing/TC_RVCOPSTATE_2_1.py'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --load-from-env /tmp/test_env.yaml --script src/python_testing/TC_RVCOPSTATE_2_3.py'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --load-from-env /tmp/test_env.yaml --script src/python_testing/TC_RVCOPSTATE_2_4.py'
+                  scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --load-from-env /tmp/test_env.yaml --script src/python_testing/TC_SC_7_1.py'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --script "src/python_testing/TestConformanceSupport.py" --script-args "--trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --script "src/python_testing/TestMatterTestingSupport.py" --script-args "--trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'
                   scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --script "src/python_testing/TestSpecParsingSupport.py" --script-args "--trace-to json:out/trace_data/test-{SCRIPT_BASE_NAME}.json --trace-to perfetto:out/trace_data/test-{SCRIPT_BASE_NAME}.perfetto"'
@@ -585,6 +586,7 @@ jobs:
                   scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/TestSpecParsingDeviceType.py'
                   scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/TestConformanceSupport.py'
                   scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/test_testing/test_IDM_10_4.py'
+                  scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/test_testing/test_TC_SC_7_1.py'
 
             - name: Uploading core files
               uses: actions/upload-artifact@v4

src/python_testing/TC_DA_1_7.py

@@ -158,15 +158,13 @@ def steps_TC_DA_1_7(self):
 
     @async_test_body
     async def test_TC_DA_1_7(self):
-        # post_cert_tests (or sdk) can use the qr or manual code
-        # We don't currently support this in cert because the base doesn't support multiple QR/manual
         num = 0
         if self.matter_test_config.discriminators:
             num += len(self.matter_test_config.discriminators)
         if self.matter_test_config.qr_code_content:
-            num += 1
+            num += len(self.matter_test_config.qr_code_content)
         if self.matter_test_config.manual_code:
-            num += 1
+            num += len(self.matter_test_config.manual_code)
 
         if num != self.expected_number_of_DUTs():
             if self.allow_sdk_dac:

src/python_testing/TC_SC_7_1.py

@@ -0,0 +1,114 @@
+#
+#    Copyright (c) 2022 Project CHIP Authors
+#    All rights reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License");
+#    you may not use this file except in compliance with the License.
+#    You may obtain a copy of the License at
+#
+#        http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS,
+#    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#    See the License for the specific language governing permissions and
+#    limitations under the License.
+#
+
+# See https://github.com/project-chip/connectedhomeip/blob/master/docs/testing/python.md#defining-the-ci-test-arguments
+# for details about the block below.
+#
+# === BEGIN CI TEST ARGUMENTS ===
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 2222 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --bool-arg post_cert_test:true  --qr-code MT:-24J0KCZ16750648G00 --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+# === END CI TEST ARGUMENTS ===
+
+# Note that in the CI we are using the post-cert test as we can only start one app from the current script.
+# This should still be fine as this test has unit tests for other conditions. See test_TC_SC_7_1.py
+import logging
+
+import chip.clusters as Clusters
+from matter_testing_support import MatterBaseTest, TestStep, async_test_body, default_matter_test_main
+from mobly import asserts
+
+
+def _trusted_root_test_step(dut_num: int) -> TestStep:
+    read_trusted_roots_over_pase = f'TH establishes a PASE session to DUT{dut_num} using the provided setup code and reads the TrustedRootCertificates attribute from the operational credentials cluster over PASE'
+    return TestStep(dut_num, read_trusted_roots_over_pase, "List should be empty as the DUT should be in factory reset ")
+
+
+class TC_SC_7_1(MatterBaseTest):
+    ''' TC-SC-7.1
+
+        This test requires two instances of the DUT with the same PID/VID to confirm that the individual
+        devices are provisioned with different discriminators and PAKE salts in the same product line.
+
+        This test MUST be run on a factory reset device, over PASE, with no commissioned fabrics.
+    '''
+
+    def __init__(self, *args):
+        super().__init__(*args)
+        self.post_cert_test = False
+
+    def setup_class(self):
+        super().setup_class()
+        self.post_cert_test = self.user_params.get("post_cert_test", False)
+
+    def expected_number_of_DUTs(self) -> int:
+        return 1 if self.post_cert_test else 2
+
+    def steps_TC_SC_7_1(self):
+        if self.post_cert_test:
+            return [_trusted_root_test_step(1),
+                    TestStep(2, "TH extracts the discriminator from the provided setup code", "Ensure the code is not the default")]
+
+        return [_trusted_root_test_step(1),
+                _trusted_root_test_step(2),
+                TestStep(3, "TH compares the discriminators from the provided setup codes", "Discriminators do not match")]
+
+    # TODO: Need a pics or something to limit this to devices that have a factory-provided matter setup code (as opposed to a field upgradable device / device with a custom commissioning where this test won't apply)
+
+    @async_test_body
+    async def test_TC_SC_7_1(self):
+        # For now, this test is WAY easier if we just ask for the setup code instead of discriminator / passcode
+        asserts.assert_false(self.matter_test_config.discriminators,
+                             "This test needs to be run with either the QR or manual setup code. The QR code is preferred.")
+
+        if len(self.matter_test_config.qr_code_content + self.matter_test_config.manual_code) != self.expected_number_of_DUTs():
+            if self.post_cert_test:
+                msg = "The post_cert_test flag is only for use post-certification. When using this flag, specify a single discriminator, manual-code or qr-code-content"
+            else:
+                msg = "This test requires two devices for use at certification. Specify two device discriminators or QR codes ex. --discriminator 1234 5678"
+            asserts.fail(msg)
+
+        # Make sure these are no fabrics on the device so we know we're looking at the factory discriminator. This also ensures that the provided codes are correct.
+        for i, setup_code in enumerate(self.matter_test_config.qr_code_content + self.matter_test_config.manual_code):
+            self.step(i+1)
+            await self.default_controller.FindOrEstablishPASESession(setupCode=setup_code, nodeid=i+1)
+            root_certs = await self.read_single_attribute_check_success(node_id=i+1, cluster=Clusters.OperationalCredentials, attribute=Clusters.OperationalCredentials.Attributes.TrustedRootCertificates, endpoint=0)
+            asserts.assert_equal(
+                root_certs, [], "Root certificates found on device. Device must be factory reset before running this test.")
+
+        self.step(i+2)
+        setup_payload_info = self.get_setup_payload_info()
+        if self.post_cert_test:
+            # For post-cert, we're testing against the defaults
+            # TODO: Does it even make sense to test against a manual code in post-cert? It's such a small space, collisions are likely. Should we restrict post-cert to QR? What if one isn't provided?
+            asserts.assert_not_equal(setup_payload_info[0].filter_value, 3840, "Device is using the default discriminator")
+        else:
+            if setup_payload_info[0].filter_value == setup_payload_info[1].filter_value and self.matter_test_config.manual_code is not None:
+                logging.warn("The two provided discriminators are the same. Note that this CAN occur by chance, especially when using manual codes with the short discriminator. Consider using a QR code, or a different device if you believe the DUTs have individually provisioned")
+            asserts.assert_not_equal(
+                setup_payload_info[0].filter_value, setup_payload_info[1].filter_value, "Devices are using the same discriminator values")
+
+        # TODO: add test for PAKE salt. This needs to be plumbed through starting from HandlePBKDFParamResponse.
+        # Will handle in a separate follow up as the plumbing here is aggressive and through some of the crypto layers.
+        # TODO: Other unit-specific values?
+
+
+if __name__ == "__main__":
+    default_matter_test_main()

src/python_testing/basic_composition_support.py

@@ -99,9 +99,11 @@ def ConvertValue(value) -> Any:
 
 class BasicCompositionTests:
     async def connect_over_pase(self, dev_ctrl):
-        setupCode = self.matter_test_config.qr_code_content if self.matter_test_config.qr_code_content is not None else self.matter_test_config.manual_code
-        asserts.assert_true(setupCode, "Require either --qr-code or --manual-code.")
-        await dev_ctrl.FindOrEstablishPASESession(setupCode, self.dut_node_id)
+        asserts.assert_true(self.matter_test_config.qr_code_content == [] or self.matter_test_config.manual_code == [],
+                            "Cannot have both QR and manual code specified")
+        setupCode = self.matter_test_config.qr_code_content + self.matter_test_config.manual_code
+        asserts.assert_equal(len(setupCode), 1, "Require one of either --qr-code or --manual-code.")
+        await dev_ctrl.FindOrEstablishPASESession(setupCode[0], self.dut_node_id)
 
     def dump_wildcard(self, dump_device_composition_path: typing.Optional[str]):
         node_dump_dict = {endpoint_id: MatterTlvToJson(self.endpoints_tlv[endpoint_id]) for endpoint_id in self.endpoints_tlv}

src/python_testing/matter_testing_support.py

@@ -366,8 +366,8 @@ class MatterTestConfig:
     # This allows cert tests to be run without re-commissioning for RR-1.1.
     maximize_cert_chains: bool = True
 
-    qr_code_content: Optional[str] = None
-    manual_code: Optional[str] = None
+    qr_code_content: List[str] = field(default_factory=list)
+    manual_code: List[str] = field(default_factory=list)
 
     wifi_ssid: Optional[str] = None
     wifi_passphrase: Optional[str] = None
@@ -1067,34 +1067,45 @@ def step(self, step: typing.Union[int, str]):
         self.current_step_index = self.current_step_index + 1
         self.step_skipped = False
 
-    def get_setup_payload_info(self) -> SetupPayloadInfo:
-        if self.matter_test_config.qr_code_content is not None:
-            qr_code = self.matter_test_config.qr_code_content
+    def get_setup_payload_info(self) -> List[SetupPayloadInfo]:
+        setup_payloads = []
+        for qr_code in self.matter_test_config.qr_code_content:
             try:
-                setup_payload = SetupPayload().ParseQrCode(qr_code)
+                setup_payloads.append(SetupPayload().ParseQrCode(qr_code))
             except ChipStackError:
                 asserts.fail(f"QR code '{qr_code} failed to parse properly as a Matter setup code.")
 
-        elif self.matter_test_config.manual_code is not None:
-            manual_code = self.matter_test_config.manual_code
+        for manual_code in self.matter_test_config.manual_code:
             try:
-                setup_payload = SetupPayload().ParseManualPairingCode(manual_code)
+                setup_payloads.append(SetupPayload().ParseManualPairingCode(manual_code))
             except ChipStackError:
                 asserts.fail(
                     f"Manual code code '{manual_code}' failed to parse properly as a Matter setup code. Check that all digits are correct and length is 11 or 21 characters.")
-        else:
-            asserts.fail("Require either --qr-code or --manual-code.")
-
-        info = SetupPayloadInfo()
-        info.passcode = setup_payload.setup_passcode
-        if setup_payload.short_discriminator is not None:
-            info.filter_type = discovery.FilterType.SHORT_DISCRIMINATOR
-            info.filter_value = setup_payload.short_discriminator
-        else:
-            info.filter_type = discovery.FilterType.LONG_DISCRIMINATOR
-            info.filter_value = setup_payload.long_discriminator
 
-        return info
+        infos = []
+        for setup_payload in setup_payloads:
+            info = SetupPayloadInfo()
+            info.passcode = setup_payload.setup_passcode
+            if setup_payload.short_discriminator is not None:
+                info.filter_type = discovery.FilterType.SHORT_DISCRIMINATOR
+                info.filter_value = setup_payload.short_discriminator
+            else:
+                info.filter_type = discovery.FilterType.LONG_DISCRIMINATOR
+                info.filter_value = setup_payload.long_discriminator
+            infos.append(info)
+
+        num_passcodes = 0 if self.matter_test_config.setup_passcodes is None else len(self.matter_test_config.setup_passcodes)
+        num_discriminators = 0 if self.matter_test_config.discriminators is None else len(self.matter_test_config.discriminators)
+        asserts.assert_equal(num_passcodes, num_discriminators, "Must have same number of discriminators as passcodes")
+        if self.matter_test_config.discriminators:
+            for idx, discriminator in enumerate(self.matter_test_config.discriminators):
+                info = SetupPayloadInfo()
+                info.passcode = self.matter_test_config.setup_passcodes[idx]
+                info.filter_type = DiscoveryFilterType.LONG_DISCRIMINATOR
+                info.filter_value = discriminator
+                infos.append(info)
+
+        return infos
 
     def wait_for_user_input(self,
                             prompt_msg: str,
@@ -1293,36 +1304,31 @@ def populate_commissioning_args(args: argparse.Namespace, config: MatterTestConf
     config.commissioning_method = args.commissioning_method
     config.commission_only = args.commission_only
 
-    # TODO: this should also allow multiple once QR and manual codes are supported.
-    config.qr_code_content = args.qr_code
-    if args.manual_code:
-        config.manual_code = args.manual_code
-    else:
-        config.manual_code = None
+    config.qr_code_content.extend(args.qr_code)
+    config.manual_code.extend(args.manual_code)
 
     if args.commissioning_method is None:
         return True
 
-    if args.discriminators is None and (args.qr_code is None and args.manual_code is None):
+    if args.discriminators == [] and (args.qr_code == [] and args.manual_code == []):
         print("error: Missing --discriminator when no --qr-code/--manual-code present!")
         return False
     config.discriminators = args.discriminators
 
-    if args.passcodes is None and (args.qr_code is None and args.manual_code is None):
+    if args.passcodes == [] and (args.qr_code == [] and args.manual_code == []):
         print("error: Missing --passcode when no --qr-code/--manual-code present!")
         return False
     config.setup_passcodes = args.passcodes
 
-    if args.qr_code is not None and args.manual_code is not None:
+    if args.qr_code != [] and args.manual_code != []:
         print("error: Cannot have both --qr-code and --manual-code present!")
         return False
 
     if len(config.discriminators) != len(config.setup_passcodes):
         print("error: supplied number of discriminators does not match number of passcodes")
         return False
 
-    device_descriptors = [config.qr_code_content] if config.qr_code_content is not None else [
-        config.manual_code] if config.manual_code is not None else config.discriminators
+    device_descriptors = config.qr_code_content + config.manual_code + config.discriminators
 
     if len(config.dut_node_ids) > len(device_descriptors):
         print("error: More node IDs provided than discriminators")
@@ -1491,9 +1497,9 @@ def parse_matter_test_args(argv: Optional[List[str]] = None) -> MatterTestConfig
     code_group = parser.add_mutually_exclusive_group(required=False)
 
     code_group.add_argument('-q', '--qr-code', type=str,
-                            metavar="QR_CODE", help="QR setup code content (overrides passcode and discriminator)")
+                            metavar="QR_CODE", default=[], help="QR setup code content (overrides passcode and discriminator)", nargs="+")
     code_group.add_argument('--manual-code', type=str_from_manual_code,
-                            metavar="MANUAL_CODE", help="Manual setup code content (overrides passcode and discriminator)")
+                            metavar="MANUAL_CODE", default=[], help="Manual setup code content (overrides passcode and discriminator)", nargs="+")
 
     fabric_group = parser.add_argument_group(
         title="Fabric selection", description="Fabric selection for single-fabric basic usage, and commissioning")
@@ -1567,15 +1573,7 @@ async def _commission_device(self, i) -> bool:
         dev_ctrl = self.default_controller
         conf = self.matter_test_config
 
-        # TODO: qr code and manual code aren't lists
-
-        if conf.qr_code_content or conf.manual_code:
-            info = self.get_setup_payload_info()
-        else:
-            info = SetupPayloadInfo()
-            info.passcode = conf.setup_passcodes[i]
-            info.filter_type = DiscoveryFilterType.LONG_DISCRIMINATOR
-            info.filter_value = conf.discriminators[i]
+        info = self.get_setup_payload_info()[i]
 
         if conf.commissioning_method == "on-network":
             try:

src/python_testing/post_certification_tests/production_device_checks.py

@@ -352,9 +352,9 @@ def __init__(self, code: str, code_type: SetupCodeType):
         self.config = MatterTestConfig(endpoint=0, dut_node_ids=[
                                        1], global_test_params=global_test_params, storage_path=self.admin_storage)
         if code_type == SetupCodeType.QR:
-            self.config.qr_code_content = code
+            self.config.qr_code_content = [code]
         else:
-            self.config.manual_code = code
+            self.config.manual_code = [code]
         self.config.paa_trust_store_path = Path(self.paa_path)
         # Set for DA-1.2, which uses the CD signing certs for verification. This test is now set to use the production CD signing certs from the DCL.
         self.config.global_test_params['cd_cert_dir'] = tmpdir_cd