Bluetooth: let the crypto subsystem generate the ecc privkey
authorTudor Ambarus <tudor.ambarus@microchip.com>
Thu, 28 Sep 2017 14:14:55 +0000 (17:14 +0300)
committerMarcel Holtmann <marcel@holtmann.org>
Fri, 6 Oct 2017 18:35:47 +0000 (20:35 +0200)
That Bluetooth SMP knows about the private key is pointless, since the
detection of debug key usage is actually via the public key portion.
With this patch, the Bluetooth SMP will stop keeping a copy of the
ecdh private key and will let the crypto subsystem to generate and
handle the ecdh private key, potentially benefiting of hardware
ecc private key generation and retention.

The loop that tries to generate a correct private key is now removed and
we trust the crypto subsystem to generate a correct private key. This
backup logic should be done in crypto, if really needed.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
net/bluetooth/ecdh_helper.c
net/bluetooth/ecdh_helper.h
net/bluetooth/selftest.c
net/bluetooth/smp.c

index 16e022f5ab276c8475d314b26366a5e57ce8c078..2155ce8028774fefcd2703a7eb16e1c3e1a6e11c 100644 (file)
@@ -49,15 +49,21 @@ static inline void swap_digits(u64 *in, u64 *out, unsigned int ndigits)
                out[i] = __swab64(in[ndigits - 1 - i]);
 }
 
+/* compute_ecdh_secret() - function assumes that the private key was
+ *                         already set.
+ * @tfm:          KPP tfm handle allocated with crypto_alloc_kpp().
+ * @public_key:   pair's ecc public key.
+ * secret:        memory where the ecdh computed shared secret will be saved.
+ *
+ * Return: zero on success; error code in case of error.
+ */
 int compute_ecdh_secret(struct crypto_kpp *tfm, const u8 public_key[64],
-                       const u8 private_key[32], u8 secret[32])
+                       u8 secret[32])
 {
        struct kpp_request *req;
-       struct ecdh p;
+       u8 *tmp;
        struct ecdh_completion result;
        struct scatterlist src, dst;
-       u8 *tmp, *buf;
-       unsigned int buf_len;
        int err;
 
        tmp = kmalloc(64, GFP_KERNEL);
@@ -72,28 +78,6 @@ int compute_ecdh_secret(struct crypto_kpp *tfm, const u8 public_key[64],
 
        init_completion(&result.completion);
 
-       /* Security Manager Protocol holds digits in litte-endian order
-        * while ECC API expect big-endian data
-        */
-       swap_digits((u64 *)private_key, (u64 *)tmp, 4);
-       p.key = (char *)tmp;
-       p.key_size = 32;
-       /* Set curve_id */
-       p.curve_id = ECC_CURVE_NIST_P256;
-       buf_len = crypto_ecdh_key_len(&p);
-       buf = kmalloc(buf_len, GFP_KERNEL);
-       if (!buf) {
-               err = -ENOMEM;
-               goto free_req;
-       }
-
-       crypto_ecdh_encode_key(buf, buf_len, &p);
-
-       /* Set A private Key */
-       err = crypto_kpp_set_secret(tfm, (void *)buf, buf_len);
-       if (err)
-               goto free_all;
-
        swap_digits((u64 *)public_key, (u64 *)tmp, 4); /* x */
        swap_digits((u64 *)&public_key[32], (u64 *)&tmp[32], 4); /* y */
 
@@ -118,26 +102,76 @@ int compute_ecdh_secret(struct crypto_kpp *tfm, const u8 public_key[64],
        memcpy(secret, tmp, 32);
 
 free_all:
-       kzfree(buf);
-free_req:
        kpp_request_free(req);
 free_tmp:
        kzfree(tmp);
        return err;
 }
 
-int generate_ecdh_keys(struct crypto_kpp *tfm, u8 public_key[64],
-                      u8 private_key[32])
+/* set_ecdh_privkey() - set or generate ecc private key.
+ *
+ * Function generates an ecc private key in the crypto subsystem when receiving
+ * a NULL private key or sets the received key when not NULL.
+ *
+ * @tfm:           KPP tfm handle allocated with crypto_alloc_kpp().
+ * @private_key:   user's ecc private key. When not NULL, the key is expected
+ *                 in little endian format.
+ *
+ * Return: zero on success; error code in case of error.
+ */
+int set_ecdh_privkey(struct crypto_kpp *tfm, const u8 private_key[32])
+{
+       u8 *buf, *tmp = NULL;
+       unsigned int buf_len;
+       int err;
+       struct ecdh p = {0};
+
+       p.curve_id = ECC_CURVE_NIST_P256;
+
+       if (private_key) {
+               tmp = kmalloc(32, GFP_KERNEL);
+               if (!tmp)
+                       return -ENOMEM;
+               swap_digits((u64 *)private_key, (u64 *)tmp, 4);
+               p.key = tmp;
+               p.key_size = 32;
+       }
+
+       buf_len = crypto_ecdh_key_len(&p);
+       buf = kmalloc(buf_len, GFP_KERNEL);
+       if (!buf) {
+               err = -ENOMEM;
+               goto free_tmp;
+       }
+
+       err = crypto_ecdh_encode_key(buf, buf_len, &p);
+       if (err)
+               goto free_all;
+
+       err = crypto_kpp_set_secret(tfm, buf, buf_len);
+       /* fall through */
+free_all:
+       kzfree(buf);
+free_tmp:
+       kzfree(tmp);
+       return err;
+}
+
+/* generate_ecdh_public_key() - function assumes that the private key was
+ *                              already set.
+ *
+ * @tfm:          KPP tfm handle allocated with crypto_alloc_kpp().
+ * @public_key:   memory where the computed ecc public key will be saved.
+ *
+ * Return: zero on success; error code in case of error.
+ */
+int generate_ecdh_public_key(struct crypto_kpp *tfm, u8 public_key[64])
 {
        struct kpp_request *req;
-       struct ecdh p;
+       u8 *tmp;
        struct ecdh_completion result;
        struct scatterlist dst;
-       u8 *tmp, *buf;
-       unsigned int buf_len;
        int err;
-       const unsigned short max_tries = 16;
-       unsigned short tries = 0;
 
        tmp = kmalloc(64, GFP_KERNEL);
        if (!tmp)
@@ -150,63 +184,47 @@ int generate_ecdh_keys(struct crypto_kpp *tfm, u8 public_key[64],
        }
 
        init_completion(&result.completion);
+       sg_init_one(&dst, tmp, 64);
+       kpp_request_set_input(req, NULL, 0);
+       kpp_request_set_output(req, &dst, 64);
+       kpp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+                                ecdh_complete, &result);
 
-       /* Set curve_id */
-       p.curve_id = ECC_CURVE_NIST_P256;
-       p.key_size = 32;
-       buf_len = crypto_ecdh_key_len(&p);
-       buf = kmalloc(buf_len, GFP_KERNEL);
-       if (!buf)
-               goto free_req;
-
-       do {
-               if (tries++ >= max_tries)
-                       goto free_all;
-
-               /* Set private Key */
-               p.key = (char *)private_key;
-               crypto_ecdh_encode_key(buf, buf_len, &p);
-               err = crypto_kpp_set_secret(tfm, buf, buf_len);
-               if (err)
-                       goto free_all;
-
-               sg_init_one(&dst, tmp, 64);
-               kpp_request_set_input(req, NULL, 0);
-               kpp_request_set_output(req, &dst, 64);
-               kpp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-                                        ecdh_complete, &result);
-
-               err = crypto_kpp_generate_public_key(req);
-
-               if (err == -EINPROGRESS) {
-                       wait_for_completion(&result.completion);
-                       err = result.err;
-               }
-
-               /* Private key is not valid. Regenerate */
-               if (err == -EINVAL)
-                       continue;
-
-               if (err < 0)
-                       goto free_all;
-               else
-                       break;
-
-       } while (true);
-
-       /* Keys are handed back in little endian as expected by Security
-        * Manager Protocol
+       err = crypto_kpp_generate_public_key(req);
+       if (err == -EINPROGRESS) {
+               wait_for_completion(&result.completion);
+               err = result.err;
+       }
+       if (err < 0)
+               goto free_all;
+
+       /* The public key is handed back in little endian as expected by
+        * the Security Manager Protocol.
         */
        swap_digits((u64 *)tmp, (u64 *)public_key, 4); /* x */
        swap_digits((u64 *)&tmp[32], (u64 *)&public_key[32], 4); /* y */
-       swap_digits((u64 *)private_key, (u64 *)tmp, 4);
-       memcpy(private_key, tmp, 32);
 
 free_all:
-       kzfree(buf);
-free_req:
        kpp_request_free(req);
 free_tmp:
        kfree(tmp);
        return err;
 }
+
+/* generate_ecdh_keys() - generate ecc key pair.
+ *
+ * @tfm:          KPP tfm handle allocated with crypto_alloc_kpp().
+ * @public_key:   memory where the computed ecc public key will be saved.
+ *
+ * Return: zero on success; error code in case of error.
+ */
+int generate_ecdh_keys(struct crypto_kpp *tfm, u8 public_key[64])
+{
+       int err;
+
+       err = set_ecdh_privkey(tfm, NULL);
+       if (err)
+               return err;
+
+       return generate_ecdh_public_key(tfm, public_key);
+}
index 50e6676aebf5b49f9a6f855a81758cc3eb034916..a6f8d03d4aaf65514da3462eda3594c4795362f5 100644 (file)
@@ -23,7 +23,8 @@
 #include <crypto/kpp.h>
 #include <linux/types.h>
 
-int compute_ecdh_secret(struct crypto_kpp *tfm, const u8 pub_a[64],
-                       const u8 priv_b[32], u8 secret[32]);
-int generate_ecdh_keys(struct crypto_kpp *tfm, u8 public_key[64],
-                      u8 private_key[32]);
+int compute_ecdh_secret(struct crypto_kpp *tfm, const u8 pair_public_key[64],
+                       u8 secret[32]);
+int set_ecdh_privkey(struct crypto_kpp *tfm, const u8 *private_key);
+int generate_ecdh_public_key(struct crypto_kpp *tfm, u8 public_key[64]);
+int generate_ecdh_keys(struct crypto_kpp *tfm, u8 public_key[64]);
index ce99648ed870837d940fe47f3fc144a7c583dae3..2d1519d0affa154a9ac3fb7f42b3e8afa43604cf 100644 (file)
@@ -152,11 +152,11 @@ static int __init test_ecdh_sample(struct crypto_kpp *tfm, const u8 priv_a[32],
        dhkey_a = &tmp[0];
        dhkey_b = &tmp[32];
 
-       ret = compute_ecdh_secret(tfm, pub_b, priv_a, dhkey_a);
+       ret = set_ecdh_privkey(tfm, priv_a);
        if (ret)
                goto out;
 
-       ret = compute_ecdh_secret(tfm, pub_a, priv_b, dhkey_b);
+       ret = compute_ecdh_secret(tfm, pub_b, dhkey_a);
        if (ret)
                goto out;
 
@@ -165,9 +165,17 @@ static int __init test_ecdh_sample(struct crypto_kpp *tfm, const u8 priv_a[32],
                goto out;
        }
 
+       ret = set_ecdh_privkey(tfm, priv_b);
+       if (ret)
+               goto out;
+
+       ret = compute_ecdh_secret(tfm, pub_a, dhkey_b);
+       if (ret)
+               goto out;
+
        if (memcmp(dhkey_b, dhkey, 32))
                ret = -EINVAL;
-
+       /* fall through*/
 out:
        kfree(tmp);
        return ret;
index af7e6100e55ba3f00657dc60223af56ce2610580..d41449b9e9d6e296ca464c9f8a7aff6643be9b16 100644 (file)
@@ -84,7 +84,6 @@ enum {
 struct smp_dev {
        /* Secure Connections OOB data */
        u8                      local_pk[64];
-       u8                      local_sk[32];
        u8                      local_rand[16];
        bool                    debug_key;
 
@@ -126,7 +125,6 @@ struct smp_chan {
 
        /* Secure Connections variables */
        u8                      local_pk[64];
-       u8                      local_sk[32];
        u8                      remote_pk[64];
        u8                      dhkey[32];
        u8                      mackey[16];
@@ -568,24 +566,22 @@ int smp_generate_oob(struct hci_dev *hdev, u8 hash[16], u8 rand[16])
 
        if (hci_dev_test_flag(hdev, HCI_USE_DEBUG_KEYS)) {
                BT_DBG("Using debug keys");
+               err = set_ecdh_privkey(smp->tfm_ecdh, debug_sk);
+               if (err)
+                       return err;
                memcpy(smp->local_pk, debug_pk, 64);
-               memcpy(smp->local_sk, debug_sk, 32);
                smp->debug_key = true;
        } else {
                while (true) {
-                       /* Seed private key with random number */
-                       get_random_bytes(smp->local_sk, 32);
-
-                       /* Generate local key pair for Secure Connections */
-                       err = generate_ecdh_keys(smp->tfm_ecdh, smp->local_pk,
-                                                smp->local_sk);
+                       /* Generate key pair for Secure Connections */
+                       err = generate_ecdh_keys(smp->tfm_ecdh, smp->local_pk);
                        if (err)
                                return err;
 
                        /* This is unlikely, but we need to check that
                         * we didn't accidentially generate a debug key.
                         */
-                       if (crypto_memneq(smp->local_sk, debug_sk, 32))
+                       if (crypto_memneq(smp->local_pk, debug_pk, 64))
                                break;
                }
                smp->debug_key = false;
@@ -593,7 +589,6 @@ int smp_generate_oob(struct hci_dev *hdev, u8 hash[16], u8 rand[16])
 
        SMP_DBG("OOB Public Key X: %32phN", smp->local_pk);
        SMP_DBG("OOB Public Key Y: %32phN", smp->local_pk + 32);
-       SMP_DBG("OOB Private Key:  %32phN", smp->local_sk);
 
        get_random_bytes(smp->local_rand, 16);
 
@@ -1900,7 +1895,6 @@ static u8 sc_send_public_key(struct smp_chan *smp)
                smp_dev = chan->data;
 
                memcpy(smp->local_pk, smp_dev->local_pk, 64);
-               memcpy(smp->local_sk, smp_dev->local_sk, 32);
                memcpy(smp->lr, smp_dev->local_rand, 16);
 
                if (smp_dev->debug_key)
@@ -1911,23 +1905,20 @@ static u8 sc_send_public_key(struct smp_chan *smp)
 
        if (hci_dev_test_flag(hdev, HCI_USE_DEBUG_KEYS)) {
                BT_DBG("Using debug keys");
+               if (set_ecdh_privkey(smp->tfm_ecdh, debug_sk))
+                       return SMP_UNSPECIFIED;
                memcpy(smp->local_pk, debug_pk, 64);
-               memcpy(smp->local_sk, debug_sk, 32);
                set_bit(SMP_FLAG_DEBUG_KEY, &smp->flags);
        } else {
                while (true) {
-                       /* Seed private key with random number */
-                       get_random_bytes(smp->local_sk, 32);
-
-                       /* Generate local key pair for Secure Connections */
-                       if (generate_ecdh_keys(smp->tfm_ecdh, smp->local_pk,
-                                              smp->local_sk))
+                       /* Generate key pair for Secure Connections */
+                       if (generate_ecdh_keys(smp->tfm_ecdh, smp->local_pk))
                                return SMP_UNSPECIFIED;
 
                        /* This is unlikely, but we need to check that
                         * we didn't accidentially generate a debug key.
                         */
-                       if (crypto_memneq(smp->local_sk, debug_sk, 32))
+                       if (crypto_memneq(smp->local_pk, debug_pk, 64))
                                break;
                }
        }
@@ -1935,7 +1926,6 @@ static u8 sc_send_public_key(struct smp_chan *smp)
 done:
        SMP_DBG("Local Public Key X: %32phN", smp->local_pk);
        SMP_DBG("Local Public Key Y: %32phN", smp->local_pk + 32);
-       SMP_DBG("Local Private Key:  %32phN", smp->local_sk);
 
        smp_send_cmd(smp->conn, SMP_CMD_PUBLIC_KEY, 64, smp->local_pk);
 
@@ -2663,6 +2653,7 @@ static int smp_cmd_public_key(struct l2cap_conn *conn, struct sk_buff *skb)
        struct l2cap_chan *chan = conn->smp;
        struct smp_chan *smp = chan->data;
        struct hci_dev *hdev = hcon->hdev;
+       struct crypto_kpp *tfm_ecdh;
        struct smp_cmd_pairing_confirm cfm;
        int err;
 
@@ -2695,8 +2686,18 @@ static int smp_cmd_public_key(struct l2cap_conn *conn, struct sk_buff *skb)
        SMP_DBG("Remote Public Key X: %32phN", smp->remote_pk);
        SMP_DBG("Remote Public Key Y: %32phN", smp->remote_pk + 32);
 
-       if (!compute_ecdh_secret(smp->tfm_ecdh, smp->remote_pk, smp->local_sk,
-                                smp->dhkey))
+       /* Compute the shared secret on the same crypto tfm on which the private
+        * key was set/generated.
+        */
+       if (test_bit(SMP_FLAG_LOCAL_OOB, &smp->flags)) {
+               struct smp_dev *smp_dev = chan->data;
+
+               tfm_ecdh = smp_dev->tfm_ecdh;
+       } else {
+               tfm_ecdh = smp->tfm_ecdh;
+       }
+
+       if (compute_ecdh_secret(tfm_ecdh, smp->remote_pk, smp->dhkey))
                return SMP_UNSPECIFIED;
 
        SMP_DBG("DHKey %32phN", smp->dhkey);
@@ -3522,27 +3523,18 @@ void smp_unregister(struct hci_dev *hdev)
 
 #if IS_ENABLED(CONFIG_BT_SELFTEST_SMP)
 
-static inline void swap_digits(u64 *in, u64 *out, unsigned int ndigits)
-{
-       int i;
-
-       for (i = 0; i < ndigits; i++)
-               out[i] = __swab64(in[ndigits - 1 - i]);
-}
-
 static int __init test_debug_key(struct crypto_kpp *tfm_ecdh)
 {
-       u8 pk[64], sk[32];
+       u8 pk[64];
        int err;
 
-       swap_digits((u64 *)debug_sk, (u64 *)sk, 4);
-
-       err = generate_ecdh_keys(tfm_ecdh, pk, sk);
+       err = set_ecdh_privkey(tfm_ecdh, debug_sk);
        if (err)
                return err;
 
-       if (crypto_memneq(sk, debug_sk, 32))
-               return -EINVAL;
+       err = generate_ecdh_public_key(tfm_ecdh, pk);
+       if (err)
+               return err;
 
        if (crypto_memneq(pk, debug_pk, 64))
                return -EINVAL;