diff --git a/common/src/main/java/bisq/common/ExcludeForHashAwareProto.java b/common/src/main/java/bisq/common/ExcludeForHashAwareProto.java index 18977fb3b9f..284e223c5c6 100644 --- a/common/src/main/java/bisq/common/ExcludeForHashAwareProto.java +++ b/common/src/main/java/bisq/common/ExcludeForHashAwareProto.java @@ -103,7 +103,7 @@ default Set getExcludedFields() { default B clearAnnotatedFields(B builder) { Set excludedFields = getExcludedFields(); if (!excludedFields.isEmpty()) { - getLogger().error("Clear fields in builder annotated with @ExcludeForHash: {}", excludedFields); + getLogger().debug("Clear fields in builder annotated with @ExcludeForHash: {}", excludedFields); } for (Descriptors.FieldDescriptor fieldDesc : builder.getAllFields().keySet()) { if (excludedFields.contains(fieldDesc.getName())) { diff --git a/core/src/main/java/bisq/core/filter/Filter.java b/core/src/main/java/bisq/core/filter/Filter.java index 069cc91dbf4..2461dd0179f 100644 --- a/core/src/main/java/bisq/core/filter/Filter.java +++ b/core/src/main/java/bisq/core/filter/Filter.java @@ -20,6 +20,8 @@ import bisq.network.p2p.storage.payload.ExpirablePayload; import bisq.network.p2p.storage.payload.ProtectedStoragePayload; +import bisq.common.ExcludeForHash; +import bisq.common.ExcludeForHashAwareProto; import bisq.common.crypto.Sig; import bisq.common.proto.ProtoUtil; import bisq.common.proto.network.GetDataResponsePriority; @@ -27,6 +29,8 @@ import bisq.common.util.ExtraDataMapValidator; import bisq.common.util.Utilities; +import protobuf.StoragePayload; + import com.google.protobuf.ByteString; import com.google.common.annotations.VisibleForTesting; @@ -49,7 +53,7 @@ @Slf4j @Getter @EqualsAndHashCode -public final class Filter implements ProtectedStoragePayload, ExpirablePayload { +public final class Filter implements ProtectedStoragePayload, ExpirablePayload, ExcludeForHashAwareProto { public static final long TTL = TimeUnit.DAYS.toMillis(180); private final List bannedOfferIds; @@ -124,8 +128,14 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload { private final List delayedPayoutPaymentAccounts; // Added at v 1.9.16 + @ExcludeForHash private final List addedBtcNodes; + @ExcludeForHash private final List addedSeedNodes; + // As we might add more ExcludeForHash we want to ensure to have a unique identifier. + // The hash of the data is not unique anymore if the only change have been at + // the ExcludeForHash annotated fields. + private final String uid; // After we have created the signature from the filter data we clone it and apply the signature static Filter cloneWithSig(Filter filter, String signatureAsBase64) { @@ -166,7 +176,8 @@ static Filter cloneWithSig(Filter filter, String signatureAsBase64) { filter.getTakerFeeBsq(), filter.getDelayedPayoutPaymentAccounts(), filter.getAddedBtcNodes(), - filter.getAddedSeedNodes()); + filter.getAddedSeedNodes(), + filter.getUid()); } // Used for signature verification as we created the sig without the signatureAsBase64 field we set it to null again @@ -208,7 +219,8 @@ static Filter cloneWithoutSig(Filter filter) { filter.getTakerFeeBsq(), filter.getDelayedPayoutPaymentAccounts(), filter.getAddedBtcNodes(), - filter.getAddedSeedNodes()); + filter.getAddedSeedNodes(), + filter.getUid()); } public Filter(List bannedOfferIds, @@ -245,7 +257,8 @@ public Filter(List bannedOfferIds, long takerFeeBsq, List delayedPayoutPaymentAccounts, List addedBtcNodes, - List addedSeedNodes) { + List addedSeedNodes, + String uid) { this(bannedOfferIds, nodeAddressesBannedFromTrading, bannedPaymentAccounts, @@ -283,7 +296,8 @@ public Filter(List bannedOfferIds, takerFeeBsq, delayedPayoutPaymentAccounts, addedBtcNodes, - addedSeedNodes); + addedSeedNodes, + uid); } @@ -329,7 +343,8 @@ public Filter(List bannedOfferIds, long takerFeeBsq, List delayedPayoutPaymentAccounts, List addedBtcNodes, - List addedSeedNodes) { + List addedSeedNodes, + String uid) { this.bannedOfferIds = bannedOfferIds; this.nodeAddressesBannedFromTrading = nodeAddressesBannedFromTrading; this.bannedPaymentAccounts = bannedPaymentAccounts; @@ -368,6 +383,7 @@ public Filter(List bannedOfferIds, this.delayedPayoutPaymentAccounts = delayedPayoutPaymentAccounts; this.addedBtcNodes = addedBtcNodes; this.addedSeedNodes = addedSeedNodes; + this.uid = uid; // ownerPubKeyBytes can be null when called from tests if (ownerPubKeyBytes != null) { @@ -379,6 +395,24 @@ public Filter(List bannedOfferIds, @Override public protobuf.StoragePayload toProtoMessage() { + return toProto(false); + } + + @Override + public protobuf.StoragePayload toProto(boolean serializeForHash) { + return resolveProto(serializeForHash); + } + + @Override + public protobuf.StoragePayload.Builder getBuilder(boolean serializeForHash) { + return StoragePayload.newBuilder().setFilter(toFilterProto(serializeForHash)); + } + + private protobuf.Filter toFilterProto(boolean serializeForHash) { + return resolveBuilder(getFilterBuilder(serializeForHash), serializeForHash).build(); + } + + private protobuf.Filter.Builder getFilterBuilder(boolean serializeForHash) { List paymentAccountFilterList = bannedPaymentAccounts.stream() .map(PaymentAccountFilter::toProtoMessage) .collect(Collectors.toList()); @@ -421,12 +455,13 @@ public protobuf.StoragePayload toProtoMessage() { .setTakerFeeBsq(takerFeeBsq) .addAllDelayedPayoutPaymentAccounts(delayedPayoutPaymentAccountList) .addAllAddedBtcNodes(addedBtcNodes) - .addAllAddedSeedNodes(addedSeedNodes); + .addAllAddedSeedNodes(addedSeedNodes) + .setUid(uid); Optional.ofNullable(signatureAsBase64).ifPresent(builder::setSignatureAsBase64); Optional.ofNullable(extraDataMap).ifPresent(builder::putAllExtraData); - return protobuf.StoragePayload.newBuilder().setFilter(builder).build(); + return builder; } public static Filter fromProto(protobuf.Filter proto) { @@ -474,7 +509,8 @@ public static Filter fromProto(protobuf.Filter proto) { proto.getTakerFeeBsq(), delayedPayoutPaymentAccounts, ProtoUtil.protocolStringListToList(proto.getAddedBtcNodesList()), - ProtoUtil.protocolStringListToList(proto.getAddedSeedNodesList()) + ProtoUtil.protocolStringListToList(proto.getAddedSeedNodesList()), + proto.getUid() ); } @@ -534,6 +570,7 @@ public String toString() { ",\n takerFeeBsq=" + takerFeeBsq + ",\n addedBtcNodes=" + addedBtcNodes + ",\n addedSeedNodes=" + addedSeedNodes + + ",\n uid=" + uid + "\n}"; } } diff --git a/core/src/main/java/bisq/core/filter/FilterManager.java b/core/src/main/java/bisq/core/filter/FilterManager.java index ad0c29f3264..c0fb5bae852 100644 --- a/core/src/main/java/bisq/core/filter/FilterManager.java +++ b/core/src/main/java/bisq/core/filter/FilterManager.java @@ -701,8 +701,7 @@ private ECKey toECKey(String privKeyString) { } private Sha256Hash getSha256Hash(Filter filter) { - byte[] filterData = filter.serializeForHash(); - return Sha256Hash.of(filterData); + return Sha256Hash.of(filter.serializeForHash()); } private String getPubKeyAsHex(ECKey ecKey) { diff --git a/core/src/test/java/bisq/core/filter/FilterWithExcludeForHashFieldTests.java b/core/src/test/java/bisq/core/filter/FilterWithExcludeForHashFieldTests.java new file mode 100644 index 00000000000..91d10185a83 --- /dev/null +++ b/core/src/test/java/bisq/core/filter/FilterWithExcludeForHashFieldTests.java @@ -0,0 +1,63 @@ +/* + * This file is part of Bisq. + * + * Bisq is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or (at + * your option) any later version. + * + * Bisq is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public + * License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with Bisq. If not, see . + */ + +package bisq.core.filter; + +import bisq.common.crypto.Sig; + +import org.bouncycastle.jce.provider.BouncyCastleProvider; + +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.PublicKey; +import java.security.Security; + +import java.util.Arrays; + +import org.mockito.junit.jupiter.MockitoExtension; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@ExtendWith(MockitoExtension.class) +public class FilterWithExcludeForHashFieldTests { + static { + Security.addProvider(new BouncyCastleProvider()); + } + + private final PublicKey ownerPublicKey; + + public FilterWithExcludeForHashFieldTests() throws NoSuchAlgorithmException, NoSuchProviderException { + KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance(Sig.KEY_ALGO, "BC"); + KeyPair ownerKeyPair = keyPairGenerator.generateKeyPair(); + ownerPublicKey = ownerKeyPair.getPublic(); + } + + @Test + void testSerializeForHashAnnotation() { + Filter filterWithoutSig = TestFilter.createFilter(ownerPublicKey, "invalidPubKeyAsHex"); + byte[] serializeForHashBytes = filterWithoutSig.serializeForHash(); + byte[] serializeBytes = filterWithoutSig.serialize(); + assertFalse(Arrays.equals(serializeForHashBytes, serializeBytes)); + assertTrue(serializeBytes.length > serializeForHashBytes.length); + } +} diff --git a/core/src/test/java/bisq/core/filter/TestFilter.java b/core/src/test/java/bisq/core/filter/TestFilter.java index ef232fa8827..ff2c041f0c3 100644 --- a/core/src/test/java/bisq/core/filter/TestFilter.java +++ b/core/src/test/java/bisq/core/filter/TestFilter.java @@ -32,6 +32,7 @@ import java.util.Collections; import java.util.List; +import java.util.UUID; import static org.bitcoinj.core.Utils.HEX; @@ -103,8 +104,9 @@ public static Filter createFilter(PublicKey ownerPublicKey, String signerPubKeyA 1, 1, Collections.emptyList(), - Collections.emptyList(), - Collections.emptyList() + List.of("test1.onion:1221"), + List.of("test2.onion:1221"), + UUID.randomUUID().toString() ); } diff --git a/core/src/test/java/bisq/core/user/UserPayloadModelVOTest.java b/core/src/test/java/bisq/core/user/UserPayloadModelVOTest.java index 930bedadb72..52152c3118f 100644 --- a/core/src/test/java/bisq/core/user/UserPayloadModelVOTest.java +++ b/core/src/test/java/bisq/core/user/UserPayloadModelVOTest.java @@ -26,6 +26,7 @@ import com.google.common.collect.Lists; import java.util.HashSet; +import java.util.UUID; import org.junit.jupiter.api.Disabled; @@ -79,7 +80,8 @@ public void testRoundtripFull() { 0, Lists.newArrayList(), Lists.newArrayList(), - Lists.newArrayList())); + Lists.newArrayList(), + UUID.randomUUID().toString())); vo.setRegisteredArbitrator(ArbitratorTest.getArbitratorMock()); vo.setRegisteredMediator(MediatorTest.getMediatorMock()); diff --git a/desktop/src/main/java/bisq/desktop/main/overlays/windows/FilterWindow.java b/desktop/src/main/java/bisq/desktop/main/overlays/windows/FilterWindow.java index c997271d66f..b736f73fee0 100644 --- a/desktop/src/main/java/bisq/desktop/main/overlays/windows/FilterWindow.java +++ b/desktop/src/main/java/bisq/desktop/main/overlays/windows/FilterWindow.java @@ -60,6 +60,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.List; +import java.util.UUID; import java.util.stream.Collectors; import static bisq.desktop.util.FormBuilder.addInputTextField; @@ -129,6 +130,8 @@ private void addContent() { InputTextField keyTF = addInputTextField(gridPane, ++rowIndex, Res.get("shared.unlock"), 10); + InputTextField uidTF = addInputTextField(gridPane, ++rowIndex, + "UID", 10); if (useDevPrivilegeKeys) { keyTF.setText(DevEnv.getDEV_PRIVILEGE_PRIV_KEY()); } @@ -212,6 +215,7 @@ private void addContent() { Filter filter = filterManager.getDevFilter(); if (filter != null) { + uidTF.setText(filter.getUid()); setupFieldFromList(offerIdsTF, filter.getBannedOfferIds()); setupFieldFromList(bannedFromTradingTF, filter.getNodeAddressesBannedFromTrading()); setupFieldFromList(bannedFromNetworkTF, filter.getNodeAddressesBannedFromNetwork()); @@ -247,6 +251,8 @@ private void addContent() { makerFeeBsqTF.setText(bsqFormatter.formatBSQSatoshis(filter.getMakerFeeBsq())); takerFeeBsqTF.setText(bsqFormatter.formatBSQSatoshis(filter.getTakerFeeBsq())); setupFieldFromPaymentAccountFiltersList(delayedPayoutTF, filter.getDelayedPayoutPaymentAccounts()); + } else { + uidTF.setText(UUID.randomUUID().toString()); } Button removeFilterMessageButton = new AutoTooltipButton(Res.get("filterWindow.remove")); @@ -292,7 +298,8 @@ private void addContent() { ParsingUtils.parseToCoin(takerFeeBsqTF.getText(), bsqFormatter).value, readAsPaymentAccountFiltersList(delayedPayoutTF), readAsList(addedBtcNodesTF), - readAsList(addedSeedNodesTF) + readAsList(addedSeedNodesTF), + uidTF.getText() ); // We remove first the old filter 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 c80f6c2720c..d8ac6dddc18 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/P2PDataStorage.java @@ -55,6 +55,7 @@ import bisq.network.p2p.storage.persistence.ResourceDataStoreService; import bisq.network.p2p.storage.persistence.SequenceNumberMap; +import bisq.common.ExcludeForHashAwareProto; import bisq.common.Timer; import bisq.common.UserThread; import bisq.common.app.Capabilities; @@ -71,6 +72,8 @@ import bisq.common.util.Tuple2; import bisq.common.util.Utilities; +import protobuf.StoragePayload; + import com.google.protobuf.ByteString; import com.google.inject.name.Named; @@ -1253,8 +1256,8 @@ public static byte[] get32ByteHash(NetworkPayload data) { */ @EqualsAndHashCode @ToString - public static final class DataAndSeqNrPair implements NetworkPayload { - // data are only used for calculating cryptographic hash from both values so they are kept private + public static final class DataAndSeqNrPair implements NetworkPayload, ExcludeForHashAwareProto { + // data are only used for calculating cryptographic hash from both values, so they are kept private private final ProtectedStoragePayload protectedStoragePayload; private final int sequenceNumber; @@ -1263,13 +1266,31 @@ public DataAndSeqNrPair(ProtectedStoragePayload protectedStoragePayload, int seq this.sequenceNumber = sequenceNumber; } - // Used only for calculating hash of byte array from PB object @Override - public com.google.protobuf.Message toProtoMessage() { + public protobuf.DataAndSeqNrPair toProtoMessage() { + return toProto(false); + } + + @Override + public protobuf.DataAndSeqNrPair toProto(boolean serializeForHash) { + return resolveProto(serializeForHash); + } + + @Override + public protobuf.DataAndSeqNrPair.Builder getBuilder(boolean serializeForHash) { return protobuf.DataAndSeqNrPair.newBuilder() - .setPayload((protobuf.StoragePayload) protectedStoragePayload.toProtoMessage()) - .setSequenceNumber(sequenceNumber) - .build(); + .setPayload(toStoragePayloadProto(serializeForHash)) + .setSequenceNumber(sequenceNumber); + } + + private protobuf.StoragePayload toStoragePayloadProto(boolean serializeForHash) { + if (protectedStoragePayload instanceof ExcludeForHashAwareProto) { + ExcludeForHashAwareProto proto = (ExcludeForHashAwareProto) protectedStoragePayload; + StoragePayload.Builder builder = (StoragePayload.Builder) proto.getBuilder(serializeForHash); + return resolveBuilder(builder, serializeForHash).build(); + } else { + return (StoragePayload) protectedStoragePayload.toProtoMessage(); + } } } diff --git a/p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedStorageEntry.java b/p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedStorageEntry.java index 9f4f61c2853..1f1c4e1f696 100644 --- a/p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedStorageEntry.java +++ b/p2p/src/main/java/bisq/network/p2p/storage/payload/ProtectedStorageEntry.java @@ -32,6 +32,8 @@ import com.google.common.base.Preconditions; +import org.bouncycastle.util.encoders.Hex; + import java.security.PublicKey; import java.time.Clock; @@ -215,7 +217,9 @@ boolean isSignatureValid() { boolean result = Sig.verify(this.ownerPubKey, hashOfDataAndSeqNr, this.signature); if (!result) - log.warn("ProtectedStorageEntry::isSignatureValid() failed.\n{}}", this); + log.warn("Invalid signature for {}.\nSerialized data as hex={}}", + protectedStoragePayload.getClass().getSimpleName(), + Hex.toHexString(protectedStoragePayload.toProtoMessage().toByteArray())); return result; } catch (CryptoException e) { diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto index f5be6a1f66b..034343f1996 100644 --- a/proto/src/main/proto/pb.proto +++ b/proto/src/main/proto/pb.proto @@ -769,6 +769,7 @@ message Filter { repeated PaymentAccountFilter delayedPayoutPaymentAccounts = 36; repeated string addedBtcNodes = 37; repeated string addedSeedNodes = 38; + string uid = 39; } /* Deprecated */