Bound plain-message ACK attempts to decrypted data

src/helpers/BaseChatMesh.cpp

@@ -377,7 +377,8 @@ void BaseChatMesh::onPeerDataRecv(mesh::Packet* packet, uint8_t type, int sender
       uint8_t ack_hash[6];    // calc truncated hash of the message timestamp + text + sender pub_key, to prove to sender that we got it
       mesh::Utils::sha256(ack_hash, 4, data, 5 + text_len, from.id.pub_key, PUB_KEY_SIZE);
       // NEW: append (potential) extended attempt byte (to make packethash unique)
-      ack_hash[4] = data[5 + text_len + 1];
+      const size_t attempt_offset = 5 + text_len + 1;
+      ack_hash[4] = attempt_offset < len ? data[attempt_offset] : 0;
       getRNG()->random(&ack_hash[5], 1);  // make 6th byte random
 
       if (packet->isRouteFlood()) {

test/fixtures/room_receive_admission/test.cpp

@@ -42,8 +42,13 @@ struct Packet {
   bool isRouteFlood() const { return flood; }
 };
 struct Utils {
-  static void sha256(uint8_t* out, size_t size, const uint8_t*, size_t,
-                     const uint8_t*, size_t) { memset(out, 0x55, size); }
+  inline static std::vector<uint8_t> hash_input, hash_key;
+  static void sha256(uint8_t* out, size_t size, const uint8_t* input, size_t input_length,
+                     const uint8_t* key, size_t key_length) {
+    hash_input.assign(input, input + input_length);
+    hash_key.assign(key, key + key_length);
+    memset(out, 0x55, size);
+  }
 };
 }
 struct ContactInfo {
@@ -58,7 +63,11 @@ struct Clock {
   uint32_t getCurrentTime() const { return now; }
   uint32_t getCurrentTimeUnique() { return now++; }
 };
-struct Rng { void random(uint8_t* out, size_t length) { memset(out, 0, length); } };
+struct Rng {
+  unsigned calls = 0;
+  size_t last_length = 0;
+  void random(uint8_t* out, size_t length) { ++calls; last_length = length; memset(out, 0, length); }
+};
 struct BaseChatMesh {
   ContactInfo contacts[MAX_CONTACTS];
   int matching_peer_indexes[1] = {0}, num_contacts = MAX_CONTACTS;
@@ -168,7 +177,69 @@ static unsigned effects(const MyMesh& m) {
   return m.ui.messages + m.ui.notifications + m.ui.overrides + m.serial.writes
       + m._notifications.remote + m._notifications.ordinary + m.active + unsigned(m.saved.size());
 }
-int main() {
+static void plainAckBounds() {
+  unsigned cases = 0;
+  for (bool flood : {false, true}) {
+    // Mesh::onRecvPacket decrypts whole AES blocks into an otherwise uncleared
+    // 184-byte local buffer. Seed its unused tail independently of the text.
+    // Exercise every representable text length, not only ordinary sender caps.
+    for (size_t text_length = 0; text_length <= 171; ++text_length) {
+      MyMesh m; mesh::Packet packet; packet.flood = flood;
+      uint8_t data[184], secret[32] = {};
+      memset(data, 0xa5, sizeof(data));
+      const size_t wire_length = 5 + text_length;
+      const size_t decrypted_length = (wire_length + 15) / 16 * 16;
+      memset(data, 0, decrypted_length);
+      memset(data + 5, 'X', text_length);
+      m.onPeerDataRecv(&packet, PAYLOAD_TYPE_TXT_MSG, 0, secret, data, decrypted_length);
+      assert(m.direct_acks == unsigned(!flood) && m.flood_acks == unsigned(flood));
+      assert(m.ack.size() == 6 && m.ack[5] == 0);
+      assert(m.ack[4] == 0 && "ACK attempt must come only from authenticated plaintext");
+      assert(std::equal(m.ack.begin(), m.ack.begin() + 4, "\x55\x55\x55\x55"));
+      assert(mesh::Utils::hash_input == std::vector<uint8_t>(data, data + wire_length));
+      assert(mesh::Utils::hash_key == std::vector<uint8_t>(m.contacts[0].id.pub_key,
+          m.contacts[0].id.pub_key + PUB_KEY_SIZE));
+      assert(m.rng.calls == 1 && m.rng.last_length == 1);
+      assert(data[decrypted_length] == 0 && data[decrypted_length + 1] == 0xa5);
+      assert(m.contacts[0].sync_since == 50 && m.contacts[0].lastmod == 1200);
+      ++cases;
+    }
+    // composeMsgPacket encodes extended attempts after the text terminator.
+    // Preserve the complete byte, including values outside its low two bits.
+    for (size_t text_length = 0; text_length <= 158; ++text_length) {
+      for (uint8_t attempt : {0, 4, 5, 127, 255}) {
+        MyMesh m; mesh::Packet packet; packet.flood = flood;
+        uint8_t data[184], secret[32] = {};
+        memset(data, 0xa5, sizeof(data));
+        const size_t wire_length = 5 + text_length + 2;
+        const size_t decrypted_length = (wire_length + 15) / 16 * 16;
+        memset(data, 0, decrypted_length);
+        data[4] = attempt & 3;
+        memset(data + 5, 'X', text_length);
+        data[5 + text_length + 1] = attempt;
+        m.onPeerDataRecv(&packet, PAYLOAD_TYPE_TXT_MSG, 0, secret, data, decrypted_length);
+        assert(m.direct_acks == unsigned(!flood) && m.flood_acks == unsigned(flood));
+        assert(m.ack.size() == 6 && m.ack[4] == attempt && m.ack[5] == 0);
+        assert(std::equal(m.ack.begin(), m.ack.begin() + 4, "\x55\x55\x55\x55"));
+        assert(mesh::Utils::hash_input == std::vector<uint8_t>(data, data + 5 + text_length));
+        assert(mesh::Utils::hash_key == std::vector<uint8_t>(m.contacts[0].id.pub_key,
+            m.contacts[0].id.pub_key + PUB_KEY_SIZE));
+        assert(m.rng.calls == 1 && m.rng.last_length == 1);
+        assert(data[decrypted_length] == 0 && data[decrypted_length + 1] == 0xa5);
+        ++cases;
+      }
+    }
+  }
+  assert(cases == 1934);
+  printf("PASS: production plain ACK bounds (%u cases, direct/flood, AES padding and complete attempt bytes)\n", cases);
+}
+int main(int argc, char** argv) {
+  if (argc == 2) {
+    assert(strcmp(argv[1], "ack-bounds") == 0);
+    plainAckBounds();
+    return 0;
+  }
+  assert(argc == 1);
   unsigned cases = 0;
   for (int version : {2, 3}) for (bool flood : {false, true}) for (bool connected : {false, true}) {
     MyMesh m; m.app_target_ver = version; m.serial.connected = connected; m.fill();

test/test_room_receive_admission.py

@@ -18,6 +18,15 @@ SANITIZERS = (["-fsanitize=address,undefined", "-fno-sanitize-recover=all",
 
 class RoomReceiveAdmissionTests(unittest.TestCase):
     def test_actual_receive_queue_retry_and_notifications(self):
+        self.run_fixture()
+
+    def test_plain_ack_attempt_field_bounds_and_valid_extensions(self):
+        self.run_fixture(ack_bounds=True)
+
+    def test_unbounded_plain_ack_attempt_negative_control(self):
+        self.run_fixture(ack_bounds=True, unbounded_attempt=True)
+
+    def run_fixture(self, *, ack_bounds=False, unbounded_attempt=False):
         compiler = shutil.which("g++") or shutil.which("clang++")
         self.assertIsNotNone(compiler, "a host C++17 compiler is required")
         base = (ROOT / "src/helpers/BaseChatMesh.cpp").read_text()
@@ -45,6 +54,12 @@ class RoomReceiveAdmissionTests(unittest.TestCase):
             ("notify-without-admission", receive, queue.replace(
                 "if (txt_type == TXT_TYPE_SIGNED_PLAIN && !queued) return false;", ""), False),
         )
+        if ack_bounds:
+            if unbounded_attempt:
+                bounded = "ack_hash[4] = attempt_offset < len ? data[attempt_offset] : 0;"
+                self.assertEqual(receive.count(bounded), 1)
+                receive = receive.replace(bounded, "ack_hash[4] = data[attempt_offset];", 1)
+            variants = (("plain-ack-bounds", receive, queue, not unbounded_attempt),)
         with tempfile.TemporaryDirectory(prefix="room-receive-admission-") as directory:
             work = Path(directory)
             (work / "frame.inc").write_text(extract_braced(header, "struct Frame") + ";\n", encoding="ascii")
@@ -58,13 +73,17 @@ class RoomReceiveAdmissionTests(unittest.TestCase):
                         "-Wno-unused-parameter", *SANITIZERS, f"-I{work}", f"-I{ROOT / 'src'}",
                         str(FIXTURE), "-o", str(binary)], capture_output=True, text=True, timeout=60)
                     self.assertEqual(built.returncode, 0, built.stdout + built.stderr)
-                    tested = subprocess.run([str(binary)], capture_output=True, text=True, timeout=20)
+                    command = [str(binary)] + (["ack-bounds"] if ack_bounds else [])
+                    tested = subprocess.run(command, capture_output=True, text=True, timeout=20)
                     if passes:
                         self.assertEqual(tested.returncode, 0, tested.stdout + tested.stderr)
-                        self.assertIn("PASS: production room receive admission", tested.stdout)
+                        self.assertIn("PASS: production plain ACK bounds" if ack_bounds else
+                                      "PASS: production room receive admission", tested.stdout)
                     else:
                         self.assertNotEqual(tested.returncode, 0, "negative control escaped the delivery checks")
                         self.assertIn("Assertion", tested.stderr)
+                        if unbounded_attempt:
+                            self.assertIn("ACK attempt must come only from authenticated plaintext", tested.stderr)
 
 
 if __name__ == "__main__":