search

project-chip / connectedhomeip

Matter (formerly Project CHIP) creates more connections between more objects, simplifying development for manufacturers and increasing compatibility for consumers, guided by the Connectivity Standards Alliance.
https://buildwithmatter.com
Apache License 2.0
7.34k stars 1.97k forks source link

[BUG] KeySetRemove Command returns SUCCESS status code when the parameter is {} #28679

Closed Agatha2333 closed 1 year ago

Agatha2333 commented 1 year ago

Reproduction steps

spec reference: 11.2.9.4. KeySetRemove Command This command SHALL fail with an INVALID_COMMAND status code back to the initiator if the GroupKeySetID being removed is 0, which is the Key Set associated with the Identity Protection Key (IPK).

KeySetRemove Command field with uint16 type

  1. use the chip-tool to send the script 
    chip-tool groupkeymanagement command-by-id 0x03 '{}'

Error.txt

Bug prevalence

NA

GitHub hash of the SDK that was being used

na

Platform

other

Platform Version(s)

NA

Anything else?

It appears that this script deletes the IPK, which could potentially cause a bug to occur on all genuine devices, such as the Govee Light strip, Nanoleaf Light strip, SwitchBot Hub 2, and EVE Energy Matter devices. Once this script is executed, subsequent scripts may prompt the devices to respond with errors like "Invalid CASE parameter" or "No shared trusted root."

Device type Protocol Vendor Error
Smart Plug Mini Matter over Wifi Tapo Invalid Case parameter
Smart Plug Matter over Wifi tp-link Invalid Case parameter
Hub Matter over Wifi SwitchBot No shared trusted root
Smart Bulb Matter over Wifi OREIN No shared trusted root
Smart Sensor for Doors & Windows Matter over Thread Eve Invalid Case parameter
Light strip Matter over Thread Nanoleaf Invalid Case parameter

bzbarsky-apple commented 1 year ago

GitHub hash of the SDK that was being used na

It's not "na"; it's critically important, @Agatha2333. Which SHA was the server running? Did it include PR #28524?

Agatha2333 commented 1 year ago

I downloaded both SDKs and executed the chip-tool using the following versions:

  1. From the master branch: git show e75d35a7581ced7152a4642e070c21f942ab66d7
  2. Using the tagged release: v1.1.0

And, both of them exhibit the same issue.

Regarding whether it includes the changes from https://github.com/project-chip/connectedhomeip/pull/28524, I am uncertain. I attempted to track this by utilizing GDB (bt) with the virtual device examples (lighting and locker). Interestingly, providing a parameter of 0 or {} appears to trigger distinct translation functions. Furthermore, during virtual device testing, any parameter, such as 3, results in the deletion of IPK.

bzbarsky-apple commented 1 year ago

Regarding whether it includes the changes from https://github.com/project-chip/connectedhomeip/pull/28524, I am uncertain

That's why you should include the SHA.

https://github.com/project-chip/connectedhomeip/pull/28524 merged in ceb14fda9bd3bbd200ab88394cb6b19f35bcceae

% git merge-base ceb14fda9bd3bbd200ab88394cb6b19f35bcceae e75d35a7581ced7152a4642e070c21f942ab66d7
e75d35a7581ced7152a4642e070c21f942ab66d7

so e75d35a7581ced7152a4642e070c21f942ab66d7 predates the fix. v1.1.0 is even older.

Duplicate of https://github.com/project-chip/connectedhomeip/issues/28518