graph/db: treat empty channel signatures as missing
What changed, and why it matters
This commit fixes a bug in LND's Lightning Network graph database where empty channel signatures were being treated as valid proof that a channel was publicly announced. Because the code previously treated empty byte slices the same as real signatures, a node connected only by such a 'channel' could incorrectly be considered public. The fix makes the database store empty signatures as NULL and changes all public-channel checks to require a signature length greater than zero. The commit message frames this as a correctness improvement and adds regression tests, but does not label it a security vulnerability.
Treat as a bug-fix commit with possible security side effects. Review whether any existing node/channel was incorrectly marked public due to empty signatures and consider re-evaluating public-node state. No immediate emergency response is indicated, but operators running versions prior to this commit may be exposed to incorrect gossip visibility decisions.
Security signals we found
Logic flaw: empty byte slice treated as valid authentication proof
Public-node/channel classification depends on signature presence
SQL NULL vs empty bytea inconsistency in stored proof data
Regression test added for empty V1 and V2 signatures
Potential information-disclosure / gossip-policy bypass if nodes are incorrectly advertised as public
Evidence from the diff
The patch changes three layers: (1) graph/db/models/channel_auth_proof.go now returns nil instead of []byte{} from signature accessors, so SQL drivers store missing signatures as NULL; (2) graph/db/kv_store.go’s isPublic() now requires AuthProof non-nil, !IsEmpty(), and len(BitcoinSig1()) > 0 before marking a node public; (3) sqldb/sqlc/queries/graph.sql and generated code replace IS NOT NULL checks with COALESCE(length(signature), 0) > 0 for V1 bitcoin_1_signature/node_1_signature and V2 signature. A regression test verifies that empty V1 and V2 channel signatures do not make a node public. The bug could cause IsPublicNode, public channel queries, and node-publicity pruning logic to misclassify nodes/channels as public when their announcement signatures were empty.
Changed components
graph/db/models/channel_auth_proof.gograph/db/kv_store.gosqldb/sqlc/graph.sql.gosqldb/sqlc/queries/graph.sqlgraph/db/graph_test.goInspect captured patch +85 / −27
diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go
index a6dcb10..5148f63 100644
--- a/graph/db/graph_test.go
+++ b/graph/db/graph_test.go
@@ -154,6 +154,10 @@ var versionedTests = []versionedTest{
name: "node is public",
test: testNodeIsPublic,
},
+ {
+ name: "node is public empty channel signature",
+ test: testIsPublicNodeEmptyChannelSignature,
+ },
}
// TestVersionedDBs runs various tests against both v1 and v2 versioned
@@ -4265,6 +4269,59 @@ func testNodeIsPublic(t *testing.T, v lnwire.GossipVersion) {
)
}
+// testIsPublicNodeEmptyChannelSignature ensures empty channel signatures don't
+// mark nodes as public.
+func testIsPublicNodeEmptyChannelSignature(t *testing.T,
+ v lnwire.GossipVersion) {
+
+ t.Parallel()
+ ctx := t.Context()
+
+ testGraph := MakeTestGraph(t)
+ graph := NewVersionedGraph(testGraph, v)
+
+ // Set a source node as it's required for IsPublicNode.
+ sourceNode := createTestVertex(t, v)
+ err := graph.SetSourceNode(ctx, sourceNode)
+ require.NoError(t, err)
+
+ node1 := createTestVertex(t, v)
+
+ node1.LastUpdate = nextUpdateTime()
+
+ err = graph.AddNode(ctx, node1)
+ require.NoError(t, err)
+
+ // Create an edge between source node and node1, with
+ // empty signatures. This tests that empty signatures
+ // don't mark nodes as public.
+ edgeInfo, _ := createEdge(
+ v, 10, 0, 0, 0, sourceNode, node1,
+ true,
+ )
+
+ switch v {
+ case lnwire.GossipVersion1:
+ edgeInfo.AuthProof =
+ models.NewV1ChannelAuthProof(
+ []byte{}, []byte{},
+ []byte{}, []byte{},
+ )
+ case lnwire.GossipVersion2:
+ edgeInfo.AuthProof =
+ models.NewV2ChannelAuthProof([]byte{})
+ }
+
+ err = graph.AddChannelEdge(ctx, edgeInfo)
+ require.NoError(t, err)
+
+ // node1 should NOT be considered public because the
+ // channel announcement has empty signatures.
+ isPublic, err := graph.IsPublicNode(node1.PubKeyBytes)
+ require.NoError(t, err)
+ require.False(t, isPublic)
+}
+
// BenchmarkIsPublicNode measures the performance of IsPublicNode when checking
// a large number of nodes.
func BenchmarkIsPublicNode(b *testing.B) {
diff --git a/graph/db/kv_store.go b/graph/db/kv_store.go
index 550ed32..eb4c3b9 100644
--- a/graph/db/kv_store.go
+++ b/graph/db/kv_store.go
@@ -3377,8 +3377,11 @@ func (c *KVStore) isPublic(tx kvdb.RTx, nodePub route.Vertex,
}
// Since the edge _does_ extend to the source node, we'll also
- // need to ensure that this is a public edge.
- if info.AuthProof != nil {
+ // need to ensure that this is a public edge with valid
+ // signatures (not empty).
+ if info.AuthProof != nil && !info.AuthProof.IsEmpty() &&
+ len(info.AuthProof.BitcoinSig1()) > 0 {
+
nodeIsPublic = true
return errDone
}
diff --git a/graph/db/models/channel_auth_proof.go b/graph/db/models/channel_auth_proof.go
index 1e1fb03..e4acc2f 100644
--- a/graph/db/models/channel_auth_proof.go
+++ b/graph/db/models/channel_auth_proof.go
@@ -89,31 +89,29 @@ func NewV2ChannelAuthProof(signature []byte) *ChannelAuthProof {
}
}
-// NodeSig1 returns the first node signature bytes, or an empty slice if not
-// present.
+// NodeSig1 returns the first node signature bytes, or nil if not present.
func (c *ChannelAuthProof) NodeSig1() []byte {
- return c.NodeSig1Bytes.UnwrapOr([]byte{})
+ return c.NodeSig1Bytes.UnwrapOr(nil)
}
-// NodeSig2 returns the second node signature bytes, or an empty slice if not
-// present.
+// NodeSig2 returns the second node signature bytes, or nil if not present.
func (c *ChannelAuthProof) NodeSig2() []byte {
- return c.NodeSig2Bytes.UnwrapOr([]byte{})
+ return c.NodeSig2Bytes.UnwrapOr(nil)
}
-// BitcoinSig1 returns the first bitcoin signature bytes, or an empty slice if
-// not present.
+// BitcoinSig1 returns the first bitcoin signature bytes, or nil if not
+// present.
func (c *ChannelAuthProof) BitcoinSig1() []byte {
- return c.BitcoinSig1Bytes.UnwrapOr([]byte{})
+ return c.BitcoinSig1Bytes.UnwrapOr(nil)
}
-// BitcoinSig2 returns the second bitcoin signature bytes, or an empty slice if
-// not present.
+// BitcoinSig2 returns the second bitcoin signature bytes, or nil if not
+// present.
func (c *ChannelAuthProof) BitcoinSig2() []byte {
- return c.BitcoinSig2Bytes.UnwrapOr([]byte{})
+ return c.BitcoinSig2Bytes.UnwrapOr(nil)
}
-// Sig returns the v2 signature bytes, or an empty slice if not present.
+// Sig returns the v2 signature bytes, or nil if not present.
func (c *ChannelAuthProof) Sig() []byte {
- return c.Signature.UnwrapOr([]byte{})
+ return c.Signature.UnwrapOr(nil)
}
diff --git a/sqldb/sqlc/graph.sql.go b/sqldb/sqlc/graph.sql.go
index 135f00d..b8bb884 100644
--- a/sqldb/sqlc/graph.sql.go
+++ b/sqldb/sqlc/graph.sql.go
@@ -2085,7 +2085,7 @@ WHERE last_update >= $1
SELECT 1
FROM graph_channels c
WHERE c.version = 1
- AND c.bitcoin_1_signature IS NOT NULL
+ AND COALESCE(length(c.bitcoin_1_signature), 0) > 0
AND (c.node_id_1 = graph_nodes.id OR c.node_id_2 = graph_nodes.id)
)
)
@@ -2211,7 +2211,7 @@ func (q *Queries) GetPruneTip(ctx context.Context) (GraphPruneLog, error) {
const getPublicV1ChannelsBySCID = `-- name: GetPublicV1ChannelsBySCID :many
SELECT id, version, scid, node_id_1, node_id_2, outpoint, capacity, bitcoin_key_1, bitcoin_key_2, node_1_signature, node_2_signature, bitcoin_1_signature, bitcoin_2_signature, signature, funding_pk_script, merkle_root_hash
FROM graph_channels
-WHERE node_1_signature IS NOT NULL
+WHERE COALESCE(length(node_1_signature), 0) > 0
AND scid >= $1
AND scid < $2
`
@@ -2732,14 +2732,14 @@ SELECT EXISTS (
-- one of the signatures since we only ever set them
-- together.
WHERE c.version = 1
- AND c.bitcoin_1_signature IS NOT NULL
+ AND COALESCE(length(c.bitcoin_1_signature), 0) > 0
AND n.pub_key = $1
UNION ALL
SELECT 1
FROM graph_channels c
JOIN graph_nodes n ON n.id = c.node_id_2
WHERE c.version = 1
- AND c.bitcoin_1_signature IS NOT NULL
+ AND COALESCE(length(c.bitcoin_1_signature), 0) > 0
AND n.pub_key = $1
)
`
@@ -2760,7 +2760,7 @@ SELECT EXISTS (
-- here that determine if a node is public is specific
-- to the V2 gossip protocol.
WHERE c.version = 2
- AND c.signature IS NOT NULL
+ AND COALESCE(length(c.signature), 0) > 0
AND n.pub_key = $1
UNION ALL
diff --git a/sqldb/sqlc/queries/graph.sql b/sqldb/sqlc/queries/graph.sql
index 1817fbf..0ad71f8 100644
--- a/sqldb/sqlc/queries/graph.sql
+++ b/sqldb/sqlc/queries/graph.sql
@@ -101,14 +101,14 @@ SELECT EXISTS (
-- one of the signatures since we only ever set them
-- together.
WHERE c.version = 1
- AND c.bitcoin_1_signature IS NOT NULL
+ AND COALESCE(length(c.bitcoin_1_signature), 0) > 0
AND n.pub_key = $1
UNION ALL
SELECT 1
FROM graph_channels c
JOIN graph_nodes n ON n.id = c.node_id_2
WHERE c.version = 1
- AND c.bitcoin_1_signature IS NOT NULL
+ AND COALESCE(length(c.bitcoin_1_signature), 0) > 0
AND n.pub_key = $1
);
@@ -121,7 +121,7 @@ SELECT EXISTS (
-- here that determine if a node is public is specific
-- to the V2 gossip protocol.
WHERE c.version = 2
- AND c.signature IS NOT NULL
+ AND COALESCE(length(c.signature), 0) > 0
AND n.pub_key = $1
UNION ALL
@@ -251,7 +251,7 @@ WHERE last_update >= @start_time
SELECT 1
FROM graph_channels c
WHERE c.version = 1
- AND c.bitcoin_1_signature IS NOT NULL
+ AND COALESCE(length(c.bitcoin_1_signature), 0) > 0
AND (c.node_id_1 = graph_nodes.id OR c.node_id_2 = graph_nodes.id)
)
)
@@ -731,7 +731,7 @@ WHERE c.version = $1
-- name: GetPublicV1ChannelsBySCID :many
SELECT *
FROM graph_channels
-WHERE node_1_signature IS NOT NULL
+WHERE COALESCE(length(node_1_signature), 0) > 0
AND scid >= @start_scid
AND scid < @end_scid;
@@ -1217,4 +1217,4 @@ ON CONFLICT (channel_id, node_id, version)
message_flags = EXCLUDED.message_flags,
channel_flags = EXCLUDED.channel_flags,
signature = EXCLUDED.signature
-RETURNING id;
\ No newline at end of file
+RETURNING id;
Why this scored 51/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.