[PATCH BlueZ] Add SetRemoteProperty method for OOB mechanism

[PATCH BlueZ] Add SetRemoteProperty method for OOB mechanism

[Bartosz Szatkowski]

It might be necessary in use cases when there is OOB pairing and some
portion of remote device properties is needed before profiles are
connected. For now only class of device is supported.
---
 doc/oob-api.txt   |   10 ++++++++++
 plugins/dbusoob.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/doc/oob-api.txt b/doc/oob-api.txt
index d838712..e15fc29 100644
--- a/doc/oob-api.txt
+++ b/doc/oob-api.txt
@@ -36,3 +36,13 @@ Methods		array{byte} hash, array{byte} randomizer ReadLocalData()
 
 			Possible errors: org.bluez.Error.Failed
 					 org.bluez.Error.InvalidArguments
+
+		void SetRemoteProperty(string address, string property,
+								variant value)
+
+			This method adds new property for device with specified
+			address, to be used when device is created.
+			For now only Class of device is supported.
+
+			Possible errors: org.bluez.Error.Failed
+					 org.bluez.Error.InvalidArguments
diff --git a/plugins/dbusoob.c b/plugins/dbusoob.c
index 2c03780..c9ca57a 100644
--- a/plugins/dbusoob.c
+++ b/plugins/dbusoob.c
@@ -43,6 +43,7 @@
 #include "event.h"
 #include "error.h"
 #include "oob.h"
+#include "storage.h"
 
 #define OOB_INTERFACE	"org.bluez.OutOfBand"
 
@@ -153,6 +154,57 @@ static DBusMessage *add_remote_data(DBusConnection *conn, DBusMessage *msg,
 	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
 
+static DBusMessage *set_remote_property(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct btd_adapter *adapter = data;
+	DBusMessageIter iter, sub;
+	char *addr, *property;
+	bdaddr_t local, peer;
+
+	if (!dbus_message_iter_init(msg, &iter))
+		return btd_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return btd_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &addr);
+	dbus_message_iter_next(&iter);
+
+	if (bachk(addr))
+		return btd_error_invalid_args(msg);
+
+	adapter_get_address(adapter, &local);
+	str2ba(addr, &peer);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return btd_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &property);
+	dbus_message_iter_next(&iter);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
+		return btd_error_invalid_args(msg);
+
+	dbus_message_iter_recurse(&iter, &sub);
+
+	if (g_str_equal("Class", property)) {
+		uint32_t class;
+
+		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT32)
+			return btd_error_invalid_args(msg);
+
+		dbus_message_iter_get_basic(&sub, &class);
+
+		if (write_remote_class(&local, &peer, class) < 0)
+			return btd_error_failed(msg, "Request failed");
+	} else {
+		return btd_error_invalid_args(msg);
+	}
+
+	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
 static DBusMessage *remove_remote_data(DBusConnection *conn, DBusMessage *msg,
 								void *data)
 {
@@ -177,6 +229,7 @@ static DBusMessage *remove_remote_data(DBusConnection *conn, DBusMessage *msg,
 
 static GDBusMethodTable oob_methods[] = {
 	{"AddRemoteData",	"sayay",	"",	add_remote_data},
+	{"SetRemoteProperty",	"ssv",		"",	set_remote_property},
 	{"RemoveRemoteData",	"s",		"",	remove_remote_data},
 	{"ReadLocalData",	"",		"ayay",	read_local_data,
 						G_DBUS_METHOD_FLAG_ASYNC},
-- 
1.7.4.1

Re: [PATCH BlueZ] Add SetRemoteProperty method for OOB mechanism

[Luiz Augusto von Dentz]

Hi Bartosz,

On Mon, Jul 4, 2011 at 5:49 PM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
> It might be necessary in use cases when there is OOB pairing and some
> portion of remote device properties is needed before profiles are
> connected. For now only class of device is supported.
> ---
>  doc/oob-api.txt   |   10 ++++++++++
>  plugins/dbusoob.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 63 insertions(+), 0 deletions(-)
>
> diff --git a/doc/oob-api.txt b/doc/oob-api.txt
> index d838712..e15fc29 100644
> --- a/doc/oob-api.txt
> +++ b/doc/oob-api.txt
> @@ -36,3 +36,13 @@ Methods              array{byte} hash, array{byte} randomizer ReadLocalData()
>
>                        Possible errors: org.bluez.Error.Failed
>                                         org.bluez.Error.InvalidArguments
> +
> +               void SetRemoteProperty(string address, string property,
> +                                                               variant value)
> +
> +                       This method adds new property for device with specified
> +                       address, to be used when device is created.
> +                       For now only Class of device is supported.
> +
> +                       Possible errors: org.bluez.Error.Failed
> +                                        org.bluez.Error.InvalidArguments
> diff --git a/plugins/dbusoob.c b/plugins/dbusoob.c
> index 2c03780..c9ca57a 100644
> --- a/plugins/dbusoob.c
> +++ b/plugins/dbusoob.c
> @@ -43,6 +43,7 @@
>  #include "event.h"
>  #include "error.h"
>  #include "oob.h"
> +#include "storage.h"
>
>  #define OOB_INTERFACE  "org.bluez.OutOfBand"
>
> @@ -153,6 +154,57 @@ static DBusMessage *add_remote_data(DBusConnection *conn, DBusMessage *msg,
>        return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
>  }
>
> +static DBusMessage *set_remote_property(DBusConnection *conn, DBusMessage *msg,
> +                                                               void *data)
> +{
> +       struct btd_adapter *adapter = data;
> +       DBusMessageIter iter, sub;
> +       char *addr, *property;
> +       bdaddr_t local, peer;
> +
> +       if (!dbus_message_iter_init(msg, &iter))
> +               return btd_error_invalid_args(msg);
> +
> +       if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> +               return btd_error_invalid_args(msg);
> +
> +       dbus_message_iter_get_basic(&iter, &addr);
> +       dbus_message_iter_next(&iter);
> +
> +       if (bachk(addr))
> +               return btd_error_invalid_args(msg);
> +
> +       adapter_get_address(adapter, &local);
> +       str2ba(addr, &peer);
> +
> +       if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> +               return btd_error_invalid_args(msg);
> +
> +       dbus_message_iter_get_basic(&iter, &property);
> +       dbus_message_iter_next(&iter);
> +
> +       if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
> +               return btd_error_invalid_args(msg);
> +
> +       dbus_message_iter_recurse(&iter, &sub);
> +
> +       if (g_str_equal("Class", property)) {
> +               uint32_t class;
> +
> +               if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT32)
> +                       return btd_error_invalid_args(msg);
> +
> +               dbus_message_iter_get_basic(&sub, &class);
> +
> +               if (write_remote_class(&local, &peer, class) < 0)
> +                       return btd_error_failed(msg, "Request failed");
> +       } else {
> +               return btd_error_invalid_args(msg);
> +       }
> +
> +       return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
> +}
> +
>  static DBusMessage *remove_remote_data(DBusConnection *conn, DBusMessage *msg,
>                                                                void *data)
>  {
> @@ -177,6 +229,7 @@ static DBusMessage *remove_remote_data(DBusConnection *conn, DBusMessage *msg,
>
>  static GDBusMethodTable oob_methods[] = {
>        {"AddRemoteData",       "sayay",        "",     add_remote_data},
> +       {"SetRemoteProperty",   "ssv",          "",     set_remote_property},
>        {"RemoveRemoteData",    "s",            "",     remove_remote_data},
>        {"ReadLocalData",       "",             "ayay", read_local_data,
>                                                G_DBUS_METHOD_FLAG_ASYNC},
> --
> 1.7.4.1

I don't think this is the right approach for OOB data because this
information may become private to bluetoothd if the device is not
created since it is not associated with any device object/path, IMO
the OOB data needs to be passed in the CreateDevice which should take
a dictionary containing all the known information of the device, but
that breaks the API otherwise I would already send a patch proposing
this.


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ] Add SetRemoteProperty method for OOB mechanism

[Bartosz Szatkowski]

On Tue, Jul 5, 2011 at 10:42 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Bartosz,
>
> On Mon, Jul 4, 2011 at 5:49 PM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
>> It might be necessary in use cases when there is OOB pairing and some
>> portion of remote device properties is needed before profiles are
>> connected. For now only class of device is supported.
>> ---
>>  doc/oob-api.txt   |   10 ++++++++++
>>  plugins/dbusoob.c |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 63 insertions(+), 0 deletions(-)
>>
>> diff --git a/doc/oob-api.txt b/doc/oob-api.txt
>> index d838712..e15fc29 100644
>> --- a/doc/oob-api.txt
>> +++ b/doc/oob-api.txt
>> @@ -36,3 +36,13 @@ Methods              array{byte} hash, array{byte} randomizer ReadLocalData()
>>
>>                        Possible errors: org.bluez.Error.Failed
>>                                         org.bluez.Error.InvalidArguments
>> +
>> +               void SetRemoteProperty(string address, string property,
>> +                                                               variant value)
>> +
>> +                       This method adds new property for device with specified
>> +                       address, to be used when device is created.
>> +                       For now only Class of device is supported.
>> +
>> +                       Possible errors: org.bluez.Error.Failed
>> +                                        org.bluez.Error.InvalidArguments
>> diff --git a/plugins/dbusoob.c b/plugins/dbusoob.c
>> index 2c03780..c9ca57a 100644
>> --- a/plugins/dbusoob.c
>> +++ b/plugins/dbusoob.c
>> @@ -43,6 +43,7 @@
>>  #include "event.h"
>>  #include "error.h"
>>  #include "oob.h"
>> +#include "storage.h"
>>
>>  #define OOB_INTERFACE  "org.bluez.OutOfBand"
>>
>> @@ -153,6 +154,57 @@ static DBusMessage *add_remote_data(DBusConnection *conn, DBusMessage *msg,
>>        return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
>>  }
>>
>> +static DBusMessage *set_remote_property(DBusConnection *conn, DBusMessage *msg,
>> +                                                               void *data)
>> +{
>> +       struct btd_adapter *adapter = data;
>> +       DBusMessageIter iter, sub;
>> +       char *addr, *property;
>> +       bdaddr_t local, peer;
>> +
>> +       if (!dbus_message_iter_init(msg, &iter))
>> +               return btd_error_invalid_args(msg);
>> +
>> +       if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
>> +               return btd_error_invalid_args(msg);
>> +
>> +       dbus_message_iter_get_basic(&iter, &addr);
>> +       dbus_message_iter_next(&iter);
>> +
>> +       if (bachk(addr))
>> +               return btd_error_invalid_args(msg);
>> +
>> +       adapter_get_address(adapter, &local);
>> +       str2ba(addr, &peer);
>> +
>> +       if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
>> +               return btd_error_invalid_args(msg);
>> +
>> +       dbus_message_iter_get_basic(&iter, &property);
>> +       dbus_message_iter_next(&iter);
>> +
>> +       if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
>> +               return btd_error_invalid_args(msg);
>> +
>> +       dbus_message_iter_recurse(&iter, &sub);
>> +
>> +       if (g_str_equal("Class", property)) {
>> +               uint32_t class;
>> +
>> +               if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT32)
>> +                       return btd_error_invalid_args(msg);
>> +
>> +               dbus_message_iter_get_basic(&sub, &class);
>> +
>> +               if (write_remote_class(&local, &peer, class) < 0)
>> +                       return btd_error_failed(msg, "Request failed");
>> +       } else {
>> +               return btd_error_invalid_args(msg);
>> +       }
>> +
>> +       return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
>> +}
>> +
>>  static DBusMessage *remove_remote_data(DBusConnection *conn, DBusMessage *msg,
>>                                                                void *data)
>>  {
>> @@ -177,6 +229,7 @@ static DBusMessage *remove_remote_data(DBusConnection *conn, DBusMessage *msg,
>>
>>  static GDBusMethodTable oob_methods[] = {
>>        {"AddRemoteData",       "sayay",        "",     add_remote_data},
>> +       {"SetRemoteProperty",   "ssv",          "",     set_remote_property},
>>        {"RemoveRemoteData",    "s",            "",     remove_remote_data},
>>        {"ReadLocalData",       "",             "ayay", read_local_data,
>>                                                G_DBUS_METHOD_FLAG_ASYNC},
>> --
>> 1.7.4.1
>
> I don't think this is the right approach for OOB data because this
> information may become private to bluetoothd if the device is not
> created since it is not associated with any device object/path, IMO
> the OOB data needs to be passed in the CreateDevice which should take
> a dictionary containing all the known information of the device, but
> that breaks the API otherwise I would already send a patch proposing
> this.
>
>
> --
> Luiz Augusto von Dentz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Right, i'v discussed this approach with Johan and Szymon on freenode
-- i'v changed few things after that (mostly because implementing it
in AddRemoteData would break api, so i moved it to separate method) --
maybe it would be a good idea to have this functionality now in this
form and change it when the api break would be possible (5.0?)?

-- 
Pozdrowienia,
Bartosz Szatkowski

Re: [PATCH BlueZ] Add SetRemoteProperty method for OOB mechanism

[Luiz Augusto von Dentz]

Hi Bartosz,

On Tue, Jul 5, 2011 at 11:55 AM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
>
> Right, i'v discussed this approach with Johan and Szymon on freenode
> -- i'v changed few things after that (mostly because implementing it
> in AddRemoteData would break api, so i moved it to separate method) --
> maybe it would be a good idea to have this functionality now in this
> form and change it when the api break would be possible (5.0?)?
>

Apparently the idea is to use this interface is to emulate a
DeviceFound result, which I guess would make sense if the user pairs
using the bt application/agent, but in the other hand this can be
misused by application and they actually start overwriting device
properties e.g. restore tool. So the question is how much do we trust
application to provide this information without properly creating a
device? To me it sounds that we either need the agent to actually
accept this information as valid, which btw normally requires an
object path to identify the device to query its properties, or we do
it all together as I suggested in CreateDevice so we validate the
information by pairing/connecting to the device.

Btw, there is also the problem that D-Bus round trips are expensive
and with such API one have to set one by one the properties to finally
do a CreateDevice in the end, so at least the current design should
make sure that an application can set all its known properties in one
call e.g. SetRemoteProperties(string address, dict properties).

-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ] Add SetRemoteProperty method for OOB mechanism

[Johan Hedberg]

Hi,

> > I don't think this is the right approach for OOB data because this
> > information may become private to bluetoothd if the device is not
> > created since it is not associated with any device object/path, IMO
> > the OOB data needs to be passed in the CreateDevice which should take
> > a dictionary containing all the known information of the device, but
> > that breaks the API otherwise I would already send a patch proposing
> > this.
> 
> Right, i'v discussed this approach with Johan and Szymon on freenode
> -- i'v changed few things after that (mostly because implementing it
> in AddRemoteData would break api, so i moved it to separate method) --
> maybe it would be a good idea to have this functionality now in this
> form and change it when the api break would be possible (5.0?)?

To me, the closest analogy of receiving data like this over some OOB
channel is an inquiry result HCI event. Therefore, it'd be intuitive to
have this behave in a similar way: the data goes to storage and triggers
a DeviceFound event. Calling Create(Paired)Device remains separate from
all this.

This doesn't of course mean that we couldn't/shouldn't restructure the
device discovery process for 5.x to e.g. have D-Bus objects for every
found device and a Device.Pair() method, but the device information
received over an OOB channel should still map to a similar set of steps
as what happens when receiving a HCI inquiry result event.

Johan

Re: [PATCH BlueZ] Add SetRemoteProperty method for OOB mechanism

[Bartosz Szatkowski]

On Tue, Jul 5, 2011 at 11:51 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Bartosz,
>
> On Tue, Jul 5, 2011 at 11:55 AM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
>>
>> Right, i'v discussed this approach with Johan and Szymon on freenode
>> -- i'v changed few things after that (mostly because implementing it
>> in AddRemoteData would break api, so i moved it to separate method) --
>> maybe it would be a good idea to have this functionality now in this
>> form and change it when the api break would be possible (5.0?)?
>>
>
> Apparently the idea is to use this interface is to emulate a
> DeviceFound result, which I guess would make sense if the user pairs
> using the bt application/agent, but in the other hand this can be
> misused by application and they actually start overwriting device
> properties e.g. restore tool. So the question is how much do we trust
> application to provide this information without properly creating a
> device? To me it sounds that we either need the agent to actually
> accept this information as valid, which btw normally requires an
> object path to identify the device to query its properties, or we do
> it all together as I suggested in CreateDevice so we validate the
> information by pairing/connecting to the device.
>
> Btw, there is also the problem that D-Bus round trips are expensive
> and with such API one have to set one by one the properties to finally
> do a CreateDevice in the end, so at least the current design should
> make sure that an application can set all its known properties in one
> call e.g. SetRemoteProperties(string address, dict properties).
>
> --
> Luiz Augusto von Dentz
>

Yeah it sound good, but are we sure that there is need for setting
other parameters via OOB? The idea that was cleared on irc wast to add
CoD parameter to OOB.AddRemoteData(address, hash, randomizer, CoD)

-- 
Pozdrowienia,
Bartosz Szatkowski

Re: [PATCH BlueZ] Add SetRemoteProperty method for OOB mechanism

[Bartosz Szatkowski]

On Tue, Jul 5, 2011 at 11:59 AM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
> On Tue, Jul 5, 2011 at 11:51 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Bartosz,
>>
>> On Tue, Jul 5, 2011 at 11:55 AM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
>>>
>>> Right, i'v discussed this approach with Johan and Szymon on freenode
>>> -- i'v changed few things after that (mostly because implementing it
>>> in AddRemoteData would break api, so i moved it to separate method) --
>>> maybe it would be a good idea to have this functionality now in this
>>> form and change it when the api break would be possible (5.0?)?
>>>
>>
>> Apparently the idea is to use this interface is to emulate a
>> DeviceFound result, which I guess would make sense if the user pairs
>> using the bt application/agent, but in the other hand this can be
>> misused by application and they actually start overwriting device
>> properties e.g. restore tool. So the question is how much do we trust
>> application to provide this information without properly creating a
>> device? To me it sounds that we either need the agent to actually
>> accept this information as valid, which btw normally requires an
>> object path to identify the device to query its properties, or we do
>> it all together as I suggested in CreateDevice so we validate the
>> information by pairing/connecting to the device.
>>
>> Btw, there is also the problem that D-Bus round trips are expensive
>> and with such API one have to set one by one the properties to finally
>> do a CreateDevice in the end, so at least the current design should
>> make sure that an application can set all its known properties in one
>> call e.g. SetRemoteProperties(string address, dict properties).
>>
>> --
>> Luiz Augusto von Dentz
>>
>
> Yeah it sound good, but are we sure that there is need for setting
> other parameters via OOB? The idea that was cleared on irc wast to add
> CoD parameter to OOB.AddRemoteData(address, hash, randomizer, CoD)
>
> --
> Pozdrowienia,
> Bartosz Szatkowski
>

Is there any conclusion ? :)
Are we leaving it that way or changing it as Luiz suggested (array)?
Or maybe something else?

-- 
Pozdrowienia,
Bartosz Szatkowski

Re: [PATCH BlueZ] Add SetRemoteProperty method for OOB mechanism

[Johan Hedberg]

Hi Bartosz,

On Wed, Jul 06, 2011, Bartosz Szatkowski wrote:
> On Tue, Jul 5, 2011 at 11:59 AM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
> > On Tue, Jul 5, 2011 at 11:51 AM, Luiz Augusto von Dentz
> >> On Tue, Jul 5, 2011 at 11:55 AM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
> >>> Right, i'v discussed this approach with Johan and Szymon on freenode
> >>> -- i'v changed few things after that (mostly because implementing it
> >>> in AddRemoteData would break api, so i moved it to separate method) --
> >>> maybe it would be a good idea to have this functionality now in this
> >>> form and change it when the api break would be possible (5.0?)?
> >>
> >> Apparently the idea is to use this interface is to emulate a
> >> DeviceFound result, which I guess would make sense if the user pairs
> >> using the bt application/agent, but in the other hand this can be
> >> misused by application and they actually start overwriting device
> >> properties e.g. restore tool. So the question is how much do we trust
> >> application to provide this information without properly creating a
> >> device? To me it sounds that we either need the agent to actually
> >> accept this information as valid, which btw normally requires an
> >> object path to identify the device to query its properties, or we do
> >> it all together as I suggested in CreateDevice so we validate the
> >> information by pairing/connecting to the device.
> >>
> >> Btw, there is also the problem that D-Bus round trips are expensive
> >> and with such API one have to set one by one the properties to finally
> >> do a CreateDevice in the end, so at least the current design should
> >> make sure that an application can set all its known properties in one
> >> call e.g. SetRemoteProperties(string address, dict properties).
> >
> > Yeah it sound good, but are we sure that there is need for setting
> > other parameters via OOB? The idea that was cleared on irc wast to add
> > CoD parameter to OOB.AddRemoteData(address, hash, randomizer, CoD)
> 
> Is there any conclusion ? :)
> Are we leaving it that way or changing it as Luiz suggested (array)?
> Or maybe something else?

For things like name and class (I suppose a list of UUIDs can also come
through the OOB data?). I think a single method call taking a dictionary
would be best. For the hash and randomizer, we could either keep the
existing method, or just define new "Hash" and "Rand" dictionary
entries. Since it's likely that all of this data comes in the same
instant over OOB I'm leaning towards the later solution.

Johan

Re: [PATCH BlueZ] Add SetRemoteProperty method for OOB mechanism

[Bartosz Szatkowski]

Hi Johan,
OK, I'll change it probably tomorrow.

On Tue, Jul 26, 2011 at 10:11 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Bartosz,
>
> On Wed, Jul 06, 2011, Bartosz Szatkowski wrote:
>> On Tue, Jul 5, 2011 at 11:59 AM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
>> > On Tue, Jul 5, 2011 at 11:51 AM, Luiz Augusto von Dentz
>> >> On Tue, Jul 5, 2011 at 11:55 AM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
>> >>> Right, i'v discussed this approach with Johan and Szymon on freenode
>> >>> -- i'v changed few things after that (mostly because implementing it
>> >>> in AddRemoteData would break api, so i moved it to separate method) --
>> >>> maybe it would be a good idea to have this functionality now in this
>> >>> form and change it when the api break would be possible (5.0?)?
>> >>
>> >> Apparently the idea is to use this interface is to emulate a
>> >> DeviceFound result, which I guess would make sense if the user pairs
>> >> using the bt application/agent, but in the other hand this can be
>> >> misused by application and they actually start overwriting device
>> >> properties e.g. restore tool. So the question is how much do we trust
>> >> application to provide this information without properly creating a
>> >> device? To me it sounds that we either need the agent to actually
>> >> accept this information as valid, which btw normally requires an
>> >> object path to identify the device to query its properties, or we do
>> >> it all together as I suggested in CreateDevice so we validate the
>> >> information by pairing/connecting to the device.
>> >>
>> >> Btw, there is also the problem that D-Bus round trips are expensive
>> >> and with such API one have to set one by one the properties to finally
>> >> do a CreateDevice in the end, so at least the current design should
>> >> make sure that an application can set all its known properties in one
>> >> call e.g. SetRemoteProperties(string address, dict properties).
>> >
>> > Yeah it sound good, but are we sure that there is need for setting
>> > other parameters via OOB? The idea that was cleared on irc wast to add
>> > CoD parameter to OOB.AddRemoteData(address, hash, randomizer, CoD)
>>
>> Is there any conclusion ? :)
>> Are we leaving it that way or changing it as Luiz suggested (array)?
>> Or maybe something else?
>
> For things like name and class (I suppose a list of UUIDs can also come
> through the OOB data?). I think a single method call taking a dictionary
> would be best. For the hash and randomizer, we could either keep the
> existing method, or just define new "Hash" and "Rand" dictionary
> entries. Since it's likely that all of this data comes in the same
> instant over OOB I'm leaning towards the later solution.
>
> Johan
>



-- 
Pozdrowienia,
Bartosz Szatkowski

[PATCH BlueZ] Add SetRemoteProperty method for OOB mechanism

[Bartosz Szatkowski]

Add possibility of suppling some additional data (like class of device)
when using OOB mechanism for pairing.
---
Sadly to handle all possible combination of acceptable parameters (extreme case
is when there is a string and an array of a dict entries with string and variant
with an array of bytes - sa{sv} -> v: ay) function must be rather complicated.
I thought of dividing it, but this use case is rather individual and refactoring
part of it out would tangled whole architecture.

 doc/oob-api.txt   |   16 ++++++++
 plugins/dbusoob.c |  104 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+), 0 deletions(-)

diff --git a/doc/oob-api.txt b/doc/oob-api.txt
index d838712..4f60ca1 100644
--- a/doc/oob-api.txt
+++ b/doc/oob-api.txt
@@ -36,3 +36,19 @@ Methods		array{byte} hash, array{byte} randomizer ReadLocalData()
 
 			Possible errors: org.bluez.Error.Failed
 					 org.bluez.Error.InvalidArguments
+
+		void SetRemoteProperty(string address, dict data)
+
+			This method set new properties for device with specified
+			address, to be used when device is created. Please note
+			that Hash and Randomizer must be both given in the same 
+			method call, otherwise it will be ignored.
+
+			Currently supported keys:
+
+				Class		uint32
+				Hash		array{byte}
+				Randomizer	array{byte}
+
+			Possible errors: org.bluez.Error.Failed
+					 org.bluez.Error.InvalidArguments
diff --git a/plugins/dbusoob.c b/plugins/dbusoob.c
index 2c03780..3647a3e 100644
--- a/plugins/dbusoob.c
+++ b/plugins/dbusoob.c
@@ -43,6 +43,7 @@
 #include "event.h"
 #include "error.h"
 #include "oob.h"
+#include "storage.h"
 
 #define OOB_INTERFACE	"org.bluez.OutOfBand"
 
@@ -153,6 +154,108 @@ static DBusMessage *add_remote_data(DBusConnection *conn, DBusMessage *msg,
 	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
 
+static DBusMessage *set_remote_property(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct btd_adapter *adapter = data;
+	DBusMessageIter iter_msg, iter_dict, iter_entry, iter_variant,
+								iter_array;
+	char *addr;
+	bdaddr_t local, peer;
+	uint8_t *hash = NULL, *rand = NULL;
+	int32_t hlen, rlen;
+
+	if (!dbus_message_iter_init(msg, &iter_msg))
+		return btd_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter_msg) != DBUS_TYPE_STRING)
+		return btd_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter_msg, &addr);
+
+	if (bachk(addr))
+		return btd_error_invalid_args(msg);
+
+	adapter_get_address(adapter, &local);
+	str2ba(addr, &peer);
+
+	dbus_message_iter_get_basic(&iter_msg, &addr);
+	dbus_message_iter_next(&iter_msg);
+
+	if (dbus_message_iter_get_arg_type(&iter_msg) != DBUS_TYPE_ARRAY)
+		return btd_error_invalid_args(msg);
+
+	dbus_message_iter_recurse(&iter_msg, &iter_dict);
+
+	for (; dbus_message_iter_get_arg_type(&iter_dict) != DBUS_TYPE_INVALID;
+					dbus_message_iter_next(&iter_dict)) {
+		char *property;
+
+		if (dbus_message_iter_get_arg_type(&iter_dict) !=
+							DBUS_TYPE_DICT_ENTRY)
+			return btd_error_invalid_args(msg);
+
+		dbus_message_iter_recurse(&iter_dict, &iter_entry);
+
+		if (dbus_message_iter_get_arg_type(&iter_entry) !=
+							DBUS_TYPE_STRING)
+			return btd_error_invalid_args(msg);
+
+		dbus_message_iter_get_basic(&iter_entry, &property);
+		dbus_message_iter_next(&iter_entry);
+
+		if (dbus_message_iter_get_arg_type(&iter_entry) !=
+							DBUS_TYPE_VARIANT)
+			return btd_error_invalid_args(msg);
+
+		dbus_message_iter_recurse(&iter_entry, &iter_variant);
+
+		if (g_str_equal("Class", property)) {
+			uint32_t class;
+
+			if (dbus_message_iter_get_arg_type(&iter_variant) !=
+							DBUS_TYPE_UINT32)
+				return btd_error_invalid_args(msg);
+
+			dbus_message_iter_get_basic(&iter_variant, &class);
+
+			if (write_remote_class(&local, &peer, class) < 0)
+				return btd_error_failed(msg, "Request failed");
+		} else if (g_str_equal("Hash", property)) {
+			if (dbus_message_iter_get_arg_type(&iter_variant) !=
+								DBUS_TYPE_ARRAY)
+				return btd_error_invalid_args(msg);
+
+			dbus_message_iter_recurse(&iter_variant, &iter_array);
+
+			dbus_message_iter_get_fixed_array(&iter_array, &hash,
+									&hlen);
+			if (hlen != 16)
+				return btd_error_invalid_args(msg);
+		} else if (g_str_equal("Randomizer", property)) {
+			if (dbus_message_iter_get_arg_type(&iter_variant) !=
+								DBUS_TYPE_ARRAY)
+				return btd_error_invalid_args(msg);
+
+			dbus_message_iter_recurse(&iter_variant, &iter_array);
+
+			dbus_message_iter_get_fixed_array(&iter_array, &rand,
+									&rlen);
+			if (rlen != 16)
+				return btd_error_invalid_args(msg);
+		} else {
+			error("Parameter name not recognized: %s", property);
+		}
+	}
+
+	if (rand != NULL && hash != NULL)
+		if (btd_adapter_add_remote_oob_data(adapter, &peer, hash, rand))
+			return btd_error_failed(msg,
+						"Adding remote data failed");
+
+	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
 static DBusMessage *remove_remote_data(DBusConnection *conn, DBusMessage *msg,
 								void *data)
 {
@@ -177,6 +280,7 @@ static DBusMessage *remove_remote_data(DBusConnection *conn, DBusMessage *msg,
 
 static GDBusMethodTable oob_methods[] = {
 	{"AddRemoteData",	"sayay",	"",	add_remote_data},
+	{"SetRemoteProperty",	"sa{sv}",	"",	set_remote_property},
 	{"RemoveRemoteData",	"s",		"",	remove_remote_data},
 	{"ReadLocalData",	"",		"ayay",	read_local_data,
 						G_DBUS_METHOD_FLAG_ASYNC},
-- 
1.7.4.1