All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: Howard Chung <howardchung@google.com>
Cc: "linux-bluetooth@vger.kernel.org"
	<linux-bluetooth@vger.kernel.org>,
	Yun-Hao Chung <howardchung@chromium.org>
Subject: Re: [Bluez PATCH v7 03/13] mcap: add adapter authorization
Date: Mon, 2 Aug 2021 11:49:50 -0700	[thread overview]
Message-ID: <CABBYNZJTrym0dmQ0DnBsAcoaHeAqA3z8gxV7DJRbMQ+jdnk2VQ@mail.gmail.com> (raw)
In-Reply-To: <20210802141140.Bluez.v7.3.If0cf6e1feb9e9cc8106793bcaea60202852d7095@changeid>

Hi Howard,

On Sun, Aug 1, 2021 at 11:13 PM Howard Chung <howardchung@google.com> wrote:
>
> From: Yun-Hao Chung <howardchung@chromium.org>
>
> Currently mcap is the only profile that doesn't request adatper
> authorization. This patch adds a argument when creating the mcap
> instance to set authorize method. The reason why we don't use
> btd_request_authorization directly like all other profiles is because
> tools/mcaptest includes the profile/health/mcap.h. If we add dependency
> to adapter.h in mcap.h, it will make mcaptest depend on adapter and be
> not able to build independently.
> ---
>
> (no changes since v1)
>
>  android/health.c       |  2 +-
>  profiles/health/hdp.c  |  1 +
>  profiles/health/mcap.c | 38 ++++++++++++++++++++++++++++++++++++--
>  profiles/health/mcap.h |  9 +++++++++
>  tools/mcaptest.c       |  2 +-
>  5 files changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/android/health.c b/android/health.c
> index 9a29964b1be2..de50db98e988 100644
> --- a/android/health.c
> +++ b/android/health.c
> @@ -2008,7 +2008,7 @@ bool bt_health_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode)
>                                         mcl_connected, mcl_reconnected,
>                                         mcl_disconnected, mcl_uncached,
>                                         NULL, /* CSP is not used right now */
> -                                       NULL, &err);
> +                                       NULL, NULL, &err);
>         if (!mcap) {
>                 error("health: MCAP instance creation failed %s", err->message);
>                 g_error_free(err);
> diff --git a/profiles/health/hdp.c b/profiles/health/hdp.c
> index 6bc41946fef3..efa8955efaea 100644
> --- a/profiles/health/hdp.c
> +++ b/profiles/health/hdp.c
> @@ -1347,6 +1347,7 @@ static gboolean update_adapter(struct hdp_adapter *hdp_adapter)
>                                 mcl_connected, mcl_reconnected,
>                                 mcl_disconnected, mcl_uncached,
>                                 NULL, /* CSP is not used by now */
> +                               btd_request_authorization,
>                                 hdp_adapter, &err);
>         if (hdp_adapter->mi == NULL) {
>                 error("Error creating the MCAP instance: %s", err->message);
> diff --git a/profiles/health/mcap.c b/profiles/health/mcap.c
> index be13af37a0b8..a7eaba51693a 100644
> --- a/profiles/health/mcap.c
> +++ b/profiles/health/mcap.c
> @@ -14,6 +14,7 @@
>  #endif
>
>  #define _GNU_SOURCE
> +#include <dbus/dbus.h>
>  #include <netinet/in.h>
>  #include <stdlib.h>
>  #include <errno.h>
> @@ -23,6 +24,7 @@
>  #include <glib.h>
>
>  #include "lib/bluetooth.h"
> +#include "lib/uuid.h"
>  #include "bluetooth/l2cap.h"
>  #include "btio/btio.h"
>  #include "src/log.h"
> @@ -1980,7 +1982,6 @@ static void set_mcl_conf(GIOChannel *chan, struct mcap_mcl *mcl)
>         mcl->state = MCL_CONNECTED;
>         mcl->role = MCL_ACCEPTOR;
>         mcl->req = MCL_AVAILABLE;
> -       mcl->cc = g_io_channel_ref(chan);
>         mcl->ctrl |= MCAP_CTRL_STD_OP;
>
>         mcap_sync_init(mcl);
> @@ -2005,19 +2006,38 @@ static void set_mcl_conf(GIOChannel *chan, struct mcap_mcl *mcl)
>                 mcl->mi->mcl_connected_cb(mcl, mcl->mi->user_data);
>  }
>
> +static void auth_callback(DBusError *derr, void *user_data)
> +{
> +       struct mcap_mcl *mcl = user_data;
> +
> +       if (derr) {
> +               error("Access denied: %s", derr->message);
> +               goto reject;
> +       }
> +
> +       set_mcl_conf(mcl->cc, mcl);
> +       return;
> +
> +reject:
> +       g_io_channel_shutdown(mcl->cc, TRUE, NULL);
> +       g_io_channel_unref(mcl->cc);
> +}
> +
>  static void connect_mcl_event_cb(GIOChannel *chan, GError *gerr,
>                                                         gpointer user_data)
>  {
>         struct mcap_instance *mi = user_data;
>         struct mcap_mcl *mcl;
> -       bdaddr_t dst;
> +       bdaddr_t src, dst;
>         char address[18], srcstr[18];
>         GError *err = NULL;
> +       guint ret;
>
>         if (gerr)
>                 return;
>
>         bt_io_get(chan, &err,
> +                       BT_IO_OPT_SOURCE_BDADDR, &src,
>                         BT_IO_OPT_DEST_BDADDR, &dst,
>                         BT_IO_OPT_DEST, address,
>                         BT_IO_OPT_INVALID);
> @@ -2044,6 +2064,18 @@ static void connect_mcl_event_cb(GIOChannel *chan, GError *gerr,
>                 mcl->next_mdl = (rand() % MCAP_MDLID_FINAL) + 1;
>         }
>
> +       mcl->cc = g_io_channel_ref(chan);
> +       if (mi->authorize_cb) {
> +               ret = mi->authorize_cb(&src, &dst, HDP_UUID, auth_callback,
> +                                                                       mcl);
> +               if (ret != 0)
> +                       return;
> +
> +               error("HDP: authorization for device %s failed", address);
> +               g_io_channel_unref(mcl->cc);
> +               goto drop;
> +       }
> +
>         set_mcl_conf(chan, mcl);
>
>         return;
> @@ -2060,6 +2092,7 @@ struct mcap_instance *mcap_create_instance(const bdaddr_t *src,
>                                         mcap_mcl_event_cb mcl_disconnected,
>                                         mcap_mcl_event_cb mcl_uncached,
>                                         mcap_info_ind_event_cb mcl_sync_info_ind,
> +                                       mcap_authorize_cb authorize_cb,
>                                         gpointer user_data,
>                                         GError **gerr)
>  {
> @@ -2089,6 +2122,7 @@ struct mcap_instance *mcap_create_instance(const bdaddr_t *src,
>         mi->mcl_disconnected_cb = mcl_disconnected;
>         mi->mcl_uncached_cb = mcl_uncached;
>         mi->mcl_sync_infoind_cb = mcl_sync_info_ind;
> +       mi->authorize_cb = authorize_cb;
>         mi->user_data = user_data;
>         mi->csp_enabled = FALSE;
>
> diff --git a/profiles/health/mcap.h b/profiles/health/mcap.h
> index 5a94c8b63bea..48b7d7846c57 100644
> --- a/profiles/health/mcap.h
> +++ b/profiles/health/mcap.h
> @@ -9,6 +9,8 @@
>   *
>   */

These changes to mcap.h are causing a lot problems with CI, it seems
the comments are not going over 80 columns so if you really need to
touch this file you might as well move the comments above each field,
or perhaps don't bother changing the health plugin since it is
normally not enabled by default and we might as well consider it
deprecated given that has been many years without maintenance and
afaik these profiles have been moved to LE at some point.

> +#include <dbus/dbus.h>
> +
>  #define MCAP_VERSION   0x0100  /* current version 01.00 */
>
>  /* bytes to get MCAP Supported Procedures */
> @@ -249,6 +251,11 @@ typedef void (* mcap_sync_set_cb) (struct mcap_mcl *mcl,
>                                         GError *err,
>                                         gpointer data);
>
> +typedef void mcap_auth_cb(DBusError *derr, void *user_data);
> +typedef guint (* mcap_authorize_cb) (const bdaddr_t *src, const bdaddr_t *dst,
> +                                       const char *uuid, mcap_auth_cb cb,
> +                                       void *user_data);
> +
>  struct mcap_mdl_cb {
>         mcap_mdl_event_cb               mdl_connected;  /* Remote device has created a MDL */
>         mcap_mdl_event_cb               mdl_closed;     /* Remote device has closed a MDL */
> @@ -271,6 +278,7 @@ struct mcap_instance {
>         mcap_mcl_event_cb       mcl_disconnected_cb;    /* MCL disconnected */
>         mcap_mcl_event_cb       mcl_uncached_cb;        /* MCL has been removed from MCAP cache */
>         mcap_info_ind_event_cb  mcl_sync_infoind_cb;    /* (CSP Master) Received info indication */
> +       mcap_authorize_cb       authorize_cb;           /* Method to request authorization */
>         gpointer                user_data;              /* Data to be provided in callbacks */
>         int                     ref;                    /* Reference counter */
>
> @@ -404,6 +412,7 @@ struct mcap_instance *mcap_create_instance(const bdaddr_t *src,
>                                         mcap_mcl_event_cb mcl_disconnected,
>                                         mcap_mcl_event_cb mcl_uncached,
>                                         mcap_info_ind_event_cb mcl_sync_info_ind,
> +                                       mcap_authorize_cb authorize_cb,
>                                         gpointer user_data,
>                                         GError **gerr);
>  void mcap_release_instance(struct mcap_instance *mi);
> diff --git a/tools/mcaptest.c b/tools/mcaptest.c
> index dcef0b908ac8..63ee22149a40 100644
> --- a/tools/mcaptest.c
> +++ b/tools/mcaptest.c
> @@ -434,7 +434,7 @@ int main(int argc, char *argv[])
>                                         mcl_connected, mcl_reconnected,
>                                         mcl_disconnected, mcl_uncached,
>                                         NULL, /* CSP is not used right now */
> -                                       NULL, &err);
> +                                       NULL, NULL, &err);
>
>         if (!mcap) {
>                 printf("MCAP instance creation failed %s\n", err->message);
> --
> 2.32.0.554.ge1b32706d8-goog
>


-- 
Luiz Augusto von Dentz

  reply	other threads:[~2021-08-02 18:50 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-02  6:12 [Bluez PATCH v7 00/13] Admin policy series Howard Chung
2021-08-02  6:12 ` [Bluez PATCH v7 01/13] core: add is_allowed property in btd_service Howard Chung
2021-08-02  6:50   ` Admin policy series bluez.test.bot
2021-08-02  6:12 ` [Bluez PATCH v7 02/13] core: add adapter and device allowed_uuid functions Howard Chung
2021-08-02  6:12 ` [Bluez PATCH v7 03/13] mcap: add adapter authorization Howard Chung
2021-08-02 18:49   ` Luiz Augusto von Dentz [this message]
2021-08-02  6:12 ` [Bluez PATCH v7 04/13] core: block not allowed UUID connect in auth Howard Chung
2021-08-02  6:12 ` [Bluez PATCH v7 05/13] core: add device_added and device_removed to adapter driver Howard Chung
2021-08-02  6:12 ` [Bluez PATCH v7 06/13] plugins: new plugin Howard Chung
2021-08-02  6:12 ` [Bluez PATCH v7 07/13] plugins/admin: add admin_policy adapter driver Howard Chung
2021-08-02  6:12 ` [Bluez PATCH v7 08/13] plugins/admin: add ServiceAllowList method Howard Chung
2021-08-02  6:12 ` [Bluez PATCH v7 09/13] plugins/admin: add ServiceAllowList property Howard Chung
2021-08-02  6:12 ` [Bluez PATCH v7 10/13] plugins/admin: add device callbacks Howard Chung
2021-08-02  6:12 ` [Bluez PATCH v7 11/13] plugins/admin: add AffectedByPolicy property Howard Chung
2021-08-02  6:12 ` [Bluez PATCH v7 12/13] plugins/admin: persist policy settings Howard Chung
2021-08-02 18:51   ` Luiz Augusto von Dentz
2021-08-02  6:12 ` [Bluez PATCH v7 13/13] doc: add description of admin policy Howard Chung

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABBYNZJTrym0dmQ0DnBsAcoaHeAqA3z8gxV7DJRbMQ+jdnk2VQ@mail.gmail.com \
    --to=luiz.dentz@gmail.com \
    --cc=howardchung@chromium.org \
    --cc=howardchung@google.com \
    --cc=linux-bluetooth@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.