Code in manual has 1 error + 1 warning - requires changes that could result in unexpected results

[SethArchambault]

I'm trying to get a very basic use of argon2 working, but not matter what I do it won't match the correct results. I'm sure it's something simple, but I'm finding it very hard to debug.

Salt: ffffddddffffdddd Password: OkayPassword

Output In Hex Form:
68e427376e16216f19425333ccdcc688d51b92d31b7ba1b2096b6f450254e850
Output in Encoded Form:
$argon2i$v=19$m=4096,t=3,p=1$ZmZmZmRkZGRmZmZmZGRkZA$aOQnN24WIW8ZQlMzzNzGiNUbktMbe6GyCWtvRQJU6FA

When I compare this to running the hashing algorithm from this website I get: https://argon2.online/

Output In Hex Form:
02f9d1e398c0db9676e44550c03e77a4a0d93c74e90c5966d3dd48756a65c5bd
Output in Encoded Form:
$argon2i$v=19$m=4096,t=3,p=1$ZmZmZmRkZGRmZmZmZGRkZA$AvnR45jA25Z25EVQwD53pKDZPHTpDFlm091IdWplxb0

And the same results show in command line:

$ echo -n OkayPassword | argon2 ffffddddffffdddd -i -t 3 -k 4096 -p 1
Type:           Argon2i
Iterations:     3
Memory:         4096 KiB
Parallelism:    1
Hash:           02f9d1e398c0db9676e44550c03e77a4a0d93c74e90c5966d3dd48756a65c5bd
Encoded:        $argon2i$v=19$m=4096,t=3,p=1$ZmZmZmRkZGRmZmZmZGRkZA$AvnR45jA25Z25EVQwD53pKDZPHTpDFlm091IdWplxb0
0.019 seconds
Verification ok

Here's my code:

build.sh

set -e
clang++ -g main.cpp
./a.out

main.cpp

#include <stdio.h>
#include <stdlib.h>
#include "monocypher.h"
#include "monocypher.c"

#define assert(expr) if(!(expr)) { printf("%s:%d %s() %s\n",__FILE__,__LINE__, __func__, #expr); *(volatile int *)0 = 0; }

#include <stdint.h>
#include <string.h>

struct Arena {
    int len;
    void * mem;
};

struct Bytes {
    int len;
    uint8_t * mem;
};

char *base64(Arena arena, Bytes in)
{
...
}

int main() {
    uint8_t              hash[32];               /* Output hash     */
    uint8_t              salt[] = "ffffddddffffdddd";               /* Random salt     */
    crypto_argon2_config config = {
        .algorithm = CRYPTO_ARGON2_I,            /* Argon2i         */
        .nb_blocks = 4 * 1024,                     /* 100 megabytes   */
        .nb_passes = 3,                          /* 3 iterations    */
        .nb_lanes  = 1                           /* Single-threaded */
    };
    uint8_t password[] = "OkayPassword";
    crypto_argon2_inputs inputs = {
        .pass      = password,                     /* User password */
        .salt      = salt,                 /* Salt for the password */
        .pass_size = sizeof(password),           /* Password length */
        .salt_size = 16
    };
    crypto_argon2_extras extras = {0};   /* Extra parameters unused */

    void *work_area = malloc((size_t)config.nb_blocks * 1024*2);
    if (work_area != NULL) {
        crypto_argon2(hash, 32, work_area,
                      config, inputs, extras);
        Arena arena;
        arena.len = 32 * 2;
        arena.mem = malloc(arena.len);
        printf("Output In Hex Form:\n");
        for(int i = 0 ; i <32; ++i) {
            printf("%0.2x", hash[i]);
        }
        printf("\nOutput in Encoded Form:\n");
        printf("$argon2i$v=19$m=%d,t=3,p=1$%s", config.nb_blocks, 
                base64(arena, {.len=16, .mem=salt}));
        printf("$%s\n", base64(arena, {.len=32, .mem=hash}));
        crypto_wipe(password, sizeof(password));
        free(work_area);
    }
    else 
    {
        crypto_wipe(password, sizeof(password));
    }

    return 0;
}

Any ideas what the issue is here? I'm running on MacOS Ventura 13.6..

Thanks!

[SethArchambault]

Oh wow. I think I learned something very valuable. Sizeof includes the 0 key. Right.

This solved it:

    uint8_t password[] = "OkayPassword";
    printf("sizeof password: %d\n", sizeof(password));
    crypto_argon2_inputs inputs = {
        .pass      = password,                     /* User password */
        .salt      = salt,                 /* Salt for the password */
        .pass_size = 12,           /* Password length */
        .salt_size = 16
    };

Output In Hex Form:
02f9d1e398c0db9676e44550c03e77a4a0d93c74e90c5966d3dd48756a65c5bd
Output in Encoded Form:
$argon2i$v=19$m=4096,t=3,p=1$ZmZmZmRkZGRmZmZmZGRkZA$AvnR45jA25Z25EVQwD53pKDZPHTpDFlm091IdWplxb0

Phew!

Okay, just a note - the manual says to do this:

uint8_t password[14] = "Okay Password!";

But that produces this error:

main.cpp:83:28: error: initializer-string for char array is too long, array size is 14 but initializer has size 15 (including the null terminating character)
    uint8_t password[14] = "Okay Password!";

So in trying to sidestep that error, I introduced a catastrophic one!

A change to the docs would have saved me a bunch of hours:

uint8_t password[] = "Okay Password!";  // <--- maybe remove number here
crypto_argon2_inputs inputs = {
    .pass      = password,
    .pass_size = sizeof(password) - 1,           /* <--- and add minus 1 here?*/
    .salt      = salt,            
    .salt_size = 16
};

[SethArchambault]

I'll create a PR - here are the errors and warnings I get when running the example code:

main.cpp:83:24: error: initializer-string for char array is too long, array size is 14 but initializer has size 15 (including the null terminating character)
uint8_t password[14] = "Okay Password!";
                       ^~~~~~~~~~~~~~~~
main.cpp:87:5: warning: ISO C++ requires field designators to be specified in declaration order; field 'pass_size' will be initialized after field 'salt' [-Wreorder-init-list]
    .salt      = salt,                 /* Salt for the password */
    ^~~~~~~~~~~~~~~~~
main.cpp:86:18: note: previous initialization for field 'pass_size' is here
    .pass_size = sizeof(password),           /* Password length */
                 ^~~~~~~~~~~~~~~~

(Could be this is a result of me running this in a cpp file instead of a c file..)

[LoupVaillant]

Hi, thanks for the heroic debugging effort. As the implementer I believe you when you say it was very hard to debug, sorry about that.

(Could be this is a result of me running this in a cpp file instead of a c file..)

I believe that's it: I have a script that compile the example codes, but I didn't think to run it in C++. Considering that Monocypher can be compiled both as C and C++, and more importantly is explicitly meant to be used from C++ as well as C, I reckon your errors are important and should be addressed.

Me, I should try and compile the examples as C++, that will probably find some more errors.

[LoupVaillant]

Hi, thanks for the heroic debugging effort. As the implementer I believe you when you say it was very hard to debug, sorry about that.

(Could be this is a result of me running this in a cpp file instead of a c file..)

I believe that's it: I have a script that compile the example codes, but I didn't think to run it in C++. Considering that Monocypher can be compiled both as C and C++, and more importantly is explicitly meant to be used from C++ as well as C, I reckon your errors are important and should be addressed.

Me, I should try and compile the examples as C++, that will probably find some more errors.

[SethArchambault]

All good! Thanks again for your work!

Sidenote - went to great conference this year called Handmade Boston and Mārtiņš Možeiko recommended a bunch of these options for surfacing issues: -Werror -Wall -Wextra -Wshadow -Wconversion -Wno-deprecated-declarations -Wno-unused-parameter

And -fsanitize=fuzzer which I'm looking forward to trying out..

Not saying any of that would catch this, but maybe it'll help surface some other stuff!