Date:   Thu, 18 Apr 2019 11:27:38 +0200
From:   Michal Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
To:     "Gix, Brian" <brian.gix@intel.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "Stotland, Inga" <inga.stotland@intel.com>,
        "marcel@holtmann.org" <marcel@holtmann.org>,
        "johan.hedberg@gmail.com" <johan.hedberg@gmail.com>
Subject: Re: [PATCH BlueZ v5 1/1] mesh: Add APIs for Provisioner and Config
 Client
Message-ID: <20190418092738.wdohlkvcej5ffw5x@scytale>
Mail-Followup-To: "Gix, Brian" <brian.gix@intel.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "Stotland, Inga" <inga.stotland@intel.com>,
        "marcel@holtmann.org" <marcel@holtmann.org>,
        "johan.hedberg@gmail.com" <johan.hedberg@gmail.com>
References: <20190415194946.13121-1-brian.gix@intel.com>
 <20190415194946.13121-2-brian.gix@intel.com>
 <20190417055132.scxvzhusx2suasdv@kynes>
 <DEBB0CAA2616974FAE35E4B560B9A4376CAFE9DF@ORSMSX103.amr.corp.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <DEBB0CAA2616974FAE35E4B560B9A4376CAFE9DF@ORSMSX103.amr.corp.intel.com>
User-Agent: NeoMutt/20180716
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Brian,

On 04/17, Gix, Brian wrote:
> > Not sure what are the size constraints (if any), but I think it might be better
> > to pass the JSON as a string.
> Indeed, this has been discussed internally as well, and is still
> subject to the change you mention. We are still wait8ing for input
> from all stakeholders, and your preference is noted.
Ok, thanks.

> > > +	void AddNetKey
> > > +	void AddAppKey
> >
> > If I understand correctly, these are convenience functions for
> > SendWithDeviceKey, so that the application wouldn't need to construct
> > access payloads for configuration messages?
> > 
> > If so, I guess we will need more functions like that, ideally to cover all
> > messages mentioned in section 4.3.4 of the mesh profile?
> 
> These two functions are explicitly separated out from the rest of the
> section 4.3.4 messages in order to keep the security of the keys in
> the hands of the daemon.
Ok, understood.

> The messages where keys are explicitly sent over-the-air are the only
> messages we anticipate providing specific D-Bus methods for to
> minimize D-Bus API bloat.
> (...)
> The external Config Client App is responsible for:
> * deciding *when* to send App and Net key adds and updates
> * sending all pub/sub/binding/feature settings
>
> so in this case, the controlling Config Client App will be composing
> all Config Client messages (except for OTA key messages)
So if I understand correctly, to add a new application key one would
need to either:
 - call CreateAppKey to have the daemon generate a new application key
   internally.
 - manually create access layer payload for Config Client App Key Add
   message, then send it to the local node via loopack

After one of these operations, the key can be sent to remote nodes using
AddAppKey API.

This seems a bit assymetric: the application that would like to act as a
Config Client would need to use a mixture of manually constructed
access payloads (e.g. to configure binds and subscriptions) and a
higher-level APIs used to add keys.

I think it would be cleaner to have the application use just the API,
and relieve it of requirement to implement Config Client messages,
especially given the fact the daemon implements Config Server internally.

This would indeed make the API surface larger, which certainly is a
drawback, but I think it would make it easier to implement actual
applications, because foundation (and *only* foundation) models (both
servers and clients) would be provided by the daemon.


Another point would be adding explicit ImportNetKey and ImportAppKey
operations, so that an application using external provisioning database
would be able to inject keys into the daemon's database without sending
raw Config Client messages. This is similar to Import*Node operations we
discussed before.

> >  - SendApplicationMessage + ApplicationMessageReceived
> >  - SendDeviceMessage + DeviceMessageReceived
> 
> That seems reasonable. We will rework for naming consistency.
Thank you.

> >  - SendControlMessage + ControlMessageReceived (but I don't see a need
> >    to expose this to application, at least not at the moment)
> I am not sure there are *any* control messages that I would expose to
> any of the Apps.  Certainly none of the existing set of control
> messages, and I worry that exposing the Control interface would be
> seen as an invitation for "Vendor Usage" which could be difficult to
> back out later.
Fully agreed. I mentioned these only for completeness.

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