In this commit, we fix `IsRoutable` to correctly reject IPv6 addresses
whose first 16-bit group is zero (i.e., in the `0000::/16` reserved
block per RFC 4291). Differential fuzzing between btcd and Bitcoin Core
revealed that addresses like `0:9881:8181:8181:fe00🅰️9e:9801` were
slipping through as routable when they shouldn't be.
We add a new `zero6Net` (`0000::/16`) definition alongside the existing
`zero4Net`, and introduce an `IsZero` helper that checks both. The /16
prefix width is intentional: the broader /8 reservation would
incorrectly catch allocated sub-ranges like `0064:ff9b::/96` (RFC 6052,
NAT64). We also carve out an exception for RFC 6145 translated IPv4
addresses (`::ffff:0:0:0/96`), which live within `0000::/16` but are
valid for routing.
Test coverage includes the original bug report address, various
zero-prefix IPv6 addresses, the RFC 6145 exclusion, and corresponding
`GroupKey` entries to confirm they land in the "unroutable" bucket.
Fixes#2431
This issue was discovered through differential fuzzing between btcd and
Bitcoin Core.
Add detection and filtering of IPv6 ORCHIDv2 addresses as defined by
RFC7343 (2001:20::/28). These addresses are overlay routable
cryptographic hash identifiers and should not be treated as routable
peer addresses, consistent with Bitcoin Core's behavior.
Changes:
- Add rfc7343Net variable for the 2001:20::/28 address block
- Add IsRFC7343() function to detect ORCHIDv2 addresses
- Update IsRoutable() to exclude RFC7343 addresses
- Add comprehensive test coverage for RFC7343 detection
- Fix incorrect error message in TestIPTypes (IsRFC1918 -> IsRFC6145)
Sometimes the tests in the addrmanager_internal_test will fail with:
"addrmanager_internal_test.go: expected to find 5 addresses, found 4"
This is because the generated address with randAddr() may not always
generate an address that's routable. If the address is not routable,
that address will not be added to the manager when calling AddAddress().
This bug is fixed by creating a new function routableRandAddr() that
always creates a routable random address.
The doc formatting changes introduced in the recent go version is
increasing the diff for all of the new commits. Formatting it all in
this commit will help the readability of future PRs by reducing the
diff.
This gives KnownAddress a sync.RWMutex so the exported methods may
safely access the na (*wire.NetAddress) and lastattempt fields.
The AddrManager is updated to lock the new KnownAddress mutex before
assigning to na or lastattempt.
The other KnownAddress fields are only accessed by AddrManager, using
its own Mutex for synchronization.
In this commit, we update the serialized version of the AddressManager
to also store each address' service bits. This allows an address'
service bits to be properly set when read from disk, which allows
external callers to reliably filter out addresses which do not support
their required services.
Since the service bits were not previously stored, the serialization
version needed to be bumped. A test has been added to test the behavior
of upgrading from version 1 to 2.
This exposes a new method named SetServices to the address manager which
can be used to update the services for a known address. This will be
useful to keep known services up to date.
Backported from Decred.
This commit modifies the logic within the block manager and service to
preferentially fetch transactions and blocks which include witness data
from fully upgraded peers.
Once the initial version handshake has completed, the server now tracks
which of the connected peers are witness enabled (they advertise
SFNodeWitness). From then on, if a peer is witness enabled, then btcd
will always request full witness data when fetching
transactions/blocks.
Replace assignments to individual fields of wire.NetAddress with
creating the entire object at once, as one would do if the type was
immutable.
In some places this replaces the creation of a NetAddress with a
high-precision timestamp with a call to a 'constructor' that converts
the timestamp to single second precision. For consistency, the tests
have also been changed to use single-precision timestamps.
Lastly, the number of allocations in readNetAddress have been reduced by
reading the services directly into the NetAddress instead of first into
a temporary variable.
addrmgr.GetAddress() had a parameter `class string` originally intended
to support looking up addresses according to some type of filter such as
IPv4, IPv6, and only those which support specific wire.ServiceFlags
(full nodes, nodes that support bloom filters, nodes that support
segwit, etc). But currently the parameter is unused and also has an
inappropriate type `string`.
If it would ever be used, it's easy to add back and should then get an
appropriate type such as something that allows bitflags to be set so
that the caller could request combinations such as peers that support
IPv6, are full nodes, and support bloom filters.
This makes two modifications based on the results of staticcheck.
The first is to use a simplified zero value for the time in the addrmgr
package tests.
The second is check any errors when opening a directory while attempting
to perform old version upgrades.
allAddr was being allocated with counters instead of the actual size
of the address map. This led to the possibility of including nils
in the returned slice, which resulted in a panic.
This is mostly a backport of some of the same modifications made in
Decred along with a few additional things cleaned up. In particular,
this updates the code to make use of the new chainhash package.
Also, since this required API changes anyways and the hash algorithm is
no longer tied specifically to SHA, all other functions throughout the
code base which had "Sha" in their name have been changed to Hash so
they are not incorrectly implying the hash algorithm.
The following is an overview of the changes:
- Remove the wire.ShaHash type
- Update all references to wire.ShaHash to the new chainhash.Hash type
- Rename the following functions and update all references:
- wire.BlockHeader.BlockSha -> BlockHash
- wire.MsgBlock.BlockSha -> BlockHash
- wire.MsgBlock.TxShas -> TxHashes
- wire.MsgTx.TxSha -> TxHash
- blockchain.ShaHashToBig -> HashToBig
- peer.ShaFunc -> peer.HashFunc
- Rename all variables that included sha in their name to include hash
instead
- Update for function name changes in other dependent packages such as
btcutil
- Update copyright dates on all modified files
- Update glide.lock file to use the required version of btcutil
This change reduces fingerprinting via timestamps in addr messages.
Previously, the last seen time for an address was updated when
certain protocol commands were received. Now, the last seen time
is set when the peer disconnects if the peer had sent a verack
message and was connected for more than 20 minutes.
This mimics Bitcoin Core commit:
9c2737901b5203f267d21d728019d64b46f1d9f3
Also, add additional sanity checking before updating the peer's
timestamp. These include:
- Do not mark a peer as connected if we never received
a version message.
- Disconnect a peer for sending a verack before btcd
sent a version
- Disconnect a peer for sending multiple verack's
This change was suggested as Countermeasure 2 in
Eclipse Attacks on Bitcoin's Peer-to-Peer Network, Ethan
Heilman, Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive
Report 2015/263. March 2015.
This mimics Bitcoin Core commit c6a63ceeb4956933588995bcf01dc3095aaeb1fc
This change was suggested as Countermeasure 2 in Eclipse Attacks on
Bitcoin's Peer-to-Peer Network, Ethan Heilman, Alison Kendler, Aviv
Zohar, Sharon Goldberg. ePrint Archive Report 2015/263. March 2015.
This mimics Bitcoin Core commit
f68ba3f67bd500a64fb8932c6b41924ddc31d76f
This change was suggested as Countermeasure 6 in
Eclipse Attacks on Bitcoin’s Peer-to-Peer Network, Ethan
Heilman, Alison Kendler, Aviv Zohar, Sharon Goldberg. ePrint Archive
Report 2015/263. March 2015.
This mimics Bitcoin Core commit 1d21ba2f5ecbf03086d0b65c4c4c80a39a94c2ee
As pointed out in #189, according to the Go documentation, a ticker must
be stopped to release associated resources. This commit adds a defer call
to stop two tickers there were previously not being stopped as well as
changes two others that were being stopped over to use defer so it's more
consistent.
The other ticker in ScheduleShutdown is replaced and already calls Stop
before replacing it, so it has not been modified.
Closes#189.
ok @jrick
