The state of null

[boenrobot]

Encrypt can accept null, and yet decrypt returns null on success or on failure. This is a bad design choice. There's no way to identify an error on null.

One of the following should happen:

1. Make null unacceptable for encrypt(). That's what I currently do outside of the lib, but it would be better if the lib itself would throw if null is given.
2. Make decrypt() throw an error on failure, instead of returning null. Contrary to the docs, decrypt() does throw an error on one occasion - if hmac verification is on, and the HMACs mismatch... Just extend that to also happen for input that can't be decrypted for any reason, not just HMAC mismatch.

Personally, I would prefer option 2, as it would also allow me to handle the error in a custom way. To make handling easier, perhaps a single error type can be thrown that wraps whatever internal error was thrown.

And on that note, if option 2 is to be done, maybe don't log the error to the console, but let the user log if they want to, wherever they want to.

Of course, I understand both of these are potential BC breaks, so a major version change would probably be a good idea.

[sehrope]

Option 2 is the direction I'd like to go with this. The decrypt operation would throw an Error if the HMAC does not match. Other ideas I'm considering are having the return value be a Buffer and an explict canonical URL-safe text format.

Any one of those would be a breaking change for a number of parts of the library so it will be part of a semver major upgrade. Likely with some upgrade / compatibility shim to aid migration for existing users.

[boenrobot]

I don't think returning a buffer or any sort of intermediate is a good idea, as it makes this otherwise simple library more complicated to use.

Unless you mean you'd store it in this intermediate form when encrypting it, and restore it from that form when decrypting... in which case, that's fine I guess.

[boenrobot]

Speaking of migration for existing users and intermediate forms... Maybe it might be a good idea to take lessons from php-encryption and implement something similar.

i.e. include an intermediate form with some sort of meta data that defines the version of the encrypted string, so that the proper decryption can be picked and (equally importantly) in order to allow the detection of outdated encrypted strings, and allow their re-encryption with stronger encryption.

[sehrope]

Yes the next version will include an explicit version string as well. One trick we can use is that the current format is prefixed by the hex HMAC or hex IV. The characters for that can only be [0-9a-f]. By picking anything else for the version prefix of the next version (ex: use a "g" prefix), we can easily differentiate between old and new formats.

[boenrobot]

I suggest "v2|", where "v" is the prefix to differenciate versions, "2" is the number, and "|" is the delimiter for the version token, so that in the future, "2" could be anything arbitrary, as long as it doesn't contain "|"... like, to take an extreme example 2.1@beta~prerelase!github#deadbeef.

Make sure the version is included in the computed HMAC also... since a version change should mean a new HMAC (even when all else is the same), and thus, it should be in need of re-ecnrypting.