Unclear what the int being returned from several XChaCha20 functions is

[kozross]

According to the documentation for XChaCha20, multiple functions return an int. However, the documentation doesn't specify what this int means or what values it can take. What's the intent here?

[enkore]

It's somewhere in the docs (but I'm not finding it now, so it could/should be somewhere more prominent, I looked at the "Usage" page and searched), but libsodium uses the "inverted return code" convention: 0 means success and negative means error.

These functions can't fail though, as they're just a stream cipher. Note that wrong parameters in libsodium are handled using the misuse mechanism - which calls abort() (outside tests).

[enkore]

Returning a status from all crypto functions has an advantage: It makes the API more regular, even if the particular implementations in question can't fail.

[kozross]

'Regularity' is not a useful goal in and of itself: 'everything is a string lol' is very regular behaviour in certain places, but it's horrible to work with.

If a function returns a value, the expectation is that the value is meaningful. If it's always the same, why even bother? In fact, I'd argue it's confusing, because (just like I did above), you expect it to mean something. It also doesn't change the fact that things should be clearly documented close to their place of use: having to trawl docs for a default that's not clearly signposted anywhere is tedious at best, actively dangerous at worst, since it invites misuse.

[Jookia]

Maybe you could just add to each function in the documentation that it always returns 0 and that checking the return code is not necessary?

[KizzyCode]

As far as I understand it, it makes the API more generic; especially if there might be future implementations that can fail.

Think of it as some kind of implicit protocol/trait for functions that serve a similar purpose. If the signatures are always the same, it is possible to do something like:

int algo = /* Get algorithm from packet header */;
int(*decrypt_fn)(...) = NULL;
switch(algo) {
    case 1:
        decrypt_fn = &decrypt_algo_a;
        break;
    case 2:
        decrypt_fn = &decrypt_algo_b;
        break;
    ...
}

However I agree that there are some places which could benefit from a more explicit (inline?) documentation 😊

[Kleidukos]

I'm not in disagreement with the API especially when NaCl has the same API for XSalsa20, but yes I would submit a PR to put more explanations of this convention. :)

[jedisct1]

https://libsodium.gitbook.io/doc/quickstart#how-to-check-if-a-function-call-succeeded

[Jookia]

Yes, that is helpful! But it's not useful for a reference where you want to know everything about a function in one place.

[kozross]

I agree. This information needs to be replicated at every function, because otherwise, how am I supposed to know where to find it? 'Quickstart' is not the place I go looking for error conventions.

[Kleidukos]

@jedisct1 Would you be open to a patch to point this out more clearly?