[PATCH] Re: Implement SSL certificate checking

Sebastian Spaeth Sebastian at SSpaeth.de
Wed Dec 15 02:22:10 GMT 2010


On Tue, 14 Dec 2010 21:56:09 +0100, Johannes Stezenbach <js at sig21.net> wrote:
> You're quick, that's good.  But it's also bad...
> If you mix cleanups and functional changes, the patch is much harder
> to review.  Please take the time to seperate the changes before asking
> others to spend time reviewing it.

I needed to "mix up" a few things, as introducing the cert verification
e.g. also reqiured getting rid of the way we did our ssl wrapping.

It's not like I posted a monster-patch that is totally convoluted, after
all it is  "only" 3 files, 132 insertions(+), 72 deletions(-). I did
start work to break out some of the changes in a separate patch. I would
still like to hear whether the approach is ok, before I invest more time
in it. Release early, Release often is what I got told :).

> "Works for me" is inadequate.  Seemingly working, but subtly broken
> crypto is much worse than no crypto.  People need to be able to
> rely on it to protect their mail.

I do believe that the patch does the right thing. "Works for me" is a
necessary but not sufficient condition that the patch might be
acceptable. I never claimed that "Works for me" is a proof for the
correctness of the patch.
 
> I hope that doesn't sound too negative, I just think
> it's important to get that right.

So do I. That's why I post it for review :).

Sebastian




More information about the OfflineIMAP-project mailing list