GH-22: Fix Multi Channel Handling of Secure NIF #22

nkljajic · 2023-11-28T08:28:42Z

Change

Some S0 multichannel endpoints will not respond to multichannel encapsulated SECURITY_COMMANDS_SUPPORTED_GET command [ 60 0D 00 01 98 02 ]. In such case, copy endpoint NIF to Secure NIF.

For reference, see document:
https://sdomembers.z-wavealliance.org/document/dl/652
Z-Wave Transport-Encapsulation Command Class Specification
3.5.4.1 Multi Channel Handling
Page 46

The implicit rule that all non-secure command classes for an End Point must be controllable securely is still in effect, if the endpoint is reported secure.

Checklist

A Contribution License Agreement has been established between @SiliconLabs and author's company (matching email domain)

rzr

Build: https://github.com/nkljajic/UnifySDK/actions/runs/7016067209/job/19086528904
Forwarded-SiliconLabs: task/UIC-3082/phcoval/GH-22/develop

rzr · 2024-01-16T17:04:22Z

Tests might need to be revised, current results:

zwave_command_class_node_info_resolver_test.c:76:test_resolve_node_info_endpoint_4:FAIL: Expected 257 Was 258
zwave_command_class_node_info_resolver_test.c:76:test_resolve_node_info_endpoint_0:FAIL: Expected 257 Was 258
zwave_command_class_node_info_resolver_test.c:76:test_resolve_node_info_zpc:FAIL: Expected 257 Was 258
zwave_command_class_node_info_resolver_test.c:76:test_resolve_node_info_undefined_endpoint:FAIL: Expected 257 Was 258
zwave_command_class_node_info_resolver_test.c:76:test_resolve_secure_node_info_s2_key:FAIL: Expected 257 Was 258
zwave_command_class_node_info_resolver_test.c:76:test_resolve_secure_node_info_s0_key:FAIL: Expected 257 Was 258
zwave_command_class_node_info_resolver_test.c:76:test_resolve_secure_node_info_no_granted_key:FAIL: Expected 257 Was 258
zwave_command_class_node_info_resolver_test.c:76:test_creating_secure_nif_after_nif_granted_keys_updates:FAIL: Expected 257 Was 258
zwave_command_class_node_info_resolver_test.c:76:test_not_creating_secure_nif_for_zpc:FAIL: Expected 257 Was 258
zwave_command_class_node_info_resolver_test.c:76:test_not_creating_secure_nif_for_valueless_node_id:FAIL: Expected 257 Was 258
zwave_command_class_node_info_resolver_test.c:76:test_on_node_information_update:FAIL: Expected 257 Was 258
zwave_command_class_node_info_resolver_test.c:76:test_on_node_information_update_downgrade:FAIL: Expected 257 Was 258
zwave_command_class_node_info_resolver_test.c:76:test_on_nif_resolution_aborted:FAIL: Expected 257 Was 258
-----------------------
13 Tests 13 Failures 0 Ignored 
FAIL

Some S0 multichannel endpoints will not respond to multichannel encapsulated SECURITY_COMMANDS_SUPPORTED_GET command [ 60 0D 00 01 98 02 ]. In such case, copy endpoint NIF to Secure NIF. For reference, see: Z-Wave Transport-Encapsulation Command Class Specification 3.5.4.1 Multi Channel Handling The implicit rule that all non-secure command classes for an End Point must be controllable securely is still in effect, if the endpoint is reported secure. Signed-off-by: Nenad Kljajic <nkljajic@control4.com>

rzr · 2024-01-17T13:19:36Z

Let's try to resolve this on your branch:

nkljajic#2

rzr · 2024-01-17T16:13:58Z

don't bother looking we fixed it you can merge this PR:

nkljajic#2

Then expect an other PR from us that fix test like I've done before

SiliconLabsGH-14: Enable testing by default

Some S0 multichannel endpoints will not respond to multichannel encapsulated SECURITY_COMMANDS_SUPPORTED_GET command [ 60 0D 00 01 98 02 ]. In such case, copy endpoint NIF to Secure NIF. For reference, see: Z-Wave Transport-Encapsulation Command Class Specification 3.5.4.1 Multi Channel Handling The implicit rule that all non-secure command classes for an End Point must be controllable securely is still in effect, if the endpoint is reported secure. Signed-off-by: Nenad Kljajic <nkljajic@control4.com>

SiliconLabsGH-22 add a new callback on the give_up_listener on the ATTRIBUTE_ZWAVE_SECURE_NIF attribute causing the test case to fail. The test only expect this listener to be called on the non-secure NIF ATTRIBUTE_ZWAVE_NIF. This change make sure that the code is only executed in the context of ATTRIBUTE_ZWAVE_NIF and ignored for ATTRIBUTE_ZWAVE_SECURE_NIF as this test don't care about it. Bug-SiliconLabs: UIC-3082 Relate-to: SiliconLabs#22 Forwarded-SiliconLabs: task/UIC-3082/phcoval/SiliconLabsGH-22/develop

silabs-borisl · 2024-01-18T10:17:13Z

We should be able to merge ;

https://github.com/silabs-borisl/UnifySDK/actions/runs/7568335791/job/20609337322

Thomasdjb · 2024-01-18T13:42:25Z

applications/zpc/components/zwave_command_classes/src/zwave_command_class_node_info_resolver.c

+  attribute_store_node_t node_id_node
+    = attribute_store_get_first_parent_with_type(secure_nif_node, ATTRIBUTE_NODE_ID);
+
+  if (get_zpc_node_id_node() == node_id_node) {


nitpicking: refactor to avoid creating node_id_node

compiler will optimize that extra variable ? will it ?

Thomasdjb · 2024-01-18T13:43:32Z

applications/zpc/components/zwave_command_classes/src/zwave_command_class_node_info_resolver.c

+                                                        REPORTED_ATTRIBUTE,
+                                                        nif,
+                                                        &nif_length))) {
+      uint8_t j=0;


nitpicking: add space before and after =

Let's do this in a next patch (clang format will help)

Bug-SiliconLabs: UIC-3082 Relate-to: SiliconLabs#22 Forwarded-SiliconLabs: task/UIC-3082/phcoval/SiliconLabsGH-22/develop Signed-off-by: Philippe Coval <philippe.coval@silabs.com>

Bug-SiliconLabs: UIC-3082 Relate-to: SiliconLabs#22 Forwarded-SiliconLabs: task/UIC-3082/phcoval/SiliconLabsGH-22/develop Origin: silabs-borisl#2 Signed-off-by: Philippe Coval <philippe.coval@silabs.com>

SiliconLabsGH-22: Lint code for SonarCube

rzr · 2024-01-22T15:43:36Z

Origin-SiliconLabs: ver_1.5.0-unstable-107-g1eb22d3fd

SiliconLabsGH-22: Fix zwave_command_class_node_info_resolver unit test

Some S0 multichannel endpoints will not respond to multichannel encapsulated SECURITY_COMMANDS_SUPPORTED_GET command [ 60 0D 00 01 98 02 ]. In such case, copy endpoint NIF to Secure NIF. For reference, see: Z-Wave Transport-Encapsulation Command Class Specification 3.5.4.1 Multi Channel Handling The implicit rule that all non-secure command classes for an End Point must be controllable securely is still in effect, if the endpoint is reported secure. Signed-off-by: Nenad Kljajic <nkljajic@control4.com>

SiliconLabsGH-22 add a new callback on the give_up_listener on the ATTRIBUTE_ZWAVE_SECURE_NIF attribute causing the test case to fail. The test only expect this listener to be called on the non-secure NIF ATTRIBUTE_ZWAVE_NIF. This change make sure that the code is only executed in the context of ATTRIBUTE_ZWAVE_NIF and ignored for ATTRIBUTE_ZWAVE_SECURE_NIF as this test don't care about it. Bug-SiliconLabs: UIC-3082 Relate-to: SiliconLabs#22 Forwarded-SiliconLabs: task/UIC-3082/phcoval/SiliconLabsGH-22/develop

Bug-SiliconLabs: UIC-3082 Relate-to: SiliconLabs#22 Forwarded-SiliconLabs: task/UIC-3082/phcoval/SiliconLabsGH-22/develop Origin: silabs-borisl#2 Signed-off-by: Philippe Coval <philippe.coval@silabs.com>

Some S0 multichannel endpoints will not respond to multichannel encapsulated SECURITY_COMMANDS_SUPPORTED_GET command [ 60 0D 00 01 98 02 ]. In such case, copy endpoint NIF to Secure NIF. For reference, see: Z-Wave Transport-Encapsulation Command Class Specification 3.5.4.1 Multi Channel Handling The implicit rule that all non-secure command classes for an End Point must be controllable securely is still in effect, if the endpoint is reported secure. Signed-off-by: Nenad Kljajic <nkljajic@control4.com>

GH-22 add a new callback on the give_up_listener on the ATTRIBUTE_ZWAVE_SECURE_NIF attribute causing the test case to fail. The test only expect this listener to be called on the non-secure NIF ATTRIBUTE_ZWAVE_NIF. This change make sure that the code is only executed in the context of ATTRIBUTE_ZWAVE_NIF and ignored for ATTRIBUTE_ZWAVE_SECURE_NIF as this test don't care about it. Bug-SiliconLabs: UIC-3082 Relate-to: #22 Forwarded-SiliconLabs: task/UIC-3082/phcoval/GH-22/develop

Bug-SiliconLabs: UIC-3082 Relate-to: #22 Forwarded-SiliconLabs: task/UIC-3082/phcoval/GH-22/develop Origin: silabs-borisl#2 Signed-off-by: Philippe Coval <philippe.coval@silabs.com>

rzr approved these changes Jan 15, 2024

View reviewed changes

nkljajic added a commit to nkljajic/UnifySDK that referenced this pull request Jan 17, 2024

Merge pull request #2 from rzr/SiliconLabsGH-22/UIC-3082/phcoval/main

5a26493

SiliconLabsGH-14: Enable testing by default

nkljajic force-pushed the fix-mc-secure-nif branch from 5a26493 to cbba778 Compare January 17, 2024 17:25

nkljajic force-pushed the fix-mc-secure-nif branch from cbba778 to 84733be Compare January 17, 2024 17:26

silabs-borisl mentioned this pull request Jan 18, 2024

GH-22: Fix zwave_command_class_node_info_resolver unit test nkljajic/UnifySDK#3

Merged

1 task

Thomasdjb approved these changes Jan 18, 2024

View reviewed changes

rzr mentioned this pull request Jan 22, 2024

GH-22: Lint code for SonarCube silabs-borisl/UnifySDK#2

Merged

silabs-borisl added a commit to silabs-borisl/UnifySDK that referenced this pull request Jan 22, 2024

Merge pull request #2 from rzr/SiliconLabsGH-22/UIC-3082/phcoval/main

a4a11cd

SiliconLabsGH-22: Lint code for SonarCube

silabs-borisl added a commit to silabs-borisl/UnifySDK that referenced this pull request Jan 22, 2024

SiliconLabsGH-22: Add test coverage for new function

92ea570

nkljajic added a commit to nkljajic/UnifySDK that referenced this pull request Jan 23, 2024

Merge pull request #3 from silabs-borisl/fix-mc-secure-nif

8ebdf7f

SiliconLabsGH-22: Fix zwave_command_class_node_info_resolver unit test

nkljajic and others added 4 commits January 23, 2024 16:20

SiliconLabsGH-22: Lint code for SonarCube

9132d78

Bug-SiliconLabs: UIC-3082 Relate-to: SiliconLabs#22 Forwarded-SiliconLabs: task/UIC-3082/phcoval/SiliconLabsGH-22/develop Origin: silabs-borisl#2 Signed-off-by: Philippe Coval <philippe.coval@silabs.com>

SiliconLabsGH-22: Add test coverage for new function

1a34f42

nkljajic force-pushed the fix-mc-secure-nif branch from 8ebdf7f to 1a34f42 Compare January 23, 2024 15:21

agarwal57 merged commit f172eca into SiliconLabs:main Feb 13, 2024

agarwal57 pushed a commit that referenced this pull request Feb 13, 2024

GH-22: Lint code for SonarCube

7a72adf

Bug-SiliconLabs: UIC-3082 Relate-to: #22 Forwarded-SiliconLabs: task/UIC-3082/phcoval/GH-22/develop Origin: silabs-borisl#2 Signed-off-by: Philippe Coval <philippe.coval@silabs.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-22: Fix Multi Channel Handling of Secure NIF #22

GH-22: Fix Multi Channel Handling of Secure NIF #22

nkljajic commented Nov 28, 2023 •

edited

Loading

rzr left a comment

rzr commented Jan 16, 2024

rzr commented Jan 17, 2024

rzr commented Jan 17, 2024 •

edited

Loading

silabs-borisl commented Jan 18, 2024

Thomasdjb Jan 18, 2024

rzr Jan 22, 2024

Thomasdjb Jan 18, 2024

rzr Jan 19, 2024

rzr Jan 22, 2024

rzr commented Jan 22, 2024

GH-22: Fix Multi Channel Handling of Secure NIF #22

GH-22: Fix Multi Channel Handling of Secure NIF #22

Conversation

nkljajic commented Nov 28, 2023 • edited Loading

Change

Checklist

rzr left a comment

Choose a reason for hiding this comment

rzr commented Jan 16, 2024

rzr commented Jan 17, 2024

rzr commented Jan 17, 2024 • edited Loading

silabs-borisl commented Jan 18, 2024

Thomasdjb Jan 18, 2024

Choose a reason for hiding this comment

rzr Jan 22, 2024

Choose a reason for hiding this comment

Thomasdjb Jan 18, 2024

Choose a reason for hiding this comment

rzr Jan 19, 2024

Choose a reason for hiding this comment

rzr Jan 22, 2024

Choose a reason for hiding this comment

rzr commented Jan 22, 2024

nkljajic commented Nov 28, 2023 •

edited

Loading

rzr commented Jan 17, 2024 •

edited

Loading