From cc20a3b6c7c325c7670fd9bb4582b5e887a583cc Mon Sep 17 00:00:00 2001 From: chimp1984 Date: Mon, 31 Aug 2020 12:33:45 -0500 Subject: [PATCH] - Add support for multiple pubKeys - Add more validation - Add SignerPubKeyAsHex to filter - Refactor add/remove filter code --- .../main/java/bisq/core/filter/Filter.java | 26 +++- .../java/bisq/core/filter/FilterManager.java | 124 ++++++++++++++---- .../core/user/UserPayloadModelVOTest.java | 1 + .../core/util/FeeReceiverSelectorTest.java | 1 + .../main/overlays/windows/FilterWindow.java | 77 +++++------ proto/src/main/proto/pb.proto | 1 + 6 files changed, 159 insertions(+), 71 deletions(-) diff --git a/core/src/main/java/bisq/core/filter/Filter.java b/core/src/main/java/bisq/core/filter/Filter.java index 93744ef9e9..f6c28bc575 100644 --- a/core/src/main/java/bisq/core/filter/Filter.java +++ b/core/src/main/java/bisq/core/filter/Filter.java @@ -60,7 +60,10 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload { // created by cloning the object with a non-null sig. @Nullable private final String signatureAsBase64; - // The pub key used for the data protection in teh p2p storage + // The pub EC key from the dev who has signed and published the filter (different to ownerPubKeyBytes) + private final String signerPubKeyAsHex; + + // The pub key used for the data protection in the p2p storage private final byte[] ownerPubKeyBytes; private final boolean disableDao; private final String disableDaoBelowVersion; @@ -104,7 +107,8 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload { filter.getOwnerPubKeyBytes(), filter.getCreationDate(), filter.getExtraDataMap(), - signatureAsBase64); + signatureAsBase64, + filter.getSignerPubKeyAsHex()); } // Used for signature verification as we created the sig without the signatureAsBase64 field we set it to null again @@ -129,7 +133,8 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload { filter.getOwnerPubKeyBytes(), filter.getCreationDate(), filter.getExtraDataMap(), - null); + null, + filter.getSignerPubKeyAsHex()); } public Filter(List bannedOfferIds, @@ -149,7 +154,8 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload { List refundAgents, List bannedSignerPubKeys, List btcFeeReceiverAddresses, - PublicKey ownerPubKey) { + PublicKey ownerPubKey, + String signerPubKeyAsHex) { this(bannedOfferIds, bannedNodeAddress, bannedPaymentAccounts, @@ -170,7 +176,8 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload { Sig.getPublicKeyBytes(ownerPubKey), System.currentTimeMillis(), null, - null); + null, + signerPubKeyAsHex); } @@ -199,7 +206,8 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload { byte[] ownerPubKeyBytes, long creationDate, @Nullable Map extraDataMap, - @Nullable String signatureAsBase64) { + @Nullable String signatureAsBase64, + String signerPubKeyAsHex) { this.bannedOfferIds = bannedOfferIds; this.bannedNodeAddress = bannedNodeAddress; this.bannedPaymentAccounts = bannedPaymentAccounts; @@ -221,6 +229,7 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload { this.creationDate = creationDate; this.extraDataMap = ExtraDataMapValidator.getValidatedExtraDataMap(extraDataMap); this.signatureAsBase64 = signatureAsBase64; + this.signerPubKeyAsHex = signerPubKeyAsHex; // ownerPubKeyBytes can be null when called from tests if (ownerPubKeyBytes != null) { @@ -254,6 +263,7 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload { .addAllBannedSignerPubKeys(bannedSignerPubKeys) .addAllBtcFeeReceiverAddresses(btcFeeReceiverAddresses) .setOwnerPubKeyBytes(ByteString.copyFrom(ownerPubKeyBytes)) + .setSignerPubKeyAsHex(signerPubKeyAsHex) .setCreationDate(creationDate); Optional.ofNullable(signatureAsBase64).ifPresent(builder::setSignatureAsBase64); @@ -288,7 +298,8 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload { proto.getOwnerPubKeyBytes().toByteArray(), proto.getCreationDate(), CollectionUtils.isEmpty(proto.getExtraDataMap()) ? null : proto.getExtraDataMap(), - proto.getSignatureAsBase64() + proto.getSignatureAsBase64(), + proto.getSignerPubKeyAsHex() ); } @@ -316,6 +327,7 @@ public final class Filter implements ProtectedStoragePayload, ExpirablePayload { ",\n preventPublicBtcNetwork=" + preventPublicBtcNetwork + ",\n btcNodes=" + btcNodes + ",\n signatureAsBase64='" + signatureAsBase64 + '\'' + + ",\n signerPubKeyAsHex='" + signerPubKeyAsHex + '\'' + ",\n ownerPubKeyBytes=" + Utilities.bytesAsHexString(ownerPubKeyBytes) + ",\n disableDao=" + disableDao + ",\n disableDaoBelowVersion='" + disableDaoBelowVersion + '\'' + diff --git a/core/src/main/java/bisq/core/filter/FilterManager.java b/core/src/main/java/bisq/core/filter/FilterManager.java index 8ddaf91d79..816c779605 100644 --- a/core/src/main/java/bisq/core/filter/FilterManager.java +++ b/core/src/main/java/bisq/core/filter/FilterManager.java @@ -38,7 +38,6 @@ import bisq.common.crypto.KeyRing; import org.bitcoinj.core.ECKey; import org.bitcoinj.core.Sha256Hash; -import org.bitcoinj.core.Utils; import javax.inject.Inject; import javax.inject.Named; @@ -55,6 +54,7 @@ import java.nio.charset.StandardCharsets; import java.math.BigInteger; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -94,8 +94,7 @@ public class FilterManager { private final boolean ignoreDevMsg; private final ObjectProperty filterProperty = new SimpleObjectProperty<>(); private final List listeners = new CopyOnWriteArrayList<>(); - private final String pubKeyAsHex; - + private final List publicKeys; private ECKey filterSigningKey; @@ -120,9 +119,12 @@ public class FilterManager { this.providersRepository = providersRepository; this.ignoreDevMsg = ignoreDevMsg; - pubKeyAsHex = useDevPrivilegeKeys ? - DevEnv.DEV_PRIVILEGE_PUB_KEY : - "022ac7b7766b0aedff82962522c2c14fb8d1961dabef6e5cfd10edc679456a32f1"; + //todo test + publicKeys = !useDevPrivilegeKeys ? + Collections.singletonList(DevEnv.DEV_PRIVILEGE_PUB_KEY) : + List.of("0358d47858acdc41910325fce266571540681ef83a0d6fedce312bef9810793a27", + "029340c3e7d4bb0f9e651b5f590b434fecb6175aeaa57145c7804ff05d210e534f"); + } @@ -139,6 +141,7 @@ public class FilterManager { .map(ProtectedStorageEntry::getProtectedStoragePayload) .filter(protectedStoragePayload -> protectedStoragePayload instanceof Filter) .map(protectedStoragePayload -> (Filter) protectedStoragePayload) + .filter(this::isFilterPublicKeyInList) .filter(this::verifySignature) .forEach(this::onFilterAddedFromNetwork); @@ -149,7 +152,7 @@ public class FilterManager { .filter(protectedStorageEntry -> protectedStorageEntry.getProtectedStoragePayload() instanceof Filter) .forEach(protectedStorageEntry -> { Filter filter = (Filter) protectedStorageEntry.getProtectedStoragePayload(); - if (verifySignature(filter)) { + if (isFilterPublicKeyInList(filter) && verifySignature(filter)) { onFilterAddedFromNetwork(filter); } }); @@ -161,7 +164,7 @@ public class FilterManager { .filter(protectedStorageEntry -> protectedStorageEntry.getProtectedStoragePayload() instanceof Filter) .forEach(protectedStorageEntry -> { Filter filter = (Filter) protectedStorageEntry.getProtectedStoragePayload(); - if (verifySignature(filter)) { + if (isFilterPublicKeyInList(filter) && verifySignature(filter)) { onFilterRemovedFromNetwork(filter); } }); @@ -209,20 +212,24 @@ public class FilterManager { }); } - public boolean isValidDevPrivilegeKey(String privKeyString) { - try { - ECKey filterSigningKey = toECKey(privKeyString); - return pubKeyAsHex.equals(Utils.HEX.encode(filterSigningKey.getPubKey())); - } catch (Throwable t) { + public boolean canAddDevFilter(String privKeyString) { + if (privKeyString == null || privKeyString.isEmpty()) { return false; } + if (!isValidDevPrivilegeKey(privKeyString)) { + log.warn("There is no persisted dev filter to be removed."); + return false; + } + return true; } - public void setFilterSigningKey(String privKeyString) { - this.filterSigningKey = toECKey(privKeyString); + public String getSignerPubKeyAsHex(String privKeyString) { + ECKey ecKey = toECKey(privKeyString); + return getPubKeyAsHex(ecKey); } - public void publishFilter(Filter filterWithoutSig) { + public void addDevFilter(Filter filterWithoutSig, String privKeyString) { + setFilterSigningKey(privKeyString); String signatureAsBase64 = getSignature(filterWithoutSig); Filter filterWithSig = Filter.cloneWithSig(filterWithoutSig, signatureAsBase64); user.setDevelopersFilter(filterWithSig); @@ -230,8 +237,42 @@ public class FilterManager { p2PService.addProtectedStorageEntry(filterWithSig); } + public boolean canRemoveDevFilter(String privKeyString) { + if (privKeyString == null || privKeyString.isEmpty()) { + return false; + } - public void removeFilter() { + Filter developersFilter = getDevFilter(); + if (developersFilter == null) { + log.warn("There is no persisted dev filter to be removed."); + return false; + } + + if (!isPublicKeyInList(developersFilter.getSignerPubKeyAsHex())) { + log.warn("The SignerPubKey from the filter is not in our pubKey list. " + + "filterSignerPubKey={}, publicKeys={}", developersFilter.getSignerPubKeyAsHex(), publicKeys); + return false; + } + + if (!isValidDevPrivilegeKey(privKeyString)) { + log.warn("There is no persisted dev filter to be removed."); + return false; + } + + ECKey ecKeyFromPrivate = toECKey(privKeyString); + String pubKeyAsHex = getPubKeyAsHex(ecKeyFromPrivate); + if (!developersFilter.getSignerPubKeyAsHex().equals(pubKeyAsHex)) { + log.warn("pubKeyAsHex derived from private key does not match filterSignerPubKey. " + + "filterSignerPubKey={}, pubKeyAsHex derived from private key={}", + developersFilter.getSignerPubKeyAsHex(), pubKeyAsHex); + return false; + } + + return true; + } + + public void removeDevFilter(String privKeyString) { + setFilterSigningKey(privKeyString); Filter filterWithSig = user.getDevelopersFilter(); if (filterWithSig == null) { // Should not happen as UI button is deactivated in that case @@ -259,10 +300,14 @@ public class FilterManager { } @Nullable - public Filter getDevelopersFilter() { + public Filter getDevFilter() { return user.getDevelopersFilter(); } + public PublicKey getOwnerPubKey() { + return keyRing.getSignatureKeyPair().getPublic(); + } + public boolean isCurrencyBanned(String currencyCode) { return getFilter() != null && getFilter().getBannedCurrencies() != null && @@ -347,10 +392,6 @@ public class FilterManager { .anyMatch(e -> e.equals(witnessSignerPubKeyAsHex)); } - public PublicKey getOwnerPubKey() { - return keyRing.getSignatureKeyPair().getPublic(); - } - /////////////////////////////////////////////////////////////////////////////////////////// // Private @@ -423,6 +464,20 @@ public class FilterManager { configFileEditor.clearOption(optionName); } + private boolean isValidDevPrivilegeKey(String privKeyString) { + try { + ECKey filterSigningKey = toECKey(privKeyString); + String pubKeyAsHex = getPubKeyAsHex(filterSigningKey); + return isPublicKeyInList(pubKeyAsHex); + } catch (Throwable t) { + return false; + } + } + + private void setFilterSigningKey(String privKeyString) { + this.filterSigningKey = toECKey(privKeyString); + } + private String getSignature(Filter filterWithoutSig) { Sha256Hash hash = getSha256Hash(filterWithoutSig); ECKey.ECDSASignature ecdsaSignature = filterSigningKey.sign(hash); @@ -430,6 +485,15 @@ public class FilterManager { return new String(Base64.encode(encodeToDER), StandardCharsets.UTF_8); } + private boolean isFilterPublicKeyInList(Filter filter) { + String signerPubKeyAsHex = filter.getSignerPubKeyAsHex(); + if (!isPublicKeyInList(signerPubKeyAsHex)) { + log.warn("signerPubKeyAsHex from filter is not part of our pub key list. filter={}, publicKeys={}", filter, publicKeys); + return false; + } + return true; + } + private boolean verifySignature(Filter filter) { try { Filter filterForSigVerification = Filter.cloneWithoutSig(filter); @@ -439,7 +503,9 @@ public class FilterManager { byte[] sigData = Base64.decode(filter.getSignatureAsBase64()); ECKey.ECDSASignature ecdsaSignature = ECKey.ECDSASignature.decodeFromDER(sigData); - ECKey ecPubKey = ECKey.fromPublicOnly(HEX.decode(pubKeyAsHex)); + String signerPubKeyAsHex = filter.getSignerPubKeyAsHex(); + byte[] decode = HEX.decode(signerPubKeyAsHex); + ECKey ecPubKey = ECKey.fromPublicOnly(decode); return ecPubKey.verify(hash, ecdsaSignature); } catch (Throwable e) { log.warn("verifySignature failed. filter={}", filter); @@ -455,4 +521,16 @@ public class FilterManager { byte[] filterData = filter.toProtoMessage().toByteArray(); return Sha256Hash.of(filterData); } + + private String getPubKeyAsHex(ECKey ecKey) { + return HEX.encode(ecKey.getPubKey()); + } + + private boolean isPublicKeyInList(String pubKeyAsHex) { + boolean isPublicKeyInList = publicKeys.contains(pubKeyAsHex); + if (!isPublicKeyInList) { + log.warn("pubKeyAsHex is not part of our pub key list. pubKeyAsHex={}, publicKeys={}", pubKeyAsHex, publicKeys); + } + return isPublicKeyInList; + } } diff --git a/core/src/test/java/bisq/core/user/UserPayloadModelVOTest.java b/core/src/test/java/bisq/core/user/UserPayloadModelVOTest.java index de685c9ba4..050fce18ec 100644 --- a/core/src/test/java/bisq/core/user/UserPayloadModelVOTest.java +++ b/core/src/test/java/bisq/core/user/UserPayloadModelVOTest.java @@ -60,6 +60,7 @@ public class UserPayloadModelVOTest { null, 0, null, + null, null)); vo.setRegisteredArbitrator(ArbitratorTest.getArbitratorMock()); diff --git a/core/src/test/java/bisq/core/util/FeeReceiverSelectorTest.java b/core/src/test/java/bisq/core/util/FeeReceiverSelectorTest.java index 2f059edd84..e5319f8ecd 100644 --- a/core/src/test/java/bisq/core/util/FeeReceiverSelectorTest.java +++ b/core/src/test/java/bisq/core/util/FeeReceiverSelectorTest.java @@ -119,6 +119,7 @@ public class FeeReceiverSelectorTest { null, 0, null, + null, null); } } 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 0839cbfb31..8ca9aa4484 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 @@ -79,7 +79,7 @@ public class FilterWindow extends Overlay { if (headLine == null) headLine = Res.get("filterWindow.headline"); - width = 968; + width = 1000; createGridPane(); @@ -87,7 +87,7 @@ public class FilterWindow extends Overlay { scrollPane.setContent(gridPane); scrollPane.setFitToWidth(true); scrollPane.setFitToHeight(true); - scrollPane.setMaxHeight(1000); + scrollPane.setMaxHeight(700); scrollPane.setHbarPolicy(ScrollPane.ScrollBarPolicy.NEVER); addHeadLine(); @@ -139,7 +139,7 @@ public class FilterWindow extends Overlay { InputTextField disableDaoBelowVersionInputTextField = addInputTextField(gridPane, ++rowIndex, Res.get("filterWindow.disableDaoBelowVersion")); InputTextField disableTradeBelowVersionInputTextField = addInputTextField(gridPane, ++rowIndex, Res.get("filterWindow.disableTradeBelowVersion")); - final Filter filter = filterManager.getDevelopersFilter(); + Filter filter = filterManager.getDevFilter(); if (filter != null) { setupFieldFromList(offerIdsInputTextField, filter.getBannedOfferIds()); setupFieldFromList(nodesInputTextField, filter.getBannedNodeAddress()); @@ -162,55 +162,50 @@ public class FilterWindow extends Overlay { } Button removeFilterMessageButton = new AutoTooltipButton(Res.get("filterWindow.remove")); - removeFilterMessageButton.setDisable(filterManager.getDevelopersFilter() == null); + removeFilterMessageButton.setDisable(filterManager.getDevFilter() == null); Button sendButton = new AutoTooltipButton(Res.get("filterWindow.add")); - String privKeyString = keyInputTextField.getText(); sendButton.setOnAction(e -> { - if (privKeyString.isEmpty()) { - return; - } - Filter newFilter = new Filter( - readAsList(offerIdsInputTextField), - readAsList(nodesInputTextField), - readAsPaymentAccountFiltersList(paymentAccountFilterInputTextField), - readAsList(bannedCurrenciesInputTextField), - readAsList(bannedPaymentMethodsInputTextField), - readAsList(arbitratorsInputTextField), - readAsList(seedNodesInputTextField), - readAsList(priceRelayNodesInputTextField), - preventPublicBtcNetworkCheckBox.isSelected(), - readAsList(btcNodesInputTextField), - disableDaoCheckBox.isSelected(), - disableDaoBelowVersionInputTextField.getText(), - disableTradeBelowVersionInputTextField.getText(), - readAsList(mediatorsInputTextField), - readAsList(refundAgentsInputTextField), - readAsList(bannedSignerPubKeysInputTextField), - readAsList(btcFeeReceiverAddressesInputTextField), - filterManager.getOwnerPubKey() - ); - if (filterManager.isValidDevPrivilegeKey(privKeyString)) { - filterManager.setFilterSigningKey(privKeyString); - filterManager.publishFilter(newFilter); - removeFilterMessageButton.setDisable(filterManager.getDevelopersFilter() == null); + String privKeyString = keyInputTextField.getText(); + if (filterManager.canAddDevFilter(privKeyString)) { + String signerPubKeyAsHex = filterManager.getSignerPubKeyAsHex(privKeyString); + Filter newFilter = new Filter( + readAsList(offerIdsInputTextField), + readAsList(nodesInputTextField), + readAsPaymentAccountFiltersList(paymentAccountFilterInputTextField), + readAsList(bannedCurrenciesInputTextField), + readAsList(bannedPaymentMethodsInputTextField), + readAsList(arbitratorsInputTextField), + readAsList(seedNodesInputTextField), + readAsList(priceRelayNodesInputTextField), + preventPublicBtcNetworkCheckBox.isSelected(), + readAsList(btcNodesInputTextField), + disableDaoCheckBox.isSelected(), + disableDaoBelowVersionInputTextField.getText(), + disableTradeBelowVersionInputTextField.getText(), + readAsList(mediatorsInputTextField), + readAsList(refundAgentsInputTextField), + readAsList(bannedSignerPubKeysInputTextField), + readAsList(btcFeeReceiverAddressesInputTextField), + filterManager.getOwnerPubKey(), + signerPubKeyAsHex + ); + + filterManager.addDevFilter(newFilter, privKeyString); + removeFilterMessageButton.setDisable(filterManager.getDevFilter() == null); hide(); } else { - new Popup().warning(Res.get("shared.invalidKey")).width(300).onClose(this::blurAgain).show(); + new Popup().warning(Res.get("shared.invalidKey")).onClose(this::blurAgain).show(); } }); removeFilterMessageButton.setOnAction(e -> { - if (privKeyString.isEmpty()) { - return; - } - - if (filterManager.isValidDevPrivilegeKey(privKeyString)) { - filterManager.setFilterSigningKey(privKeyString); - filterManager.removeFilter(); + String privKeyString = keyInputTextField.getText(); + if (filterManager.canRemoveDevFilter(privKeyString)) { + filterManager.removeDevFilter(privKeyString); hide(); } else { - new Popup().warning(Res.get("shared.invalidKey")).width(300).onClose(this::blurAgain).show(); + new Popup().warning(Res.get("shared.invalidKey")).onClose(this::blurAgain).show(); } }); diff --git a/proto/src/main/proto/pb.proto b/proto/src/main/proto/pb.proto index cf448e81a5..c1e8de2f5a 100644 --- a/proto/src/main/proto/pb.proto +++ b/proto/src/main/proto/pb.proto @@ -639,6 +639,7 @@ message Filter { repeated string bannedSignerPubKeys = 19; repeated string btc_fee_receiver_addresses = 20; int64 creation_date = 21; + string signer_pub_key_as_hex = 22; } // not used anymore from v0.6 on. But leave it for receiving TradeStatistics objects from older