Skip to content

Commit

Permalink
bugfix: make Sep10Challenge.verifyTransactionSignatures resilient a…
Browse files Browse the repository at this point in the history
…gainst account IDs that are not compliant with ed25519 (#440)

### What
Make `Sep10Challenge.verifyTransactionSignatures` resilient against account IDs that are not compliant with ed25519. If a non-compliant account was provided in the list of signers, this account is simply ignored.

Accounts whose ID is not a valid ed25519 public key won't have a signing key, so they can't have signed the challenge transaction.

After the `Sep10Challenge.verifyTransactionSignatures` method finds the intersection between `account IDs that signed the transaction` ∩ `signers provided in the list`, it will return to the caller and the number of accounts and their weight will be verified accordingly.

### Why

Close #439.
  • Loading branch information
marcelosalloum authored Jun 30, 2022
2 parents 8dcb082 + a838a77 commit 1d9a83e
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 3 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ As this project is pre 1.0, breaking changes may happen for minor version bumps.

## Pending

## 0.34.1

* Fix the `Sep10Challenge.verifyTransactionSignatures` method to handle/ignore signers that are not ed25519 compliant. ([#440](https://github.com/stellar/java-stellar-sdk/pull/440))

## 0.34.0
* Add memo to `Sep10Challenge.newChallenge()` and `Sep10Challenge.readChallengeTransaction`. ([#435](https://github.com/stellar/java-stellar-sdk/pull/435))

Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ plugins {
}

sourceCompatibility = 1.6
version = '0.34.0'
version = '0.34.1'
group = 'stellar'
jar.enabled = false

Expand Down
7 changes: 6 additions & 1 deletion src/main/java/org/stellar/sdk/Sep10Challenge.java
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,12 @@ private static Set<String> verifyTransactionSignatures(Transaction transaction,
}

for (String signer : signers) {
KeyPair keyPair = KeyPair.fromAccountId(signer);
KeyPair keyPair;
try {
keyPair = KeyPair.fromAccountId(signer);
} catch (RuntimeException e) {
continue;
}
SignatureHint hint = keyPair.getSignatureHint();

for (Signature signature : signatures.get(hint)) {
Expand Down
34 changes: 33 additions & 1 deletion src/test/java/org/stellar/sdk/Sep10ChallengeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public void testChallenge() throws InvalidSep10ChallengeException {
}

@Test
public void testNewChallengeRejectsMuxedClientAccount() throws InvalidSep10ChallengeException {
public void testNewChallengeRejectsMuxedClientAccount() {
try {
KeyPair server = KeyPair.random();

Expand Down Expand Up @@ -1608,6 +1608,38 @@ public void testVerifyChallengeTransactionThresholdValidServerAndMultipleClientK
assertEquals(new HashSet<String>(Arrays.asList(masterClient.getAccountId(), signerClient1.getAccountId())), signersFound);
}

@Test
public void testVerifyChallengeTransactionWithAccountIdNonCompliantWithEd25519() throws IOException, InvalidSep10ChallengeException {
Network network = Network.TESTNET;
KeyPair server = KeyPair.random();
KeyPair masterClient = KeyPair.random();
long now = System.currentTimeMillis() / 1000L;
long end = now + 300;
TimeBounds timeBounds = new TimeBounds(now, end);
String domainName = "example.com";
String webAuthDomain = "example.com";

Transaction transaction = Sep10Challenge.newChallenge(
server,
network,
masterClient.getAccountId(),
domainName,
webAuthDomain,
timeBounds
);

transaction.sign(masterClient);

Set<Sep10Challenge.Signer> signers = new HashSet<Sep10Challenge.Signer>(Arrays.asList(
new Sep10Challenge.Signer(masterClient.getAccountId(), 2),
new Sep10Challenge.Signer("GA2T6GR7VXXXBETTERSAFETHANSORRYXXXPROTECTEDBYLOBSTRVAULT", 1)
));

int threshold = 2;
Set<String> signersFound = Sep10Challenge.verifyChallengeTransactionThreshold(transaction.toEnvelopeXdrBase64(), server.getAccountId(), network, domainName, webAuthDomain, threshold, signers);
assertEquals(new HashSet<String>(Collections.singletonList(masterClient.getAccountId())), signersFound);
}

@Test
public void testVerifyChallengeTransactionThresholdValidServerAndMultipleClientKeyMeetingThresholdSomeUnusedIgnorePreauthTxHashAndXHash() throws IOException, InvalidSep10ChallengeException {
Network network = Network.TESTNET;
Expand Down

0 comments on commit 1d9a83e

Please sign in to comment.