On 10 January 2014 06:41, Florian Zumbiehl via RT <r...@openssl.org> wrote:
> Hi,
>
> > The fix is to change || in the above code to &&. Then, the
> > command-line parameters are used to set the certificate path, and if
> > that fails, the defaults are used instead. This then gives the
>
> while the behaviour with your patch is a lot saner than without it, I would
> argue that it's still broken, as it exhibits fail-open behaviour:
> SSL_CTX_load_verify_locations() probably can fail for reasons other than
> !(CAfile||CApath), and it's unlikely that the user meant "this CA, or any
> other if loading this one fails for whatever reason".
>
So in that case it should try only the user's option if the user gave a
-CApath or -CAfile, and otherwise the default option?
> (Arguably, SSL_CTX_load_verify_locations() is actually broken in that it
> returns failure for an empty set of CAs,
To cope with that, we could have a new function (to be used in place of the
current
if ((!SSL_CTX_load_verify_locations(ctx,CAfile,CApath)) ||
(!SSL_CTX_set_default_verify_paths(ctx)))
), which goes something like:
int SSL_CTX_load_verify_locations_or_default(SSL_CTX *ctx, const char
*CAfile, const char *CApath)
{
if (CAfile || CApath)
return SSL_CTX_load_verify_locations(ctx, CAfile, CApath);
return SSL_CTX_set_default_verify_paths(ctx);
}
and the above code can then be replaced by:
if (!SSL_CTX_load_verify_locations_or_default(ctx, CAfile, CApath))
> as it's logically perfectly
> consistent to authenticate against a deny-all policy.)
Indeed.
The suggestion above has the advantage that it does not require
SSL_CTX_load_verify_locations to be changed (as its behavior of failing
when CApath and CAfile are both NULL is documented). However, if it were
changed, then the code above would still work.
The correct behavior is, as I hope I've made clear, outside my competence
to decide, but I'm quite happy to work up an acceptable patch if guided as
to what exactly it should implement.
--
http://rrt.sc3d.org
