Not logged in
Log in now
Create an account
Subscribe to LWN
LWN.net Weekly Edition for May 23, 2013
An "enum" for Python 3
An unexpected perf feature
LWN.net Weekly Edition for May 16, 2013
A look at the PyPy 2.0 release
A critical security bug in tarsnap
Posted Jan 20, 2011 2:17 UTC (Thu) by wahern (subscriber, #37304)
The weird thing is, though, the nonce isn't incremented in his crypto_aesctr_stream() (i.e. after each block generation, which is the natural way to do it). So I suspect that he's concocted some sort of protocol atop AES-CTR, which would explain why he didn't just use OpenSSL's API. Alternatively, maybe he wrote the code before OpenSSL provided AES-CTR. But AES-CTR has been in OpenSSL for several years, and it doesn't explain why the nonce isn't incremented in the obvious place.
The author's description of the issue doesn't answer these questions.
Posted Jan 20, 2011 5:53 UTC (Thu) by wahern (subscriber, #37304)
The code is indeed reimplementing OpenSSL's CTR support. There's no reason to do this unless it was implemented before OpenSSL exported that functionality. But regardless the bug isn't in the duplicated code; it's in code which would have been necessary whether using OpenSSL's CTR mode or not.
If there's a moral to this story, it's to properly comment code. All the existing comments just repeat in English _what_ the C code is doing (which is superfluous if you understand C well). None of the relevant comments explain _why_ the logic exists (i.e. explain the chunking scheme and the relation of the nonce to ensuring a unique counter). The why is the most important part, and had the why comment existed the bug might never had come about.
This incident is a classic lesson for how to properly comment code. Classic. They should use this as an example in classrooms.
Copyright © 2013, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds