On 28/04/2014 15:19, Steffan Karger wrote:
Hi,

On 27-04-14 22:10, Steffan Karger wrote:
On 27-04-14 19:53, Gert Doering wrote:
On Mon, Apr 21, 2014 at 01:10:04AM -0600, James Yonan wrote: The
attached patch is what I intend to commit to release/2.3 *only*,
not to master - as agreed at the IRC meeting.  "Please ACK" :-)

Sorry, but NAK.

On a more constructive note: attached a new proposal for this patch.

The OpenSSL bits look fine

On a closer look, the wrapping "if (tls_version_min > TLS_VER_UNSPEC)"
in tls_ctx_set_options() seems redundant, because TLS_VER_UNSPEC <
TLS_VER_1_0 < TLS_VER_1_1 < TLS_VER_1_2 and the ifs are checking for
"tls_version_min > TLS_VER_1_x". I've removed these changes in the
attached patch proposal.

the PolarSSL bits
would also allow for SSL_MINOR_VERSION_0, which is SSLv3 and thus a
reduction in security (and actually increases the handshake complexity).

To elaborate a bit: The naming is a bit confusing, but
SSL_MAJOR_VERSION_3 + SSL_MINOR_VERSION_O means SSLv3, ... +
SSL_MINOR_VERSION_1 means TLSv1.0, ... + SSL_MINOR_VERSION_2 means
TLSv1.1, etc. If none are given (what would happen in the previous
version of the patch), PolarSSL defaults the minimum version to SSLv3
and maximum to TLSv1.2. The attached patch thus removes the wrapping "if
(tls_version_min > TLS_VER_UNSPEC)" and sets the default to TLSv1.0 to
TLSv1.2 again. That is equal to the behaviour prior to the TLS
versioning patch.

-Steffan

This is fine.  I can ACK this.

Thanks,
James


Reply via email to