From 81b80b8e563e74317020aeed76d7360b16435c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 28 May 2026 12:40:48 +0200 Subject: [PATCH] fix(node:https,http2): accept Buffer-typed key/cert PEMs (#2132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `https.createServer({ key, cert })` and `http2.createSecureServer` parsed their options object via `JSON.stringify` → `serde_json` → `.as_str()`. Node-idiomatic callers pass PEMs as Buffers (`fs.readFileSync('key.pem')` with no encoding), which round-trip through that path as `{"type":"Buffer","data":[...]}` — losing the `.as_str()` extraction and falling through to the `"no recognized PEM private key"` branch with an empty PEM buffer. Add a shared `json_value_to_pem_bytes` helper that recognizes the Node Buffer shape and a bare numeric array, so both helpers accept either string- or Buffer-typed PEMs. Threads byte buffers (not `String`) end-to-end since PEM is opaque to the TLS parsers. This was the diff-bucket cluster surfaced by the node-core radar for the three https tests cited in #2132 (agent-abort-controller, agent-keylog, byteswritten); after the fix, agent-abort-controller flips to pass, the other two move past the PEM error to surface unrelated downstream gaps (cert version, console output). Includes unit tests covering string PEM, Buffer-shape JSON, bare numeric array, and the empty/unknown fallbacks. --- .../perry-ext-http-server/src/http2_server.rs | 33 ++++----- .../perry-ext-http-server/src/https_server.rs | 38 +++++----- crates/perry-ext-http-server/src/tls.rs | 71 +++++++++++++++++++ 3 files changed, 102 insertions(+), 40 deletions(-) diff --git a/crates/perry-ext-http-server/src/http2_server.rs b/crates/perry-ext-http-server/src/http2_server.rs index 1b3c2affb4..4e2a678f27 100644 --- a/crates/perry-ext-http-server/src/http2_server.rs +++ b/crates/perry-ext-http-server/src/http2_server.rs @@ -38,35 +38,30 @@ use crate::request::{ }; use crate::response::{alloc_server_response, HyperResponseShape, ResponseBody}; use crate::server::{synthesize_default_response_if_needed, HttpPendingRequest, HttpServer}; -use crate::tls::{build_server_config, parse_cert_chain, parse_private_key}; +use crate::tls::{ + build_server_config, json_value_to_pem_bytes, parse_cert_chain, parse_private_key, +}; /// Decode `{ key, cert }` from a NaN-boxed JsValue object. Mirrors -/// the helper in `https_server.rs` but omits the alpnProtocols flag -/// since http2 server always advertises `[h2, http/1.1]`. -unsafe fn parse_h2_opts(opts_f64: f64) -> (String, String) { +/// the helper in `https_server.rs` (including Buffer-typed PEM +/// support, #2132) but omits the alpnProtocols flag since http2 +/// server always advertises `[h2, http/1.1]`. +unsafe fn parse_h2_opts(opts_f64: f64) -> (Vec, Vec) { use perry_ffi::JsValue; let v = JsValue::from_bits(opts_f64.to_bits()); if !v.is_pointer() { - return (String::new(), String::new()); + return (Vec::new(), Vec::new()); } let json = match perry_ffi::json_stringify(v) { Some(j) => j, - None => return (String::new(), String::new()), + None => return (Vec::new(), Vec::new()), }; let parsed: serde_json::Value = match serde_json::from_str(&json) { Ok(p) => p, - Err(_) => return (String::new(), String::new()), + Err(_) => return (Vec::new(), Vec::new()), }; - let key_pem = parsed - .get("key") - .and_then(|k| k.as_str()) - .unwrap_or_default() - .to_string(); - let cert_pem = parsed - .get("cert") - .and_then(|c| c.as_str()) - .unwrap_or_default() - .to_string(); + let key_pem = json_value_to_pem_bytes(parsed.get("key")); + let cert_pem = json_value_to_pem_bytes(parsed.get("cert")); (key_pem, cert_pem) } use crate::types::{ @@ -90,8 +85,8 @@ pub unsafe extern "C" fn js_node_http2_create_secure_server(opts_f64: f64, handl ensure_gc_scanner_registered(); let (key_pem, cert_pem) = parse_h2_opts(opts_f64); - let cert_chain = parse_cert_chain(cert_pem.as_bytes()); - let private_key = parse_private_key(key_pem.as_bytes()); + let cert_chain = parse_cert_chain(&cert_pem); + let private_key = parse_private_key(&key_pem); let tls_config = match private_key { Some(k) => match build_server_config(cert_chain, k, true) { diff --git a/crates/perry-ext-http-server/src/https_server.rs b/crates/perry-ext-http-server/src/https_server.rs index e3fd023a57..06ee6012fc 100644 --- a/crates/perry-ext-http-server/src/https_server.rs +++ b/crates/perry-ext-http-server/src/https_server.rs @@ -28,37 +28,33 @@ use crate::request::{ }; use crate::response::{alloc_server_response, HyperResponseShape, ResponseBody}; use crate::server::{HttpPendingRequest, HttpServer}; -use crate::tls::{build_server_config, parse_cert_chain, parse_private_key}; +use crate::tls::{ + build_server_config, json_value_to_pem_bytes, parse_cert_chain, parse_private_key, +}; /// Decode `{ key, cert, alpnProtocols? }` from a NaN-boxed JsValue -/// object literal into Rust strings + a flag for whether to advertise -/// `h2` in ALPN. Falls back to empty PEMs (which the cert-chain -/// parser then rejects) on any extraction failure so the user sees -/// a clear bind error. -unsafe fn parse_https_opts(opts_f64: f64) -> (String, String, bool) { +/// object literal into the PEM byte buffers + a flag for whether to +/// advertise `h2` in ALPN. `key`/`cert` accept either a PEM string +/// OR a `Buffer` (the form `fs.readFileSync('key.pem')` returns when +/// no encoding is supplied) — see `json_value_to_pem_bytes`. Falls +/// back to empty PEMs (which the cert-chain parser then rejects) on +/// any extraction failure so the user sees a clear bind error. +unsafe fn parse_https_opts(opts_f64: f64) -> (Vec, Vec, bool) { use perry_ffi::JsValue; let v = JsValue::from_bits(opts_f64.to_bits()); if !v.is_pointer() { - return (String::new(), String::new(), true); + return (Vec::new(), Vec::new(), true); } let json = match perry_ffi::json_stringify(v) { Some(j) => j, - None => return (String::new(), String::new(), true), + None => return (Vec::new(), Vec::new(), true), }; let parsed: serde_json::Value = match serde_json::from_str(&json) { Ok(p) => p, - Err(_) => return (String::new(), String::new(), true), + Err(_) => return (Vec::new(), Vec::new(), true), }; - let key_pem = parsed - .get("key") - .and_then(|k| k.as_str()) - .unwrap_or_default() - .to_string(); - let cert_pem = parsed - .get("cert") - .and_then(|c| c.as_str()) - .unwrap_or_default() - .to_string(); + let key_pem = json_value_to_pem_bytes(parsed.get("key")); + let cert_pem = json_value_to_pem_bytes(parsed.get("cert")); // Default ALPN to `[http/1.1]` only — node:https is HTTP/1.1 // by spec; users wanting HTTP/2 should reach for node:http2's // createSecureServer instead. Opt-in via `alpnProtocols: ["h2", "http/1.1"]`. @@ -91,8 +87,8 @@ pub unsafe extern "C" fn js_node_https_create_server(opts_f64: f64, handler: i64 let (key_pem, cert_pem, enable_http2_alpn) = parse_https_opts(opts_f64); - let cert_chain = parse_cert_chain(cert_pem.as_bytes()); - let private_key = match parse_private_key(key_pem.as_bytes()) { + let cert_chain = parse_cert_chain(&cert_pem); + let private_key = match parse_private_key(&key_pem) { Some(k) => k, None => { eprintln!("[node:https] no recognized PEM private key"); diff --git a/crates/perry-ext-http-server/src/tls.rs b/crates/perry-ext-http-server/src/tls.rs index 343ef17b27..e121978a69 100644 --- a/crates/perry-ext-http-server/src/tls.rs +++ b/crates/perry-ext-http-server/src/tls.rs @@ -7,6 +7,42 @@ use std::sync::Arc; use rustls::pki_types::{CertificateDer, PrivateKeyDer}; use rustls::ServerConfig; +/// Decode a `key`/`cert`-shaped JSON value into the PEM bytes the +/// rustls parsers expect. +/// +/// Node lets users pass either a PEM string OR a `Buffer` (the form +/// `fs.readFileSync('key.pem')` returns when no encoding is supplied). +/// `https.createServer` / `http2.createSecureServer` decode their +/// options object via `JSON.stringify` → `serde_json`, which +/// round-trips a `Buffer` as `{ "type": "Buffer", "data": [..] }`. +/// Without this helper, the `.as_str()` extraction silently yielded +/// an empty string for Buffer-typed PEMs and the user saw a +/// `"no recognized PEM private key"` error even with valid input +/// (#2132). +pub fn json_value_to_pem_bytes(v: Option<&serde_json::Value>) -> Vec { + let Some(v) = v else { return Vec::new() }; + if let Some(s) = v.as_str() { + return s.as_bytes().to_vec(); + } + if let Some(obj) = v.as_object() { + if obj.get("type").and_then(|t| t.as_str()) == Some("Buffer") { + if let Some(arr) = obj.get("data").and_then(|d| d.as_array()) { + return arr + .iter() + .filter_map(|n| n.as_u64().map(|u| u as u8)) + .collect(); + } + } + } + if let Some(arr) = v.as_array() { + return arr + .iter() + .filter_map(|n| n.as_u64().map(|u| u as u8)) + .collect(); + } + Vec::new() +} + /// Parse PEM-encoded certificate chain bytes into rustls /// `CertificateDer`s. Returns an empty vec on parse failure (caller /// must check for emptiness before building a ServerConfig — empty @@ -56,6 +92,41 @@ fn ensure_crypto_provider_installed() { }); } +#[cfg(test)] +mod tests { + use super::json_value_to_pem_bytes; + use serde_json::json; + + #[test] + fn string_value_returns_utf8_bytes() { + let v = json!("-----BEGIN RSA PRIVATE KEY-----\n"); + assert_eq!( + json_value_to_pem_bytes(Some(&v)), + b"-----BEGIN RSA PRIVATE KEY-----\n" + ); + } + + #[test] + fn node_buffer_shape_returns_data_bytes() { + // `JSON.stringify(Buffer.from("hi"))` → `{"type":"Buffer","data":[104,105]}`. + let v = json!({"type":"Buffer","data":[104,105]}); + assert_eq!(json_value_to_pem_bytes(Some(&v)), b"hi"); + } + + #[test] + fn plain_numeric_array_returns_bytes() { + let v = json!([104, 105]); + assert_eq!(json_value_to_pem_bytes(Some(&v)), b"hi"); + } + + #[test] + fn none_and_unknown_shapes_return_empty() { + assert!(json_value_to_pem_bytes(None).is_empty()); + assert!(json_value_to_pem_bytes(Some(&json!(42))).is_empty()); + assert!(json_value_to_pem_bytes(Some(&json!({"foo": "bar"}))).is_empty()); + } +} + /// Build a rustls `ServerConfig` ready for `tokio_rustls::TlsAcceptor`. /// `alpn_protocols` is set to `[h2, http/1.1]` so an HTTP/2-aware /// negotiator can pick the upgraded transport on the same port —