What the developer wrote
Authored by Bastien Teinturier
Allow reply-less pings (#3284)
BOLT1 says:
```
A node receiving a ping message:
- if num_pong_bytes is less than 65532:
- MUST respond by sending a pong message, with byteslen equal to num_pong_bytes.
- otherwise (num_pong_bytes is not less than 65532):
- MUST ignore the ping.
```
These reply-less pings are used to insert cover traffic to thwart
network-level traffic analysis. We wanted to allow them in #3278
but we missed this ping flood protection that needed to be updated.
We also had an off-by-one in the maximum pong length that
is now fixed.
What changed, and why it matters
This commit fixes two related bugs in how Eclair handles Lightning network 'ping' messages. First, it corrects an off-by-one error so pings asking for a 65532-byte reply are now ignored (matching the BOLT1 spec), instead of being answered. Second, it stops treating these 'reply-less' pings as a flood attack, so they no longer count toward the pending-ping limit that disconnects peers. The practical effect is that legitimate cover-traffic pings no longer risk getting a peer disconnected, and the protocol boundary now matches the specification.
Review whether the relaxed flood protection could allow an attacker to send a high volume of reply-less pings to consume bandwidth or CPU without triggering the disconnect threshold; consider rate-limiting reply-less pings separately if not already done. Otherwise, treat as a spec-compliance and hardening patch.
Security signals we found
Protocol compliance fix (BOLT1 ping/pong boundary)
Off-by-one error correction in length check
DoS/flood-protection logic adjustment
Cover-traffic feature support
Evidence from the diff
In TransportHandler.scala, the pending-ping counter is now only incremented when Ping.pongLength < 65532, exempting reply-less pings from the flood-protection disconnect logic. In PeerConnection.scala, the reply condition changes from pongLength <= 65532 to pongLength < 65532, aligning with BOLT1’s ‘less than 65532’ rule. A test is updated to verify that a ping with pongLength=65532 is forwarded but does not trigger disconnection, while a second normal ping still triggers termination. The commit also notes an off-by-one fix in the maximum acceptable pong length.
Changed components
eclair-core/src/main/scala/fr/acinq/eclair/crypto/TransportHandler.scalaeclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scalaeclair-core/src/test/scala/fr/acinq/eclair/crypto/TransportHandlerSpec.scalaInspect captured patch +18 / −11
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/crypto/TransportHandler.scala b/eclair-core/src/main/scala/fr/acinq/eclair/crypto/TransportHandler.scala
index 25e9277..262246c 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/crypto/TransportHandler.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/crypto/TransportHandler.scala
@@ -102,12 +102,15 @@ class TransportHandler(keyPair: KeyPair, rs: Option[ByteVector], connection: Act
case Attempt.Successful(DecodeResult(message, _)) =>
logMessage(message, "IN")
Monitoring.Metrics.MessageSize.withTag(Monitoring.Tags.MessageDirection, Monitoring.Tags.MessageDirections.IN).record(plaintext.size)
- if (message.isInstanceOf[Ping]) {
- pendingPings += 1
- if (pendingPings > 1) {
- // We will kill the connection anyway, no need to process remaining messages
- return Right(m)
- }
+ message match {
+ // Note that "reply-less pings" are allowed, when the pong length exceeds 65531.
+ case ping: Ping if ping.pongLength < 65532 =>
+ pendingPings += 1
+ if (pendingPings > 1) {
+ // We will kill the connection anyway, no need to process remaining messages
+ return Right(m)
+ }
+ case _ =>
}
listener ! message
m += (message -> (m.getOrElse(message, 0) + 1))
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala b/eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala
index 0083db4..12aa113 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala
@@ -245,7 +245,7 @@ class PeerConnection(keyPair: KeyPair, conf: PeerConnection.Conf, switchboard: A
case Event(ping@Ping(pongLength, _, _), d: ConnectedData) =>
d.transport ! TransportHandler.ReadAck(ping)
- if (pongLength <= 65532) {
+ if (pongLength < 65532) {
// See BOLT 1: we reply only if requested pong length is acceptable.
// Senders may use unacceptable pong length when they don't want a response (to generate cover traffic).
d.transport ! Pong(ByteVector.fill(pongLength)(0.toByte))
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/crypto/TransportHandlerSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/crypto/TransportHandlerSpec.scala
index 8f9adce..f9fb8bb 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/crypto/TransportHandlerSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/crypto/TransportHandlerSpec.scala
@@ -217,13 +217,17 @@ class TransportHandlerSpec extends TestKitBaseClass with AnyFunSuiteLike with Be
awaitCond(initiator.stateName == TransportHandler.Normal)
awaitCond(responder.stateName == TransportHandler.Normal)
- initiator.tell(Ping(1105, ByteVector("hello 1".getBytes)), probe1.ref)
- probe2.expectMsg(Ping(1105, ByteVector("hello 1".getBytes)))
+ initiator.tell(Ping(1105, ByteVector("this is a real ping".getBytes)), probe1.ref)
+ probe2.expectMsg(Ping(1105, ByteVector("this is a real ping".getBytes)))
- initiator.tell(Ping(1105, ByteVector("hello 2".getBytes)), probe1.ref)
- probe2.expectNoMessage()
+ initiator.tell(Ping(65532, ByteVector("this is a reply-less ping for cover traffic".getBytes)), probe1.ref)
+ probe2.expectMsg(Ping(65532, ByteVector("this is a reply-less ping for cover traffic".getBytes)))
probe1.watch(initiator)
+ probe1.expectNoMessage()
+
+ initiator.tell(Ping(1105, ByteVector("this is a ping flood".getBytes)), probe1.ref)
+ probe2.expectNoMessage()
probe1.expectTerminated(initiator)
probe1.watch(responder)
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.