Code in manual has 1 error + 1 warning - requires changes that could result in unexpected results
Closed this issue · 5 comments
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!
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
};
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..)
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.
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.
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!