Graceperiod not correctly updated if rate limit is reached

I am running the following scenario:

I enter a wrong key code for 6 times. Each time a wrong code is entered I display an alert message on the device screen.
If we reach the limit of graceperiod, then I will lock the device for 12 minutes, the time needed to have a new trial.
This is done when handle_key code function is called:

`else if (nx_keycode_is_rate_limited())`

This check is done on the base of rl_bucket and not the graceperiod value.
At this level the rl_bucket will be lower than 12 minutes but the garceperiod is 1

The time between process call is 4 minutes.

const uint32_t NEXUS_COMMON_IDLE_TIME_BETWEEN_PROCESS_CALL_SECONDS = 240;

We know that the garceperiod will be updated to be 0 only when next periodic processing is called.

Let’s assume that power off of the device is done right away after having the screen locked.

Then we power on again the device. The result of the initialisation will be as follow:

    // Fill rate limiting bucket with grace keycodes upon power up
    _this_core.rl_bucket =
        _this_core.stored.graceperiod_keycodes *
        (uint32_t) NEXUS_KEYCODE_PROTOCOL_RATE_LIMIT_REFILL_SECONDS_PER_ATTEMPT;

Since the last stored value of graceperiod_keycode is 1, then the rl_bucket will be 12 minutes. On that case the device will give the user the possibility to enter a key code one more time which should not happen.

My suggestion is to have the update of remaining attempts done right away when updating the rl_bucket value. On that case will have a kind of coherence between rl_bucket and grace period counter.

Ayoub,

Thank you for reporting this. I think you might be right:

I have not tested this myself, so let me know if the explanation above matches your understanding.

Eric

Eric,

Thank you for the response.

Yes, that’s exactly what I meant. The graceperiod will not be immediately updated.

Using the nx_keycode_handle_single_key could be an alternative, but I am not planning to go for it in the moment.

So I have a suggestion. Is it possible to use the same approach you are using for the nx_keycode_handle_single_key. I mean when nexus_keycode_mas_bookend_push is called the first thing that is done is to call nxp_common_request_processing(); in order to update both, the rate limiting bucket and the graceperiod.

Could we do the same for the nx_keycode_handle_complete_keycode ? I mean do the processing before doing the check on nx_keycode_is_rate_limited? ( between the check nexus_keycode_core_init_completed and nx_keycode_is_rate_limited)
Or may be there was a reason behind not doing it already?

I think there are no risk or drawbacks on the whole process if we do this.

With regards,
Ayoub

Ayoub,

Yes, I think you could do that. In fact I think it should work that way as this is a bug.

However, I don’t have the time to make the change and test it. If you want to make the change yourself and open a pull request then please feel free, then I or someone else from the community might be able to look at it later.

Thanks,

Eric