LoupVaillant / Monocypher

An easy to use, easy to deploy crypto library
https://monocypher.org
Other
580 stars 80 forks source link

Support Argon2id #243

Closed Wastus closed 1 year ago

Wastus commented 1 year ago

I know the thread on support for Argon2d but I'd like to reiterate the issue, this time for Argon2id (which needs Argon2d internally).

The RFC 9106 has two recommendations:

The Argon2id variant with t=1 and 2 GiB memory is the FIRST RECOMMENDED option and is suggested as a default setting for all environments. This setting is secure against side-channel attacks and maximizes adversarial costs on dedicated brute-force hardware. The Argon2id variant with t=3 and 64 MiB memory is the SECOND RECOMMENDED option and is suggested as a default setting for memory- constrained environments.

Both of which cannot be implemented when using monocypher.

From what I can gather is that Argon2i is not as secure against Time-Space Trade-off Attacks as Argon2d. And Argon2d is vulnerable against side channel attacks. So Argon2id is a combination to make Argon2i more resistant against TSTO (or TMTO) attacks and at the same time take the security against side channel attacks from Argon2i.

There are quite a few recommendations around, that if you are unsure which attack schemes apply to your application using Argon2id would be the best approach. As it is quite difficult to assess which attack vectors are viable and attackers become more and more creative - I'd say having Argon2id in moncypher would be a great addition.

PS: Thanks for the work on this. It can be used on a Cortex M0+ with the Argon2i functionality needing around 26 kB in an optimized build. I'm just not sure how good Argon2i is with the limited 8 kB block I can work with... Great that the key parameter is also exposed in the API, because I think storing a unique key in the not easily accessible flash of the µC can increase the security for such an restricted environment a bit.

LoupVaillant commented 1 year ago

So Argon2id is a combination to make Argon2i more resistant against TSTO (or TMTO) attacks and at the same time take the security against side channel attacks from Argon2i.

Most people are confused about this bit, and I blame the RFC. I take issue with the following sentence:

This setting is secure against side-channel attacks and maximizes adversarial costs on dedicated brute-force hardware.

This is false. Argon2id has some side channel resistance, but unlike Argon2i it is not immune. Saying the setting is "secure" against side channel attacks is a gross and misleading oversimplification: adversarial costs are maximised only to the extent that no side channel attacks were performed. In fact, a successful timing attack may reduce adversarial costs well below the normal costs of a 3-pass Argon2i. In other words:

This is why so far, I’ve stuck with Argon2i even as Libsodium’s default switched to Argon2id. I disagree with RFC’s reasoning, and it’s not clear in practice which of Argon2i and Argon2id is better in any given situation: it mostly comes down to how much you fear timing attacks.

That said, Argon2id is not that bad. And the peace of mind that comes with complying with an RFC does have value. I’ll think about it, no guarantee.


I'm just not sure how good Argon2i is with the limited 8 kB block I can work with

I hear it’s pretty terrible. But you have another, perhaps more serious problem there: Argon2i uses 64-bit multiplication. While that’s perfect for modern desktop CPUs, your Cortex M0+ is a 32-bit processor, right? It won’t have the 64->64 MUL instruction required to make that fast, which will significantly reduce speed, and increase attacker advantage accordingly. My advice: avoid Argon2 in this processor. Either offload the computation to the cloud or something, or use an algorithm that better exploit the performance characteristics of your hardware.

I suspect that with your processor with so little memory, PBKDF2 is the way to go. You can build it on top of Blake2b if you want to use Monocypher, but bear in mind that even that is not ideal: Blake2b is optimised for 64-bit processors. Ideally you’d want Blake2s instead. And if your your processor has instructions to implement SHA-2, SHA-256 will be even faster, letting you do even more iterations.

fscoto commented 1 year ago

I'll come out of my little corner of standards compliance and point out that I-D.draft-bar-cfrg-spake2plus-08 specifying the SPAKE2+ PAKE (which was sent to the RFC Editor; barring unusual circumstances, it's on its merry way to becoming an RFC) recommends Argon2id and scrypt, but makes no hard requirement either way. I-D.draft-irtf-cfrg-opaque-09 has no recommendations for any particular Argon2id variant. I'm not sold on adding Argon2id unless there's a spec that requires it, which is usually when demand skyrockets, cf. how things went with IETF ChaCha20.

LoupVaillant commented 1 year ago

OK, let’s leave this issue open for a few months, see how many people need this.

samuel-lucas6 commented 1 year ago

@fscoto I would like to point out that the Argon2 RFC says the following:

Argon2id MUST be supported by any implementation of this document, whereas Argon2d and Argon2i MAY be supported.

Barely anything I've seen using Argon2 uses Argon2i. I don't think that's surprising given the RFC, Argon2id recommendations elsewhere (e.g. blogs, forums, libraries, etc), attacks, weaker adversarial costs, and the existence of cache-timing attacks for scrypt but people still using it.

However, I completely agree that the secure against side-channel attacks claim is misleading. Argon2id unfortunately has no real use case because it should basically be viewed as Argon2d.

All of the current/popular password hashing algorithms have problems:

Argon2 is still one of the best. Otherwise, I'd say switch to Balloon and be done with it.

LoupVaillant commented 1 year ago

I plan to add Argon2id support in Monocypher 4, and I'm breaking the API in the process. I've just pushed a tentative design for the new API (usable right now, though only Argon2i is supported for now). The commit message discusses rationales & uncertainties. Feedback would be very welcome.

samuel-lucas6 commented 1 year ago

Awesome. I don't think default values are a good idea for exactly the reasons you outlined. The version hopefully won't change, so I suppose that could be omitted. Probably makes sense to have nb_lanes in case. Can't help with the other questions.

Wastus commented 1 year ago
LoupVaillant commented 1 year ago

(Hello everyone, I hope you had a nice solstice, and wish you a happy new year!)

Monocypher's Argon 2 implementation is almost complete. It all works as far as I can tell (confirmation pending with TIS-CI). I'm quite happy with the result: I managed to cram more functionality in fewer lines of code. Now we need to decide what the API should look like.

Argon2 specs mandate a fairly big API. For Monocypher we need the following:

In C that's 14 freaking parameters. Cramming all of them into function arguments is unreasonable, that's too unwieldy. We need to put at least some of them in a struct context, and use that as a poor man's named arguments. That's what I've done in the current API:

typedef struct {
    uint32_t algorithm;  // Argon2i, Argon2d, Argon2id
    uint32_t nb_blocks;  // memory hardness, >= 8
    uint32_t nb_passes;  // CPU hardness, >= 1 (>= 3 recommended for Argon2i)
    uint32_t nb_lanes;   // parallelism level (single threaded anyway)
    uint32_t salt_size;  // we recommend 16 bytes
    uint32_t hash_size;  // we recommend 32 bytes per key
    const uint8_t *key;  // pointers are aligned to 8 bytes
    const uint8_t *ad;
    uint32_t key_size;
    uint32_t ad_size;
} crypto_argon2_ctx;

#define CRYPTO_ARGON2_D  0
#define CRYPTO_ARGON2_I  1
#define CRYPTO_ARGON2_ID 2

void crypto_argon2(uint8_t       *hash,
                   void          *work_area,
                   const uint8_t *password,  uint32_t password_size,
                   const uint8_t *salt,
                   const crypto_argon2_ctx *s);

The idea here is to put the more stable arguments in the crypto_argon2_ctx, and the less stable ones as function arguments. This has the disadvantages of separating buffer pointers from the associated sizes, which I fear may be a tad confusing.

An alternative design would involve proposing reasonable defaults for all 3 variants of Argon2. Alas, some recent reading has convinced me that there is no such thing as a reasonable default. The state of the art is too unstable, the threat models too varied. Even the number of passes is not quite stable depending on the use case. So, no defaults for us I'm afraid.

Alternatively, we could put everything in the context. Or everything except the output and maybe the work area. I guess it would work, but it would be cumbersome. But I think there's another way: using several structs:

typedef struct {
    uint32_t algorithm;
    uint32_t nb_blocks;
    uint32_t nb_passes;
    uint32_t nb_lanes;
} crypto_argon2_config;

typedef struct {
    const uint8_t *pass;
    const uint8_t *salt;
    uint32_t pass_size;
    uint32_t salt_size;
    const uint8_t *key;
    const uint8_t *ad;
    uint32_t key_size;
    uint32_t ad_size;
} crypto_argon2_inputs;

void crypto_argon2(uint8_t *hash,uint32_t hash_size;
                   void    *work_area,
                   const crypto_argon2_config *config,
                   const crypto_argon2_inputs *inputs);

One little problem here is that most of the time, we wouldn't be using the key and the additional data at all, forcing us to either omit those fields during initialisation (and risk compiler warnings), or manually set the whole struct to zero thus:

crypto_argon2_inputs inputs;
memset(&inputs, 0, sizeof(inputs); // set key and ad to zero
inputs.pass = pass;
inputs.salt = salt;
inputs.pass_size = pass_size;
inputs.salt_size = salt_size;

I'm personally tempted to split the inputs in two: the regular ones, and the extra ones nobody uses. That way we can provide a default all-zero value for the extra parameters:

typedef struct {
    uint32_t algorithm;
    uint32_t nb_blocks;
    uint32_t nb_passes;
    uint32_t nb_lanes;
} crypto_argon2_config;

typedef struct {
    const uint8_t *password;
    const uint8_t *salt;
    uint32_t password_size;
    uint32_t salt_size;
} crypto_argon2_inputs;

typedef struct {
    const uint8_t *key;
    const uint8_t *ad;
    uint32_t key_size;
    uint32_t ad_size;
} crypto_argon2_extras;

extern crypto_argon2_extras CRYPTO_ARGON2_NO_EXTRA

void crypto_argon2(uint8_t *hash,uint32_t hash_size;
                   void    *work_area,
                   const crypto_argon2_config *config,
                   const crypto_argon2_inputs *inputs);
                   const crypto_argon2_extras *extras);

We could also allow the extras parameter to be NULL, so the user wouldn't need the constant at all. Handy but not the most consistent.

What do you think?

LoupVaillant commented 1 year ago

Okay, I believe this should be the final version of the Argon2 API. @fscoto, if you're okay documenting this that would be awesome. No rush, I still need to spend some weeks on the other issues before we call it a release.

fscoto commented 1 year ago

Just for your information, this might take a bit. I'm not sure how I want to tackle this yet. I'll get around to it though.

LoupVaillant commented 1 year ago

Everything done & documented. Maybe some details are left, but I believe this is good enough to close the issue as completed.