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

Protocol Feature: Remove Canonical Signature Check #1106

Open
bhazzard opened this issue Jan 16, 2025 · 4 comments
Open

Protocol Feature: Remove Canonical Signature Check #1106

bhazzard opened this issue Jan 16, 2025 · 4 comments
Labels
consensus-protocol Change to the consensus protocol. Impacts light client validation. discussion

Comments

@bhazzard
Copy link

Requiring signatures to be canonical is necessary to prevent Signature Malleability attacks.

However, in Antelope-based chains, this requirement serves no functional purpose since the signature does not affect the transaction hash generation.

Consequently, even if an attacker alters the signature, they cannot exploit it.

The primary problem with canonical checks is that they require the signer to repeat the entire signing process until a canonical signature is found. This can be quite time-consuming for applications that need high-speed signing.

@spoonincode
Copy link
Member

See AntelopeIO/leap#2395 & EOSIO/eos#6699 for history

@heifner heifner added the consensus-protocol Change to the consensus protocol. Impacts light client validation. label Jan 23, 2025
@bhazzard bhazzard removed the triage label Jan 23, 2025
@bhazzard bhazzard added this to the Spring v1.x Cusp milestone Jan 23, 2025
@bhazzard
Copy link
Author

Next step is technical proposal for how to attack this.

@0xKodzy
Copy link

0xKodzy commented Feb 8, 2025

Theoretically, it should be as simple as just replacing function

bool public_key::is_canonical( const compact_signature& c ) {
        return !(c.data[1] & 0x80)
               && !(c.data[1] == 0 && !(c.data[2] & 0x80))
               && !(c.data[33] & 0x80)
               && !(c.data[33] == 0 && !(c.data[34] & 0x80));
}

with

bool public_key::is_canonical( const compact_signature& c ) {
        return true;
}

@spoonincode
Copy link
Member

It's not quite that simple -- we need to guard it behind a protocol feature. But yeah it's a very small change.

One (now resolved after some discussion with others) question was whether to keep the requirement that blocks are signed with a low-s signature (while still removing the weird Antelope-specific canonical check). The benefit (I thought) to this is that it will ensure that every node's block log is byte identical -- a nice property imo. I was aware that we lost this property with WTMSIG_BLOCK_SIGNATURES though I initially thought we removed WTMSIG_BLOCK_SIGNATURES as part of Savanna but.. we didn't. So it appears that is no benefit in enforcing low-s since we still cannot ensure block logs are byte identical due to the continued support of WTMSIG_BLOCK_SIGNATURES.

The last remaining item I'd like to noodle through is how (if at all) nodeos can convey to wallets that a canonical signature is or is not required. Certainly a wallet could probe get_activated_protocol_features but that doesn't seem ideal. I suppose a wallet could probe get_activated_protocol_features and if this new protocol feature is activated the wallet could internally remember that given some chainid it never needs to probe get_activated_protocol_features again. A wallet taking that strategy is nice as it won't bang on get_activated_protocol_features forever, though it puts more onus on a wallet to remember this sort of stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus-protocol Change to the consensus protocol. Impacts light client validation. discussion
Projects
Status: Todo
Development

No branches or pull requests

5 participants