fix(ext/node): ed25519 signing and cipheriv autopadding fixes (#24957)

- Return auth tag for GCM ciphers from auto padding shortcircuit
- Use _ring_ for ed25519 signing

---------

Co-authored-by: Divy Srivastava <dj.srivastava23@gmail.com>

ext/node/lib.rs

@@ -226,6 +226,7 @@ deno_core::extension!(deno_node,
     ops::crypto::op_node_cipheriv_encrypt,
     ops::crypto::op_node_cipheriv_final,
     ops::crypto::op_node_cipheriv_set_aad,
+    ops::crypto::op_node_cipheriv_take,
     ops::crypto::op_node_create_cipheriv,
     ops::crypto::op_node_create_decipheriv,
     ops::crypto::op_node_create_hash,
@@ -260,7 +261,9 @@ deno_core::extension!(deno_node,
     ops::crypto::op_node_scrypt_async,
     ops::crypto::op_node_scrypt_sync,
     ops::crypto::op_node_sign,
+    ops::crypto::op_node_sign_ed25519,
     ops::crypto::op_node_verify,
+    ops::crypto::op_node_verify_ed25519,
     ops::crypto::keys::op_node_create_private_key,
     ops::crypto::keys::op_node_create_public_key,
     ops::crypto::keys::op_node_create_secret_key,

ext/node/ops/crypto/cipher.rs

@@ -64,6 +64,10 @@ impl CipherContext {
     self.cipher.borrow_mut().encrypt(input, output);
   }
 
+  pub fn take_tag(self) -> Tag {
+    Rc::try_unwrap(self.cipher).ok()?.into_inner().take_tag()
+  }
+
   pub fn r#final(
     self,
     auto_pad: bool,
@@ -290,6 +294,15 @@ impl Cipher {
       }
     }
   }
+
+  fn take_tag(self) -> Tag {
+    use Cipher::*;
+    match self {
+      Aes128Gcm(cipher) => Some(cipher.finish().to_vec()),
+      Aes256Gcm(cipher) => Some(cipher.finish().to_vec()),
+      _ => None,
+    }
+  }
 }
 
 impl Decipher {

ext/node/ops/crypto/keys.rs

@@ -496,14 +496,9 @@ impl KeyObjectHandle {
         AsymmetricPrivateKey::X25519(x25519_dalek::StaticSecret::from(bytes))
       }
       ED25519_OID => {
-        let string_ref = OctetStringRef::from_der(pk_info.private_key)
+        let signing_key = ed25519_dalek::SigningKey::try_from(pk_info)
           .map_err(|_| type_error("invalid Ed25519 private key"))?;
-        if string_ref.as_bytes().len() != 32 {
+        AsymmetricPrivateKey::Ed25519(signing_key)
-          return Err(type_error("Ed25519 private key is the wrong length"));
-        }
-        let mut bytes = [0; 32];
-        bytes.copy_from_slice(string_ref.as_bytes());
-        AsymmetricPrivateKey::Ed25519(ed25519_dalek::SigningKey::from(bytes))
       }
       DH_KEY_AGREEMENT_OID => {
         let params = pk_info
@@ -643,16 +638,8 @@ impl KeyObjectHandle {
         AsymmetricPublicKey::X25519(x25519_dalek::PublicKey::from(bytes))
       }
       ED25519_OID => {
-        let mut bytes = [0; 32];
+        let verifying_key = ed25519_dalek::VerifyingKey::try_from(spki)
-        let data = spki.subject_public_key.as_bytes().ok_or_else(|| {
+          .map_err(|_| type_error("invalid Ed25519 private key"))?;
-          type_error("malformed or missing public key in ed25519 spki")
-        })?;
-        if data.len() < 32 {
-          return Err(type_error("ed25519 public key is too short"));
-        }
-        bytes.copy_from_slice(&data[0..32]);
-        let verifying_key = ed25519_dalek::VerifyingKey::from_bytes(&bytes)
-          .map_err(|_| type_error("ed25519 public key is malformed"))?;
         AsymmetricPublicKey::Ed25519(verifying_key)
       }
       DH_KEY_AGREEMENT_OID => {

ext/node/ops/crypto/mod.rs

@@ -20,6 +20,7 @@ use num_bigint_dig::BigUint;
 use rand::distributions::Distribution;
 use rand::distributions::Uniform;
 use rand::Rng;
+use ring::signature::Ed25519KeyPair;
 use std::future::Future;
 use std::rc::Rc;
 
@@ -272,6 +273,18 @@ pub fn op_node_cipheriv_final(
   context.r#final(auto_pad, input, output)
 }
 
+#[op2]
+#[buffer]
+pub fn op_node_cipheriv_take(
+  state: &mut OpState,
+  #[smi] rid: u32,
+) -> Result<Option<Vec<u8>>, AnyError> {
+  let context = state.resource_table.take::<cipher::CipherContext>(rid)?;
+  let context = Rc::try_unwrap(context)
+    .map_err(|_| type_error("Cipher context is already in use"))?;
+  Ok(context.take_tag())
+}
+
 #[op2(fast)]
 #[smi]
 pub fn op_node_create_decipheriv(
@@ -938,3 +951,50 @@ pub fn op_node_diffie_hellman(
 
   Ok(res)
 }
+
+#[op2(fast)]
+pub fn op_node_sign_ed25519(
+  #[cppgc] key: &KeyObjectHandle,
+  #[buffer] data: &[u8],
+  #[buffer] signature: &mut [u8],
+) -> Result<(), AnyError> {
+  let private = key
+    .as_private_key()
+    .ok_or_else(|| type_error("Expected private key"))?;
+
+  let ed25519 = match private {
+    AsymmetricPrivateKey::Ed25519(private) => private,
+    _ => return Err(type_error("Expected Ed25519 private key")),
+  };
+
+  let pair = Ed25519KeyPair::from_seed_unchecked(ed25519.as_bytes().as_slice())
+    .map_err(|_| type_error("Invalid Ed25519 private key"))?;
+  signature.copy_from_slice(pair.sign(data).as_ref());
+
+  Ok(())
+}
+
+#[op2(fast)]
+pub fn op_node_verify_ed25519(
+  #[cppgc] key: &KeyObjectHandle,
+  #[buffer] data: &[u8],
+  #[buffer] signature: &[u8],
+) -> Result<bool, AnyError> {
+  let public = key
+    .as_public_key()
+    .ok_or_else(|| type_error("Expected public key"))?;
+
+  let ed25519 = match &*public {
+    AsymmetricPublicKey::Ed25519(public) => public,
+    _ => return Err(type_error("Expected Ed25519 public key")),
+  };
+
+  let verified = ring::signature::UnparsedPublicKey::new(
+    &ring::signature::ED25519,
+    ed25519.as_bytes().as_slice(),
+  )
+  .verify(data, signature)
+  .is_ok();
+
+  Ok(verified)
+}

ext/node/ops/crypto/sign.rs

@@ -2,12 +2,6 @@
 use deno_core::error::generic_error;
 use deno_core::error::type_error;
 use deno_core::error::AnyError;
-use digest::Digest;
-use digest::FixedOutput;
-use digest::FixedOutputReset;
-use digest::OutputSizeUser;
-use digest::Reset;
-use digest::Update;
 use rand::rngs::OsRng;
 use rsa::signature::hazmat::PrehashSigner as _;
 use rsa::signature::hazmat::PrehashVerifier as _;
@@ -146,29 +140,9 @@ impl KeyObjectHandle {
       AsymmetricPrivateKey::X25519(_) => {
         Err(type_error("x25519 key cannot be used for signing"))
       }
-      AsymmetricPrivateKey::Ed25519(key) => {
+      AsymmetricPrivateKey::Ed25519(_) => Err(type_error(
-        if !matches!(
+        "Ed25519 key cannot be used for prehashed signing",
-          digest_type,
+      )),
-          "rsa-sha512" | "sha512" | "sha512withrsaencryption"
-        ) {
-          return Err(type_error(format!(
-            "digest not allowed for Ed25519 signature: {}",
-            digest_type
-          )));
-        }
-
-        let mut precomputed_digest = PrecomputedDigest([0; 64]);
-        if digest.len() != precomputed_digest.0.len() {
-          return Err(type_error("Invalid sha512 digest"));
-        }
-        precomputed_digest.0.copy_from_slice(digest);
-
-        let signature = key
-          .sign_prehashed(precomputed_digest, None)
-          .map_err(|_| generic_error("failed to sign digest with Ed25519"))?;
-
-        Ok(signature.to_bytes().into())
-      }
       AsymmetricPrivateKey::Dh(_) => {
         Err(type_error("DH key cannot be used for signing"))
       }
@@ -275,122 +249,12 @@ impl KeyObjectHandle {
       AsymmetricPublicKey::X25519(_) => {
         Err(type_error("x25519 key cannot be used for verification"))
       }
-      AsymmetricPublicKey::Ed25519(key) => {
+      AsymmetricPublicKey::Ed25519(_) => Err(type_error(
-        if !matches!(
+        "Ed25519 key cannot be used for prehashed verification",
-          digest_type,
+      )),
-          "rsa-sha512" | "sha512" | "sha512withrsaencryption"
-        ) {
-          return Err(type_error(format!(
-            "digest not allowed for Ed25519 signature: {}",
-            digest_type
-          )));
-        }
-
-        let mut signature_fixed = [0u8; 64];
-        if signature.len() != signature_fixed.len() {
-          return Err(type_error("Invalid Ed25519 signature"));
-        }
-        signature_fixed.copy_from_slice(signature);
-
-        let signature = ed25519_dalek::Signature::from_bytes(&signature_fixed);
-
-        let mut precomputed_digest = PrecomputedDigest([0; 64]);
-        precomputed_digest.0.copy_from_slice(digest);
-
-        Ok(
-          key
-            .verify_prehashed_strict(precomputed_digest, None, &signature)
-            .is_ok(),
-        )
-      }
       AsymmetricPublicKey::Dh(_) => {
         Err(type_error("DH key cannot be used for verification"))
       }
     }
   }
 }
-
-struct PrecomputedDigest([u8; 64]);
-
-impl OutputSizeUser for PrecomputedDigest {
-  type OutputSize = <sha2::Sha512 as OutputSizeUser>::OutputSize;
-}
-
-impl Digest for PrecomputedDigest {
-  fn new() -> Self {
-    unreachable!()
-  }
-
-  fn new_with_prefix(_data: impl AsRef<[u8]>) -> Self {
-    unreachable!()
-  }
-
-  fn update(&mut self, _data: impl AsRef<[u8]>) {
-    unreachable!()
-  }
-
-  fn chain_update(self, _data: impl AsRef<[u8]>) -> Self {
-    unreachable!()
-  }
-
-  fn finalize(self) -> digest::Output<Self> {
-    self.0.into()
-  }
-
-  fn finalize_into(self, _out: &mut digest::Output<Self>) {
-    unreachable!()
-  }
-
-  fn finalize_reset(&mut self) -> digest::Output<Self>
-  where
-    Self: digest::FixedOutputReset,
-  {
-    unreachable!()
-  }
-
-  fn finalize_into_reset(&mut self, _out: &mut digest::Output<Self>)
-  where
-    Self: digest::FixedOutputReset,
-  {
-    unreachable!()
-  }
-
-  fn reset(&mut self)
-  where
-    Self: digest::Reset,
-  {
-    unreachable!()
-  }
-
-  fn output_size() -> usize {
-    unreachable!()
-  }
-
-  fn digest(_data: impl AsRef<[u8]>) -> digest::Output<Self> {
-    unreachable!()
-  }
-}
-
-impl Reset for PrecomputedDigest {
-  fn reset(&mut self) {
-    unreachable!()
-  }
-}
-
-impl FixedOutputReset for PrecomputedDigest {
-  fn finalize_into_reset(&mut self, _out: &mut digest::Output<Self>) {
-    unreachable!()
-  }
-}
-
-impl FixedOutput for PrecomputedDigest {
-  fn finalize_into(self, _out: &mut digest::Output<Self>) {
-    unreachable!()
-  }
-}
-
-impl Update for PrecomputedDigest {
-  fn update(&mut self, _data: &[u8]) {
-    unreachable!()
-  }
-}

ext/node/polyfills/internal/crypto/cipher.ts

@@ -12,6 +12,7 @@ import {
   op_node_cipheriv_encrypt,
   op_node_cipheriv_final,
   op_node_cipheriv_set_aad,
+  op_node_cipheriv_take,
   op_node_create_cipheriv,
   op_node_create_decipheriv,
   op_node_decipheriv_decrypt,
@@ -194,7 +195,11 @@ export class Cipheriv extends Transform implements Cipher {
 
   final(encoding: string = getDefaultEncoding()): Buffer | string {
     const buf = new Buffer(16);
-
+    if (this.#cache.cache.byteLength == 0) {
+      const maybeTag = op_node_cipheriv_take(this.#context);
+      if (maybeTag) this.#authTag = Buffer.from(maybeTag);
+      return encoding === "buffer" ? Buffer.from([]) : "";
+    }
     if (!this.#autoPadding && this.#cache.cache.byteLength != 16) {
       throw new Error("Invalid final block size");
     }

ext/node/polyfills/internal/crypto/sig.ts

@@ -7,8 +7,11 @@
 import {
   op_node_create_private_key,
   op_node_create_public_key,
+  op_node_get_asymmetric_key_type,
   op_node_sign,
+  op_node_sign_ed25519,
   op_node_verify,
+  op_node_verify_ed25519,
 } from "ext:core/ops";
 
 import {
@@ -30,6 +33,8 @@ import {
   kConsumePublic,
   KeyObject,
   prepareAsymmetricKey,
+  PrivateKeyObject,
+  PublicKeyObject,
 } from "ext:deno_node/internal/crypto/keys.ts";
 import { createHash } from "ext:deno_node/internal/crypto/hash.ts";
 import { ERR_CRYPTO_SIGN_KEY_REQUIRED } from "ext:deno_node/internal/errors.ts";
@@ -191,7 +196,34 @@ export function signOneShot(
     throw new ERR_CRYPTO_SIGN_KEY_REQUIRED();
   }
 
-  const result = Sign(algorithm!).update(data).sign(key);
+  const res = prepareAsymmetricKey(key, kConsumePrivate);
+  let handle;
+  if ("handle" in res) {
+    handle = res.handle;
+  } else {
+    handle = op_node_create_private_key(
+      res.data,
+      res.format,
+      res.type ?? "",
+      res.passphrase,
+    );
+  }
+
+  let result: Buffer;
+  if (op_node_get_asymmetric_key_type(handle) === "ed25519") {
+    if (algorithm != null && algorithm !== "sha512") {
+      throw new TypeError("Only 'sha512' is supported for Ed25519 keys");
+    }
+    result = new Buffer(64);
+    op_node_sign_ed25519(handle, data, result);
+  } else if (algorithm == null) {
+    throw new TypeError(
+      "Algorithm must be specified when using non-Ed25519 keys",
+    );
+  } else {
+    result = Sign(algorithm!).update(data)
+      .sign(new PrivateKeyObject(handle));
+  }
 
   if (callback) {
     setTimeout(() => callback(null, result));
@@ -219,7 +251,33 @@ export function verifyOneShot(
     throw new ERR_CRYPTO_SIGN_KEY_REQUIRED();
   }
 
-  const result = Verify(algorithm!).update(data).verify(key, signature);
+  const res = prepareAsymmetricKey(key, kConsumePublic);
+  let handle;
+  if ("handle" in res) {
+    handle = res.handle;
+  } else {
+    handle = op_node_create_public_key(
+      res.data,
+      res.format,
+      res.type ?? "",
+      res.passphrase,
+    );
+  }
+
+  let result: boolean;
+  if (op_node_get_asymmetric_key_type(handle) === "ed25519") {
+    if (algorithm != null && algorithm !== "sha512") {
+      throw new TypeError("Only 'sha512' is supported for Ed25519 keys");
+    }
+    result = op_node_verify_ed25519(handle, data, signature);
+  } else if (algorithm == null) {
+    throw new TypeError(
+      "Algorithm must be specified when using non-Ed25519 keys",
+    );
+  } else {
+    result = Verify(algorithm!).update(data)
+      .verify(new PublicKeyObject(handle), signature);
+  }
 
   if (callback) {
     setTimeout(() => callback(null, result));