mirrors/bisq
1
0
Fork 0
mirror of https://github.com/bisq-network/bisq.git synced 2025-02-23 15:00:30 +01:00
Code Issues Projects Releases Packages Wiki Activity

If a new received protectedStorageEntry is expired we do not store it

Browse source 
and do not broadcast.

It is unclear why we receive expired data (some are very old), but a
manipulated node might produce that and as it only removed at each
batch process running each minute to clean out expired data it still
could propagate. Is an attack vector also to flood the network with
outdated offers where the maker is likely not online.

Should fix https://github.com/bisq-network/bisq/issues/4026
This commit is contained in:
chimp1984 2020-04-29 18:37:41 -05:00
parent 92cd7ec5a6
commit d9628802cb
No known key found for this signature in database
GPG key ID: 9801B4EC591F90E3
2 changed files with 22 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
desktop/src/main/java/bisq/desktop/main/offer/offerbook/OfferBook.java
View file

@ -85,7 +85,7 @@ public class OfferBook {
.filter(item -> item.getOffer().getId().equals(offer.getId()))
.findAny();
if (candidateWithSameId.isPresent()) {
log.warn("We had an old offer in the list with the same Offer ID. Might be that the state or errorMessage was different. " +
log.warn("We had an old offer in the list with the same Offer ID. We remove the old one. " +
"old offerBookListItem={}, new offerBookListItem={}", candidateWithSameId.get(), offerBookListItem);
offerBookListItems.remove(candidateWithSameId.get());
}

36
p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java
View file

@ -38,10 +38,9 @@ import bisq.network.p2p.storage.messages.RemoveDataMessage;
import bisq.network.p2p.storage.messages.RemoveMailboxDataMessage;
import bisq.network.p2p.storage.payload.CapabilityRequiringPayload;
import bisq.network.p2p.storage.payload.DateTolerantPayload;
import bisq.network.p2p.storage.payload.ExpirablePayload;
import bisq.network.p2p.storage.payload.ProcessOncePersistableNetworkPayload;
import bisq.network.p2p.storage.payload.MailboxStoragePayload;
import bisq.network.p2p.storage.payload.PersistableNetworkPayload;
import bisq.network.p2p.storage.payload.ProcessOncePersistableNetworkPayload;
import bisq.network.p2p.storage.payload.ProtectedMailboxStorageEntry;
import bisq.network.p2p.storage.payload.ProtectedStorageEntry;
import bisq.network.p2p.storage.payload.ProtectedStoragePayload;
@ -76,7 +75,6 @@ import javax.inject.Inject;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Maps;
import java.security.KeyPair;
import java.security.PublicKey;
@ -215,7 +213,7 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
// PersistedStoragePayload items don't get removed, so we don't have an issue with the case that
// an object gets removed in between PreliminaryGetDataRequest and the GetUpdatedDataRequest and we would
// miss that event if we do not load the full set or use some delta handling.
Set<byte[]> excludedKeys =this.appendOnlyDataStoreService.getMap().keySet().stream()
Set<byte[]> excludedKeys = this.appendOnlyDataStoreService.getMap().keySet().stream()
.map(e -> e.bytes)
.collect(Collectors.toSet());
@ -248,7 +246,7 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
.filter(e -> limit.decrementAndGet() >= 0)
.map(Map.Entry::getValue)
.filter(networkPayload -> shouldTransmitPayloadToPeer(peerCapabilities,
objToPayload.apply(networkPayload)))
objToPayload.apply(networkPayload)))
.collect(Collectors.toSet());
if (limit.get() < 0)
@ -351,7 +349,7 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
}
} else {
// We don't broadcast here as we are only connected to the seed node and would be pointless
addPersistableNetworkPayload(e, sender,false, false, false);
addPersistableNetworkPayload(e, sender, false, false, false);
}
});
log.info("Processing {} persistableNetworkPayloads took {} ms.",
@ -374,7 +372,6 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
@VisibleForTesting
void removeExpiredEntries() {
log.trace("removeExpiredEntries");
// The moment when an object becomes expired will not be synchronous in the network and we could
// get add network_messages after the object has expired. To avoid repeated additions of already expired
// object when we get it sent from new peers, we don’t remove the sequence number from the map.
@ -382,8 +379,8 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
// is equal and not larger as expected.
ArrayList<Map.Entry<ByteArray, ProtectedStorageEntry>> toRemoveList =
map.entrySet().stream()
.filter(entry -> entry.getValue().isExpired(this.clock))
.collect(Collectors.toCollection(ArrayList::new));
.filter(entry -> entry.getValue().isExpired(this.clock))
.collect(Collectors.toCollection(ArrayList::new));
// Batch processing can cause performance issues, so do all of the removes first, then update the listeners
// to let them know about the removes.
@ -452,7 +449,7 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
map.values().stream()
.filter(protectedStorageEntry -> protectedStorageEntry.getProtectedStoragePayload() instanceof RequiresOwnerIsOnlinePayload)
.filter(protectedStorageEntry -> ((RequiresOwnerIsOnlinePayload) protectedStorageEntry.getProtectedStoragePayload()).getOwnerNodeAddress().equals(peersNodeAddress))
.forEach(protectedStorageEntry -> {
.forEach(protectedStorageEntry -> {
// We only set the data back by half of the TTL and remove the data only if is has
// expired after that back dating.
// We might get connection drops which are not caused by the node going offline, so
@ -567,9 +564,9 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
}
private boolean addProtectedStorageEntry(ProtectedStorageEntry protectedStorageEntry,
@Nullable NodeAddress sender,
@Nullable BroadcastHandler.Listener listener,
boolean allowBroadcast) {
@Nullable NodeAddress sender,
@Nullable BroadcastHandler.Listener listener,
boolean allowBroadcast) {
ProtectedStoragePayload protectedStoragePayload = protectedStorageEntry.getProtectedStoragePayload();
ByteArray hashOfPayload = get32ByteHashAsByteArray(protectedStoragePayload);
@ -580,6 +577,15 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
return false;
}
// To avoid that expired data get stored and broadcast we check early for expire date.
if (protectedStorageEntry.isExpired(clock)) {
log.warn("We received an expired protectedStorageEntry from ppper {}. getCreationTimeStamp={}, protectedStorageEntry={}",
sender != null ? sender.getFullAddress() : "sender is null",
new Date(protectedStorageEntry.getCreationTimeStamp()),
protectedStorageEntry);
return false;
}
ProtectedStorageEntry storedEntry = map.get(hashOfPayload);
// If we have seen a more recent operation for this payload and we have a payload locally, ignore it
@ -652,7 +658,7 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
// If we have seen a more recent operation for this payload, we ignore the current one
if(!hasSequenceNrIncreased(updatedEntry.getSequenceNumber(), hashOfPayload))
if (!hasSequenceNrIncreased(updatedEntry.getSequenceNumber(), hashOfPayload))
return false;
// Verify the updated ProtectedStorageEntry is well formed and valid for update
@ -723,7 +729,7 @@ public class P2PDataStorage implements MessageListener, ConnectionListener, Pers
}
return true;
}
}
/**