* [PATCH v2] Loading keys that should be blocked from bluetoothd.
@ 2019-12-10 17:59 Alain Michaud
2019-12-15 21:34 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 2+ messages in thread
From: Alain Michaud @ 2019-12-10 17:59 UTC (permalink / raw)
To: BlueZ; +Cc: Marcel Holtmann, Alain Michaud
This changes will send a list of known bad keys that should be blocked
if supported by the kernel.
In particular keys from the Google Titan Security key are being blocked.
For additional information, please see
https://security.googleblog.com/2019/05/titan-keys-update.html
Signed-off-by: Alain Michaud <alainm@chromium.org>
---
lib/mgmt.h | 16 ++++++++++++
src/adapter.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/lib/mgmt.h b/lib/mgmt.h
index 570dec997..a99b18c88 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -583,6 +583,22 @@ struct mgmt_cp_set_phy_confguration {
uint32_t selected_phys;
} __packed;
+#define MGMT_OP_SET_BLOCKED_KEYS 0x0046
+
+#define HCI_BLOCKED_KEY_TYPE_LINKKEY 0x00
+#define HCI_BLOCKED_KEY_TYPE_LTK 0x01
+#define HCI_BLOCKED_KEY_TYPE_IRK 0x02
+
+struct mgmt_blocked_key_info {
+ uint8_t type;
+ uint8_t val[16];
+} __packed;
+
+struct mgmt_cp_set_blocked_keys {
+ uint16_t key_count;
+ struct mgmt_blocked_key_info keys[0];
+} __packed;
+#define MGMT_OP_SET_BLOCKED_KEYS_SIZE 0
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
diff --git a/src/adapter.c b/src/adapter.c
index cef25616f..2958870e6 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -99,10 +99,27 @@
#define DISTANCE_VAL_INVALID 0x7FFF
#define PATHLOSS_MAX 137
+/*
+ * These are known security keys that have been compromised.
+ * If this grows or there are needs to be platform specific, it is
+ * conceivable that these could be read from a config file.
+ */
+static const struct mgmt_blocked_key_info blocked_keys[] = {
+ /* Google Titan Security Keys */
+ { HCI_BLOCKED_KEY_TYPE_LTK,
+ {0xbf, 0x01, 0xfb, 0x9d, 0x4e, 0xf3, 0xbc, 0x36,
+ 0xd8, 0x74, 0xf5, 0x39, 0x41, 0x38, 0x68, 0x4c}},
+ { HCI_BLOCKED_KEY_TYPE_IRK,
+ {0xa5, 0x99, 0xba, 0xe4, 0xe1, 0x7c, 0xa6, 0x18,
+ 0x22, 0x8e, 0x07, 0x56, 0xb4, 0xe8, 0x5f, 0x01}},
+};
+
static DBusConnection *dbus_conn = NULL;
static bool kernel_conn_control = false;
+static bool kernel_blocked_keys_supported = false;
+
static GList *adapter_list = NULL;
static unsigned int adapter_remaining = 0;
static bool powering_down = false;
@@ -8568,6 +8585,40 @@ static bool set_static_addr(struct btd_adapter *adapter)
return false;
}
+static void set_blocked_keys_complete(uint8_t status, uint16_t length,
+ const void *param, void *user_data)
+{
+ struct btd_adapter *adapter = user_data;
+
+ if (status != MGMT_STATUS_SUCCESS) {
+ btd_error(adapter->dev_id,
+ "Failed to set blocked keys: %s (0x%02x)",
+ mgmt_errstr(status), status);
+ return;
+ }
+
+ DBG("Successfully set blocked keys for index %u", adapter->dev_id);
+}
+
+static bool set_blocked_keys(struct btd_adapter *adapter)
+{
+ uint8_t buffer[sizeof(struct mgmt_cp_set_blocked_keys) +
+ sizeof(blocked_keys)] = { 0 };
+ struct mgmt_cp_set_blocked_keys *cp =
+ (struct mgmt_cp_set_blocked_keys *)buffer;
+ int i;
+
+ cp->key_count = G_N_ELEMENTS(blocked_keys);
+ for (i = 0; i < cp->key_count; ++i) {
+ cp->keys[i].type = blocked_keys[i].type;
+ memcpy(cp->keys[i].val, blocked_keys[i].val, sizeof(cp->keys[i].val));
+ }
+
+ return mgmt_send(mgmt_master, MGMT_OP_SET_BLOCKED_KEYS, adapter->dev_id,
+ sizeof(buffer), buffer, set_blocked_keys_complete,
+ adapter, NULL);
+}
+
static void read_info_complete(uint8_t status, uint16_t length,
const void *param, void *user_data)
{
@@ -8795,6 +8846,12 @@ static void read_info_complete(uint8_t status, uint16_t length,
set_name(adapter, btd_adapter_get_name(adapter));
+ if (kernel_blocked_keys_supported && !set_blocked_keys(adapter)) {
+ btd_error(adapter->dev_id,
+ "Failed to set blocked keys for index %u", adapter->dev_id);
+ goto failed;
+ }
+
if (main_opts.pairable &&
!(adapter->current_settings & MGMT_SETTING_BONDABLE))
set_mode(adapter, MGMT_OP_SET_BONDABLE, 0x01);
@@ -8972,9 +9029,17 @@ static void read_commands_complete(uint8_t status, uint16_t length,
for (i = 0; i < num_commands; i++) {
uint16_t op = get_le16(rp->opcodes + i);
- if (op == MGMT_OP_ADD_DEVICE) {
+ switch (op) {
+ case MGMT_OP_ADD_DEVICE:
DBG("enabling kernel-side connection control");
kernel_conn_control = true;
+ break;
+ case MGMT_OP_SET_BLOCKED_KEYS:
+ DBG("kernel supports the set_blocked_keys op");
+ kernel_blocked_keys_supported = true;
+ break;
+ default:
+ break;
}
}
}
--
2.24.0.525.g8f36a354ae-goog
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] Loading keys that should be blocked from bluetoothd.
2019-12-10 17:59 [PATCH v2] Loading keys that should be blocked from bluetoothd Alain Michaud
@ 2019-12-15 21:34 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 2+ messages in thread
From: Luiz Augusto von Dentz @ 2019-12-15 21:34 UTC (permalink / raw)
To: Alain Michaud; +Cc: BlueZ, Marcel Holtmann
Hi Alain,
On Tue, Dec 10, 2019 at 8:00 PM Alain Michaud <alainm@chromium.org> wrote:
>
> This changes will send a list of known bad keys that should be blocked
> if supported by the kernel.
>
> In particular keys from the Google Titan Security key are being blocked.
> For additional information, please see
> https://security.googleblog.com/2019/05/titan-keys-update.html
>
> Signed-off-by: Alain Michaud <alainm@chromium.org>
Signed off by is not used for userspace, also in the future please
prefix the userspace patches with BlueZ to make it easier to identify
them:
https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/HACKING#n118
https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/HACKING#n136
> ---
>
> lib/mgmt.h | 16 ++++++++++++
> src/adapter.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 82 insertions(+), 1 deletion(-)
>
> diff --git a/lib/mgmt.h b/lib/mgmt.h
> index 570dec997..a99b18c88 100644
> --- a/lib/mgmt.h
> +++ b/lib/mgmt.h
> @@ -583,6 +583,22 @@ struct mgmt_cp_set_phy_confguration {
> uint32_t selected_phys;
> } __packed;
>
> +#define MGMT_OP_SET_BLOCKED_KEYS 0x0046
> +
> +#define HCI_BLOCKED_KEY_TYPE_LINKKEY 0x00
> +#define HCI_BLOCKED_KEY_TYPE_LTK 0x01
> +#define HCI_BLOCKED_KEY_TYPE_IRK 0x02
> +
> +struct mgmt_blocked_key_info {
> + uint8_t type;
> + uint8_t val[16];
> +} __packed;
> +
> +struct mgmt_cp_set_blocked_keys {
> + uint16_t key_count;
> + struct mgmt_blocked_key_info keys[0];
> +} __packed;
> +#define MGMT_OP_SET_BLOCKED_KEYS_SIZE 0
lib/ changes should probably be in a separate patch.
> #define MGMT_EV_CMD_COMPLETE 0x0001
> struct mgmt_ev_cmd_complete {
> diff --git a/src/adapter.c b/src/adapter.c
> index cef25616f..2958870e6 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -99,10 +99,27 @@
> #define DISTANCE_VAL_INVALID 0x7FFF
> #define PATHLOSS_MAX 137
>
> +/*
> + * These are known security keys that have been compromised.
> + * If this grows or there are needs to be platform specific, it is
> + * conceivable that these could be read from a config file.
> + */
> +static const struct mgmt_blocked_key_info blocked_keys[] = {
> + /* Google Titan Security Keys */
> + { HCI_BLOCKED_KEY_TYPE_LTK,
> + {0xbf, 0x01, 0xfb, 0x9d, 0x4e, 0xf3, 0xbc, 0x36,
> + 0xd8, 0x74, 0xf5, 0x39, 0x41, 0x38, 0x68, 0x4c}},
> + { HCI_BLOCKED_KEY_TYPE_IRK,
> + {0xa5, 0x99, 0xba, 0xe4, 0xe1, 0x7c, 0xa6, 0x18,
> + 0x22, 0x8e, 0x07, 0x56, 0xb4, 0xe8, 0x5f, 0x01}},
> +};
> +
> static DBusConnection *dbus_conn = NULL;
>
> static bool kernel_conn_control = false;
>
> +static bool kernel_blocked_keys_supported = false;
> +
I guess in the long run we would probably have a single
kernel_features and then use bits instead of a bool for each of the
kernel features.
> static GList *adapter_list = NULL;
> static unsigned int adapter_remaining = 0;
> static bool powering_down = false;
> @@ -8568,6 +8585,40 @@ static bool set_static_addr(struct btd_adapter *adapter)
> return false;
> }
>
> +static void set_blocked_keys_complete(uint8_t status, uint16_t length,
> + const void *param, void *user_data)
> +{
> + struct btd_adapter *adapter = user_data;
> +
> + if (status != MGMT_STATUS_SUCCESS) {
> + btd_error(adapter->dev_id,
> + "Failed to set blocked keys: %s (0x%02x)",
> + mgmt_errstr(status), status);
> + return;
> + }
> +
> + DBG("Successfully set blocked keys for index %u", adapter->dev_id);
> +}
> +
> +static bool set_blocked_keys(struct btd_adapter *adapter)
> +{
> + uint8_t buffer[sizeof(struct mgmt_cp_set_blocked_keys) +
> + sizeof(blocked_keys)] = { 0 };
> + struct mgmt_cp_set_blocked_keys *cp =
> + (struct mgmt_cp_set_blocked_keys *)buffer;
> + int i;
> +
> + cp->key_count = G_N_ELEMENTS(blocked_keys);
At some point we would be converting to use libell, might take a while
though, so if we could perhaps use something like:
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
Perhaps it could be added to src/shared/util.h that way we don't have
to keep adding it all the time.
> + for (i = 0; i < cp->key_count; ++i) {
> + cp->keys[i].type = blocked_keys[i].type;
> + memcpy(cp->keys[i].val, blocked_keys[i].val, sizeof(cp->keys[i].val));
> + }
> +
> + return mgmt_send(mgmt_master, MGMT_OP_SET_BLOCKED_KEYS, adapter->dev_id,
> + sizeof(buffer), buffer, set_blocked_keys_complete,
> + adapter, NULL);
> +}
> +
> static void read_info_complete(uint8_t status, uint16_t length,
> const void *param, void *user_data)
> {
> @@ -8795,6 +8846,12 @@ static void read_info_complete(uint8_t status, uint16_t length,
>
> set_name(adapter, btd_adapter_get_name(adapter));
>
> + if (kernel_blocked_keys_supported && !set_blocked_keys(adapter)) {
> + btd_error(adapter->dev_id,
> + "Failed to set blocked keys for index %u", adapter->dev_id);
> + goto failed;
> + }
> +
> if (main_opts.pairable &&
> !(adapter->current_settings & MGMT_SETTING_BONDABLE))
> set_mode(adapter, MGMT_OP_SET_BONDABLE, 0x01);
> @@ -8972,9 +9029,17 @@ static void read_commands_complete(uint8_t status, uint16_t length,
> for (i = 0; i < num_commands; i++) {
> uint16_t op = get_le16(rp->opcodes + i);
>
> - if (op == MGMT_OP_ADD_DEVICE) {
> + switch (op) {
> + case MGMT_OP_ADD_DEVICE:
> DBG("enabling kernel-side connection control");
> kernel_conn_control = true;
> + break;
> + case MGMT_OP_SET_BLOCKED_KEYS:
> + DBG("kernel supports the set_blocked_keys op");
> + kernel_blocked_keys_supported = true;
> + break;
> + default:
> + break;
> }
> }
> }
> --
> 2.24.0.525.g8f36a354ae-goog
Other than that it looks pretty good, Im just missing the changes to
the documentation that would be great to have in the same set.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-12-15 21:34 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10 17:59 [PATCH v2] Loading keys that should be blocked from bluetoothd Alain Michaud
2019-12-15 21:34 ` Luiz Augusto von Dentz
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.