Cooperative Cancellation in PasswordHash

[huven]

This issue to check if a PR would be accepted that adds support to abort a key derivation operation. This is useful e.g. when the user aborts a request that needs a computationally expensive key.

This could be implemented by adding a set_stop_token(std::stop_token) to PasswordHash. A sub-class can check the token at well-chosen intervals. If stop requested, the derivation can throw a cancellation exception to signal that the operation was aborted.

Would something along these lines be a valuable addition, or too specific to my use-case?

As C++20 is already required this should not introduce a new dependency.

[huven]

To illustrate how this might look like

huven/botan@06846bcac47a8370f46920970275fa0a428e2227

Here I added the ivar to the PasswordHash class, and Argon2 checks it at regular intervals, throwing an exception when set. Pretty simple, yet effective.

[reneme]

Would something along these lines be a valuable addition, or too specific to my use-case?

I really like the idea in general. This is a long-running computation by definition and allowing to cooperatively abort it would certainly be nice. Also, given that std::stop_token is a standard way of doing this, I personally don't see why we shouldn't provide this hook. It would certainly need a prominent mention in the docs to make people understand how that is meant to be used.

Independently, there might even be other places that could benefit from such a thing. I'm thinking of RSA key generation, for instance. On slow hardware (or under a broken RNG) this can take arbitrarily long as well.

This could be implemented by adding a set_stop_token() to PasswordHash.

Instead of adding the std::stop_token to the state of the password hash object, I would suggest to allow passing it into the ::hash() and ::derive_key() methods as a std::optional. That way the behavior is more self-explanatory when re-using the same password hash object for multiple derivations and we don't have to deal with any state handling or protected: members that are used in derived classes.

As C++20 is already required this should not introduce a new dependency.

According to the compiler support table this is fully supported starting with Clang 20 only. 🫨 That might become a showstopper or need a feature toggle based on the compiler version at laest. A quick test on godbolt seemed to be fine, though. I guess, a reliable unittest would definitely be useful to see if it passes CI.

[huven]

Instead of adding the std::stop_token to the state of the password hash object, I would suggest to allow passing it into the ::hash() and ::derive_key() methods as a std::optional. That way the behavior is more self-explanatory when re-using the same password hash object for multiple derivations and we don't have to deal with any state handling or protected: members that are used in derived classes.

That is better indeed, thanks, will change this.

I guess, a reliable unittest would definitely be useful to see if it passes CI

Yeah, that was next on my list 😄 was waiting on a confirmation like this before proceeding with that. I already added a supports_cooperative_cancellation() in my repo to accommodate testing cancellations.

Ok, I'll proceed and prepare a PR.