From:   "=?utf-8?Q?Micha=C5=82?= Lowas-Rzechonek" 
        <michal.lowas-rzechonek@silvair.com>
Date:   Mon, 1 Apr 2019 12:42:13 +0200
To:     Brian Gix <brian.gix@intel.com>
Cc:     linux-bluetooth@vger.kernel.org, inga.stotland@intel.com,
        marcel@holtmann.org, luiz.dentz@gmail.com, johan.hedberg@gmail.com
Subject: Re: [PATCH BlueZ 1/1] mesh: Add APIs for Provisioner and Config
 Client
Message-ID: <20190401104213.vpqhgw55leg6olza@scytale>
Mail-Followup-To: Brian Gix <brian.gix@intel.com>,
        linux-bluetooth@vger.kernel.org, inga.stotland@intel.com,
        marcel@holtmann.org, luiz.dentz@gmail.com, johan.hedberg@gmail.com
References: <20190322225754.27039-1-brian.gix@intel.com>
 <20190322225754.27039-2-brian.gix@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20190322225754.27039-2-brian.gix@intel.com>
User-Agent: NeoMutt/20180716
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Brian,

A couple of comments regarding device key management and local/remote
provisioning flow in general.


On 03/22, Brian Gix wrote:
> +	uint64 token CreateNetwork(object app_root, array{byte}[16] uuid)
> +
> +		This is the first method that an application calls to become
> +		a Provisioner node, and a Configuration Client on a newly
> +		created Mesh Network.

What if I would like my node to act as a Config Client without being a
provisioner? In our application, nodes can obtain network details from a
remote storage, so you don't need to be the one provisioning the network
in order to have access to device keys.

I think we should distinguish creating a node within meshd and
provisioning it, as there are a couple of ways to provision a new node:

 - the node can advertise itself as unprovisioned device, waiting for
   PB-ADV
 - the node can advertise itself as a connectable, exposing PB-GATT
   service
 - the node might not want to advertise at all, and provision itself
   "out of band", getting provisioning information from a remote
   database

The 3rd option is of particular concern for new networks, and also
automatically adding devices to existing ones.

Because of that, I think we should rather split the
'org.bluez.mesh.Network1.Join' method into something like:


Mesh Network Hierarchy
======================
Service         org.bluez.mesh
Interface       org.bluez.mesh.Network1
Object path     /org/bluez/mesh

Methods:
        uint64 token
                Create(array{byte}[16] uuid)

            This is the first method that an application has to call to
            become a provisioned node on a mesh network. The call will
            allocate resources for the new node within the mesh daemon.

            The token parameter serves as a unique identifier of the
            particular node. The token must be preserved by the application
            in order to authenticate itself to the mesh daemon and attach to
            the network as a mesh node by calling Attach() method or
            permanently remove the identity of the mesh node by calling
            Remove() method.

            The created node is initially idle, particularly it doesn't 
            initiate broadcasting of Unprovisioned Device Beacon.

            (...)

        void Remove(uint64 token)

            This removes the configuration information about the mesh node
            identified by the 64-bit token parameter. The token parameter
            has been obtained as a result of successful Create() method call.


The application (after calling Attach) can then proceed to either
provision the node itself, or let it broadcast Unprovisioned Device
Beacons:


Mesh Node Hierarchy
===================
Service         org.bluez.mesh
Interface       org.bluez.mesh.Node1

        void Provision(array{byte}[16] network_key, uint16_t address)

            This call will add an unprovisioned local node to a given
            network, using provided primary unicast address.

        void Join(void)

            The call will initiate broadcasting of Unprovisioned Device
            Beacon.

            If regular bluetoothd is available on the system bus, mesh
            daemon will also register itself as a GATT application,
            providing PB-GATT service.

        void Cancel()

            Cancels an outstanding provisioning request initiated by Join()
            method.


After provisioning, the application will then receive freshly generate
device key, so that it can be stored externally.


Mesh Application Hierarchy
==========================
Service         unique name
Interface       org.bluez.mesh.Application1
Object path     <app_defined_root>

        void JoinComplete(array{byte}[16] device_key)


Having the application aware of the device key of a local node would
also mean that we could get rid of magic 0x7fff index for device key in
Send() call. Instead, we could provide two separate calls for
application-level and device-level security:

void SendWithApplicationKey(object element_path, uint16 destination,
                                uint16 key_index, array{byte} data)

void SendWithDeviceKey(object element_path, uint16 destination,
            					array{byte}[16] device_key, array{byte} data)

>  Mesh Node Hierarchy
>  ===================
> @@ -146,6 +178,88 @@ Methods:
>  			org.bluez.mesh.Error.InvalidArguments
>  			org.bluez.mesh.Error.NotFound
>  
> +	void SendWithDeviceKey(object element_path, uint16 destination,
> +					uint16 net_index, array{byte} data)
> +
> +		This method is used to send a message originated by a local
> +		model encoded with the device key of the remote node.
> +
> +		The element_path parameter is the object path of an element from
> +		a collection of the application elements (see Mesh Application
> +		Hierarchy section).
> +
> +		The destination parameter contains the destination address. This
> +		destination must be a uint16 to a unicast address, or a well
> +		known group address.

I don't think it makes sense to send messages using device keys to
non-unicast addresses.

Also, since mesh daemon might not be the one who originally provisioned
device we would like to communicate with, I think this method should
accept the device key as an argument, moving key management duties to
the application.

Also, because it's not possible to configure publications using a device
key, I think device keys make sense only in case of unicast *requests*.
As far as I understand the mesh architecture, such responses are the
only case where we are receiving messages encrypted with remote node's
device key.

Therefore, I think the mesh daemon might want to retain the device key
passed to SendWithDeviceKey call for a certain time period, in order to
process these response.

This would also mean that this call:

> +	void AddNode(array{byte}[16] uuid)
> +
> +		This method is used by the application that supports
> +		org.bluez.mesh.Provisioner1 interface to add the
> +		unprovisioned device specified by uuid, to the Network.
> +
> +		The uuid parameter is a 16-byte array that contains Device UUID
> +		of the unprovisioned device to be added to the network.

should return generated device key back to the caller, so that it can
store the device key for later use, and/or send it to the remote
storage, so that other config clients can retrieve/use them.

All of that relieves the mesh daemon from keeping the network database,
and gives us certain symmetry in the API wrt local vs. remote nodes.

What do you think?

regards
-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND