From c44b1df45978ae57a2b573b0654759408e38baa0 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sun, 23 Nov 2014 12:11:01 -0500 Subject: [PATCH] Add test for renego client_version quirk. In upstream's f4e1169341ad1217e670387db5b0c12d680f95f4, the client_version was made constant across renegotiations, even if the server negotiated a lower version. NSS has the same quirk, reportedly for SChannel: https://code.google.com/p/chromium/codesearch#chromium/src/net/third_party/nss/ssl/ssl3con.c&sq=package:chromium&l=5103 Add a test to ensure we do not regress this. Change-Id: I214e062463c203b86a9bab00f8503442e1bf74fe Reviewed-on: https://boringssl-review.googlesource.com/2405 Reviewed-by: Adam Langley --- ssl/s3_clnt.c | 6 ------ ssl/test/runner/common.go | 5 +++++ ssl/test/runner/conn.go | 2 ++ ssl/test/runner/handshake_server.go | 7 +++++++ ssl/test/runner/runner.go | 10 ++++++++++ 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 8d60d8fc..cc034c60 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -717,14 +717,8 @@ int ssl3_send_client_hello(SSL *s) * client_version in client hello and not resetting it to * the negotiated version. */ -#if 0 - *(p++)=s->version>>8; - *(p++)=s->version&0xff; - s->client_version=s->version; -#else *(p++)=s->client_version>>8; *(p++)=s->client_version&0xff; -#endif /* Random stuff */ memcpy(p,s->s3->client_random,SSL3_RANDOM_SIZE); diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index 02ee7e2e..628c208b 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go @@ -577,6 +577,11 @@ type ProtocolBugs struct { // CertificateRequest message. None the less, the configured set will // still be enforced. NoSignatureAndHashes bool + + // RequireSameRenegoClientVersion, if true, causes the server + // to require that all ClientHellos match in offered version + // across a renego. + RequireSameRenegoClientVersion bool } func (c *Config) serverInit() { diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go index 94d7434d..177e4580 100644 --- a/ssl/test/runner/conn.go +++ b/ssl/test/runner/conn.go @@ -58,6 +58,8 @@ type Conn struct { srtpProtectionProfile uint16 + clientVersion uint16 + // input/output in, out halfConn // in.Mutex < out.Mutex rawInput *block // raw input, right off the wire diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index ec79b79f..4bdede15 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go @@ -162,6 +162,13 @@ func (hs *serverHandshakeState) readClientHello() (isResume bool, err error) { hs.clientHello = newClientHello } + if config.Bugs.RequireSameRenegoClientVersion && c.clientVersion != 0 { + if c.clientVersion != hs.clientHello.vers { + return false, fmt.Errorf("tls: client offered different version on renego") + } + } + c.clientVersion = hs.clientHello.vers + c.vers, ok = config.mutualVersion(hs.clientHello.vers) if !ok { c.sendAlert(alertProtocolVersion) diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 79f8ee06..73563882 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -2109,6 +2109,16 @@ func addRenegotiationTests() { }, renegotiateCiphers: []uint16{TLS_RSA_WITH_RC4_128_SHA}, }) + testCases = append(testCases, testCase{ + name: "Renegotiate-SameClientVersion", + renegotiate: true, + config: Config{ + MaxVersion: VersionTLS10, + Bugs: ProtocolBugs{ + RequireSameRenegoClientVersion: true, + }, + }, + }) } func addDTLSReplayTests() {