zsmartsystems / com.zsmartsystems.zigbee

ZigBee Cluster Library Java framework supporting multiple dongles
Eclipse Public License 1.0
142 stars 88 forks source link

Explicitly disallow rejoins using a well-known key #1269

Closed triller-telekom closed 2 years ago

triller-telekom commented 2 years ago

Fixes #1268

Signed-off-by: Stefan Triller stefan.triller@telekom.de

cdjackson commented 2 years ago

Thanks @triller-telekom. I've not found what the default for this policy is (maybe you have worked this out?) so I'm unclear if this is a "breaking" change (ie if it changes the default). However, even if it's not breaking, I think we need to provide a mechanism to allow application level changes of this - potentially for short durations (there is a timer associated with this as well) to allow users, or system designers. to change this setting.

As discussed in the other thread, this probably should be done through the TransportConfigOption etc classes to keep things consistent. Do you want to add something to this PR to cover this?

triller-telekom commented 2 years ago

ZigBeeDongleEzsp does have a updateDefaultPolicy method where an application can set anything, like I mentioned in my comment https://github.com/zsmartsystems/com.zsmartsystems.zigbee/issues/1268#issuecomment-942076199

So I don't see any reason of not merging this.

cdjackson commented 2 years ago

This feels inconsistent to me. We provide methods to change other configuration in this area (ie joining / rejoining) through the TransportConfigOption. This provides a consistent way to configure the dongle from the framework perspective and allows the configuration and policies to be changed on the fly.

Also, the updateDefaultPolicy method only updates the policy in the default configuration - so this can only be configured prior to startup and doesn't allow someone (for example) to set a high security level, then reduce the security to make a some network change, and then increase the security again. It could only be done by restarting the system each time.

cdjackson commented 2 years ago

I have checked the default on this policy, and the default value is set to 00 so this will not be a breaking change and I'm therefore happy to merge.