Hacker News new | ask | show | jobs
by ejcx 2596 days ago
The first thing I do whenever someone writes their own password manager is to read the Encrypt function. This one is AES-CBC with its own hand rolled integrity scheme. Not very strong by modern standards
1 comments

Doesn't look very hand rolled to me. It's standard HMAC. The only unusual thing is the timing-unsafe comparison[1], which probably needs fixing. It looks like an attempt was made at a constant-time comparison (|= ^ pattern sure looks like it), but the early return breaks it again. I'm not sure if much can be gained from a timing attack in this particular instance though, since the key fully depends on user input in the first place.

(By the way, even Microsoft's own documentation doesn't use constant-time comparisons for HMAC[2]!)

[1] https://github.com/nrosvall/ylva/blob/2a4afcfb3727151fa09fdd...

[2] See the example on https://docs.microsoft.com/en-us/dotnet/api/system.security....

I'm not a C# expert by any means. Is the IntegrityHash of the plaintext, and not the ciphertext? https://github.com/nrosvall/ylva/blob/2a4afcfb3727151fa09fdd...

That would be a really serious flaw. If not, hand rolled AES-CBC-SHA256.... why not just use an AEAD implementation? This is exactly why I look at these. There's a lot of nuance to that one decision, and so it usually gives quite a bit of signal about the project as a whole.