Date: Wed, 15 Jun 2016 13:01:16 +0200 From: Joerg Sonnenberger <jo...@bec.de>
On Wed, Jun 15, 2016 at 05:01:58AM +0000, matthew green wrote: > replace the previous hack with something that i believe is actually > correct and, more importantly ;), works properly. This is still broken by assuming alignment, isn't it? The code certainly doesn't make itself easy to audit with all its superfluous casting, but after ten minutes of reading I haven't found any cases of misaligned access. The endian conversion business is applied only to aligned arrays. The input need not be aligned because it is always read with the analogue of le32dec; likewise the hash buffer (nh_ctx::data). The hash state (nh_ctx::state) and result buffers are actually uint64_t arrays. The only part that remains clearly questionable, I think, is the subkey array nh_ctx::nh_key and indexing into it, but that requires more flow analysis than my little brain can handle in a quick skim this early in the morning. Now, I would ding this code because it is written to resist auditing and provides no advantage over the much simpler and faster and smaller-cache-footprint poly1305, but that's independent of whether or not mrg@ eliminated this particular no-strict-aliasing hack.