Evidenced in gen_key() function, though it's a more general problem with the file format.

The problem here is that password hashing is a rather fast moving field, where the relatively weak entropy of passwords encourage us to routinely update the strength of our password hashes, or even the algorithm itself. And depending on context (are you using your beefy desktop or your crappy palmtop?) you may want to adjust the password strength. Version numbers are insufficient to deal with such a fast evolution, or the various use cases. Which is why I believe it is worth putting at least Argon2 parameters in the file format itself:

• At least 1 bit to chose between Argon2i and Argon2id, though I would recommend a full byte for (i) easy of coding, and (ii) having some room to use other password hashing algorithms. Note : while I used to favour Argon2i, and still believe the case for Argon2id is quite flimsy, I'm now mostly uncertain about which is best, most notably because this is so highly dependent on the threat model: how much do you fear timing attacks, vs just having your key file stolen and attacked offline?
• At least 7 bits for the number of passes.
• 32 bits for the number of blocks.
• At least 4 bits for the number of lanes. Or make it 8, we never know when we'll get those 256 core CPUs. Or give up and just use 1 lane, I don't think it's the most important parameter.

Speaking of lanes, I recall some papers noting that parallel versions of Argon2 are a bit weaker than just running several serial instances in parallel (with different additional data to distinguish them), then hashing the results together. So if you use a parallelism parameter, you should probably do that instead of using Argon2 lanes (whose support is not universal anyway, see Monocypher not supporting threads, and libsodium not even supporting lanes).

Of course, nothing here stops you from setting good defaults, or even hard code the parameters you encode into the key file. You can always add options or change the defaults later, as long as the file format itself (and the reader side) is prepared for the change.

I noticed a couple recurring patterns:

if (!condition) {
    BAIL(ERROR_CODE);
}

With a little macro those 3 lines could be reduced to just one:

#define CHECK(condition, ERROR_CODE) do { \
        if (!condition) {                 \
            BAIL(ERROR_CODE);             \
        }                                 \
    } while (0)

// ...

CHECK(condition, ERROR_CODE);

You could stop there. But I noticed another, quite frequent sub-pattern:

err = function(arg1, arg2, arg3);
if (err != ERR_OK) {
    BAIL(err);
}

This one could also be reduced to a single line. (Gcc and Clang support having zero optional argument, but they may complain about it not being standard).

#define CHECKED_CALL(function, ...) do { \
        err = function(__VA_ARGS__);     \
        if (err != ERR_OK) {             \
            BAIL(err);                   \
        }                                \
    } while (0)

// ...

CHECKED_CALL(function, arg1, arg2, arg3);
CHECKED_CALL(function2); // zero optional arguments is not standard unfortunately

Or you can avoid __VA_ARGS__ altogether by wrapping the function call itself:

#define CHECKED_EXPR(expr) do { \
        err = expr;             \
        if (err != ERR_OK) {    \
            BAIL(err);          \
        }                       \
    } while (0)

// ...

CHECKED_EXPR(function(arg1, arg2, arg3));
CHECKED_EXPR(function2()); // perfectly standard !

Now many would say this is too many macros. Or the macros are too heavy, or it's unmaintainable because they're macros… Perhaps. It's a balance between compactness and approachability. My guess here is that those macros are worth the trouble, because they are part of a consistent error handling system that pervades the whole program. The patterns they replace are everywhere, so the 30 seconds spent on understanding them are quickly saved when reading the rest of the program.

And compactness in my opinion doesn't just make the code shorter, it makes it more readable: less error handling noise, more business logic, and with shorter function there's a greater chance that the exit point is visible in the same screen as the BAIL() call.

Now that's the kind of thing that should be tested, see if it suits your taste. I've tested this on other projects and it certainly suits mine. 😄

Spotted here.

The problem with /dev/urandom is that it's a file. Many errors specific to reading files could happen, up to and including failing to fill the buffer. Even if we assume the return code is enough to detect all errors (which may be a safe assumption, I won't bet my hat), returning an error code is the wrong API in my opinion: what's the user to do when faced with the failure something as basic as the RNG? In practice I believe the only choice is to panic with a suitable error message.

What I'm saying is, what you really want is an interface like this:

void fillrandom(uint8_t *buf, size_t len);

(Note my preference for size_t when it comes down to buffer sizes. Others would use ssize_t instead. The point is to use a type that can describe the whole range of possible sizes, and those are it. Though of course in practice you'll rarely do that on huge buffers, so if you're crystal clear that the admissible range is small, int or an unsigned can be enough.)

That way users don't have to, and cannot forget to, check the return value. fillrandom() will either successfully complete, or panic. Now panicking is not nice, so we want to minimise that ever happening. On non-Windows systems my answer would be:

• If arc4random_buf(3) is available, use it: it does exactly what we want and virtually never crashes.
• On Linux systems unlucky enough to lack arc4random_buf(3), try getrandom(2) instead. This one can fail, so you may want to read the exact error and panic or retry accordingly.
• If your system is lacking any sort of system call to get randomness, either use /dev/urandom or (and I would consider it a valid approach in 2023) refuse to compile altogether: properly handling file related errors and retries is no picnic. @fscoto once showed me a soberingly long example.