mirrors/bisq
1
0
Fork 0
mirror of https://github.com/bisq-network/bisq.git synced 2025-03-13 11:09:10 +01:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #3682 from ripcurlx/improve-account-signing

Browse source 
Improve account signing security
This commit is contained in:
Florian Reimair 2019-11-26 12:50:12 +01:00 • committed by GitHub
commit d12843a74c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 65 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
core/src/main/java/bisq/core/account/sign/SignedWitnessService.java
View file

@ -61,6 +61,7 @@ import lombok.extern.slf4j.Slf4j;
public class SignedWitnessService {
public static final long SIGNER_AGE_DAYS = 30;
public static final long SIGNER_AGE = SIGNER_AGE_DAYS * ChronoUnit.DAYS.getDuration().toMillis();
static final Coin MINIMUM_TRADE_AMOUNT_FOR_SIGNING = Coin.parseCoin("0.0025");
private final KeyRing keyRing;
private final P2PService p2PService;
@ -190,6 +191,11 @@ public class SignedWitnessService {
return;
}
if (!isSufficientTradeAmountForSigning(tradeAmount)) {
log.warn("Trader tried to sign account with too little trade amount");
return;
}
byte[] signature = Sig.sign(keyRing.getSignatureKeyPair().getPrivate(), accountAgeWitness.getHash());
SignedWitness signedWitness = new SignedWitness(SignedWitness.VerificationMethod.TRADE,
accountAgeWitness.getHash(),
@ -281,6 +287,10 @@ public class SignedWitnessService {
return isSignerAccountAgeWitness(accountAgeWitness, new Date().getTime());
}
public boolean isSufficientTradeAmountForSigning(Coin tradeAmount) {
return !tradeAmount.isLessThan(MINIMUM_TRADE_AMOUNT_FOR_SIGNING);
}
/**
* Checks whether the accountAgeWitness has a valid signature from a peer/arbitrator and is allowed to sign
* other accounts.

4
core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java
View file

@ -703,6 +703,10 @@ public class AccountAgeWitnessService {
return signedWitnessService.isSignerAccountAgeWitness(accountAgeWitness);
}
public boolean tradeAmountIsSufficient(Coin tradeAmount) {
return signedWitnessService.isSufficientTradeAmountForSigning(tradeAmount);
}
public SignState getSignState(Offer offer) {
return findWitness(offer)
.map(this::getSignState)

2
core/src/main/resources/i18n/displayStrings.properties
View file

@ -736,8 +736,6 @@ portfolio.pending.step5_buyer.takersMiningFee=Total mining fees
portfolio.pending.step5_buyer.refunded=Refunded security deposit
portfolio.pending.step5_buyer.withdrawBTC=Withdraw your bitcoin
portfolio.pending.step5_buyer.amount=Amount to withdraw
portfolio.pending.step5_buyer.signer=By withdrawing your bitcoins, you verify that the \
counterparty has acted according to the trade protocol.
portfolio.pending.step5_buyer.withdrawToAddress=Withdraw to address
portfolio.pending.step5_buyer.moveToBisqWallet=Move funds to Bisq wallet
portfolio.pending.step5_buyer.withdrawExternal=Withdraw to external wallet

45
core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java
View file

@ -21,11 +21,16 @@ package bisq.core.account.sign;
import bisq.core.account.witness.AccountAgeWitness;
import bisq.core.support.dispute.arbitration.arbitrator.ArbitratorManager;
import bisq.network.p2p.P2PService;
import bisq.network.p2p.storage.payload.PersistableNetworkPayload;
import bisq.network.p2p.storage.persistence.AppendOnlyDataStoreService;
import bisq.common.crypto.CryptoException;
import bisq.common.crypto.KeyRing;
import bisq.common.crypto.Sig;
import bisq.common.util.Utilities;
import org.bitcoinj.core.Coin;
import org.bitcoinj.core.ECKey;
import com.google.common.base.Charsets;
@ -44,9 +49,7 @@ import static bisq.core.account.sign.SignedWitness.VerificationMethod.ARBITRATOR
import static bisq.core.account.sign.SignedWitness.VerificationMethod.TRADE;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.*;
public class SignedWitnessServiceTest {
private SignedWitnessService signedWitnessService;
@ -74,6 +77,8 @@ public class SignedWitnessServiceTest {
private long SIGN_AGE_1 = SignedWitnessService.SIGNER_AGE_DAYS * 3 + 5;
private long SIGN_AGE_2 = SignedWitnessService.SIGNER_AGE_DAYS * 2 + 4;
private long SIGN_AGE_3 = SignedWitnessService.SIGNER_AGE_DAYS + 3;
private KeyRing keyRing;
private P2PService p2pService;
@Before
@ -81,7 +86,9 @@ public class SignedWitnessServiceTest {
AppendOnlyDataStoreService appendOnlyDataStoreService = mock(AppendOnlyDataStoreService.class);
ArbitratorManager arbitratorManager = mock(ArbitratorManager.class);
when(arbitratorManager.isPublicKeyInList(any())).thenReturn(true);
signedWitnessService = new SignedWitnessService(null, null, arbitratorManager, null, appendOnlyDataStoreService, null);
keyRing = mock(KeyRing.class);
p2pService = mock(P2PService.class);
signedWitnessService = new SignedWitnessService(keyRing, p2pService, arbitratorManager, null, appendOnlyDataStoreService, null);
account1DataHash = org.bitcoinj.core.Utils.sha256hash160(new byte[]{1});
account2DataHash = org.bitcoinj.core.Utils.sha256hash160(new byte[]{2});
account3DataHash = org.bitcoinj.core.Utils.sha256hash160(new byte[]{3});
@ -331,5 +338,35 @@ public class SignedWitnessServiceTest {
return Instant.ofEpochMilli(new Date().getTime()).minus(days, ChronoUnit.DAYS).toEpochMilli();
}
@Test
public void testSignAccountAgeWitness_withTooLowTradeAmount() throws CryptoException {
long accountCreationTime = getTodayMinusNDays(SIGN_AGE_1 + 1);
KeyPair peerKeyPair = Sig.generateKeyPair();
KeyPair signerKeyPair = Sig.generateKeyPair();
when(keyRing.getSignatureKeyPair()).thenReturn(signerKeyPair);
AccountAgeWitness accountAgeWitness = new AccountAgeWitness(account1DataHash, accountCreationTime);
signedWitnessService.signAccountAgeWitness(Coin.ZERO, accountAgeWitness, peerKeyPair.getPublic());
verify(p2pService, never()).addPersistableNetworkPayload(any(PersistableNetworkPayload.class), anyBoolean());
}
@Test
public void testSignAccountAgeWitness_withSufficientTradeAmount() throws CryptoException {
long accountCreationTime = getTodayMinusNDays(SIGN_AGE_1 + 1);
KeyPair peerKeyPair = Sig.generateKeyPair();
KeyPair signerKeyPair = Sig.generateKeyPair();
when(keyRing.getSignatureKeyPair()).thenReturn(signerKeyPair);
AccountAgeWitness accountAgeWitness = new AccountAgeWitness(account1DataHash, accountCreationTime);
signedWitnessService.signAccountAgeWitness(SignedWitnessService.MINIMUM_TRADE_AMOUNT_FOR_SIGNING, accountAgeWitness, peerKeyPair.getPublic());
verify(p2pService, times(1)).addPersistableNetworkPayload(any(PersistableNetworkPayload.class), anyBoolean());
}
}

15
desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/PendingTradesViewModel.java
View file

@ -32,8 +32,8 @@ import bisq.core.trade.Contract;
import bisq.core.trade.Trade;
import bisq.core.trade.closed.ClosedTradableManager;
import bisq.core.user.User;
import bisq.core.util.coin.BsqFormatter;
import bisq.core.util.FormattingUtils;
import bisq.core.util.coin.BsqFormatter;
import bisq.core.util.coin.CoinFormatter;
import bisq.core.util.validation.BtcAddressValidator;
@ -352,19 +352,18 @@ public class PendingTradesViewModel extends ActivatableWithDataModel<PendingTrad
///////////////////////////////////////////////////////////////////////////////////////////
public boolean isSignWitnessTrade(boolean asSeller) {
public boolean isSignWitnessTrade() {
checkNotNull(trade, "trade must not be null");
checkNotNull(trade.getOffer(), "offer must not be null");
AccountAgeWitness myWitness = accountAgeWitnessService.getMyWitness(asSeller ?
dataModel.getSellersPaymentAccountPayload() :
dataModel.getBuyersPaymentAccountPayload());
AccountAgeWitness myWitness = accountAgeWitnessService.getMyWitness(dataModel.getSellersPaymentAccountPayload());
return accountAgeWitnessService.accountIsSigner(myWitness) &&
!accountAgeWitnessService.peerHasSignedWitness(trade);
!accountAgeWitnessService.peerHasSignedWitness(trade) &&
accountAgeWitnessService.tradeAmountIsSufficient(trade.getTradeAmount());
}
public void maybeSignWitness(boolean asSeller) {
if (isSignWitnessTrade(asSeller)) {
public void maybeSignWitness() {
if (isSignWitnessTrade()) {
accountAgeWitnessService.traderSignPeersAccountAgeWitness(trade);
}
}

8
desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/buyer/BuyerStep4View.java
View file

@ -120,12 +120,6 @@ public class BuyerStep4View extends TradeStepView {
withdrawAddressTextField.setManaged(false);
withdrawAddressTextField.setVisible(false);
if (model.isSignWitnessTrade(false)) {
Label signLabel = new Label(Res.get("portfolio.pending.step5_buyer.signer"));
GridPane.setRowIndex(signLabel, ++gridRow);
gridPane.getChildren().add(signLabel);
}
HBox hBox = new HBox();
hBox.setSpacing(10);
useSavingsWalletButton = new AutoTooltipButton(Res.get("portfolio.pending.step5_buyer.moveToBisqWallet"));
@ -141,12 +135,10 @@ public class BuyerStep4View extends TradeStepView {
gridPane.getChildren().add(hBox);
useSavingsWalletButton.setOnAction(e -> {
model.maybeSignWitness(false);
handleTradeCompleted();
model.dataModel.tradeManager.addTradeToClosedTrades(trade);
});
withdrawToExternalWalletButton.setOnAction(e -> {
model.maybeSignWitness(false);
onWithdrawal();
});

6
desktop/src/main/java/bisq/desktop/main/portfolio/pendingtrades/steps/seller/SellerStep3View.java
View file

@ -294,7 +294,7 @@ public class SellerStep3View extends TradeStepView {
}
}
message += Res.get("portfolio.pending.step3_seller.onPaymentReceived.note");
if (model.isSignWitnessTrade(true)) {
if (model.isSignWitnessTrade()) {
message += Res.get("portfolio.pending.step3_seller.onPaymentReceived.signer");
}
new Popup()
@ -351,7 +351,7 @@ public class SellerStep3View extends TradeStepView {
message += Res.get("portfolio.pending.step3_seller.bankCheck", optionalHolderName.get(), part);
}
if (model.isSignWitnessTrade(true)) {
if (model.isSignWitnessTrade()) {
message += Res.get("portfolio.pending.step3_seller.onPaymentReceived.signer");
}
}
@ -370,7 +370,7 @@ public class SellerStep3View extends TradeStepView {
if (!trade.isPayoutPublished())
trade.setState(Trade.State.SELLER_CONFIRMED_IN_UI_FIAT_PAYMENT_RECEIPT);
model.maybeSignWitness(true);
model.maybeSignWitness();
model.dataModel.onFiatPaymentReceived(() -> {
// In case the first send failed we got the support button displayed.