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.

Sign up for GitHub

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

Account Signing: Fix verified usage #3392

Merged
merged 4 commits into from
Oct 12, 2019
Merged

Account Signing: Fix verified usage #3392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
00953c2
Rename witnessHash -> accountAgeWitnessHash
sqrrm Oct 10, 2019
0ae162d
Add enum for SignedWitness verification method
sqrrm Oct 10, 2019
8505a59
Fix usage of isValidAccountAgeWitness
sqrrm Oct 11, 2019
9676436
Revert icon for signstate change
sqrrm Oct 11, 2019
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
10 changes: 8 additions & 2 deletions common/src/main/proto/pb.proto
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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