Looking for code reviews for my AES implementation in cpp
Posted by Almajhoule@reddit | learnprogramming | View on Reddit | 5 comments
Hi, I just finished this AES implementation, and I wonder if I can add something, edit something, or fix any security things. I'll be happy if you leave your honest review (I'm trying to improve myself).
This AES implementation support : AES128 AES192 AES256
opentabs-dev@reddit
one concrete thing nobody mentioned yet: if you're using a precomputed sbox table (the usual textbook approach), your impl is vulnerable to cache-timing attacks. the lookup index depends on the secret key bytes, so an attacker on the same machine can recover the key by watching cache misses. real-world libs use either bitsliced AES or just delegate to AES-NI intrinsics. also +1 on running NIST KAT vectors - if those don't pass byte-for-byte your impl is broken regardless of how clean the code looks.
Suspicious_Coat3244@reddit
To be honest, the fact that you implemented AES yourself puts you already in the lead. That's the kind of project where you suddenly become aware of the sheer number of dirty implementation details inherent in actual cryptography.
Here are some suggestions, although: Crypto code will be reviewed much differently than typical projects; "it works" is merely table stakes. People will scrutinize many more details like timing attacks, side channels, memory handling, constant-time operation, key deletion, test vectors, padding correctness, etc.
Another important thing to do: make sure that your results match one of the official NIST AES test vectors. This is an indispensable part of crypto implementation. Adding fuzzing + known answer tests would probably be more convincing than any additional feature.
The second key point is documentation. Clearly laying out your design choices, the mode of operation (or modes) that you support, limitations, and security assumptions can be invaluable, because reviewers get suspicious when crypto repos feel like they're too sparsely
grantrules@reddit
scr should be src
devseglinux@reddit
Honestly, first off: implementing AES yourself as a learning exercise is already a really solid project. A lot of people talk about cryptography, but actually sitting down and implementing block ciphers teaches you a ton about:
And honestly, asking for review instead of assuming “it’s secure because it works” is a very good sign too.
One thing I’d strongly recommend though:
if this is meant for real-world security use, be extremely careful. Crypto implementations are one of those areas where:
…can be very different things.
A few things reviewers will probably look for immediately:
Also honestly, one of the most important things for crypto projects is documenting the scope clearly:
That distinction matters a lot.
From a learning perspective though, projects like this are genuinely valuable because they force you to understand what high-level crypto APIs normally hide from developers.
Really cool project overall, and the fact that you’re actively inviting criticism/review already puts you ahead of a lot of “I built military-grade encryption” GitHub repos
Almajhoule@reddit (OP)
hi thnx for your advice and about the :