michelp / pgsodium

Modern cryptography for PostgreSQL using libsodium.
Other
546 stars 32 forks source link

Better NULL input checking? #47

Closed michelp closed 1 year ago

michelp commented 1 year ago

Reported by @ioguix in #43

By the way, note that if I give NULL as additional data, it just crash:

pgsodium.crypto_aead_det_decrypt was incorrectly not labeled STRICT so it crashed on NULL input, I can fix that, or I'm thinking it makes more sense for all functions (in most cases) to throw errors on NULL input instead of returning NULL, which means removing STRICT and doing NULL checking as shown in this branch:

https://github.com/michelp/pgsodium/compare/fix/better-null-checking?expand=1

this is just one example of many that would need to be added. Thoughts?

ioguix commented 1 year ago

Hi,

Throwing error from functions seems better:

ioguix commented 1 year ago

I'm not qualified enough with encryption to review your patch, but it seems nonce and additional data are optional with aead_det_*, am I right? See: https://github.com/jedisct1/libsodium-xchacha20-siv#usage

With ietf variant, it seems only additional data can be left NULL, right? https://doc.libsodium.org/secret-key_cryptography/aead/chacha20-poly1305/ietf_chacha20-poly1305_construction#combined-mode

Does it worth sticking with this from the pgsodium point of view?

michelp commented 1 year ago

Yep, I've pushed some changes to the same that allow for NULL associated with det and ietf and keeping support for NULL nonce with det. It's working well, so I'm going to apply the same pattern across the library.