As defined in Core Specification 4.2:
"If a connection is already encrypted with LE security mode 1, level 2
or level 3 as defined in [Vol 3] Part C, Section 10.2 then, a Write
Without Response as defined in Section 4.9.1 shall be used instead of
a Signed Write Without Response."
---
android/gatt.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index 53b1983..9e348d7 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -3358,14 +3358,14 @@ static void handle_client_write_characteristic(const void *buf, uint16_t len)
goto failed;
}
- if (get_sec_level(conn->device) != BT_SECURITY_LOW) {
- error("gatt: Cannot write signed on encrypted link");
- status = HAL_STATUS_FAILED;
- goto failed;
- }
-
- res = signed_write_cmd(conn->device, ch->ch.value_handle,
- cmd->value, cmd->len);
+ if (get_sec_level(conn->device) > BT_SECURITY_LOW)
+ res = gatt_write_cmd(conn->device->attrib,
+ ch->ch.value_handle, cmd->value,
+ cmd->len, NULL, NULL);
+ else
+ res = signed_write_cmd(conn->device,
+ ch->ch.value_handle, cmd->value,
+ cmd->len);
break;
default:
error("gatt: Write type %d unsupported", cmd->write_type);
--
1.9.3
On Friday 27 of February 2015 16:01:09 Szymon Janc wrote:
> As defined in Core Specification 4.2:
> "If a connection is already encrypted with LE security mode 1, level 2
> or level 3 as defined in [Vol 3] Part C, Section 10.2 then, a Write
> Without Response as defined in Section 4.9.1 shall be used instead of
> a Signed Write Without Response."
> ---
> android/gatt.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/android/gatt.c b/android/gatt.c
> index 53b1983..9e348d7 100644
> --- a/android/gatt.c
> +++ b/android/gatt.c
> @@ -3358,14 +3358,14 @@ static void handle_client_write_characteristic(const
> void *buf, uint16_t len) goto failed;
> }
>
> - if (get_sec_level(conn->device) != BT_SECURITY_LOW) {
> - error("gatt: Cannot write signed on encrypted link");
> - status = HAL_STATUS_FAILED;
> - goto failed;
> - }
> -
> - res = signed_write_cmd(conn->device, ch->ch.value_handle,
> - cmd->value, cmd->len);
> + if (get_sec_level(conn->device) > BT_SECURITY_LOW)
> + res = gatt_write_cmd(conn->device->attrib,
> + ch->ch.value_handle, cmd->value,
> + cmd->len, NULL, NULL);
> + else
> + res = signed_write_cmd(conn->device,
> + ch->ch.value_handle, cmd->value,
> + cmd->len);
> break;
> default:
> error("gatt: Write type %d unsupported", cmd->write_type);
Patches 1-5 are applied.
--
BR
Szymon Janc
Hi Szymon,
On Fri, Feb 27, 2015 at 5:27 PM, Szymon Janc <[email protected]> wrote:
> Hi Michael,
>
> On Friday 27 of February 2015 07:21:51 Michael Janssen wrote:
>> Hi Szymon,
>>
>> On Fri, Feb 27, 2015 at 7:01 AM, Szymon Janc <[email protected]> wrote:
>> > If local key wasn't set by bt_att_set_local_key() assume that signing
>> > is done by bt_att_send() caller ie. gattrib code.
>> > ---
>> > src/shared/att.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/src/shared/att.c b/src/shared/att.c
>> > index 9787377..b20af21 100644
>> > --- a/src/shared/att.c
>> > +++ b/src/shared/att.c
>> > @@ -308,7 +308,7 @@ static bool encode_pdu(struct bt_att *att, struct att_send_op *op,
>> >
>> > sign = att->local_sign;
>> > if (!sign)
>> > - goto fail;
>> > + true;
>>
>> This should be 'return true;' I think?
>
> Good catch. I've got this right locally but forgot to amend before generating
> patch :)
Yep, but as discussed in the IRC there are more problems with it
because we increase the data length before this, I guess we are doing
to need some refactoring to fix this properly.
--
Luiz Augusto von Dentz
Hi Michael,
On Friday 27 of February 2015 07:21:51 Michael Janssen wrote:
> Hi Szymon,
>
> On Fri, Feb 27, 2015 at 7:01 AM, Szymon Janc <[email protected]> wrote:
> > If local key wasn't set by bt_att_set_local_key() assume that signing
> > is done by bt_att_send() caller ie. gattrib code.
> > ---
> > src/shared/att.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/shared/att.c b/src/shared/att.c
> > index 9787377..b20af21 100644
> > --- a/src/shared/att.c
> > +++ b/src/shared/att.c
> > @@ -308,7 +308,7 @@ static bool encode_pdu(struct bt_att *att, struct att_send_op *op,
> >
> > sign = att->local_sign;
> > if (!sign)
> > - goto fail;
> > + true;
>
> This should be 'return true;' I think?
Good catch. I've got this right locally but forgot to amend before generating
patch :)
>
> >
> > if (!sign->counter(&sign_cnt, sign->user_data))
> > goto fail;
> > --
> > 1.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
>
--
Best regards,
Szymon Janc
Hi Szymon,
On Fri, Feb 27, 2015 at 7:01 AM, Szymon Janc <[email protected]> wrote:
> If local key wasn't set by bt_att_set_local_key() assume that signing
> is done by bt_att_send() caller ie. gattrib code.
> ---
> src/shared/att.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/shared/att.c b/src/shared/att.c
> index 9787377..b20af21 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -308,7 +308,7 @@ static bool encode_pdu(struct bt_att *att, struct att_send_op *op,
>
> sign = att->local_sign;
> if (!sign)
> - goto fail;
> + true;
This should be 'return true;' I think?
>
> if (!sign->counter(&sign_cnt, sign->user_data))
> goto fail;
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Michael Janssen
If local key wasn't set by bt_att_set_local_key() assume that signing
is done by bt_att_send() caller ie. gattrib code.
---
src/shared/att.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/shared/att.c b/src/shared/att.c
index 9787377..b20af21 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -308,7 +308,7 @@ static bool encode_pdu(struct bt_att *att, struct att_send_op *op,
sign = att->local_sign;
if (!sign)
- goto fail;
+ true;
if (!sign->counter(&sign_cnt, sign->user_data))
goto fail;
--
1.9.3
This allows to get information if CSRK is authenticated or not.
For client it is don't care if CSRK is authenticated or not.
---
android/bluetooth.c | 35 ++++++++++++++++++++++-------------
android/bluetooth.h | 12 +++---------
android/gatt.c | 8 ++++----
3 files changed, 29 insertions(+), 26 deletions(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index dca6f48..e684d31 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -4098,22 +4098,33 @@ bool bt_read_device_rssi(const bdaddr_t *addr, bt_read_device_rssi_done cb,
return true;
}
-bool bt_get_csrk(const bdaddr_t *addr, enum bt_csrk_type type, uint8_t key[16],
- uint32_t *sign_cnt)
+bool bt_get_csrk(const bdaddr_t *addr, bool local, uint8_t key[16],
+ uint32_t *sign_cnt, bool *authenticated)
{
struct device *dev;
- bool local = (type == LOCAL_CSRK);
dev = find_device(addr);
if (!dev)
return false;
if (local && dev->valid_local_csrk) {
- memcpy(key, dev->local_csrk, 16);
- *sign_cnt = dev->local_sign_cnt;
+ if (key)
+ memcpy(key, dev->local_csrk, 16);
+
+ if (sign_cnt)
+ *sign_cnt = dev->local_sign_cnt;
+
+ if (authenticated)
+ *authenticated = dev->local_csrk_auth;
} else if (!local && dev->valid_remote_csrk) {
- memcpy(key, dev->remote_csrk, 16);
- *sign_cnt = dev->remote_sign_cnt;
+ if (key)
+ memcpy(key, dev->remote_csrk, 16);
+
+ if (sign_cnt)
+ *sign_cnt = dev->remote_sign_cnt;
+
+ if (authenticated)
+ *authenticated = dev->remote_csrk_auth;
} else {
return false;
}
@@ -4121,12 +4132,11 @@ bool bt_get_csrk(const bdaddr_t *addr, enum bt_csrk_type type, uint8_t key[16],
return true;
}
-static void store_sign_counter(struct device *dev, enum bt_csrk_type type)
+static void store_sign_counter(struct device *dev, bool local)
{
const char *sign_cnt_s;
uint32_t sign_cnt;
GKeyFile *key_file;
- bool local = (type == LOCAL_CSRK);
gsize length = 0;
char addr[18];
@@ -4152,8 +4162,7 @@ static void store_sign_counter(struct device *dev, enum bt_csrk_type type)
g_key_file_free(key_file);
}
-void bt_update_sign_counter(const bdaddr_t *addr, enum bt_csrk_type type,
- uint32_t val)
+void bt_update_sign_counter(const bdaddr_t *addr, bool local, uint32_t val)
{
struct device *dev;
@@ -4161,12 +4170,12 @@ void bt_update_sign_counter(const bdaddr_t *addr, enum bt_csrk_type type,
if (!dev)
return;
- if (type == LOCAL_CSRK)
+ if (local)
dev->local_sign_cnt = val;
else
dev->remote_sign_cnt = val;
- store_sign_counter(dev, type);
+ store_sign_counter(dev, local);
}
static uint8_t set_adapter_scan_mode(const void *buf, uint16_t len)
diff --git a/android/bluetooth.h b/android/bluetooth.h
index 9e7ab2c..8b8a1f0 100644
--- a/android/bluetooth.h
+++ b/android/bluetooth.h
@@ -21,11 +21,6 @@
*
*/
-enum bt_csrk_type {
- LOCAL_CSRK,
- REMOTE_CSRK,
-};
-
typedef void (*bt_bluetooth_ready)(int err, const bdaddr_t *addr);
bool bt_bluetooth_start(int index, bool mgmt_dbg, bt_bluetooth_ready cb);
@@ -69,11 +64,10 @@ typedef void (*bt_read_device_rssi_done)(uint8_t status, const bdaddr_t *addr,
bool bt_read_device_rssi(const bdaddr_t *addr, bt_read_device_rssi_done cb,
void *user_data);
-bool bt_get_csrk(const bdaddr_t *addr, enum bt_csrk_type type,
- uint8_t key[16], uint32_t *sign_cnt);
+bool bt_get_csrk(const bdaddr_t *addr, bool local, uint8_t key[16],
+ uint32_t *sign_cnt, bool *authenticated);
-void bt_update_sign_counter(const bdaddr_t *addr, enum bt_csrk_type type,
- uint32_t val);
+void bt_update_sign_counter(const bdaddr_t *addr, bool local, uint32_t val);
void bt_store_gatt_ccc(const bdaddr_t *addr, uint16_t value);
diff --git a/android/gatt.c b/android/gatt.c
index 9e348d7..3f98e71 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -3263,7 +3263,7 @@ static guint signed_write_cmd(struct gatt_device *dev, uint16_t handle,
memset(csrk, 0, 16);
- if (!bt_get_csrk(&dev->bdaddr, LOCAL_CSRK, csrk, &sign_cnt)) {
+ if (!bt_get_csrk(&dev->bdaddr, true, csrk, &sign_cnt, NULL)) {
error("gatt: Could not get csrk key");
return 0;
}
@@ -3275,7 +3275,7 @@ static guint signed_write_cmd(struct gatt_device *dev, uint16_t handle,
return 0;
}
- bt_update_sign_counter(&dev->bdaddr, LOCAL_CSRK, ++sign_cnt);
+ bt_update_sign_counter(&dev->bdaddr, true, ++sign_cnt);
return res;
}
@@ -6396,7 +6396,7 @@ static void write_signed_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
return;
}
- if (!bt_get_csrk(&dev->bdaddr, REMOTE_CSRK, csrk, &sign_cnt)) {
+ if (!bt_get_csrk(&dev->bdaddr, false, csrk, &sign_cnt, NULL)) {
error("gatt: No valid csrk from remote device");
return;
}
@@ -6438,7 +6438,7 @@ static void write_signed_cmd_request(const uint8_t *cmd, uint16_t cmd_len,
return;
}
/* Signature OK, proceed with write */
- bt_update_sign_counter(&dev->bdaddr, REMOTE_CSRK, r_sign_cnt);
+ bt_update_sign_counter(&dev->bdaddr, false, r_sign_cnt);
gatt_db_attribute_write(attrib, 0, value, vlen, cmd[0],
g_attrib_get_att(dev->attrib),
write_confirm, NULL);
--
1.9.3
This adds support for storing information if CSRK is authenticated.
---
android/bluetooth.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index d017418..dca6f48 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -120,10 +120,12 @@ struct device {
unsigned int confirm_id; /* mgtm command id if command pending */
bool valid_remote_csrk;
+ bool remote_csrk_auth;
uint8_t remote_csrk[16];
uint32_t remote_sign_cnt;
bool valid_local_csrk;
+ bool local_csrk_auth;
uint8_t local_csrk[16];
uint32_t local_sign_cnt;
uint16_t gatt_ccc;
@@ -2320,6 +2322,9 @@ static void store_csrk(struct device *dev)
dev->local_csrk[i]);
g_key_file_set_string(key_file, addr, "LocalCSRK", key_str);
+
+ g_key_file_set_boolean(key_file, addr, "LocalCSRKAuthenticated",
+ dev->local_csrk_auth);
}
if (dev->valid_remote_csrk) {
@@ -2328,6 +2333,10 @@ static void store_csrk(struct device *dev)
dev->remote_csrk[i]);
g_key_file_set_string(key_file, addr, "RemoteCSRK", key_str);
+
+ g_key_file_set_boolean(key_file, addr,
+ "RemoteCSRKAuthenticated",
+ dev->remote_csrk_auth);
}
data = g_key_file_to_data(key_file, &length, NULL);
@@ -2360,12 +2369,14 @@ static void new_csrk_callback(uint16_t index, uint16_t length,
memcpy(dev->local_csrk, ev->key.val, 16);
dev->local_sign_cnt = 0;
dev->valid_local_csrk = true;
+ dev->local_csrk_auth = ev->key.type == 0x02;
break;
case 0x01:
case 0x03:
memcpy(dev->remote_csrk, ev->key.val, 16);
dev->remote_sign_cnt = 0;
dev->valid_remote_csrk = true;
+ dev->remote_csrk_auth = ev->key.type == 0x03;
break;
default:
error("Unknown CSRK key type 02%02x", ev->key.type);
@@ -2962,6 +2973,9 @@ static struct device *create_device_from_info(GKeyFile *key_file,
dev->local_sign_cnt = g_key_file_get_integer(key_file, peer,
"LocalCSRKSignCounter", NULL);
+
+ dev->local_csrk_auth = g_key_file_get_boolean(key_file, peer,
+ "LocalCSRKAuthenticated", NULL);
}
str = g_key_file_get_string(key_file, peer, "RemoteCSRK", NULL);
@@ -2976,6 +2990,10 @@ static struct device *create_device_from_info(GKeyFile *key_file,
dev->remote_sign_cnt = g_key_file_get_integer(key_file, peer,
"RemoteCSRKSignCounter", NULL);
+
+ dev->remote_csrk_auth = g_key_file_get_boolean(key_file, peer,
+ "RemoteCSRKAuthenticated",
+ NULL);
}
str = g_key_file_get_string(key_file, peer, "GattCCC", NULL);
--
1.9.3
---
android/gatt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/android/gatt.c b/android/gatt.c
index 2e3a5c9..6e9181e 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -3271,7 +3271,7 @@ static guint signed_write_cmd(struct gatt_device *dev, uint16_t handle,
res = gatt_signed_write_cmd(dev->attrib, handle, value, vlen, crypto,
csrk, sign_cnt, NULL, NULL);
if (!res) {
- error("gatt: Could write signed cmd");
+ error("gatt: Could not write signed cmd");
return 0;
}
--
1.9.3
---
android/gatt.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/android/gatt.c b/android/gatt.c
index 3f98e71..2e3a5c9 100644
--- a/android/gatt.c
+++ b/android/gatt.c
@@ -4683,9 +4683,15 @@ static uint8_t check_device_permissions(struct gatt_device *device,
if (!(permissions & GATT_PERM_WRITE_SIGNED))
return ATT_ECODE_WRITE_NOT_PERM;
- if ((permissions & GATT_PERM_WRITE_SIGNED_MITM) &&
- sec_level < BT_SECURITY_HIGH)
+ if (permissions & GATT_PERM_WRITE_SIGNED_MITM) {
+ bool auth;
+
+ if (bt_get_csrk(&device->bdaddr, true, NULL, NULL,
+ &auth) && auth)
+ break;
+
return ATT_ECODE_AUTHENTICATION;
+ }
break;
case ATT_OP_READ_BY_TYPE_REQ:
case ATT_OP_READ_REQ:
--
1.9.3