Skip to content

Commit

Permalink
[crypto] remove malleability check
Browse files Browse the repository at this point in the history
if we want this, we can use strict_verify

Closes: aptos-labs#425
  • Loading branch information
davidiw authored and aptos-bot committed Apr 7, 2022
1 parent 2284fec commit e623f22
Show file tree
Hide file tree
Showing 6 changed files with 7 additions and 38 deletions.
9 changes: 0 additions & 9 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,6 @@ jobs:
steps:
- dev-setup
- run: cargo x bench --no-run
crypto:
docker:
- image: cimg/base:stable
resource_class: medium
steps:
- dev-setup
- run: cargo nextest --nextest-profile ci --package aptos-crypto --features='u32' --no-default-features
- run: cargo nextest --nextest-profile ci --package aptos-crypto --features='u64' --no-default-features
lint:
docker:
- image: cimg/base:2020.01
Expand Down Expand Up @@ -210,7 +202,6 @@ workflows:
- equal: [ canary, << pipeline.git.branch >> ]
jobs:
# - build-benchmarks
- crypto
- e2e-test
- lint
- unit-test
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 1 addition & 26 deletions crates/aptos-crypto/src/ed25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,32 +356,7 @@ impl TryFrom<&[u8]> for Ed25519PublicKey {
/// Deserialize an Ed25519PublicKey. This method will also check for key validity, for instance
/// it will only deserialize keys that are safe against small subgroup attacks.
fn try_from(bytes: &[u8]) -> std::result::Result<Ed25519PublicKey, CryptoMaterialError> {
// We need to access the Edwards point which is not directly accessible from
// ed25519_dalek::PublicKey, so we need to do some custom deserialization.
if bytes.len() != ED25519_PUBLIC_KEY_LENGTH {
return Err(CryptoMaterialError::WrongLengthError);
}

let mut bits = [0u8; ED25519_PUBLIC_KEY_LENGTH];
bits.copy_from_slice(&bytes[..ED25519_PUBLIC_KEY_LENGTH]);

let compressed = curve25519_dalek::edwards::CompressedEdwardsY(bits);
let point = compressed
.decompress()
.ok_or(CryptoMaterialError::DeserializationError)?;

// Check if the point lies on a small subgroup. This is required
// when using curves with a small cofactor (in ed25519, cofactor = 8).
if point.is_small_order() {
return Err(CryptoMaterialError::SmallSubgroupError);
}

// Unfortunately, tuple struct `PublicKey` is private so we cannot
// Ok(Ed25519PublicKey(ed25519_dalek::PublicKey(compressed, point)))
// and we have to again invoke deserialization.
let public_key = Ed25519PublicKey::from_bytes_unchecked(bytes)?;
add_tag!(&public_key, ValidatedPublicKeyTag); // This key has gone through validity checks.
Ok(public_key)
Ed25519PublicKey::from_bytes_unchecked(bytes)
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/aptos-crypto/src/unit_tests/ed25519_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ proptest! {
// modifying the public key and the R component, under a pathological yet
// admissible s < l value for the signature. It shows the difference
// between `verify` and `verify_strict` in ed25519-dalek
#[ignore]
#[test]
fn verify_sig_strict_torsion(idx in 0usize..8usize){
let message = b"hello_world";
Expand Down Expand Up @@ -438,6 +439,7 @@ proptest! {
}

// Test against known small subgroup public keys.
#[ignore]
#[test]
fn test_publickey_smallorder() {
for torsion_point in &EIGHT_TORSION {
Expand Down
1 change: 1 addition & 0 deletions crates/aptos-crypto/src/unit_tests/multi_ed25519_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ fn test_multi_ed25519_public_key_serialization() {
}

// Test against known small subgroup public key.
#[ignore]
#[test]
fn test_publickey_smallorder() {
// A small group point with threshold 1 (last byte).
Expand Down
4 changes: 2 additions & 2 deletions crates/aptos-workspace-hack/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ tokio = { version = "1.17.0", features = ["bytes", "fs", "full", "io-std", "io-u
tokio-util = { version = "0.6.9", features = ["codec", "compat", "futures-io", "io"] }
tracing = { version = "0.1.32", features = ["attributes", "log", "std", "tracing-attributes"] }
warp = { version = "0.3.2", features = ["multipart", "tls", "tokio-rustls", "tokio-tungstenite", "websocket"] }
zeroize = { version = "1.4.0", features = ["alloc", "zeroize_derive"] }
zeroize = { version = "1.5.4", features = ["alloc", "zeroize_derive"] }

[build-dependencies]
Inflector = { version = "0.11.4", features = ["heavyweight", "lazy_static", "regex"] }
Expand Down Expand Up @@ -104,7 +104,7 @@ tokio = { version = "1.17.0", features = ["bytes", "fs", "full", "io-std", "io-u
tokio-util = { version = "0.6.9", features = ["codec", "compat", "futures-io", "io"] }
tracing = { version = "0.1.32", features = ["attributes", "log", "std", "tracing-attributes"] }
warp = { version = "0.3.2", features = ["multipart", "tls", "tokio-rustls", "tokio-tungstenite", "websocket"] }
zeroize = { version = "1.4.0", features = ["alloc", "zeroize_derive"] }
zeroize = { version = "1.5.4", features = ["alloc", "zeroize_derive"] }

[target.x86_64-unknown-linux-gnu.dependencies]
hashbrown = { version = "0.11.2", default-features = false, features = ["inline-more", "raw"] }
Expand Down

0 comments on commit e623f22

Please sign in to comment.