On 16/07/17 17:24, Antonio Quartulli wrote: > (re-adding the ml: please keep it in the CC list) > > On 16/07/17 17:16, Szilárd Pfeiffer wrote: >> On 2017-07-16 10:47, Antonio Quartulli wrote: >>> Hi Szilárd, >>> >>> >>> On 16/07/17 16:22, Szilárd Pfeiffer wrote: >>>> --- >>>> doc/openvpn.8 | 4 ++++ >>>> src/openvpn/options.c | 17 +++++++++++++++++ >>>> src/openvpn/ssl_common.h | 7 ++++--- >>>> src/openvpn/ssl_openssl.c | 6 ++++++ >>>> 4 files changed, 31 insertions(+), 3 deletions(-) >>>> >>> how about adding some text in the commit message explaining what the >>> patch is doing and *why* it would be helpful? casual reader not into TLS >>> (like me) may miss the benefit of this changes. >> The option does what ssl_prefer_server_ciphers options does in NGINX, the >> SSLHonorCipherOrder option does in Apache, ... >> >> http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_prefer_server_ciphers >> https://httpd.apache.org/docs/current/mod/mod_ssl.html#sslhonorcipherorder >> >> The reason explained for example in the following blog post. >> >> https://tribut.de/blog/secure-your-services-using-sane-cipher-ordering/#what-cipher-ordering-is-all-about >>> > > why not putting some of this information in the commit message as well? > >>> And why making it a compile time option? >> What do you mean? It can be set from command line. >> > > all your code is surrounded by "#ifdef SSL_OP_CIPHER_SERVER_PREFERENCE", > hence I imagined you wanted your code to be included based on a compile > time option. However, I don't see where SSL_OP_CIPHER_SERVER_PREFERENCE > should be defined? Am I missing something or it was forgotten? >
Sorry, but I realized just now that this is an OpenSSL specific mechanism and that the define is coming from OpenSSL itself. This is not my field, so I'll leave the review to somebody else, but still, this should be explained in the commit message, Cheers, -- Antonio Quartulli
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 Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
