Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(13/13) [BUGFIX] Validate Entry.receiversPubKey when adding a MailboxPayload #3609

Merged
merged 45 commits into from Nov 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
de5ffd4
[BUGFIX] Don't try and remove() if addMailboxData() fails
julianknutsen Nov 3, 2019
5512f34
[REFACTOR] P2PDataStorage::addPersistableNetworkPayload()
julianknutsen Oct 26, 2019
f2f6399
[REFACTOR] P2PDataStorage::addProtectedStorageEntry()
julianknutsen Nov 4, 2019
ae50270
[REFACTOR] P2PDataStorage::refreshTTL()
julianknutsen Nov 4, 2019
a569852
[REFACTOR] P2PDataStorage::remove()
julianknutsen Nov 4, 2019
66e3ece
[REFACTOR] P2PDataStorage::removeMailboxData()
julianknutsen Nov 4, 2019
86c8c83
[PR COMMENTS] Clean up logging messages
julianknutsen Nov 5, 2019
c1ad6b4
Update behavior of P2PDataStorage::addProtectedStorageEntry() on dupl…
julianknutsen Nov 4, 2019
cbda653
Update behavior of P2PDataStorage::refreshTTL() on duplicates
julianknutsen Nov 4, 2019
e5f9261
Update behavior of P2PDataStorage::remove() & removeMailboxData() on …
julianknutsen Nov 4, 2019
de72d39
Use dependency injected Clock in P2PDataStore
julianknutsen Nov 6, 2019
4268003
[REFACTOR] Clean up ProtectedStorageEntry ctor
julianknutsen Nov 6, 2019
10eb9c0
Use Clock in ProtectedStorageEntry
julianknutsen Nov 7, 2019
89ad234
[TESTS] Use ClockFake in tests to control time
julianknutsen Nov 7, 2019
898d7fc
[TESTS] Test onBootstrapComplete()
julianknutsen Nov 7, 2019
454b2d7
[TESTS] Lower entries required for purge in tests
julianknutsen Nov 7, 2019
2c2a57e
[REFACTOR] Remove duplicated code in refreshTTL
julianknutsen Nov 8, 2019
54b4b4d
[TESTS] Add more tests around mis-wrapped ProtectedStorageEntrys
julianknutsen Nov 7, 2019
ebf33e2
[REFACTOR] ProtectedStorageEntry::validForAddOperation()
julianknutsen Nov 7, 2019
a631777
[REFACTOR] ProtectedStorageEntry::isValidForRemoveOperation()
julianknutsen Nov 7, 2019
217a321
[REFACTOR] Remove checkPublicKeys()
julianknutsen Nov 7, 2019
40337ff
Clean up toString() methods
julianknutsen Nov 8, 2019
9c7dc0c
[REFACTOR] Move signature validation behind isValidForAddOperation()
julianknutsen Nov 8, 2019
f915a03
[REFACTOR] Move signature validation behind isValidForRemoveOperation()
julianknutsen Nov 8, 2019
28a7bc8
[REFACTOR] Move receiversPubKey check behind isValidForRemoveOperation()
julianknutsen Nov 8, 2019
0c07883
Remove duplicate check in refreshTTL
julianknutsen Nov 8, 2019
289788e
Introduce isMetadataEquals and use it
julianknutsen Nov 8, 2019
53b5feb
[TESTS] Update remove validation with BroadcastMessage type
julianknutsen Nov 8, 2019
5ae9dd1
Combine remove() and removeMailboxData()
julianknutsen Nov 8, 2019
5d35d08
[PR COMMENTS] Logging format and function rename
julianknutsen Nov 13, 2019
2852148
[TESTS] Start deprecating integrations tests
julianknutsen Nov 9, 2019
26afd11
[TESTS] Remove dead tests and code
julianknutsen Nov 9, 2019
d980932
[DEAD CODE] Remove unused functions and imports
julianknutsen Nov 9, 2019
c652528
[DEAD CODE] Remove obsolete tests
julianknutsen Nov 9, 2019
eb2f8f3
[TESTS] Add JavaDocs for test objects
julianknutsen Nov 9, 2019
10384ac
[TESTS] Split monolithic P2PDataStoreTest into separate files
julianknutsen Nov 9, 2019
fa82c17
[TESTS] Clean up TestState static methods
julianknutsen Nov 9, 2019
fc55312
[TESTS] Make SavedTestState ctor private to TestState
julianknutsen Nov 9, 2019
14c78c1
[TESTS] Add missing license text to test files
julianknutsen Nov 9, 2019
c81f8a6
[TESTS] Rename P2PDataStoreTest
julianknutsen Nov 9, 2019
bdef1e4
Add payload safety checks in ProtectedStorageEntry
julianknutsen Nov 14, 2019
8ecb5b9
[REFACTOR] Clean up removeExpiredEntries
julianknutsen Nov 14, 2019
b10a603
[BUGFIX] Correctly remove PersistablePayload in onDisconnect path
julianknutsen Nov 14, 2019
9ffbcf7
[REFACTOR] Use common path for updating map/data store on remove
julianknutsen Nov 14, 2019
bdfe32b
[BUGFIX] Validate Entry.receiversPubKey for MailboxPayloads
julianknutsen Nov 14, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -20,11 +20,15 @@
import bisq.common.proto.ProtoResolver;
import bisq.common.proto.ProtobufferException;

import java.time.Clock;


public interface NetworkProtoResolver extends ProtoResolver {
NetworkEnvelope fromProto(protobuf.NetworkEnvelope proto) throws ProtobufferException;

NetworkPayload fromProto(protobuf.StoragePayload proto);

NetworkPayload fromProto(protobuf.StorageEntryWrapper proto);

Clock getClock();
}
6 changes: 6 additions & 0 deletions core/src/main/java/bisq/core/proto/CoreProtoResolver.java
Expand Up @@ -59,10 +59,16 @@
import bisq.common.proto.ProtobufferRuntimeException;
import bisq.common.proto.persistable.PersistableEnvelope;

import java.time.Clock;

import lombok.Getter;
import lombok.extern.slf4j.Slf4j;

@Slf4j
public class CoreProtoResolver implements ProtoResolver {
@Getter
protected Clock clock;

@Override
public PaymentAccountPayload fromProto(protobuf.PaymentAccountPayload proto) {
if (proto != null) {
Expand Down
Expand Up @@ -88,14 +88,17 @@
import javax.inject.Inject;
import javax.inject.Singleton;

import java.time.Clock;

import lombok.extern.slf4j.Slf4j;

// TODO Use ProtobufferException instead of ProtobufferRuntimeException
@Slf4j
@Singleton
public class CoreNetworkProtoResolver extends CoreProtoResolver implements NetworkProtoResolver {
@Inject
public CoreNetworkProtoResolver() {
public CoreNetworkProtoResolver(Clock clock) {
this.clock = clock;
}

@Override
Expand Down
6 changes: 4 additions & 2 deletions monitor/src/main/java/bisq/monitor/metric/P2PNetworkLoad.java
Expand Up @@ -49,6 +49,8 @@

import org.springframework.core.env.PropertySource;

import java.time.Clock;

import java.io.File;

import java.util.Collections;
Expand Down Expand Up @@ -118,7 +120,7 @@ protected void execute() {

// start the network node
networkNode = new TorNetworkNode(Integer.parseInt(configuration.getProperty(TOR_PROXY_PORT, "9053")),
new CoreNetworkProtoResolver(), false,
new CoreNetworkProtoResolver(Clock.systemDefaultZone()), false,
new AvailableTor(Monitor.TOR_WORKING_DIR, torHiddenServiceDir.getName()));
networkNode.start(this);

Expand All @@ -139,7 +141,7 @@ public String getProperty(String name) {
});
CorruptedDatabaseFilesHandler corruptedDatabaseFilesHandler = new CorruptedDatabaseFilesHandler();
int maxConnections = Integer.parseInt(configuration.getProperty(MAX_CONNECTIONS, "12"));
NetworkProtoResolver networkProtoResolver = new CoreNetworkProtoResolver();
NetworkProtoResolver networkProtoResolver = new CoreNetworkProtoResolver(Clock.systemDefaultZone());
CorePersistenceProtoResolver persistenceProtoResolver = new CorePersistenceProtoResolver(null,
networkProtoResolver, storageDir, corruptedDatabaseFilesHandler);
DefaultSeedNodeRepository seedNodeRepository = new DefaultSeedNodeRepository(environment, null);
Expand Down
Expand Up @@ -39,6 +39,8 @@
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.SettableFuture;

import java.time.Clock;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -90,7 +92,7 @@ public P2PSeedNodeSnapshotBase(Reporter reporter) {
protected void execute() {
// start the network node
final NetworkNode networkNode = new TorNetworkNode(Integer.parseInt(configuration.getProperty(TOR_PROXY_PORT, "9054")),
new CoreNetworkProtoResolver(), false,
new CoreNetworkProtoResolver(Clock.systemDefaultZone()), false,
new AvailableTor(Monitor.TOR_WORKING_DIR, "unused"));
// we do not need to start the networkNode, as we do not need the HS
//networkNode.start(this);
Expand Down
12 changes: 6 additions & 6 deletions p2p/src/main/java/bisq/network/p2p/P2PService.java
Expand Up @@ -700,12 +700,12 @@ public void onBroadcastFailed(String errorMessage) {
};
boolean result = p2PDataStorage.addProtectedStorageEntry(protectedMailboxStorageEntry, networkNode.getNodeAddress(), listener, true);
if (!result) {
//TODO remove and add again with a delay to ensure the data will be broadcasted
// The p2PDataStorage.remove makes probably sense but need to be analysed more.
// Don't change that if it is not 100% clear.
sendMailboxMessageListener.onFault("Data already exists in our local database");
boolean removeResult = p2PDataStorage.remove(protectedMailboxStorageEntry, networkNode.getNodeAddress(), true);
log.debug("remove result=" + removeResult);

// This should only fail if there are concurrent calls to addProtectedStorageEntry with the
// same ProtectedMailboxStorageEntry. This is an unexpected use case so if it happens we
// want to see it, but it is not worth throwing an exception.
log.error("Unexpected state: adding mailbox message that already exists.");
}
} catch (CryptoException e) {
log.error("Signing at getDataWithSignedSeqNr failed. That should never happen.");
Expand Down Expand Up @@ -759,7 +759,7 @@ private void delayedRemoveEntryFromMailbox(DecryptedMessageWithPubKey decryptedM
expirableMailboxStoragePayload,
keyRing.getSignatureKeyPair(),
receiversPubKey);
p2PDataStorage.removeMailboxData(protectedMailboxStorageEntry, networkNode.getNodeAddress(), true);
p2PDataStorage.remove(protectedMailboxStorageEntry, networkNode.getNodeAddress(), true);
} catch (CryptoException e) {
log.error("Signing at getDataWithSignedSeqNr failed. That should never happen.");
}
Expand Down