diff options
| -rw-r--r-- | ssl/s3_clnt.c | 11 | ||||
| -rw-r--r-- | ssl/s3_srvr.c | 24 | ||||
| -rw-r--r-- | ssl/test/runner/runner.go | 28 |
3 files changed, 47 insertions, 16 deletions
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index aeb2604..bade13b 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -974,6 +974,17 @@ int ssl3_get_server_hello(SSL *s) goto f_err; } s->s3->tmp.new_cipher=c; + + /* Most clients also require that the negotiated version match the + * session's version if resuming. However OpenSSL has historically not + * had the corresponding logic on the server, so this may not be + * compatible, depending on other factors. (Whether the ClientHello + * version is clamped to the session's version and whether the session + * cache is keyed on IP address.) + * + * TODO(davidben): See if we can still enforce this? Perhaps for the + * future TLS 1.3 and forward if this is fixed upstream. */ + /* Don't digest cached records if no sigalgs: we may need them for * client authentication. */ diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 5f861a7..d591e19 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c @@ -881,21 +881,27 @@ int ssl3_get_client_hello(SSL *s) } else { - i=ssl_get_prev_session(s, &early_ctx); - if (i == 1) - { /* previous session */ - s->hit=1; + i = ssl_get_prev_session(s, &early_ctx); + if (i == PENDING_SESSION) + { + ret = PENDING_SESSION; + goto err; } else if (i == -1) - goto err; - else if (i == PENDING_SESSION) { - ret = PENDING_SESSION; goto err; } - else /* i == 0 */ + + /* Only resume if the session's version matches the negotiated + * version: most clients do not accept a mismatch. */ + if (i == 1 && s->version == s->session->ssl_version) + { + s->hit = 1; + } + else { - if (!ssl_get_new_session(s,1)) + /* No session was found or it was unacceptable. */ + if (!ssl_get_new_session(s, 1)) goto err; } } diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index f8649d3..9172223 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -1745,13 +1745,6 @@ func addResumptionVersionTests() { } suffix := "-" + sessionVers.name + "-" + resumeVers.name - // TODO(davidben): Write equivalent tests for the server - // and clean up the server's logic. This requires being - // able to give the shim a different set of SSL_OP_NO_* - // flags between the initial connection and the - // resume. Perhaps resumption should be tested by - // serializing the SSL_SESSION and starting a second - // shim. testCases = append(testCases, testCase{ name: "Resume-Client" + suffix, resumeSession: true, @@ -1789,6 +1782,27 @@ func addResumptionVersionTests() { }, expectedResumeVersion: resumeVers.version, }) + + var flags []string + if sessionVers.version != resumeVers.version { + flags = append(flags, "-expect-session-miss") + } + testCases = append(testCases, testCase{ + testType: serverTest, + name: "Resume-Server" + suffix, + flags: flags, + resumeSession: true, + config: Config{ + MaxVersion: sessionVers.version, + CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA}, + }, + expectedVersion: sessionVers.version, + resumeConfig: &Config{ + MaxVersion: resumeVers.version, + CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA}, + }, + expectedResumeVersion: resumeVers.version, + }) } } } |