Hi Johan, > -----Original Message----- > From: linux-crypto-owner@xxxxxxxxxxxxxxx [mailto:linux-crypto- > owner@xxxxxxxxxxxxxxx] On Behalf Of Johan Hedberg > Sent: Thursday, May 12, 2016 7:05 PM > To: Benedetto, Salvatore <salvatore.benedetto@xxxxxxxxx> > Cc: herbert@xxxxxxxxxxxxxxxxxxx; gustavo@xxxxxxxxxxx; linux- > bluetooth@xxxxxxxxxxxxxxx; linux-crypto@xxxxxxxxxxxxxxx; > marcel@xxxxxxxxxxxx > Subject: Re: [PATCH v2] Bluetooth: convert smp and selftest to crypto kpp > API > > Hi Salvatore, > > On Mon, May 09, 2016, Salvatore Benedetto wrote: > > * Convert both smp and selftest to crypto kpp API > > * Remove module ecc as not more required > > * Add ecdh_helper functions for wrapping kpp async calls > > > > This patch has been tested *only* with selftest, which is called on > > module loading. smp-tester passes all tests but the first one, which > > often times out. Same behavior is observed without this patch though. > > > > This patch is based on https://patchwork.kernel.org/patch/9050061/ > > > > Signed-off-by: Salvatore Benedetto <salvatore.benedetto@xxxxxxxxx> > > --- > > net/bluetooth/Makefile | 2 +- > > net/bluetooth/ecc.c | 816 -------------------------------------------- > > net/bluetooth/ecc.h | 54 --- > > net/bluetooth/ecdh_helper.c | 204 +++++++++++ > > net/bluetooth/ecdh_helper.h | 32 ++ > > net/bluetooth/selftest.c | 6 +- > > net/bluetooth/smp.c | 8 +- > > 7 files changed, 244 insertions(+), 878 deletions(-) delete mode > > 100644 net/bluetooth/ecc.c delete mode 100644 net/bluetooth/ecc.h > > create mode 100644 net/bluetooth/ecdh_helper.c create mode 100644 > > net/bluetooth/ecdh_helper.h > > I tried this together with your kpp patches and I was able to successfully pair, > so from that perspective we're fine and I have no objections for those > patches being merged. > > There are a couple of things to improve with this patch however: > > The first issue is that you're missing a 'select CRYPTO_ECDH' in > net/bluetooth/Kconfig. Without this you'll end up creating a kernel that > either doesn't build or a module that doesn't load. > Right the module won't load. I'll fix this. > > +#ifndef __ECDH_HELPER_H > > +#define __ECDH_HELPER_H > > Please leave out these include guards for internal headers. We try to avoid > them there to force keeping the dependency chains simple. I realize you > might have copied this from smp.h (which is the only internal header I could > see having this) so that might need fixing in a separate patch. > Actually I didn't pay much attention to this as I always use them. I'll remove it. > > +bool compute_ecdh_shared_secret(const u8 pub_a[64], const u8 > priv_b[32], > > + u8 secret[32]); > > +bool generate_ecdh_key_pair(u8 public_key[64], u8 private_key[32]); > > Could you try to come up with some shorter names than these since they > cause the line lengths to go past 80 chars. In fact, can't you just keep the > same names as our ecc.c was using. I don't think those names are taken by > your new kpp code? They actually are used internally by the ecc module I reworked. I'll rename them to compute_ecdh_secret() and generate_ecdh_keys() which fixes the 80 chars per line issue. I'll send a v3. Thanks for reviewing. Regards, Salvatore -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
