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:23 UTC

[trafficserver] branch quic-latest updated (6183b8e -> 4cb3a5b)

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

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


    from 6183b8e  Re-randomize Connection ID only if TS received RETRY packet
     new ae3c68d  Refactoring QUICHandshake
     new 4cb3a5b  Ignore INITIAL packet on state_connection_established

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 iocore/net/P_QUICNetVConnection.h |   1 +
 iocore/net/QUICNetVConnection.cc  |  53 ++++++----
 iocore/net/quic/QUICHandshake.cc  | 200 +++++---------------------------------
 iocore/net/quic/QUICHandshake.h   |  30 ++----
 4 files changed, 71 insertions(+), 213 deletions(-)

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

[trafficserver] 01/02: Refactoring QUICHandshake

Posted by ma...@apache.org.
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.

[trafficserver] 02/02: Ignore INITIAL packet on state_connection_established

Posted by ma...@apache.org.
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 4cb3a5bf56446d8b2dec7a0890af1cba09e3fc5d
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Wed Mar 28 11:13:58 2018 +0900

    Ignore INITIAL packet on state_connection_established
    
    Client might retransmit INITLA packet even if connection is established.
    This made crash on a assert in connection migration logic. Just ignore it for now.
---
 iocore/net/QUICNetVConnection.cc | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/iocore/net/QUICNetVConnection.cc b/iocore/net/QUICNetVConnection.cc
index 34286fa..2709454 100644
--- a/iocore/net/QUICNetVConnection.cc
+++ b/iocore/net/QUICNetVConnection.cc
@@ -907,31 +907,34 @@ QUICNetVConnection::_state_common_receive_packet()
       continue;
     }
 
-    // Check connection migration
-    if (this->_handshake_handler->is_completed() && p->connection_id() != this->_quic_connection_id) {
-      for (unsigned int i = 0; i < countof(this->_alt_quic_connection_ids); ++i) {
-        AltConnectionInfo &info = this->_alt_quic_connection_ids[i];
-        if (info.id == p->connection_id()) {
-          // Migrate connection
-          // TODO Address Validation
-          // TODO Adjust expected packet number with a gap computed based on info.seq_num
-          // TODO Unregister the old connection id (Should we wait for a while?)
-          this->_quic_connection_id = info.id;
-          this->_reset_token        = info.token;
-          this->_update_alt_connection_ids(i);
-          break;
-        }
-      }
-      ink_assert(p->connection_id() == this->_quic_connection_id);
-    }
-
     // Process the packet
     switch (p->type()) {
     case QUICPacketType::PROTECTED:
+      // Check connection migration
+      if (this->_handshake_handler->is_completed() && p->connection_id() != this->_quic_connection_id) {
+        for (unsigned int i = 0; i < countof(this->_alt_quic_connection_ids); ++i) {
+          AltConnectionInfo &info = this->_alt_quic_connection_ids[i];
+          if (info.id == p->connection_id()) {
+            // Migrate connection
+            // TODO Address Validation
+            // TODO Adjust expected packet number with a gap computed based on info.seq_num
+            // TODO Unregister the old connection id (Should we wait for a while?)
+            this->_quic_connection_id = info.id;
+            this->_reset_token        = info.token;
+            this->_update_alt_connection_ids(i);
+            break;
+          }
+        }
+        ink_assert(p->connection_id() == this->_quic_connection_id);
+      }
+
       error = this->_state_connection_established_process_packet(std::move(p));
       break;
+    case QUICPacketType::INITIAL:
     case QUICPacketType::HANDSHAKE:
       // FIXME Just ignore for now but it has to be acked (GitHub#2609)
+      QUICConDebug("Ignore %s packet", QUICDebugNames::packet_type(p->type()));
+
       break;
     default:
       QUICConDebug("Unknown packet type: %s(%" PRIu8 ")", QUICDebugNames::packet_type(p->type()), p->type());

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