From 00953c2d1679a80ba3f5ce915ba5af0e3c348645 Mon Sep 17 00:00:00 2001 From: sqrrm Date: Thu, 10 Oct 2019 14:12:17 +0200 Subject: [PATCH 1/4] Rename witnessHash -> accountAgeWitnessHash --- common/src/main/proto/pb.proto | 2 +- .../java/bisq/core/account/sign/SignedWitness.java | 14 +++++++------- .../core/account/sign/SignedWitnessService.java | 10 +++++----- .../bisq/core/account/sign/SignedWitnessTest.java | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/common/src/main/proto/pb.proto b/common/src/main/proto/pb.proto index 43911ea5f75..405a24f58c1 100644 --- a/common/src/main/proto/pb.proto +++ b/common/src/main/proto/pb.proto @@ -723,7 +723,7 @@ message AccountAgeWitness { message SignedWitness { bool signed_by_arbitrator = 1; - bytes witness_hash = 2; + bytes account_age_witness_hash = 2; bytes signature = 3; bytes signer_pub_key = 4; bytes witness_owner_pub_key = 5; diff --git a/core/src/main/java/bisq/core/account/sign/SignedWitness.java b/core/src/main/java/bisq/core/account/sign/SignedWitness.java index 111bb195cbd..f88ae3cde46 100644 --- a/core/src/main/java/bisq/core/account/sign/SignedWitness.java +++ b/core/src/main/java/bisq/core/account/sign/SignedWitness.java @@ -49,7 +49,7 @@ public class SignedWitness implements LazyProcessedPayload, PersistableNetworkPa private static final long TOLERANCE = TimeUnit.DAYS.toMillis(1); private final boolean signedByArbitrator; - private final byte[] witnessHash; + private final byte[] accountAgeWitnessHash; private final byte[] signature; private final byte[] signerPubKey; private final byte[] witnessOwnerPubKey; @@ -59,14 +59,14 @@ public class SignedWitness implements LazyProcessedPayload, PersistableNetworkPa transient private final byte[] hash; public SignedWitness(boolean signedByArbitrator, - byte[] witnessHash, + byte[] accountAgeWitnessHash, byte[] signature, byte[] signerPubKey, byte[] witnessOwnerPubKey, long date, long tradeAmount) { this.signedByArbitrator = signedByArbitrator; - this.witnessHash = witnessHash.clone(); + this.accountAgeWitnessHash = accountAgeWitnessHash.clone(); this.signature = signature.clone(); this.signerPubKey = signerPubKey.clone(); this.witnessOwnerPubKey = witnessOwnerPubKey.clone(); @@ -80,7 +80,7 @@ public SignedWitness(boolean signedByArbitrator, // same peer to add more security as if that one would be colluding it would be not detected anyway. The total // number of signed trades with different peers is still available and can be considered more valuable data for // security. - byte[] data = Utilities.concatenateByteArrays(witnessHash, signature); + byte[] data = Utilities.concatenateByteArrays(accountAgeWitnessHash, signature); data = Utilities.concatenateByteArrays(data, signerPubKey); hash = Hash.getSha256Ripemd160hash(data); } @@ -94,7 +94,7 @@ public SignedWitness(boolean signedByArbitrator, public protobuf.PersistableNetworkPayload toProtoMessage() { final protobuf.SignedWitness.Builder builder = protobuf.SignedWitness.newBuilder() .setSignedByArbitrator(signedByArbitrator) - .setWitnessHash(ByteString.copyFrom(witnessHash)) + .setAccountAgeWitnessHash(ByteString.copyFrom(accountAgeWitnessHash)) .setSignature(ByteString.copyFrom(signature)) .setSignerPubKey(ByteString.copyFrom(signerPubKey)) .setWitnessOwnerPubKey(ByteString.copyFrom(witnessOwnerPubKey)) @@ -109,7 +109,7 @@ protobuf.SignedWitness toProtoSignedWitness() { public static SignedWitness fromProto(protobuf.SignedWitness proto) { return new SignedWitness(proto.getSignedByArbitrator(), - proto.getWitnessHash().toByteArray(), + proto.getAccountAgeWitnessHash().toByteArray(), proto.getSignature().toByteArray(), proto.getSignerPubKey().toByteArray(), proto.getWitnessOwnerPubKey().toByteArray(), @@ -158,7 +158,7 @@ P2PDataStorage.ByteArray getHashAsByteArray() { public String toString() { return "SignedWitness{" + ",\n signedByArbitrator=" + signedByArbitrator + - ",\n witnessHash=" + Utilities.bytesAsHexString(witnessHash) + + ",\n witnessHash=" + Utilities.bytesAsHexString(accountAgeWitnessHash) + ",\n signature=" + Utilities.bytesAsHexString(signature) + ",\n signerPubKey=" + Utilities.bytesAsHexString(signerPubKey) + ",\n witnessOwnerPubKey=" + Utilities.bytesAsHexString(witnessOwnerPubKey) + diff --git a/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java b/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java index f847a3d13e6..9867a9829cb 100644 --- a/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java +++ b/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java @@ -193,7 +193,7 @@ public boolean verifySignature(SignedWitness signedWitness) { private boolean verifySignatureWithECKey(SignedWitness signedWitness) { try { - String message = Utilities.encodeToHex(signedWitness.getWitnessHash()); + String message = Utilities.encodeToHex(signedWitness.getAccountAgeWitnessHash()); String signatureBase64 = new String(signedWitness.getSignature(), Charsets.UTF_8); ECKey key = ECKey.fromPublicOnly(signedWitness.getSignerPubKey()); if (arbitratorManager.isPublicKeyInList(Utilities.encodeToHex(key.getPubKey()))) { @@ -213,7 +213,7 @@ private boolean verifySignatureWithECKey(SignedWitness signedWitness) { private boolean verifySignatureWithDSAKey(SignedWitness signedWitness) { try { PublicKey signaturePubKey = Sig.getPublicKeyFromBytes(signedWitness.getSignerPubKey()); - Sig.verify(signaturePubKey, signedWitness.getWitnessHash(), signedWitness.getSignature()); + Sig.verify(signaturePubKey, signedWitness.getAccountAgeWitnessHash(), signedWitness.getSignature()); return true; } catch (CryptoException e) { log.warn("verifySignature signedWitness failed. signedWitness={}", signedWitness); @@ -224,7 +224,7 @@ private boolean verifySignatureWithDSAKey(SignedWitness signedWitness) { private Set getSignedWitnessSet(AccountAgeWitness accountAgeWitness) { return signedWitnessMap.values().stream() - .filter(e -> Arrays.equals(e.getWitnessHash(), accountAgeWitness.getHash())) + .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), accountAgeWitness.getHash())) .collect(Collectors.toSet()); } @@ -232,7 +232,7 @@ private Set getSignedWitnessSet(AccountAgeWitness accountAgeWitne public Set getArbitratorsSignedWitnessSet(AccountAgeWitness accountAgeWitness) { return signedWitnessMap.values().stream() .filter(SignedWitness::isSignedByArbitrator) - .filter(e -> Arrays.equals(e.getWitnessHash(), accountAgeWitness.getHash())) + .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), accountAgeWitness.getHash())) .collect(Collectors.toSet()); } @@ -240,7 +240,7 @@ public Set getArbitratorsSignedWitnessSet(AccountAgeWitness accou public Set getTrustedPeerSignedWitnessSet(AccountAgeWitness accountAgeWitness) { return signedWitnessMap.values().stream() .filter(e -> !e.isSignedByArbitrator()) - .filter(e -> Arrays.equals(e.getWitnessHash(), accountAgeWitness.getHash())) + .filter(e -> Arrays.equals(e.getAccountAgeWitnessHash(), accountAgeWitness.getHash())) .collect(Collectors.toSet()); } diff --git a/core/src/test/java/bisq/core/account/sign/SignedWitnessTest.java b/core/src/test/java/bisq/core/account/sign/SignedWitnessTest.java index 305a539151c..8911e94079c 100644 --- a/core/src/test/java/bisq/core/account/sign/SignedWitnessTest.java +++ b/core/src/test/java/bisq/core/account/sign/SignedWitnessTest.java @@ -41,9 +41,9 @@ public void testProtoRoundTrip() { public void isImmutable() { byte[] signerPubkey = arbitrator1Key.getPubKey(); SignedWitness signedWitness = new SignedWitness(true, witnessHash, witnessHashSignature, signerPubkey, witnessOwner1PubKey, Instant.now().getEpochSecond(), 100); - byte[] originalWitnessHash = signedWitness.getWitnessHash().clone(); + byte[] originalWitnessHash = signedWitness.getAccountAgeWitnessHash().clone(); witnessHash[0] += 1; - assertArrayEquals(originalWitnessHash, signedWitness.getWitnessHash()); + assertArrayEquals(originalWitnessHash, signedWitness.getAccountAgeWitnessHash()); byte[] originalWitnessHashSignature = signedWitness.getSignature().clone(); witnessHashSignature[0] += 1; From 0ae162dfb6a26b611b4e0f7562d94dfe13cba1bc Mon Sep 17 00:00:00 2001 From: sqrrm Date: Fri, 11 Oct 2019 00:08:05 +0200 Subject: [PATCH 2/4] Add enum for SignedWitness verification method --- common/src/main/proto/pb.proto | 8 ++- .../bisq/core/account/sign/SignedWitness.java | 31 ++++++++--- .../account/sign/SignedWitnessService.java | 4 +- .../main/java/bisq/core/app/BisqSetup.java | 2 +- .../sign/SignedWitnessServiceTest.java | 52 ++++++++++--------- .../core/account/sign/SignedWitnessTest.java | 6 ++- 6 files changed, 66 insertions(+), 37 deletions(-) diff --git a/common/src/main/proto/pb.proto b/common/src/main/proto/pb.proto index 405a24f58c1..b8c9a2b4c5d 100644 --- a/common/src/main/proto/pb.proto +++ b/common/src/main/proto/pb.proto @@ -722,7 +722,13 @@ message AccountAgeWitness { } message SignedWitness { - bool signed_by_arbitrator = 1; + enum VerificationMethod { + PB_ERROR = 0; + ARBITRATOR = 1; + TRADE = 2; + } + + VerificationMethod verification_method = 1; bytes account_age_witness_hash = 2; bytes signature = 3; bytes signer_pub_key = 4; diff --git a/core/src/main/java/bisq/core/account/sign/SignedWitness.java b/core/src/main/java/bisq/core/account/sign/SignedWitness.java index f88ae3cde46..1190c15e3c9 100644 --- a/core/src/main/java/bisq/core/account/sign/SignedWitness.java +++ b/core/src/main/java/bisq/core/account/sign/SignedWitness.java @@ -26,6 +26,7 @@ import bisq.common.app.Capabilities; import bisq.common.app.Capability; import bisq.common.crypto.Hash; +import bisq.common.proto.ProtoUtil; import bisq.common.proto.persistable.PersistableEnvelope; import bisq.common.util.Utilities; @@ -46,9 +47,23 @@ @Value public class SignedWitness implements LazyProcessedPayload, PersistableNetworkPayload, PersistableEnvelope, DateTolerantPayload, CapabilityRequiringPayload { + + public enum VerificationMethod { + ARBITRATOR, + TRADE; + + public static SignedWitness.VerificationMethod fromProto(protobuf.SignedWitness.VerificationMethod method) { + return ProtoUtil.enumFromProto(SignedWitness.VerificationMethod.class, method.name()); + } + + public static protobuf.SignedWitness.VerificationMethod toProtoMessage(SignedWitness.VerificationMethod method) { + return protobuf.SignedWitness.VerificationMethod.valueOf(method.name()); + } + } + private static final long TOLERANCE = TimeUnit.DAYS.toMillis(1); - private final boolean signedByArbitrator; + private final VerificationMethod verificationMethod; private final byte[] accountAgeWitnessHash; private final byte[] signature; private final byte[] signerPubKey; @@ -58,14 +73,14 @@ public class SignedWitness implements LazyProcessedPayload, PersistableNetworkPa transient private final byte[] hash; - public SignedWitness(boolean signedByArbitrator, + public SignedWitness(VerificationMethod verificationMethod, byte[] accountAgeWitnessHash, byte[] signature, byte[] signerPubKey, byte[] witnessOwnerPubKey, long date, long tradeAmount) { - this.signedByArbitrator = signedByArbitrator; + this.verificationMethod = verificationMethod; this.accountAgeWitnessHash = accountAgeWitnessHash.clone(); this.signature = signature.clone(); this.signerPubKey = signerPubKey.clone(); @@ -93,7 +108,7 @@ public SignedWitness(boolean signedByArbitrator, @Override public protobuf.PersistableNetworkPayload toProtoMessage() { final protobuf.SignedWitness.Builder builder = protobuf.SignedWitness.newBuilder() - .setSignedByArbitrator(signedByArbitrator) + .setVerificationMethod(VerificationMethod.toProtoMessage(verificationMethod)) .setAccountAgeWitnessHash(ByteString.copyFrom(accountAgeWitnessHash)) .setSignature(ByteString.copyFrom(signature)) .setSignerPubKey(ByteString.copyFrom(signerPubKey)) @@ -108,7 +123,8 @@ protobuf.SignedWitness toProtoSignedWitness() { } public static SignedWitness fromProto(protobuf.SignedWitness proto) { - return new SignedWitness(proto.getSignedByArbitrator(), + return new SignedWitness( + SignedWitness.VerificationMethod.fromProto(proto.getVerificationMethod()), proto.getAccountAgeWitnessHash().toByteArray(), proto.getSignature().toByteArray(), proto.getSignerPubKey().toByteArray(), @@ -145,6 +161,9 @@ public byte[] getHash() { return hash; } + public boolean isSignedByArbitrator() { + return verificationMethod == VerificationMethod.ARBITRATOR; + } /////////////////////////////////////////////////////////////////////////////////////////// // Getters @@ -157,7 +176,7 @@ P2PDataStorage.ByteArray getHashAsByteArray() { @Override public String toString() { return "SignedWitness{" + - ",\n signedByArbitrator=" + signedByArbitrator + + ",\n verificationMethod=" + verificationMethod + ",\n witnessHash=" + Utilities.bytesAsHexString(accountAgeWitnessHash) + ",\n signature=" + Utilities.bytesAsHexString(signature) + ",\n signerPubKey=" + Utilities.bytesAsHexString(signerPubKey) + diff --git a/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java b/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java index 9867a9829cb..6febe7528fd 100644 --- a/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java +++ b/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java @@ -149,7 +149,7 @@ public SignedWitness signAccountAgeWitness(Coin tradeAmount, String accountAgeWitnessHashAsHex = Utilities.encodeToHex(accountAgeWitness.getHash()); String signatureBase64 = key.signMessage(accountAgeWitnessHashAsHex); - SignedWitness signedWitness = new SignedWitness(true, + SignedWitness signedWitness = new SignedWitness(SignedWitness.VerificationMethod.ARBITRATOR, accountAgeWitness.getHash(), signatureBase64.getBytes(Charsets.UTF_8), key.getPubKey(), @@ -171,7 +171,7 @@ public SignedWitness signAccountAgeWitness(Coin tradeAmount, } byte[] signature = Sig.sign(keyRing.getSignatureKeyPair().getPrivate(), accountAgeWitness.getHash()); - SignedWitness signedWitness = new SignedWitness(false, + SignedWitness signedWitness = new SignedWitness(SignedWitness.VerificationMethod.TRADE, accountAgeWitness.getHash(), signature, keyRing.getSignatureKeyPair().getPublic().getEncoded(), diff --git a/core/src/main/java/bisq/core/app/BisqSetup.java b/core/src/main/java/bisq/core/app/BisqSetup.java index 95aa8cfcf29..9562a23b2fb 100644 --- a/core/src/main/java/bisq/core/app/BisqSetup.java +++ b/core/src/main/java/bisq/core/app/BisqSetup.java @@ -775,7 +775,7 @@ private boolean isSignedWitnessOfMineWithState(PersistableNetworkPayload payload // Check if new signed witness is for one of my own accounts return user.getPaymentAccounts().stream() .filter(a -> PaymentMethod.hasChargebackRisk(a.getPaymentMethod(), a.getTradeCurrencies())) - .filter(a -> Arrays.equals(((SignedWitness) payload).getWitnessHash(), + .filter(a -> Arrays.equals(((SignedWitness) payload).getAccountAgeWitnessHash(), accountAgeWitnessService.getMyWitness(a.getPaymentAccountPayload()).getHash())) .anyMatch(a -> accountAgeWitnessService.getSignState(accountAgeWitnessService.getMyWitness( a.getPaymentAccountPayload())).equals(state)); diff --git a/core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java b/core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java index 711bbb7eba5..8f6dac6f7bb 100644 --- a/core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java +++ b/core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java @@ -40,6 +40,8 @@ import org.junit.Before; import org.junit.Test; +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; @@ -112,9 +114,9 @@ public void setup() throws Exception { @Test public void testIsValidAccountAgeWitnessOk() { - SignedWitness sw1 = new SignedWitness(true, account1DataHash, signature1, signer1PubKey, witnessOwner1PubKey, date1, tradeAmount1); - SignedWitness sw2 = new SignedWitness(false, account2DataHash, signature2, signer2PubKey, witnessOwner2PubKey, date2, tradeAmount2); - SignedWitness sw3 = new SignedWitness(false, account3DataHash, signature3, signer3PubKey, witnessOwner3PubKey, date3, tradeAmount3); + SignedWitness sw1 = new SignedWitness(ARBITRATOR, account1DataHash, signature1, signer1PubKey, witnessOwner1PubKey, date1, tradeAmount1); + SignedWitness sw2 = new SignedWitness(TRADE, account2DataHash, signature2, signer2PubKey, witnessOwner2PubKey, date2, tradeAmount2); + SignedWitness sw3 = new SignedWitness(TRADE, account3DataHash, signature3, signer3PubKey, witnessOwner3PubKey, date3, tradeAmount3); signedWitnessService.addToMap(sw1); signedWitnessService.addToMap(sw2); @@ -129,9 +131,9 @@ public void testIsValidAccountAgeWitnessOk() { public void testIsValidAccountAgeWitnessArbitratorSignatureProblem() { signature1 = new byte[]{1, 2, 3}; - SignedWitness sw1 = new SignedWitness(true, account1DataHash, signature1, signer1PubKey, witnessOwner1PubKey, date1, tradeAmount1); - SignedWitness sw2 = new SignedWitness(false, account2DataHash, signature2, signer2PubKey, witnessOwner2PubKey, date2, tradeAmount2); - SignedWitness sw3 = new SignedWitness(false, account3DataHash, signature3, signer3PubKey, witnessOwner3PubKey, date3, tradeAmount3); + SignedWitness sw1 = new SignedWitness(ARBITRATOR, account1DataHash, signature1, signer1PubKey, witnessOwner1PubKey, date1, tradeAmount1); + SignedWitness sw2 = new SignedWitness(TRADE, account2DataHash, signature2, signer2PubKey, witnessOwner2PubKey, date2, tradeAmount2); + SignedWitness sw3 = new SignedWitness(TRADE, account3DataHash, signature3, signer3PubKey, witnessOwner3PubKey, date3, tradeAmount3); signedWitnessService.addToMap(sw1); signedWitnessService.addToMap(sw2); @@ -146,9 +148,9 @@ public void testIsValidAccountAgeWitnessArbitratorSignatureProblem() { public void testIsValidAccountAgeWitnessPeerSignatureProblem() { signature2 = new byte[]{1, 2, 3}; - SignedWitness sw1 = new SignedWitness(true, account1DataHash, signature1, signer1PubKey, witnessOwner1PubKey, date1, tradeAmount1); - SignedWitness sw2 = new SignedWitness(false, account2DataHash, signature2, signer2PubKey, witnessOwner2PubKey, date2, tradeAmount2); - SignedWitness sw3 = new SignedWitness(false, account3DataHash, signature3, signer3PubKey, witnessOwner3PubKey, date3, tradeAmount3); + SignedWitness sw1 = new SignedWitness(ARBITRATOR, account1DataHash, signature1, signer1PubKey, witnessOwner1PubKey, date1, tradeAmount1); + SignedWitness sw2 = new SignedWitness(TRADE, account2DataHash, signature2, signer2PubKey, witnessOwner2PubKey, date2, tradeAmount2); + SignedWitness sw3 = new SignedWitness(TRADE, account3DataHash, signature3, signer3PubKey, witnessOwner3PubKey, date3, tradeAmount3); signedWitnessService.addToMap(sw1); signedWitnessService.addToMap(sw2); @@ -167,9 +169,9 @@ public void testIsValidSelfSignatureOk() throws Exception { signature2 = Sig.sign(peer1KeyPair.getPrivate(), Utilities.encodeToHex(account2DataHash).getBytes(Charsets.UTF_8)); signature3 = Sig.sign(peer1KeyPair.getPrivate(), Utilities.encodeToHex(account3DataHash).getBytes(Charsets.UTF_8)); - SignedWitness sw1 = new SignedWitness(true, account1DataHash, signature1, signer1PubKey, signer2PubKey, date1, tradeAmount1); - SignedWitness sw2 = new SignedWitness(false, account2DataHash, signature2, signer2PubKey, signer2PubKey, date2, tradeAmount2); - SignedWitness sw3 = new SignedWitness(false, account3DataHash, signature3, signer2PubKey, signer2PubKey, date3, tradeAmount3); + SignedWitness sw1 = new SignedWitness(ARBITRATOR, account1DataHash, signature1, signer1PubKey, signer2PubKey, date1, tradeAmount1); + SignedWitness sw2 = new SignedWitness(TRADE, account2DataHash, signature2, signer2PubKey, signer2PubKey, date2, tradeAmount2); + SignedWitness sw3 = new SignedWitness(TRADE, account3DataHash, signature3, signer2PubKey, signer2PubKey, date3, tradeAmount3); signedWitnessService.addToMap(sw1); signedWitnessService.addToMap(sw2); @@ -192,9 +194,9 @@ public void testIsValidSimpleLoopSignatureProblem() throws Exception { signature2 = Sig.sign(peer1KeyPair.getPrivate(), Utilities.encodeToHex(account2DataHash).getBytes(Charsets.UTF_8)); signature3 = Sig.sign(peer2KeyPair.getPrivate(), Utilities.encodeToHex(account3DataHash).getBytes(Charsets.UTF_8)); - SignedWitness sw1 = new SignedWitness(true, account1DataHash, signature1, signer1PubKey, user1PubKey, date1, tradeAmount1); - SignedWitness sw2 = new SignedWitness(false, account2DataHash, signature2, user1PubKey, user2PubKey, date2, tradeAmount2); - SignedWitness sw3 = new SignedWitness(false, account3DataHash, signature3, user2PubKey, user1PubKey, date3, tradeAmount3); + SignedWitness sw1 = new SignedWitness(ARBITRATOR, account1DataHash, signature1, signer1PubKey, user1PubKey, date1, tradeAmount1); + SignedWitness sw2 = new SignedWitness(TRADE, account2DataHash, signature2, user1PubKey, user2PubKey, date2, tradeAmount2); + SignedWitness sw3 = new SignedWitness(TRADE, account3DataHash, signature3, user2PubKey, user1PubKey, date3, tradeAmount3); signedWitnessService.addToMap(sw1); signedWitnessService.addToMap(sw2); @@ -209,9 +211,9 @@ public void testIsValidSimpleLoopSignatureProblem() throws Exception { public void testIsValidAccountAgeWitnessDateTooSoonProblem() { date3 = getTodayMinusNDays(SIGN_AGE_2 - 1); - SignedWitness sw1 = new SignedWitness(true, account1DataHash, signature1, signer1PubKey, witnessOwner1PubKey, date1, tradeAmount1); - SignedWitness sw2 = new SignedWitness(false, account2DataHash, signature2, signer2PubKey, witnessOwner2PubKey, date2, tradeAmount2); - SignedWitness sw3 = new SignedWitness(false, account3DataHash, signature3, signer3PubKey, witnessOwner3PubKey, date3, tradeAmount3); + SignedWitness sw1 = new SignedWitness(ARBITRATOR, account1DataHash, signature1, signer1PubKey, witnessOwner1PubKey, date1, tradeAmount1); + SignedWitness sw2 = new SignedWitness(TRADE, account2DataHash, signature2, signer2PubKey, witnessOwner2PubKey, date2, tradeAmount2); + SignedWitness sw3 = new SignedWitness(TRADE, account3DataHash, signature3, signer3PubKey, witnessOwner3PubKey, date3, tradeAmount3); signedWitnessService.addToMap(sw1); signedWitnessService.addToMap(sw2); @@ -226,9 +228,9 @@ public void testIsValidAccountAgeWitnessDateTooSoonProblem() { public void testIsValidAccountAgeWitnessDateTooLateProblem() { date3 = getTodayMinusNDays(3); - SignedWitness sw1 = new SignedWitness(true, account1DataHash, signature1, signer1PubKey, witnessOwner1PubKey, date1, tradeAmount1); - SignedWitness sw2 = new SignedWitness(false, account2DataHash, signature2, signer2PubKey, witnessOwner2PubKey, date2, tradeAmount2); - SignedWitness sw3 = new SignedWitness(false, account3DataHash, signature3, signer3PubKey, witnessOwner3PubKey, date3, tradeAmount3); + SignedWitness sw1 = new SignedWitness(ARBITRATOR, account1DataHash, signature1, signer1PubKey, witnessOwner1PubKey, date1, tradeAmount1); + SignedWitness sw2 = new SignedWitness(TRADE, account2DataHash, signature2, signer2PubKey, witnessOwner2PubKey, date2, tradeAmount2); + SignedWitness sw3 = new SignedWitness(TRADE, account3DataHash, signature3, signer3PubKey, witnessOwner3PubKey, date3, tradeAmount3); signedWitnessService.addToMap(sw1); signedWitnessService.addToMap(sw2); @@ -278,9 +280,9 @@ public void testIsValidAccountAgeWitnessEndlessLoop() throws Exception { long tradeAmount2 = 1001; long tradeAmount3 = 1001; - SignedWitness sw1 = new SignedWitness(false, account1DataHash, signature1, signer1PubKey, witnessOwner1PubKey, date1, tradeAmount1); - SignedWitness sw2 = new SignedWitness(false, account2DataHash, signature2, signer2PubKey, witnessOwner2PubKey, date2, tradeAmount2); - SignedWitness sw3 = new SignedWitness(false, account3DataHash, signature3, signer3PubKey, witnessOwner3PubKey, date3, tradeAmount3); + SignedWitness sw1 = new SignedWitness(TRADE, account1DataHash, signature1, signer1PubKey, witnessOwner1PubKey, date1, tradeAmount1); + SignedWitness sw2 = new SignedWitness(TRADE, account2DataHash, signature2, signer2PubKey, witnessOwner2PubKey, date2, tradeAmount2); + SignedWitness sw3 = new SignedWitness(TRADE, account3DataHash, signature3, signer3PubKey, witnessOwner3PubKey, date3, tradeAmount3); signedWitnessService.addToMap(sw1); signedWitnessService.addToMap(sw2); @@ -318,7 +320,7 @@ public void testIsValidAccountAgeWitnessLongLoop() throws Exception { } byte[] witnessOwnerPubKey = Sig.getPublicKeyBytes(signedKeyPair.getPublic()); long date = getTodayMinusNDays((iterations - i) * (SignedWitnessService.SIGNER_AGE_DAYS + 1)); - SignedWitness sw = new SignedWitness(i == 0, accountDataHash, signature, signerPubKey, witnessOwnerPubKey, date, tradeAmount1); + SignedWitness sw = new SignedWitness(i == 0 ? ARBITRATOR : TRADE, accountDataHash, signature, signerPubKey, witnessOwnerPubKey, date, tradeAmount1); signedWitnessService.addToMap(sw); } assertFalse(signedWitnessService.isValidAccountAgeWitness(aew)); diff --git a/core/src/test/java/bisq/core/account/sign/SignedWitnessTest.java b/core/src/test/java/bisq/core/account/sign/SignedWitnessTest.java index 8911e94079c..7b4381c81d9 100644 --- a/core/src/test/java/bisq/core/account/sign/SignedWitnessTest.java +++ b/core/src/test/java/bisq/core/account/sign/SignedWitnessTest.java @@ -13,6 +13,8 @@ import org.junit.Before; import org.junit.Test; +import static bisq.core.account.sign.SignedWitness.VerificationMethod.ARBITRATOR; +import static bisq.core.account.sign.SignedWitness.VerificationMethod.TRADE; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; @@ -33,14 +35,14 @@ public void setUp() { @Test public void testProtoRoundTrip() { - SignedWitness signedWitness = new SignedWitness(true, witnessHash, witnessHashSignature, arbitrator1Key.getPubKey(), witnessOwner1PubKey, Instant.now().getEpochSecond(), 100); + SignedWitness signedWitness = new SignedWitness(ARBITRATOR, witnessHash, witnessHashSignature, arbitrator1Key.getPubKey(), witnessOwner1PubKey, Instant.now().getEpochSecond(), 100); assertEquals(signedWitness, SignedWitness.fromProto(signedWitness.toProtoMessage().getSignedWitness())); } @Test public void isImmutable() { byte[] signerPubkey = arbitrator1Key.getPubKey(); - SignedWitness signedWitness = new SignedWitness(true, witnessHash, witnessHashSignature, signerPubkey, witnessOwner1PubKey, Instant.now().getEpochSecond(), 100); + SignedWitness signedWitness = new SignedWitness(TRADE, witnessHash, witnessHashSignature, signerPubkey, witnessOwner1PubKey, Instant.now().getEpochSecond(), 100); byte[] originalWitnessHash = signedWitness.getAccountAgeWitnessHash().clone(); witnessHash[0] += 1; assertArrayEquals(originalWitnessHash, signedWitness.getAccountAgeWitnessHash()); From 8505a5967774f69792e73d970c8eea216b51e1f5 Mon Sep 17 00:00:00 2001 From: sqrrm Date: Fri, 11 Oct 2019 13:44:51 +0200 Subject: [PATCH 3/4] Fix usage of isValidAccountAgeWitness --- .../bisq/core/account/sign/SignedWitness.java | 2 +- .../account/sign/SignedWitnessService.java | 27 +++++++---- .../witness/AccountAgeWitnessService.java | 16 ++----- .../sign/SignedWitnessServiceTest.java | 46 +++++++++---------- .../account/content/PaymentAccountsView.java | 2 + 5 files changed, 48 insertions(+), 45 deletions(-) diff --git a/core/src/main/java/bisq/core/account/sign/SignedWitness.java b/core/src/main/java/bisq/core/account/sign/SignedWitness.java index 1190c15e3c9..e5af940470c 100644 --- a/core/src/main/java/bisq/core/account/sign/SignedWitness.java +++ b/core/src/main/java/bisq/core/account/sign/SignedWitness.java @@ -140,7 +140,7 @@ public static SignedWitness fromProto(protobuf.SignedWitness proto) { @Override public boolean isDateInTolerance(Clock clock) { - // We don't allow older or newer then 1 day. + // We don't allow older or newer than 1 day. // Preventing forward dating is also important to protect against a sophisticated attack return Math.abs(new Date().getTime() - date) <= TOLERANCE; } diff --git a/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java b/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java index 6febe7528fd..6991b86b0d9 100644 --- a/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java +++ b/core/src/main/java/bisq/core/account/sign/SignedWitnessService.java @@ -142,7 +142,7 @@ public SignedWitness signAccountAgeWitness(Coin tradeAmount, AccountAgeWitness accountAgeWitness, ECKey key, PublicKey peersPubKey) { - if (isValidAccountAgeWitness(accountAgeWitness)) { + if (isSignedAccountAgeWitness(accountAgeWitness)) { log.warn("Arbitrator trying to sign already signed accountagewitness {}", accountAgeWitness.toString()); return null; } @@ -165,7 +165,7 @@ public SignedWitness signAccountAgeWitness(Coin tradeAmount, public SignedWitness signAccountAgeWitness(Coin tradeAmount, AccountAgeWitness accountAgeWitness, PublicKey peersPubKey) throws CryptoException { - if (isValidAccountAgeWitness(accountAgeWitness)) { + if (isSignedAccountAgeWitness(accountAgeWitness)) { log.warn("Trader trying to sign already signed accountagewitness {}", accountAgeWitness.toString()); return null; } @@ -254,18 +254,27 @@ private Set getSignedWitnessSetByOwnerPubKey(byte[] ownerPubKey, .collect(Collectors.toSet()); } + public boolean isSignedAccountAgeWitness(AccountAgeWitness accountAgeWitness) { + return isSignerAccountAgeWitness(accountAgeWitness, new Date().getTime() + SIGNER_AGE); + } + + public boolean isSignerAccountAgeWitness(AccountAgeWitness accountAgeWitness) { + return isSignerAccountAgeWitness(accountAgeWitness, new Date().getTime()); + } + /** - * Checks whether the accountAgeWitness has a valid signature from a peer/arbitrator. + * Checks whether the accountAgeWitness has a valid signature from a peer/arbitrator and is allowed to sign + * other accounts. * * @param accountAgeWitness accountAgeWitness - * @return true if accountAgeWitness is valid, false otherwise. + * @param time time of signing + * @return true if accountAgeWitness is allowed to sign at time, false otherwise. */ - public boolean isValidAccountAgeWitness(AccountAgeWitness accountAgeWitness) { + private boolean isSignerAccountAgeWitness(AccountAgeWitness accountAgeWitness, long time) { Stack excludedPubKeys = new Stack<>(); - long now = new Date().getTime(); Set signedWitnessSet = getSignedWitnessSet(accountAgeWitness); for (SignedWitness signedWitness : signedWitnessSet) { - if (isValidSignedWitnessInternal(signedWitness, now, excludedPubKeys)) { + if (isValidSignerWitnessInternal(signedWitness, time, excludedPubKeys)) { return true; } } @@ -281,7 +290,7 @@ public boolean isValidAccountAgeWitness(AccountAgeWitness accountAgeWitness) { * @param excludedPubKeys stack to prevent recursive loops * @return true if signedWitness is valid, false otherwise. */ - private boolean isValidSignedWitnessInternal(SignedWitness signedWitness, + private boolean isValidSignerWitnessInternal(SignedWitness signedWitness, long childSignedWitnessDateMillis, Stack excludedPubKeys) { if (!verifySignature(signedWitness)) { @@ -303,7 +312,7 @@ private boolean isValidSignedWitnessInternal(SignedWitness signedWitness, // Iterate over signedWitness signers Set signerSignedWitnessSet = getSignedWitnessSetByOwnerPubKey(signedWitness.getSignerPubKey(), excludedPubKeys); for (SignedWitness signerSignedWitness : signerSignedWitnessSet) { - if (isValidSignedWitnessInternal(signerSignedWitness, signedWitness.getDate(), excludedPubKeys)) { + if (isValidSignerWitnessInternal(signerSignedWitness, signedWitness.getDate(), excludedPubKeys)) { return true; } } diff --git a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java index 89cc9074af4..470b5bd0617 100644 --- a/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java +++ b/core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java @@ -619,7 +619,7 @@ public List getTraderPaymentAccounts(long safeDate, PaymentMetho .filter(this::isBuyerWinner) .flatMap(this::getTraderData) .filter(traderDataItem -> - !signedWitnessService.isValidAccountAgeWitness(traderDataItem.getAccountAgeWitness())) + !signedWitnessService.isSignedAccountAgeWitness(traderDataItem.getAccountAgeWitness())) .filter(traderDataItem -> traderDataItem.getAccountAgeWitness().getDate() < safeDate) .distinct() .collect(Collectors.toList()); @@ -662,28 +662,20 @@ private Stream getTraderData(Dispute dispute) { return Stream.of(buyerData, sellerData); } - // Check if my account has a signed witness - public boolean myHasSignedWitness(PaymentAccountPayload paymentAccountPayload) { - return signedWitnessService.isValidAccountAgeWitness(getMyWitness(paymentAccountPayload)); - } - public boolean hasSignedWitness(Offer offer) { return findWitness(offer) - .map(signedWitnessService::isValidAccountAgeWitness) + .map(signedWitnessService::isSignedAccountAgeWitness) .orElse(false); } public boolean peerHasSignedWitness(Trade trade) { return findTradePeerWitness(trade) - .map(signedWitnessService::isValidAccountAgeWitness) + .map(signedWitnessService::isSignedAccountAgeWitness) .orElse(false); } public boolean accountIsSigner(AccountAgeWitness accountAgeWitness) { - if (signedWitnessService.isSignedByArbitrator(accountAgeWitness)) { - return true; - } - return getWitnessSignAge(accountAgeWitness, new Date()) > SignedWitnessService.SIGNER_AGE; + return signedWitnessService.isSignerAccountAgeWitness(accountAgeWitness); } public SignState getSignState(Offer offer) { diff --git a/core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java b/core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java index 8f6dac6f7bb..e5a58a94cbd 100644 --- a/core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java +++ b/core/src/test/java/bisq/core/account/sign/SignedWitnessServiceTest.java @@ -122,9 +122,9 @@ public void testIsValidAccountAgeWitnessOk() { signedWitnessService.addToMap(sw2); signedWitnessService.addToMap(sw3); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew1)); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew2)); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew3)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew1)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew2)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew3)); } @Test @@ -139,9 +139,9 @@ public void testIsValidAccountAgeWitnessArbitratorSignatureProblem() { signedWitnessService.addToMap(sw2); signedWitnessService.addToMap(sw3); - assertFalse(signedWitnessService.isValidAccountAgeWitness(aew1)); - assertFalse(signedWitnessService.isValidAccountAgeWitness(aew2)); - assertFalse(signedWitnessService.isValidAccountAgeWitness(aew3)); + assertFalse(signedWitnessService.isSignerAccountAgeWitness(aew1)); + assertFalse(signedWitnessService.isSignerAccountAgeWitness(aew2)); + assertFalse(signedWitnessService.isSignerAccountAgeWitness(aew3)); } @Test @@ -156,9 +156,9 @@ public void testIsValidAccountAgeWitnessPeerSignatureProblem() { signedWitnessService.addToMap(sw2); signedWitnessService.addToMap(sw3); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew1)); - assertFalse(signedWitnessService.isValidAccountAgeWitness(aew2)); - assertFalse(signedWitnessService.isValidAccountAgeWitness(aew3)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew1)); + assertFalse(signedWitnessService.isSignerAccountAgeWitness(aew2)); + assertFalse(signedWitnessService.isSignerAccountAgeWitness(aew3)); } @Test @@ -177,9 +177,9 @@ public void testIsValidSelfSignatureOk() throws Exception { signedWitnessService.addToMap(sw2); signedWitnessService.addToMap(sw3); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew1)); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew2)); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew3)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew1)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew2)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew3)); } @Test @@ -202,9 +202,9 @@ public void testIsValidSimpleLoopSignatureProblem() throws Exception { signedWitnessService.addToMap(sw2); signedWitnessService.addToMap(sw3); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew1)); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew2)); - assertFalse(signedWitnessService.isValidAccountAgeWitness(aew3)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew1)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew2)); + assertFalse(signedWitnessService.isSignerAccountAgeWitness(aew3)); } @Test @@ -219,9 +219,9 @@ public void testIsValidAccountAgeWitnessDateTooSoonProblem() { signedWitnessService.addToMap(sw2); signedWitnessService.addToMap(sw3); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew1)); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew2)); - assertFalse(signedWitnessService.isValidAccountAgeWitness(aew3)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew1)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew2)); + assertFalse(signedWitnessService.isSignerAccountAgeWitness(aew3)); } @Test @@ -236,9 +236,9 @@ public void testIsValidAccountAgeWitnessDateTooLateProblem() { signedWitnessService.addToMap(sw2); signedWitnessService.addToMap(sw3); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew1)); - assertTrue(signedWitnessService.isValidAccountAgeWitness(aew2)); - assertFalse(signedWitnessService.isValidAccountAgeWitness(aew3)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew1)); + assertTrue(signedWitnessService.isSignerAccountAgeWitness(aew2)); + assertFalse(signedWitnessService.isSignerAccountAgeWitness(aew3)); } @@ -288,7 +288,7 @@ public void testIsValidAccountAgeWitnessEndlessLoop() throws Exception { signedWitnessService.addToMap(sw2); signedWitnessService.addToMap(sw3); - assertFalse(signedWitnessService.isValidAccountAgeWitness(aew3)); + assertFalse(signedWitnessService.isSignerAccountAgeWitness(aew3)); } @@ -323,7 +323,7 @@ public void testIsValidAccountAgeWitnessLongLoop() throws Exception { SignedWitness sw = new SignedWitness(i == 0 ? ARBITRATOR : TRADE, accountDataHash, signature, signerPubKey, witnessOwnerPubKey, date, tradeAmount1); signedWitnessService.addToMap(sw); } - assertFalse(signedWitnessService.isValidAccountAgeWitness(aew)); + assertFalse(signedWitnessService.isSignerAccountAgeWitness(aew)); } diff --git a/desktop/src/main/java/bisq/desktop/main/account/content/PaymentAccountsView.java b/desktop/src/main/java/bisq/desktop/main/account/content/PaymentAccountsView.java index 983b817acf2..b56aa14d96d 100644 --- a/desktop/src/main/java/bisq/desktop/main/account/content/PaymentAccountsView.java +++ b/desktop/src/main/java/bisq/desktop/main/account/content/PaymentAccountsView.java @@ -130,6 +130,8 @@ public void updateItem(final PaymentAccount item, boolean empty) { switch (signState) { case PEER_SIGNER: case ARBITRATOR: + case PEER_LIMIT_LIFTED: + case PEER_INITIAL: label.setIcon(MaterialDesignIcon.APPROVAL, info); break; default: From 96764360cac6a2b12d1a78597bb0c3ab57ffaaa8 Mon Sep 17 00:00:00 2001 From: sqrrm Date: Fri, 11 Oct 2019 18:30:53 +0200 Subject: [PATCH 4/4] Revert icon for signstate change --- .../bisq/desktop/main/account/content/PaymentAccountsView.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/desktop/src/main/java/bisq/desktop/main/account/content/PaymentAccountsView.java b/desktop/src/main/java/bisq/desktop/main/account/content/PaymentAccountsView.java index b56aa14d96d..983b817acf2 100644 --- a/desktop/src/main/java/bisq/desktop/main/account/content/PaymentAccountsView.java +++ b/desktop/src/main/java/bisq/desktop/main/account/content/PaymentAccountsView.java @@ -130,8 +130,6 @@ public void updateItem(final PaymentAccount item, boolean empty) { switch (signState) { case PEER_SIGNER: case ARBITRATOR: - case PEER_LIMIT_LIFTED: - case PEER_INITIAL: label.setIcon(MaterialDesignIcon.APPROVAL, info); break; default: