nghttpx: Fix tls handshake bug

This fixes 2 things:
1. potential busy loop
2. disabling ticket is not working after resumption
This commit is contained in:
Tatsuhiro Tsujikawa 2015-08-09 18:33:49 +09:00
parent 73442ba5ba
commit ff44e211ed
5 changed files with 38 additions and 27 deletions

View File

@ -303,13 +303,21 @@ int Connection::tls_handshake() {
} }
tls.rb->write(nread); tls.rb->write(nread);
switch (tls.handshake_state) { // We have limited space for read buffer, so stop reading if it
case TLS_CONN_WAIT_FOR_SESSION_CACHE: // filled up.
if (tls.rb->wleft() == 0) { if (tls.rb->wleft() == 0) {
// Input buffer is full. Disable read until cache is returned
rlimit.stopw(); rlimit.stopw();
ev_timer_stop(loop, &rt); ev_timer_stop(loop, &rt);
} }
switch (tls.handshake_state) {
case TLS_CONN_WAIT_FOR_SESSION_CACHE:
if (nread > 0) {
if (LOG_ENABLED(INFO)) {
LOG(INFO) << "tls: client sent addtional data after client hello";
}
return -1;
}
return SHRPX_ERR_INPROGRESS; return SHRPX_ERR_INPROGRESS;
case TLS_CONN_GOT_SESSION_CACHE: { case TLS_CONN_GOT_SESSION_CACHE: {
// Use the same trick invented by @kazuho in h2o project // Use the same trick invented by @kazuho in h2o project
@ -317,12 +325,16 @@ int Connection::tls_handshake() {
tls.rb->pos = tls.rb->begin(); tls.rb->pos = tls.rb->begin();
auto ssl_ctx = SSL_get_SSL_CTX(tls.ssl); auto ssl_ctx = SSL_get_SSL_CTX(tls.ssl);
auto ssl_opts = SSL_get_options(tls.ssl);
SSL_free(tls.ssl); SSL_free(tls.ssl);
auto ssl = ssl::create_server_ssl(ssl_ctx, nullptr); auto ssl = ssl::create_ssl(ssl_ctx);
if (!ssl) { if (!ssl) {
return -1; return -1;
} }
if (ssl_opts & SSL_OP_NO_TICKET) {
SSL_set_options(ssl, SSL_OP_NO_TICKET);
}
set_ssl(ssl); set_ssl(ssl);
@ -360,6 +372,8 @@ int Connection::tls_handshake() {
} }
if (tls.wb->rleft()) { if (tls.wb->rleft()) {
// First write indicates that resumption stuff has done.
tls.handshake_state = TLS_CONN_WRITE_STARTED;
auto nwrite = write_clear(tls.wb->pos, tls.wb->rleft()); auto nwrite = write_clear(tls.wb->pos, tls.wb->rleft());
if (nwrite < 0) { if (nwrite < 0) {
if (LOG_ENABLED(INFO)) { if (LOG_ENABLED(INFO)) {
@ -373,6 +387,16 @@ int Connection::tls_handshake() {
if (tls.wb->rleft()) { if (tls.wb->rleft()) {
wlimit.startw(); wlimit.startw();
ev_timer_again(loop, &wt); ev_timer_again(loop, &wt);
} else {
tls.wb->reset();
}
if (tls.handshake_state == TLS_CONN_WRITE_STARTED && tls.rb->rleft() == 0) {
tls.rb->reset();
// We may have stopped reading
rlimit.startw();
ev_timer_again(loop, &rt);
} }
if (rv != 1) { if (rv != 1) {

View File

@ -46,6 +46,7 @@ enum {
TLS_CONN_WAIT_FOR_SESSION_CACHE, TLS_CONN_WAIT_FOR_SESSION_CACHE,
TLS_CONN_GOT_SESSION_CACHE, TLS_CONN_GOT_SESSION_CACHE,
TLS_CONN_CANCEL_SESSION_CACHE, TLS_CONN_CANCEL_SESSION_CACHE,
TLS_CONN_WRITE_STARTED,
}; };
struct TLSConnection { struct TLSConnection {

View File

@ -323,7 +323,7 @@ int Http2Session::initiate_connection() {
// We are establishing TLS connection. If conn_.tls.ssl, we may // We are establishing TLS connection. If conn_.tls.ssl, we may
// reuse the previous session. // reuse the previous session.
if (!conn_.tls.ssl) { if (!conn_.tls.ssl) {
auto ssl = ssl::create_client_ssl(ssl_ctx_); auto ssl = ssl::create_ssl(ssl_ctx_);
if (!ssl) { if (!ssl) {
return -1; return -1;
} }

View File

@ -695,7 +695,6 @@ SSL_CTX *create_ssl_client_context() {
return ssl_ctx; return ssl_ctx;
} }
namespace {
SSL *create_ssl(SSL_CTX *ssl_ctx) { SSL *create_ssl(SSL_CTX *ssl_ctx) {
auto ssl = SSL_new(ssl_ctx); auto ssl = SSL_new(ssl_ctx);
if (!ssl) { if (!ssl) {
@ -706,23 +705,6 @@ SSL *create_ssl(SSL_CTX *ssl_ctx) {
return ssl; return ssl;
} }
} // namespace
SSL *create_server_ssl(SSL_CTX *ssl_ctx, Worker *worker) {
auto ssl = create_ssl(ssl_ctx);
if (!ssl) {
return nullptr;
}
// Disable TLS session ticket if we don't have working ticket keys.
if (worker && !worker->get_ticket_keys()) {
SSL_set_options(ssl, SSL_OP_NO_TICKET);
}
return ssl;
}
SSL *create_client_ssl(SSL_CTX *ssl_ctx) { return create_ssl(ssl_ctx); }
ClientHandler *accept_connection(Worker *worker, int fd, sockaddr *addr, ClientHandler *accept_connection(Worker *worker, int fd, sockaddr *addr,
int addrlen) { int addrlen) {
@ -746,10 +728,15 @@ ClientHandler *accept_connection(Worker *worker, int fd, sockaddr *addr,
SSL *ssl = nullptr; SSL *ssl = nullptr;
auto ssl_ctx = worker->get_sv_ssl_ctx(); auto ssl_ctx = worker->get_sv_ssl_ctx();
if (ssl_ctx) { if (ssl_ctx) {
ssl = create_server_ssl(ssl_ctx, worker); ssl = create_ssl(ssl_ctx);
if (!ssl) { if (!ssl) {
return nullptr; return nullptr;
} }
// Disable TLS session ticket if we don't have working ticket
// keys.
if (!worker->get_ticket_keys()) {
SSL_set_options(ssl, SSL_OP_NO_TICKET);
}
} }
return new ClientHandler(worker, fd, ssl, host, service); return new ClientHandler(worker, fd, ssl, host, service);

View File

@ -172,8 +172,7 @@ SSL_CTX *setup_client_ssl_context();
// this function returns nullptr. // this function returns nullptr.
CertLookupTree *create_cert_lookup_tree(); CertLookupTree *create_cert_lookup_tree();
SSL *create_server_ssl(SSL_CTX *ssl_ctx, Worker *worker); SSL *create_ssl(SSL_CTX *ssl_ctx);
SSL *create_client_ssl(SSL_CTX *ssl_ctx);
} // namespace ssl } // namespace ssl