fix(p2p): make NullCrypto usable and stabilize handshake

GaspardKirira · GaspardKirira · commit fc3ef1d450c0 · 2026-01-31T13:17:54.000+03:00
diff --git a/include/vix/p2p/Crypto.hpp b/include/vix/p2p/Crypto.hpp
@@ -17,6 +17,8 @@
 #include <span>
 #include <vector>
 #include <string>
+#include <array>
+#include <random>
 
 namespace vix::p2p
 {
@@ -68,16 +70,56 @@ namespace vix::p2p
   class NullCrypto final : public Crypto
   {
   public:
-    KeyPair generate_keypair() override { return {}; }
+    KeyPair generate_keypair() override
+    {
+      KeyPair kp;
+      kp.public_key.resize(32);
+      kp.private_key.resize(32);
+
+      static thread_local std::mt19937_64 rng{std::random_device{}()};
+
+      auto fill = [&](std::vector<std::uint8_t> &v)
+      {
+        for (size_t i = 0; i < v.size(); ++i)
+          v[i] = static_cast<std::uint8_t>(rng() & 0xFF);
+      };
+
+      fill(kp.public_key);
+      fill(kp.private_key);
 
-    std::vector<std::uint8_t> sign(std::span<const std::uint8_t>, std::span<const std::uint8_t>) override
+      bool all0 = true;
+      for (auto b : kp.public_key)
+      {
+        if (b != 0)
+        {
+          all0 = false;
+          break;
+        }
+      }
+      if (all0)
+        kp.public_key[0] = 1;
+
+      return kp;
+    }
+
+    std::vector<std::uint8_t> sign(std::span<const std::uint8_t> data,
+                                   std::span<const std::uint8_t> private_key) override
     {
-      return {};
+      // DEV: signature = kdf_32(data || priv) (non secure)
+      std::vector<std::uint8_t> t;
+      t.reserve(data.size() + private_key.size());
+      t.insert(t.end(), data.begin(), data.end());
+      t.insert(t.end(), private_key.begin(), private_key.end());
+      return kdf_32(t);
     }
 
-    bool verify(std::span<const std::uint8_t>, std::span<const std::uint8_t>, std::span<const std::uint8_t>) override
+    bool verify(std::span<const std::uint8_t> data,
+                std::span<const std::uint8_t> signature,
+                std::span<const std::uint8_t> public_key) override
     {
-      return true;
+      (void)data;
+      (void)public_key;
+      return !signature.empty();
     }
 
     std::vector<std::uint8_t> encrypt(std::span<const std::uint8_t> plaintext) override
diff --git a/include/vix/p2p/Transport.hpp b/include/vix/p2p/Transport.hpp
@@ -42,6 +42,7 @@ namespace vix::p2p
     virtual void close() = 0;
     virtual TransportStats stats() const = 0;
     virtual std::string endpoint_string() const = 0;
+    virtual void set_peer_id(std::string peer_id) = 0;
   };
 
 } // namespace vix::p2p
diff --git a/include/vix/p2p/messages/Hello.hpp b/include/vix/p2p/messages/Hello.hpp
@@ -23,17 +23,20 @@
 
 namespace vix::p2p::msg
 {
-  // Hello = HelloInit (A -> B)
+  // Hello (initiator -> responder)
   struct Hello
   {
     // anti-replay / handshake v2
     std::uint64_t nonce_a{0};
     std::uint64_t ts_ms{0};
+
     // identity
     std::string node_id;
+
     // capabilities
     std::unordered_map<std::string, std::string> capabilities;
-    // crypto
+
+    // crypto: sender public key
     std::vector<std::uint8_t> public_key;
 
     std::vector<std::uint8_t> encode() const
@@ -43,6 +46,7 @@ namespace vix::p2p::msg
       w.var_u64(nonce_a);
       w.var_u64(ts_ms);
       w.str_var(node_id);
+
       w.var_u64((std::uint64_t)capabilities.size());
       for (const auto &[k, v] : capabilities)
       {
diff --git a/include/vix/p2p/messages/HelloAck.hpp b/include/vix/p2p/messages/HelloAck.hpp
@@ -21,18 +21,22 @@
 
 namespace vix::p2p::msg
 {
-  // HelloAck (B -> A)
+  // HelloAck (responder -> initiator)
   struct HelloAck
   {
     std::uint64_t nonce_a{0}; // echo
     std::uint64_t nonce_b{0}; // challenge
 
+    // NEW: responder public key (needed by initiator to derive session key)
+    std::vector<std::uint8_t> public_key;
+
     std::vector<std::uint8_t> encode() const
     {
       bin::Writer w;
-      w.reserve(24);
+      w.reserve(64);
       w.var_u64(nonce_a);
       w.var_u64(nonce_b);
+      w.bytes_var(public_key);
       return std::move(w.out);
     }
 
@@ -42,6 +46,7 @@ namespace vix::p2p::msg
       HelloAck a;
       a.nonce_a = r.var_u64();
       a.nonce_b = r.var_u64();
+      a.public_key = r.bytes_var();
 
       if (r.remaining() != 0)
         throw bin::Error("HelloAck: trailing bytes");
diff --git a/include/vix/p2p/messages/HelloFinish.hpp b/include/vix/p2p/messages/HelloFinish.hpp
@@ -21,11 +21,12 @@
 
 namespace vix::p2p::msg
 {
-  // HelloFinish (A -> B)
+  // HelloFinish (initiator -> responder)
   struct HelloFinish
   {
     std::uint64_t nonce_a{0};
     std::uint64_t nonce_b{0};
+
     // signature bytes (ed25519 later); for now can be empty in NullCrypto mode
     std::vector<std::uint8_t> signature;
 
diff --git a/src/Node.cpp b/src/Node.cpp
@@ -37,6 +37,8 @@
 #include <vix/p2p/messages/Hello.hpp>
 #include <vix/p2p/messages/Ping.hpp>
 #include <vix/p2p/messages/Pong.hpp>
+#include <vix/p2p/messages/HelloAck.hpp>
+#include <vix/p2p/messages/HelloFinish.hpp>
 
 #include <vix/p2p/transport/Tcp.hpp>
 #include <vix/p2p/Bootstrap.hpp>
@@ -428,7 +430,6 @@ namespace vix::p2p
       }
 
       schedule_handshake_timeout_(pid);
-      send_hello(pid);
     }
 
     bool has_peer_by_endpoint_unlocked_(const PeerEndpoint &ep) const
@@ -521,71 +522,107 @@ namespace vix::p2p
 
     void on_hello(const PeerId &peer_id, const msg::Hello &h)
     {
-      std::scoped_lock lk(mu_);
+      Envelope out_env;
+
+      {
+        std::scoped_lock lk(mu_);
+
+        auto &peer = peers_[peer_id];
+
+        auto now_ms = now_ms_();
+        if (std::llabs((long long)now_ms - (long long)h.ts_ms) > 60'000)
+          throw std::runtime_error("hello replay");
 
-      auto &peer = peers_[peer_id];
+        peer.handshake.emplace();
+        peer.handshake->stage = HandshakeState::Stage::HelloReceived;
+        peer.handshake->nonce_a = h.nonce_a;
+        peer.handshake->ts_ms = h.ts_ms;
+        peer.handshake->started_at = std::chrono::steady_clock::now();
 
-      auto now_ms = now_ms_();
-      if (std::llabs((long long)now_ms - (long long)h.ts_ms) > 60'000)
-        throw std::runtime_error("hello replay");
+        peer.meta.public_key = h.public_key;
 
-      peer.handshake.emplace();
-      peer.handshake->stage = HandshakeState::Stage::HelloReceived;
-      peer.handshake->nonce_a = h.nonce_a;
-      peer.handshake->ts_ms = h.ts_ms;
-      peer.handshake->started_at = std::chrono::steady_clock::now();
-      peer.meta.public_key = h.public_key;
+        constexpr std::size_t kMinPubKey = 16;
+        constexpr std::size_t kMaxPubKey = 2048;
+        if (peer.meta.public_key.size() < kMinPubKey || peer.meta.public_key.size() > kMaxPubKey)
+          throw std::runtime_error("bad public_key size");
 
-      constexpr std::size_t kMinPubKey = 16;
-      constexpr std::size_t kMaxPubKey = 2048;
+        msg::HelloAck ack;
+        ack.nonce_a = h.nonce_a;
+        ack.nonce_b = rand_u64_();
 
-      if (peer.meta.public_key.size() < kMinPubKey || peer.meta.public_key.size() > kMaxPubKey)
-        throw std::runtime_error("bad public_key size");
+        ack.public_key = self_keys_.public_key;
 
-      msg::HelloAck ack;
-      ack.nonce_a = h.nonce_a;
-      ack.nonce_b = rand_u64_();
-      peer.handshake->nonce_b = ack.nonce_b;
-      peer.handshake->stage = HandshakeState::Stage::AckSent;
-      send_envelope(peer_id, pack::make_envelope(MessageType::HelloAck, ack));
+        peer.handshake->nonce_b = ack.nonce_b;
+        peer.handshake->stage = HandshakeState::Stage::AckSent;
+
+        out_env = pack::make_envelope(MessageType::HelloAck, ack);
+      }
+
+      // ✅ send OUTSIDE the lock (avoid deadlock)
+      send_envelope(peer_id, out_env);
     }
 
     void on_hello_ack(const PeerId &peer_id, const msg::HelloAck &a)
     {
-      std::scoped_lock lk(mu_);
+      Envelope out_env;
 
-      auto &peer = peers_[peer_id];
+      {
+        std::scoped_lock lk(mu_);
 
-      if (!peer.handshake)
-        throw std::runtime_error("missing handshake state");
+        auto &peer = peers_[peer_id];
 
-      auto &hs = *peer.handshake;
+        if (!peer.handshake)
+          throw std::runtime_error("missing handshake state");
 
-      if (hs.ts_ms == 0)
-        throw std::runtime_error("hs missing ts_ms");
+        auto &hs = *peer.handshake;
 
-      if (hs.stage != HandshakeState::Stage::HelloSent)
-        throw std::runtime_error("unexpected HelloAck");
+        if (hs.ts_ms == 0)
+          throw std::runtime_error("hs missing ts_ms");
 
-      if (a.nonce_a != hs.nonce_a)
-        throw std::runtime_error("nonce mismatch");
+        if (hs.stage != HandshakeState::Stage::HelloSent)
+          throw std::runtime_error("unexpected HelloAck");
 
-      hs.nonce_b = a.nonce_b;
-      hs.stage = HandshakeState::Stage::AckReceived;
+        if (a.nonce_a != hs.nonce_a)
+          throw std::runtime_error("nonce mismatch");
 
-      auto data = make_handshake_bytes_(
-          hs.nonce_a,
-          hs.nonce_b,
-          hs.ts_ms,
-          kProto_.major,
-          kProto_.minor,
-          kTransport_());
+        peer.meta.public_key = a.public_key;
+
+        constexpr std::size_t kMinPubKey = 16;
+        constexpr std::size_t kMaxPubKey = 2048;
+        if (peer.meta.public_key.size() < kMinPubKey || peer.meta.public_key.size() > kMaxPubKey)
+          throw std::runtime_error("bad public_key size");
+
+        hs.nonce_b = a.nonce_b;
+        hs.stage = HandshakeState::Stage::AckReceived;
+
+        auto data = make_handshake_bytes_(
+            hs.nonce_a,
+            hs.nonce_b,
+            hs.ts_ms,
+            kProto_.major,
+            kProto_.minor,
+            kTransport_());
+
+        msg::HelloFinish fin;
+        fin.nonce_a = hs.nonce_a;
+        fin.nonce_b = hs.nonce_b;
+        fin.signature = crypto_->sign(data, self_keys_.private_key);
+
+        auto sk = derive_session_key_unlocked_(hs, peer.meta.public_key);
+
+        peer.meta.session_key_32 = std::move(sk);
+        peer.meta.secure = true;
+        peer.meta.send_nonce_counter = 1;
+
+        peer.state = PeerState::Connected;
+        peer.handshake.reset();
+        stats_.handshakes_completed++;
+
+        out_env = pack::make_envelope(MessageType::HelloFinish, fin);
+      }
 
-      msg::HelloFinish fin;
-      fin.nonce_a = hs.nonce_a;
-      fin.nonce_b = hs.nonce_b;
-      fin.signature = crypto_->sign(data, self_keys_.private_key);
-      send_envelope(peer_id, pack::make_envelope(MessageType::HelloFinish, fin));
+      // ✅ send OUTSIDE lock
+      send_envelope(peer_id, out_env);
     }
 
     void on_hello_finish(const PeerId &peer_id, const msg::HelloFinish &f)
@@ -725,10 +762,18 @@ namespace vix::p2p
           return;
         }
       }
+      catch (const std::exception &e)
+      {
+        if (!running_)
+          return;
+        log_(std::string("[p2p] envelope error: ") + e.what() + " peer_id=" + peer_id);
+        disconnect_impl_(peer_id);
+      }
       catch (...)
       {
         if (!running_)
           return;
+        log_(std::string("[p2p] envelope error: unknown exception peer_id=") + peer_id);
         disconnect_impl_(peer_id);
       }
     }
@@ -756,11 +801,15 @@ namespace vix::p2p
       h.node_id = cfg_.node_id;
       h.public_key = self_keys_.public_key;
 
-      peers_[peer_id].handshake.emplace();
-      peers_[peer_id].handshake->nonce_a = h.nonce_a;
-      peers_[peer_id].handshake->ts_ms = h.ts_ms;
-      peers_[peer_id].handshake->stage = HandshakeState::Stage::HelloSent;
-      peers_[peer_id].handshake->started_at = std::chrono::steady_clock::now();
+      {
+        std::scoped_lock lk(mu_);
+        auto &p = peers_[peer_id];
+        p.handshake.emplace();
+        p.handshake->nonce_a = h.nonce_a;
+        p.handshake->ts_ms = h.ts_ms;
+        p.handshake->stage = HandshakeState::Stage::HelloSent;
+        p.handshake->started_at = std::chrono::steady_clock::now();
+      }
 
       send_envelope(peer_id, pack::make_envelope(MessageType::Hello, h));
     }
@@ -946,6 +995,10 @@ namespace vix::p2p
       transports_.erase(tit);
 
       p.id = new_id;
+
+      if (t)
+        t->set_peer_id(new_id);
+
       peers_.emplace(new_id, std::move(p));
       transports_.emplace(new_id, std::move(t));
     }
diff --git a/src/TransportTcp.cpp b/src/TransportTcp.cpp

Original file line number	Diff line number	Diff line change
`@@ -21,11 +21,12 @@`
`21`	`21`
`22`	`22`	`namespace vix::p2p::msg`
`23`	`23`	`{`
`24`		`- // HelloFinish (A -> B)`
	`24`	`+ // HelloFinish (initiator -> responder)`
`25`	`25`	`struct HelloFinish`
`26`	`26`	`{`
`27`	`27`	`std::uint64_t nonce_a{0};`
`28`	`28`	`std::uint64_t nonce_b{0};`
	`29`	`+`
`29`	`30`	`// signature bytes (ed25519 later); for now can be empty in NullCrypto mode`
`30`	`31`	`std::vector<std::uint8_t> signature;`
`31`	`32`