* [PATCH BlueZ] mesh: Add App and Net Key Refresh keyring back-end @ 2019-05-21 17:01 Brian Gix 2019-05-22 6:44 ` Stotland, Inga 0 siblings, 1 reply; 3+ messages in thread From: Brian Gix @ 2019-05-21 17:01 UTC (permalink / raw) To: linux-bluetooth; +Cc: inga.stotland, brian.gix Fill in skeleton App Key and Net Key KeyRing methods that are affected by the Key Refresh Procedure. --- doc/mesh-api.txt | 2 + mesh/manager.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 170 insertions(+), 30 deletions(-) diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt index 112990a5d..9644553e8 100644 --- a/doc/mesh-api.txt +++ b/doc/mesh-api.txt @@ -434,6 +434,7 @@ Methods: PossibleErrors: org.bluez.mesh.Error.InvalidArguments org.bluez.mesh.Error.DoesNotExist + org.bluez.mesh.Error.Busy void DeleteSubnet(uint16 net_index) @@ -527,6 +528,7 @@ Methods: PossibleErrors: org.bluez.mesh.Error.InvalidArguments org.bluez.mesh.Error.DoesNotExist + org.bluez.mesh.Error.Busy void DeleteAppKey(uint16 app_index) diff --git a/mesh/manager.c b/mesh/manager.c index d990ceec2..8ef32d735 100644 --- a/mesh/manager.c +++ b/mesh/manager.c @@ -24,9 +24,12 @@ #define _GNU_SOURCE #include <ell/ell.h> +#include "mesh/mesh-defs.h" #include "mesh/dbus.h" #include "mesh/error.h" #include "mesh/mesh.h" +#include "mesh/node.h" +#include "mesh/keyring.h" #include "mesh/manager.h" static struct l_dbus_message *add_node_call(struct l_dbus *dbus, @@ -86,51 +89,110 @@ static struct l_dbus_message *cancel_scan_call(struct l_dbus *dbus, return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); } +static struct l_dbus_message *store_new_subnet(struct mesh_node *node, + struct l_dbus_message *msg, + uint16_t net_idx, uint8_t *new_key) +{ + struct keyring_net_key key; + + if (net_idx > MAX_KEY_IDX) + return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); + + if (keyring_get_net_key(node, net_idx, &key)) { + /* Allow redundent calls only if they match */ + if (!memcmp(key.old_key, new_key, 16)) + return l_dbus_message_new_method_return(msg); + + return dbus_error(msg, MESH_ERROR_ALREADY_EXISTS, NULL); + } + + memcpy(key.old_key, new_key, 16); + key.net_idx = net_idx; + key.phase = KEY_REFRESH_PHASE_NONE; + + if (!keyring_put_net_key(node, net_idx, &key)) + return dbus_error(msg, MESH_ERROR_FAILED, NULL); + + return l_dbus_message_new_method_return(msg); +} + static struct l_dbus_message *create_subnet_call(struct l_dbus *dbus, struct l_dbus_message *msg, void *user_data) { + struct mesh_node *node = user_data; + uint8_t key[16]; uint16_t net_idx; - if (!l_dbus_message_get_arguments(msg, "q", &net_idx)) + if (!l_dbus_message_get_arguments(msg, "q", &net_idx) || !net_idx) return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); - /* TODO */ - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); + /* Generate key and submit */ + l_getrandom(key, sizeof(key)); + + return store_new_subnet(node, msg, net_idx, key); } static struct l_dbus_message *update_subnet_call(struct l_dbus *dbus, struct l_dbus_message *msg, void *user_data) { + struct mesh_node *node = user_data; + struct keyring_net_key key; uint16_t net_idx; - if (!l_dbus_message_get_arguments(msg, "q", &net_idx)) + if (!l_dbus_message_get_arguments(msg, "q", &net_idx) || + net_idx > MAX_KEY_IDX) return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); - /* TODO */ - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); + if (!keyring_get_net_key(node, net_idx, &key)) + return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST, NULL); + + switch (key.phase) { + case KEY_REFRESH_PHASE_NONE: + /* Generate Key and update phase */ + l_getrandom(key.new_key, sizeof(key.new_key)); + key.phase = KEY_REFRESH_PHASE_ONE; + if (!keyring_put_net_key(node, net_idx, &key)) + return dbus_error(msg, MESH_ERROR_FAILED, NULL); + + /* Fall Through */ + + case KEY_REFRESH_PHASE_ONE: + /* Allow redundent calls to start Key Refresh */ + return l_dbus_message_new_method_return(msg); + + default: + break; + } + + /* All other phases mean KR already in progress over-the-air */ + return dbus_error(msg, MESH_ERROR_BUSY, "KR in progress"); } static struct l_dbus_message *delete_subnet_call(struct l_dbus *dbus, struct l_dbus_message *msg, void *user_data) { + struct mesh_node *node = user_data; uint16_t net_idx; - if (!l_dbus_message_get_arguments(msg, "q", &net_idx)) + if (!l_dbus_message_get_arguments(msg, "q", &net_idx) || + net_idx > MAX_KEY_IDX) return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); - /* TODO */ - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); + keyring_del_net_key(node, net_idx); + + return l_dbus_message_new_method_return(msg); } static struct l_dbus_message *import_subnet_call(struct l_dbus *dbus, struct l_dbus_message *msg, void *user_data) { - uint16_t net_idx; + struct mesh_node *node = user_data; struct l_dbus_message_iter iter_key; + uint16_t net_idx; uint8_t *key; uint32_t n; @@ -142,55 +204,116 @@ static struct l_dbus_message *import_subnet_call(struct l_dbus *dbus, return dbus_error(msg, MESH_ERROR_INVALID_ARGS, "Bad network key"); - /* TODO */ - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); + return store_new_subnet(node, msg, net_idx, key); +} + +static struct l_dbus_message *store_new_appkey(struct mesh_node *node, + struct l_dbus_message *msg, + uint16_t net_idx, uint16_t app_idx, + uint8_t *new_key) +{ + struct keyring_net_key net_key; + struct keyring_app_key app_key; + + if (net_idx > MAX_KEY_IDX || app_idx > MAX_KEY_IDX) + return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); + + if (!keyring_get_net_key(node, net_idx, &net_key)) + return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST, NULL); + + if (keyring_get_app_key(node, app_idx, &app_key)) { + /* Allow redundent calls with identical data */ + if (!memcmp(app_key.old_key, new_key, 16) && + app_key.net_idx == net_idx) + return l_dbus_message_new_method_return(msg); + + return dbus_error(msg, MESH_ERROR_ALREADY_EXISTS, NULL); + } + + memcpy(app_key.old_key, new_key, 16); + app_key.net_idx = net_idx; + app_key.app_idx = app_idx; + + if (!keyring_put_app_key(node, app_idx, net_idx, &app_key)) + return dbus_error(msg, MESH_ERROR_FAILED, NULL); + + return l_dbus_message_new_method_return(msg); } static struct l_dbus_message *create_appkey_call(struct l_dbus *dbus, struct l_dbus_message *msg, void *user_data) { + struct mesh_node *node = user_data; uint16_t net_idx, app_idx; + uint8_t key[16]; if (!l_dbus_message_get_arguments(msg, "qq", &net_idx, &app_idx)) return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); - /* TODO */ - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); + l_getrandom(key, sizeof(key)); + + return store_new_appkey(node, msg, net_idx, app_idx, key); } static struct l_dbus_message *update_appkey_call(struct l_dbus *dbus, struct l_dbus_message *msg, void *user_data) { - uint16_t net_idx, app_idx; + struct mesh_node *node = user_data; + struct keyring_net_key net_key; + struct keyring_app_key app_key; + uint16_t app_idx; - if (!l_dbus_message_get_arguments(msg, "qq", &net_idx, &app_idx)) + if (!l_dbus_message_get_arguments(msg, "q", &app_idx) || + app_idx > MAX_KEY_IDX) return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); - /* TODO */ - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); + if (!keyring_get_app_key(node, app_idx, &app_key) || + !keyring_get_net_key(node, app_key.net_idx, &net_key)) + return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST, NULL); + + switch (net_key.phase) { + case KEY_REFRESH_PHASE_NONE: + case KEY_REFRESH_PHASE_ONE: + /* Generate Key if in acceptable phase */ + l_getrandom(app_key.new_key, sizeof(app_key.new_key)); + if (!keyring_put_app_key(node, app_idx, app_key.net_idx, + &app_key)) + return dbus_error(msg, MESH_ERROR_FAILED, NULL); + + return l_dbus_message_new_method_return(msg); + + default: + break; + } + + /* Don't allow Updates from invalid phase */ + return dbus_error(msg, MESH_ERROR_BUSY, "KR in progress"); } static struct l_dbus_message *delete_appkey_call(struct l_dbus *dbus, struct l_dbus_message *msg, void *user_data) { - uint16_t net_idx, app_idx; + struct mesh_node *node = user_data; + uint16_t app_idx; - if (!l_dbus_message_get_arguments(msg, "qq", &net_idx, &app_idx)) + if (!l_dbus_message_get_arguments(msg, "q", &app_idx)) return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); - /* TODO */ - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); + keyring_del_app_key(node, app_idx); + + return l_dbus_message_new_method_return(msg); } static struct l_dbus_message *import_appkey_call(struct l_dbus *dbus, struct l_dbus_message *msg, void *user_data) { - uint16_t net_idx, app_idx; + struct mesh_node *node = user_data; struct l_dbus_message_iter iter_key; + uint16_t net_idx, app_idx; uint8_t *key; uint32_t n; @@ -203,22 +326,37 @@ static struct l_dbus_message *import_appkey_call(struct l_dbus *dbus, return dbus_error(msg, MESH_ERROR_INVALID_ARGS, "Bad application key"); - /* TODO */ - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); + return store_new_appkey(node, msg, net_idx, app_idx, key); } static struct l_dbus_message *set_key_phase_call(struct l_dbus *dbus, struct l_dbus_message *msg, void *user_data) { + struct mesh_node *node = user_data; + struct keyring_net_key key; uint16_t net_idx; uint8_t phase; - if (!l_dbus_message_get_arguments(msg, "qy", &net_idx, &phase)) + if (!l_dbus_message_get_arguments(msg, "qy", &net_idx, &phase) || + phase == KEY_REFRESH_PHASE_ONE || + phase > KEY_REFRESH_PHASE_THREE) return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); - /* TODO */ - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); + if (!keyring_get_net_key(node, net_idx, &key)) + return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST, NULL); + + if (phase == KEY_REFRESH_PHASE_THREE && + key.phase != KEY_REFRESH_PHASE_NONE) { + memcpy(key.old_key, key.new_key, 16); + key.phase = KEY_REFRESH_PHASE_NONE; + } else + key.phase = phase; + + if (!keyring_put_net_key(node, net_idx, &key)) + return dbus_error(msg, MESH_ERROR_FAILED, NULL); + + return l_dbus_message_new_method_return(msg); } static void setup_management_interface(struct l_dbus_interface *iface) @@ -242,9 +380,9 @@ static void setup_management_interface(struct l_dbus_interface *iface) l_dbus_interface_method(iface, "CreateAppKey", 0, create_appkey_call, "", "qq", "", "net_index", "app_index"); l_dbus_interface_method(iface, "UpdateAppKey", 0, update_appkey_call, - "", "qq", "", "net_index", "app_index"); + "", "q", "", "app_index"); l_dbus_interface_method(iface, "DeleteAppKey", 0, delete_appkey_call, - "", "qq", "", "net_index", "app_index"); + "", "q", "", "app_index"); l_dbus_interface_method(iface, "ImportAppKey", 0, import_appkey_call, "", "qqay", "", "net_index", "app_index", "app_key"); -- 2.14.5 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH BlueZ] mesh: Add App and Net Key Refresh keyring back-end 2019-05-21 17:01 [PATCH BlueZ] mesh: Add App and Net Key Refresh keyring back-end Brian Gix @ 2019-05-22 6:44 ` Stotland, Inga 2019-05-22 16:20 ` Gix, Brian 0 siblings, 1 reply; 3+ messages in thread From: Stotland, Inga @ 2019-05-22 6:44 UTC (permalink / raw) To: linux-bluetooth, Gix, Brian [-- Attachment #1: Type: text/plain, Size: 13445 bytes --] Hi Brian, On Tue, 2019-05-21 at 10:01 -0700, Brian Gix wrote: > Fill in skeleton App Key and Net Key KeyRing methods that are > affected > by the Key Refresh Procedure. Would be nice to list the implemented methods here. I would strongly recommned breaking this in to two, maybe even three patches: net key methods, app key methods, phase method > --- > doc/mesh-api.txt | 2 + > mesh/manager.c | 198 > ++++++++++++++++++++++++++++++++++++++++++++++--------- > 2 files changed, 170 insertions(+), 30 deletions(-) > > diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt > index 112990a5d..9644553e8 100644 > --- a/doc/mesh-api.txt > +++ b/doc/mesh-api.txt > @@ -434,6 +434,7 @@ Methods: > PossibleErrors: > org.bluez.mesh.Error.InvalidArguments > org.bluez.mesh.Error.DoesNotExist > + org.bluez.mesh.Error.Busy > > void DeleteSubnet(uint16 net_index) > > @@ -527,6 +528,7 @@ Methods: > PossibleErrors: > org.bluez.mesh.Error.InvalidArguments > org.bluez.mesh.Error.DoesNotExist > + org.bluez.mesh.Error.Busy > > void DeleteAppKey(uint16 app_index) > > diff --git a/mesh/manager.c b/mesh/manager.c > index d990ceec2..8ef32d735 100644 > --- a/mesh/manager.c > +++ b/mesh/manager.c > @@ -24,9 +24,12 @@ > #define _GNU_SOURCE > #include <ell/ell.h> > > +#include "mesh/mesh-defs.h" > #include "mesh/dbus.h" > #include "mesh/error.h" > #include "mesh/mesh.h" > +#include "mesh/node.h" > +#include "mesh/keyring.h" > #include "mesh/manager.h" > > static struct l_dbus_message *add_node_call(struct l_dbus *dbus, > @@ -86,51 +89,110 @@ static struct l_dbus_message > *cancel_scan_call(struct l_dbus *dbus, > return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > } > > +static struct l_dbus_message *store_new_subnet(struct mesh_node > *node, > + struct l_dbus_message *msg, > + uint16_t net_idx, uint8_t > *new_key) > +{ > + struct keyring_net_key key; > + > + if (net_idx > MAX_KEY_IDX) > + return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > + > + if (keyring_get_net_key(node, net_idx, &key)) { > + /* Allow redundent calls only if they match */ Allow redundant calls only if the key values match > + if (!memcmp(key.old_key, new_key, 16)) > + return l_dbus_message_new_method_return(msg); > + > + return dbus_error(msg, MESH_ERROR_ALREADY_EXISTS, > NULL); > + } > + > + memcpy(key.old_key, new_key, 16); > + key.net_idx = net_idx; > + key.phase = KEY_REFRESH_PHASE_NONE; > + > + if (!keyring_put_net_key(node, net_idx, &key)) > + return dbus_error(msg, MESH_ERROR_FAILED, NULL); > + > + return l_dbus_message_new_method_return(msg); > +} > + > static struct l_dbus_message *create_subnet_call(struct l_dbus > *dbus, > struct l_dbus_message > *msg, > void *user_data) > { > + struct mesh_node *node = user_data; > + uint8_t key[16]; > uint16_t net_idx; > > - if (!l_dbus_message_get_arguments(msg, "q", &net_idx)) > + if (!l_dbus_message_get_arguments(msg, "q", &net_idx) || > !net_idx) Maybe use explicit value for net_idx check, i.e., net_idx == MESH_PRIMARY_NET_INDEX_DEFAULT Currently DEFAULT_PRIMARY_NET_INDEX is defined in node.c WE should move it into mesh-defs.h and rename accordingly > return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > > - /* TODO */ > - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > + /* Generate key and submit */ > + l_getrandom(key, sizeof(key)); > + > + return store_new_subnet(node, msg, net_idx, key); > } > > static struct l_dbus_message *update_subnet_call(struct l_dbus > *dbus, > struct l_dbus_message > *msg, > void *user_data) > { > + struct mesh_node *node = user_data; > + struct keyring_net_key key; > uint16_t net_idx; > > - if (!l_dbus_message_get_arguments(msg, "q", &net_idx)) > + if (!l_dbus_message_get_arguments(msg, "q", &net_idx) || > + net_idx > MAX_KEY_IDX) > return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > > - /* TODO */ > - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > + if (!keyring_get_net_key(node, net_idx, &key)) > + return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST, > NULL); > + > + switch (key.phase) { > + case KEY_REFRESH_PHASE_NONE: > + /* Generate Key and update phase */ > + l_getrandom(key.new_key, sizeof(key.new_key)); > + key.phase = KEY_REFRESH_PHASE_ONE; line break? > + if (!keyring_put_net_key(node, net_idx, &key)) > + return dbus_error(msg, MESH_ERROR_FAILED, > NULL); Wouldn't we want to revert the key phase back to KEY_REFRESH_PHASE_NONE in case of failure? > + > + /* Fall Through */ > + > + case KEY_REFRESH_PHASE_ONE: > + /* Allow redundent calls to start Key Refresh */ "redundant" > + return l_dbus_message_new_method_return(msg); > + > + default: > + break; > + } > + > + /* All other phases mean KR already in progress over-the-air */ > + return dbus_error(msg, MESH_ERROR_BUSY, "KR in progress"); KR -> Key refresh > } > > static struct l_dbus_message *delete_subnet_call(struct l_dbus > *dbus, > struct l_dbus_message > *msg, > void *user_data) > { > + struct mesh_node *node = user_data; > uint16_t net_idx; > > - if (!l_dbus_message_get_arguments(msg, "q", &net_idx)) > + if (!l_dbus_message_get_arguments(msg, "q", &net_idx) || > + net_idx > MAX_KEY_IDX) > return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > > - /* TODO */ > - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > + keyring_del_net_key(node, net_idx); > + > + return l_dbus_message_new_method_return(msg); > } > > static struct l_dbus_message *import_subnet_call(struct l_dbus > *dbus, > struct l_dbus_message > *msg, > void *user_data) > { > - uint16_t net_idx; > + struct mesh_node *node = user_data; > struct l_dbus_message_iter iter_key; > + uint16_t net_idx; > uint8_t *key; > uint32_t n; > > @@ -142,55 +204,116 @@ static struct l_dbus_message > *import_subnet_call(struct l_dbus *dbus, > return dbus_error(msg, MESH_ERROR_INVALID_ARGS, > "Bad network > key"); > > - /* TODO */ > - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > + return store_new_subnet(node, msg, net_idx, key); > +} > + > +static struct l_dbus_message *store_new_appkey(struct mesh_node > *node, > + struct l_dbus_message *msg, > + uint16_t net_idx, uint16_t > app_idx, > + uint8_t *new_key) > +{ > + struct keyring_net_key net_key; > + struct keyring_app_key app_key; > + > + if (net_idx > MAX_KEY_IDX || app_idx > MAX_KEY_IDX) > + return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > + > + if (!keyring_get_net_key(node, net_idx, &net_key)) > + return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST, > NULL); NULL -> "Bound net key not found" Also, I don't think "org.bluez.mesh.Error.DoesNotExist" error is documented in mesh-api.txt for either CreateAppKey() or ImportAppKey(), needs to be added. > + > + if (keyring_get_app_key(node, app_idx, &app_key)) { > + /* Allow redundent calls with identical data */ "Allow redundant calls with identical values" > + if (!memcmp(app_key.old_key, new_key, 16) && > + app_key.net_idx == > net_idx) > + return l_dbus_message_new_method_return(msg); > + > + return dbus_error(msg, MESH_ERROR_ALREADY_EXISTS, > NULL); > + } > + > + memcpy(app_key.old_key, new_key, 16); > + app_key.net_idx = net_idx; > + app_key.app_idx = app_idx; > + > + if (!keyring_put_app_key(node, app_idx, net_idx, &app_key)) > + return dbus_error(msg, MESH_ERROR_FAILED, NULL); > + > + return l_dbus_message_new_method_return(msg); > } > > static struct l_dbus_message *create_appkey_call(struct l_dbus > *dbus, > struct l_dbus_message > *msg, > void *user_data) > { > + struct mesh_node *node = user_data; > uint16_t net_idx, app_idx; > + uint8_t key[16]; > > if (!l_dbus_message_get_arguments(msg, "qq", &net_idx, > &app_idx)) > return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > > - /* TODO */ > - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > + l_getrandom(key, sizeof(key)); > + > + return store_new_appkey(node, msg, net_idx, app_idx, key); > } > > static struct l_dbus_message *update_appkey_call(struct l_dbus > *dbus, > struct l_dbus_message > *msg, > void *user_data) > { > - uint16_t net_idx, app_idx; > + struct mesh_node *node = user_data; > + struct keyring_net_key net_key; > + struct keyring_app_key app_key; > + uint16_t app_idx; > > - if (!l_dbus_message_get_arguments(msg, "qq", &net_idx, > &app_idx)) > + if (!l_dbus_message_get_arguments(msg, "q", &app_idx) || > + app_idx > MAX_KEY_IDX) > return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > > - /* TODO */ > - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > + if (!keyring_get_app_key(node, app_idx, &app_key) || > + !keyring_get_net_key(node, app_key.net_idx, > &net_key)) > + return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST, > NULL); > + > + switch (net_key.phase) { > + case KEY_REFRESH_PHASE_NONE: > + case KEY_REFRESH_PHASE_ONE: What if the net key is in key refresh phase one and This method has been called mre than once? i.e., do we want to protect the application from potentially overwriting already updated application key? > + /* Generate Key if in acceptable phase */ > + l_getrandom(app_key.new_key, sizeof(app_key.new_key)); line break > + if (!keyring_put_app_key(node, app_idx, > app_key.net_idx, > + &app_ke > y)) > + return dbus_error(msg, MESH_ERROR_FAILED, > NULL); > + > + return l_dbus_message_new_method_return(msg); > + > + default: > + break; > + } > + > + /* Don't allow Updates from invalid phase */ > + return dbus_error(msg, MESH_ERROR_BUSY, "KR in progress"); KR -> Key refresh > } > > static struct l_dbus_message *delete_appkey_call(struct l_dbus > *dbus, > struct l_dbus_message > *msg, > void *user_data) > { > - uint16_t net_idx, app_idx; > + struct mesh_node *node = user_data; > + uint16_t app_idx; > > - if (!l_dbus_message_get_arguments(msg, "qq", &net_idx, > &app_idx)) > + if (!l_dbus_message_get_arguments(msg, "q", &app_idx)) > return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > > - /* TODO */ > - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > + keyring_del_app_key(node, app_idx); > + > + return l_dbus_message_new_method_return(msg); > } > > static struct l_dbus_message *import_appkey_call(struct l_dbus > *dbus, > struct l_dbus_message > *msg, > void *user_data) > { > - uint16_t net_idx, app_idx; > + struct mesh_node *node = user_data; > struct l_dbus_message_iter iter_key; > + uint16_t net_idx, app_idx; > uint8_t *key; > uint32_t n; > > @@ -203,22 +326,37 @@ static struct l_dbus_message > *import_appkey_call(struct l_dbus *dbus, > return dbus_error(msg, MESH_ERROR_INVALID_ARGS, > "Bad > application key"); > > - /* TODO */ > - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > + return store_new_appkey(node, msg, net_idx, app_idx, key); > } > > static struct l_dbus_message *set_key_phase_call(struct l_dbus > *dbus, > struct l_dbus_message > *msg, > void *user_data) > { > + struct mesh_node *node = user_data; > + struct keyring_net_key key; > uint16_t net_idx; > uint8_t phase; > > - if (!l_dbus_message_get_arguments(msg, "qy", &net_idx, &phase)) > + if (!l_dbus_message_get_arguments(msg, "qy", &net_idx, &phase) > || > + phase == KEY_REFRESH_PHASE_ONE > || > + phase > > KEY_REFRESH_PHASE_THREE) > return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > > - /* TODO */ > - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > + if (!keyring_get_net_key(node, net_idx, &key)) > + return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST, > NULL); > + > + if (phase == KEY_REFRESH_PHASE_THREE && > + key.phase != > KEY_REFRESH_PHASE_NONE) { > + memcpy(key.old_key, key.new_key, 16); > + key.phase = KEY_REFRESH_PHASE_NONE; > + } else > + key.phase = phase; > + > + if (!keyring_put_net_key(node, net_idx, &key)) > + return dbus_error(msg, MESH_ERROR_FAILED, NULL); Restore the phase to the previous value in case of failure? > + > + return l_dbus_message_new_method_return(msg); > } > > static void setup_management_interface(struct l_dbus_interface > *iface) > @@ -242,9 +380,9 @@ static void setup_management_interface(struct > l_dbus_interface *iface) > l_dbus_interface_method(iface, "CreateAppKey", 0, > create_appkey_call, > "", "qq", "", "net_index", > "app_index"); > l_dbus_interface_method(iface, "UpdateAppKey", 0, > update_appkey_call, > - "", "qq", "", "net_index", > "app_index"); > + "", "q", "", "app_index"); > l_dbus_interface_method(iface, "DeleteAppKey", 0, > delete_appkey_call, > - "", "qq", "", "net_index", > "app_index"); > + "", "q", "", "app_index"); > l_dbus_interface_method(iface, "ImportAppKey", 0, > import_appkey_call, > "", "qqay", "", "net_index", > "app_index", > "app_ke > y"); Thank you, Inga [-- Attachment #2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 3265 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH BlueZ] mesh: Add App and Net Key Refresh keyring back-end 2019-05-22 6:44 ` Stotland, Inga @ 2019-05-22 16:20 ` Gix, Brian 0 siblings, 0 replies; 3+ messages in thread From: Gix, Brian @ 2019-05-22 16:20 UTC (permalink / raw) To: linux-bluetooth, Stotland, Inga Hi Inga... I will correct the various spelling issues, and perhaps also segment the patches as you suggest On Tue, 2019-05-21 at 23:44 -0700, Stotland, Inga wrote: > On Tue, 2019-05-21 at 10:01 -0700, Brian Gix wrote: > > > > + struct mesh_node *node = user_data; > > + uint8_t key[16]; > > uint16_t net_idx; > > > > - if (!l_dbus_message_get_arguments(msg, "q", &net_idx)) > > + if (!l_dbus_message_get_arguments(msg, "q", &net_idx) || > > !net_idx) > > > Maybe use explicit value for net_idx check, i.e., > net_idx == MESH_PRIMARY_NET_INDEX_DEFAULT > > Currently DEFAULT_PRIMARY_NET_INDEX is defined in node.c > WE should move it into mesh-defs.h and rename accordingly I can do this... Although I don't know that we will ever have any other Net Index except 0 ever be defined by the spec as "Primary". > > > - if (!l_dbus_message_get_arguments(msg, "q", &net_idx)) > > + if (!l_dbus_message_get_arguments(msg, "q", &net_idx) || > > + net_idx > MAX_KEY_IDX) > > return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > > > > - /* TODO */ > > - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > > + if (!keyring_get_net_key(node, net_idx, &key)) > > + return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST, > > NULL); > > + > > + switch (key.phase) { > > + case KEY_REFRESH_PHASE_NONE: > > + /* Generate Key and update phase */ > > + l_getrandom(key.new_key, sizeof(key.new_key)); > > + key.phase = KEY_REFRESH_PHASE_ONE; > > line break? > > > + if (!keyring_put_net_key(node, net_idx, &key)) > > + return dbus_error(msg, MESH_ERROR_FAILED, > > NULL); > > Wouldn't we want to revert the key phase back to KEY_REFRESH_PHASE_NONE > in case of failure? I don't think this is neccessary. This is the only place in the function where a *Change* is made to the persistent data... If it fails then it fails, and the old data will still be there. > > > +static struct l_dbus_message *store_new_appkey(struct mesh_node > > *node, > > + struct l_dbus_message *msg, > > + uint16_t net_idx, uint16_t > > app_idx, > > + uint8_t *new_key) > > +{ > > + struct keyring_net_key net_key; > > + struct keyring_app_key app_key; > > + > > + if (net_idx > MAX_KEY_IDX || app_idx > MAX_KEY_IDX) > > + return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > > + > > + if (!keyring_get_net_key(node, net_idx, &net_key)) > > + return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST, > > NULL); > > NULL -> "Bound net key not found" > Also, I don't think "org.bluez.mesh.Error.DoesNotExist" error is > documented in mesh-api.txt for either CreateAppKey() or ImportAppKey(), > needs to be added. Will add > > > > > static struct l_dbus_message *update_appkey_call(struct l_dbus > > *dbus, > > struct l_dbus_message > > *msg, > > void *user_data) > > { > > - uint16_t net_idx, app_idx; > > + struct mesh_node *node = user_data; > > + struct keyring_net_key net_key; > > + struct keyring_app_key app_key; > > + uint16_t app_idx; > > > > - if (!l_dbus_message_get_arguments(msg, "qq", &net_idx, > > &app_idx)) > > + if (!l_dbus_message_get_arguments(msg, "q", &app_idx) || > > + app_idx > MAX_KEY_IDX) > > return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > > > > - /* TODO */ > > - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > > + if (!keyring_get_app_key(node, app_idx, &app_key) || > > + !keyring_get_net_key(node, app_key.net_idx, > > &net_key)) > > + return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST, > > NULL); > > + > > + switch (net_key.phase) { > > + case KEY_REFRESH_PHASE_NONE: > > + case KEY_REFRESH_PHASE_ONE: > > What if the net key is in key refresh phase one and This method has > been called mre than once? i.e., do we want to protect the application > from potentially overwriting already updated application key? Well, Application keys don't actually have a phase... It is inherited from the bound net key. The controlling Config Client app needs to set up *all* the updates in the keyring before starting to send the updates to the remote nodes. We can warn in the mesh-api.txt the correct order of things, but as long as the App does not start sending the updates over-the-air, there is no problem with Updating the key-ring's App Key redundantly prior to starting to propagate. As long as the *spec* is followed, we don't have a problem. > > > > > static struct l_dbus_message *set_key_phase_call(struct l_dbus > > *dbus, > > struct l_dbus_message > > *msg, > > void *user_data) > > { > > + struct mesh_node *node = user_data; > > + struct keyring_net_key key; > > uint16_t net_idx; > > uint8_t phase; > > > > - if (!l_dbus_message_get_arguments(msg, "qy", &net_idx, &phase)) > > + if (!l_dbus_message_get_arguments(msg, "qy", &net_idx, &phase) > > > > > > > > + phase == KEY_REFRESH_PHASE_ONE > > > > > > > > + phase > > > KEY_REFRESH_PHASE_THREE) > > return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL); > > > > - /* TODO */ > > - return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED, NULL); > > + if (!keyring_get_net_key(node, net_idx, &key)) > > + return dbus_error(msg, MESH_ERROR_DOES_NOT_EXIST, > > NULL); > > + > > + if (phase == KEY_REFRESH_PHASE_THREE && > > + key.phase != > > KEY_REFRESH_PHASE_NONE) { > > + memcpy(key.old_key, key.new_key, 16); > > + key.phase = KEY_REFRESH_PHASE_NONE; > > + } else > > + key.phase = phase; > > + > > + if (!keyring_put_net_key(node, net_idx, &key)) > > + return dbus_error(msg, MESH_ERROR_FAILED, NULL); > > Restore the phase to the previous value in case of failure? As with the above comment, a failure is a failure... If this method fails for *any* reason, it means nothing about the keys were updated, so there is nothing to restore. > > > > Thank you, > > > Inga ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-05-22 16:20 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-21 17:01 [PATCH BlueZ] mesh: Add App and Net Key Refresh keyring back-end Brian Gix 2019-05-22 6:44 ` Stotland, Inga 2019-05-22 16:20 ` Gix, Brian
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).