Discussion:
[bitcoin-dev] BIP151 protocol incompatibility
Eric Voskuil via bitcoin-dev
2017-02-13 05:18:41 UTC
Permalink
This proposal is backward compatible. Non-supporting peers will ignore
the encinit messages.

This statement is incorrect. Sending content that existing nodes do not
expect is clearly an incompatibility. An implementation that ignores
invalid content leaves itself wide open to DOS attacks. The version
handshake must be complete before the protocol level can be determined.
While it may be desirable for this change to precede the version
handshake it cannot be described as backward compatible.

e
Pieter Wuille via bitcoin-dev
2017-02-13 08:47:38 UTC
Permalink
On Feb 12, 2017 23:58, "Eric Voskuil via bitcoin-dev" <
This proposal is backward compatible. Non-supporting peers will ignore
the encinit messages.

This statement is incorrect. Sending content that existing nodes do not
expect is clearly an incompatibility. An implementation that ignores
invalid content leaves itself wide open to DOS attacks. The version
handshake must be complete before the protocol level can be determined.
While it may be desirable for this change to precede the version
handshake it cannot be described as backward compatible.


The worst possible effect of ignoring unknown messages is a waste of
downstream bandwidth. The same is already possible by being sent addr
messages.

Using the protocol level requires a strict linear progression of (allowed)
network protocol features, which I expect to become harder and harder to
maintain.

Using otherwise ignored messages for determining optional features is
elegant, simple and opens no new attack vectors. I think it's very much
preferable over continued increments of the protocol version.
--
Pieter
Eric Voskuil via bitcoin-dev
2017-02-13 09:36:21 UTC
Permalink
Post by Pieter Wuille via bitcoin-dev
On Feb 12, 2017 23:58, "Eric Voskuil via bitcoin-dev"
This proposal is backward compatible. Non-supporting peers will ignore
the encinit messages.
This statement is incorrect. Sending content that existing nodes do not
expect is clearly an incompatibility. An implementation that ignores
invalid content leaves itself wide open to DOS attacks. The version
handshake must be complete before the protocol level can be determined.
While it may be desirable for this change to precede the version
handshake it cannot be described as backward compatible.
The worst possible effect of ignoring unknown messages is a waste of
downstream bandwidth. The same is already possible by being sent addr
messages.
Using the protocol level requires a strict linear progression of
(allowed) network protocol features, which I expect to become harder and
harder to maintain.
Using otherwise ignored messages for determining optional features is
elegant, simple and opens no new attack vectors. I think it's very much
preferable over continued increments of the protocol version.
As I said, it *may* be desirable, but it is *not* backward compatible,
and you do not actually dispute that above.

There are other control messages that qualify as "optional messages" but
these are only sent if the peer is at a version to expect them -
explicit in their BIPs. All adopted BIPs to date have followed this
pattern. This is not the same and it is not helpful to imply that it is
just following that pattern.

As for DOS, waste of bandwidth is not something to be ignored. If a peer
is flooding a node with addr message the node can manage it because it
understands the semantics of addr messages. If a node is required to
allow any message that it cannot understand it has no recourse. It
cannot determine whether it is under attack or if the behavior is
correct and for proper continued operation must be ignored.

This approach breaks any implementation that validates traffic, which is
clearly correct behavior given the existence of the version handshake.
Your comments make it clear that this is a *change* in network behavior
- essentially abandoning the version handshake. Whether is is harder to
maintain is irrelevant to the question of whether it is a break with
existing protocol.

If you intend for the network to abandon the version handshake and/or
promote changes that break it I propose that you write up this new
behavior as a BIP and solicit community feedback. There are a lot of
devices connected to the network and it would be irresponsible to break
something as fundamental as the P2P protocol handshake because you have
a feeling it's going to be hard to maintain.

e
Jonas Schnelli via bitcoin-dev
2017-02-13 10:07:12 UTC
Permalink
Post by Eric Voskuil via bitcoin-dev
All adopted BIPs to date have followed this
pattern. This is not the same and it is not helpful to imply that it is
just following that pattern.
Look at feefilter BIP 133
(https://github.com/bitcoin/bips/blob/master/bip-0133.mediawiki#backward-compatibility)
or sendheaders BIP130
(https://github.com/bitcoin/bips/blob/master/bip-0130.mediawiki#backward-compatibility)
Isn't it the same there?
Once BIP151 is implemented, it would make sense to bump the protocol
version, but this needs to be done once this has been
implemented/deployed. Or do I make a mistake somewhere?
Post by Eric Voskuil via bitcoin-dev
As for DOS, waste of bandwidth is not something to be ignored. If a peer
is flooding a node with addr message the node can manage it because it
understands the semantics of addr messages. If a node is required to
allow any message that it cannot understand it has no recourse. It
cannot determine whether it is under attack or if the behavior is
correct and for proper continued operation must be ignored.
How do you threat any other not known message types? Any peer can send
you any type of message anytime. Why would your implementation how you
threat unknown messages be different for messages specified in BIP151?


</jonas>
Jonas Schnelli via bitcoin-dev
2017-02-13 11:14:01 UTC
Permalink
Post by Jonas Schnelli via bitcoin-dev
Look at feefilter BIP 133
(https://github.com/bitcoin/bips/blob/master/bip-0133.mediawiki#backward-compatibility)
or sendheaders BIP130
(https://github.com/bitcoin/bips/blob/master/bip-0130.mediawiki#backward-compatibility)
Isn't it the same there?
No. This is what I was referring to. These messages are enabled by
protocol version. If they are received by a node below the version at
which they are activated, they are unknown messages, implying an invalid
peer. The above messages cannot be sent until *after* the version is
negotiated. BIP151 violates this rule by allowing the new control
message to be sent *before* the version handshake.
This indeed is not ideal for compatibility checks, but increases security.
I could not find a protocol specification that said communication must
be terminated when messages are transmitted before the version handshake
has been done. I mostly looked into Bitcoin-Cores implementation (which
means also into BitcoinXT/UT, where this is allowed).

Also. BIP151 clearly says that the requesting peer needs to initiate the
encryption (encinit).
In case of light clients not supporting BIP151 connecting to peers
supporting BIP151, there should never be transmission of new message
types specified in BIP151.
Post by Jonas Schnelli via bitcoin-dev
Once BIP151 is implemented, it would make sense to bump the protocol
version, but this needs to be done once this has been
implemented/deployed.
There are already nodes out there breaking connections based on the BIP.
It could very likely be possible that the initial responding peer tries
to initiate a encryption session which would mean that BIP151 was not
implemented correctly.
Correct me if I'm wrong please.
Post by Jonas Schnelli via bitcoin-dev
Or do I make a mistake somewhere?
Yes, the ordering of the messages. New messages can only be added after
the handshake negotiates the higher version. Otherwise the handshake is
both irrelevant (as Pieter is implying) and broken (for all existing
protocol versions).
I could not find evidence of the protocol specification that would
forbid (=terminate connection) such messages and I think allowing
unknown-messages before the version handshake makes the protocol flexible.

Are there any reasons we should drop peers if they send us unknown, but
correctly formatted p2p packages (magic, checksum, etc.) before the
version handshake, ... but not drop them if we have received unknown
messages after the version handshake?

I can't see that a such spec. would reduce the DOS attack vector?
Post by Jonas Schnelli via bitcoin-dev
Post by Eric Voskuil via bitcoin-dev
As for DOS, waste of bandwidth is not something to be ignored. If a peer
is flooding a node with addr message the node can manage it because it
understands the semantics of addr messages. If a node is required to
allow any message that it cannot understand it has no recourse. It
cannot determine whether it is under attack or if the behavior is
correct and for proper continued operation must be ignored.
How do you threat any other not known message types?
You may be more familiar with non-validating peers. If a message type is
not known it is an invalid message and the peer is immediately dropped.
We started seeing early drops in handshakes with bcoin nodes because of
this issue.
If this had happened, it's very likely because the responding peer tried
to initiate a encryption session which is against BIP151 specs.
Post by Jonas Schnelli via bitcoin-dev
Any peer can send you any type of message anytime.
Sure, a peer can do what it wants. It can send photos. But I'm not sure
what makes you think it would be correct to maintain the connection when
an *invalid* message is received.
Check:
https://github.com/bitcoin/bitcoin/blob/a06ede9a138d0fb86b0de17c42b936d9fe6e2158/src/net_processing.cpp#L2595
I think it was a wise implementation decision to allow unknown (not
invalid) messages.
This had allowed us to deploy stuff like compact blocks, feefilter, etc.
without breaking backward compatibility.
IMO, without a such flexibility, the deployment complexity would be
irresponsible high without really solving the DOS problem.
Post by Jonas Schnelli via bitcoin-dev
Why would your implementation how you threat unknown messages be
different for messages specified in BIP151?
Because it properly validates the protocol.
For feefilter or compact block or sendheaders?
You can't link a (unimplemented) specification (improvement process) to
a protocol version before deployment. Or can you?
Once it has been widely deployed, we should set a protocol minversion
for BIP151, right.

</jonas>
Eric Voskuil via bitcoin-dev
2017-02-14 19:54:37 UTC
Permalink
Post by Jonas Schnelli via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
Look at feefilter BIP 133
(https://github.com/bitcoin/bips/blob/master/bip-0133.mediawiki#backward-compatibility)
or sendheaders BIP130
(https://github.com/bitcoin/bips/blob/master/bip-0130.mediawiki#backward-compatibility)
Isn't it the same there?
No. This is what I was referring to. These messages are enabled by
protocol version. If they are received by a node below the version at
which they are activated, they are unknown messages, implying an invalid
peer. The above messages cannot be sent until *after* the version is
negotiated. BIP151 violates this rule by allowing the new control
message to be sent *before* the version handshake.
This indeed is not ideal for compatibility checks, but increases security.
The issue I raised is that it is not backward compatible. It sounds like
you agree but consider it a fair trade. My suggesting was that the BIP
be updated to reflect the lack of compatibility.
Post by Jonas Schnelli via bitcoin-dev
I could not find a protocol specification that said communication must
be terminated when messages are transmitted before the version handshake
has been done.
It doesn't need to be specified, most of Bitcoin is unspecified. The
version handshake establishes the negotiated version. It is not possible
to determine if a message is of the negotiated version before the
version is negotiated. All messages apart from this one have followed
that rule.
Post by Jonas Schnelli via bitcoin-dev
Also. BIP151 clearly says that the requesting peer needs to initiate the
encryption (encinit).
An incoming connection will be dropped due to invalid protocol and
potentially banned depending on the implementation.
Post by Jonas Schnelli via bitcoin-dev
In case of light clients not supporting BIP151 connecting to peers
supporting BIP151, there should never be transmission of new message
types specified in BIP151.
Not working with peers not supporting BIP151 is the compatibility issue.
But it sort of seems the intent in this case is to rely on that
incompatibility (expecting connections to nonsupporting peers to fail as
opposed to negotiating).
Post by Jonas Schnelli via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
Once BIP151 is implemented, it would make sense to bump the protocol
version, but this needs to be done once this has been
implemented/deployed.
There are already nodes out there breaking connections based on the BIP.
It could very likely be possible that the initial responding peer tries
to initiate a encryption session which would mean that BIP151 was not
implemented correctly.
Correct me if I'm wrong please.
I did consider the possibility, but there's this:

"Encryption initialization must happen before sending any other messages
to the responding peer (encinit message after a version message must be
ignored)."

https://github.com/bitcoin/bips/blob/master/bip-0151.mediawiki#specification

The BIP does not define "responding" and "requesting" peers, but:

"A peer that supports encryption must accept encryption requests from
all peers... The responding peer accepts the encryption request by
sending a encack message."

This implies the requesting peer is the peer that sends the message. You
seem to be saying that the requesting peer is the one that initiated
the connection and the responding peer is the connection receiver. If
this is the case it should be more clearly documented. But in the
case I experienced the "requester" of an encrypted session was also
the "receiver" of the connection.
Post by Jonas Schnelli via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
Or do I make a mistake somewhere?
Yes, the ordering of the messages. New messages can only be added after
the handshake negotiates the higher version. Otherwise the handshake is
both irrelevant (as Pieter is implying) and broken (for all existing
protocol versions).
I could not find evidence of the protocol specification that would
forbid (=terminate connection) such messages and I think allowing
unknown-messages before the version handshake makes the protocol flexible.
Flexible is certainly one word for it. Another way to describe it is
dirty. Allowing invalid messages in a protocol encourages protocol
incompatibility. You end up with various implementations and eventually
have no way of knowing how they are impacted by changes. There could be
a range of peers inter-operating with the full network while running
their own sub-protocols. Given the network is public and strong
identification of peers is undesirable, the invalid messages would
reasonably just get sent to everyone. So over time, what is the
protocol? Due to certain "flexibility" it is already a hassle to
properly implement.
Post by Jonas Schnelli via bitcoin-dev
Are there any reasons we should drop peers if they send us unknown, but
correctly formatted p2p packages (magic, checksum, etc.) before the
version handshake, ... but not drop them if we have received unknown
messages after the version handshake?
There is no reason to treat invalid messages differently based on where
they occur in the communication. After the handshake the agreed version
is known to both peers. As a result there is never a reason for an
invalid message to be sent. Therefore it is always proper to drop a peer
that sends an invalid message.
Post by Jonas Schnelli via bitcoin-dev
I can't see that a such spec. would reduce the DOS attack vector?
This was previously addressed (immediately below).
Post by Jonas Schnelli via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
Post by Eric Voskuil via bitcoin-dev
As for DOS, waste of bandwidth is not something to be ignored. If a peer
is flooding a node with addr message the node can manage it because it
understands the semantics of addr messages. If a node is required to
allow any message that it cannot understand it has no recourse. It
cannot determine whether it is under attack or if the behavior is
correct and for proper continued operation must be ignored.
How do you threat any other not known message types?
You may be more familiar with non-validating peers. If a message type is
not known it is an invalid message and the peer is immediately dropped.
We started seeing early drops in handshakes with bcoin nodes because of
this issue.
If this had happened, it's very likely because the responding peer tried
to initiate a encryption session which is against BIP151 specs.
See above.
Post by Jonas Schnelli via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
Any peer can send you any type of message anytime.
Sure, a peer can do what it wants. It can send photos. But I'm not sure
what makes you think it would be correct to maintain the connection when
an *invalid* message is received.
https://github.com/bitcoin/bitcoin/blob/a06ede9a138d0fb86b0de17c42b936d9fe6e2158/src/net_processing.cpp#L2595
I think it was a wise implementation decision to allow unknown (not
invalid) messages.
This had allowed us to deploy stuff like compact blocks, feefilter, etc.
without breaking backward compatibility.
IMO, without a such flexibility, the deployment complexity would be
irresponsible high without really solving the DOS problem.
This is a misinterpretation. The failure to validate did not enable
anything except possibly some broken peers not getting dropped. None of
the protocol changes previously deployed require the older version peer
to allow invalid messages. While it may appear otherwise, due to a
particular implementation, it is never necessary to send a message to a
peer that the peer does not understand. The handshake gives each peer
the other peer's version. That obligates the newer peer to conform to
the older (or disconnect if the old is insufficient). That's the nature
of backward compatibility.
Post by Jonas Schnelli via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
Why would your implementation how you threat unknown messages be
different for messages specified in BIP151?
Because it properly validates the protocol.
For feefilter or compact block or sendheaders?
Yes, this is the purpose of version negotiation, which is why there are
version and verack messages. And this is also why, in the satoshi
client, two of the above messages are sent from the verack handler. The
feefilter message is sent dynamically but only if the peer's version
allows it.
Post by Jonas Schnelli via bitcoin-dev
You can't link a (unimplemented) specification (improvement process) to
a protocol version before deployment. Or can you?
I'm not sure I follow your question. The BIP should presumably declare a
version number if one is necessary.
Post by Jonas Schnelli via bitcoin-dev
Once it has been widely deployed, we should set a protocol minversion
for BIP151, right.
In general you should set a version before it's ever live on the
network. But if it precedes the protocol version negotiation the
protocol version number is moot.

I've been asked to throttle the discussion in the interest of reducing
list volume. I think the issue is pretty clearly addressed at this
point, but feel free to follow up directly and/or via the libbitcoin
development list (copied).

e
Jonas Schnelli via bitcoin-dev
2017-02-14 20:58:54 UTC
Permalink
Post by Eric Voskuil via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
This indeed is not ideal for compatibility checks, but increases security.
The issue I raised is that it is not backward compatible. It sounds like
you agree but consider it a fair trade. My suggesting was that the BIP
be updated to reflect the lack of compatibility.
It's still backward compatible. All (?) SPV clients and full node
implementation would still work if BIP151 has been implemented.
Isn't that backward compatibility?
Post by Eric Voskuil via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
I could not find a protocol specification that said communication must
be terminated when messages are transmitted before the version handshake
has been done.
It doesn't need to be specified, most of Bitcoin is unspecified. The
version handshake establishes the negotiated version. It is not possible
to determine if a message is of the negotiated version before the
version is negotiated. All messages apart from this one have followed
that rule.
Yes. But encryption negotiation must be done before the version
handshake (security).
Post by Eric Voskuil via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
Also. BIP151 clearly says that the requesting peer needs to initiate the
encryption (encinit).
An incoming connection will be dropped due to invalid protocol and
potentially banned depending on the implementation.
This is not true. If the connecting peer (assume the SPV client) will
not request encryption, the responding peer (the node) will not enforce
or ask for encryption.
This is clearly written in the BIP.
Post by Eric Voskuil via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
It could very likely be possible that the initial responding peer tries
to initiate a encryption session which would mean that BIP151 was not
implemented correctly.
Correct me if I'm wrong please.
"Encryption initialization must happen before sending any other messages
to the responding peer (encinit message after a version message must be
ignored)."
https://github.com/bitcoin/bips/blob/master/bip-0151.mediawiki#specification
"A peer that supports encryption must accept encryption requests from
all peers... The responding peer accepts the encryption request by
sending a encack message."
This implies the requesting peer is the peer that sends the message. You
seem to be saying that the requesting peer is the one that initiated
the connection and the responding peer is the connection receiver. If
this is the case it should be more clearly documented. But in the
case I experienced the "requester" of an encrypted session was also
the "receiver" of the connection.
I think the BIP makes this very clear. IMO you are trying to hide your
standpoint behind a wired interpretations of the BIP.

From the BIP:
«To request encrypted communication, the requesting peer generates an EC
ephemeral-session-keypair and sends an encinit message to the responding
peer and waits for a encack message. The responding node must do the
same encinit/encack interaction for the opposite communication direction.»

This seems to be pretty clear to me. You can interpret the "requesting
peer" and "responding peer" per message interaction. But then the whole
BIP would make no sense.

I'm happy if you can do a PR on the BIP that makes the wording better.
This would actually be a productive step.
Post by Eric Voskuil via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
Or do I make a mistake somewhere?
Yes, the ordering of the messages. New messages can only be added after
the handshake negotiates the higher version. Otherwise the handshake is
both irrelevant (as Pieter is implying) and broken (for all existing
protocol versions).
I could not find evidence of the protocol specification that would
forbid (=terminate connection) such messages and I think allowing
unknown-messages before the version handshake makes the protocol flexible.
Flexible is certainly one word for it. Another way to describe it is
dirty. Allowing invalid messages in a protocol encourages protocol
incompatibility. You end up with various implementations and eventually
have no way of knowing how they are impacted by changes. There could be
a range of peers inter-operating with the full network while running
their own sub-protocols. Given the network is public and strong
identification of peers is undesirable, the invalid messages would
reasonably just get sent to everyone. So over time, what is the
protocol? Due to certain "flexibility" it is already a hassle to
properly implement.
Then you would have to go after all BIPs deployed this way. This
argument has nothing to do with BIP151 it questions the whole new
protocol features deployment.
Again, check this code part:

https://github.com/bitcoin/bitcoin/blob/a06ede9a138d0fb86b0de17c42b936d9fe6e2158/src/net_processing.cpp#L2595
Post by Eric Voskuil via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
Are there any reasons we should drop peers if they send us unknown, but
correctly formatted p2p packages (magic, checksum, etc.) before the
version handshake, ... but not drop them if we have received unknown
messages after the version handshake?
There is no reason to treat invalid messages differently based on where
they occur in the communication. After the handshake the agreed version
is known to both peers. As a result there is never a reason for an
invalid message to be sent. Therefore it is always proper to drop a peer
that sends an invalid message.
That's up to the implementation. But the current flexibility exists
because we not drop.
Again, see above.
Post by Eric Voskuil via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
I can't see that a such spec. would reduce the DOS attack vector?
This was previously addressed (immediately below).
No. I'd like to hear from you why the DOS attack vector would be larger
if the encryption neg. would be after the version handshake.
Post by Eric Voskuil via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
Post by Eric Voskuil via bitcoin-dev
As for DOS, waste of bandwidth is not something to be ignored. If a peer
is flooding a node with addr message the node can manage it because it
understands the semantics of addr messages. If a node is required to
allow any message that it cannot understand it has no recourse. It
cannot determine whether it is under attack or if the behavior is
correct and for proper continued operation must be ignored.
How do you threat any other not known message types?
You may be more familiar with non-validating peers. If a message type is
not known it is an invalid message and the peer is immediately dropped.
We started seeing early drops in handshakes with bcoin nodes because of
this issue.
Yes, this is the purpose of version negotiation, which is why there are
version and verack messages. And this is also why, in the satoshi
client, two of the above messages are sent from the verack handler. The
feefilter message is sent dynamically but only if the peer's version
allows it.
Again. Encryption – for the sake of security – must be the first
interaction.
This is exceptional for BIP151 and I'd like to hear the real downsides
of doing that.
Post by Eric Voskuil via bitcoin-dev
Post by Jonas Schnelli via bitcoin-dev
You can't link a (unimplemented) specification (improvement process) to
a protocol version before deployment. Or can you?
I'm not sure I follow your question. The BIP should presumably declare a
version number if one is necessary.
What? You want to define protocol version number in draft improvement
specifications?
How should that be possible?
It's like defining a new HTML version number if you propose/draft a new
video streaming format.


</jonas>

Matt Corallo via bitcoin-dev
2017-02-13 10:16:13 UTC
Permalink
For the reasons Pieter listed, an explicit part of our version handshake and protocol negotiation is the exchange of otherwise-ignored messages to set up optional features.

Peers that do not support this ignore such messages, just as if they had indicated they wouldn't support it, see, eg BIP 152's handshake. Not sure why you consider this backwards incompatible, as I would say it's pretty clearly allowing old nodes to communicate just fine.
Post by Pieter Wuille via bitcoin-dev
On Feb 12, 2017 23:58, "Eric Voskuil via bitcoin-dev"
This proposal is backward compatible. Non-supporting peers will
ignore
Post by Pieter Wuille via bitcoin-dev
the encinit messages.
This statement is incorrect. Sending content that existing nodes
do not
Post by Pieter Wuille via bitcoin-dev
expect is clearly an incompatibility. An implementation that
ignores
Post by Pieter Wuille via bitcoin-dev
invalid content leaves itself wide open to DOS attacks. The
version
Post by Pieter Wuille via bitcoin-dev
handshake must be complete before the protocol level can be
determined.
Post by Pieter Wuille via bitcoin-dev
While it may be desirable for this change to precede the version
handshake it cannot be described as backward compatible.
The worst possible effect of ignoring unknown messages is a waste of
downstream bandwidth. The same is already possible by being sent addr
messages.
Using the protocol level requires a strict linear progression of
(allowed) network protocol features, which I expect to become harder
and
Post by Pieter Wuille via bitcoin-dev
harder to maintain.
Using otherwise ignored messages for determining optional features is
elegant, simple and opens no new attack vectors. I think it's very
much
Post by Pieter Wuille via bitcoin-dev
preferable over continued increments of the protocol version.
As I said, it *may* be desirable, but it is *not* backward compatible,
and you do not actually dispute that above.
There are other control messages that qualify as "optional messages" but
these are only sent if the peer is at a version to expect them -
explicit in their BIPs. All adopted BIPs to date have followed this
pattern. This is not the same and it is not helpful to imply that it is
just following that pattern.
As for DOS, waste of bandwidth is not something to be ignored. If a peer
is flooding a node with addr message the node can manage it because it
understands the semantics of addr messages. If a node is required to
allow any message that it cannot understand it has no recourse. It
cannot determine whether it is under attack or if the behavior is
correct and for proper continued operation must be ignored.
This approach breaks any implementation that validates traffic, which is
clearly correct behavior given the existence of the version handshake.
Your comments make it clear that this is a *change* in network behavior
- essentially abandoning the version handshake. Whether is is harder to
maintain is irrelevant to the question of whether it is a break with
existing protocol.
If you intend for the network to abandon the version handshake and/or
promote changes that break it I propose that you write up this new
behavior as a BIP and solicit community feedback. There are a lot of
devices connected to the network and it would be irresponsible to break
something as fundamental as the P2P protocol handshake because you have
a feeling it's going to be hard to maintain.
e
Matt Corallo via bitcoin-dev
2017-02-13 11:11:11 UTC
Permalink
I believe many, if not all, of those messages are sent irrespective of version number.

In any case, I fail to see how adding any additional messages which are ignored by old peers amounts to a lack of backward compatibility.
Post by Matt Corallo via bitcoin-dev
Post by Matt Corallo via bitcoin-dev
For the reasons Pieter listed, an explicit part of our version
handshake and protocol negotiation is the exchange of otherwise-ignored
messages to set up optional features.
https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L217-L242
https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L211-L216
https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L204-L210
https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L170-L196
Post by Matt Corallo via bitcoin-dev
Peers that do not support this ignore such messages, just as if they
had indicated they wouldn't support it, see, eg BIP 152's handshake.
Not
sure why you consider this backwards incompatible, as I would say it's
pretty clearly allowing old nodes to communicate just fine.
No, it is not the same as BIP152. Control messages apart from BIP151
are
not sent until *after* the version is negotiated.
I assume that BIP151 is different in this manner because it has a
desire
to negotiate encryption before any other communications, including
version.
e
Eric Voskuil via bitcoin-dev
2017-02-13 11:17:11 UTC
Permalink
Post by Matt Corallo via bitcoin-dev
I believe many, if not all, of those messages are sent irrespective of version number.
In the interest of perfect clarity, see your code:

https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L1372-L1403

Inside of the VERACK handler (i.e. after the handshake) there is a peer
version test before sending SENDCMPCT (and SENDHEADERS).

I have no idea where the fee filter message is sent, if it is sent at
all. But I have *never* seen any control messages arrive before the
handshake is complete.
Post by Matt Corallo via bitcoin-dev
In any case, I fail to see how adding any additional messages which
are ignored by old peers amounts to a lack of backward compatibility.

See preceding messages in this thread, I think it's pretty clearly
spelled out.

e
Post by Matt Corallo via bitcoin-dev
Post by Matt Corallo via bitcoin-dev
Post by Matt Corallo via bitcoin-dev
For the reasons Pieter listed, an explicit part of our version
handshake and protocol negotiation is the exchange of otherwise-ignored
messages to set up optional features.
https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L217-L242
https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L211-L216
https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L204-L210
https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L170-L196
Post by Matt Corallo via bitcoin-dev
Peers that do not support this ignore such messages, just as if they
had indicated they wouldn't support it, see, eg BIP 152's handshake.
Not
sure why you consider this backwards incompatible, as I would say it's
pretty clearly allowing old nodes to communicate just fine.
No, it is not the same as BIP152. Control messages apart from BIP151
are
not sent until *after* the version is negotiated.
I assume that BIP151 is different in this manner because it has a
desire
to negotiate encryption before any other communications, including
version.
e
Matt Corallo via bitcoin-dev
2017-02-13 13:04:15 UTC
Permalink
Sorry, I'm still missing it...
So your claim is that a) ignoring incoming messages of a type you do not recognize is bad, and thus b) we should be disconnecting/banning peers which send us messages we do not recognize (can you spell out why? Anyone is free to send your host address messages/transactions they are generating/etc/etc, we don't ban nodes for such messages, as that would be crazy - why should we ban a peer for sending us an extra 50 bytes which we ignore?), and thus c) this would be backwards incompatible with software which does not currently exist?

Usually "backwards incompatible" refers to breaking existing software, not breaking theoretical software. Note that, last I heard, BIP 151 is still a draft, if such software actually exists we can discuss changing it, but there are real wins in sending these messages before VERSION.
Post by Matt Corallo via bitcoin-dev
Post by Matt Corallo via bitcoin-dev
I believe many, if not all, of those messages are sent irrespective
of version number.
https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L1372-L1403
Inside of the VERACK handler (i.e. after the handshake) there is a peer
version test before sending SENDCMPCT (and SENDHEADERS).
I have no idea where the fee filter message is sent, if it is sent at
all. But I have *never* seen any control messages arrive before the
handshake is complete.
Post by Matt Corallo via bitcoin-dev
In any case, I fail to see how adding any additional messages which
are ignored by old peers amounts to a lack of backward compatibility.
See preceding messages in this thread, I think it's pretty clearly
spelled out.
e
Post by Matt Corallo via bitcoin-dev
On February 13, 2017 11:54:23 AM GMT+01:00, Eric Voskuil
Post by Matt Corallo via bitcoin-dev
Post by Matt Corallo via bitcoin-dev
For the reasons Pieter listed, an explicit part of our version
handshake and protocol negotiation is the exchange of
otherwise-ignored
Post by Matt Corallo via bitcoin-dev
Post by Matt Corallo via bitcoin-dev
messages to set up optional features.
https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L217-L242
https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L211-L216
https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L204-L210
https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L170-L196
Post by Matt Corallo via bitcoin-dev
Post by Matt Corallo via bitcoin-dev
Post by Matt Corallo via bitcoin-dev
Peers that do not support this ignore such messages, just as if
they
Post by Matt Corallo via bitcoin-dev
Post by Matt Corallo via bitcoin-dev
had indicated they wouldn't support it, see, eg BIP 152's handshake.
Not
sure why you consider this backwards incompatible, as I would say
it's
Post by Matt Corallo via bitcoin-dev
Post by Matt Corallo via bitcoin-dev
pretty clearly allowing old nodes to communicate just fine.
No, it is not the same as BIP152. Control messages apart from BIP151
are
not sent until *after* the version is negotiated.
I assume that BIP151 is different in this manner because it has a
desire
to negotiate encryption before any other communications, including
version.
e
Loading...