Search Mailing List Archives
reed at unsafeword.org
Mon Jul 8 11:48:45 PDT 2013
On Mon, Jul 8, 2013 at 11:00 AM, David Goulet <dgoulet at 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?
More information about the liberationtech