Skip to content

Conversation

@brndnmtthws
Copy link
Owner

@brndnmtthws brndnmtthws commented Mar 20, 2025

Adds functions to validate Ed25519 points and X25519 public keys:

  1. crypto_core_ed25519_is_valid_point() in crypto_core.rs:

    • Implements strict Ed25519 point validation:
      • Rejects non-canonical encodings (high bit set)
      • Rejects all-zero point
      • Rejects identity element (small-order point)
    • Uses curve25519-dalek for curve equation check.
    • Stricter than libsodium for enhanced security.
  2. crypto_core_ed25519_is_valid_point_relaxed() in crypto_core.rs:

    • A variant of the above that allows the high bit to be set.
    • Use this for validating public keys generated by crypto_sign_keypair.
  3. KeyPair::is_valid_public_key() in keypair.rs:

    • Validates X25519 public keys for crypto_box.
    • Checks for non-zero point and high bit cleared.
    • Does not perform full curve check (per X25519 requirements).
  4. KeyPair::is_valid_ed25519_key() in keypair.rs:

    • Convenience wrapper for crypto_core_ed25519_is_valid_point_relaxed.

Also includes comprehensive tests for all new validation logic and adds
the CRYPTO_CORE_ED25519_BYTES constant.

These functions help prevent security issues from invalid cryptographic inputs.

@netlify
Copy link

netlify bot commented Mar 20, 2025

Deploy Preview for dryoc-docs ready!

Name Link
🔨 Latest commit 154b687
🔍 Latest deploy log https://app.netlify.com/sites/dryoc-docs/deploys/68141dab5c6b79000864b151
😎 Deploy Preview https://deploy-preview-84--dryoc-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@brndnmtthws brndnmtthws marked this pull request as draft March 20, 2025 13:38
@brndnmtthws brndnmtthws marked this pull request as ready for review May 2, 2025 00:38
@brndnmtthws brndnmtthws requested a review from Copilot May 2, 2025 00:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request adds a function to verify Ed25519 public key points and a complementary X25519 public key validator, with updated constants and tests to ensure correct cryptographic validations.

  • Introduces the function is_valid_public_key() in src/keypair.rs for X25519 key validation.
  • Adds the function crypto_core_ed25519_is_valid_point() in src/classic/crypto_core.rs with stricter checks for Ed25519 points and corresponding tests.
  • Updates constants in src/constants.rs and adds a dependency in Cargo.toml to support relevant functionality.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/keypair.rs Added is_valid_public_key() and related tests for X25519 public key validation.
src/constants.rs Introduced CRYPTO_CORE_ED25519_BYTES constant for use in Ed25519 point validation.
src/classic/crypto_core.rs Added crypto_core_ed25519_is_valid_point() with stricter validation checks and tests.
Cargo.toml Added libsodium-sys dependency.

@codecov
Copy link

codecov bot commented May 2, 2025

Codecov Report

Attention: Patch coverage is 88.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 67.66%. Comparing base (1b51d16) to head (154b687).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/keypair.rs 77.77% 2 Missing ⚠️
src/classic/crypto_core.rs 93.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #84      +/-   ##
==========================================
+ Coverage   67.52%   67.66%   +0.14%     
==========================================
  Files          44       44              
  Lines        3242     3266      +24     
==========================================
+ Hits         2189     2210      +21     
- Misses       1053     1056       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@brndnmtthws brndnmtthws force-pushed the is-point-valid branch 3 times, most recently from 39350d8 to b6f7e8b Compare May 2, 2025 01:15
@brndnmtthws brndnmtthws requested a review from Copilot May 2, 2025 01:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds functionality to verify whether a given public key is a valid point on the Edwards curve according to different rules—one for standard X25519 keys and one using relaxed rules for Ed25519 keys. Key changes include adding new validation methods in the keypair module, introducing corresponding tests, and adding a new constant for the Ed25519 point size.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/keypair.rs Added two public methods for validating X25519 and Ed25519 keys.
src/constants.rs Introduced a new constant for Ed25519 point length.
src/classic/crypto_sign_ed25519.rs Minor style fix (added semicolon) for consistency.
src/classic/crypto_core.rs Added strict and relaxed Ed25519 point validation functions and tests.
Comments suppressed due to low confidence (1)

src/classic/crypto_core.rs:501

  • [nitpick] Consider removing or conditionally compiling debug println! statements in tests to reduce noise in test outputs.
println!("\n=== Testing Ed25519 key validation across {} iterations ===", iterations);

Adds functions to validate Ed25519 points and X25519 public keys:

1. `crypto_core_ed25519_is_valid_point()` in `crypto_core.rs`:
   - Implements strict Ed25519 point validation:
     * Rejects non-canonical encodings (high bit set)
     * Rejects all-zero point
     * Rejects identity element (small-order point)
   - Uses curve25519-dalek for curve equation check.
   - Stricter than libsodium for enhanced security.

2. `crypto_core_ed25519_is_valid_point_relaxed()` in `crypto_core.rs`:
   - A variant of the above that allows the high bit to be set.
   - Use this for validating public keys generated by `crypto_sign_keypair`.

3. `KeyPair::is_valid_public_key()` in `keypair.rs`:
   - Validates X25519 public keys for `crypto_box`.
   - Checks for non-zero point and high bit cleared.
   - Does not perform full curve check (per X25519 requirements).

4. `KeyPair::is_valid_ed25519_key()` in `keypair.rs`:
   - Convenience wrapper for `crypto_core_ed25519_is_valid_point_relaxed`.

Also includes comprehensive tests for all new validation logic and adds
the `CRYPTO_CORE_ED25519_BYTES` constant.

These functions help prevent security issues from invalid cryptographic inputs.
@brndnmtthws brndnmtthws changed the title Add crypto_core_ed25519_is_valid_point() feat(crypto): add Ed25519 point and X25519 key validation May 2, 2025
@brndnmtthws brndnmtthws merged commit 7e8c7e5 into main May 2, 2025
27 checks passed
@brndnmtthws brndnmtthws deleted the is-point-valid branch May 2, 2025 01:24
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

Successfully merging this pull request may close these issues.

2 participants