Skip to content

Commit

Permalink
Simplify handling of invalid signatures (#4372)
Browse files Browse the repository at this point in the history 
Remove the notion of an invalid signature

Just throw exceptions when signatures deserialise incorrectly, including
deserialisation to points not in the G2 group. Note that deserialisation is
lazy, so checks occur when the signature is used, not when it is
created.
  • Loading branch information
@benjaminion
benjaminion authored Sep 21, 2021
1 parent c99224a commit 22aa754
Show file tree
Hide file tree
Showing 15 changed files with 263 additions and 230 deletions.
205 changes: 116 additions & 89 deletions bls/src/main/java/tech/pegasys/teku/bls/BLS.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import org.apache.logging.log4j.Logger;
import org.apache.tuweni.bytes.Bytes;
import tech.pegasys.teku.bls.impl.BLS12381;
import tech.pegasys.teku.bls.impl.DeserializeException;
import tech.pegasys.teku.bls.impl.BlsException;
import tech.pegasys.teku.bls.impl.PublicKey;
import tech.pegasys.teku.bls.impl.PublicKeyMessagePair;
import tech.pegasys.teku.bls.impl.blst.BlstLoader;
Expand Down Expand Up @@ -58,7 +58,7 @@ public static void resetBlsImplementation() {
BLS_IMPL = BlstLoader.INSTANCE.get();
LOG.info("BLS: loaded BLST library");
} else {
throw new RuntimeException("Failed to load Blst library.");
throw new BlsException("Failed to load Blst library.");
}
}

Expand Down Expand Up @@ -96,7 +96,7 @@ public static boolean verify(BLSPublicKey publicKey, Bytes message, BLSSignature
}
try {
return signature.getSignature().verify(publicKey.getPublicKey(), message);
} catch (DeserializeException e) {
} catch (IllegalArgumentException e) {
return false;
}
}
Expand All @@ -112,15 +112,20 @@ public static boolean verify(BLSPublicKey publicKey, Bytes message, BLSSignature
*
* @param signatures the list of signatures to be aggregated
* @return the aggregated signature
* @throws IllegalArgumentException if any of supplied signatures is invalid
* @throws BlsException if any of supplied signatures is invalid
*/
public static BLSSignature aggregate(List<BLSSignature> signatures)
throws IllegalArgumentException {
checkArgument(signatures.size() > 0, "Aggregating zero signatures is invalid.");
return new BLSSignature(
getBlsImpl()
.aggregateSignatures(
signatures.stream().map(BLSSignature::getSignature).collect(Collectors.toList())));
public static BLSSignature aggregate(List<BLSSignature> signatures) throws BlsException {
try {
checkArgument(signatures.size() > 0, "Aggregating zero signatures is invalid.");
return new BLSSignature(
getBlsImpl()
.aggregateSignatures(
signatures.stream()
.map(BLSSignature::getSignature)
.collect(Collectors.toList())));
} catch (IllegalArgumentException e) {
throw new BlsException("Failed to aggregate signatures", e);
}
}

/**
Expand All @@ -141,21 +146,26 @@ public static BLSSignature aggregate(List<BLSSignature> signatures)
*/
public static boolean aggregateVerify(
List<BLSPublicKey> publicKeys, List<Bytes> messages, BLSSignature signature) {
checkArgument(
publicKeys.size() == messages.size(),
"Number of public keys and number of messages differs.");
if (publicKeys.isEmpty()) return false;

List<PublicKeyMessagePair> publicKeyMessagePairs =
Streams.zip(
publicKeys.stream(),
messages.stream(),
(pk, msg) -> new PublicKeyMessagePair(pk.getPublicKey(), msg))
.collect(Collectors.toList());
try {
return signature.getSignature().verify(publicKeyMessagePairs);
} catch (DeserializeException e) {
return false;
checkArgument(
publicKeys.size() == messages.size(),
"Number of public keys and number of messages differs.");
if (publicKeys.isEmpty()) {
return false;
}
List<PublicKeyMessagePair> publicKeyMessagePairs =
Streams.zip(
publicKeys.stream(),
messages.stream(),
(pk, msg) -> new PublicKeyMessagePair(pk.getPublicKey(), msg))
.collect(Collectors.toList());
try {
return signature.getSignature().verify(publicKeyMessagePairs);
} catch (BlsException e) {
return false;
}
} catch (IllegalArgumentException e) {
throw new BlsException("Failed to aggregateVerify", e);
}
}

Expand All @@ -178,13 +188,20 @@ public static boolean fastAggregateVerify(
LOG.warn("Skipping bls verification.");
return true;
}
if (publicKeys.isEmpty()) return false;
List<PublicKey> publicKeyObjects =
publicKeys.stream().map(BLSPublicKey::getPublicKey).collect(Collectors.toList());

try {
return signature.getSignature().verify(publicKeyObjects, message);
} catch (DeserializeException e) {
return false;
if (publicKeys.isEmpty()) {
return false;
}
List<PublicKey> publicKeyObjects =
publicKeys.stream().map(BLSPublicKey::getPublicKey).collect(Collectors.toList());
try {
return signature.getSignature().verify(publicKeyObjects, message);
} catch (BlsException e) {
return false;
}
} catch (IllegalArgumentException e) {
throw new BlsException("Failed to fastAggregateVerify", e);
}
}

Expand All @@ -211,21 +228,25 @@ public static boolean fastAggregateVerify(
*/
public static boolean batchVerify(
List<List<BLSPublicKey>> publicKeys, List<Bytes> messages, List<BLSSignature> signatures) {
Preconditions.checkArgument(
publicKeys.size() == messages.size() && publicKeys.size() == signatures.size(),
"Different collection sizes");
try {
Preconditions.checkArgument(
publicKeys.size() == messages.size() && publicKeys.size() == signatures.size(),
"Different collection sizes");

int count = publicKeys.size();
if (count == 0) {
return false;
} else if (count == 1) {
return fastAggregateVerify(publicKeys.get(0), messages.get(0), signatures.get(0));
} else {
// double pairing variant is normally slightly faster, but when the number of
// signatures is relatively small the parallelization of hashToG2 internally
// yields more performance gain than double pairing
boolean doublePairing = count > Runtime.getRuntime().availableProcessors() * 2;
return batchVerify(publicKeys, messages, signatures, doublePairing, true);
int count = publicKeys.size();
if (count == 0) {
return false;
} else if (count == 1) {
return fastAggregateVerify(publicKeys.get(0), messages.get(0), signatures.get(0));
} else {
// double pairing variant is normally slightly faster, but when the number of
// signatures is relatively small the parallelization of hashToG2 internally
// yields more performance gain than double pairing
boolean doublePairing = count > Runtime.getRuntime().availableProcessors() * 2;
return batchVerify(publicKeys, messages, signatures, doublePairing, true);
}
} catch (IllegalArgumentException e) {
throw new BlsException("Failed to batchVerify", e);
}
}

Expand Down Expand Up @@ -258,51 +279,57 @@ public static boolean batchVerify(
LOG.warn("Skipping bls verification.");
return true;
}
Preconditions.checkArgument(
publicKeys.size() == messages.size() && publicKeys.size() == signatures.size(),
"Different collection sizes");
int count = publicKeys.size();
if (count == 0) return false;
if (doublePairing) {
Stream<List<Integer>> pairsStream =
Lists.partition(IntStream.range(0, count).boxed().collect(Collectors.toList()), 2)
.stream();

if (parallel) {
pairsStream = pairsStream.parallel();
try {
Preconditions.checkArgument(
publicKeys.size() == messages.size() && publicKeys.size() == signatures.size(),
"Different collection sizes");
int count = publicKeys.size();
if (count == 0) {
return false;
}
return completeBatchVerify(
pairsStream
.map(
idx ->
idx.size() == 1
? prepareBatchVerify(
idx.get(0),
publicKeys.get(idx.get(0)),
messages.get(idx.get(0)),
signatures.get(idx.get(0)))
: prepareBatchVerify2(
idx.get(0),
publicKeys.get(idx.get(0)),
messages.get(idx.get(0)),
signatures.get(idx.get(0)),
publicKeys.get(idx.get(1)),
messages.get(idx.get(1)),
signatures.get(idx.get(1))))
.collect(Collectors.toList()));
} else {
Stream<Integer> indexStream = IntStream.range(0, count).boxed();
if (doublePairing) {
Stream<List<Integer>> pairsStream =
Lists.partition(IntStream.range(0, count).boxed().collect(Collectors.toList()), 2)
.stream();

if (parallel) {
pairsStream = pairsStream.parallel();
}
return completeBatchVerify(
pairsStream
.map(
idx ->
idx.size() == 1
? prepareBatchVerify(
idx.get(0),
publicKeys.get(idx.get(0)),
messages.get(idx.get(0)),
signatures.get(idx.get(0)))
: prepareBatchVerify2(
idx.get(0),
publicKeys.get(idx.get(0)),
messages.get(idx.get(0)),
signatures.get(idx.get(0)),
publicKeys.get(idx.get(1)),
messages.get(idx.get(1)),
signatures.get(idx.get(1))))
.collect(Collectors.toList()));
} else {
Stream<Integer> indexStream = IntStream.range(0, count).boxed();

if (parallel) {
indexStream = indexStream.parallel();
if (parallel) {
indexStream = indexStream.parallel();
}
return completeBatchVerify(
indexStream
.map(
idx ->
prepareBatchVerify(
idx, publicKeys.get(idx), messages.get(idx), signatures.get(idx)))
.collect(Collectors.toList()));
}
return completeBatchVerify(
indexStream
.map(
idx ->
prepareBatchVerify(
idx, publicKeys.get(idx), messages.get(idx), signatures.get(idx)))
.collect(Collectors.toList()));
} catch (IllegalArgumentException e) {
throw new BlsException("Failed to batchVerify", e);
}
}

Expand Down Expand Up @@ -330,7 +357,7 @@ public static BatchSemiAggregate prepareBatchVerify(
publicKeys.stream().map(BLSPublicKey::getPublicKey).collect(Collectors.toList()),
message,
signature.getSignature());
} catch (DeserializeException e) {
} catch (BlsException e) {
return new InvalidBatchSemiAggregate();
}
}
Expand Down Expand Up @@ -360,7 +387,7 @@ public static BatchSemiAggregate prepareBatchVerify2(
publicKeys2.stream().map(BLSPublicKey::getPublicKey).collect(Collectors.toList()),
message2,
signature2.getSignature());
} catch (DeserializeException e) {
} catch (BlsException e) {
return new InvalidBatchSemiAggregate();
}
}
Expand Down
25 changes: 12 additions & 13 deletions bls/src/main/java/tech/pegasys/teku/bls/BLSSignature.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.ssz.InvalidSSZTypeException;
import org.apache.tuweni.ssz.SSZ;
import tech.pegasys.teku.bls.impl.DeserializeException;
import tech.pegasys.teku.bls.impl.BlsException;
import tech.pegasys.teku.bls.impl.Signature;

public class BLSSignature {
Expand All @@ -35,7 +35,11 @@ public class BLSSignature {
+ "0000000000000000000000000000000000000000000000000000000000000000");

/**
* Creates an empty signature (all zero bytes). Note that this is not a valid signature.
* Creates an empty signature (all zero bytes).
*
* <p>Note that this is not a valid signature. We can create the empty signature and serialise it
* without problems, but attempting to use it by calling {@link #getSignature()} will result in a
* DeserializeException.
*
* @return the empty signature
*/
Expand All @@ -56,7 +60,7 @@ public static BLSSignature fromSSZBytes(Bytes bytes) {
return SSZ.decode(
bytes, reader -> new BLSSignature(reader.readFixedBytes(SSZ_BLS_SIGNATURE_SIZE)));
} catch (InvalidSSZTypeException e) {
throw new DeserializeException("Failed to create signature from SSZ.");
throw new BlsException("Failed to create signature from SSZ.");
}
}

Expand Down Expand Up @@ -104,26 +108,21 @@ public Bytes toBytesCompressed() {
}

Signature getSignature() {
try {
return signature.get();
} catch (final IllegalArgumentException e) {
throw new DeserializeException(e.getMessage());
}
return signature.get();
}

public boolean isInfinity() {
try {
Signature signature = getSignature();
return signature.isInfinity();
} catch (final DeserializeException e) {
return getSignature().isInfinity();
} catch (final IllegalArgumentException e) {
return false;
}
}

public boolean isValid() {
try {
return getSignature().isValid();
} catch (final DeserializeException e) {
return getSignature().isInGroup();
} catch (final IllegalArgumentException e) {
return false;
}
}
Expand Down
10 changes: 5 additions & 5 deletions bls/src/main/java/tech/pegasys/teku/bls/impl/BLS12381.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ default KeyPair generateKeyPair(long seed) {
* @return a public key. Note that implementation may lazily evaluate passed bytes so the method
* may not immediately fail if the supplied bytes are invalid. Use {@link
* PublicKey#forceValidation()} to validate immediately
* @throws DeserializeException If the supplied bytes are not a valid public key However if
* implementing class lazily parses bytes the exception might not be thrown on invalid input
* but throw on later usage. Use {@link PublicKey#forceValidation()} if need to immediately
* ensure input validity
* @throws BlsException If the supplied bytes are not a valid public key However if implementing
* class lazily parses bytes the exception might not be thrown on invalid input but throw on
* later usage. Use {@link PublicKey#forceValidation()} if need to immediately ensure input
* validity
*/
PublicKey publicKeyFromCompressed(Bytes48 compressedPublicKeyBytes) throws DeserializeException;
PublicKey publicKeyFromCompressed(Bytes48 compressedPublicKeyBytes) throws BlsException;

/**
* Decode a signature from its <em>compressed</em> form serialized representation.
Expand Down
8 changes: 6 additions & 2 deletions ...s/teku/bls/impl/DeserializeException.java → ...h/pegasys/teku/bls/impl/BlsException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@

package tech.pegasys.teku.bls.impl;

public class DeserializeException extends IllegalArgumentException {
public class BlsException extends IllegalArgumentException {

public DeserializeException(String message) {
public BlsException(String message) {
super(message);
}

public BlsException(String message, Throwable cause) {
super(message, cause);
}
}
11 changes: 3 additions & 8 deletions bls/src/main/java/tech/pegasys/teku/bls/impl/Signature.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,11 @@ default boolean verify(PublicKey publicKey, Bytes message) {
boolean isInfinity();

/**
* Returns true if the point is on the curve and is in the G2 group.
* Determine if this Signature is in the G2 Group.
*
* <p>Note that the G2 point at infinity may or may not be a valid signature, depending on where
* it came from: a deserialised infinite signature is valid; an infinite signature resulting from
* a failed deserialisation is invalid.
*
* @return true if this signature is on the curve and in the G2 group, and is not a bad infinite
* signature, otherwise false.
* @return true if this signature is in the G2 group, otherwise false.
*/
boolean isValid();
boolean isInGroup();

/** Implementation must override */
@Override
Expand Down
Loading

0 comments on commit 22aa754

Please sign in to comment.