nghttpx: Drop connection before TLS finish if h2 requirement is not fulfilled

src/shrpx_client_handler.cc

@@ -452,108 +452,96 @@ void ClientHandler::reset_upstream_write_timeout(ev_tstamp t) {
 int ClientHandler::validate_next_proto() {
   const unsigned char *next_proto = nullptr;
   unsigned int next_proto_len;
-  int rv;
 
   // First set callback for catch all cases
   on_read_ = &ClientHandler::upstream_read;
 
   SSL_get0_next_proto_negotiated(conn_.tls.ssl, &next_proto, &next_proto_len);
-  for (int i = 0; i < 2; ++i) {
-    if (next_proto) {
-      if (LOG_ENABLED(INFO)) {
-        std::string proto(next_proto, next_proto + next_proto_len);
-        CLOG(INFO, this) << "The negotiated next protocol: " << proto;
-      }
-      if (!ssl::in_proto_list(get_config()->npn_list, next_proto,
-                              next_proto_len)) {
-        break;
-      }
-      if (util::check_h2_is_selected(next_proto, next_proto_len)) {
+#if OPENSSL_VERSION_NUMBER >= 0x10002000L
+  if (next_proto == nullptr) {
+    SSL_get0_alpn_selected(conn_.tls.ssl, &next_proto, &next_proto_len);
+  }
+#endif // OPENSSL_VERSION_NUMBER >= 0x10002000L
 
-        on_read_ = &ClientHandler::upstream_http2_connhd_read;
+  if (next_proto == nullptr) {
+    if (LOG_ENABLED(INFO)) {
+      CLOG(INFO, this) << "No protocol negotiated. Fallback to HTTP/1.1";
+    }
+
+    upstream_ = make_unique<HttpsUpstream>(this);
+    alpn_ = "http/1.1";
+
+    // At this point, input buffer is already filled with some bytes.
+    // The read callback is not called until new data come. So consume
+    // input buffer here.
+    if (on_read() != 0) {
+      return -1;
+    }
 
-        auto http2_upstream = make_unique<Http2Upstream>(this);
+    return 0;
+  }
 
-        if (!nghttp2::ssl::check_http2_requirement(conn_.tls.ssl)) {
-          if (LOG_ENABLED(INFO)) {
-            LOG(INFO) << "TLSv1.2 was not negotiated. "
-                      << "HTTP/2 must not be negotiated.";
-          }
+  if (LOG_ENABLED(INFO)) {
+    std::string proto(next_proto, next_proto + next_proto_len);
+    CLOG(INFO, this) << "The negotiated next protocol: " << proto;
+  }
 
-          rv = http2_upstream->terminate_session(NGHTTP2_INADEQUATE_SECURITY);
+  if (!ssl::in_proto_list(get_config()->npn_list, next_proto, next_proto_len)) {
+    if (LOG_ENABLED(INFO)) {
+      CLOG(INFO, this) << "The negotiated protocol is not supported";
+    }
+    return -1;
+  }
 
-          if (rv != 0) {
-            return -1;
-          }
-        }
+  if (util::check_h2_is_selected(next_proto, next_proto_len)) {
+    on_read_ = &ClientHandler::upstream_http2_connhd_read;
 
-        upstream_ = std::move(http2_upstream);
-        alpn_.assign(next_proto, next_proto + next_proto_len);
+    auto http2_upstream = make_unique<Http2Upstream>(this);
 
-        // At this point, input buffer is already filled with some
-        // bytes.  The read callback is not called until new data
-        // come. So consume input buffer here.
-        if (on_read() != 0) {
-          return -1;
-        }
+    upstream_ = std::move(http2_upstream);
+    alpn_.assign(next_proto, next_proto + next_proto_len);
+
+    // At this point, input buffer is already filled with some bytes.
+    // The read callback is not called until new data come. So consume
+    // input buffer here.
+    if (on_read() != 0) {
+      return -1;
+    }
+
+    return 0;
+  }
 
-        return 0;
-      } else {
 #ifdef HAVE_SPDYLAY
-        uint16_t version = spdylay_npn_get_version(next_proto, next_proto_len);
-        if (version) {
-          upstream_ = make_unique<SpdyUpstream>(version, this);
-
-          switch (version) {
-          case SPDYLAY_PROTO_SPDY2:
-            alpn_ = "spdy/2";
-            break;
-          case SPDYLAY_PROTO_SPDY3:
-            alpn_ = "spdy/3";
-            break;
-          case SPDYLAY_PROTO_SPDY3_1:
-            alpn_ = "spdy/3.1";
-            break;
-          default:
-            alpn_ = "spdy/unknown";
-          }
-
-          // At this point, input buffer is already filled with some
-          // bytes.  The read callback is not called until new data
-          // come. So consume input buffer here.
-          if (on_read() != 0) {
-            return -1;
-          }
-
-          return 0;
-        }
-#endif // HAVE_SPDYLAY
-        if (next_proto_len == 8 && memcmp("http/1.1", next_proto, 8) == 0) {
-          upstream_ = make_unique<HttpsUpstream>(this);
-          alpn_ = "http/1.1";
-
-          // At this point, input buffer is already filled with some
-          // bytes.  The read callback is not called until new data
-          // come. So consume input buffer here.
-          if (on_read() != 0) {
-            return -1;
-          }
-
-          return 0;
-        }
-      }
+  auto spdy_version = spdylay_npn_get_version(next_proto, next_proto_len);
+  if (spdy_version) {
+    upstream_ = make_unique<SpdyUpstream>(spdy_version, this);
+
+    switch (spdy_version) {
+    case SPDYLAY_PROTO_SPDY2:
+      alpn_ = "spdy/2";
+      break;
+    case SPDYLAY_PROTO_SPDY3:
+      alpn_ = "spdy/3";
       break;
+    case SPDYLAY_PROTO_SPDY3_1:
+      alpn_ = "spdy/3.1";
+      break;
+    default:
+      alpn_ = "spdy/unknown";
     }
-#if OPENSSL_VERSION_NUMBER >= 0x10002000L
-    SSL_get0_alpn_selected(conn_.tls.ssl, &next_proto, &next_proto_len);
-#else  // OPENSSL_VERSION_NUMBER < 0x10002000L
-    break;
-#endif // OPENSSL_VERSION_NUMBER < 0x10002000L
-  }
-  if (!next_proto) {
-    if (LOG_ENABLED(INFO)) {
-      CLOG(INFO, this) << "No protocol negotiated. Fallback to HTTP/1.1";
+
+    // At this point, input buffer is already filled with some bytes.
+    // The read callback is not called until new data come. So consume
+    // input buffer here.
+    if (on_read() != 0) {
+      return -1;
     }
+
+    return 0;
+  }
+#endif // HAVE_SPDYLAY
+
+  if (next_proto_len == 8 && memcmp("http/1.1", next_proto, 8) == 0) {
     upstream_ = make_unique<HttpsUpstream>(this);
     alpn_ = "http/1.1";
 

src/shrpx_connection.cc

@@ -35,6 +35,7 @@
 #include "shrpx_ssl.h"
 #include "shrpx_memcached_request.h"
 #include "memchunk.h"
+#include "util.h"
 
 using namespace nghttp2;
 
@@ -145,20 +146,7 @@ int shrpx_bio_write(BIO *b, const char *buf, int len) {
   if (conn->tls.initial_handshake_done) {
     // After handshake finished, send |buf| of length |len| to the
     // socket directly.
-    if (wbuf.rleft()) {
-      std::array<struct iovec, 4> iov;
-      auto iovcnt = wbuf.riovec(iov.data(), iov.size());
-      auto nwrite = conn->writev_clear(iov.data(), iovcnt);
-      if (nwrite < 0) {
-        return -1;
-      }
-
-      wbuf.drain(nwrite);
-      if (wbuf.rleft()) {
-        BIO_set_retry_write(b);
-        return -1;
-      }
-    }
+    assert(wbuf.rleft() == 0);
     auto nwrite = conn->write_clear(buf, len);
     if (nwrite < 0) {
       return -1;
@@ -300,6 +288,10 @@ int Connection::tls_handshake() {
     }
   }
 
+  if (tls.initial_handshake_done) {
+    return write_tls_pending_handshake();
+  }
+
   switch (tls.handshake_state) {
   case TLS_CONN_WAIT_FOR_SESSION_CACHE:
     return SHRPX_ERR_INPROGRESS;
@@ -365,7 +357,10 @@ int Connection::tls_handshake() {
     return SHRPX_ERR_INPROGRESS;
   }
 
-  if (tls.wbuf.rleft()) {
+  // Don't send handshake data if handshake was completed in OpenSSL
+  // routine.  We have to check HTTP/2 requirement if HTTP/2 was
+  // negotiated before sending finished message to the peer.
+  if (rv != 1 && tls.wbuf.rleft()) {
     // First write indicates that resumption stuff has done.
     if (tls.handshake_state != TLS_CONN_WRITE_STARTED) {
       tls.handshake_state = TLS_CONN_WRITE_STARTED;
@@ -401,8 +396,42 @@ int Connection::tls_handshake() {
     return SHRPX_ERR_INPROGRESS;
   }
 
+  // Handshake was done
+
+  rv = check_http2_requirement();
+  if (rv != 0) {
+    return -1;
+  }
+
+  // Just in case
+  tls.rbuf.disable_peek(true);
+
   tls.initial_handshake_done = true;
 
+  return write_tls_pending_handshake();
+}
+
+int Connection::write_tls_pending_handshake() {
+  // Send handshake data left in the buffer
+  while (tls.wbuf.rleft()) {
+    std::array<struct iovec, 4> iov;
+    auto iovcnt = tls.wbuf.riovec(iov.data(), iov.size());
+    auto nwrite = writev_clear(iov.data(), iovcnt);
+    if (nwrite < 0) {
+      if (LOG_ENABLED(INFO)) {
+        LOG(INFO) << "tls: handshake write error";
+      }
+      return -1;
+    }
+    if (nwrite == 0) {
+      wlimit.startw();
+      ev_timer_again(loop, &wt);
+
+      return SHRPX_ERR_INPROGRESS;
+    }
+    tls.wbuf.drain(nwrite);
+  }
+
   // We have to start read watcher, since later stage of code expects
   // this.
   rlimit.startw();
@@ -422,6 +451,31 @@ int Connection::tls_handshake() {
   return 0;
 }
 
+int Connection::check_http2_requirement() {
+  const unsigned char *next_proto = nullptr;
+  unsigned int next_proto_len;
+
+  SSL_get0_next_proto_negotiated(tls.ssl, &next_proto, &next_proto_len);
+#if OPENSSL_VERSION_NUMBER >= 0x10002000L
+  if (next_proto == nullptr) {
+    SSL_get0_alpn_selected(tls.ssl, &next_proto, &next_proto_len);
+  }
+#endif // OPENSSL_VERSION_NUMBER >= 0x10002000L
+  if (next_proto == nullptr ||
+      !util::check_h2_is_selected(next_proto, next_proto_len)) {
+    return 0;
+  }
+  if (!nghttp2::ssl::check_http2_requirement(tls.ssl)) {
+    if (LOG_ENABLED(INFO)) {
+      LOG(INFO) << "TLSv1.2 and/or black listed cipher suite was negotiated. "
+                   "HTTP/2 must not be used.";
+    }
+    return -1;
+  }
+
+  return 0;
+}
+
 namespace {
 const size_t SHRPX_SMALL_WRITE_LIMIT = 1300;
 const size_t SHRPX_WARMUP_THRESHOLD = 1 << 20;

src/shrpx_connection.h

@@ -84,6 +84,9 @@ struct Connection {
   void prepare_server_handshake();
 
   int tls_handshake();
+  int write_tls_pending_handshake();
+
+  int check_http2_requirement();
 
   // All write_* and writev_clear functions return number of bytes
   // written.  If nothing cannot be written (e.g., there is no