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

Sign account usage #2768

Closed

Conversation

ManfredKarrer
Copy link
Contributor

Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

Looks like a reasonable approach. Quite simple and not tying in to other parts.

// API
///////////////////////////////////////////////////////////////////////////////////////////

// Arbitrators sign with EC key
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a short explanation why arbitrators are using EC keys?

.collect(Collectors.toSet());
}

public boolean isValidAccountAgeWitness(AccountAgeWitness accountAgeWitness) {
Copy link
Member

Choose a reason for hiding this comment

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

There need to be a break in case of circular signing. If someone creates two witnesses that sign each other I think this recursion would go into an infinite loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm... not sure if that can happen. Can you describe the case? If Alice signs Bob and Bob signs Carol and Carol signs Bob it is not an issue as the witness objects are the relevant objects which build the chain, not the signers. But in the lookup for the signers witness we need to probably use the date to use the oldest.

It also does not support yet handling of mulitple signers and when you are allowed to sign...

Copy link
Member

Choose a reason for hiding this comment

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

Alice signs Bob's witness and Bob signs Alice's witness.

First level, trying to validaty Bob's witness, we find that it was signed by Alice. We then try to validate Alice's witness and during the second recursion we find that it's signed by Bob and will then try to validate it. Back to where we started in an infinite recursion.

Some leeway has to be given to the time stamps so an attack could be to create this pair and set the same date for both signedWitnesses.

I might well be missing something, and I don't think that's a risk during normal operation, just good to be safe and not add any attack vectors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we need to pass through the list and remove items once processed.

# Conflicts:
#	core/src/main/java/bisq/core/account/witness/AccountAgeWitnessService.java
#	desktop/src/main/java/bisq/desktop/components/PeerInfoIcon.java
@ManfredKarrer
Copy link
Contributor Author

Replaced by #2787

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants