Re: [PATCH BlueZ v3 3/3] mesh: Fix Key Ring permissions for local nodes

["Gix, Brian" <brian.gix@intel.com>]

On Thu, 2019-09-26 at 20:41 +0000, Stotland, Inga wrote:
> Hi Brian,
> 
> On Thu, 2019-09-26 at 11:14 -0700, Brian Gix wrote:
> > We do *not* automatically create populated key rings for imported or
> > joined nodes, 
> 
> Why not for Import()? Since both the DevKey and NetKey are in the
> possesion of the node...

There are two (known) use cases for Import()

1. Node previously existed on a different physical piece of hardware, and is being migrated to this daemon.

2. Node was newly provisioned Out-Of-Band, and this is the net result of the provisioning.

In *neither* case is it a given that the Node should be able to provision another node (the effect of adding
the Net Key to the key ring). In neither case is it a given that the Node should be able to modify it's own
Config Server states (the effect of adding it's Device Key to the key ring).

However, if it *is* the case that one or more of these operations should be available to the Node, the
Application that performed the Import may call the ImportSubnet() and/or the ImportRemoteNode() methods. In
actuality, if this was the intent (particularily on a Node being migrated) a key-ring of some sort should have
existed on the other physical piece of hardware.


The point of this larger patch-set is to no longer assume that every Node has the ability to use it's own
network keys to provision other nodes, or use it's own Device Key to change it's own Config Server states.

Yes, that can still happen (as it can with non BlueZ nodes) but it should be by a deliberate mechanism, not as
a "time saving default".



> > but we also do not *forbid* any node from adding a key
> > in it's possesion to the local key ring.
> > ---
> >  mesh/manager.c |  5 -----
> >  mesh/node.c    | 15 ---------------
> >  2 files changed, 20 deletions(-)
> > 
> > diff --git a/mesh/manager.c b/mesh/manager.c
> > index 501ec10fe..633597659 100644
> > --- a/mesh/manager.c
> > +++ b/mesh/manager.c
> > @@ -282,7 +282,6 @@ static struct l_dbus_message
> > *import_node_call(struct l_dbus *dbus,
> >  						void *user_data)
> >  {
> >  	struct mesh_node *node = user_data;
> > -	struct mesh_net *net = node_get_net(node);
> >  	struct l_dbus_message_iter iter_key;
> >  	uint16_t primary;
> >  	uint8_t num_ele;
> > @@ -298,10 +297,6 @@ static struct l_dbus_message
> > *import_node_call(struct l_dbus *dbus,
> >  		return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
> >  							"Bad device
> > key");
> >  
> > -	if (mesh_net_is_local_address(net, primary, num_ele))
> > -		return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
> > -					"Cannot overwrite local device
> > key");
> > -
> >  	if (!keyring_put_remote_dev_key(node, primary, num_ele, key))
> >  		return dbus_error(msg, MESH_ERROR_FAILED, NULL);
> >  
> > diff --git a/mesh/node.c b/mesh/node.c
> > index 833377e99..af45a6130 100644
> > --- a/mesh/node.c
> > +++ b/mesh/node.c
> > @@ -1681,7 +1681,6 @@ static void get_managed_objects_cb(struct
> > l_dbus_message *msg, void *user_data)
> >  
> >  	} else if (req->type == REQUEST_TYPE_IMPORT) {
> >  		struct node_import *import = req->import;
> > -		struct keyring_net_key net_key;
> >  
> >  		if (!create_node_config(node, node->uuid))
> >  			goto fail;
> > @@ -1692,23 +1691,9 @@ static void get_managed_objects_cb(struct
> > l_dbus_message *msg, void *user_data)
> >  					import->net_idx, import-
> > > net_key))
> >  			goto fail;
> >  
> > -		memcpy(net_key.old_key, import->net_key, 16);
> > -		net_key.net_idx = import->net_idx;
> > -		if (import->flags.kr)
> > -			net_key.phase = KEY_REFRESH_PHASE_TWO;
> > -		else
> > -			net_key.phase = KEY_REFRESH_PHASE_NONE;
> > -
> >  		/* Initialize directory for storing keyring info */
> >  		init_storage_dir(node);
> >  
> > -		if (!keyring_put_remote_dev_key(node, import->unicast,
> > -						num_ele, import-
> > > dev_key))
> > -			goto fail;
> > -
> > -		if (!keyring_put_net_key(node, import->net_idx,
> > &net_key))
> > -			goto fail;
> > -
> >  	} else {
> >  		/* Callback for create node request */
> >  		struct keyring_net_key net_key;
> 
>