2011-06-08 14:05:19

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: [PATCH 1/2] Add RequestSecurePinCode to agent API

The method is called if min. 16 bytes pincode is mendatory
to authenticate connection.

In practice this will be called only with mgmtops switched on. Hciops
don't support secure pin code so far.
---
doc/agent-api.txt | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/doc/agent-api.txt b/doc/agent-api.txt
index 9ab2063..b1cb354 100644
--- a/doc/agent-api.txt
+++ b/doc/agent-api.txt
@@ -31,6 +31,17 @@ Methods void Release()
Possible errors: org.bluez.Error.Rejected
org.bluez.Error.Canceled

+ string RequestSecurePinCode(object device)
+
+ This method gets called when the service daemon
+ needs to get the secure passkey for an authentication.
+
+ The return value should be a string of 16 characters
+ length. The string can be alphanumeric.
+
+ Possible errors: org.bluez.Error.Rejected
+ org.bluez.Error.Canceled
+
uint32 RequestPasskey(object device)

This method gets called when the service daemon
--
1.7.4.1



2011-06-10 01:14:44

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add RequestSecurePinCode to agent API

Hi Gustavo,

On Thu, Jun 09, 2011, Gustavo F. Padovan wrote:
> > We are getting closer to 4.100, maybe it is time to start doing the
> > transition to 5.x, which means we could start thinking on fixing the
> > current API without worrying if it breaks or not. How about that?
>
> I'm with Luiz here. Unless we want the the crazy idea of have a development
> branch we should start breaking the API someday. How about now?

I'd prefer to have mgmtops.c on par with hciops.c for BR/EDR before
doing the move to 5.x. I do prefer the RequestPinCode with the boolean
parameter option, but since it breaks the API I think it needs to wait
until we're ready to go for 5.0.

Johan

2011-06-10 00:54:39

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add RequestSecurePinCode to agent API

Hi,

On Fri, Jun 10, 2011 at 4:18 AM, Gustavo F. Padovan
<[email protected]> wrote:
> * Luiz Augusto von Dentz <[email protected]> [2011-06-09 21:38:22 +090=
0]:
>
>> Hi Marcel,
>>
>> On Thu, Jun 9, 2011 at 7:24 PM, Marcel Holtmann <[email protected]> wr=
ote:
>> > Hi Waldemar,
>> >
>> >> The method is called if min. 16 bytes pincode is mendatory
>> >> to authenticate connection.
>> >>
>> >> In practice this will be called only with mgmtops switched on. Hciops
>> >> don't support secure pin code so far.
>> >> ---
>> >> =A0doc/agent-api.txt | =A0 11 +++++++++++
>> >> =A01 files changed, 11 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/doc/agent-api.txt b/doc/agent-api.txt
>> >> index 9ab2063..b1cb354 100644
>> >> --- a/doc/agent-api.txt
>> >> +++ b/doc/agent-api.txt
>> >> @@ -31,6 +31,17 @@ Methods =A0 =A0 =A0 =A0 =A0 =A0void Release()
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 Possible errors: org.blue=
z.Error.Rejected
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0org.bluez.Error.Canceled
>> >>
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 string RequestSecurePinCode(object device)
>> >> +
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 This method gets called whe=
n the service daemon
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 needs to get the secure pas=
skey for an authentication.
>> >> +
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 The return value should be =
a string of 16 characters
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 length. The string can be a=
lphanumeric.
>> >> +
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 Possible errors: org.bluez.=
Error.Rejected
>> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0org.bluez.Error.Canceled
>> >> +
>> >
>> > I do not like this since it is really legacy stuff and makes since
>> > really complicated. And now we are making big efforts to support this
>> > and that is bad. Since Simple Pairing solved this nicely for us.
>> >
>> > However I need to talk with Johan about this and figure something out.
>>
>> We are getting closer to 4.100, maybe it is time to start doing the
>> transition to 5.x, which means we could start thinking on fixing the
>> current API without worrying if it breaks or not. How about that?
>
> I'm with Luiz here. Unless we want the the crazy idea of have a developme=
nt
> branch we should start breaking the API someday. How about now?
>
> And if we are really going for it a roadmap made public somewhere would b=
e
> good. :)

Actually we can do it without breaking 4.x API like we did with 3.x
where both API lives side by side until we decide it is stable enough,
to do that we can have the new API having a different interface prefix
e.g. org.bluez5 or just have a different connection with same
interfaces names, the 1 option is less complicated for bluetoothd but
requires more work on the client side while the second is more
complicated for the daemon while the client just need to change the
bus id.

--=20
Luiz Augusto von Dentz
Computer Engineer

2011-06-09 19:18:33

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add RequestSecurePinCode to agent API

* Luiz Augusto von Dentz <[email protected]> [2011-06-09 21:38:22 +0900]:

> Hi Marcel,
>=20
> On Thu, Jun 9, 2011 at 7:24 PM, Marcel Holtmann <[email protected]> wro=
te:
> > Hi Waldemar,
> >
> >> The method is called if min. 16 bytes pincode is mendatory
> >> to authenticate connection.
> >>
> >> In practice this will be called only with mgmtops switched on. Hciops
> >> don't support secure pin code so far.
> >> ---
> >> =A0doc/agent-api.txt | =A0 11 +++++++++++
> >> =A01 files changed, 11 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/doc/agent-api.txt b/doc/agent-api.txt
> >> index 9ab2063..b1cb354 100644
> >> --- a/doc/agent-api.txt
> >> +++ b/doc/agent-api.txt
> >> @@ -31,6 +31,17 @@ Methods =A0 =A0 =A0 =A0 =A0 =A0void Release()
> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 Possible errors: org.bluez=
=2EError.Rejected
> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0org.bluez.Error.Canceled
> >>
> >> + =A0 =A0 =A0 =A0 =A0 =A0 string RequestSecurePinCode(object device)
> >> +
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 This method gets called when=
the service daemon
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 needs to get the secure pass=
key for an authentication.
> >> +
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 The return value should be a=
string of 16 characters
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 length. The string can be al=
phanumeric.
> >> +
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 Possible errors: org.bluez.E=
rror.Rejected
> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0org.bluez.Error.Canceled
> >> +
> >
> > I do not like this since it is really legacy stuff and makes since
> > really complicated. And now we are making big efforts to support this
> > and that is bad. Since Simple Pairing solved this nicely for us.
> >
> > However I need to talk with Johan about this and figure something out.
>=20
> We are getting closer to 4.100, maybe it is time to start doing the
> transition to 5.x, which means we could start thinking on fixing the
> current API without worrying if it breaks or not. How about that?

I'm with Luiz here. Unless we want the the crazy idea of have a development
branch we should start breaking the API someday. How about now?

And if we are really going for it a roadmap made public somewhere would be
good. :)

Gustavo

2011-06-09 14:00:33

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: RE: [PATCH 1/2] Add RequestSecurePinCode to agent API

Hi Marcel,

>I do not like this since it is really legacy stuff and makes=20
>since really complicated. And now we are making big efforts to=20
>support this and that is bad. Since Simple Pairing solved this=20
>nicely for us.
>
>However I need to talk with Johan about this and figure something out.

That's what happen when we do something that should be done at the begining=
.=20
One good news, this seems like the last patchset in this topic :)

I agree with Luiz, it's good idea to do it in bluez 5.x by adding just new =
param in PinCodeRequest.

Waldek

2011-06-09 12:38:22

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add RequestSecurePinCode to agent API

Hi Marcel,

On Thu, Jun 9, 2011 at 7:24 PM, Marcel Holtmann <[email protected]> wrote=
:
> Hi Waldemar,
>
>> The method is called if min. 16 bytes pincode is mendatory
>> to authenticate connection.
>>
>> In practice this will be called only with mgmtops switched on. Hciops
>> don't support secure pin code so far.
>> ---
>> =A0doc/agent-api.txt | =A0 11 +++++++++++
>> =A01 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/doc/agent-api.txt b/doc/agent-api.txt
>> index 9ab2063..b1cb354 100644
>> --- a/doc/agent-api.txt
>> +++ b/doc/agent-api.txt
>> @@ -31,6 +31,17 @@ Methods =A0 =A0 =A0 =A0 =A0 =A0void Release()
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 Possible errors: org.bluez.E=
rror.Rejected
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0org.bluez.Error.Canceled
>>
>> + =A0 =A0 =A0 =A0 =A0 =A0 string RequestSecurePinCode(object device)
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 This method gets called when t=
he service daemon
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 needs to get the secure passke=
y for an authentication.
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 The return value should be a s=
tring of 16 characters
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 length. The string can be alph=
anumeric.
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 Possible errors: org.bluez.Err=
or.Rejected
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0org.bluez.Error.Canceled
>> +
>
> I do not like this since it is really legacy stuff and makes since
> really complicated. And now we are making big efforts to support this
> and that is bad. Since Simple Pairing solved this nicely for us.
>
> However I need to talk with Johan about this and figure something out.

We are getting closer to 4.100, maybe it is time to start doing the
transition to 5.x, which means we could start thinking on fixing the
current API without worrying if it breaks or not. How about that?



--=20
Luiz Augusto von Dentz
Computer Engineer

2011-06-09 10:24:19

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add RequestSecurePinCode to agent API

Hi Waldemar,

> The method is called if min. 16 bytes pincode is mendatory
> to authenticate connection.
>
> In practice this will be called only with mgmtops switched on. Hciops
> don't support secure pin code so far.
> ---
> doc/agent-api.txt | 11 +++++++++++
> 1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/doc/agent-api.txt b/doc/agent-api.txt
> index 9ab2063..b1cb354 100644
> --- a/doc/agent-api.txt
> +++ b/doc/agent-api.txt
> @@ -31,6 +31,17 @@ Methods void Release()
> Possible errors: org.bluez.Error.Rejected
> org.bluez.Error.Canceled
>
> + string RequestSecurePinCode(object device)
> +
> + This method gets called when the service daemon
> + needs to get the secure passkey for an authentication.
> +
> + The return value should be a string of 16 characters
> + length. The string can be alphanumeric.
> +
> + Possible errors: org.bluez.Error.Rejected
> + org.bluez.Error.Canceled
> +

I do not like this since it is really legacy stuff and makes since
really complicated. And now we are making big efforts to support this
and that is bad. Since Simple Pairing solved this nicely for us.

However I need to talk with Johan about this and figure something out.

Regards

Marcel



2011-06-09 08:45:38

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: RE: [PATCH 1/2] Add RequestSecurePinCode to agent API


Hi Gustavo,

>Why RequestPinCode() isn't enough? It can accept up to 16 characters.
>Alternatively we can add a Property to tell when to use pin
>code or not.
>
>For BlueZ 5.0 we can break this and add a length param to
>RequestPinCode().
>

It's not really matter of the length but the agent needs to know if secure pincode is required.

RequestSecurePinCode doesn't break existing API as it's used with mgmtops only. If we go for RequestPinCode(boolean secure) this will break the API immediately.
Property? Do you mean agent, adapter or device property?

Well, RequestSecurePinCode seems to be the most clean solution for me, but of course it's a matter of discussion.


Waldek





2011-06-08 17:56:49

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add RequestSecurePinCode to agent API

* Waldemar Rymarkiewicz <[email protected]> [2011-06-08 16:05:19 +0200]:

> The method is called if min. 16 bytes pincode is mendatory
> to authenticate connection.
>
> In practice this will be called only with mgmtops switched on. Hciops
> don't support secure pin code so far.
> ---
> doc/agent-api.txt | 11 +++++++++++
> 1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/doc/agent-api.txt b/doc/agent-api.txt
> index 9ab2063..b1cb354 100644
> --- a/doc/agent-api.txt
> +++ b/doc/agent-api.txt
> @@ -31,6 +31,17 @@ Methods void Release()
> Possible errors: org.bluez.Error.Rejected
> org.bluez.Error.Canceled
>
> + string RequestSecurePinCode(object device)
> +
> + This method gets called when the service daemon
> + needs to get the secure passkey for an authentication.
> +
> + The return value should be a string of 16 characters
> + length. The string can be alphanumeric.
> +
> + Possible errors: org.bluez.Error.Rejected
> + org.bluez.Error.Canceled
> +

Why RequestPinCode() isn't enough? It can accept up to 16 characters.
Alternatively we can add a Property to tell when to use pin code or not.

For BlueZ 5.0 we can break this and add a length param to RequestPinCode().

Gustavo

2011-06-08 14:05:20

by Rymarkiewicz Waldemar

[permalink] [raw]
Subject: [PATCH 2/2] Request secure pincode from agent when necessary

---
src/agent.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/agent.c b/src/agent.c
index 7bba849..d1494de 100644
--- a/src/agent.c
+++ b/src/agent.c
@@ -428,11 +428,9 @@ static int pincode_request_new(struct agent_request *req, const char *device_pat
{
struct agent *agent = req->agent;

- /* TODO: Add a new method or a new param to Agent interface to request
- secure pin. */
-
req->msg = dbus_message_new_method_call(agent->name, agent->path,
- "org.bluez.Agent", "RequestPinCode");
+ "org.bluez.Agent", secure ?
+ "RequestSecurePinCode" : "RequestPinCode");
if (req->msg == NULL) {
error("Couldn't allocate D-Bus message");
return -ENOMEM;
--
1.7.4.1