Make Challenge generation more customisable

[dscreve]

Make Challenge generation more customisable

[dimitribouniol]

I’m still not convinced we need to mess with the generator here — you really shouldn’t be picking anything other than a random number for the challenge on anything other than tests, and if you are concerned about fulfilling responses across multiple systems, you should use redis or a database to keep all of them informed of which challenges were used for whom after the response is generated, but before it is delivered to the user.

If you have a use case you think is valid, please elaborate on why you think this is a solution to that so we can see if any alternatives exist first.

[dscreve]

This pull request reponds to a need we have here while keeping the current behavior still working. I’m just adding some behavior for a given context where server has no access to the user database and work in a load balanced environment.
It seems that you have thinked about a very common architecture : just one server that serves web with direct connexion to user database. That’s not my case, and that’s why I did this contribution.
If you don’t think it make sense in your project, I’ll keep my forked version to work with.

[dimitribouniol]

I want to clarify that I’m not pushing back because I think it’s bad for the project, but because you are likely opening your system to being vulnerable if you aren’t using a random challenge as is provided. The manager is not stateful, and nothing is preventing you from signing your challenge with a private key and delivering it to the client along with the response, and you sending it back when the client delivers/signs with their key. However, you still need to make sure the challenge is not re-used or a replay attack can open you up to abuse, which brings you back to needing to design a way to store them even temporarily (see https://www.w3.org/TR/webauthn-2/#sctn-cryptographic-challenges for more here)

Controlling how these challenges are generated without being very careful will likely make it easier for others to make similar mistakes, which is my primary worry, but if you have a scheme in mind that doesn’t have any major flaws, then I’d prefer we support that directly rather than poking a hole anyone can unintentionally misuse.

[dscreve]

Here is my proposal : we keep the challenge management as it is, but we just add the ability to append custom data at the end of random array. Then, we keep the randomness of the challenge, and add the ability to add custom data.

what do you think about that ?

[dscreve]

I've put some changes that keep challenge generation internal and allow adding custom data on challenge.
Hope this fits all your requirements.