Hello everyone,
I'm trying to pair my MIPS based router with a phone (using bluez),
without success.
I have turned on debugging features of bluetooth kernel module and I
got this in dmesg:
[ 234.481526] Bluetooth: Core ver 2.22
[ 234.485231] Bluetooth: Starting self testing
[ 234.525375] Bluetooth: ECDH sample 1 failed
[ 234.529728] Bluetooth: Finished self testing
Also, I checked hcidump, and it seems that SMP "Pairing confirm"
command got rejected by the phone with an error ("Reason: Unspecified
Reason (0x08)").
As far as I understand, ECDH computation is involved in a pairing process.
However, my Linux desktop is able to successfully pair to the phone
(Ubuntu 18.04) via the same dongle, using same version of bluez.
The huge difference between my desktop and router is endianness:
router is big-endian, while desktop is little-endian. Maybe, that's
the case?
I'm not sure how to debug this, could you please advise?
Router specs:
CPU: QCA9531 (MIPS_24KC, big-endian)
Distro: OpenWRT git master
Linux: 4.14.88
BlueZ: 5.49
Bluetooth dongle: CSR8510 A10
Thanks,
Andrey
Hi Andrey,
> I'm trying to pair my MIPS based router with a phone (using bluez),
> without success.
> I have turned on debugging features of bluetooth kernel module and I
> got this in dmesg:
>
> [ 234.481526] Bluetooth: Core ver 2.22
> [ 234.485231] Bluetooth: Starting self testing
> [ 234.525375] Bluetooth: ECDH sample 1 failed
> [ 234.529728] Bluetooth: Finished self testing
>
> Also, I checked hcidump, and it seems that SMP "Pairing confirm"
> command got rejected by the phone with an error ("Reason: Unspecified
> Reason (0x08)").
> As far as I understand, ECDH computation is involved in a pairing process.
I think that our ECDH code was endian safe, but then it got changed at some point to use standard crypto and maybe something went wrong there. Can just provide the btmon -w trace.log for the SMP pairing so that I can have a look at the binary trace.
Regards
Marcel
Hello Marcel,
On Sat, Dec 29, 2018 at 9:35 AM Marcel Holtmann <[email protected]> wrote:
> I think that our ECDH code was endian safe, but then it got changed at some point to use standard crypto and maybe something went wrong there. Can just provide the btmon -w trace.log for the SMP pairing so that I can have a look at the binary trace.
Binary trace from the MIPS system is attached. Hope it helps.
Thanks,
Andrey
Hello Marcel,
> On Sat, Dec 29, 2018 at 9:35 AM Marcel Holtmann <[email protected]> wrote:
> I think that our ECDH code was endian safe, but then it got changed at some point to use standard crypto and maybe something went wrong there. Can just provide the btmon -w trace.log for the SMP pairing so that I can have a look at the binary trace.
I found out that if I change "swap_digits" method in
"net/bluetooth/ecdh_helper.c" to
static inline void swap_digits(u64 *in, u64 *out, unsigned int ndigits)
{
int i;
for (i = 0; i < ndigits; i++)
out[i] = in[ndigits - 1 - i];
}
then BLE pairing on big-endian become operational. I'm not sure what
proper fix should be: is it a problem with crypto API usage or a
problem with crypto itself?
Thanks,
Andrey
Hi Andrey,
>> On Sat, Dec 29, 2018 at 9:35 AM Marcel Holtmann <[email protected]> wrote:
>> I think that our ECDH code was endian safe, but then it got changed at some point to use standard crypto and maybe something went wrong there. Can just provide the btmon -w trace.log for the SMP pairing so that I can have a look at the binary trace.
>
> I found out that if I change "swap_digits" method in
> "net/bluetooth/ecdh_helper.c" to
>
> static inline void swap_digits(u64 *in, u64 *out, unsigned int ndigits)
> {
> int i;
>
> for (i = 0; i < ndigits; i++)
> out[i] = in[ndigits - 1 - i];
> }
>
> then BLE pairing on big-endian become operational. I'm not sure what
> proper fix should be: is it a problem with crypto API usage or a
> problem with crypto itself?
if the kernel ECC and ECDH crypto already swaps for us, then we don’t need to do it again. So all the swap_digits most likely can be removed from net/bluetooth/.
Regards
Marcel
Hi,
Den fre 18 jan. 2019 kl 04:44 skrev Marcel Holtmann <[email protected]>:
>
> Hi Andrey,
>
> >> On Sat, Dec 29, 2018 at 9:35 AM Marcel Holtmann <[email protected]> wrote:
> >> I think that our ECDH code was endian safe, but then it got changed at some point to use standard crypto and maybe something went wrong there. Can just provide the btmon -w trace.log for the SMP pairing so that I can have a look at the binary trace.
> >
> > I found out that if I change "swap_digits" method in
> > "net/bluetooth/ecdh_helper.c" to
> >
> > static inline void swap_digits(u64 *in, u64 *out, unsigned int ndigits)
> > {
> > int i;
> >
> > for (i = 0; i < ndigits; i++)
> > out[i] = in[ndigits - 1 - i];
> > }
> >
> > then BLE pairing on big-endian become operational. I'm not sure what
> > proper fix should be: is it a problem with crypto API usage or a
> > problem with crypto itself?
>
> if the kernel ECC and ECDH crypto already swaps for us, then we don’t need to do it again. So all the swap_digits most likely can be removed from net/bluetooth/.
>
> Regards
>
> Marcel
>
The Bluetooth standard is a bit strange. It assumes the AES standard
is big endian (although it is really just defined on a byte level),
but since Bluetooth is little-endian everywhere, all AES 128-bit
values must be reversed when a standard AES library is used. In
particular, SMP reverses the AES values. So the swap_digits should be
kept.
/Emil
Hi Emil,
>>>> On Sat, Dec 29, 2018 at 9:35 AM Marcel Holtmann <[email protected]> wrote:
>>>> I think that our ECDH code was endian safe, but then it got changed at some point to use standard crypto and maybe something went wrong there. Can just provide the btmon -w trace.log for the SMP pairing so that I can have a look at the binary trace.
>>>
>>> I found out that if I change "swap_digits" method in
>>> "net/bluetooth/ecdh_helper.c" to
>>>
>>> static inline void swap_digits(u64 *in, u64 *out, unsigned int ndigits)
>>> {
>>> int i;
>>>
>>> for (i = 0; i < ndigits; i++)
>>> out[i] = in[ndigits - 1 - i];
>>> }
>>>
>>> then BLE pairing on big-endian become operational. I'm not sure what
>>> proper fix should be: is it a problem with crypto API usage or a
>>> problem with crypto itself?
>>
>> if the kernel ECC and ECDH crypto already swaps for us, then we don’t need to do it again. So all the swap_digits most likely can be removed from net/bluetooth/.
>>
>> Regards
>>
>> Marcel
>>
>
> The Bluetooth standard is a bit strange. It assumes the AES standard
> is big endian (although it is really just defined on a byte level),
> but since Bluetooth is little-endian everywhere, all AES 128-bit
> values must be reversed when a standard AES library is used. In
> particular, SMP reverses the AES values. So the swap_digits should be
> kept.
so you are saying just reversing is needed, but not swapping? But then this is no longer swap_digits, it is just a reverse. What do you want us to do in this case now?
Regards
Marcel
Hi Andrey and Marcel,
Den mån 21 jan. 2019 kl 15:53 skrev Marcel Holtmann <[email protected]>:
>
> Hi Emil,
>
> >>>> On Sat, Dec 29, 2018 at 9:35 AM Marcel Holtmann <[email protected]> wrote:
> >>>> I think that our ECDH code was endian safe, but then it got changed at some point to use standard crypto and maybe something went wrong there. Can just provide the btmon -w trace.log for the SMP pairing so that I can have a look at the binary trace.
> >>>
> >>> I found out that if I change "swap_digits" method in
> >>> "net/bluetooth/ecdh_helper.c" to
> >>>
> >>> static inline void swap_digits(u64 *in, u64 *out, unsigned int ndigits)
> >>> {
> >>> int i;
> >>>
> >>> for (i = 0; i < ndigits; i++)
> >>> out[i] = in[ndigits - 1 - i];
> >>> }
> >>>
> >>> then BLE pairing on big-endian become operational. I'm not sure what
> >>> proper fix should be: is it a problem with crypto API usage or a
> >>> problem with crypto itself?
> >>
> >> if the kernel ECC and ECDH crypto already swaps for us, then we don’t need to do it again. So all the swap_digits most likely can be removed from net/bluetooth/.
> >>
> >> Regards
> >>
> >> Marcel
> >>
> >
> > The Bluetooth standard is a bit strange. It assumes the AES standard
> > is big endian (although it is really just defined on a byte level),
> > but since Bluetooth is little-endian everywhere, all AES 128-bit
> > values must be reversed when a standard AES library is used. In
> > particular, SMP reverses the AES values. So the swap_digits should be
> > kept.
>
> so you are saying just reversing is needed, but not swapping? But then this is no longer swap_digits, it is just a reverse. What do you want us to do in this case now?
First, I was a bit too quick. This is not about AES but about
NIST-ECDH. Nevertheless, the kernel API seems to assume big endian
order of the values regardless of platform, per the NIST
specification.
By looking some more into the kernel ECDH code, the Bluetooth code is
not the buggy one. Instead the ECDH kernel code turns out to be wrong.
The function ecc_swap_digits in crypto/ecc.c looks like the following:
static inline void ecc_swap_digits(const u64 *in, u64 *out,
unsigned int ndigits)
{
int i;
for (i = 0; i < ndigits; i++)
out[i] = __swab64(in[ndigits - 1 - i]);
}
It is basically used to load a pointer-casted byte buffer into an
internal little endian representation of u64 values and vice versa. So
it works on little endian platforms but will fail on big endian
platforms. Applying your patch here is the correct thing to do, and
not in the Bluetooth code. But instead of using no swap for big endian
and __swab64 for little endian platforms, simply use the be64_to_cpu
helper.
/Emil