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

Clear payment account payload info from closed trades. #6001

Merged
merged 1 commit into from Feb 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions core/src/main/java/bisq/core/support/dispute/Dispute.java
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public static protobuf.Dispute.State toProtoMessage(Dispute.State state) {
private final PubKeyRing traderPubKeyRing;
private final long tradeDate;
private final long tradePeriodEnd;
private final Contract contract;
private Contract contract;
@Nullable
private final byte[] contractHash;
@Nullable
Expand All @@ -111,7 +111,7 @@ public static protobuf.Dispute.State toProtoMessage(Dispute.State state) {
private final String depositTxId;
@Nullable
private final String payoutTxId;
private final String contractAsJson;
private String contractAsJson;
@Nullable
private final String makerContractSignature;
@Nullable
Expand Down Expand Up @@ -351,6 +351,31 @@ public void addAndPersistChatMessage(ChatMessage chatMessage) {
}
}

public boolean removeAllChatMessages() {
if (chatMessages.size() > 0) {
chatMessages.clear();
return true;
}
return false;
}

public void maybeClearSensitiveData() {
String change = "";
if (contract.maybeClearSensitiveData()) {
change += "contract;";
}
String edited = contract.sanitizeContractAsJson(contractAsJson);
if (!edited.equals(contractAsJson)) {
contractAsJson = edited;
change += "contractAsJson;";
}
if (removeAllChatMessages()) {
change += "chat messages;";
}
if (change.length() > 0) {
log.info("cleared sensitive data from {} of dispute for trade {}", change, Utilities.getShortId(getTradeId()));
}
}

///////////////////////////////////////////////////////////////////////////////////////////
// Setters
Expand Down
13 changes: 13 additions & 0 deletions core/src/main/java/bisq/core/support/dispute/DisputeManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@

import java.security.KeyPair;

import java.time.Instant;

import java.util.Date;
import java.util.List;
import java.util.Optional;
Expand Down Expand Up @@ -282,6 +284,8 @@ public void onUpdatedDataReceived() {
log.error(disputeReplayException.toString());
validationExceptions.add(disputeReplayException);
});

maybeClearSensitiveData();
}

public boolean isTrader(Dispute dispute) {
Expand All @@ -298,6 +302,15 @@ public Optional<Dispute> findOwnDispute(String tradeId) {
return disputeList.stream().filter(e -> e.getTradeId().equals(tradeId)).findAny();
}

public void maybeClearSensitiveData() {
log.info("{} checking closed disputes eligibility for having sensitive data cleared", super.getClass().getSimpleName());
Instant safeDate = closedTradableManager.getSafeDateForSensitiveDataClearing();
getDisputeList().getList().stream()
.filter(e -> e.isClosed())
.filter(e -> e.getTradeDate().toInstant().isBefore(safeDate))
Comment on lines +309 to +310
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you want to use:

.filter(Dispute::isClosed)
.filter(e -> e.getOpeningDate().toInstant().isBefore(safeDate))

instead?

.forEach(Dispute::maybeClearSensitiveData);
requestPersistence();
}

///////////////////////////////////////////////////////////////////////////////////////////
// Message handler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ public void onFailure(TxBroadcastException exception) {
sendAckMessage(chatMessage, dispute.getAgentPubKeyRing(), success, errorMessage);
}

maybeClearSensitiveData();
requestPersistence();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ public void onDisputeResultMessage(DisputeResultMessage disputeResultMessage) {
}
sendAckMessage(chatMessage, dispute.getAgentPubKeyRing(), true, null);

maybeClearSensitiveData();
requestPersistence();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ public void onDisputeResultMessage(DisputeResultMessage disputeResultMessage) {
openOfferOptional.ifPresent(openOffer -> openOfferManager.closeOpenOffer(openOffer.getOffer()));
}

maybeClearSensitiveData();
requestPersistence();
}

Expand Down
28 changes: 28 additions & 0 deletions core/src/main/java/bisq/core/trade/ClosedTradableManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,12 @@

import javafx.collections.ObservableList;

import java.time.Instant;

import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -120,10 +123,12 @@ public void readPersisted(Runnable completeHandler) {

public void onAllServicesInitialized() {
cleanupMailboxMessagesService.handleTrades(getClosedTrades());
maybeClearSensitiveData();
}

public void add(Tradable tradable) {
if (closedTradables.add(tradable)) {
maybeClearSensitiveData();
requestPersistence();
}
}
Expand Down Expand Up @@ -153,6 +158,29 @@ public Optional<Tradable> getTradableById(String id) {
return closedTradables.stream().filter(e -> e.getId().equals(id)).findFirst();
}

public void maybeClearSensitiveData() {
log.info("checking closed trades eligibility for having sensitive data cleared");
closedTradables.stream()
.filter(e -> e instanceof Trade)
.map(e -> (Trade) e)
.filter(e -> canTradeHaveSensitiveDataCleared(e.getId()))
.forEach(Trade::maybeClearSensitiveData);
requestPersistence();
}

public boolean canTradeHaveSensitiveDataCleared(String tradeId) {
Instant safeDate = getSafeDateForSensitiveDataClearing();
return closedTradables.stream()
.filter(e -> e.getId().equals(tradeId))
.filter(e -> e.getDate().toInstant().isBefore(safeDate))
.count() > 0;
}

public Instant getSafeDateForSensitiveDataClearing() {
return Instant.ofEpochSecond(Instant.now().getEpochSecond()
- TimeUnit.DAYS.toSeconds(preferences.getClearDataAfterDays()));
}

public Stream<Trade> getTradesStreamWithFundsLockedIn() {
return getClosedTrades().stream()
.filter(Trade::isFundsLockedIn);
Expand Down
24 changes: 24 additions & 0 deletions core/src/main/java/bisq/core/trade/model/bisq_v1/Contract.java
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,30 @@ public boolean isMyRoleMaker(PubKeyRing myPubKeyRing) {
return isBuyerMakerAndSellerTaker() == isMyRoleBuyer(myPubKeyRing);
}

public boolean maybeClearSensitiveData() {
boolean changed = false;
if (makerPaymentAccountPayload != null) {
makerPaymentAccountPayload = null;
changed = true;
}
if (takerPaymentAccountPayload != null) {
takerPaymentAccountPayload = null;
changed = true;
}
return changed;
}

// edits a contract json string, removing the payment account payloads
public static String sanitizeContractAsJson(String contractAsJson) {
return contractAsJson
.replaceAll(
"\"takerPaymentAccountPayload\": \\{[^}]*}",
"\"takerPaymentAccountPayload\": null")
.replaceAll(
"\"makerPaymentAccountPayload\": \\{[^}]*}",
"\"makerPaymentAccountPayload\": null");
}

public void printDiff(@Nullable String peersContractAsJson) {
String json = JsonUtil.objectToJson(this);
String diff = StringUtils.difference(json, peersContractAsJson);
Expand Down
28 changes: 28 additions & 0 deletions core/src/main/java/bisq/core/trade/model/bisq_v1/Trade.java
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,14 @@ public void addAndPersistChatMessage(ChatMessage chatMessage) {
}
}

public boolean removeAllChatMessages() {
if (chatMessages.size() > 0) {
chatMessages.clear();
return true;
}
return false;
}

public boolean mediationResultAppliedPenaltyToSeller() {
// If mediated payout is same or more then normal payout we enable otherwise a penalty was applied
// by mediators and we keep the confirm disabled to avoid that the seller can complete the trade
Expand All @@ -698,6 +706,26 @@ public boolean mediationResultAppliedPenaltyToSeller() {
return payoutAmountFromMediation < normalPayoutAmount;
}

public void maybeClearSensitiveData() {
String change = "";
if (contract != null && contract.maybeClearSensitiveData()) {
change += "contract;";
}
if (contractAsJson != null) {
String edited = contract.sanitizeContractAsJson(contractAsJson);
if (!edited.equals(contractAsJson)) {
contractAsJson = edited;
change += "contractAsJson;";
}
}
if (removeAllChatMessages()) {
change += "chat messages;";
}
if (change.length() > 0) {
log.info("cleared sensitive data from {} of trade {}", change, getShortId());
}
}


///////////////////////////////////////////////////////////////////////////////////////////
// TradeModel implementation
Expand Down
12 changes: 11 additions & 1 deletion core/src/main/java/bisq/core/user/Preferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ public final class Preferences implements PersistedDataHost, BridgeAddressProvid
));

public static final boolean USE_SYMMETRIC_SECURITY_DEPOSIT = true;

public static final int CLEAR_DATA_AFTER_DAYS_INITIAL = 99999; // feature effectively disabled until user agrees to settings notification
public static final int CLEAR_DATA_AFTER_DAYS_DEFAULT = 20; // used when user has agreed to settings notification

// payload is initialized so the default values are available for Property initialization.
@Setter
Expand Down Expand Up @@ -355,6 +356,10 @@ private void setupPreferences() {
setIgnoreDustThreshold(600);
}

if (prefPayload.getClearDataAfterDays() < 1) {
setClearDataAfterDays(Preferences.CLEAR_DATA_AFTER_DAYS_INITIAL);
}

// For users from old versions the 4 flags a false but we want to have it true by default
// PhoneKeyAndToken is also null so we can use that to enable the flags
if (prefPayload.getPhoneKeyAndToken() == null) {
Expand Down Expand Up @@ -785,6 +790,11 @@ public void setIgnoreDustThreshold(int value) {
requestPersistence();
}

public void setClearDataAfterDays(int value) {
prefPayload.setClearDataAfterDays(value);
requestPersistence();
}

public void setShowOffersMatchingMyAccounts(boolean value) {
prefPayload.setShowOffersMatchingMyAccounts(value);
requestPersistence();
Expand Down
3 changes: 3 additions & 0 deletions core/src/main/java/bisq/core/user/PreferencesPayload.java
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ public final class PreferencesPayload implements PersistableEnvelope {
private String takeOfferSelectedPaymentAccountId;
private double buyerSecurityDepositAsPercent = getDefaultBuyerSecurityDepositAsPercent();
private int ignoreDustThreshold = 600;
private int clearDataAfterDays = Preferences.CLEAR_DATA_AFTER_DAYS_INITIAL;
private double buyerSecurityDepositAsPercentForCrypto = getDefaultBuyerSecurityDepositAsPercent();
private int blockNotifyPort;
private boolean tacAcceptedV120;
Expand Down Expand Up @@ -192,6 +193,7 @@ public Message toProtoMessage() {
.setIsDaoFullNode(isDaoFullNode)
.setBuyerSecurityDepositAsPercent(buyerSecurityDepositAsPercent)
.setIgnoreDustThreshold(ignoreDustThreshold)
.setClearDataAfterDays(clearDataAfterDays)
.setBuyerSecurityDepositAsPercentForCrypto(buyerSecurityDepositAsPercentForCrypto)
.setBlockNotifyPort(blockNotifyPort)
.setTacAcceptedV120(tacAcceptedV120)
Expand Down Expand Up @@ -290,6 +292,7 @@ public static PreferencesPayload fromProto(protobuf.PreferencesPayload proto, Co
proto.getTakeOfferSelectedPaymentAccountId().isEmpty() ? null : proto.getTakeOfferSelectedPaymentAccountId(),
proto.getBuyerSecurityDepositAsPercent(),
proto.getIgnoreDustThreshold(),
proto.getClearDataAfterDays(),
proto.getBuyerSecurityDepositAsPercentForCrypto(),
proto.getBlockNotifyPort(),
proto.getTacAcceptedV120(),
Expand Down
10 changes: 10 additions & 0 deletions core/src/main/resources/i18n/displayStrings.properties
Original file line number Diff line number Diff line change
Expand Up @@ -1324,6 +1324,7 @@ setting.preferences.txFeeMin=Transaction fee must be at least {0} satoshis/vbyte
setting.preferences.txFeeTooLarge=Your input is above any reasonable value (>5000 satoshis/vbyte). Transaction fee is usually in the range of 50-400 satoshis/vbyte.
setting.preferences.ignorePeers=Ignored peers [onion address:port]
setting.preferences.ignoreDustThreshold=Min. non-dust output value
setting.preferences.clearDataAfterDays=Clear sensitive data after (days)
setting.preferences.currenciesInList=Currencies in market price feed list
setting.preferences.prefCurrency=Preferred currency
setting.preferences.displayFiat=Display national currencies
Expand Down Expand Up @@ -1386,6 +1387,15 @@ settings.preferences.editCustomExplorer.name=Name
settings.preferences.editCustomExplorer.txUrl=Transaction URL
settings.preferences.editCustomExplorer.addressUrl=Address URL

setting.info.headline=New data-privacy feature
settings.preferences.sensitiveDataRemoval.msg=To protect the privacy of yourself and other traders, Bisq intends to \
remove payment account details from old trades. This is particularly important for fiat trades which may include bank \
account details. Read more about this at [HYPERLINK:https://bisq.wiki/Data_Privacy].\n\n\
The threshold for data removal can be configured on this screen via the field "Clear sensitive data after (days)". \
It is recommended to set it as low as possible, for example 20 days. That means trades from more than 20 \
days ago will have payment account details cleared, as long as they are finished. Finished trades are ones which \
are found in the Portfolio / History tab.

settings.net.btcHeader=Bitcoin network
settings.net.p2pHeader=Bisq network
settings.net.onionAddressLabel=My onion address
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,6 @@ public BooleanProperty getShowSettingsUpdatesNotification() {
}

public void setup() {
showNotification.set(preferences.showAgain(SETTINGS_NEWS));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@
import bisq.desktop.common.view.View;
import bisq.desktop.common.view.ViewLoader;
import bisq.desktop.main.MainView;
import bisq.desktop.main.overlays.popups.Popup;
import bisq.desktop.main.presentation.SettingsPresentation;
import bisq.desktop.main.settings.about.AboutView;
import bisq.desktop.main.settings.network.NetworkSettingsView;
import bisq.desktop.main.settings.preferences.PreferencesView;

import bisq.core.locale.Res;
import bisq.core.user.DontShowAgainLookup;
import bisq.core.user.Preferences;

import javax.inject.Inject;
Expand Down Expand Up @@ -85,6 +88,9 @@ else if (newValue == aboutTab)

@Override
protected void activate() {
// Hide new badge if user saw this section
preferences.dontShowAgain(SettingsPresentation.SETTINGS_NEWS, true);

root.getSelectionModel().selectedItemProperty().addListener(tabChangeListener);
navigation.addListener(navigationListener);

Expand Down
Loading