From 72bf3d26ebd182cc9bb22b846f875acbf534be67 Mon Sep 17 00:00:00 2001
From: Miroslav Lichvar <mlichvar@redhat.com>
Date: Thu, 23 Jul 2020 15:46:57 +0200
Subject: [PATCH] nts: fix error response to NTS-KE request

When the request has an unrecognized critical record before the
NEXT_PROTOCOL and AEAD_ALGORITHM records, respond with error 0
(unrecognized critical record) instead of 1 (bad request).

When the request has multiple NEXT_PROTOCOL or AEAD_ALGORITHM records,
respond with error 1 (bad request).
---
 nts_ke_server.c           | 29 ++++++++++++++++++++++++++---
 test/unit/nts_ke_server.c | 26 +++++++++++++++-----------
 2 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/nts_ke_server.c b/nts_ke_server.c
index daf7e59..1049da0 100644
--- a/nts_ke_server.c
+++ b/nts_ke_server.c
@@ -330,6 +330,15 @@ prepare_response(NKSN_Instance session, int error, int next_protocol, int aead_a
     datum = htons(error);
     if (!NKSN_AddRecord(session, 1, NKE_RECORD_ERROR, &datum, sizeof (datum)))
       return 0;
+  } else if (next_protocol < 0) {
+    if (!NKSN_AddRecord(session, 1, NKE_RECORD_NEXT_PROTOCOL, NULL, 0))
+      return 0;
+  } else if (aead_algorithm < 0) {
+    datum = htons(next_protocol);
+    if (!NKSN_AddRecord(session, 1, NKE_RECORD_NEXT_PROTOCOL, &datum, sizeof (datum)))
+      return 0;
+    if (!NKSN_AddRecord(session, 1, NKE_RECORD_AEAD_ALGORITHM, NULL, 0))
+      return 0;
   } else {
     datum = htons(next_protocol);
     if (!NKSN_AddRecord(session, 1, NKE_RECORD_NEXT_PROTOCOL, &datum, sizeof (datum)))
@@ -376,6 +385,8 @@ prepare_response(NKSN_Instance session, int error, int next_protocol, int aead_a
 static int
 process_request(NKSN_Instance session)
 {
+  int next_protocol_records = 0, aead_algorithm_records = 0;
+  int next_protocol_values = 0, aead_algorithm_values = 0;
   int next_protocol = -1, aead_algorithm = -1, error = -1;
   int i, critical, type, length;
   uint16_t data[NKE_MAX_RECORD_BODY_LENGTH / sizeof (uint16_t)];
@@ -383,7 +394,7 @@ process_request(NKSN_Instance session)
   assert(NKE_MAX_RECORD_BODY_LENGTH % sizeof (uint16_t) == 0);
   assert(sizeof (uint16_t) == 2);
 
-  while (error == -1) {
+  while (error < 0) {
     if (!NKSN_GetRecord(session, &critical, &type, &length, &data, sizeof (data)))
       break;
 
@@ -393,7 +404,11 @@ process_request(NKSN_Instance session)
           error = NKE_ERROR_BAD_REQUEST;
           break;
         }
+
+        next_protocol_records++;
+
         for (i = 0; i < MIN(length, sizeof (data)) / 2; i++) {
+          next_protocol_values++;
           if (ntohs(data[i]) == NKE_NEXT_PROTOCOL_NTPV4)
             next_protocol = NKE_NEXT_PROTOCOL_NTPV4;
         }
@@ -403,7 +418,11 @@ process_request(NKSN_Instance session)
           error = NKE_ERROR_BAD_REQUEST;
           break;
         }
+
+        aead_algorithm_records++;
+
         for (i = 0; i < MIN(length, sizeof (data)) / 2; i++) {
+          aead_algorithm_values++;
           if (ntohs(data[i]) == AEAD_AES_SIV_CMAC_256)
             aead_algorithm = AEAD_AES_SIV_CMAC_256;
         }
@@ -419,8 +438,12 @@ process_request(NKSN_Instance session)
     }
   }
 
-  if (aead_algorithm < 0 || next_protocol < 0)
-    error = NKE_ERROR_BAD_REQUEST;
+  if (error < 0) {
+    if (next_protocol_records != 1 || next_protocol_values < 1 ||
+        (next_protocol == NKE_NEXT_PROTOCOL_NTPV4 &&
+         (aead_algorithm_records != 1 || aead_algorithm_values < 1)))
+      error = NKE_ERROR_BAD_REQUEST;
+  }
 
   if (!prepare_response(session, error, next_protocol, aead_algorithm))
     return 0;
diff --git a/test/unit/nts_ke_server.c b/test/unit/nts_ke_server.c
index 33b6108..f4f03a1 100644
--- a/test/unit/nts_ke_server.c
+++ b/test/unit/nts_ke_server.c
@@ -50,7 +50,7 @@ prepare_request(NKSN_Instance session, int valid)
   if (valid)
     index = -1;
   else
-    index = random() % 7;
+    index = random() % 9;
   DEBUG_LOG("index=%d", index);
 
   NKSN_BeginMessage(session);
@@ -61,30 +61,34 @@ prepare_request(NKSN_Instance session, int valid)
 
   if (index != 0) {
     memset(data, NKE_NEXT_PROTOCOL_NTPV4 + 1, sizeof (data));
+    data[0] = htons(NKE_NEXT_PROTOCOL_NTPV4);
     if (index == 1)
-      data[0] = htons(NKE_NEXT_PROTOCOL_NTPV4 + random() % 10 + 1);
-    else
-      data[0] = htons(NKE_NEXT_PROTOCOL_NTPV4);
-    if (index == 2)
+      length = 0;
+    else if (index == 2)
       length = 3 + random() % 15 * 2;
     else
       length = 2 + random() % 16 * 2;
     TEST_CHECK(NKSN_AddRecord(session, 1, NKE_RECORD_NEXT_PROTOCOL, data, length));
   }
 
-  if (index != 3) {
-    if (index == 4)
-      data[0] = htons(AEAD_AES_SIV_CMAC_256 + random() % 10 + 1);
-    else
-      data[0] = htons(AEAD_AES_SIV_CMAC_256);
+  if (index == 3)
+    TEST_CHECK(NKSN_AddRecord(session, 1, NKE_RECORD_NEXT_PROTOCOL, data, length));
+
+  if (index != 4) {
+    data[0] = htons(AEAD_AES_SIV_CMAC_256);
     if (index == 5)
+      length = 0;
+    else if (index == 6)
       length = 3 + random() % 15 * 2;
     else
       length = 2 + random() % 16 * 2;
     TEST_CHECK(NKSN_AddRecord(session, 1, NKE_RECORD_AEAD_ALGORITHM, data, length));
   }
 
-  if (index == 6) {
+  if (index == 7)
+    TEST_CHECK(NKSN_AddRecord(session, 1, NKE_RECORD_AEAD_ALGORITHM, data, length));
+
+  if (index == 8) {
     length = random() % (sizeof (data) + 1);
     TEST_CHECK(NKSN_AddRecord(session, 1, 1000 + random() % 1000, data, length));
   }