You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2018/03/28 02:18:24 UTC

[trafficserver] 01/02: Refactoring QUICHandshake

This is an automated email from the ASF dual-hosted git repository.

masaori pushed a commit to branch quic-latest
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit ae3c68d463fa9f8e4985ab999580f00f8cc6bad2
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Thu Mar 22 11:20:09 2018 +0900

    Refactoring QUICHandshake
---
 iocore/net/P_QUICNetVConnection.h |   1 +
 iocore/net/QUICNetVConnection.cc  |  14 +++
 iocore/net/quic/QUICHandshake.cc  | 200 +++++---------------------------------
 iocore/net/quic/QUICHandshake.h   |  30 ++----
 4 files changed, 50 insertions(+), 195 deletions(-)

diff --git a/iocore/net/P_QUICNetVConnection.h b/iocore/net/P_QUICNetVConnection.h
index ba4a657..4714488 100644
--- a/iocore/net/P_QUICNetVConnection.h
+++ b/iocore/net/P_QUICNetVConnection.h
@@ -297,6 +297,7 @@ private:
   QUICErrorUPtr _state_handshake_process_initial_client_packet(QUICPacketUPtr packet);
   QUICErrorUPtr _state_handshake_process_retry_packet(QUICPacketUPtr packet);
   QUICErrorUPtr _state_handshake_process_client_cleartext_packet(QUICPacketUPtr packet);
+  QUICErrorUPtr _state_handshake_process_protected_packet(QUICPacketUPtr packet);
   QUICErrorUPtr _state_handshake_process_zero_rtt_protected_packet(QUICPacketUPtr packet);
   QUICErrorUPtr _state_connection_established_process_packet(QUICPacketUPtr packet);
   QUICErrorUPtr _state_common_receive_packet();
diff --git a/iocore/net/QUICNetVConnection.cc b/iocore/net/QUICNetVConnection.cc
index ae53aa4..34286fa 100644
--- a/iocore/net/QUICNetVConnection.cc
+++ b/iocore/net/QUICNetVConnection.cc
@@ -811,10 +811,16 @@ QUICNetVConnection::_state_handshake_process_packet(QUICPacketUPtr packet)
   case QUICPacketType::HANDSHAKE:
     error = this->_state_handshake_process_client_cleartext_packet(std::move(packet));
     break;
+  case QUICPacketType::PROTECTED:
+    error = this->_state_handshake_process_protected_packet(std::move(packet));
+    break;
   case QUICPacketType::ZERO_RTT_PROTECTED:
     error = this->_state_handshake_process_zero_rtt_protected_packet(std::move(packet));
     break;
+
   default:
+    QUICConDebug("Unknown packet type: %s(%" PRIu8 ")", QUICDebugNames::packet_type(packet->type()), packet->type());
+
     error = QUICErrorUPtr(new QUICConnectionError(QUICTransErrorCode::INTERNAL_ERROR));
     break;
   }
@@ -865,6 +871,12 @@ QUICNetVConnection::_state_handshake_process_client_cleartext_packet(QUICPacketU
 }
 
 QUICErrorUPtr
+QUICNetVConnection::_state_handshake_process_protected_packet(QUICPacketUPtr packet)
+{
+  return this->_recv_and_ack(std::move(packet));
+}
+
+QUICErrorUPtr
 QUICNetVConnection::_state_handshake_process_zero_rtt_protected_packet(QUICPacketUPtr packet)
 {
   this->_start_application();
@@ -922,6 +934,8 @@ QUICNetVConnection::_state_common_receive_packet()
       // FIXME Just ignore for now but it has to be acked (GitHub#2609)
       break;
     default:
+      QUICConDebug("Unknown packet type: %s(%" PRIu8 ")", QUICDebugNames::packet_type(p->type()), p->type());
+
       error = QUICErrorUPtr(new QUICConnectionError(QUICTransErrorCode::INTERNAL_ERROR));
       break;
     }
diff --git a/iocore/net/quic/QUICHandshake.cc b/iocore/net/quic/QUICHandshake.cc
index cc21f5a..aec0907 100644
--- a/iocore/net/quic/QUICHandshake.cc
+++ b/iocore/net/quic/QUICHandshake.cc
@@ -103,7 +103,11 @@ QUICHandshake::QUICHandshake(QUICConnection *qc, SSL_CTX *ssl_ctx, QUICStateless
   SSL_set_ex_data(this->_ssl, QUIC::ssl_quic_hs_index, this);
   this->_hs_protocol->initialize_key_materials(this->_client_qc->original_connection_id());
 
-  SET_HANDLER(&QUICHandshake::state_initial);
+  if (this->_netvc_context == NET_VCONNECTION_OUT) {
+    this->_initial = true;
+  }
+
+  SET_HANDLER(&QUICHandshake::state_handshake);
 }
 
 QUICHandshake::~QUICHandshake()
@@ -266,50 +270,7 @@ QUICHandshake::remote_transport_parameters()
 }
 
 int
-QUICHandshake::state_initial(int event, Event *data)
-{
-  QUICHSDebug("%s (%d)", get_vc_event_name(event), event);
-
-  QUICErrorUPtr error = QUICErrorUPtr(new QUICNoError());
-  switch (event) {
-  case QUIC_EVENT_HANDSHAKE_PACKET_WRITE_COMPLETE: {
-    QUICHSDebug("Enter state_key_exchange");
-    SET_HANDLER(&QUICHandshake::state_key_exchange);
-    break;
-  }
-  case VC_EVENT_READ_READY:
-  case VC_EVENT_READ_COMPLETE: {
-    if (this->_netvc_context == NET_VCONNECTION_IN) {
-      error = this->_process_client_hello();
-    }
-    break;
-  }
-  case VC_EVENT_WRITE_READY:
-  case VC_EVENT_WRITE_COMPLETE: {
-    if (this->_netvc_context == NET_VCONNECTION_OUT) {
-      error = this->_process_initial();
-    }
-    break;
-  }
-  default:
-    break;
-  }
-
-  if (error->cls != QUICErrorClass::NONE) {
-    QUICTransErrorCode code;
-    if (dynamic_cast<QUICConnectionError *>(error.get()) != nullptr) {
-      code = error->trans_error_code;
-    } else {
-      code = QUICTransErrorCode::PROTOCOL_VIOLATION;
-    }
-    this->_abort_handshake(code);
-  }
-
-  return EVENT_DONE;
-}
-
-int
-QUICHandshake::state_key_exchange(int event, Event *data)
+QUICHandshake::state_handshake(int event, Event *data)
 {
   QUICHSDebug("%s (%d)", get_vc_event_name(event), event);
 
@@ -322,44 +283,13 @@ QUICHandshake::state_key_exchange(int event, Event *data)
         this->_abort_handshake(QUICTransErrorCode::TLS_HANDSHAKE_FAILED);
       }
     }
-
     break;
   }
   case VC_EVENT_READ_READY:
-  case VC_EVENT_READ_COMPLETE: {
-    ink_assert(this->_netvc_context == NET_VCONNECTION_OUT);
-    error = this->_process_server_hello();
-    break;
-  }
-  default:
-    break;
-  }
-
-  if (error->cls != QUICErrorClass::NONE) {
-    QUICTransErrorCode code;
-    if (dynamic_cast<QUICConnectionError *>(error.get()) != nullptr) {
-      code = error->trans_error_code;
-    } else {
-      code = QUICTransErrorCode::PROTOCOL_VIOLATION;
-    }
-    this->_abort_handshake(code);
-  }
-
-  return EVENT_DONE;
-}
-
-int
-QUICHandshake::state_auth(int event, Event *data)
-{
-  QUICHSDebug("%s (%d)", get_vc_event_name(event), event);
-
-  QUICErrorUPtr error = QUICErrorUPtr(new QUICNoError());
-  switch (event) {
-  case VC_EVENT_READ_READY:
-  case VC_EVENT_READ_COMPLETE: {
-    ink_assert(this->_netvc_context == NET_VCONNECTION_IN);
-
-    error = this->_process_finished();
+  case VC_EVENT_READ_COMPLETE:
+  case VC_EVENT_WRITE_READY:
+  case VC_EVENT_WRITE_COMPLETE: {
+    error = this->_process_handshake_msg();
     break;
   }
   default:
@@ -376,13 +306,6 @@ QUICHandshake::state_auth(int event, Event *data)
     this->_abort_handshake(code);
   }
 
-  return EVENT_CONT;
-}
-
-int
-QUICHandshake::state_address_validation(int event, void *data)
-{
-  // TODO Address validation should be implemented for the 2nd implementation draft
   return EVENT_DONE;
 }
 
@@ -453,7 +376,7 @@ QUICHandshake::_load_local_client_transport_parameters(QUICVersion initial_versi
 }
 
 int
-QUICHandshake::_do_handshake(bool initial)
+QUICHandshake::_do_handshake(size_t &out_len)
 {
   // TODO: pass stream_io
   QUICStreamIO *stream_io = this->_find_stream_io(STREAM_ID_FOR_HANDSHAKE);
@@ -461,7 +384,9 @@ QUICHandshake::_do_handshake(bool initial)
   uint8_t in[UDP_MAXIMUM_PAYLOAD_SIZE] = {0};
   int64_t in_len                       = 0;
 
-  if (!initial) {
+  if (this->_initial) {
+    this->_initial = false;
+  } else {
     // Complete message should fit in a packet and be able to read
     in_len = stream_io->read_avail();
     stream_io->read(in, in_len);
@@ -474,7 +399,6 @@ QUICHandshake::_do_handshake(bool initial)
   }
 
   uint8_t out[MAX_HANDSHAKE_MSG_LEN] = {0};
-  size_t out_len                     = 0;
   int result                         = this->_hs_protocol->handshake(out, out_len, MAX_HANDSHAKE_MSG_LEN, in, in_len);
 
   if (out_len > 0) {
@@ -495,104 +419,32 @@ QUICHandshake::_do_handshake(bool initial)
 }
 
 QUICErrorUPtr
-QUICHandshake::_process_initial()
+QUICHandshake::_process_handshake_msg()
 {
   QUICStreamIO *stream_io = this->_find_stream_io(STREAM_ID_FOR_HANDSHAKE);
-  int result              = _do_handshake(true);
-  QUICErrorUPtr error     = QUICErrorUPtr(new QUICNoError());
-
-  switch (result) {
-  case SSL_ERROR_WANT_READ: {
-    stream_io->write_reenable();
-    stream_io->read_reenable();
-
-    break;
-  }
-  default:
-    error = QUICErrorUPtr(new QUICConnectionError(QUICTransErrorCode::TLS_HANDSHAKE_FAILED));
-  }
-
-  return error;
-}
-
-QUICErrorUPtr
-QUICHandshake::_process_client_hello()
-{
-  QUICStreamIO *stream_io = this->_find_stream_io(STREAM_ID_FOR_HANDSHAKE);
-  int result              = _do_handshake();
+  size_t out_len          = 0;
+  int result              = this->_do_handshake(out_len);
   QUICErrorUPtr error     = QUICErrorUPtr(new QUICNoError());
 
   switch (result) {
   case SSL_ERROR_NONE:
-  case SSL_ERROR_WANT_READ: {
-    if (this->_hs_protocol->msg_type() == QUICHandshakeMsgType::RETRY) {
-      // TODO: Send HRR on Retry Packet directly
-      stream_io->write_reenable();
-    } else {
-      QUICHSDebug("Enter state_auth");
-      SET_HANDLER(&QUICHandshake::state_auth);
-
-      stream_io->write_reenable();
-      stream_io->read_reenable();
+    if (this->_hs_protocol->is_handshake_finished()) {
+      int res = this->_complete_handshake();
+      if (!res) {
+        error = QUICErrorUPtr(new QUICConnectionError(QUICTransErrorCode::TLS_HANDSHAKE_FAILED));
+      }
     }
-
-    break;
-  }
-  default:
-    error = QUICErrorUPtr(new QUICConnectionError(QUICTransErrorCode::TLS_HANDSHAKE_FAILED));
-  }
-
-  return error;
-}
-
-QUICErrorUPtr
-QUICHandshake::_process_server_hello()
-{
-  QUICStreamIO *stream_io = this->_find_stream_io(STREAM_ID_FOR_HANDSHAKE);
-  int result              = _do_handshake();
-  QUICErrorUPtr error     = QUICErrorUPtr(new QUICNoError());
-
-  switch (result) {
-  case SSL_ERROR_NONE: {
-    stream_io->write_reenable();
-    break;
-  }
+  // Fall-through
   case SSL_ERROR_WANT_READ: {
-    // FIXME: check if write_reenable should be called or not
-    stream_io->write_reenable();
-    stream_io->read_reenable();
-    break;
-  }
-  default:
-    error = QUICErrorUPtr(new QUICConnectionError(QUICTransErrorCode::TLS_HANDSHAKE_FAILED));
-  }
-
-  return error;
-}
-
-QUICErrorUPtr
-QUICHandshake::_process_finished()
-{
-  QUICStreamIO *stream_io = this->_find_stream_io(STREAM_ID_FOR_HANDSHAKE);
-  int result              = _do_handshake();
-  QUICErrorUPtr error     = QUICErrorUPtr(new QUICNoError());
-
-  switch (result) {
-  case SSL_ERROR_NONE: {
-    int res = this->_complete_handshake();
-    if (res) {
+    if (out_len > 0) {
       stream_io->write_reenable();
-    } else {
-      this->_abort_handshake(QUICTransErrorCode::TLS_HANDSHAKE_FAILED);
     }
-
-    break;
-  }
-  case SSL_ERROR_WANT_READ: {
     stream_io->read_reenable();
+
     break;
   }
   default:
+    QUICHSDebug("Handshake failed: %d", result);
     error = QUICErrorUPtr(new QUICConnectionError(QUICTransErrorCode::TLS_HANDSHAKE_FAILED));
   }
 
diff --git a/iocore/net/quic/QUICHandshake.h b/iocore/net/quic/QUICHandshake.h
index 6ddae16..2005041 100644
--- a/iocore/net/quic/QUICHandshake.h
+++ b/iocore/net/quic/QUICHandshake.h
@@ -32,19 +32,12 @@
  * @brief Do handshake as a QUIC application
  * @detail
  *
- * # client
- * state_initial()
+ * state_handshake()
+ *  |
  *  v
- * state_key_exchange()
- *  v
- * state_complete()
- *
- * # server
- * state_initial()
- *  v
- * state_auth()
- *  v
- * state_complete()
+ * state_complete() on success
+ *  or
+ * state_closed() on error
  */
 class QUICVersionNegotiator;
 class SSLNextProtocolSet;
@@ -64,10 +57,7 @@ public:
   QUICErrorUPtr start(const QUICPacket *initial_packet, QUICPacketFactory *packet_factory);
 
   // States
-  int state_initial(int event, Event *data);
-  int state_key_exchange(int event, Event *data);
-  int state_address_validation(int event, void *data);
-  int state_auth(int event, Event *data);
+  int state_handshake(int event, Event *data);
   int state_complete(int event, void *data);
   int state_closed(int event, void *data);
 
@@ -99,17 +89,15 @@ private:
   QUICVersionNegotiator *_version_negotiator = nullptr;
   NetVConnectionContext_t _netvc_context     = NET_VCONNECTION_UNSET;
   QUICStatelessResetToken _reset_token;
+  bool _initial         = false;
   bool _stateless_retry = false;
 
   void _load_local_server_transport_parameters(QUICVersion negotiated_version);
   void _load_local_client_transport_parameters(QUICVersion initial_version);
 
-  int _do_handshake(bool initial = false);
+  int _do_handshake(size_t &out_len);
 
-  QUICErrorUPtr _process_initial();
-  QUICErrorUPtr _process_client_hello();
-  QUICErrorUPtr _process_server_hello();
-  QUICErrorUPtr _process_finished();
+  QUICErrorUPtr _process_handshake_msg();
 
   int _complete_handshake();
   void _abort_handshake(QUICTransErrorCode code);

-- 
To stop receiving notification emails like this one, please contact
masaori@apache.org.