1 files changed, 102 insertions, 20 deletions

diff --git a/src/nostr.cpp b/src/nostr.cpp
index d286767..32cea18 100644
--- a/src/nostr.cpp
+++ b/src/nostr.cpp
@@ -6,6 +6,7 @@
 #include "headers.h"
 #include <openssl/ssl.h>
 #include <openssl/err.h>
+#include <openssl/x509.h>
 #include <openssl/x509v3.h>
 #ifdef _WIN32
 #include <wincrypt.h>
@@ -82,9 +83,20 @@ static CCriticalSection   cs_btfPeers;
 // independent relays. Kept only one Cloudflare-fronted relay (damus.io, the
 // most established) and filled the rest with relays on different hosts
 // (Hetzner, various independents) so no single upstream provider is a
-// single point of failure for peer discovery. 7 relays lands inside the
+// single point of failure for peer discovery. 8 relays lands inside the
 // commonly recommended 5-10 range for redundancy without keeping an
 // excessive number of persistent sockets open per node.
+//
+// Four entries were removed after probing all twelve from two independent
+// networks (a residential line and the pool's VPS, over both v4 and v6):
+//   relay.current.fyi, relay.nostr.bg  -- no A record, and the AAAA for both
+//     lands on the same parked host (2a04:c300:1000::381) serving a
+//     certificate that fails verification. The domains are gone.
+//   relay.nostr.band, nostr.fmt.wiz.biz -- TCP connects, TLS handshake never
+//     completes, from both networks. Whether they are down or refusing
+//     non-browser clients, this node cannot use them.
+// A dead relay is not free: each one holds a connect/retry slot and delays
+// discovery, so carrying it costs latency for no redundancy.
 const char* pszNostrRelays[] = {
     "wss://relay.damus.io",
     "wss://nos.lol",
@@ -97,6 +109,8 @@ const char* pszNostrRelays[] = {
     "wss://offchain.pub",
     "wss://nostr.mom",
     "wss://relay.primal.net",
+    "wss://nostr.oxtr.dev",
+    "wss://relay.nos.social",
 };
 const int nNostrRelays = ARRAYLEN(pszNostrRelays);
 
@@ -110,6 +124,12 @@ vector<string> vBtfMeetingRelays = {
     "31.44.4.249:8434",     // New Jersey, US
     "90.156.222.107:8434",  // Almaty, KZ
 };
+// True when the operator pinned the relay list with /rvrelay: discovered
+// relays must NOT be merged in (BtfAllRelays), otherwise one accept thread
+// per discovered relay is spawned and each one optimistically overwrites the
+// advertised meeting node (BtfSetActiveRelay) before registering -- a
+// discovered but unreachable relay then flaps the published .btf descriptor.
+bool fBtfRelayPinned = false;
 string strBtfActiveRelay;
 static CCriticalSection cs_activeRelay;
 
@@ -144,7 +164,7 @@ static map<string, BtfPoolAnnouncement> g_discoveredPools;
 static CCriticalSection cs_discoveredPools;
 
 // Basic sanity: "host:port" with a numeric 1..65535 port and a plausible host.
-static bool LooksLikeRelay(const string& r)
+bool LooksLikeRelay(const string& r)
 {
     size_t colon = r.rfind(':');
     if (colon == string::npos || colon == 0 || colon + 1 >= r.size())
@@ -172,6 +192,8 @@ static bool LooksLikeRelay(const string& r)
 vector<string> BtfAllRelays()
 {
     vector<string> all = vBtfMeetingRelays;
+    if (fBtfRelayPinned)
+        return all;
     CRITICAL_BLOCK(cs_discoveredRelays)
         foreach(const string& r, g_discoveredRelays)
             if (find(all.begin(), all.end(), r) == all.end())
@@ -550,26 +572,52 @@ public:
         {
             sslctx = SSL_CTX_new(TLS_client_method());
             if (!sslctx) return false;
+
+            // SSL_CTX_new defaults to SSL_VERIFY_NONE with no trust store
+            // loaded, so until now every wss:// relay connection was accepted
+            // without looking at the certificate at all -- any on-path
+            // attacker could impersonate a relay. Our own events are signed,
+            // so a MITM cannot forge them, but it could drop the rendezvous
+            // registration (leaving the pool unreachable to miners) or answer
+            // subscriptions with whatever it liked.
             SSL_CTX_set_verify(sslctx, SSL_VERIFY_PEER, NULL);
+
+            // OpenSSL's default verify paths are usually empty on Windows, so
+            // the system ROOT store is loaded too -- without it verification
+            // fails closed there and every relay becomes unreachable.
             bool fTrustLoaded = (SSL_CTX_set_default_verify_paths(sslctx) == 1);
 #ifdef _WIN32
             fTrustLoaded = BtfLoadWindowsRootCertificates(sslctx) || fTrustLoaded;
 #endif
             if (!fTrustLoaded)
                 return error("Nostr: failed to load TLS trust store");
+
+            // Floor the protocol version. Verification says who the peer is,
+            // not how weak a cipher suite we will agree to; without this a
+            // relay offering nothing better still gets TLS 1.0.
+            SSL_CTX_set_min_proto_version(sslctx, TLS1_2_VERSION);
+
             ssl = SSL_new(sslctx);
             if (!ssl) return false;
             SSL_set_fd(ssl, (int)hSocket);
-            if (SSL_set_tlsext_host_name(ssl, host.c_str()) != 1)
+            if (SSL_set_tlsext_host_name(ssl, host.c_str()) != 1)  // SNI
                 return error("Nostr: failed to set TLS SNI for %s", host.c_str());
+
+            // Verifying the chain is not enough on its own: without this the
+            // relay could present any certificate a trusted CA ever issued,
+            // for any name. SSL_set1_host binds the check to the host we asked
+            // for.
             if (SSL_set1_host(ssl, host.c_str()) != 1)
                 return error("Nostr: failed to set TLS hostname for %s", host.c_str());
+
             if (SSL_connect(ssl) != 1)
+            {
+                long nVerify = SSL_get_verify_result(ssl);
+                if (nVerify != X509_V_OK)
+                    return error("Nostr: certificate for %s rejected: %s",
+                                 host.c_str(), X509_verify_cert_error_string(nVerify));
                 return error("Nostr: TLS handshake with %s failed", host.c_str());
-            long nVerify = SSL_get_verify_result(ssl);
-            if (nVerify != X509_V_OK)
-                return error("Nostr: TLS certificate verification failed for %s: %s",
-                             host.c_str(), X509_verify_cert_error_string(nVerify));
+            }
         }
 
         // WebSocket handshake (HTTP Upgrade)
@@ -1017,9 +1065,14 @@ static void HandlePoolAnnouncement(const json& ev)
 
 // Resolve a `.btf` address via a relay: fetch the descriptor published by that
 // address's key and verify it self-certifies under the decoded pubkey.
+// fExchangeOk reports whether the relay actually finished the conversation
+// (an EOSE arrived) as opposed to the socket dying mid-read. The caller pools
+// this socket for reuse, and only a completed exchange leaves it in a state
+// the next caller can trust.
 static bool ResolveDescriptor(CWebSocket& ws, void* ctx, const string& btfAddr,
-                              btf::Descriptor& out)
+                              btf::Descriptor& out, bool& fExchangeOk)
 {
+    fExchangeOk = false;
     unsigned char pk[32];
     if (!btf::ParseAddress(btfAddr, pk))
         return false;
@@ -1031,26 +1084,42 @@ static bool ResolveDescriptor(CWebSocket& ws, void* ctx, const string& btfAddr,
     json req = json::array({ "REQ", "btf-resolve", filter });
     if (!ws.SendText(req.dump()))
         return false;
+    bool fFound = false;
     for (;;)
     {
         string msg;
         if (!ws.RecvText(msg))
-            break;
+            return fFound;  // socket died mid-exchange; fExchangeOk stays false
         json j;
         try { j = json::parse(msg); } catch (...) { continue; }
         if (!j.is_array() || j.empty() || !j[0].is_string()) continue;
         string t = j[0].get<string>();
+        // Every relay message names the subscription it belongs to. This socket
+        // is pooled and reused, so a message left over from an earlier exchange
+        // is the ordinary case here, not an exotic one -- and an unmatched EOSE
+        // ends this query before it has begun, which is indistinguishable from
+        // "that descriptor does not exist". Check the id.
+        if (j.size() >= 2 && j[1].is_string() && j[1].get<string>() != "btf-resolve")
+            continue;
         if (t == "EVENT" && j.size() >= 3)
         {
             const json& ev = j[2];
-            if (ev.contains("content") &&
+            if (!fFound && ev.contains("content") &&
                 btf::VerifyDescriptor(ctx, ev["content"].get<string>(), pk, out))
-                return true;
+                fFound = true;
+            // Deliberately keep reading to EOSE rather than returning here.
+            // Returning early leaves the rest of the relay's answer queued on a
+            // socket that goes straight back into the pool, and the next caller
+            // reads our leftovers as its own -- one message out of step, for
+            // the life of the process.
         }
         else if (t == "EOSE")
+        {
+            fExchangeOk = true;  // relay said "that is all": socket is clean
             break;
+        }
     }
-    return false;
+    return fFound;
 }
 
 // Resolve a `.btf` address into its rendezvous coordinates by querying the
@@ -1069,12 +1138,22 @@ bool BtfResolve(const std::string& btfAddr, std::string& meetingHostPort,
             if (!lease.Connected())
                 continue;
             btf::Descriptor d;
-            if (!ResolveDescriptor(lease.Ws(), g_nostrKey.ctx, btfAddr, d))
+            bool fExchangeOk = false;
+            if (!ResolveDescriptor(lease.Ws(), g_nostrKey.ctx, btfAddr, d, fExchangeOk))
             {
-                lease.fOk = true; // exchange completed, just no match
+                // Only keep the pooled socket if the relay actually finished
+                // the conversation. This used to say "exchange completed, just
+                // no match" on every failure -- including a dead socket -- so
+                // ~CRelayLease never closed it and fConnected was never
+                // cleared. A relay connection that died was then pinned in the
+                // pool, reported healthy, and reused for every later lookup,
+                // which is why "no Nostr descriptor found" persisted for a
+                // descriptor that was demonstrably published, and why only a
+                // restart cleared it.
+                lease.fOk = fExchangeOk;
                 continue;
             }
-            lease.fOk = true;
+            lease.fOk = fExchangeOk;
             if (d.meeting_node.empty() || d.meeting_node == "rendezvous-pending")
                 continue; // descriptor predates the owner's relay config
             if (!HexDecode(d.enc, enc_pub, 32))
@@ -1404,7 +1483,7 @@ void ThreadBtfPoolAnnouncer(void*)
     LogPrint("pool", "ThreadBtfPoolAnnouncer started\n");
     while (!fShutdown)
     {
-        if (nMineMode == MINE_OPERATOR && gPoolRunning)
+        if (nMineMode == MINE_OPERATOR && gPoolRunning && !fPoolNoAnnounce)
         {
             BtfPoolAnnouncement ann;
             ann.btfAddress = BtfLocalAddress();
@@ -1736,14 +1815,17 @@ void ThreadNostrSeed(void* parg)
             PublishDescriptor(ws, key);
             Sleep(700); // let the relay store the replaceable event
             btf::Descriptor d;
-            if (ResolveDescriptor(ws, key.ctx, key.BtfAddress(), d))
+            bool fExchangeOk = false;
+            if (ResolveDescriptor(ws, key.ctx, key.BtfAddress(), d, fExchangeOk))
             {
                 LogPrint("nostr", "Nostr: .btf self-resolve OK via %s (meeting_node=%s, enc=%s...)\n",
                        pszNostrRelays[i], d.meeting_node.c_str(), d.enc.substr(0, 16).c_str());
-                lease.fOk = true;
+                lease.fOk = fExchangeOk;
                 break;
             }
-            lease.fOk = true;
+            // Same rule as BtfResolve: a socket only goes back in the pool if
+            // the relay finished the conversation on it.
+            lease.fOk = fExchangeOk;
         }
         CATCH_PRINT_EXCEPTION("btf self-test")
     }
@@ -1775,7 +1857,7 @@ void ThreadNostrSeed(void* parg)
         }
         CATCH_PRINT_EXCEPTION("ConnectDiscoveredBtfPeers")
 
-        if (nMineMode == MINE_OPERATOR && gPoolRunning)
+        if (nMineMode == MINE_OPERATOR && gPoolRunning && !fPoolNoAnnounce)
         {
             BtfPoolAnnouncement ann;
             ann.btfAddress = BtfLocalAddress();