Issue with device:set_field/get_field persisting/leaking data during "driver switch"

csstup · December 24, 2022, 5:08pm

Using the form device:set_field("test_persistent_key", 47, {persist = true}) allows a driver to persist data to flash as documented. However, I’ve seen some possibly unintended behaviour when changing between drivers for a device.

A field value by key “K” is persisted by driver “A”.
The user changes to another driver “B”.
Driver “B” can read the values that driver “A” persisted if it uses the same key “K”.
If driver “B” saves new values stored by key “K”, if changing back to driver “A” it will see incorrect data.

The issues I’ve seen:

Its possible for sensitive data to be leaked by a driver and read by another driver if the same key is used either by accident or intentionally.
When a driver is changed to (using the driverChanged lifecycle event) its important to reset any of the set_field persisted values to empty/nil/known values as they could have been changed by another driver. One example I’ve seen is with device:get_field(constants.ELECTRICAL_MEASUREMENT_MULTIPLIER_KEY)where another driver could have set a value and its not “empty” or valid upon changing to my driver.

@nayelyz Is the intended behaviour for set_field() with persistence to live thru driver changes? If so, it may be good to document that fact in the reference guide. I was surprised that all set_field() persisted data wasn’t cleared when changing to a new driver.

Mariano_Colmenarejo · December 24, 2022, 6:00pm

Hi @csstup
After many tests to initialize the drivers after different scenarios, hub reboot, driver update, driver switched…
My experience with set_field, persist = true, is that when you change the driver they are all deleted, even if it is the exact same driver with a different id.

That’s why I use the expression if get_fied(key) == nil then to set the permanent variables to startup values on first install and when switching drivers.

In fact, in the example you give of the multiplier and divisor attributes, the stock drivers initialize that value in the doConfugure life cycle, which is not executed with the driverSwitched life cycle and is causing devices that measure power and energy not to be configured correctly when they change to Zigbee switch stock driver. Some users have to uninstall and reinstall the device for them to report power and energy.

In my driver this is set in the init lifecycle

Merry Christmas

csstup · December 24, 2022, 6:02pm

I am not seeing that at all.

I test with device:set_field("test_persistent_key", 47, {persist = true}) in my driver. Change to a stock switch driver. Change back to my driver. device:get_field("test_persistent_key") has a value of 47 still.

Mariano_Colmenarejo · December 24, 2022, 6:21pm

could be, they will have changed It again, without warning.
some complained this issue as @TAustin , I think.
I had to modify several drivers for that reason for they initialize correctly.

Just yesterday a user, who changed a smartthings plug to the stock Zigbee switch driver, had to uninstall it for the power to work

Mariano_Colmenarejo · December 25, 2022, 1:03pm

Hi @csstup

I just tested it in case it has changed the way set_field(persist = true) is handled and if it could affect the initialization of my drivers.

I have made a copy of the zigbee switch driver Mc called TEST and I have put some prints in the log so that it shows the last saved value of total energy: device:set_field("energy_Total_persist", energy_Total, {persist = true}).

This is the value printed in the log

2022-12-25T12:37:55.586466217+00:00 INFO Zigbee Switch Mc  <ZigbeeDevice: 0a4b023f-c186-43a2-b025-938277836195 [0x4B10] (Lidl Plug)> emitting event: {"attribute_id":"energy","capability_id":"energyMeter","component_id":"main","state":{"unit":"kWh","value":0.019}}
2022-12-25T12:37:55.619409217+00:00 PRINT Zigbee Switch Mc  function save_energy: set_field("energy_Total_persist", energy_Total, {persist = true})=       0.018611111111111

I have made a change of driver to another that is the TEST copy

2022-12-25T12:40:33.761295758+00:00 TRACE Zigbee Switch Mc  Received event with handler device_lifecycle
2022-12-25T12:40:33.762866758+00:00 INFO Zigbee Switch Mc  <ZigbeeDevice: 0a4b023f-c186-43a2-b025-938277836195 [0x4B10] (Lidl Plug)> received lifecycle event: removed

I have changed back to the original driver
Since it detects and prints a null value for device:get_field(“energy_Total_persist”) in the “init” lifecycle then resets it to value =0.0 kW/h

2022-12-25T12:41:15.720576778+00:00 PRINT Zigbee Switch Mc  function INIT: get_field("energy_Total_persist") =
2022-12-25T12:41:15.731908111+00:00 INFO Zigbee Switch Mc  <ZigbeeDevice: 0a4b023f-c186-43a2-b025-938277836195 [0x4B10] (Lidl Plug)> emitting event: {"attribute_id":"energyReset","capability_id":"legendabsolute60149.energyReset1","component_id":"main","state":{"value":"Last: 0.000 kWh (12/25/2022)"}}

This is on a v3 hub, with firmware 46.5

csstup · December 25, 2022, 3:11pm

Not what I’m experiencing on a production firmware 45.11 on a V2 hub.

Here’s my test. In my init handler:

local function do_init(self, device, event, args)

  local value = device:get_field("test_key")
  value = value and value or "nil"
  print ("field value on pre init: " .. value)

  device:set_field("test_key", 47, {persist = true})

  value = device:get_field("test_key")
  value = value and value or "nil"
  print ("field value on post init: " .. value)

And the results:

[ Installing the driver for a device, runs init for the first time, pre shows nil as expected.  post shows 47 as expected.]

2022-12-25T15:00:08.513723270+00:00 PRINT field value on pre init: nil
2022-12-25T15:00:08.517282145+00:00 PRINT field value on post init: 47

[changed to generic switch driver]
2022-12-25T15:01:02.558569027+00:00 INFO  <ZigbeeDevice: 74827aa5-6140-4e2a-aeef-d4d7f7e43b4b [0x4CD5] (Deep Freezer)> received lifecycle event: removed

[ changed back to test driver, both get_field() return 47.  PRE SHOULD BE NIL]
2022-12-25T15:01:41.925731906+00:00 PRINT  field value on pre init: 47
2022-12-25T15:01:41.928766531+00:00 PRINT  field value on post init: 47

Super easy to duplicate. I’ll try the test on my development 46.5 V2 hub tomorrow when I get access to it.

Mariano_Colmenarejo · December 25, 2022, 4:30pm

csstup:

local value = device:get_field("test_key")
  value = value and value or "nil"
  print ("field value on pre init: " .. value)

  device:set_field("test_key", 47, {persist = true})

  value = device:get_field("test_key")
  value = value and value or "nil"
  print ("field value on post init: " .. value)

I have copied and pasted your code into my driver, init lifecycle function and they don’t keep the set_field value. Same thing as my variables.
Something works differently on my v3 hub with firmware 46.5

[first installation driver with your code in my init lifecycle]
2022-12-25T16:12:25.843115622+00:00 PRINT Zigbee Switch Mc  field value on pre init: nil
2022-12-25T16:12:25.844252289+00:00 PRINT Zigbee Switch Mc  field value on post init: 47

[first installation driver my variable se_field saved]
2022-12-25T16:12:25.845389955+00:00 PRINT Zigbee Switch Mc  function INIT: get_field("energy_Total_persist")=      0.0044444444444444


[driver switched to a TEST driver]
2022-12-25T16:13:27.222454064+00:00 TRACE Zigbee Switch Mc  Received event with handler device_lifecycle
2022-12-25T16:13:27.223512397+00:00 INFO Zigbee Switch Mc  <ZigbeeDevice: 0a4b023f-c186-43a2-b025-938277836195 [0x4B10] (Lidl Plug)> received lifecycle event: removed

[driver switched back with your code in init lifecycle values]
2022-12-25T16:15:01.052824075+00:00 PRINT Zigbee Switch Mc  field value on pre init: nil
2022-12-25T16:15:01.053979075+00:00 PRINT Zigbee Switch Mc  field value on post init: 47

[driver switched back with my set_field recovery in init lifecycle is nil too]
2022-12-25T16:15:01.055117075+00:00 PRINT Zigbee Switch Mc  function INIT: get_field("energy_Total_persist")=

TAustin · December 25, 2022, 8:25pm

Pretty concerning finding. For what its worth, I’ve not experienced this either, but that may just be because I wasn’t looking for it! Maybe something cached in the hub isn’t being cleared as it should be. Or maybe like everything on this platform, persisted fields variables may be flat-spaced - thus the importance of using very unique field names. They may not even be associated to a driver - kind of like device_network_ids…

I’ll be interested in the answer from ST.

csstup · December 28, 2022, 12:47am

Duplicated the same behaviour on both 45.011 and 46.05 firmware. When switching drivers the persistent fields are not cleared on either version.

After switching to a stock Zigbee switch driver and then back to my driver, in my init:

2022-12-28T00:40:27.880198176+00:00 PRINT Securify Peanut Plug  field value on pre init: 47
2022-12-28T00:40:27.880837093+00:00 PRINT Securify Peanut Plug  field value on post init: 47

Running the code:

local function do_init(self, device, event, args)

  local value = device:get_field("test_key")
  value = value and value or "nil"
  print ("field value on pre init: " .. value)

  device:set_field("test_key", 47, {persist = true})

  value = device:get_field("test_key")
  value = value and value or "nil"
  print ("field value on post init: " .. value)

No idea why @Mariano_Colmenarejo is seeing a different behaviour than me on both production and beta firmware.

Zach_Varberg · January 3, 2023, 3:08pm

Hey @csstup this is not something that should be possible. I see the example you shared uses the same value for both drivers, and is not a value from one driver showing up in the other driver. Can you share an example (or even better if you are willing to share the complete drivers you are using either here, or in a DM), where a value from one driver is making it into another driver?

csstup · January 3, 2023, 5:43pm

I may not be explaining myself correctly. In my example, I’m only creating one driver. The second driver involved is a stock driver (doesn’t matter which one).

Install device using my driver. All fields are empty as you’d expect for a brand new device being paired.
Switch to another driver. In my example, I’m switching to the stock Edge Zigbee Switch driver.
Switch BACK to my custom driver. At this point, I would expect that all persistent fields would be empty again, just as if you had paired it new at step 1. Instead, persistent field values set at step 1 are still set.

It’s super easy to duplicate.

local function do_init(self, device)
  local value = device:get_field("test_key")
  value = value and value or "nil"
  print ("field value of test_key on pre init: " .. value)

  device:set_field("test_key", 47, {persist = true})

  value = device:get_field("test_key")
  value = value and value or "nil"
  print ("field value of test_key on post init: " .. value)

When you pair the device at step 1, you get the output you expect.

the field has no value on the first get_field of “test_key”
the field’s value is set to 47, and persisted.
the field has a value of 47 on the second get_field of “test_key”.

2022-12-25T15:00:08.513723270+00:00 PRINT field value of test_key on pre init: nil
2022-12-25T15:00:08.517282145+00:00 PRINT field value of test_key on post init: 47

However, when you switch back to the original driver at step 3, you get the output that is not expected.

the field STILL has a value of 47 on the first get_field of “test_key”
the field’s value is set to 47, and persisted.
the field has a value of 47 on the second get_field of “test_key”.

2022-12-25T15:01:41.925731906+00:00 PRINT  field value of test_key on pre init: 47
2022-12-25T15:01:41.928766531+00:00 PRINT  field value of test_key on post init: 47

The issue I’m reporting is that after a driver switch, the persisted field values are not being cleared. The expected behaviour is that all fields have NO VALUE after a driver switch. Instead, values are persisting THRU driver switches.

Mariano_Colmenarejo · January 3, 2023, 7:05pm

Hi @Zach_Varberg

For what it’s worth,
One difference between the test carried out by @csstup, that the result is what is not expected, and mine, that the result is what is expected, is that @csstup uses a v2 hub and I use a v3

Zach_Varberg · January 3, 2023, 7:56pm

OK, so we aren’t ever leaking info from one driver to another, instead, the issue is just that some value set on the original driver is not being cleared appropriately when swapping away and back. What is the time difference between switching to the stock driver and switching back? One possibility is that there is some caching that isn’t being fully cleared out in a shorter turn around of switch and switch back, although that would still be surprising to me as it should be cleaned up.

Another potentially diagnostic info is are you seeing the device_lifecycle removed event happening in your driver when you first switch away from it?

csstup · January 3, 2023, 8:37pm

Well my point with leaking is that the driver in the middle could be able to read data that was persisted by the first driver. If it knows the keys, it can easily read data if its not forcefully being cleared between driver switch.

The time difference doesn’t seem to matter. I waited seconds between, I’ve waited minutes (5-10 between). I didn’t test hours between switches.

Yes. When switching away from my custom driver (step 2), I get the removed event. Everything about the 2 switches are clean, it runs thru all the lifecycle events as documented. What doesn’t happen is the clearing of any persisted field data.

RBoy · January 3, 2023, 8:55pm

Having said that there is a strong case to formalize a way to persist data between drivers changes. Some security standard drivers like locks should be able to switch drivers (like groovy does) without having to clear everything from the device and start over each time. Otherwise it will create havoc for upstream apps like SLGA. Plus to make things more complicated some locks like Schlage won’t accept reprogramming existing codes so when the upstream apps try to reprogram them it will continually fail. It would be nice to have a smoother transition experience for such drivers. Atleast allow the user to opt into the option to keep the existing data while installing the drivers so that they are aware of it and can choose it to avoid issues while switching from a stock to a custom driver (or vice versa)

csstup · January 3, 2023, 9:16pm

I agree with @RBoy.

One feature I would like is if a “reason” was being passed to the lifecycle event handlers so that one could programmatically decide what path to take.

For instance, some work needs to be done in the init handler, but if I knew in that handler what reason init was being called, I may be able to handle things slightly different. For instance, for device switches, you don’t always want to do work in the later deviceSwitched event, you are doing it as part of init. So if init was passed “this is for a device switch” reason, one could perform behaviour accordingly.

Zach_Varberg · January 3, 2023, 10:36pm

Ah, so there is a slight gap in the understanding of how the persistence is done here. The data is actually stored not on the device, but in a store for the driver, and just keyed on the device. There shouldn’t be a way to access those values from the intermediate driver, as there is not a mechanism for those values to move. So in the case of it still being visible in the original driver, it’s because the data hasn’t been cleared from that drivers datastore, but that doesn’t necessarily mean it is accessible from the intermediate driver. But since the devices ID won’t change between driver switches, if the device switches away, and back, but there is a bug in the clearing code, it would still be visible for the device in that original driver.

csstup · January 3, 2023, 11:11pm

Makes sense, that clears up my misconception that the data store followed the device across drivers. Thanks! So at least leaking information across drivers shouldn’t be possible.

Mariano_Colmenarejo · January 20, 2023, 12:52pm

Hi @Zach_Varberg @csstup

I have retested the @csstup code to see if the set_fiel persistent memory is maintained.
I had noticed that something did not work the same when switching between drivers.

-- test to set_fiel
  local value = device:get_field("test_key")
  value = value and value or "nil"
  print ("field value on pre init: " .. value)

  device:set_field("test_key", 47, {persist = true})

  value = device:get_field("test_key")
  value = value and value or "nil"
  print ("field value on post init: " .. value)

Surprise !!! Now my Hub works like @csstup, it keeps the values saved in persistent memory when you change to another driver and then reverts to the original driver.

[first installation driver after write code in my init lifecycle]
2023-01-20T11:58:01.427372822+00:00 PRINT Zigbee Switch TEST  get_latest_state_energy_Total >>>>>   2.972   table: 0x264c130
2023-01-20T11:58:01.438718822+00:00 PRINT Zigbee Switch TEST  energy_Total >>>>>
2023-01-20T11:58:01.440301155+00:00 PRINT Zigbee Switch TEST  energy_Total_persist >>>>>        2.9723611111111

2023-01-20T11:58:01.443014155+00:00 PRINT Zigbee Switch TEST  field value on pre init: nil
2023-01-20T11:58:01.444344489+00:00 PRINT Zigbee Switch TEST  field value on post init: 47

[driver switched to a different zigbee switch driver]
2023-01-20T11:58:58.623458849+00:00 TRACE Zigbee Switch TEST  Received event with handler device_lifecycle
2023-01-20T11:58:58.642719849+00:00 INFO Zigbee Switch TEST  <ZigbeeDevice: 0a4b023f-c186-43a2-b025-938277836195 [0xF312] (Lidl Plug)> received lifecycle event: removed

[driver switched to a previous zigbee switch TEST driver]
2023-01-20T11:59:31.082972216+00:00 PRINT Zigbee Switch TEST  get_latest_state_energy_Total >>>>>   2.972   table: 0x14493c8
2023-01-20T11:59:31.091606531+00:00 PRINT Zigbee Switch TEST  energy_Total >>>>>
2023-01-20T11:59:31.093035579+00:00 PRINT Zigbee Switch TEST  energy_Total_persist >>>>>        2.9723611111111

2023-01-20T11:59:31.095713584+00:00 PRINT Zigbee Switch TEST  field value on pre init: 47
2023-01-20T11:59:31.097001561+00:00 PRINT Zigbee Switch TEST  field value on post init: 47

Therefore, my question is: What is the expected correct behavior, the one that existed before, the values were not maintained when changing the driver or the current one that does maintain the values?

Even as you can see in the logs, it keeps the last values of the capabilities:
device:get_latest_state("main", capabilities.energyMeter.ID, capabilities.energyMeter.energy.NAME)

I kept in permanent memory the calculated value of the total energy calculated in driver and now it is also kept

I have also tried waiting up to 5 minutes to return to the original driver and the values are also maintained

Thanks

csstup · January 20, 2023, 5:52pm

Glad I’m not crazy!

Topic		Replies	Views
Maintain data in Edge device when moved to another driver Writing Edge Drivers	33	786	May 26, 2022
[Edge] device:set_field/get_field values viewable externally? Support	3	412	August 18, 2022
Persistent State with Edge Device Driver Support	10	496	January 24, 2023
Sharing an object across devices of one driver Writing Edge Drivers	4	215	August 30, 2022
Persistent Store and the "Cost in Wear" Writing Edge Drivers	2	203	April 13, 2024

Issue with device:set_field/get_field persisting/leaking data during "driver switch"

Related topics