The record version in the client Hello will be set to the lowest supported protocol

There should have been no harm in keeping it SSL 3.0 but
unfortunately in draft-thomson-sslv3-diediedie-00
it has been marked as MUST NOT do that. That will be fixed in a later
revision but since then there are servers not accepting SSL 3.0
as a valid record version (note that this is about the record
version, which describes the format of the packet, nothing to
do with the negotiated version).

5 files changed, 48 insertions, 33 deletions

diff --git a/lib/algorithms.h b/lib/algorithms.h
index 93ef670d45..0eb53ee02b 100644
--- a/lib/algorithms.h
+++ b/lib/algorithms.h
@@ -33,7 +33,7 @@
 
 /* Functions for version handling. */
 const version_entry_st *version_to_entry(gnutls_protocol_t c);
-gnutls_protocol_t _gnutls_version_lowest(gnutls_session_t session);
+const version_entry_st *_gnutls_version_lowest(gnutls_session_t session);
 gnutls_protocol_t _gnutls_version_max(gnutls_session_t session);
 int _gnutls_version_priority(gnutls_session_t session,
 			     gnutls_protocol_t version);
diff --git a/lib/algorithms/protocols.c b/lib/algorithms/protocols.c
index cb8034ee06..f62de49438 100644
--- a/lib/algorithms/protocols.c
+++ b/lib/algorithms/protocols.c
@@ -27,13 +27,13 @@
 
 /* TLS Versions */
 static const version_entry_st sup_versions[] = {
-	{"SSL3.0", GNUTLS_SSL3, 3, 0, GNUTLS_STREAM, 1, 0, 0, 0, 0},
-	{"TLS1.0", GNUTLS_TLS1, 3, 1, GNUTLS_STREAM, 1, 0, 1, 0, 0},
-	{"TLS1.1", GNUTLS_TLS1_1, 3, 2, GNUTLS_STREAM, 1, 1, 1, 0, 0},
-	{"TLS1.2", GNUTLS_TLS1_2, 3, 3, GNUTLS_STREAM, 1, 1, 1, 1, 1},
-	{"DTLS0.9", GNUTLS_DTLS0_9, 1, 0, GNUTLS_DGRAM, 1, 1, 1, 0, 0},	/* Cisco AnyConnect (based on about OpenSSL 0.9.8e) */
-	{"DTLS1.0", GNUTLS_DTLS1_0, 254, 255, GNUTLS_DGRAM, 1, 1, 1, 0, 0},	/* 1.1 over datagram */
-	{"DTLS1.2", GNUTLS_DTLS1_2, 254, 253, GNUTLS_DGRAM, 1, 1, 1, 1, 1},	/* 1.2 over datagram */
+	{"SSL3.0", GNUTLS_SSL3, 0, 3, 0, GNUTLS_STREAM, 1, 0, 0, 0, 0},
+	{"TLS1.0", GNUTLS_TLS1, 1, 3, 1, GNUTLS_STREAM, 1, 0, 1, 0, 0},
+	{"TLS1.1", GNUTLS_TLS1_1, 2, 3, 2, GNUTLS_STREAM, 1, 1, 1, 0, 0},
+	{"TLS1.2", GNUTLS_TLS1_2, 3, 3, 3, GNUTLS_STREAM, 1, 1, 1, 1, 1},
+	{"DTLS0.9", GNUTLS_DTLS0_9, 200, 1, 0, GNUTLS_DGRAM, 1, 1, 1, 0, 0},	/* Cisco AnyConnect (based on about OpenSSL 0.9.8e) */
+	{"DTLS1.0", GNUTLS_DTLS1_0, 201, 254, 255, GNUTLS_DGRAM, 1, 1, 1, 0, 0},	/* 1.1 over datagram */
+	{"DTLS1.2", GNUTLS_DTLS1_2, 202, 254, 253, GNUTLS_DGRAM, 1, 1, 1, 1, 1},	/* 1.2 over datagram */
 	{0, 0, 0, 0, 0}
 };
 
@@ -50,6 +50,16 @@ const version_entry_st *version_to_entry(gnutls_protocol_t version)
 	return NULL;
 }
 
+static int
+version_is_valid_for_session(gnutls_session_t session,
+		     const version_entry_st *v)
+{
+	if (v->supported && v->transport == session->internals.transport) {
+		return 1;
+	}
+	return 0;
+}
+
 /* Return the priority of the provided version number */
 int
 _gnutls_version_priority(gnutls_session_t session,
@@ -68,24 +78,28 @@ _gnutls_version_priority(gnutls_session_t session,
 
 /* Returns the lowest TLS version number in the priorities.
  */
-gnutls_protocol_t _gnutls_version_lowest(gnutls_session_t session)
+const version_entry_st *_gnutls_version_lowest(gnutls_session_t session)
 {
-	unsigned int i, min = 0xff;
+	unsigned int i;
 	gnutls_protocol_t cur_prot;
+	const version_entry_st *v, *min_v = NULL;
 
-	for (i=0;i< session->internals.priorities.protocol.algorithms;i++) {
+	for (i=0;i < session->internals.priorities.protocol.algorithms;i++) {
 		cur_prot =
 		    session->internals.priorities.protocol.priority[i];
-
-		if (cur_prot < min
-		    && _gnutls_version_is_supported(session, cur_prot))
-			min = cur_prot;
+		v = version_to_entry(cur_prot);
+
+		if (v != NULL) {
+			if (min_v == NULL) {
+				min_v = v;
+			} else if (v->age < min_v->age
+			    && version_is_valid_for_session(session, v)) {
+				min_v = v;
+			}
+		}
 	}
 
-	if (min == 0xff)
-		return GNUTLS_VERSION_UNKNOWN;	/* unknown version */
-
-	return min;
+	return min_v;
 }
 
 /* Returns the maximum version in the priorities 
@@ -211,3 +225,4 @@ _gnutls_version_is_supported(gnutls_session_t session,
 	else
 		return 1;
 }
+
diff --git a/lib/gnutls_handshake.c b/lib/gnutls_handshake.c
index 5930941589..b0f7e28b52 100644
--- a/lib/gnutls_handshake.c
+++ b/lib/gnutls_handshake.c
@@ -1978,23 +1978,22 @@ static int send_client_hello(gnutls_session_t session, int again)
 		if (_gnutls_set_current_version(session, hver->id) < 0)
 			return gnutls_assert_val(GNUTLS_E_UNSUPPORTED_VERSION_PACKET);
 
-		if (session->internals.priorities.ssl3_record_version != 0) {
+		if (session->internals.priorities.min_record_version != 0) {
 			/* Advertize the SSL 3.0 record packet version in
 			 * record packets during the handshake.
 			 * That is to avoid confusing implementations
 			 * that do not support TLS 1.2 and don't know
 			 * how 3,3 version of record packets look like.
 			 */
-			if (!IS_DTLS(session))
-				_gnutls_record_set_default_version(session,
-								   3, 0);
-			else if (hver->id == GNUTLS_DTLS0_9)
-				_gnutls_record_set_default_version(session,
-								   1, 0);
-			else
+			const version_entry_st *v = _gnutls_version_lowest(session);
+
+			if (v == NULL) {
+				gnutls_assert();
+				return GNUTLS_E_INTERNAL_ERROR;
+			} else {
 				_gnutls_record_set_default_version(session,
-								   254,
-								   255);
+								   v->major, v->minor);
+			}
 		}
 
 		/* In order to know when this session was initiated.
diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h
index d9804caf8f..d3adbf0d6e 100644
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -486,6 +486,7 @@ typedef struct mac_entry_st {
 typedef struct {
 	const char *name;
 	gnutls_protocol_t id;	/* gnutls internal version number */
+	unsigned age;		/* internal ordering by protocol age */
 	uint8_t major;		/* defined by the protocol */
 	uint8_t minor;		/* defined by the protocol */
 	transport_t transport;	/* Type of transport, stream or datagram */
@@ -658,7 +659,7 @@ struct gnutls_priority_st {
 	unsigned int max_empty_records;
 	unsigned int dumbfw;
 	safe_renegotiation_t sr;
-	bool ssl3_record_version;
+	bool min_record_version;
 	bool server_precedence;
 	bool allow_wrong_pms;
 	/* Whether stateless compression will be used */
diff --git a/lib/gnutls_priority.c b/lib/gnutls_priority.c
index 92af12fd1a..3f498d9a0a 100644
--- a/lib/gnutls_priority.c
+++ b/lib/gnutls_priority.c
@@ -860,11 +860,11 @@ static void disable_safe_renegotiation(gnutls_priority_t c)
 }
 static void enable_latest_record_version(gnutls_priority_t c)
 {
-	c->ssl3_record_version = 0;
+	c->min_record_version = 0;
 }
 static void enable_ssl3_record_version(gnutls_priority_t c)
 {
-	c->ssl3_record_version = 1;
+	c->min_record_version = 1;
 }
 static void enable_verify_allow_rsa_md5(gnutls_priority_t c)
 {
@@ -1125,7 +1125,7 @@ gnutls_priority_init(gnutls_priority_t * priority_cache,
 	 * when we make it the default.
 	 */
 	(*priority_cache)->sr = SR_PARTIAL;
-	(*priority_cache)->ssl3_record_version = 1;
+	(*priority_cache)->min_record_version = 1;
 
 	(*priority_cache)->max_empty_records = DEFAULT_MAX_EMPTY_RECORDS;