diff mbox series

[BlueZ] doc/agent-api: Add AuthenticationFailed method

Message ID 20210623184355.3685859-1-luiz.dentz@gmail.com (mailing list archive)
State New, archived
Headers show
Series [BlueZ] doc/agent-api: Add AuthenticationFailed method | expand

Commit Message

Luiz Augusto von Dentz June 23, 2021, 6:43 p.m. UTC
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This introduces AuthenticationFailed method which is used to indicate
when there was an authentication failure with a remote device which is
recommended by the core spec:

BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1314:
 '5.2.2.3 Simple Pairing after authentication failure
 When the link key is stored, subsequent connections to the same device
 will use authentication but this may fail if the remote device has
 deleted the link key. Table 5.2 defines what shall be done depending
 on the type of the link key and whether bonding was performed or not.'

So following Table 5.2 recommendation the AuthenticationFailed method
would be called when authentication fails with a bonded device.
---
 doc/agent-api.txt | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Luiz Augusto von Dentz June 23, 2021, 6:45 p.m. UTC | #1
Hi,

On Wed, Jun 23, 2021 at 11:43 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This introduces AuthenticationFailed method which is used to indicate
> when there was an authentication failure with a remote device which is
> recommended by the core spec:
>
> BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1314:
>  '5.2.2.3 Simple Pairing after authentication failure
>  When the link key is stored, subsequent connections to the same device
>  will use authentication but this may fail if the remote device has
>  deleted the link key. Table 5.2 defines what shall be done depending
>  on the type of the link key and whether bonding was performed or not.'
>
> So following Table 5.2 recommendation the AuthenticationFailed method
> would be called when authentication fails with a bonded device.

This was intended to be an RFC.

> ---
>  doc/agent-api.txt | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/doc/agent-api.txt b/doc/agent-api.txt
> index 0d9347cab..91d752b0a 100644
> --- a/doc/agent-api.txt
> +++ b/doc/agent-api.txt
> @@ -183,3 +183,8 @@ Methods             void Release()
>
>                         This method gets called to indicate that the agent
>                         request failed before a reply was returned.
> +
> +               void AuthenticationFailed(object device, byte status)
> +
> +                       This method indicates that there was an authentication
> +                       failure with a remote device.
> --
> 2.31.1
>
bluez.test.bot@gmail.com June 23, 2021, 7:06 p.m. UTC | #2
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=505973

---Test result---

Test Summary:
CheckPatch                    PASS      0.36 seconds
GitLint                       PASS      0.12 seconds
Prep - Setup ELL              PASS      46.04 seconds
Build - Prep                  PASS      0.13 seconds
Build - Configure             PASS      8.09 seconds
Build - Make                  PASS      201.81 seconds
Make Check                    PASS      8.76 seconds
Make Distcheck                PASS      242.55 seconds
Build w/ext ELL - Configure   PASS      8.60 seconds
Build w/ext ELL - Make        PASS      194.76 seconds

Details
##############################
Test: CheckPatch - PASS
Desc: Run checkpatch.pl script with rule in .checkpatch.conf

##############################
Test: GitLint - PASS
Desc: Run gitlint with rule in .gitlint

##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree

##############################
Test: Make Check - PASS
Desc: Run 'make check'

##############################
Test: Make Distcheck - PASS
Desc: Run distcheck to check the distribution

##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration



---
Regards,
Linux Bluetooth
Marcel Holtmann June 26, 2021, 5:44 a.m. UTC | #3
Hi Luiz,

> This introduces AuthenticationFailed method which is used to indicate
> when there was an authentication failure with a remote device which is
> recommended by the core spec:
> 
> BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1314:
> '5.2.2.3 Simple Pairing after authentication failure
> When the link key is stored, subsequent connections to the same device
> will use authentication but this may fail if the remote device has
> deleted the link key. Table 5.2 defines what shall be done depending
> on the type of the link key and whether bonding was performed or not.'
> 
> So following Table 5.2 recommendation the AuthenticationFailed method
> would be called when authentication fails with a bonded device.
> ---
> doc/agent-api.txt | 5 +++++
> 1 file changed, 5 insertions(+)
> 
> diff --git a/doc/agent-api.txt b/doc/agent-api.txt
> index 0d9347cab..91d752b0a 100644
> --- a/doc/agent-api.txt
> +++ b/doc/agent-api.txt
> @@ -183,3 +183,8 @@ Methods		void Release()
> 
> 			This method gets called to indicate that the agent
> 			request failed before a reply was returned.
> +
> +		void AuthenticationFailed(object device, byte status)
> +
> +			This method indicates that there was an authentication
> +			failure with a remote device.

you are forgetting to describe what status is for.

Also if we follow the language from the previous agent method, then this should be IndicateAuthenticationFailure. I know it is a mouthful, but we always phrased the method names as action or information targeting a user or UI to do something.

Regards

Marcel
Bastien Nocera June 28, 2021, 2:59 p.m. UTC | #4
On Wed, 2021-06-23 at 11:43 -0700, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This introduces AuthenticationFailed method which is used to indicate
> when there was an authentication failure with a remote device which is
> recommended by the core spec:
> 
> BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1314:
>  '5.2.2.3 Simple Pairing after authentication failure
>  When the link key is stored, subsequent connections to the same device
>  will use authentication but this may fail if the remote device has
>  deleted the link key. Table 5.2 defines what shall be done depending
>  on the type of the link key and whether bonding was performed or not.'
> 
> So following Table 5.2 recommendation the AuthenticationFailed method
> would be called when authentication fails with a bonded device.
> ---
>  doc/agent-api.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/doc/agent-api.txt b/doc/agent-api.txt
> index 0d9347cab..91d752b0a 100644
> --- a/doc/agent-api.txt
> +++ b/doc/agent-api.txt
> @@ -183,3 +183,8 @@ Methods             void Release()
>  
>                         This method gets called to indicate that the
> agent
>                         request failed before a reply was returned.
> +
> +               void AuthenticationFailed(object device, byte status)
> +
> +                       This method indicates that there was an
> authentication
> +                       failure with a remote device.

Is there a way that we could have some persistent storage for when the
last authentication error happened?

This would allow showing a /!\ icon in the device list even though we
don't have an agent running in the session, just when the settings are
open, in GNOME.

This would also allow the settings to get this information in the first
place, otherwise we'd need to have some side-channel to funnel that
information from the "desktop-wide" agent to the settings agent.

I've made the upstream bug[1] public now, can you reference it in the
commit message?

Cheers

[1]: https://gitlab.gnome.org/GNOME/gnome-bluetooth/-/issues/91
Luiz Augusto von Dentz June 28, 2021, 5:32 p.m. UTC | #5
Hi Bastien,

On Mon, Jun 28, 2021 at 7:59 AM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Wed, 2021-06-23 at 11:43 -0700, Luiz Augusto von Dentz wrote:
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > This introduces AuthenticationFailed method which is used to indicate
> > when there was an authentication failure with a remote device which is
> > recommended by the core spec:
> >
> > BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1314:
> >  '5.2.2.3 Simple Pairing after authentication failure
> >  When the link key is stored, subsequent connections to the same device
> >  will use authentication but this may fail if the remote device has
> >  deleted the link key. Table 5.2 defines what shall be done depending
> >  on the type of the link key and whether bonding was performed or not.'
> >
> > So following Table 5.2 recommendation the AuthenticationFailed method
> > would be called when authentication fails with a bonded device.
> > ---
> >  doc/agent-api.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/doc/agent-api.txt b/doc/agent-api.txt
> > index 0d9347cab..91d752b0a 100644
> > --- a/doc/agent-api.txt
> > +++ b/doc/agent-api.txt
> > @@ -183,3 +183,8 @@ Methods             void Release()
> >
> >                         This method gets called to indicate that the
> > agent
> >                         request failed before a reply was returned.
> > +
> > +               void AuthenticationFailed(object device, byte status)
> > +
> > +                       This method indicates that there was an
> > authentication
> > +                       failure with a remote device.
>
> Is there a way that we could have some persistent storage for when the
> last authentication error happened?
>
> This would allow showing a /!\ icon in the device list even though we
> don't have an agent running in the session, just when the settings are
> open, in GNOME.

I guess you are suggesting introducing a property to the device object
e.g. AuthenticationStatus or something like that, that way one can
figure out if the device needs repairing or not. Alternatively we
could just remove the device, after a few attempts, if there is no
agent of the agent don't implement the new method.

> This would also allow the settings to get this information in the first
> place, otherwise we'd need to have some side-channel to funnel that
> information from the "desktop-wide" agent to the settings agent.
>
> I've made the upstream bug[1] public now, can you reference it in the
> commit message?

Sure.

> Cheers
>
> [1]: https://gitlab.gnome.org/GNOME/gnome-bluetooth/-/issues/91
>
Bastien Nocera June 28, 2021, 7:56 p.m. UTC | #6
On Mon, 2021-06-28 at 10:32 -0700, Luiz Augusto von Dentz wrote:
> Hi Bastien,
> 
> On Mon, Jun 28, 2021 at 7:59 AM Bastien Nocera <hadess@hadess.net>
> wrote:
> > 
> > On Wed, 2021-06-23 at 11:43 -0700, Luiz Augusto von Dentz wrote:
> > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > 
> > > This introduces AuthenticationFailed method which is used to
> > > indicate
> > > when there was an authentication failure with a remote device
> > > which is
> > > recommended by the core spec:
> > > 
> > > BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page
> > > 1314:
> > >  '5.2.2.3 Simple Pairing after authentication failure
> > >  When the link key is stored, subsequent connections to the same
> > > device
> > >  will use authentication but this may fail if the remote device
> > > has
> > >  deleted the link key. Table 5.2 defines what shall be done
> > > depending
> > >  on the type of the link key and whether bonding was performed or
> > > not.'
> > > 
> > > So following Table 5.2 recommendation the AuthenticationFailed
> > > method
> > > would be called when authentication fails with a bonded device.
> > > ---
> > >  doc/agent-api.txt | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/doc/agent-api.txt b/doc/agent-api.txt
> > > index 0d9347cab..91d752b0a 100644
> > > --- a/doc/agent-api.txt
> > > +++ b/doc/agent-api.txt
> > > @@ -183,3 +183,8 @@ Methods             void Release()
> > > 
> > >                         This method gets called to indicate that
> > > the
> > > agent
> > >                         request failed before a reply was
> > > returned.
> > > +
> > > +               void AuthenticationFailed(object device, byte
> > > status)
> > > +
> > > +                       This method indicates that there was an
> > > authentication
> > > +                       failure with a remote device.
> > 
> > Is there a way that we could have some persistent storage for when
> > the
> > last authentication error happened?
> > 
> > This would allow showing a /!\ icon in the device list even though
> > we
> > don't have an agent running in the session, just when the settings
> > are
> > open, in GNOME.
> 
> I guess you are suggesting introducing a property to the device
> object
> e.g. AuthenticationStatus or something like that, that way one can
> figure out if the device needs repairing or not.

That would work pretty well, yes.

>  Alternatively we
> could just remove the device, after a few attempts, if there is no
> agent of the agent don't implement the new method.

This is very much a dark pattern. I can expect to get bug reports about
devices disappearing from the known devices list on the computer when
it's actually the device losing its link keys for example.

I wouldn't recommend doing this at all, or by default.

> 
> > This would also allow the settings to get this information in the
> > first
> > place, otherwise we'd need to have some side-channel to funnel that
> > information from the "desktop-wide" agent to the settings agent.
> > 
> > I've made the upstream bug[1] public now, can you reference it in
> > the
> > commit message?
> 
> Sure.
> 
> > Cheers
> > 
> > [1]: https://gitlab.gnome.org/GNOME/gnome-bluetooth/-/issues/91
> > 
> 
>
diff mbox series

Patch

diff --git a/doc/agent-api.txt b/doc/agent-api.txt
index 0d9347cab..91d752b0a 100644
--- a/doc/agent-api.txt
+++ b/doc/agent-api.txt
@@ -183,3 +183,8 @@  Methods		void Release()
 
 			This method gets called to indicate that the agent
 			request failed before a reply was returned.
+
+		void AuthenticationFailed(object device, byte status)
+
+			This method indicates that there was an authentication
+			failure with a remote device.