[liberationtech] DecryptoCat
Nadim Kobeissi
nadim at nadim.cc
Mon Jul 8 19:44:34 PDT 2013
On 2013-07-08, at 2:48 PM, Reed Black <reed at unsafeword.org> wrote:
> 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'm sure "guehh" and so on are either exceptions or relate to very irrelevant commits.
If they're not, then we definitely have a commit documentation problem!
NK
>
>
> 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 companys at stanford.edu or changing your settings at https://mailman.stanford.edu/mailman/listinfo/liberationtech
More information about the liberationtech
mailing list