Hey Maxim, On Wed, Sep 28, 2016 at 07:40:13PM +0300, Maxim Dounin wrote: > Hello! > > On Wed, Sep 28, 2016 at 05:28:48PM +0100, Alessandro Ghedini wrote: > > > On Wed, Sep 28, 2016 at 07:00:00PM +0300, Maxim Dounin wrote: > > > Hello! > > > > > > On Wed, Sep 28, 2016 at 03:37:48PM +0100, Alessandro Ghedini wrote: > > > > > > > On Wed, Sep 28, 2016 at 05:19:02PM +0300, Maxim Dounin wrote: > > > > > Hello! > > > > > > > > > > On Wed, Sep 28, 2016 at 03:10:46PM +0100, Alessandro Ghedini wrote: > > > > > > > > > > > Hello, > > > > > > > > > > > > I don't now what your current plans for supporting BoringSSL are, > > > > > > but its > > > > > > API has been fairly stable for a while and this is the only change > > > > > > required > > > > > > to make NGINX build with it again (the other issue with error > > > > > > definitions was > > > > > > fixed in BoringSSL itself). > > > > > > > > > > > > I don't think BoringSSL is going to change the API back, so NGINX > > > > > > migh want > > > > > > to fix this if support for BoringSSL is desired (again, don't know > > > > > > your > > > > > > opinion on this). > > > > > > > > > > > > Please have a look and let me know what you think. > > > > > > > > > > Quoting > > > > > http://mailman.nginx.org/pipermail/nginx-devel/2016-August/008680.html: > > > > > > > > > > : Ok, this looks like the real reason for the patch. This looks > > > > > : like an API change in BoringSSL, and should be threated > > > > > : accordingly. > > > > > > > > > > : Given the number of various API changes BoringSSL introduces here > > > > > : and there - we probably don't want to follow, at least till some > > > > > : version is actually released. > > > > > > > > Ok, thanks, I missed that. TBH I don't think the BoringSSL team intends > > > > to > > > > release "proper" versions like OpenSSL does, so what you propose to > > > > wait for > > > > might not actually ever happen. > > > > > > Sure, and I'm fine with it. > > > > > > > I understand your concern of wanting to target a fixed release, but as I > > > > mentioned (and Piotr as well) BoringSSL's API seems to have been fairly > > > > stable > > > > for a while (except for fixes like the one for the problem mentioned in > > > > the > > > > patch you linked, which was worked around in BoringSSL itself), and > > > > AFAIK there > > > > aren't other similar compatibility problems left except for this build > > > > warning > > > > (but maybe Piotr could prove me wrong on that), so it might make sense > > > > to start > > > > looking at supporting BoringSSL again. > > > > > > Last time I looked into BoringSSL code due to ticket #993 several > > > months ago (https://trac.nginx.org/nginx/ticket/993), and my > > > impression wasn't that positive. > > > > Ah, BoringSSL actually supports the new SSL_CTX_set1_curves_list() API that > > NGINX already uses, but it doesn't seem to define SSL_CTRL_SET_CURVES_LIST > > (yet) so the other API is picked. I'll make a patch for BoringSSL to fix > > this. > > I'm afraid you are wrong here, > > $ grep -r SSL_CTX_set1_curves_list boringssl/ | wc -l > 0 > > Instead, BoringSSL introduced its own API to work with curves.
BoringSSL now provides SSL_CTX_set1_curves_list() as well: https://github.com/google/boringssl/commit/5fd1807d95f75895ae99e336ac21b422f3cc6bd3 Both me and Piotr verified that it works with NGINX. David Benjamin also suggested to modify my NGINX patch to always cast to (const char *) the argument of SSL_set_tlsext_host_name(), even when using OpenSSL since it uses macros and force-cast the argument back to (char *): https://github.com/openssl/openssl/blob/master/include/openssl/ssl.h#L1231 This way there would be no need for the #ifdef, which makes the whole thing nicer. I'll send an updated patch in a bit so you can pick the one you like the most if you decide to add BoringSSL compatibility. Cheers _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
