diff --git a/core/src/main/java/bisq/core/dao/node/messages/GetBlocksResponse.java b/core/src/main/java/bisq/core/dao/node/messages/GetBlocksResponse.java index 87f814fdee6..8a65ab6979e 100644 --- a/core/src/main/java/bisq/core/dao/node/messages/GetBlocksResponse.java +++ b/core/src/main/java/bisq/core/dao/node/messages/GetBlocksResponse.java @@ -57,13 +57,15 @@ private GetBlocksResponse(List blocks, int requestNonce, int messageVe @Override public protobuf.NetworkEnvelope toProtoNetworkEnvelope() { - return getNetworkEnvelopeBuilder() + protobuf.NetworkEnvelope proto = getNetworkEnvelopeBuilder() .setGetBlocksResponse(protobuf.GetBlocksResponse.newBuilder() .addAllRawBlocks(blocks.stream() .map(RawBlock::toProtoMessage) .collect(Collectors.toList())) .setRequestNonce(requestNonce)) .build(); + log.info("Sending a GetBlocksResponse with {} kB", proto.getSerializedSize() / 1000d); + return proto; } public static NetworkEnvelope fromProto(protobuf.GetBlocksResponse proto, int messageVersion) { diff --git a/core/src/main/java/bisq/core/filter/FilterManager.java b/core/src/main/java/bisq/core/filter/FilterManager.java index 5104929f8fc..86687d92c37 100644 --- a/core/src/main/java/bisq/core/filter/FilterManager.java +++ b/core/src/main/java/bisq/core/filter/FilterManager.java @@ -136,7 +136,10 @@ public void onAllServicesInitialized() { public void onAdded(ProtectedStorageEntry data) { if (data.getProtectedStoragePayload() instanceof Filter) { Filter filter = (Filter) data.getProtectedStoragePayload(); - addFilter(filter); + boolean wasValid = addFilter(filter); + if (!wasValid) { + UserThread.runAfter(() -> p2PService.getP2PDataStorage().removeInvalidProtectedStorageEntry(data), 1); + } } } @@ -203,7 +206,7 @@ private void resetFilters() { filterProperty.set(null); } - private void addFilter(Filter filter) { + private boolean addFilter(Filter filter) { if (verifySignature(filter)) { // Seed nodes are requested at startup before we get the filter so we only apply the banned // nodes at the next startup and don't update the list in the P2P network domain. @@ -223,6 +226,9 @@ private void addFilter(Filter filter) { if (filter.isPreventPublicBtcNetwork() && preferences.getBitcoinNodesOptionOrdinal() == BtcNodes.BitcoinNodesOption.PUBLIC.ordinal()) preferences.setBitcoinNodesOptionOrdinal(BtcNodes.BitcoinNodesOption.PROVIDED.ordinal()); + return true; + } else { + return false; } } diff --git a/p2p/src/main/java/bisq/network/p2p/peers/getdata/GetDataRequestHandler.java b/p2p/src/main/java/bisq/network/p2p/peers/getdata/GetDataRequestHandler.java index d6e2259b510..0972cf32c8c 100644 --- a/p2p/src/main/java/bisq/network/p2p/peers/getdata/GetDataRequestHandler.java +++ b/p2p/src/main/java/bisq/network/p2p/peers/getdata/GetDataRequestHandler.java @@ -41,6 +41,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; @@ -50,6 +51,7 @@ @Slf4j public class GetDataRequestHandler { private static final long TIMEOUT = 90; + private static final int MAX_ENTRIES = 10000; /////////////////////////////////////////////////////////////////////////////////////////// @@ -136,27 +138,54 @@ public void onFailure(@NotNull Throwable throwable) { private Set getFilteredPersistableNetworkPayload(GetDataRequest getDataRequest, Connection connection) { - final Set tempLookupSet = new HashSet<>(); - Set excludedKeysAsByteArray = P2PDataStorage.ByteArray.convertBytesSetToByteArraySet(getDataRequest.getExcludedKeys()); + Set tempLookupSet = new HashSet<>(); + String connectionInfo = "connectionInfo" + connection.getPeersNodeAddressOptional() + .map(e -> "node address " + e.getFullAddress()) + .orElseGet(() -> "connection UID " + connection.getUid()); - return dataStorage.getAppendOnlyDataStoreMap().entrySet().stream() + Set excludedKeysAsByteArray = P2PDataStorage.ByteArray.convertBytesSetToByteArraySet(getDataRequest.getExcludedKeys()); + AtomicInteger maxSize = new AtomicInteger(MAX_ENTRIES); + Set result = dataStorage.getAppendOnlyDataStoreMap().entrySet().stream() .filter(e -> !excludedKeysAsByteArray.contains(e.getKey())) + .filter(e -> maxSize.decrementAndGet() >= 0) .map(Map.Entry::getValue) - .filter(payload -> (connection.noCapabilityRequiredOrCapabilityIsSupported(payload))) - .filter(payload -> tempLookupSet.add(new P2PDataStorage.ByteArray(payload.getHash()))) + .filter(connection::noCapabilityRequiredOrCapabilityIsSupported) + .filter(payload -> { + boolean notContained = tempLookupSet.add(new P2PDataStorage.ByteArray(payload.getHash())); + return notContained; + }) .collect(Collectors.toSet()); + if (maxSize.get() <= 0) { + log.warn("The getData request from peer with {} caused too much PersistableNetworkPayload " + + "entries to get delivered. We limited the entries for the response to {} entries", + connectionInfo, MAX_ENTRIES); + } + log.info("The getData request from peer with {} contains {} PersistableNetworkPayload entries ", + connectionInfo, result.size()); + return result; } private Set getFilteredProtectedStorageEntries(GetDataRequest getDataRequest, Connection connection) { - final Set filteredDataSet = new HashSet<>(); - final Set lookupSet = new HashSet<>(); + Set filteredDataSet = new HashSet<>(); + Set lookupSet = new HashSet<>(); + String connectionInfo = "connectionInfo" + connection.getPeersNodeAddressOptional() + .map(e -> "node address " + e.getFullAddress()) + .orElseGet(() -> "connection UID " + connection.getUid()); + AtomicInteger maxSize = new AtomicInteger(MAX_ENTRIES); Set excludedKeysAsByteArray = P2PDataStorage.ByteArray.convertBytesSetToByteArraySet(getDataRequest.getExcludedKeys()); Set filteredSet = dataStorage.getMap().entrySet().stream() .filter(e -> !excludedKeysAsByteArray.contains(e.getKey())) + .filter(e -> maxSize.decrementAndGet() >= 0) .map(Map.Entry::getValue) .collect(Collectors.toSet()); + if (maxSize.get() <= 0) { + log.warn("The getData request from peer with {} caused too much ProtectedStorageEntry " + + "entries to get delivered. We limited the entries for the response to {} entries", + connectionInfo, MAX_ENTRIES); + } + log.info("getFilteredProtectedStorageEntries " + filteredSet.size()); for (ProtectedStorageEntry protectedStorageEntry : filteredSet) { final ProtectedStoragePayload protectedStoragePayload = protectedStorageEntry.getProtectedStoragePayload(); @@ -171,11 +200,14 @@ private Set getFilteredProtectedStorageEntries(GetDataReq doAdd = true; } if (doAdd) { - if (lookupSet.add(protectedStoragePayload.hashCode())) + boolean notContained = lookupSet.add(protectedStoragePayload.hashCode()); + if (notContained) filteredDataSet.add(protectedStorageEntry); } } + log.info("The getData request from peer with {} contains {} ProtectedStorageEntry entries ", + connectionInfo, filteredDataSet.size()); return filteredDataSet; } diff --git a/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetDataResponse.java b/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetDataResponse.java index fce89a42d10..0386d1abe1b 100644 --- a/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetDataResponse.java +++ b/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetDataResponse.java @@ -111,14 +111,14 @@ public protobuf.NetworkEnvelope toProtoNetworkEnvelope() { protobuf.NetworkEnvelope proto = getNetworkEnvelopeBuilder() .setGetDataResponse(builder) .build(); - log.info("Sending a GetDataResponse with size = {} bytes", proto.toByteArray().length); + log.info("Sending a GetDataResponse with {} kB", proto.getSerializedSize() / 1000d); return proto; } public static GetDataResponse fromProto(protobuf.GetDataResponse proto, NetworkProtoResolver resolver, int messageVersion) { - log.info("Received a GetDataResponse with size = {} bytes", proto.toByteArray().length); + log.info("Received a GetDataResponse with {} kB", proto.getSerializedSize() / 1000d); Set dataSet = new HashSet<>( proto.getDataSetList().stream() .map(entry -> (ProtectedStorageEntry) resolver.fromProto(entry)) diff --git a/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetUpdatedDataRequest.java b/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetUpdatedDataRequest.java index 0b5031e5495..eae85f6edef 100644 --- a/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetUpdatedDataRequest.java +++ b/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/GetUpdatedDataRequest.java @@ -81,12 +81,12 @@ public protobuf.NetworkEnvelope toProtoNetworkEnvelope() { NetworkEnvelope proto = getNetworkEnvelopeBuilder() .setGetUpdatedDataRequest(builder) .build(); - log.info("Sending a GetUpdatedDataRequest with size = {} bytes", proto.toByteArray().length); + log.info("Sending a GetUpdatedDataRequest with {} kB", proto.getSerializedSize() / 1000d); return proto; } public static GetUpdatedDataRequest fromProto(protobuf.GetUpdatedDataRequest proto, int messageVersion) { - log.info("Received a GetUpdatedDataRequest with size = {} bytes", proto.toByteArray().length); + log.info("Received a GetUpdatedDataRequest with {} kB", proto.getSerializedSize() / 1000d); return new GetUpdatedDataRequest(NodeAddress.fromProto(proto.getSenderNodeAddress()), proto.getNonce(), ProtoUtil.byteSetFromProtoByteStringList(proto.getExcludedKeysList()), diff --git a/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/PreliminaryGetDataRequest.java b/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/PreliminaryGetDataRequest.java index ac8b4c39342..0702b0f1756 100644 --- a/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/PreliminaryGetDataRequest.java +++ b/p2p/src/main/java/bisq/network/p2p/peers/getdata/messages/PreliminaryGetDataRequest.java @@ -79,12 +79,12 @@ public protobuf.NetworkEnvelope toProtoNetworkEnvelope() { NetworkEnvelope proto = getNetworkEnvelopeBuilder() .setPreliminaryGetDataRequest(builder) .build(); - log.info("Sending a PreliminaryGetDataRequest with size = {} bytes", proto.toByteArray().length); + log.info("Sending a PreliminaryGetDataRequest with {} kB", proto.getSerializedSize() / 1000d); return proto; } public static PreliminaryGetDataRequest fromProto(protobuf.PreliminaryGetDataRequest proto, int messageVersion) { - log.info("Received a PreliminaryGetDataRequest with size = {} bytes", proto.toByteArray().length); + log.info("Received a PreliminaryGetDataRequest with {} kB", proto.getSerializedSize() / 1000d); Capabilities supportedCapabilities = proto.getSupportedCapabilitiesList().isEmpty() ? null : Capabilities.fromIntList(proto.getSupportedCapabilitiesList()); diff --git a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java index f01157f3fcc..847277f43d2 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java @@ -504,6 +504,38 @@ && checkSignature(protectedStorageEntry) return result; } + + /** + * This method must be called only from client code not from network messages! We omit the ownership checks + * so we must apply it only if it comes from our trusted application code. It is used from client code which detects + * that the domain object violates specific domain rules. + * We could make it more generic by adding an Interface with a generic validation method. + * + * @param protectedStorageEntry The entry to be removed + */ + public void removeInvalidProtectedStorageEntry(ProtectedStorageEntry protectedStorageEntry) { + log.warn("We remove an invalid protectedStorageEntry: {}", protectedStorageEntry); + ProtectedStoragePayload protectedStoragePayload = protectedStorageEntry.getProtectedStoragePayload(); + ByteArray hashOfPayload = get32ByteHashAsByteArray(protectedStoragePayload); + + if (!map.containsKey(hashOfPayload)) { + return; + } + + doRemoveProtectedExpirableData(protectedStorageEntry, hashOfPayload); + removeFromProtectedDataStore(protectedStorageEntry); + + // We do not update the sequence number as that method is only called if we have received an invalid + // protectedStorageEntry from a previous add operation. + + // We do not call maybeAddToRemoveAddOncePayloads to avoid that an invalid object might block a valid object + // which we might receive in future (could be potential attack). + + // We do not broadcast as this is a local operation only to avoid our maps get polluted with invalid objects + // and as we do not check for ownership a node would not accept such a procedure if it would come from untrusted + // source (network). + } + private void removeFromProtectedDataStore(ProtectedStorageEntry protectedStorageEntry) { ProtectedStoragePayload protectedStoragePayload = protectedStorageEntry.getProtectedStoragePayload(); if (protectedStoragePayload instanceof PersistablePayload) {