On 20/10/16 10:42, Antonio Quartulli wrote:
> On Wed, Oct 19, 2016 at 02:22:31PM +0800, Antonio Quartulli wrote:
>> Implement the functions needed by the crl-persist logic when openssl
>> is enabled. Such functions are used in the ssl_verify module.
>> Note that the CRL file is stored in an adhoc data structure and no
>> openssl specific object is used. The data structure being used is a
>> sorted array or serials that can later be looked up in log(N) with
>> a binary search, thus guaranteeing a fast lookup operation.
>> Such data structure may be changed in the future with an optimized
>> openssl specific object.
>> Tests have been performed by using a CRL file having size 143MB.
>> Original delay upon client connection was around 5-8 seconds.
>> With this patch the delay gets close to 0.
>> Signed-off-by: Antonio Quartulli <a...@unstable.cc>
> As discussed on IRC, it might be better to first change the CRL handling code 
> in
> the OpenSSL module to use the internal routines provided by the OpenSSL 
> library.
> (apparently a patch to implement this change is in the work on somebody else's
> desk)
> At that point my patch could be changed to re-use the same code instead of
> implementing my own optimized logic.
> Note: also OpenSSL uses a sorted array + bsearch for CRL handling, therefore
> the performance of OpenSSL vs my approach should be similar.
> Does anybody else have any opinion against this?

First of all, thank you very much for this patch set!  As I said on the
IRC, this patch-set is well done when it comes to overall style and
layout.   I have not dug into the details, but the overall quick look
gave a good impression.

This new feature sounds reasonable.  The improved OpenSSL CRL handling
will be most welcome.  So basing your work on top of that makes a lot of
sense, and hopefully it can simplify your patch as well.

But I also think that we should look more carefully at the whole CRL
handling as a hole unit.  To me the --crl-persist looks like a
reasonable feature, but I'm still hoping to see more responses to this
feature from others as well.  But if it turns out that we can implement
this feature and also improve the current CRL handling, that will
hopefully give even less code duplication and an even simpler patch to
when adding the --crl-persist feature.

On IRC we also briefly discussed that the mbedTLS CRL code oaths is not
too clever and efficient (like, no btree search, just a simple
for-loop).  I would encourage you to look if you can improve the mbedTLS
code too, and provide that patch to them.  I'm sure the mbedTLS guys
will appreciate that effort.  This will again indirectly benefit us, and
we need to have even less specialized code to make the mbedTLS
integration perform well.  So it's a win-win for all of us :)

And I'm happy to see more people looking into these parts of the OpenVPN
code as well!  So thank you!

kind regards,

David Sommerseth

Attachment: signature.asc
Description: OpenPGP digital signature

Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
Openvpn-devel mailing list

Reply via email to