Skip to content

Commit

Permalink
Account Signing: Fix verified usage (#3392)
Browse files Browse the repository at this point in the history 
* Rename witnessHash -> accountAgeWitnessHash

* Add enum for SignedWitness verification method

* Fix usage of isValidAccountAgeWitness

* Revert icon for signstate change
  • Loading branch information
@sqrrm @ripcurlx
sqrrm authored and ripcurlx committed Oct 12, 2019
1 parent 901fb95 commit 526fef9
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 97 deletions.
10 changes: 8 additions & 2 deletions common/src/main/proto/pb.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -722,8 +722,14 @@ message AccountAgeWitness {
}

message SignedWitness {
bool signed_by_arbitrator = 1;
bytes witness_hash = 2;
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;
bytes witness_owner_pub_key = 5;
Expand Down
47 changes: 33 additions & 14 deletions core/src/main/java/bisq/core/account/sign/SignedWitness.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -46,10 +47,24 @@
@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 byte[] witnessHash;
private final VerificationMethod verificationMethod;
private final byte[] accountAgeWitnessHash;
private final byte[] signature;
private final byte[] signerPubKey;
private final byte[] witnessOwnerPubKey;
Expand All @@ -58,15 +73,15 @@ public class SignedWitness implements LazyProcessedPayload, PersistableNetworkPa

transient private final byte[] hash;

public SignedWitness(boolean signedByArbitrator,
byte[] witnessHash,
public SignedWitness(VerificationMethod verificationMethod,
byte[] accountAgeWitnessHash,
byte[] signature,
byte[] signerPubKey,
byte[] witnessOwnerPubKey,
long date,
long tradeAmount) {
this.signedByArbitrator = signedByArbitrator;
this.witnessHash = witnessHash.clone();
this.verificationMethod = verificationMethod;
this.accountAgeWitnessHash = accountAgeWitnessHash.clone();
this.signature = signature.clone();
this.signerPubKey = signerPubKey.clone();
this.witnessOwnerPubKey = witnessOwnerPubKey.clone();
Expand All @@ -80,7 +95,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);
}
Expand All @@ -93,8 +108,8 @@ public SignedWitness(boolean signedByArbitrator,
@Override
public protobuf.PersistableNetworkPayload toProtoMessage() {
final protobuf.SignedWitness.Builder builder = protobuf.SignedWitness.newBuilder()
.setSignedByArbitrator(signedByArbitrator)
.setWitnessHash(ByteString.copyFrom(witnessHash))
.setVerificationMethod(VerificationMethod.toProtoMessage(verificationMethod))
.setAccountAgeWitnessHash(ByteString.copyFrom(accountAgeWitnessHash))
.setSignature(ByteString.copyFrom(signature))
.setSignerPubKey(ByteString.copyFrom(signerPubKey))
.setWitnessOwnerPubKey(ByteString.copyFrom(witnessOwnerPubKey))
Expand All @@ -108,8 +123,9 @@ protobuf.SignedWitness toProtoSignedWitness() {
}

public static SignedWitness fromProto(protobuf.SignedWitness proto) {
return new SignedWitness(proto.getSignedByArbitrator(),
proto.getWitnessHash().toByteArray(),
return new SignedWitness(
SignedWitness.VerificationMethod.fromProto(proto.getVerificationMethod()),
proto.getAccountAgeWitnessHash().toByteArray(),
proto.getSignature().toByteArray(),
proto.getSignerPubKey().toByteArray(),
proto.getWitnessOwnerPubKey().toByteArray(),
Expand All @@ -124,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;
}
Expand All @@ -145,6 +161,9 @@ public byte[] getHash() {
return hash;
}

public boolean isSignedByArbitrator() {
return verificationMethod == VerificationMethod.ARBITRATOR;
}

///////////////////////////////////////////////////////////////////////////////////////////
// Getters
Expand All @@ -157,8 +176,8 @@ P2PDataStorage.ByteArray getHashAsByteArray() {
@Override
public String toString() {
return "SignedWitness{" +
",\n signedByArbitrator=" + signedByArbitrator +
",\n witnessHash=" + Utilities.bytesAsHexString(witnessHash) +
",\n verificationMethod=" + verificationMethod +
",\n witnessHash=" + Utilities.bytesAsHexString(accountAgeWitnessHash) +
",\n signature=" + Utilities.bytesAsHexString(signature) +
",\n signerPubKey=" + Utilities.bytesAsHexString(signerPubKey) +
",\n witnessOwnerPubKey=" + Utilities.bytesAsHexString(witnessOwnerPubKey) +
Expand Down
41 changes: 25 additions & 16 deletions core/src/main/java/bisq/core/account/sign/SignedWitnessService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,14 @@ 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;
}

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(),
Expand All @@ -165,13 +165,13 @@ 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;
}

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(),
Expand All @@ -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()))) {
Expand All @@ -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);
Expand All @@ -224,23 +224,23 @@ private boolean verifySignatureWithDSAKey(SignedWitness signedWitness) {

private Set<SignedWitness> 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());
}

// SignedWitness objects signed by arbitrators
public Set<SignedWitness> 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());
}

// SignedWitness objects signed by any other peer
public Set<SignedWitness> 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());
}

Expand All @@ -254,18 +254,27 @@ private Set<SignedWitness> 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<P2PDataStorage.ByteArray> excludedPubKeys = new Stack<>();
long now = new Date().getTime();
Set<SignedWitness> signedWitnessSet = getSignedWitnessSet(accountAgeWitness);
for (SignedWitness signedWitness : signedWitnessSet) {
if (isValidSignedWitnessInternal(signedWitness, now, excludedPubKeys)) {
if (isValidSignerWitnessInternal(signedWitness, time, excludedPubKeys)) {
return true;
}
}
Expand All @@ -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<P2PDataStorage.ByteArray> excludedPubKeys) {
if (!verifySignature(signedWitness)) {
Expand All @@ -303,7 +312,7 @@ private boolean isValidSignedWitnessInternal(SignedWitness signedWitness,
// Iterate over signedWitness signers
Set<SignedWitness> signerSignedWitnessSet = getSignedWitnessSetByOwnerPubKey(signedWitness.getSignerPubKey(), excludedPubKeys);
for (SignedWitness signerSignedWitness : signerSignedWitnessSet) {
if (isValidSignedWitnessInternal(signerSignedWitness, signedWitness.getDate(), excludedPubKeys)) {
if (isValidSignerWitnessInternal(signerSignedWitness, signedWitness.getDate(), excludedPubKeys)) {
return true;
}
}
Expand Down
16 changes: 4 additions & 12 deletions core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ public List<TraderDataItem> 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());
Expand Down Expand Up @@ -662,28 +662,20 @@ private Stream<TraderDataItem> 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) {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/bisq/core/app/BisqSetup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Loading

0 comments on commit 526fef9

Please sign in to comment.