btfaddr: reject descriptors whose onion endpoint is not signed

Upstream 1.2.19 signs the onion endpoint only under the optional sig3, and
VerifyDescriptor treated sig3 failure as 'drop the onion, accept the rest'.
Any relay could therefore strip an onion endpoint down to a rendezvous-only
descriptor (or, under /oniononly, cut the peer off) by deleting one field.

Now an onion field must arrive with a valid sig3 over the V3 digest or the
whole descriptor is rejected. Honest 1.2.19 writers always emit both fields,
so interop is preserved. The full two-field strip still verifies -- it is
indistinguishable from a peer that never had an onion -- and needs the onion
bound into the primary sig, which only upstream can do without breaking every
1.2.18/1.2.19 verifier; reported as an upstream protocol defect.

test_btfaddr flips the two upstream checks that enshrined the soft drop and
adds unsigned-onion and full-strip cases.

2 files changed, 41 insertions, 18 deletions

diff --git a/src/btfaddr.cpp b/src/btfaddr.cpp
index ce2ac86..502220e 100644
--- a/src/btfaddr.cpp
+++ b/src/btfaddr.cpp
@@ -319,20 +319,32 @@ bool VerifyDescriptor(void* ctxv, const std::string& jsonStr,
         std::string onion;
         if (v.contains("onion"))
         {
-            if (v["onion"].is_string() && v.contains("sig3") && v["sig3"].is_string())
-            {
-                std::string onionNorm;
-                std::string onionRaw = v["onion"].get<std::string>();
-                if (NormalizeOnionEndpoint(onionRaw, onionNorm) && onionNorm == onionRaw)
-                {
-                    unsigned char sig3[64];
-                    unsigned char digest3[32];
-                    DescriptorDigestV3(expect_pubkey, enc, meeting_node, onionNorm, created, digest3);
-                    if (HexDecode(v["sig3"].get<std::string>(), sig3, 64) &&
-                        secp256k1_schnorrsig_verify(ctx, sig3, digest3, 32, &xpub))
-                        onion = onionNorm;
-                }
-            }
+            // A descriptor that carries an onion endpoint must prove it. The
+            // endpoint is covered by sig3 and only sig3, so any failure here --
+            // missing sig3, bad hex, unnormalised endpoint, bad signature --
+            // means the descriptor was tampered with, and a tampered
+            // descriptor is rejected outright. Upstream dropped just the onion
+            // and accepted the rest, which let any relay convert an onion
+            // descriptor into a rendezvous-only one by deleting a field.
+            //
+            // Residual, by design: removing BOTH "onion" and "sig3" still
+            // verifies, because the result is indistinguishable from a peer
+            // that never had an onion endpoint. Closing that requires binding
+            // the onion into the primary sig (the V2 digest), which every
+            // upstream 1.2.18/1.2.19 verifier would reject -- an upstream
+            // protocol change, not something this fork can do unilaterally.
+            if (!v["onion"].is_string()) return false;
+            if (!v.contains("sig3") || !v["sig3"].is_string()) return false;
+            std::string onionNorm;
+            std::string onionRaw = v["onion"].get<std::string>();
+            if (!NormalizeOnionEndpoint(onionRaw, onionNorm) || onionNorm != onionRaw)
+                return false;
+            unsigned char sig3[64];
+            unsigned char digest3[32];
+            DescriptorDigestV3(expect_pubkey, enc, meeting_node, onionNorm, created, digest3);
+            if (!HexDecode(v["sig3"].get<std::string>(), sig3, 64)) return false;
+            if (!secp256k1_schnorrsig_verify(ctx, sig3, digest3, 32, &xpub)) return false;
+            onion = onionNorm;
         }
 
         memcpy(out.pubkey, expect_pubkey, 32);
diff --git a/src/test_btfaddr.cpp b/src/test_btfaddr.cpp
index 8de426e..9d33d63 100644
--- a/src/test_btfaddr.cpp
+++ b/src/test_btfaddr.cpp
@@ -133,12 +133,23 @@ int main()
         std::string d3 = btf::SignDescriptor(ctx, sk, ENC_HEX, "node-1", 5, ONION);
         json j3 = json::parse(d3);
         j3["onion"] = "bcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcde.onion:8433";
-        CHECK(btf::VerifyDescriptor(ctx, j3.dump(), pk, o) && o.onion.empty(),
-              "tampered onion ignored rather than trusted");
+        CHECK(!btf::VerifyDescriptor(ctx, j3.dump(), pk, o),
+              "tampered onion rejects the descriptor");
         json j4 = json::parse(d3);
         j4["sig3"] = std::string(128, '0');
-        CHECK(btf::VerifyDescriptor(ctx, j4.dump(), pk, o) && o.onion.empty(),
-              "bad onion signature ignored rather than trusted");
+        CHECK(!btf::VerifyDescriptor(ctx, j4.dump(), pk, o),
+              "bad onion signature rejects the descriptor");
+        json j5 = json::parse(d3);
+        j5.erase("sig3");
+        CHECK(!btf::VerifyDescriptor(ctx, j5.dump(), pk, o),
+              "unsigned onion rejects the descriptor");
+        // Removing both fields still verifies: the result is indistinguishable
+        // from a peer that never had an onion endpoint. That residual strip
+        // needs the onion bound into the primary sig, an upstream change.
+        json j6 = json::parse(d3);
+        j6.erase("onion"); j6.erase("sig3");
+        CHECK(btf::VerifyDescriptor(ctx, j6.dump(), pk, o) && o.onion.empty(),
+              "full strip indistinguishable from no-onion (documented residual)");
     }
 
     printf("full_resolve_chain_from_address\n");