reverting recent SSL-related patches

Nicolas Sebrecht nicolas.s-dev at laposte.net
Mon Jan 12 12:36:39 UTC 2015


On Mon, Jan 12, 2015 at 03:12:32AM +0300, Eygene Ryabinkin wrote:

> I have a fix for this in my "next" since today.  It basically inhibits
> usage of OS-default CA bundle if cert_fingerprint is configured.

Which is what I was thinking first but I realized it's wrong. Since
there are defaults, users might want/expect OS-default CA bundle to
apply even if they manually add a fingerprint. But it can't be both
cases and we have to make a choice.

> I am not against pruning these patches from "master", since the
> stability of a branch from which -RC and releases are carved is vital.

Honestly not that much for release candidates. It's acceptable to have
early -rc not running. That's the purpose of release candidates: find
the bugs.

I impose myslef more stability than usual because of the unicode
support I'm working on.

> But I don't see much point in removing them from "next" apart from
> potential problems of merge conflicts.

The work is already done and I already solved the merge conflicts.

>                                         But since we're in the release
> cycle, changes from "next" are really meant to be cherry-picked and
> applied to "master" with manual control, unless I am terribly missing
> the whole release engineering process.

You're right. All changes in "next" are meant to be merged in "master"
at release time.

> I also have 4 more patches to apply to the current "next", so with
> cert_fingerprint/sslcacertfile fix this will be five:

Yes, I didn't publish the reverting patches, yet. I was looking if
someone could give a good reason to not revert them or definetly fix the
issue.

>  - http://codelabs.ru/patches/offlineimap/2015-Properly-re-raise-exception-to-save-original-traceback.diff

Looks good from a quick review. Apply it.

>  - http://codelabs.ru/patches/offlineimap/2015-Fix-API-documentation-syntax.diff
>  - http://codelabs.ru/patches/offlineimap/2015-API-documentation-properly-auto-document-main-class.diff
>  - http://codelabs.ru/patches/offlineimap/2015-API-documentation-fix-typo.diff

Not going to review these ones: it's about documentation. Apply them.

>  - http://codelabs.ru/patches/offlineimap/2015-SSL-properly-configure-certificate-and-fingerprint-validation.diff
>
> It works for me both if repository has sslcacertfile and if only
> cert_fingerprint is configured and OS-default CA bundle is present.

Like discussed above, I'm not plainly satisfied. I have no strong
opinion but in the same time I couldn't find a fix I'm totally satisfied
with. Edd proposed me off-list to add yet another configuration option
and I'm not tottaly satisfied with this fix neither.

The added comments to the configuration file should make the
situation clear enough. So, you might want go with the cuurent fix.

Apply, if you think it's the best approach. Consider me out of the
course. ,-)

> Basically, first and last commits are most interesting ones
> and
>   2015-SSL-properly-configure-certificate-and-fingerprint-validation.diff
> won't apply cleanly without
>   2015-Properly-re-raise-exception-to-save-original-traceback.diff
> since it was the way the fix was developed: it is terribly hard
> to work with non-original tracebacks ;)

Yeah, this reminds me where we come from and how hard it was to fix bugs
without the vital tracebacks years ago.

> Edd, if you will be able to test this, it will be very good.

-- 
Nicolas Sebrecht



More information about the OfflineIMAP-project mailing list