From b61fd622a5facc0ec29a8c3b04289ff5354ae03f Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Sun, 11 Aug 2024 06:28:54 -0700 Subject: [PATCH] fix(ext/node): rewrite X509Certificate resource and add `publicKey()` (#24988) **Changes**: - Remove unsafe usage, rewrite Rust representation with `yoke`. - Implement `X509Certificate.prototype.publicKey()` Fixes https://github.com/denoland/deno/issues/23307 --- Cargo.lock | 42 +++++++++- ext/node/Cargo.toml | 3 +- ext/node/lib.rs | 1 + ext/node/ops/crypto/keys.rs | 53 ++++++++++++ ext/node/ops/crypto/x509.rs | 94 +++++++++++++++------- ext/node/polyfills/internal/crypto/x509.ts | 13 +-- 6 files changed, 168 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dfbd3e49d6..ef4c29ccc3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -256,7 +256,7 @@ dependencies = [ "proc-macro2", "quote", "syn 1.0.109", - "synstructure", + "synstructure 0.12.6", ] [[package]] @@ -1831,6 +1831,7 @@ dependencies = [ "simd-json", "sm3", "spki", + "stable_deref_trait", "thiserror", "tokio", "url", @@ -7059,6 +7060,17 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "synstructure" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8af7666ab7b6390ab78131fb5b0fce11d6b7a6951602017c35fa82800708971" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.72", +] + [[package]] name = "syntect" version = "5.2.0" @@ -8517,9 +8529,22 @@ checksum = "6c5b1314b079b0930c31e3af543d8ee1757b1951ae1e1565ec704403a7240ca5" dependencies = [ "serde", "stable_deref_trait", + "yoke-derive", "zerofrom", ] +[[package]] +name = "yoke-derive" +version = "0.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "28cc31741b18cb6f1d5ff12f5b7523e3d6eb0852bbbad19d73905511d9849b95" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.72", + "synstructure 0.13.1", +] + [[package]] name = "zerocopy" version = "0.7.32" @@ -8546,6 +8571,21 @@ name = "zerofrom" version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "91ec111ce797d0e0784a1116d0ddcdbea84322cd79e5d5ad173daeba4f93ab55" +dependencies = [ + "zerofrom-derive", +] + +[[package]] +name = "zerofrom-derive" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ea7b4a3637ea8669cedf0f1fd5c286a17f3de97b8dd5a70a6c167a1730e63a5" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.72", + "synstructure 0.13.1", +] [[package]] name = "zeroize" diff --git a/ext/node/Cargo.toml b/ext/node/Cargo.toml index 3eee1a03fe..b33540b024 100644 --- a/ext/node/Cargo.toml +++ b/ext/node/Cargo.toml @@ -90,13 +90,14 @@ signature.workspace = true simd-json = "0.13.4" sm3 = "0.4.2" spki.workspace = true +stable_deref_trait = "1.2.0" thiserror.workspace = true tokio.workspace = true url.workspace = true winapi.workspace = true x25519-dalek = { version = "2.0.0", features = ["static_secrets"] } x509-parser = "0.15.0" -yoke = "0.7.4" +yoke = { version = "0.7.4", features = ["derive"] } [target.'cfg(windows)'.dependencies] windows-sys.workspace = true diff --git a/ext/node/lib.rs b/ext/node/lib.rs index 8f4adfbd18..73b4497fc7 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -311,6 +311,7 @@ deno_core::extension!(deno_node, ops::crypto::x509::op_node_x509_get_valid_to, ops::crypto::x509::op_node_x509_get_serial_number, ops::crypto::x509::op_node_x509_key_usage, + ops::crypto::x509::op_node_x509_public_key, ops::fs::op_node_fs_exists_sync

, ops::fs::op_node_fs_exists

, ops::fs::op_node_cp_sync

, diff --git a/ext/node/ops/crypto/keys.rs b/ext/node/ops/crypto/keys.rs index cebafd5842..45849cbd93 100644 --- a/ext/node/ops/crypto/keys.rs +++ b/ext/node/ops/crypto/keys.rs @@ -45,6 +45,7 @@ use spki::der::Reader as _; use spki::DecodePublicKey as _; use spki::EncodePublicKey as _; use spki::SubjectPublicKeyInfoRef; +use x509_parser::x509; use super::dh; use super::dh::DiffieHellmanGroup; @@ -518,6 +519,58 @@ impl KeyObjectHandle { Ok(KeyObjectHandle::AsymmetricPrivate(private_key)) } + pub fn new_x509_public_key( + spki: &x509::SubjectPublicKeyInfo, + ) -> Result { + use x509_parser::der_parser::asn1_rs::oid; + use x509_parser::public_key::PublicKey; + + let key = match spki.parsed()? { + PublicKey::RSA(key) => { + let public_key = RsaPublicKey::new( + rsa::BigUint::from_bytes_be(key.modulus), + rsa::BigUint::from_bytes_be(key.exponent), + )?; + AsymmetricPublicKey::Rsa(public_key) + } + PublicKey::EC(point) => { + let data = point.data(); + if let Some(params) = &spki.algorithm.parameters { + let curve_oid = params.as_oid()?; + const ID_SECP224R1: &[u8] = &oid!(raw 1.3.132.0.33); + const ID_SECP256R1: &[u8] = &oid!(raw 1.2.840.10045.3.1.7); + const ID_SECP384R1: &[u8] = &oid!(raw 1.3.132.0.34); + + match curve_oid.as_bytes() { + ID_SECP224R1 => { + let public_key = p224::PublicKey::from_sec1_bytes(data)?; + AsymmetricPublicKey::Ec(EcPublicKey::P224(public_key)) + } + ID_SECP256R1 => { + let public_key = p256::PublicKey::from_sec1_bytes(data)?; + AsymmetricPublicKey::Ec(EcPublicKey::P256(public_key)) + } + ID_SECP384R1 => { + let public_key = p384::PublicKey::from_sec1_bytes(data)?; + AsymmetricPublicKey::Ec(EcPublicKey::P384(public_key)) + } + _ => return Err(type_error("unsupported ec named curve")), + } + } else { + return Err(type_error("missing ec parameters")); + } + } + PublicKey::DSA(_) => { + let verifying_key = dsa::VerifyingKey::from_public_key_der(spki.raw) + .map_err(|_| type_error("malformed DSS public key"))?; + AsymmetricPublicKey::Dsa(verifying_key) + } + _ => return Err(type_error("unsupported x509 public key type")), + }; + + Ok(KeyObjectHandle::AsymmetricPublic(key)) + } + pub fn new_asymmetric_public_key_from_js( key: &[u8], format: &str, diff --git a/ext/node/ops/crypto/x509.rs b/ext/node/ops/crypto/x509.rs index c9a23aca09..b44ff3a4b3 100644 --- a/ext/node/ops/crypto/x509.rs +++ b/ext/node/ops/crypto/x509.rs @@ -2,7 +2,6 @@ use deno_core::error::AnyError; use deno_core::op2; -use deno_core::v8; use x509_parser::der_parser::asn1_rs::Any; use x509_parser::der_parser::asn1_rs::Tag; @@ -11,19 +10,33 @@ use x509_parser::extensions; use x509_parser::pem; use x509_parser::prelude::*; +use super::KeyObjectHandle; + +use std::ops::Deref; +use yoke::Yoke; +use yoke::Yokeable; + use digest::Digest; +enum CertificateSources { + Der(Box<[u8]>), + Pem(pem::Pem), +} + +#[derive(Yokeable)] +struct CertificateView<'a> { + cert: X509Certificate<'a>, +} + pub(crate) struct Certificate { - _buf: Vec, - pem: Option, - cert: X509Certificate<'static>, + inner: Yoke, Box>, } impl deno_core::GarbageCollected for Certificate {} impl Certificate { fn fingerprint(&self) -> Option { - self.pem.as_ref().map(|pem| { + if let CertificateSources::Pem(pem) = self.inner.backing_cart().as_ref() { let mut hasher = D::new(); hasher.update(&pem.contents); let bytes = hasher.finalize(); @@ -33,13 +46,15 @@ impl Certificate { hex.push_str(&format!("{:02X}:", byte)); } hex.pop(); - hex - }) + Some(hex) + } else { + None + } } } -impl std::ops::Deref for Certificate { - type Target = X509Certificate<'static>; +impl<'a> Deref for CertificateView<'a> { + type Target = X509Certificate<'a>; fn deref(&self) -> &Self::Target { &self.cert @@ -47,36 +62,35 @@ impl std::ops::Deref for Certificate { } #[op2] -pub fn op_node_x509_parse<'s>( - scope: &'s mut v8::HandleScope, +#[cppgc] +pub fn op_node_x509_parse( #[buffer] buf: &[u8], -) -> Result, AnyError> { - let pem = match pem::parse_x509_pem(buf) { - Ok((_, pem)) => Some(pem), - Err(_) => None, +) -> Result { + let source = match pem::parse_x509_pem(buf) { + Ok((_, pem)) => CertificateSources::Pem(pem), + Err(_) => CertificateSources::Der(buf.to_vec().into_boxed_slice()), }; - let cert = pem - .as_ref() - .map(|pem| pem.parse_x509()) - .unwrap_or_else(|| X509Certificate::from_der(buf).map(|(_, cert)| cert))?; + let inner = + Yoke::, Box>::try_attach_to_cart( + Box::new(source), + |source| { + let cert = match source { + CertificateSources::Pem(pem) => pem.parse_x509()?, + CertificateSources::Der(buf) => { + X509Certificate::from_der(buf).map(|(_, cert)| cert)? + } + }; + Ok::<_, AnyError>(CertificateView { cert }) + }, + )?; - let cert = Certificate { - _buf: buf.to_vec(), - // SAFETY: Extending the lifetime of the certificate. Backing buffer is - // owned by the resource. - cert: unsafe { - std::mem::transmute::, X509Certificate<'_>>(cert) - }, - pem, - }; - - let obj = deno_core::cppgc::make_cppgc_object(scope, cert); - Ok(obj) + Ok(Certificate { inner }) } #[op2(fast)] pub fn op_node_x509_ca(#[cppgc] cert: &Certificate) -> Result { + let cert = cert.inner.get().deref(); Ok(cert.is_ca()) } @@ -85,6 +99,7 @@ pub fn op_node_x509_check_email( #[cppgc] cert: &Certificate, #[string] email: &str, ) -> Result { + let cert = cert.inner.get().deref(); let subject = cert.subject(); if subject .iter_email() @@ -144,6 +159,7 @@ pub fn op_node_x509_fingerprint512( pub fn op_node_x509_get_issuer( #[cppgc] cert: &Certificate, ) -> Result { + let cert = cert.inner.get().deref(); Ok(x509name_to_string(cert.issuer(), oid_registry())?) } @@ -152,9 +168,21 @@ pub fn op_node_x509_get_issuer( pub fn op_node_x509_get_subject( #[cppgc] cert: &Certificate, ) -> Result { + let cert = cert.inner.get().deref(); Ok(x509name_to_string(cert.subject(), oid_registry())?) } +#[op2] +#[cppgc] +pub fn op_node_x509_public_key( + #[cppgc] cert: &Certificate, +) -> Result { + let cert = cert.inner.get().deref(); + let public_key = &cert.tbs_certificate.subject_pki; + + KeyObjectHandle::new_x509_public_key(public_key) +} + // Attempt to convert attribute to string. If type is not a string, return value is the hex // encoding of the attribute value fn attribute_value_to_string( @@ -220,6 +248,7 @@ fn x509name_to_string( pub fn op_node_x509_get_valid_from( #[cppgc] cert: &Certificate, ) -> Result { + let cert = cert.inner.get().deref(); Ok(cert.validity().not_before.to_string()) } @@ -228,6 +257,7 @@ pub fn op_node_x509_get_valid_from( pub fn op_node_x509_get_valid_to( #[cppgc] cert: &Certificate, ) -> Result { + let cert = cert.inner.get().deref(); Ok(cert.validity().not_after.to_string()) } @@ -236,6 +266,7 @@ pub fn op_node_x509_get_valid_to( pub fn op_node_x509_get_serial_number( #[cppgc] cert: &Certificate, ) -> Result { + let cert = cert.inner.get().deref(); let mut s = cert.serial.to_str_radix(16); s.make_ascii_uppercase(); Ok(s) @@ -245,6 +276,7 @@ pub fn op_node_x509_get_serial_number( pub fn op_node_x509_key_usage( #[cppgc] cert: &Certificate, ) -> Result { + let cert = cert.inner.get().deref(); let key_usage = cert .extensions() .iter() diff --git a/ext/node/polyfills/internal/crypto/x509.ts b/ext/node/polyfills/internal/crypto/x509.ts index 50a7ab48d1..699e45a51b 100644 --- a/ext/node/polyfills/internal/crypto/x509.ts +++ b/ext/node/polyfills/internal/crypto/x509.ts @@ -17,9 +17,13 @@ import { op_node_x509_get_valid_to, op_node_x509_key_usage, op_node_x509_parse, + op_node_x509_public_key, } from "ext:core/ops"; -import { KeyObject } from "ext:deno_node/internal/crypto/keys.ts"; +import { + KeyObject, + PublicKeyObject, +} from "ext:deno_node/internal/crypto/keys.ts"; import { Buffer } from "node:buffer"; import { ERR_INVALID_ARG_TYPE } from "ext:deno_node/internal/errors.ts"; import { isArrayBufferView } from "ext:deno_node/internal/util/types.ts"; @@ -144,10 +148,9 @@ export class X509Certificate { return result; } - get publicKey(): KeyObject { - notImplemented("crypto.X509Certificate.prototype.publicKey"); - - return {} as KeyObject; + get publicKey(): PublicKeyObject { + const handle = op_node_x509_public_key(this.#handle); + return new PublicKeyObject(handle); } get raw(): Buffer {