Hello! On Mon, Sep 08, 2014 at 10:19:17AM -0700, Quanah Gibson-Mount wrote:
> --On Friday, August 22, 2014 5:13 PM -0500 Kunal Pariani > <[email protected]> wrote: > > > > > > >Any comments on this yet ? > > Any nginx developers who could comment on this? Some obvious problems with the patch: - it's corrupted by author's mail client, and hence can't be applied/tested; - there are various style violations, like C++-style comments; - it introduces yet another "SSL without certificate verification" case, which is believed to be bad (similar thing was recently resolved by introducing proxy_ssl_verify in the http proxy module); Some more comments below. [...] > >+ // don't support SSLv2 anymore > > > >+ if (ngx_ssl_create(pcf->ssl, NGX_SSL_SSLv3|NGX_SSL_TLSv1, NULL) > > > >+ != NGX_OK) { It is incorrect to support SSLv3 and TLSv1 only. By default NGX_SSL_TLSv1_1 and NGX_SSL_TLSv1_2 should be allowed, too. It's also may be a good idea to make this configurable like in http proxy module. Also, 2 style issues here: "//" comment and incorrectly placed "{". [...] > >+ if (ngx_ssl_create_connection(pcf->ssl, c, > > > >+ NGX_SSL_BUFFER|NGX_SSL_CLIENT) > > > >+ != NGX_OK) > > > >+ { The NGX_SSL_BUFFER is incorrect here. It won't currently make any difference as the code only uses c->recv() / c->send(), but nevertheless mail protocols doesn't assume buffering, and the mail module doesn't use NGX_SSL_BUFFER in ngx_ssl_create_connection() calls intentionally. -- Maxim Dounin http://nginx.org/ _______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
