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

Multiplying by a co-factor in [batch]verification #115

Closed
valerini opened this issue Feb 21, 2020 · 5 comments
Closed

Multiplying by a co-factor in [batch]verification #115

valerini opened this issue Feb 21, 2020 · 5 comments

Comments

@valerini
Copy link

I have a question around batch/single signature verification. As dalek is not multiplying by a co-factor in verifications (which in my opinion is ok for security, as well as multiplying by a co-factor is ok for security), this might create discrepancies when parties need to agree whether a batch of signatures is valid or not.

Imagine, a malicious user injects in a batch a signature sig = (S, R) with non-empty small-subgroup component in R (where the public key A is clean of small-subgroup components), where the small order subgroup component is of order 2, then batch verification with this signature will fail with probability 1/2 and succeed with probability 1/2. This will cause some problems for using batch-verification in consensus. I believe multiplying by 8 in verifications should resolve this issue, but would be very appreciative to see other opinions or to be corrected!

P.S. Thank you for the great work on this library!

@tarcieri
Copy link
Contributor

I believe the ambiguities of “cofactorless” verification are inherent to batch verification. See this thread for some background:

https://moderncrypto.org/mail-archive/curves/2016/000836.html

The effects of this on a consensus protocol are that consensus participants need to perform a full verification of all signatures, but that batch verification is still useful for non-consensus nodes and light clients (since the consensus participants have verified the absence of these ambiguities in advance).

@burdges
Copy link

burdges commented Feb 23, 2020

Appears @isislovecruft provided a batch_deterministic feature for deterministic delinearization, presumably to address this.

I think an argument works works better than a feature here, so maybe

fn verify_batch(
    messages: &[&[u8]],
    signatures: &[Signature],
    public_keys: &[PublicKey],
    deterministic: bool,
) -> Result<(), SignatureError> 

or if bools bother you then

fn verify_batch_rng<R: RngCore>(
    messages: &[&[u8]],
    signatures: &[Signature],
    public_keys: &[PublicKey],
    rng: R,
) -> Result<(), SignatureError> 
{
    ...
}

pub fn verify_batch(
    messages: &[&[u8]],
    signatures: &[Signature],
    public_keys: &[PublicKey],
) -> Result<(), SignatureError>
{
    verify_batch_rng(messages,signatures,public_keys,thread_rng())
}

pub fn verify_batch_deterministic(
    messages: &[&[u8]],
    signatures: &[Signature],
    public_keys: &[PublicKey],
) -> Result<(), SignatureError>
{
    verify_batch_rng(messages,signatures,public_keys,ZeroRng)
}

As an aside, rand_core alone provides system randomness, so I put a feature gate on rand being a dependency and using thread_rng like https://github.com/w3f/schnorrkel/blob/master/src/lib.rs#L226-L234 but not really sure if that's the best choice.

@kchalkias
Copy link

kchalkias commented Feb 27, 2020

@burdges @tarcieri deterministic batch verification was proposed and implemented for many reasons:

  • to ensure correctness even in clients with low or zero entropy. Otherwise, one can target such devices to falsely pass an invalid signature (i..e, imagine if all z values are zero or the same for some reason).
  • In blockchain smart contracts (Libra, Ethereum, Corda) you might not have access to rng at all. Then you cannot take advantage of non-deterministic batch verification anyway, you need a deterministic one for one-shot multi-sig contracts.
  • If for some reason a blockchain decides to support non-deterministic batch verification, one can exploit it using what @valerini mentioned and cause inconsistency to DLT states as well. In fact, it's the same as if you had an RNG available.
  • Multiplying by the cofactor (or set the last 3 bits of z values to zero) will make batch verification compatible with NIST's standardization effort on single signature EdDSA, where (wisely IMO) they eventually chose to multiply by 8 and not allow optionality.

Note: there is a common misconception that NIST should have picked between multiplying and not multiplying by the cofactor and any choice would be acceptable. This is not correct, because if an application accepts both single and batch verification, multiplying by the cofactor is safer to ensure compatibility between those two (if a sig does not pass the batch verification, it shouldn't pass the iterative single verification too and vice versa).

@isislovecruft
Copy link
Member

Note that we currently multiply by the cofactor in verify_strict() but we do not yet have an equivalent verify_batch_strict() function yet.

huitseeker added a commit to huitseeker/ed25519-dalek that referenced this issue Jan 5, 2022
In a nutshell, this offers an opt-in way of performing some public key checks relating to small order components, without having to pay an additional point decompression.

In detail:
Since dalek-cryptography@8dbaf9a, the `PublicKey` type is the performant way to carry public key material, with an eager check that the point is on curve.

However, some applications which may like eager point decompression also need to check whether the point is small order, or even torsion-free:
- aligning a discrepancy in verification between batch verification and iterated verification (see dalek-cryptography#115),
- avoiding small subgroup confinement attacks in a DH,
- ...

`verify_strict` was introduced to offer an opt-in approach to some of this sort of scrutiny at the time the key is used, but cannot be performed eagerly, e.g. at the time of deserializing a public key.

Rejecting small order keys (or worse non-torsion-free) keys on deserialization would have a performance impact. However, it's still desirable to have the option to do so long before the key is ever used for any actual cryptographic purpose (e.g. signature verification).

In order to perform this sort of check, some code bases have taken to [re-implementing the check from the bytes representation of the key, which involves an additional decompression](https://github.com/diem/diem/blob/a290b0859a6152a5ffd6f85773a875f17334adac/crates/diem-crypto/src/ed25519.rs#L358-L386).
The added functions of this PR allow the checks to be performed without additional decompression.
huitseeker added a commit to huitseeker/ed25519-dalek that referenced this issue Jan 5, 2022
In a nutshell, this offers an opt-in way of performing some public key checks relating to small order components, without having to pay an additional point decompression.

In detail:
Since dalek-cryptography@8dbaf9a, the `PublicKey` type is the performant way to carry public key material, with an eager check that the point is on curve.

However, some applications which may like eager point decompression also need to check whether the point is small order, or even torsion-free:
- aligning a discrepancy in verification between batch verification and iterated verification (see dalek-cryptography#115),
- avoiding small subgroup confinement attacks in a DH,
- ...

`verify_strict` was introduced to offer an opt-in approach to some of this sort of scrutiny at the time the key is used, but cannot be performed eagerly, e.g. at the time of deserializing a public key.

Rejecting small order keys (or worse non-torsion-free) keys on deserialization would have a performance impact. However, it's still desirable to have the option to do so long before the key is ever used for any actual cryptographic purpose (e.g. signature verification).

In order to perform this sort of check, some code bases have taken to [re-implementing the check from the bytes representation of the key, which involves an additional decompression](https://github.com/diem/diem/blob/a290b0859a6152a5ffd6f85773a875f17334adac/crates/diem-crypto/src/ed25519.rs#L358-L386).
The added functions of this PR allow the checks to be performed without additional decompression.
@rozbb
Copy link
Contributor

rozbb commented Jan 20, 2023

Discussion about verify_batch_strict seems dead. Original topic, which was cofactor multiplication for batch verification, is a dupe of #152 . Closing

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

No branches or pull requests

6 participants