Re: [v2 PATCH 5/5] Bluetooth: let the crypto subsystem generate the ecc privkey
Hi Tudor, >>> 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>>> --- >>> net/bluetooth/ecdh_helper.c | 186 >>> >>> net/bluetooth/ecdh_helper.h | 9 ++- >>> net/bluetooth/selftest.c| 14 +++- >>> net/bluetooth/smp.c | 66 +++- >>> 4 files changed, 147 insertions(+), 128 deletions(-) >>> >>> diff --git a/net/bluetooth/ecdh_helper.c b/net/bluetooth/ecdh_helper.c >>> index 16e022f..2155ce8 100644 >>> --- a/net/bluetooth/ecdh_helper.c >>> +++ b/net/bluetooth/ecdh_helper.c >>> @@ -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(); >>> >>> - /* 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(); >>> - buf = kmalloc(buf_len, GFP_KERNEL); >>> - if (!buf) { >>> - err = -ENOMEM; >>> - goto free_req; >>> - } >>> - >>> - crypto_ecdh_encode_key(buf, buf_len, ); >>> - >>> - /* 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 *)_key[32], (u64 *)[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(); >>> + buf = kmalloc(buf_len, GFP_KERNEL); >>> + if (!buf) { >>> + err = -ENOMEM; >>> + goto free_tmp; >>> + } >>> + >>> + err = crypto_ecdh_encode_key(buf, buf_len, ); >>> + if (err) >>> + goto free_all; >>> + >>> + err = crypto_kpp_set_secret(tfm, buf, buf_len); >>> + /* fall through */
Re: [v2 PATCH 5/5] Bluetooth: let the crypto subsystem generate the ecc privkey
Hi, Marcel, On 09/28/2017 07:50 PM, Marcel Holtmann wrote: Hi Tudor, 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--- net/bluetooth/ecdh_helper.c | 186 net/bluetooth/ecdh_helper.h | 9 ++- net/bluetooth/selftest.c| 14 +++- net/bluetooth/smp.c | 66 +++- 4 files changed, 147 insertions(+), 128 deletions(-) diff --git a/net/bluetooth/ecdh_helper.c b/net/bluetooth/ecdh_helper.c index 16e022f..2155ce8 100644 --- a/net/bluetooth/ecdh_helper.c +++ b/net/bluetooth/ecdh_helper.c @@ -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(); - /* 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(); - buf = kmalloc(buf_len, GFP_KERNEL); - if (!buf) { - err = -ENOMEM; - goto free_req; - } - - crypto_ecdh_encode_key(buf, buf_len, ); - - /* 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 *)_key[32], (u64 *)[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(); + buf = kmalloc(buf_len, GFP_KERNEL); + if (!buf) { + err = -ENOMEM; + goto free_tmp; + } + + err = crypto_ecdh_encode_key(buf, buf_len, ); + 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
Re: [v2 PATCH 5/5] Bluetooth: let the crypto subsystem generate the ecc privkey
Hi Tudor, > 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> --- > net/bluetooth/ecdh_helper.c | 186 > net/bluetooth/ecdh_helper.h | 9 ++- > net/bluetooth/selftest.c| 14 +++- > net/bluetooth/smp.c | 66 +++- > 4 files changed, 147 insertions(+), 128 deletions(-) > > diff --git a/net/bluetooth/ecdh_helper.c b/net/bluetooth/ecdh_helper.c > index 16e022f..2155ce8 100644 > --- a/net/bluetooth/ecdh_helper.c > +++ b/net/bluetooth/ecdh_helper.c > @@ -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(); > > - /* 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(); > - buf = kmalloc(buf_len, GFP_KERNEL); > - if (!buf) { > - err = -ENOMEM; > - goto free_req; > - } > - > - crypto_ecdh_encode_key(buf, buf_len, ); > - > - /* 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 *)_key[32], (u64 *)[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(); > + buf = kmalloc(buf_len, GFP_KERNEL); > + if (!buf) { > + err = -ENOMEM; > + goto free_tmp; > + } > + > + err = crypto_ecdh_encode_key(buf, buf_len, ); > + 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
[v2 PATCH 5/5] Bluetooth: let the crypto subsystem generate the ecc privkey
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--- net/bluetooth/ecdh_helper.c | 186 net/bluetooth/ecdh_helper.h | 9 ++- net/bluetooth/selftest.c| 14 +++- net/bluetooth/smp.c | 66 +++- 4 files changed, 147 insertions(+), 128 deletions(-) diff --git a/net/bluetooth/ecdh_helper.c b/net/bluetooth/ecdh_helper.c index 16e022f..2155ce8 100644 --- a/net/bluetooth/ecdh_helper.c +++ b/net/bluetooth/ecdh_helper.c @@ -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(); - /* 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(); - buf = kmalloc(buf_len, GFP_KERNEL); - if (!buf) { - err = -ENOMEM; - goto free_req; - } - - crypto_ecdh_encode_key(buf, buf_len, ); - - /* 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 *)_key[32], (u64 *)[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(); + buf = kmalloc(buf_len, GFP_KERNEL); + if (!buf) { + err = -ENOMEM; + goto free_tmp; + } + + err = crypto_ecdh_encode_key(buf, buf_len, ); + 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: