Skip to content

Commit

Permalink
fix: fix bug with signing auth entries in multi-sig scenarios (#638)
Browse files Browse the repository at this point in the history
  • Loading branch information
@overcat
overcat authored Sep 19, 2024
1 parent f178542 commit 547b88e
Show file tree
Hide file tree
Showing 3 changed files with 226 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Update
- refactor!: remove the constructor from `KeyPair`, use `KeyPair.fromSecretSeed` or `KeyPair.fromAccountId` instead.
- fix!: fix bug with signing auth entries in multi-sig scenarios.

## 1.0.0-alpha0
We are thrilled to announce the release of version 1.0.0-alpha0 for java-stellar-sdk,
Expand Down
26 changes: 18 additions & 8 deletions src/main/java/org/stellar/sdk/Auth.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.security.SecureRandom;
import java.util.Collections;
import java.util.LinkedHashMap;
import lombok.Value;
import org.stellar.sdk.exception.UnexpectedException;
import org.stellar.sdk.scval.Scv;
import org.stellar.sdk.xdr.EnvelopeType;
Expand Down Expand Up @@ -92,7 +93,8 @@ public static SorobanAuthorizationEntry authorizeEntry(
throw new IllegalArgumentException("Unable to convert preimage to bytes", e);
}
byte[] payload = Util.hash(data);
return signer.sign(payload);
byte[] signature = signer.sign(payload);
return new Signature(signer.getPublicKey(), signature);
};

return authorizeEntry(entry, entrySigner, validUntilLedgerSeq, network);
Expand Down Expand Up @@ -184,8 +186,8 @@ public static SorobanAuthorizationEntry authorizeEntry(
.signatureExpirationLedger(addressCredentials.getSignatureExpirationLedger())
.build())
.build();
byte[] signature = signer.sign(preimage);
byte[] publicKey = Address.fromSCAddress(addressCredentials.getAddress()).getBytes();

Signature signature = signer.sign(preimage);

byte[] data;
try {
Expand All @@ -194,7 +196,7 @@ public static SorobanAuthorizationEntry authorizeEntry(
throw new IllegalArgumentException("Unable to convert preimage to bytes", e);
}
byte[] payload = Util.hash(data);
if (!KeyPair.fromPublicKey(publicKey).verify(payload, signature)) {
if (!KeyPair.fromPublicKey(signature.publicKey).verify(payload, signature.signature)) {
throw new IllegalArgumentException("signature does not match payload");
}

Expand All @@ -205,8 +207,8 @@ public static SorobanAuthorizationEntry authorizeEntry(
Scv.toMap(
new LinkedHashMap<SCVal, SCVal>() {
{
put(Scv.toSymbol("public_key"), Scv.toBytes(publicKey));
put(Scv.toSymbol("signature"), Scv.toBytes(signature));
put(Scv.toSymbol("public_key"), Scv.toBytes(signature.getPublicKey()));
put(Scv.toSymbol("signature"), Scv.toBytes(signature.getSignature()));
}
});
addressCredentials.setSignature(Scv.toVec(Collections.singleton(sigScVal)));
Expand Down Expand Up @@ -244,7 +246,8 @@ public static SorobanAuthorizationEntry authorizeInvocation(
preimage -> {
try {
byte[] payload = Util.hash(preimage.toXdrByteArray());
return signer.sign(payload);
byte[] signature = signer.sign(payload);
return new Signature(signer.getPublicKey(), signature);
} catch (IOException e) {
throw new UnexpectedException(e);
}
Expand Down Expand Up @@ -303,8 +306,15 @@ public static SorobanAuthorizationEntry authorizeInvocation(
return authorizeEntry(entry, signer, validUntilLedgerSeq, network);
}

/** A signature, consisting of a public key and a signature. */
@Value
public static class Signature {
byte[] publicKey;
byte[] signature;
}

/** An interface for signing a {@link HashIDPreimage} to produce a signature. */
public interface Signer {
byte[] sign(HashIDPreimage preimage);
Signature sign(HashIDPreimage preimage);
}
}
211 changes: 207 additions & 4 deletions src/test/java/org/stellar/sdk/AuthTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ public void testSignAuthorizeEntryWithBase64EntryAndFunctionSigner() throws IOEx
throw new IllegalArgumentException("Unable to convert preimage to bytes", e);
}
byte[] payload = Util.hash(data);
return signer.sign(payload);
byte[] signature = signer.sign(payload);
return new Auth.Signature(signer.getPublicKey(), signature);
};
long validUntilLedgerSeq = 654656L;
Network network = Network.TESTNET;
Expand Down Expand Up @@ -319,7 +320,8 @@ public void testSignAuthorizeEntryWithXdrEntryAndFunctionSigner() throws IOExcep
throw new IllegalArgumentException("Unable to convert preimage to bytes", e);
}
byte[] payload = Util.hash(data);
return signer.sign(payload);
byte[] signature = signer.sign(payload);
return new Auth.Signature(signer.getPublicKey(), signature);
};
long validUntilLedgerSeq = 654656L;
Network network = Network.TESTNET;
Expand Down Expand Up @@ -449,6 +451,12 @@ public void testSignAuthorizeEntryWithSignatureMismatchThrows() throws IOExcepti
String contractId = "CDCYWK73YTYFJZZSJ5V7EDFNHYBG4QN3VUNG2IGD27KJDDPNCZKBCBXK";
KeyPair signer =
KeyPair.fromSecretSeed("SAEZSI6DY7AXJFIYA4PM6SIBNEYYXIEM2MSOTHFGKHDW32MBQ7KVO6EN");
Auth.Signer entrySigner =
preimage -> {
byte[] invalidData = new byte[20];
byte[] signature = signer.sign(invalidData);
return new Auth.Signature(signer.getPublicKey(), signature);
};
long validUntilLedgerSeq = 654656L;
Network network = Network.TESTNET;

Expand Down Expand Up @@ -485,7 +493,7 @@ public void testSignAuthorizeEntryWithSignatureMismatchThrows() throws IOExcepti
.build();

try {
Auth.authorizeEntry(entry.toXdrBase64(), signer, validUntilLedgerSeq, network);
Auth.authorizeEntry(entry.toXdrBase64(), entrySigner, validUntilLedgerSeq, network);
fail();
} catch (IllegalArgumentException e) {
assertEquals("signature does not match payload", e.getMessage());
Expand Down Expand Up @@ -556,7 +564,8 @@ public void authorizeInvocationWithFunctionSigner() {
throw new IllegalArgumentException("Unable to convert preimage to bytes", e);
}
byte[] payload = Util.hash(data);
return signer.sign(payload);
byte[] signature = signer.sign(payload);
return new Auth.Signature(signer.getPublicKey(), signature);
};
long validUntilLedgerSeq = 654656L;
Network network = Network.TESTNET;
Expand Down Expand Up @@ -603,4 +612,198 @@ public void authorizeInvocationWithFunctionSigner() {
assertEquals(
signedEntry.getCredentials().getAddress().getSignature().getVec().getSCVec().length, 1);
}

@Test
public void testSignAuthorizeEntryWithKeypairSignerNotEqualCredentialAddress()
throws IOException {
String contractId = "CDCYWK73YTYFJZZSJ5V7EDFNHYBG4QN3VUNG2IGD27KJDDPNCZKBCBXK";
KeyPair signer =
KeyPair.fromSecretSeed("SAEZSI6DY7AXJFIYA4PM6SIBNEYYXIEM2MSOTHFGKHDW32MBQ7KVO6EN");
long validUntilLedgerSeq = 654656L;
Network network = Network.TESTNET;
String credentialAddress = "GADBBY4WFXKKFJ7CMTG3J5YAUXMQDBILRQ6W3U5IWN5TQFZU4MWZ5T4K";

SorobanCredentials credentials =
SorobanCredentials.builder()
.discriminant(SorobanCredentialsType.SOROBAN_CREDENTIALS_ADDRESS)
.address(
SorobanAddressCredentials.builder()
.address(new Address(credentialAddress).toSCAddress())
.nonce(new Int64(123456789L))
.signatureExpirationLedger(new Uint32(new XdrUnsignedInteger(0L)))
.signature(Scv.toVoid())
.build())
.build();
SorobanAuthorizedInvocation invocation =
SorobanAuthorizedInvocation.builder()
.function(
SorobanAuthorizedFunction.builder()
.discriminant(
SorobanAuthorizedFunctionType.SOROBAN_AUTHORIZED_FUNCTION_TYPE_CONTRACT_FN)
.contractFn(
InvokeContractArgs.builder()
.contractAddress(new Address(contractId).toSCAddress())
.functionName(Scv.toSymbol("increment").getSym())
.args(new SCVal[0])
.build())
.build())
.subInvocations(new SorobanAuthorizedInvocation[0])
.build();
SorobanAuthorizationEntry entry =
SorobanAuthorizationEntry.builder()
.credentials(credentials)
.rootInvocation(invocation)
.build();

SorobanAuthorizationEntry signedEntry =
Auth.authorizeEntry(entry, signer, validUntilLedgerSeq, network);

HashIDPreimage preimage =
HashIDPreimage.builder()
.discriminant(EnvelopeType.ENVELOPE_TYPE_SOROBAN_AUTHORIZATION)
.sorobanAuthorization(
HashIDPreimage.HashIDPreimageSorobanAuthorization.builder()
.networkID(new Hash(network.getNetworkId()))
.nonce(new Int64(123456789L))
.invocation(invocation)
.signatureExpirationLedger(
new Uint32(new XdrUnsignedInteger(validUntilLedgerSeq)))
.build())
.build();
byte[] signature = signer.sign(Util.hash(preimage.toXdrByteArray()));
SorobanCredentials expectedCredentials =
SorobanCredentials.builder()
.discriminant(SorobanCredentialsType.SOROBAN_CREDENTIALS_ADDRESS)
.address(
SorobanAddressCredentials.builder()
.address(new Address(credentialAddress).toSCAddress())
.nonce(new Int64(123456789L))
.signatureExpirationLedger(
new Uint32(new XdrUnsignedInteger(validUntilLedgerSeq)))
.signature(
Scv.toVec(
Collections.singleton(
Scv.toMap(
new LinkedHashMap<SCVal, SCVal>() {
{
put(
Scv.toSymbol("public_key"),
Scv.toBytes(signer.getPublicKey()));
put(Scv.toSymbol("signature"), Scv.toBytes(signature));
}
}))))
.build())
.build();

SorobanAuthorizationEntry expectedEntry =
SorobanAuthorizationEntry.builder()
.credentials(expectedCredentials)
.rootInvocation(invocation)
.build();
assertEquals(expectedEntry, signedEntry);
assertNotSame(entry, signedEntry);
}

@Test
public void testSignAuthorizeEntryWithFunctionSignerNotEqualCredentialAddress()
throws IOException {
String contractId = "CDCYWK73YTYFJZZSJ5V7EDFNHYBG4QN3VUNG2IGD27KJDDPNCZKBCBXK";
KeyPair signer =
KeyPair.fromSecretSeed("SAEZSI6DY7AXJFIYA4PM6SIBNEYYXIEM2MSOTHFGKHDW32MBQ7KVO6EN");
Auth.Signer entrySigner =
preimage -> {
byte[] data;
try {
data = preimage.toXdrByteArray();
} catch (IOException e) {
throw new IllegalArgumentException("Unable to convert preimage to bytes", e);
}
byte[] payload = Util.hash(data);
byte[] signature = signer.sign(payload);
return new Auth.Signature(signer.getPublicKey(), signature);
};
long validUntilLedgerSeq = 654656L;
Network network = Network.TESTNET;
String credentialAddress = "GADBBY4WFXKKFJ7CMTG3J5YAUXMQDBILRQ6W3U5IWN5TQFZU4MWZ5T4K";

SorobanCredentials credentials =
SorobanCredentials.builder()
.discriminant(SorobanCredentialsType.SOROBAN_CREDENTIALS_ADDRESS)
.address(
SorobanAddressCredentials.builder()
.address(new Address(credentialAddress).toSCAddress())
.nonce(new Int64(123456789L))
.signatureExpirationLedger(new Uint32(new XdrUnsignedInteger(0L)))
.signature(Scv.toVoid())
.build())
.build();
SorobanAuthorizedInvocation invocation =
SorobanAuthorizedInvocation.builder()
.function(
SorobanAuthorizedFunction.builder()
.discriminant(
SorobanAuthorizedFunctionType.SOROBAN_AUTHORIZED_FUNCTION_TYPE_CONTRACT_FN)
.contractFn(
InvokeContractArgs.builder()
.contractAddress(new Address(contractId).toSCAddress())
.functionName(Scv.toSymbol("increment").getSym())
.args(new SCVal[0])
.build())
.build())
.subInvocations(new SorobanAuthorizedInvocation[0])
.build();
SorobanAuthorizationEntry entry =
SorobanAuthorizationEntry.builder()
.credentials(credentials)
.rootInvocation(invocation)
.build();

SorobanAuthorizationEntry signedEntry =
Auth.authorizeEntry(entry, entrySigner, validUntilLedgerSeq, network);

HashIDPreimage preimage =
HashIDPreimage.builder()
.discriminant(EnvelopeType.ENVELOPE_TYPE_SOROBAN_AUTHORIZATION)
.sorobanAuthorization(
HashIDPreimage.HashIDPreimageSorobanAuthorization.builder()
.networkID(new Hash(network.getNetworkId()))
.nonce(new Int64(123456789L))
.invocation(invocation)
.signatureExpirationLedger(
new Uint32(new XdrUnsignedInteger(validUntilLedgerSeq)))
.build())
.build();
byte[] signature = signer.sign(Util.hash(preimage.toXdrByteArray()));
SorobanCredentials expectedCredentials =
SorobanCredentials.builder()
.discriminant(SorobanCredentialsType.SOROBAN_CREDENTIALS_ADDRESS)
.address(
SorobanAddressCredentials.builder()
.address(new Address(credentialAddress).toSCAddress())
.nonce(new Int64(123456789L))
.signatureExpirationLedger(
new Uint32(new XdrUnsignedInteger(validUntilLedgerSeq)))
.signature(
Scv.toVec(
Collections.singleton(
Scv.toMap(
new LinkedHashMap<SCVal, SCVal>() {
{
put(
Scv.toSymbol("public_key"),
Scv.toBytes(signer.getPublicKey()));
put(Scv.toSymbol("signature"), Scv.toBytes(signature));
}
}))))
.build())
.build();

SorobanAuthorizationEntry expectedEntry =
SorobanAuthorizationEntry.builder()
.credentials(expectedCredentials)
.rootInvocation(invocation)
.build();
assertEquals(expectedEntry, signedEntry);
assertNotSame(entry, signedEntry);
}
}

0 comments on commit 547b88e

Please sign in to comment.