search

narfbg / SimpleEncryption

Simple Encryption for PHP
22 stars 6 forks source link

Audit #1

Open narfbg opened 10 years ago

narfbg commented 10 years ago

I'm pretty confident in the code so far, but obviously, as a cryptographic library it needs more eyeballs.

Two notable pieces of code that could be a concern:

@sarciszewski @defuse @GDmac @ircmaxell You guys have previously given me suggestions about CodeIgniter's Encryption library, so I take it that you'd be interested in having a look at this one too. :)

defuse commented 10 years ago

I wrote a relevant comment about re-using the key for double HMAC verification and/or using MD5 here: https://github.com/defuse/php-encryption/issues/21#issuecomment-48827719

defuse commented 10 years ago

For HKDF it's more standard to generate the two keys by calling HKDF twice with different info parameters. I don't see an immediate problem with using HKDF then splitting, though. I think it's fine, it's just not really the way HKDF was supposed to be used.

ircmaxell commented 10 years ago

I raised a few issues, most of which with suggestions, but a few with actual bugs or problems found. It's a start :-)

GDmac commented 10 years ago

Good points @ircmaxell , i like the suggestions on #4 (salt based derivative keys) and #2 (stateless). In my (noobish) opinion, the workflow and steps of the library should be easy to follow for reference and for audit. As to the order of hash-mac on cyphertext, cyphertext+iv, or pre- or post-base64 encoding, i'll leave that up to @ircmaxell and/or @defuse to discuss - and maybe what the RFCs say about that.