* [bitcoin-dev] BIP151 protocol incompatibility
@ 2017-02-13 5:18 Eric Voskuil
2017-02-13 8:47 ` Pieter Wuille
0 siblings, 1 reply; 13+ messages in thread
From: Eric Voskuil @ 2017-02-13 5:18 UTC (permalink / raw)
To: Bitcoin Protocol Discussion, libbitcoin
[-- Attachment #1: Type: text/plain, Size: 546 bytes --]
The BIP151 proposal states:
> 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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [bitcoin-dev] BIP151 protocol incompatibility 2017-02-13 5:18 [bitcoin-dev] BIP151 protocol incompatibility Eric Voskuil @ 2017-02-13 8:47 ` Pieter Wuille 2017-02-13 9:36 ` Eric Voskuil 0 siblings, 1 reply; 13+ messages in thread From: Pieter Wuille @ 2017-02-13 8:47 UTC (permalink / raw) To: Bitcoin Dev, Eric Voskuil; +Cc: libbitcoin [-- Attachment #1: Type: text/plain, Size: 1158 bytes --] On Feb 12, 2017 23:58, "Eric Voskuil via bitcoin-dev" < bitcoin-dev@lists•linuxfoundation.org> wrote: The BIP151 proposal states: > 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 [-- Attachment #2: Type: text/html, Size: 1833 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [bitcoin-dev] BIP151 protocol incompatibility 2017-02-13 8:47 ` Pieter Wuille @ 2017-02-13 9:36 ` Eric Voskuil 2017-02-13 10:07 ` Jonas Schnelli 2017-02-13 10:16 ` Matt Corallo 0 siblings, 2 replies; 13+ messages in thread From: Eric Voskuil @ 2017-02-13 9:36 UTC (permalink / raw) To: Pieter Wuille, Bitcoin Dev; +Cc: libbitcoin [-- Attachment #1: Type: text/plain, Size: 2944 bytes --] On 02/13/2017 12:47 AM, Pieter Wuille wrote: > On Feb 12, 2017 23:58, "Eric Voskuil via bitcoin-dev" > <bitcoin-dev@lists•linuxfoundation.org wrote: > > The BIP151 proposal states: > > > 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 [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [bitcoin-dev] BIP151 protocol incompatibility 2017-02-13 9:36 ` Eric Voskuil @ 2017-02-13 10:07 ` Jonas Schnelli 2017-02-13 10:30 ` Eric Voskuil 2017-02-13 10:16 ` Matt Corallo 1 sibling, 1 reply; 13+ messages in thread From: Jonas Schnelli @ 2017-02-13 10:07 UTC (permalink / raw) To: bitcoin-dev [-- Attachment #1.1: Type: text/plain, Size: 1225 bytes --] > 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? > > 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> [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [bitcoin-dev] BIP151 protocol incompatibility 2017-02-13 10:07 ` Jonas Schnelli @ 2017-02-13 10:30 ` Eric Voskuil 2017-02-13 11:14 ` Jonas Schnelli 0 siblings, 1 reply; 13+ messages in thread From: Eric Voskuil @ 2017-02-13 10:30 UTC (permalink / raw) To: Jonas Schnelli, Bitcoin Protocol Discussion [-- Attachment #1: Type: text/plain, Size: 2960 bytes --] On 02/13/2017 02:07 AM, Jonas Schnelli via bitcoin-dev wrote: >> 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? 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. > 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. > 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). >> 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. > 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. > Why would your implementation how you threat unknown messages be different for messages specified in BIP151? Because it properly validates the protocol. More than that it supports a configurable protocol range. So by setting the min protocol (below which the node won't connect) and the max protocol (at which it desires to connect) we can observe the behavior of the network at any protocol levels (currently between 31402 and 70013). This is very helpful for a development stack as it allows one to easily test against each protocol level that one wishes to support. e [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [bitcoin-dev] BIP151 protocol incompatibility 2017-02-13 10:30 ` Eric Voskuil @ 2017-02-13 11:14 ` Jonas Schnelli 2017-02-14 19:54 ` Eric Voskuil 0 siblings, 1 reply; 13+ messages in thread From: Jonas Schnelli @ 2017-02-13 11:14 UTC (permalink / raw) To: Eric Voskuil, Bitcoin Protocol Discussion [-- Attachment #1.1: Type: text/plain, Size: 4579 bytes --] >> 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. > >> 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. > >> 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? > >>> 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. > >> 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. > >> 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> [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [bitcoin-dev] BIP151 protocol incompatibility 2017-02-13 11:14 ` Jonas Schnelli @ 2017-02-14 19:54 ` Eric Voskuil 2017-02-14 20:58 ` Jonas Schnelli 0 siblings, 1 reply; 13+ messages in thread From: Eric Voskuil @ 2017-02-14 19:54 UTC (permalink / raw) To: Jonas Schnelli, Bitcoin Protocol Discussion, libbitcoin [-- Attachment #1: Type: text/plain, Size: 8857 bytes --] On 02/13/2017 03:14 AM, Jonas Schnelli wrote: >>> 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. > 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. > 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. > 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). >>> 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. >>> 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. > 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. > I can't see that a such spec. would reduce the DOS attack vector? This was previously addressed (immediately below). >>>> 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. >>> 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. 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. >>> 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. > 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. > 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 [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [bitcoin-dev] BIP151 protocol incompatibility 2017-02-14 19:54 ` Eric Voskuil @ 2017-02-14 20:58 ` Jonas Schnelli 0 siblings, 0 replies; 13+ messages in thread From: Jonas Schnelli @ 2017-02-14 20:58 UTC (permalink / raw) To: Eric Voskuil, Bitcoin Protocol Discussion, libbitcoin [-- Attachment #1.1: Type: text/plain, Size: 7515 bytes --] >> 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? >> 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). > >> 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. >> 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. 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. > >>>> 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 > >> 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. >> 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. > >>>>> 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. > >> 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> [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [bitcoin-dev] BIP151 protocol incompatibility 2017-02-13 9:36 ` Eric Voskuil 2017-02-13 10:07 ` Jonas Schnelli @ 2017-02-13 10:16 ` Matt Corallo 2017-02-13 10:54 ` Eric Voskuil 1 sibling, 1 reply; 13+ messages in thread From: Matt Corallo @ 2017-02-13 10:16 UTC (permalink / raw) To: Eric Voskuil, Bitcoin Protocol Discussion, Eric Voskuil via bitcoin-dev, Pieter Wuille, Bitcoin Dev Cc: libbitcoin 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. On February 13, 2017 10:36:21 AM GMT+01:00, Eric Voskuil via bitcoin-dev <bitcoin-dev@lists•linuxfoundation.org> wrote: >On 02/13/2017 12:47 AM, Pieter Wuille wrote: >> On Feb 12, 2017 23:58, "Eric Voskuil via bitcoin-dev" >> <bitcoin-dev@lists•linuxfoundation.org wrote: >> >> The BIP151 proposal states: >> >> > 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [bitcoin-dev] BIP151 protocol incompatibility 2017-02-13 10:16 ` Matt Corallo @ 2017-02-13 10:54 ` Eric Voskuil 2017-02-13 11:11 ` Matt Corallo 0 siblings, 1 reply; 13+ messages in thread From: Eric Voskuil @ 2017-02-13 10:54 UTC (permalink / raw) To: Matt Corallo, Bitcoin Protocol Discussion, Pieter Wuille; +Cc: libbitcoin [-- Attachment #1: Type: text/plain, Size: 1224 bytes --] On 02/13/2017 02:16 AM, Matt Corallo wrote: > 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. Only if the peer is at the protocol level that allows the message: compact blocks: https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L217-L242 fee filter: https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L211-L216 send headers: https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L204-L210 filters: https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L170-L196 > 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 [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [bitcoin-dev] BIP151 protocol incompatibility 2017-02-13 10:54 ` Eric Voskuil @ 2017-02-13 11:11 ` Matt Corallo 2017-02-13 11:17 ` Eric Voskuil 0 siblings, 1 reply; 13+ messages in thread From: Matt Corallo @ 2017-02-13 11:11 UTC (permalink / raw) To: Eric Voskuil, Bitcoin Protocol Discussion, Pieter Wuille; +Cc: libbitcoin 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. On February 13, 2017 11:54:23 AM GMT+01:00, Eric Voskuil <eric@voskuil•org> wrote: >On 02/13/2017 02:16 AM, Matt Corallo wrote: >> 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. > >Only if the peer is at the protocol level that allows the message: > >compact blocks: > >https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L217-L242 > >fee filter: > >https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L211-L216 > >send headers: > >https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L204-L210 > >filters: > >https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L170-L196 > >> 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [bitcoin-dev] BIP151 protocol incompatibility 2017-02-13 11:11 ` Matt Corallo @ 2017-02-13 11:17 ` Eric Voskuil 2017-02-13 13:04 ` Matt Corallo 0 siblings, 1 reply; 13+ messages in thread From: Eric Voskuil @ 2017-02-13 11:17 UTC (permalink / raw) To: Matt Corallo, Bitcoin Protocol Discussion, Pieter Wuille; +Cc: libbitcoin [-- Attachment #1: Type: text/plain, Size: 2224 bytes --] On 02/13/2017 03:11 AM, Matt Corallo wrote: > 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. > 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 > On February 13, 2017 11:54:23 AM GMT+01:00, Eric Voskuil <eric@voskuil•org> wrote: >> On 02/13/2017 02:16 AM, Matt Corallo wrote: >>> 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. >> >> Only if the peer is at the protocol level that allows the message: >> >> compact blocks: >> >> https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L217-L242 >> >> fee filter: >> >> https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L211-L216 >> >> send headers: >> >> https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L204-L210 >> >> filters: >> >> https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L170-L196 >> >>> 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 [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [bitcoin-dev] BIP151 protocol incompatibility 2017-02-13 11:17 ` Eric Voskuil @ 2017-02-13 13:04 ` Matt Corallo 0 siblings, 0 replies; 13+ messages in thread From: Matt Corallo @ 2017-02-13 13:04 UTC (permalink / raw) To: Eric Voskuil, Bitcoin Protocol Discussion, Pieter Wuille; +Cc: libbitcoin 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. On February 13, 2017 12:17:11 PM GMT+01:00, Eric Voskuil <eric@voskuil•org> wrote: >On 02/13/2017 03:11 AM, Matt Corallo wrote: >> 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. > >> 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 > >> On February 13, 2017 11:54:23 AM GMT+01:00, Eric Voskuil ><eric@voskuil•org> wrote: >>> On 02/13/2017 02:16 AM, Matt Corallo wrote: >>>> 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. >>> >>> Only if the peer is at the protocol level that allows the message: >>> >>> compact blocks: >>> >>> >https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L217-L242 >>> >>> fee filter: >>> >>> >https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L211-L216 >>> >>> send headers: >>> >>> >https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L204-L210 >>> >>> filters: >>> >>> >https://github.com/bitcoin/bitcoin/blob/master/src/protocol.h#L170-L196 >>> >>>> 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-02-14 20:59 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-13 5:18 [bitcoin-dev] BIP151 protocol incompatibility Eric Voskuil 2017-02-13 8:47 ` Pieter Wuille 2017-02-13 9:36 ` Eric Voskuil 2017-02-13 10:07 ` Jonas Schnelli 2017-02-13 10:30 ` Eric Voskuil 2017-02-13 11:14 ` Jonas Schnelli 2017-02-14 19:54 ` Eric Voskuil 2017-02-14 20:58 ` Jonas Schnelli 2017-02-13 10:16 ` Matt Corallo 2017-02-13 10:54 ` Eric Voskuil 2017-02-13 11:11 ` Matt Corallo 2017-02-13 11:17 ` Eric Voskuil 2017-02-13 13:04 ` Matt Corallo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox