Add message validation and signature verification for FROST protocol#39
Add message validation and signature verification for FROST protocol#39kwsantiago merged 2 commits intomainfrom
Conversation
|
Warning Rate limit exceeded@kwsantiago has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 16 minutes and 5 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis PR adds runtime validation and cryptographic verification to the message handling pipeline. It introduces size and resource limit constants with a message validation method in the protocol layer, enforces pre- and post-decryption size checks during message decryption, and adds immediate cryptographic verification of aggregated FROST signatures before serialization. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
keep-frost-net/src/event.rs (1)
192-218: Well-structured defense-in-depth for message size validation.The two-layer validation approach is sound:
- Pre-decryption guard (line 193) prevents resource exhaustion from oversized encrypted payloads
- Post-decryption guard (line 214) enforces the protocol's MAX_MESSAGE_SIZE limit on actual content
Both checks return clear Protocol errors, providing consistent error handling across the validation pipeline.
The 2x multiplier for encrypted content size is conservative. NIP-44 v2 adds a fixed 65-byte overhead (1-byte version + 32-byte nonce + 32-byte MAC) plus base64 encoding expansion (~1.33x for the payload), so the encrypted size typically approximates 1.33x the plaintext for large messages—well below the 2x buffer applied here.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
keep-frost-net/src/event.rskeep-frost-net/src/protocol.rskeep-frost-net/src/session.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
keep-frost-net/src/protocol.rs (3)
8-16: Well-designed validation constants.The size limits and resource constraints are appropriate for the FROST protocol. The 64KB message limit provides reasonable headroom while preventing memory exhaustion, and the cryptographic size limits (128 bytes for commitments, 64 bytes for signature shares) align well with secp256k1 parameters.
68-121: Excellent validation implementation.The validation strategy is well-executed:
- Enforces limits at the deserialization boundary, preventing invalid messages from entering the system
- Each message variant is validated against appropriate constraints
- Fixed-size message types (SignatureComplete, Ping, Pong) are correctly excluded via the wildcard pattern
- Static error messages provide clear feedback without allocations
455-560: Comprehensive test coverage.The test suite thoroughly validates all boundary conditions:
- Each size limit is tested with values exceeding the threshold by 1
- Valid cases are tested to ensure legitimate messages pass validation
- All message variants with validation rules are covered
This gives strong confidence that the validation logic behaves correctly.
keep-frost-net/src/session.rs (1)
211-217: Critical security improvement: signature verification before use.Excellent addition of immediate cryptographic verification after aggregation. This ensures that:
- Malformed or invalid aggregated signatures are caught before serialization
- Session state is properly set to
Failedon verification failure- The verification happens at the right point in the flow (post-aggregation, pre-serialization)
This significantly strengthens the protocol's security guarantees.
f090103 to
b420192
Compare
b420192 to
ed4f32b
Compare
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.