ARMmbed / mbed-client-c

DEPRECATED: Coap+mbed-client-c libraries.
https://cloud.mbed.com/docs/current
Other
2 stars 11 forks source link

Random CoAP message ID #59

Closed markus-becker-tridonic-com closed 7 years ago

markus-becker-tridonic-com commented 8 years ago

Why is time() used for seeding the RNG? This only properly works on RTC enabled devices. It should now be seeded using arm_random_seed_get(void), or?

Then randLIB_get_8bit() could be used in sn_coap_protocol.c:sn_coap_protocol_init() https://github.com/ARMmbed/mbed-client-c/blob/master/source/libCoap/src/sn_coap_protocol.c#L202.

@mlnx

ciarmcom commented 8 years ago

ARM Internal Ref: IOTCLT-828

MarceloSalazar commented 8 years ago

@sbutcher-arm could you please provide an answer for this?

yogpan01 commented 8 years ago

@mlnx This is mbed-client component, we can update this . I will put this into our backlog and provide fix.

markus-becker-tridonic-com commented 8 years ago

Is there an ETA for this feature/fix?

yogpan01 commented 8 years ago

Hi, We need to study this solution a bit more because we need to see the impact of using arm_random_seed_get(void) directly. This component is fairly platform independent and we need to check if we are not introducing any hard dependency by using this API directly. We are also pressed with other items in our backlog so this will take few more weeks before we resolve it. We will however try to speed up this issue for early resolution.

@MarceloSalazar

kjbracey commented 8 years ago

As a library module, seeding should not be mbed-client-c's problem - it makes sense to simply use rand() or randLIB_get_16bit(), in both cases on the assumption that the system/application initialisation has already taken care of appropriate seeding.

I'm not sure whether randLIB is treated as part of mbed-client-c's HAL expectation, but I would have thought it makes sense to do so.

(randLIB_get_16bit seems the appropriate choice here - not sure why it restricts itself to a range of 400 at the moment. And randLIB_get_16bit also covers the possibility of rand() only returning a 15-bit number).

I am aware though that some parts of the mbed client stack have been trying to do their own workarounds for lack of correct seeding on some systems - this may be part of that.

markus-becker-tridonic-com commented 8 years ago

@MarceloSalazar what is the status of this in mbed5?

yogpan01 commented 8 years ago

Hi, There is randomization API that we can use randLIB_get_16bit() but this has to be instantiated by some application , we are trying to find the right component which could do the seeding and then we can use this API. We will take this up now and try to resolve it in a way that it is backward compatible as well as platform compatible (ie. works on non mbed OS platform as well)

simosillankorva commented 8 years ago

Hi, I have created a pull request https://github.com/ARMmbed/mbed-client-c/pull/91, that should fix this issue by using randLIB component (https://github.com/ARMmbed/mbed-client-randlib) for the message ID randomization.

This library calls platform spesific function "uint32_t arm_random_seed_get(void)" to seed the random when initialized. Alternatively it can be also seeded by calling "void randLIB_add_seed(uint64_t seed)".

@markus-becker-tridonic-com could you please check if this fixes the issue?

MarceloSalazar commented 7 years ago

@markus-becker-tridonic-com could you please check if this fixes the issue?

markus-becker-tridonic-com commented 7 years ago

@MarceloSalazar Yes. this is perfect. Please merge as soon as possible.

simosillankorva commented 7 years ago

@markus-becker-tridonic-com @MarceloSalazar The fix has now been merged.