On Mon, Jul 8, 2013 at 11:00 AM, David Goulet <dgou...@ev0ke.net> wrote: > > Furthermore, looking at those lines of code, there is simply NO comments at > all, > nothing to help peer review, to explain why this or that is done that way and > nothing linked to any design document. This is in my opinion VERY important > that > developers design their system/subsystem beforehand *especially* a crypto > design > in a public document. And, if it has to change, the design should be peer > reviewed before even making one line of code. > > So, the technical critical issue, CryptoCat responded well, quickly but the > point here is that there is a problem in terms of how development is done and > how *little* the maintainability of the code is.
I think there is a bigger problem in the commit messages. Looking at the history, many are "tweak" "guehh" "update" "FIX THE BUG" and some of those are tied to large many-purpose Swiss Army Knife commits. Without descriptive commit messages and single-purpose commits, community review is highly unlikely. It takes an order of magnitude more effort for a reviewer to suss out the intent of a code change. The reviewer is also much less likely to ask about suspicious side effects if he's starting with infinite possibility of intent on first encountering the code. Few volunteers will make a routine effort. Remember when someone tried slipping this into the Linux kernel? + if ((options == (__WCLONE|__WALL)) && (current->uid = 0)) + retval = -EINVAL; Ask if something that subtle have been spotted so quickly if it were one of many Swiss Army Knife "guehh" commits. I think any project that relies on community review for security needs to first stop and ask what would make community review likely. At the least, that's some venue for review discussion where the developers are reading, single-function commits, and descriptive commit messages. Does anyone know if there's something like a "best practices" page to point to for maintaining a healthy reviewer community? -- Too many emails? Unsubscribe, change to digest, or change password by emailing moderator at compa...@stanford.edu or changing your settings at https://mailman.stanford.edu/mailman/listinfo/liberationtech
