---
TODO | 6 ------
attrib/gatttool.c | 15 +++++++++++++--
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/TODO b/TODO
index c0a25f1..49a9e76 100644
--- a/TODO
+++ b/TODO
@@ -123,12 +123,6 @@ ATT/GATT
Priority: Low
Complexity: C1
-- Add command line support to use medium instead of (default) low
- security level with gatttool (--sec-level)
-
- Priority: Low
- Complexity: C1
-
- Implement Server characteristic Configuration support in the attribute
server to manage characteristic value broadcasting. There is a single
instance of the Server Characteristic Configuration for all clients.
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index b9f5138..bb4fb80 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -49,6 +49,7 @@
static gchar *opt_src = NULL;
static gchar *opt_dst = NULL;
static gchar *opt_value = NULL;
+static gchar *opt_sec_level = "low";
static int opt_start = 0x0001;
static int opt_end = 0xffff;
static int opt_handle = -1;
@@ -84,6 +85,7 @@ static GIOChannel *do_connect(gboolean le)
GIOChannel *chan;
bdaddr_t sba, dba;
GError *err = NULL;
+ BtIOSecLevel sec_level;
/* This check is required because currently setsockopt() returns no
* errors for MTU values smaller than the allowed minimum. */
@@ -109,13 +111,20 @@ static GIOChannel *do_connect(gboolean le)
} else
bacpy(&sba, BDADDR_ANY);
+ if (strncmp(opt_sec_level, "medium", 6) == 0)
+ sec_level = BT_IO_SEC_MEDIUM;
+ else if (strncmp(opt_sec_level, "high", 4) == 0)
+ sec_level = BT_IO_SEC_HIGH;
+ else
+ sec_level = BT_IO_SEC_LOW;
+
if (le)
chan = bt_io_connect(BT_IO_L2CAP, connect_cb, NULL, NULL, &err,
BT_IO_OPT_SOURCE_BDADDR, &sba,
BT_IO_OPT_DEST_BDADDR, &dba,
BT_IO_OPT_CID, GATT_CID,
BT_IO_OPT_OMTU, opt_mtu,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);
else
chan = bt_io_connect(BT_IO_L2CAP, connect_cb, NULL, NULL, &err,
@@ -123,7 +132,7 @@ static GIOChannel *do_connect(gboolean le)
BT_IO_OPT_DEST_BDADDR, &dba,
BT_IO_OPT_PSM, opt_psm,
BT_IO_OPT_OMTU, opt_mtu,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_LOW,
+ BT_IO_OPT_SEC_LEVEL, sec_level,
BT_IO_OPT_INVALID);
if (err) {
@@ -519,6 +528,8 @@ static GOptionEntry options[] = {
"Specify the MTU size", "MTU" },
{ "psm", 'p', 0, G_OPTION_ARG_INT, &opt_psm,
"Specify the PSM for GATT/ATT over BR/EDR", "PSM" },
+ { "sec-level", 'l', 0, G_OPTION_ARG_STRING, &opt_sec_level,
+ "Set security level. Default: low", "[low | medium | high]"},
{ NULL },
};
--
1.6.2.rc2
Hi Tim,
Nice to see you on this list! :)
On Wed, Nov 17, 2010, [email protected] wrote:
> > [Mtsai] I am not sure what are the definition of "low", "medium" or
> > "high". By the spec of Core 4.0, LE has 2 security modes and different
> > security levels based on the method of pairing (or bonding). It may be
> > appeal to end user with "low", "medium" and "high" definition, but it
> > can't be reference with LE spec. I would suggest, instead, following
> > terms,
> >
> > "No security",
> > "unauthenticated encryption",
> > "authenticated encryption",
> > "unauthenticated data signing",
> > "authenticated data signing,
>
> To some extent I agree; however, the semantics of such an API would
> have to be careful. A particular profile should not "force" data
> signing because if the link is already encrypted there is little point
> using data signing. So from that point of view exposing a more
> abstract API (a bit like "high") is better. However, it is hard to
> map "high" onto any of the ones you listed (which I agree is a good
> list). So perhaps it is better to have the API semantics as
> "advisory" or "requests" which can be fulfilled by the underlying
> stack in other ways (eg encryption for data-signing).
Something like that will probably be needed, yes. However the idea of
the current command line switch to gatttool is to simply map to the
existing kernel API, and that API only has low, medium and high. So at
least in the short term the patch is fine.
Johan
Hi Mike,
-----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-
> [email protected]] On Behalf Of Johan Hedberg
> Sent: Tuesday, November 16, 2010 7:37 AM
> To: Sheldon Demario
> Cc: [email protected]
> Subject: Re: [PATCH] Adding a new option to specify security level for
> gatttool
>
> Hi Sheldon,
>
> On Tue, Nov 16, 2010, Sheldon Demario wrote:
> > ---
> > TODO | 6 ------
> > attrib/gatttool.c | 15 +++++++++++++--
> > 2 files changed, 13 insertions(+), 8 deletions(-)
>
> In general the patch seems fine, but:
>
> > + if (strncmp(opt_sec_level, "medium", 6) == 0)
> > + sec_level = BT_IO_SEC_MEDIUM;
> > + else if (strncmp(opt_sec_level, "high", 4) == 0)
> > + sec_level = BT_IO_SEC_HIGH;
> > + else
> > + sec_level = BT_IO_SEC_LOW;
>
> Why do you use strncmp instead of strcmp (or even g_str_equal)? I don't
> think there's any need to map e.g. "mediumfoobar" to BT_IO_SEC_MEDIUM
> ;)
>
> [Mtsai] I am not sure what are the definition of "low", "medium" or
> "high". By the spec of Core 4.0, LE has 2 security modes and different
> security levels based on the method of pairing (or bonding). It may be
> appeal to end user with "low", "medium" and "high" definition, but it
> can't be reference with LE spec. I would suggest, instead, following
> terms,
>
> "No security",
> "unauthenticated encryption",
> "authenticated encryption",
> "unauthenticated data signing",
> "authenticated data signing,
>
> Mike
To some extent I agree; however, the semantics of such an API would have to be careful. A particular profile should not "force" data signing because if the link is already encrypted there is little point using data signing. So from that point of view exposing a more abstract API (a bit like "high") is better. However, it is hard to map "high" onto any of the ones you listed (which I agree is a good list). So perhaps it is better to have the API semantics as "advisory" or "requests" which can be fulfilled by the underlying stack in other ways (eg encryption for data-signing).
Cheers
Tim
This message is for the designated recipient only and may contain privileged, proprietary, or otherwise private information. If you have received it in error, please notify the sender immediately and delete the original. Any other use of the email by you is prohibited.
Hi Tim,
-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: Wednesday, November 17, 2010 7:04 AM
To: Mike Tsai
Cc: [email protected]
Subject: RE: [PATCH] Adding a new option to specify security level for gatttool
Hi Mike,
-----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-
> [email protected]] On Behalf Of Johan Hedberg
> Sent: Tuesday, November 16, 2010 7:37 AM
> To: Sheldon Demario
> Cc: [email protected]
> Subject: Re: [PATCH] Adding a new option to specify security level for
> gatttool
>
> Hi Sheldon,
>
> On Tue, Nov 16, 2010, Sheldon Demario wrote:
> > ---
> > TODO | 6 ------
> > attrib/gatttool.c | 15 +++++++++++++--
> > 2 files changed, 13 insertions(+), 8 deletions(-)
>
> In general the patch seems fine, but:
>
> > + if (strncmp(opt_sec_level, "medium", 6) == 0)
> > + sec_level = BT_IO_SEC_MEDIUM;
> > + else if (strncmp(opt_sec_level, "high", 4) == 0)
> > + sec_level = BT_IO_SEC_HIGH;
> > + else
> > + sec_level = BT_IO_SEC_LOW;
>
> Why do you use strncmp instead of strcmp (or even g_str_equal)? I don't
> think there's any need to map e.g. "mediumfoobar" to BT_IO_SEC_MEDIUM
> ;)
>
> [Mtsai] I am not sure what are the definition of "low", "medium" or
> "high". By the spec of Core 4.0, LE has 2 security modes and different
> security levels based on the method of pairing (or bonding). It may be
> appeal to end user with "low", "medium" and "high" definition, but it
> can't be reference with LE spec. I would suggest, instead, following
> terms,
>
> "No security",
> "unauthenticated encryption",
> "authenticated encryption",
> "unauthenticated data signing",
> "authenticated data signing,
>
> Mike
To some extent I agree; however, the semantics of such an API would have to be careful. A particular profile should not "force" data signing because if the link is already encrypted there is little point using data signing. So from that point of view exposing a more abstract API (a bit like "high") is better. However, it is hard to map "high" onto any of the ones you listed (which I agree is a good list). So perhaps it is better to have the API semantics as "advisory" or "requests" which can be fulfilled by the underlying stack in other ways (eg encryption for data-signing).
Cheers
Tim
[Mtsai] I fully embrace the "abstract" API concept. However, all new LE profiles have clearly defined the security request per each attribute now. As a generic attribute profile (as GATT), without clear indication from Application what level of security is requested, GATT/GAP can handle the security request incorrectly and cause IOP issues. For example, what does "low" mean when GAP/SM gets this request from Application? It does not tell GAP/SM what kind of pairing shall be performed and if encryption or data signing for accessing the attributes.
Cheers,
Mike
This message is for the designated recipient only and may contain privileged, proprietary, or otherwise private information. If you have received it in error, please notify the sender immediately and delete the original. Any other use of the email by you is prohibited.
Hi,
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Johan Hedberg
Sent: Tuesday, November 16, 2010 7:37 AM
To: Sheldon Demario
Cc: [email protected]
Subject: Re: [PATCH] Adding a new option to specify security level for gatttool
Hi Sheldon,
On Tue, Nov 16, 2010, Sheldon Demario wrote:
> ---
> TODO | 6 ------
> attrib/gatttool.c | 15 +++++++++++++--
> 2 files changed, 13 insertions(+), 8 deletions(-)
In general the patch seems fine, but:
> + if (strncmp(opt_sec_level, "medium", 6) == 0)
> + sec_level = BT_IO_SEC_MEDIUM;
> + else if (strncmp(opt_sec_level, "high", 4) == 0)
> + sec_level = BT_IO_SEC_HIGH;
> + else
> + sec_level = BT_IO_SEC_LOW;
Why do you use strncmp instead of strcmp (or even g_str_equal)? I don't
think there's any need to map e.g. "mediumfoobar" to BT_IO_SEC_MEDIUM ;)
[Mtsai] I am not sure what are the definition of "low", "medium" or "high". By the spec of Core 4.0, LE has 2 security modes and different security levels based on the method of pairing (or bonding). It may be appeal to end user with "low", "medium" and "high" definition, but it can't be reference with LE spec. I would suggest, instead, following terms,
"No security",
"unauthenticated encryption",
"authenticated encryption",
"unauthenticated data signing",
"authenticated data signing,
Mike
Johan
Hi Sheldon,
On Tue, Nov 16, 2010, Sheldon Demario wrote:
> ---
> TODO | 6 ------
> attrib/gatttool.c | 15 +++++++++++++--
> 2 files changed, 13 insertions(+), 8 deletions(-)
In general the patch seems fine, but:
> + if (strncmp(opt_sec_level, "medium", 6) == 0)
> + sec_level = BT_IO_SEC_MEDIUM;
> + else if (strncmp(opt_sec_level, "high", 4) == 0)
> + sec_level = BT_IO_SEC_HIGH;
> + else
> + sec_level = BT_IO_SEC_LOW;
Why do you use strncmp instead of strcmp (or even g_str_equal)? I don't
think there's any need to map e.g. "mediumfoobar" to BT_IO_SEC_MEDIUM ;)
Johan