[TC-DD-3.17] - DUT should terminate the commissioning process when QR code encodes a test vendor id

[sowmyassp]

Issue : According to test-plan of [TC-DD-3.17] In Step 6b its mentioned DUT should terminate the commissioning process when we pass invalid 21 digit manual code generated in step 6a but DUT successfully completed the commissioning process.

Expected behaviour : DUT should terminate the commissioning mode when invalid 21 digit manual code is passed

Actual behaviour : DUT successfully completed the commissioning process.

Commit used : c1e1d01

Steps to reproduce :
DUT side :
sudo rm -rf /tmp/chip_*
sudo ./chip-all-clusters-app --wifi (Discriminator - 3841, Passcode - 20202021)

TH side

1. ./out/chip-tool/chip-tool pairing manualcode 1 749701123365521327694(0xFFF1)
2. ./out/chip-tool/chip-tool pairing manualcode 1 749701123365522327692(0xFFF2)
3. ./out/chip-tool/chip-tool pairing manualcode 1 749701123365523327697(0xFFF3)
4. ./out/chip-tool/chip-tool pairing manualcode 1 749701123365524327693(0xFFF4)

App used - all-clusters-app

Test platform :
Chip-tool - RPI-4, 8GB RAM
DUT - RPI - RPI-4, 8GB RAM

Test-plan reference :
https://github.com/CHIP-Specifications/chip-test-plans/blob/master/src/devicediscovery.adoc#tc-dd-3-17-commissioning-flow-21-digit-manual-pairing-code-negative-scenario-dut-commissioner

PFA Logs:

TC_DD_3.17_dut.txt
TC_DD_3.17_cntrl.txt

[bzbarsky-apple]

What the test plan says here is:

DUT terminates the commissioning process in a DUT-specific manner according to the DUT manufacturer’s instructions, unless the user is made fully aware of the security risks of providing an uncertified device with operational and networking credentials. [emphasis mine]

The test plan does NOT require a DUT to fail to commission these vendor ids; it just requires that the user be clearly notified that these are uncertified devices.

And even that notification is not reasonable as a default behavior for our test tools, because commissioning things with test vendor ids is by far the most common use of those test tools.

I think we need to figure out exactly what this test plan means in practice for our test tools and whether it applies to them at all. For example, does using a test tool constitute a priori notification that there are risks?

[liamgonyea]

@bzbarsky-apple is correct here and that this test step should not mean the DUT fails it because the DUT commissions the TH (that contains a test, invalid vendor ID).

Given that chip-tool is a testing tool, I am not sure that the warning is necessary but maybe a nice to have.

@krypton36 do you think that darwin-framework-tool needs to present the user with a waring that the Vendor ID (a test one, not valid for certification) is not valid?

[andy31415]

@liamgonyea please provide the spec reference for this requirement.

I believe our current SDK implementation is "If a QR code is provided, always use the long discriminator to commission". I claim this is spec compliant because vendor and product ids are optional in advertisement packets, so making use of them or trying to validate is not guaranteed.

I agree that it could be made more robust (i.e. I scan a QR code that has all fields, then I discover a device and can validate that these IDs match or not to not mistakenly commission something else) however I want to ensure that the spec says I MUST do this (aka SHALL wording). If the spec does not specify this, this is not a spec requirement.

[andy31415]

For specific examples: why is the manual code 749701123365521327694 invalid? It has a test vendor id I guess ... where does the spec say "QR codes containing a test vendor id are invalid"? How would we test devices then?

[andy31415]

./out/linux-x64-chip-tool/chip-tool payload parse-setup-payload 749701123365521327694
[1655479375.139081][3332995:3332995] CHIP:SPL: Parsing decimalRepresentation: 749701123365521327694
[1655479375.139101][3332995:3332995] CHIP:SPL: Version:       0
[1655479375.139103][3332995:3332995] CHIP:SPL: VendorID:      65521
[1655479375.139104][3332995:3332995] CHIP:SPL: ProductID:     32769
[1655479375.139105][3332995:3332995] CHIP:SPL: Custom flow:   2    (CUSTOM)
[1655479375.139106][3332995:3332995] CHIP:SPL: Capabilities:  0x00 (NONE)
[1655479375.139107][3332995:3332995] CHIP:SPL: Discriminator: 3840
[1655479375.139108][3332995:3332995] CHIP:SPL: Passcode:      20202021

is the Custom flow an issue?

[andy31415]

Reading more comments, I guess the request is 'show a nice message about this being a test id' .... It sounds like that is a nice to have (guessing not a spec SHALL requirement) and probably not a cert blocker for our internal test tools like chip_tool.

You can make a test plan to use parse-setup-payload during certification to validate ids that vendors generate. But that seems independent of tooling or SDK examples.

[bzbarsky-apple]

guessing not a spec SHALL requirement

That's a good question. I thought it was, but all I am finding right now is:

The Test Vendor IDs are reserved for test and development by device manufacturers or hobbyists. Commissioners SHOULD NOT commission devices using one of these VIDs onto an operational Fabric under normal operation unless the user is made fully aware of the security risks of providing an uncertified device with operational and networking credentials.

in section "2.5.2. Vendor Identifier (Vendor ID, VID)" of the spec. That is not a SHALL, indeed.

[andy31415]

So it sounds like this is a test case for "commissioners". We have to require any commissioner to show proper notices.

Does this extend to chip-tool though? I guess it could and we can add some text. I also believe that chip-tool is a wall-of-text in terms of the output so any warnings will be missed. To be in the spirit of the spec, I believe we should reject by default and have some flag of --allow-test-ids so that the user has to manually allow them ... but that will get annoying fast for developers.

[andy31415]

Are we testing DUT or testing chip-tool as a spec compliant commissioner? I believe former, so then this is not a spec blocker for the SDK.

[andy31415]

To expand on this: cert blocker is if you cannot prove that you can write a spec-compliant app using the SDK.
SDK proves it can parse qr codes and display them (using payload parse-setup-payload) so an implementation can display any necessary texts.

Unless we want to certify chip-tool, I don't think there is an issue here.

[woody-apple]

Cert Blocker Review: The spec does not mandate this, and this usage seems perfectly valid for a test tool. Closing this issue.