aead_chacha20poly1305_* argument divergence about enacl HOT 6 CLOSED

I have a patch for this now, fixing the order.

There are other problems in the code which I also have fixes for:

• Subtle memleaks on the error path
• It really implements the IETF construction

This is good, because it means I can now break people's code if they have used the old API and rename the functions. While here, I'm also going to fix the parameter order (which is already in a branch slated for master)

from enacl.

I think we should try to make the parameter order be the same as in the Libsodium call chain. But maybe @hanssv had a reason to implement them the other way around. I know that in a language with currying, the parameter order where Key and Nonce comes first means you can build a partial function over the current operation. But in Erlang, where no currying is present (sadly), I think it would be better to make this harder to abuse.

Though I'd like to wait a bit on Hans, because he (and by extension Aeternity) are likely users of that call already and they'll need a heads up to change it around.

The alternative to a do ... while (0) block is likely to have a cleanup label in the epilogue of the function body which you jump to. Personally, I think I would prefer the cleanup label, but this is mostly because I think this is more in the style C programmers tend to write code (I might be wrong, however).

The do-while cleanup I'd say: go ahead if you want to make the change. We should be able to test against correctness as long as we make sure the memzero happens.

from enacl.

No, no fundamental underlying reason ;-) Just bad style from me...

We (Aeternity) have yet to switch back to main enacl (using our fork at the moment) so please fix this to you liking, and we can make the necessary adjustments when we do the switch.

from enacl.

@hanssv I'm not sure if it's bad style or not. I got some opinions on that, and it was half and half. Half liked it, half didn't get it.

from enacl.

If we have the go-ahead from Hans here, I think we should aim to have the API reflect whatever libsodium is using if possible all the way. It does remove one layer of possible misinformation, which could otherwise happen.

While we can handle return values such as keypairs as #{ public := PK, secret := SK } so you have a harder time swapping those, I'm not sure we can make the input argument easier. Passing a map or record seems somewhat defeatist to me. So by all means, go ahead!

from enacl.

This has been solved and 1.0.0 is out.

Note: do test it before you go havoc in using as I've made a number of changes, some of which might have stability effect. OTOH, the code is in a far better place now moving forward.

from enacl.