From 1c1d2b5f0ac29e1be337e7bd0161d94add76a522 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Sat, 23 May 2020 10:21:27 +0900
Subject: [PATCH] Ignore VN if it contains version selected by client

---
 lib/ngtcp2_conn.c        | 39 ++++++++++++++++-----------
 tests/main.c             |  2 ++
 tests/ngtcp2_conn_test.c | 57 ++++++++++++++++++++++++++++++++++++++++
 tests/ngtcp2_conn_test.h |  1 +
 4 files changed, 84 insertions(+), 15 deletions(-)

diff --git a/lib/ngtcp2_conn.c b/lib/ngtcp2_conn.c
index 92a7c56b..1f10547c 100644
--- a/lib/ngtcp2_conn.c
+++ b/lib/ngtcp2_conn.c
@@ -3438,6 +3438,7 @@ static int conn_on_version_negotiation(ngtcp2_conn *conn,
   uint32_t *p;
   int rv = 0;
   size_t nsv;
+  size_t i;
 
   if (payloadlen % sizeof(uint32_t)) {
     return NGTCP2_ERR_INVALID_ARGUMENT;
@@ -3452,28 +3453,41 @@ static int conn_on_version_negotiation(ngtcp2_conn *conn,
     p = sv;
   }
 
-  /* TODO Just move to the terminal state for now in order not to send
-     CONNECTION_CLOSE frame. */
-  conn->state = NGTCP2_CS_DRAINING;
-
   nsv = ngtcp2_pkt_decode_version_negotiation(p, payload, payloadlen);
 
   ngtcp2_log_rx_vn(&conn->log, hd, p, nsv);
 
+  for (i = 0; i < nsv; ++i) {
+    if (sv[i] == conn->version) {
+      ngtcp2_log_info(&conn->log, NGTCP2_LOG_EVENT_PKT,
+                      "ignore Version Negotiation because it contains version "
+                      "selected by client");
+
+      rv = NGTCP2_ERR_INVALID_ARGUMENT;
+      goto fin;
+    }
+  }
+
   if (conn->callbacks.recv_version_negotiation) {
     rv = conn->callbacks.recv_version_negotiation(conn, hd, p, nsv,
                                                   conn->user_data);
+    if (rv != 0) {
+      rv = NGTCP2_ERR_CALLBACK_FAILURE;
+    }
   }
 
-  if (p != sv) {
-    ngtcp2_mem_free(conn->mem, p);
+  if (rv == 0) {
+    /* TODO Just move to the terminal state for now in order not to
+       send CONNECTION_CLOSE frame. */
+    conn->state = NGTCP2_CS_DRAINING;
   }
 
-  if (rv != 0) {
-    return NGTCP2_ERR_CALLBACK_FAILURE;
+fin:
+  if (p != sv) {
+    ngtcp2_mem_free(conn->mem, p);
   }
 
-  return 0;
+  return rv;
 }
 
 static uint64_t conn_tx_strmq_first_cycle(ngtcp2_conn *conn) {
@@ -4333,12 +4347,7 @@ static ngtcp2_ssize conn_recv_handshake_pkt(ngtcp2_conn *conn,
       return NGTCP2_ERR_DISCARD_PKT;
     }
 
-    /* TODO Do not change state here? */
-    rv = conn_verify_dcid(conn, NULL, &hd);
-    if (rv != 0) {
-      if (ngtcp2_err_is_fatal(rv)) {
-        return rv;
-      }
+    if (!ngtcp2_cid_eq(&conn->oscid, &hd.dcid)) {
       ngtcp2_log_info(&conn->log, NGTCP2_LOG_EVENT_PKT,
                       "packet was ignored because of mismatched DCID");
       return NGTCP2_ERR_DISCARD_PKT;
diff --git a/tests/main.c b/tests/main.c
index c2b6e836..1470a475 100644
--- a/tests/main.c
+++ b/tests/main.c
@@ -244,6 +244,8 @@ int main() {
                    test_ngtcp2_conn_recv_client_initial_token) ||
       !CU_add_test(pSuite, "conn_get_active_dcid",
                    test_ngtcp2_conn_get_active_dcid) ||
+      !CU_add_test(pSuite, "conn_recv_version_negotiation",
+                   test_ngtcp2_conn_recv_version_negotiation) ||
       !CU_add_test(pSuite, "pkt_write_connection_close",
                    test_ngtcp2_pkt_write_connection_close) ||
       !CU_add_test(pSuite, "map", test_ngtcp2_map) ||
diff --git a/tests/ngtcp2_conn_test.c b/tests/ngtcp2_conn_test.c
index da94fb0a..04388fd8 100644
--- a/tests/ngtcp2_conn_test.c
+++ b/tests/ngtcp2_conn_test.c
@@ -5025,6 +5025,63 @@ void test_ngtcp2_conn_get_active_dcid(void) {
   ngtcp2_conn_del(conn);
 }
 
+void test_ngtcp2_conn_recv_version_negotiation(void) {
+  ngtcp2_conn *conn;
+  const ngtcp2_cid *dcid;
+  ngtcp2_ssize spktlen;
+  uint8_t buf[1500];
+  uint32_t nsv[3];
+  int rv;
+  ngtcp2_tstamp t = 0;
+
+  setup_handshake_client(&conn);
+
+  spktlen = ngtcp2_conn_write_pkt(conn, NULL, buf, sizeof(buf), ++t);
+
+  CU_ASSERT(spktlen > 0);
+
+  dcid = ngtcp2_conn_get_dcid(conn);
+
+  nsv[0] = 0xffffffff;
+
+  spktlen = ngtcp2_pkt_write_version_negotiation(
+      buf, sizeof(buf), 0xfe, conn->oscid.data, conn->oscid.datalen, dcid->data,
+      dcid->datalen, nsv, 1);
+
+  CU_ASSERT(spktlen > 0);
+
+  rv = ngtcp2_conn_read_pkt(conn, &null_path, buf, (size_t)spktlen, ++t);
+
+  CU_ASSERT(NGTCP2_ERR_RECV_VERSION_NEGOTIATION == rv);
+
+  ngtcp2_conn_del(conn);
+
+  /* Ignore Version Negotiation if it contains version selected by
+     client */
+  setup_handshake_client(&conn);
+
+  spktlen = ngtcp2_conn_write_pkt(conn, NULL, buf, sizeof(buf), ++t);
+
+  CU_ASSERT(spktlen > 0);
+
+  dcid = ngtcp2_conn_get_dcid(conn);
+
+  nsv[0] = 0xfffffff0;
+  nsv[1] = conn->version;
+
+  spktlen = ngtcp2_pkt_write_version_negotiation(
+      buf, sizeof(buf), 0x50, conn->oscid.data, conn->oscid.datalen, dcid->data,
+      dcid->datalen, nsv, 2);
+
+  CU_ASSERT(spktlen > 0);
+
+  rv = ngtcp2_conn_read_pkt(conn, &null_path, buf, (size_t)spktlen, ++t);
+
+  CU_ASSERT(0 == rv);
+
+  ngtcp2_conn_del(conn);
+}
+
 void test_ngtcp2_pkt_write_connection_close(void) {
   ngtcp2_ssize spktlen;
   uint8_t buf[1200];
diff --git a/tests/ngtcp2_conn_test.h b/tests/ngtcp2_conn_test.h
index 10743a2b..1bd088f5 100644
--- a/tests/ngtcp2_conn_test.h
+++ b/tests/ngtcp2_conn_test.h
@@ -69,6 +69,7 @@ void test_ngtcp2_conn_handshake_loss(void);
 void test_ngtcp2_conn_recv_client_initial_retry(void);
 void test_ngtcp2_conn_recv_client_initial_token(void);
 void test_ngtcp2_conn_get_active_dcid(void);
+void test_ngtcp2_conn_recv_version_negotiation(void);
 void test_ngtcp2_pkt_write_connection_close(void);
 
 #endif /* NGTCP2_CONN_TEST_H */
-- 
GitLab