reverting recent SSL-related patches

[Eygene Ryabinkin]

Mon, Jan 12, 2015 at 01:36:39PM +0100, Nicolas Sebrecht wrote:
> 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.

My thinking is the following:

 1. if there is just a default (or non-default) CA bundle and IMAP
   server's certificate can't be verified with it, an error will occur
   (OI will error out with X.509 verification error); this is the
   default and expected behaviour;

 2. prior to the introduction of default CA bundle, users with just
   cert_fingerprint enabled will get no CA root verification (obviously);

 3. making cert_fingerprint to inhibit the implicit usage of default
   bundle retains the standard behaviour [2] and explicit usage (either
   full pathname or OS-DEFAULT meta-value) retains the expected behaviour
   [1] too;

 4. this undermines the notion of a default CA bundle and that's the
   problem you're pointing our and it is the real one.  I had tried
   to do my best in documenting this in offlineimap.conf, but I realize
   that it won't be enough in many cases.

However, I had tried to build the model for the class of users that
will be fooled by this behaviour (taking for granted that they don't
read the documentation).  Basically, cert_fingerprint is a
quick'n'dirty way to solve the problem of CA roots and the only
affected users are those who are using this option.  Moreover, to be
fooled about default behaviour they should know what is default CA
bundle, because the only broken expectation is that the bundle will be
still used to verify connection even when cert_fingerprint is used.
And these users will have no sslcacertfile set, so either

 - they had never tried to use real CA roots (and found out that
   default CA bundle errors out for them) and found relief in
   cert_fingerprint;

 - they come from pre-default bundle times, so they are fine with
   the fact that their server's certificate won't be verified;

 - they are fine with default bundle, but believe that cert_fingerprint
   will add extra protection layer.

Only the latter ones seem to be really fooled by OI behaviour I am
trying to introduce, expectation of other aren't harmed.  It is likely
that they will read the documentation, but I am still not relying
on this.


Theoretically, I can modify the patch to have the full processing
of both CA bundle and cert_fingerprint, falling back to bundle-less
verification that is supplemented with warning about problems with
implicitely-configured CA bundle.  This won't break any compatibility
with prior behaviour.

Or I may break the compatibility and introduce a way to disable default
CA bundle, but users will then be forced to modify their configuration.

Or there can be new option that enables default CA bundle, but that is
off by-default.

Not that I am trying to convince anyone that the current approach is
the best one, just thinking loudly in a hope that either I or anyone
else will be striked by the good idea about this.


The reason why I do care for default CA bundles is that it seems that
people and OS really care more for security in the X.509 world and
it is not something exotic to have distro with properly-configured
default CAs.  So, the more out-of-the-box it will be, the more
likelihood of spotting malicious actions there'll be.  And that's the
right thing.

> >  - 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.

Pushed these, thanks!

> >  - 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. ,-)

I'll then postpone the last patch to allow for some more thinking.
Feel free either to wait for some time or to remove the default CA
bundle commits -- I'll reintegrate them when/if the time will come.
-- 
rea