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

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


The following commit(s) were added to refs/heads/quic-latest by this push:
     new 73d0ad3  Don't decrypt packets that have unsupported versions
73d0ad3 is described below

commit 73d0ad369de49adf1480c08255594f1a9cdaeb19
Author: Masakazu Kitajo <mas...@apache.org>
AuthorDate: Fri Feb 23 11:13:29 2018 +0900

    Don't decrypt packets that have unsupported versions
---
 iocore/net/QUICNetVConnection.cc | 34 +++++++++-------
 iocore/net/quic/QUICPacket.cc    | 87 ++++++++++++++++++++++------------------
 iocore/net/quic/QUICTypes.h      |  1 +
 3 files changed, 68 insertions(+), 54 deletions(-)

diff --git a/iocore/net/QUICNetVConnection.cc b/iocore/net/QUICNetVConnection.cc
index 6df7a20..e181afc 100644
--- a/iocore/net/QUICNetVConnection.cc
+++ b/iocore/net/QUICNetVConnection.cc
@@ -528,7 +528,7 @@ QUICNetVConnection::state_handshake(int event, Event *data)
         error = QUICErrorUPtr(new QUICNoError());
       } else if (result == QUICPacketCreationResult::FAILED) {
         error = QUICConnectionErrorUPtr(new 
QUICConnectionError(QUICTransErrorCode::TLS_FATAL_ALERT_GENERATED));
-      } else if (result == QUICPacketCreationResult::SUCCESS) {
+      } else if (result == QUICPacketCreationResult::SUCCESS || result == 
QUICPacketCreationResult::UNSUPPORTED) {
         error = this->_state_handshake_process_packet(std::move(packet));
       }
 
@@ -1175,26 +1175,30 @@ 
QUICNetVConnection::_dequeue_recv_packet(QUICPacketCreationResult &result)
     written += b->read_avail();
     b = b->next.get();
   }
+  udp_packet->free();
+
   quic_packet = this->_packet_factory.create(std::move(pkt), written, 
this->largest_received_packet_number(), result);
-  if (result == QUICPacketCreationResult::NOT_READY) {
+  switch (result) {
+  case QUICPacketCreationResult::NOT_READY:
     QUICConDebug("Not ready to decrypt the packet");
     // FIXME: unordered packet should be buffered and retried
-    udp_packet->free();
     if (this->_packet_recv_queue.size > 0) {
       result = QUICPacketCreationResult::IGNORED;
     }
-  } else if (result == QUICPacketCreationResult::IGNORED) {
-    QUICConDebug("Ignore to decrypt the packet");
-
-    udp_packet->free();
-  } else {
-    udp_packet->free();
-    if (result == QUICPacketCreationResult::SUCCESS) {
-      QUICConDebug("type=%s pkt_num=%" PRIu64 " size=%u", 
QUICDebugNames::packet_type(quic_packet->type()),
-                   quic_packet->packet_number(), quic_packet->size());
-    } else {
-      QUICConDebug("Failed to decrypt the packet");
-    }
+    break;
+  case QUICPacketCreationResult::IGNORED:
+    QUICConDebug("Ignored");
+    break;
+  case QUICPacketCreationResult::UNSUPPORTED:
+    QUICConDebug("Unsupported version");
+    break;
+  case QUICPacketCreationResult::SUCCESS:
+    QUICConDebug("type=%s pkt_num=%" PRIu64 " size=%u", 
QUICDebugNames::packet_type(quic_packet->type()),
+                 quic_packet->packet_number(), quic_packet->size());
+    break;
+  default:
+    QUICConDebug("Failed to decrypt the packet");
+    break;
   }
 
   return quic_packet;
diff --git a/iocore/net/quic/QUICPacket.cc b/iocore/net/quic/QUICPacket.cc
index bb505f6..a085d9b 100644
--- a/iocore/net/quic/QUICPacket.cc
+++ b/iocore/net/quic/QUICPacket.cc
@@ -663,29 +663,52 @@ QUICPacketFactory::create(ats_unique_buf buf, size_t len, 
QUICPacketNumber base_
 
   QUICPacketHeader *header = QUICPacketHeader::load(buf.release(), len, 
base_packet_number);
 
-  switch (header->type()) {
-  case QUICPacketType::VERSION_NEGOTIATION:
-  case QUICPacketType::STATELESS_RESET:
-    // These packets are unprotected. Just copy the payload
+  if (header->has_version() && 
!QUICTypeUtil::is_supported_version(header->version())) {
+    // We can't decrypt packets that have unknown versions
+    result = QUICPacketCreationResult::UNSUPPORTED;
     memcpy(plain_txt.get(), header->payload(), header->payload_size());
     plain_txt_len = header->payload_size();
-    result        = QUICPacketCreationResult::SUCCESS;
-    break;
-  case QUICPacketType::PROTECTED:
-    if (this->_hs_protocol->is_key_derived()) {
-      if (this->_hs_protocol->decrypt(plain_txt.get(), plain_txt_len, 
max_plain_txt_len, header->payload(), header->payload_size(),
-                                      header->packet_number(), header->buf(), 
header->size(), header->key_phase())) {
-        result = QUICPacketCreationResult::SUCCESS;
+  } else {
+    switch (header->type()) {
+    case QUICPacketType::VERSION_NEGOTIATION:
+    case QUICPacketType::STATELESS_RESET:
+      // These packets are unprotected. Just copy the payload
+      memcpy(plain_txt.get(), header->payload(), header->payload_size());
+      plain_txt_len = header->payload_size();
+      result        = QUICPacketCreationResult::SUCCESS;
+      break;
+    case QUICPacketType::PROTECTED:
+      if (this->_hs_protocol->is_key_derived()) {
+        if (this->_hs_protocol->decrypt(plain_txt.get(), plain_txt_len, 
max_plain_txt_len, header->payload(),
+                                        header->payload_size(), 
header->packet_number(), header->buf(), header->size(),
+                                        header->key_phase())) {
+          result = QUICPacketCreationResult::SUCCESS;
+        } else {
+          result = QUICPacketCreationResult::FAILED;
+        }
       } else {
-        result = QUICPacketCreationResult::FAILED;
+        result = QUICPacketCreationResult::NOT_READY;
       }
-    } else {
-      result = QUICPacketCreationResult::NOT_READY;
-    }
-    break;
-  case QUICPacketType::INITIAL:
-    if (!this->_hs_protocol->is_key_derived()) {
-      if (QUICTypeUtil::is_supported_version(header->version())) {
+      break;
+    case QUICPacketType::INITIAL:
+      if (!this->_hs_protocol->is_key_derived()) {
+        if (QUICTypeUtil::is_supported_version(header->version())) {
+          if (this->_hs_protocol->decrypt(plain_txt.get(), plain_txt_len, 
max_plain_txt_len, header->payload(),
+                                          header->payload_size(), 
header->packet_number(), header->buf(), header->size(),
+                                          QUICKeyPhase::CLEARTEXT)) {
+            result = QUICPacketCreationResult::SUCCESS;
+          } else {
+            result = QUICPacketCreationResult::FAILED;
+          }
+        } else {
+          result = QUICPacketCreationResult::SUCCESS;
+        }
+      } else {
+        result = QUICPacketCreationResult::IGNORED;
+      }
+      break;
+    case QUICPacketType::HANDSHAKE:
+      if (!this->_hs_protocol->is_key_derived()) {
         if (this->_hs_protocol->decrypt(plain_txt.get(), plain_txt_len, 
max_plain_txt_len, header->payload(),
                                         header->payload_size(), 
header->packet_number(), header->buf(), header->size(),
                                         QUICKeyPhase::CLEARTEXT)) {
@@ -694,31 +717,17 @@ QUICPacketFactory::create(ats_unique_buf buf, size_t len, 
QUICPacketNumber base_
           result = QUICPacketCreationResult::FAILED;
         }
       } else {
-        result = QUICPacketCreationResult::SUCCESS;
-      }
-    } else {
-      result = QUICPacketCreationResult::IGNORED;
-    }
-    break;
-  case QUICPacketType::HANDSHAKE:
-    if (!this->_hs_protocol->is_key_derived()) {
-      if (this->_hs_protocol->decrypt(plain_txt.get(), plain_txt_len, 
max_plain_txt_len, header->payload(), header->payload_size(),
-                                      header->packet_number(), header->buf(), 
header->size(), QUICKeyPhase::CLEARTEXT)) {
-        result = QUICPacketCreationResult::SUCCESS;
-      } else {
-        result = QUICPacketCreationResult::FAILED;
+        result = QUICPacketCreationResult::IGNORED;
       }
-    } else {
-      result = QUICPacketCreationResult::IGNORED;
+      break;
+    default:
+      result = QUICPacketCreationResult::FAILED;
+      break;
     }
-    break;
-  default:
-    result = QUICPacketCreationResult::FAILED;
-    break;
   }
 
   QUICPacket *packet = nullptr;
-  if (result == QUICPacketCreationResult::SUCCESS) {
+  if (result == QUICPacketCreationResult::SUCCESS || result == 
QUICPacketCreationResult::UNSUPPORTED) {
     packet = quicPacketAllocator.alloc();
     new (packet) QUICPacket(header, std::move(plain_txt), plain_txt_len);
   }
diff --git a/iocore/net/quic/QUICTypes.h b/iocore/net/quic/QUICTypes.h
index 8ab5248..5f01a08 100644
--- a/iocore/net/quic/QUICTypes.h
+++ b/iocore/net/quic/QUICTypes.h
@@ -121,6 +121,7 @@ enum class QUICPacketCreationResult {
   FAILED,
   NOT_READY,
   IGNORED,
+  UNSUPPORTED,
 };
 
 enum class QUICErrorClass {

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

Reply via email to