refactor(ext/tls): use cppgc to deduplicate the tls key loading code (#23289)

Pass the certificates and key files as CPPGC objects.

Towards #23233

ext/fetch/22_http_client.js

@@ -14,6 +14,7 @@ import { core, primordials } from "ext:core/mod.js";
 
 import { SymbolDispose } from "ext:deno_web/00_infra.js";
 import { op_fetch_custom_client } from "ext:core/ops";
+import { loadTlsKeyPair } from "ext:deno_net/02_tls.js";
 
 const { internalRidSymbol } = core;
 const { ObjectDefineProperty } = primordials;
@@ -24,9 +25,16 @@ const { ObjectDefineProperty } = primordials;
  */
 function createHttpClient(options) {
   options.caCerts ??= [];
+  const keyPair = loadTlsKeyPair(
+    options.cert,
+    undefined,
+    options.key,
+    undefined,
+  );
   return new HttpClient(
     op_fetch_custom_client(
       options,
+      keyPair,
     ),
   );
 }

ext/fetch/lib.rs

@@ -44,6 +44,8 @@ use deno_tls::Proxy;
 use deno_tls::RootCertStoreProvider;
 
 use data_url::DataUrl;
+use deno_tls::TlsKey;
+use deno_tls::TlsKeys;
 use http_v02::header::CONTENT_LENGTH;
 use http_v02::Uri;
 use reqwest::header::HeaderMap;
@@ -78,7 +80,7 @@ pub struct Options {
   pub request_builder_hook:
     Option<fn(RequestBuilder) -> Result<RequestBuilder, AnyError>>,
   pub unsafely_ignore_certificate_errors: Option<Vec<String>>,
-  pub client_cert_chain_and_key: Option<(String, String)>,
+  pub client_cert_chain_and_key: Option<TlsKey>,
   pub file_fetch_handler: Rc<dyn FetchHandler>,
 }
 
@@ -794,8 +796,6 @@ impl HttpClientResource {
 pub struct CreateHttpClientArgs {
   ca_certs: Vec<String>,
   proxy: Option<Proxy>,
-  cert: Option<String>,
-  key: Option<String>,
   pool_max_idle_per_host: Option<usize>,
   pool_idle_timeout: Option<serde_json::Value>,
   #[serde(default = "default_true")]
@@ -815,6 +815,7 @@ fn default_true() -> bool {
 pub fn op_fetch_custom_client<FP>(
   state: &mut OpState,
   #[serde] args: CreateHttpClientArgs,
+  #[cppgc] tls_keys: &deno_tls::TlsKeys,
 ) -> Result<ResourceId, AnyError>
 where
   FP: FetchPermissions + 'static,
@@ -825,19 +826,9 @@ where
     permissions.check_net_url(&url, "Deno.createHttpClient()")?;
   }
 
-  let client_cert_chain_and_key = {
+  let client_cert_chain_and_key = match tls_keys {
-    if args.cert.is_some() || args.key.is_some() {
+    TlsKeys::Null => None,
-      let cert_chain = args
+    TlsKeys::Static(key) => Some(key.clone()),
-        .cert
-        .ok_or_else(|| type_error("No certificate chain provided"))?;
-      let private_key = args
-        .key
-        .ok_or_else(|| type_error("No private key provided"))?;
-
-      Some((cert_chain, private_key))
-    } else {
-      None
-    }
   };
 
   let options = state.borrow::<Options>();
@@ -885,7 +876,7 @@ pub struct CreateHttpClientOptions {
   pub ca_certs: Vec<Vec<u8>>,
   pub proxy: Option<Proxy>,
   pub unsafely_ignore_certificate_errors: Option<Vec<String>>,
-  pub client_cert_chain_and_key: Option<(String, String)>,
+  pub client_cert_chain_and_key: Option<TlsKey>,
   pub pool_max_idle_per_host: Option<usize>,
   pub pool_idle_timeout: Option<Option<u64>>,
   pub http1: bool,

ext/http/00_serve.js

@@ -70,7 +70,7 @@ import {
   resourceForReadableStream,
 } from "ext:deno_web/06_streams.js";
 import { listen, listenOptionApiName, TcpConn } from "ext:deno_net/01_net.js";
-import { listenTls } from "ext:deno_net/02_tls.js";
+import { hasTlsKeyPairOptions, listenTls } from "ext:deno_net/02_tls.js";
 import { SymbolAsyncDispose } from "ext:deno_web/00_infra.js";
 
 const _upgraded = Symbol("_upgraded");
@@ -535,7 +535,7 @@ function serve(arg1, arg2) {
     options = {};
   }
 
-  const wantsHttps = options.cert || options.key;
+  const wantsHttps = hasTlsKeyPairOptions(options);
   const wantsUnix = ObjectHasOwn(options, "path");
   const signal = options.signal;
   const onError = options.onError ?? function (error) {

ext/kv/remote.rs

@@ -16,6 +16,7 @@ use deno_fetch::CreateHttpClientOptions;
 use deno_tls::rustls::RootCertStore;
 use deno_tls::Proxy;
 use deno_tls::RootCertStoreProvider;
+use deno_tls::TlsKey;
 use denokv_remote::MetadataEndpoint;
 use denokv_remote::Remote;
 use url::Url;
@@ -26,7 +27,7 @@ pub struct HttpOptions {
   pub root_cert_store_provider: Option<Arc<dyn RootCertStoreProvider>>,
   pub proxy: Option<Proxy>,
   pub unsafely_ignore_certificate_errors: Option<Vec<String>>,
-  pub client_cert_chain_and_key: Option<(String, String)>,
+  pub client_cert_chain_and_key: Option<TlsKey>,
 }
 
 impl HttpOptions {

ext/net/02_tls.js

@@ -7,11 +7,15 @@ import {
   op_net_connect_tls,
   op_net_listen_tls,
   op_tls_handshake,
+  op_tls_key_null,
+  op_tls_key_static,
+  op_tls_key_static_from_file,
   op_tls_start,
 } from "ext:core/ops";
 const {
   Number,
   ObjectDefineProperty,
+  ReflectHas,
   TypeError,
 } = primordials;
 
@@ -91,9 +95,11 @@ async function connectTls({
   }
   cert ??= certChain;
   key ??= privateKey;
+  const keyPair = loadTlsKeyPair(cert, undefined, key, undefined);
   const { 0: rid, 1: localAddr, 2: remoteAddr } = await op_net_connect_tls(
     { hostname, port },
     { certFile, caCerts, cert, key, alpnProtocols },
+    keyPair,
   );
   localAddr.transport = "tcp";
   remoteAddr.transport = "tcp";
@@ -131,6 +137,36 @@ class TlsListener extends Listener {
   }
 }
 
+function hasTlsKeyPairOptions(options) {
+  return (ReflectHas(options, "cert") || ReflectHas(options, "key") ||
+    ReflectHas(options, "certFile") ||
+    ReflectHas(options, "keyFile"));
+}
+
+function loadTlsKeyPair(
+  cert,
+  certFile,
+  key,
+  keyFile,
+) {
+  if ((certFile !== undefined) ^ (keyFile !== undefined)) {
+    throw new TypeError(
+      "If certFile is specified, keyFile must also be specified",
+    );
+  }
+  if ((cert !== undefined) ^ (key !== undefined)) {
+    throw new TypeError("If cert is specified, key must also be specified");
+  }
+
+  if (certFile !== undefined) {
+    return op_tls_key_static_from_file("Deno.listenTls", certFile, keyFile);
+  } else if (cert !== undefined) {
+    return op_tls_key_static(cert, key);
+  } else {
+    return op_tls_key_null();
+  }
+}
+
 function listenTls({
   port,
   cert,
@@ -159,9 +195,12 @@ function listenTls({
       "Pass the cert file contents to the `Deno.ListenTlsOptions.cert` option instead.",
     );
   }
+
+  const keyPair = loadTlsKeyPair(cert, certFile, key, keyFile);
   const { 0: rid, 1: localAddr } = op_net_listen_tls(
     { hostname, port: Number(port) },
-    { cert, certFile, key, keyFile, alpnProtocols, reusePort },
+    { alpnProtocols, reusePort },
+    keyPair,
   );
   return new TlsListener(rid, localAddr);
 }
@@ -184,4 +223,12 @@ async function startTls(
   return new TlsConn(rid, remoteAddr, localAddr);
 }
 
-export { connectTls, listenTls, startTls, TlsConn, TlsListener };
+export {
+  connectTls,
+  hasTlsKeyPairOptions,
+  listenTls,
+  loadTlsKeyPair,
+  startTls,
+  TlsConn,
+  TlsListener,
+};

ext/net/lib.rs

@@ -84,6 +84,9 @@ deno_core::extension!(deno_net,
     ops::op_set_nodelay,
     ops::op_set_keepalive,
 
+    ops_tls::op_tls_key_null,
+    ops_tls::op_tls_key_static,
+    ops_tls::op_tls_key_static_from_file<P>,
     ops_tls::op_tls_start<P>,
     ops_tls::op_net_connect_tls<P>,
     ops_tls::op_net_listen_tls<P>,

ext/net/ops_tls.rs

@@ -12,9 +12,9 @@ use deno_core::error::bad_resource;
 use deno_core::error::custom_error;
 use deno_core::error::generic_error;
 use deno_core::error::invalid_hostname;
-use deno_core::error::type_error;
 use deno_core::error::AnyError;
 use deno_core::op2;
+use deno_core::v8;
 use deno_core::AsyncRefCell;
 use deno_core::AsyncResult;
 use deno_core::CancelHandle;
@@ -31,7 +31,8 @@ use deno_tls::rustls::PrivateKey;
 use deno_tls::rustls::ServerConfig;
 use deno_tls::rustls::ServerName;
 use deno_tls::SocketUse;
-use io::Read;
+use deno_tls::TlsKey;
+use deno_tls::TlsKeys;
 use rustls_tokio_stream::TlsStreamRead;
 use rustls_tokio_stream::TlsStreamWrite;
 use serde::Deserialize;
@@ -43,9 +44,9 @@ use std::cell::RefCell;
 use std::convert::From;
 use std::convert::TryFrom;
 use std::fs::File;
-use std::io;
 use std::io::BufReader;
 use std::io::ErrorKind;
+use std::io::Read;
 use std::num::NonZeroUsize;
 use std::path::Path;
 use std::rc::Rc;
@@ -145,8 +146,6 @@ impl Resource for TlsStreamResource {
 pub struct ConnectTlsArgs {
   cert_file: Option<String>,
   ca_certs: Vec<String>,
-  cert: Option<String>,
-  key: Option<String>,
   alpn_protocols: Option<Vec<String>>,
 }
 
@@ -159,6 +158,59 @@ pub struct StartTlsArgs {
   alpn_protocols: Option<Vec<String>>,
 }
 
+#[op2]
+pub fn op_tls_key_null<'s>(
+  scope: &mut v8::HandleScope<'s>,
+) -> Result<v8::Local<'s, v8::Object>, AnyError> {
+  Ok(deno_core::cppgc::make_cppgc_object(scope, TlsKeys::Null))
+}
+
+#[op2]
+pub fn op_tls_key_static<'s>(
+  scope: &mut v8::HandleScope<'s>,
+  #[string] cert: String,
+  #[string] key: String,
+) -> Result<v8::Local<'s, v8::Object>, AnyError> {
+  let cert = load_certs(&mut BufReader::new(cert.as_bytes()))?;
+  let key = load_private_keys(key.as_bytes())?
+    .into_iter()
+    .next()
+    .unwrap();
+  Ok(deno_core::cppgc::make_cppgc_object(
+    scope,
+    TlsKeys::Static(TlsKey(cert, key)),
+  ))
+}
+
+/// Legacy op -- will be removed in Deno 2.0.
+#[op2]
+pub fn op_tls_key_static_from_file<'s, NP>(
+  state: &mut OpState,
+  scope: &mut v8::HandleScope<'s>,
+  #[string] api: String,
+  #[string] cert_file: String,
+  #[string] key_file: String,
+) -> Result<v8::Local<'s, v8::Object>, AnyError>
+where
+  NP: NetPermissions + 'static,
+{
+  {
+    let permissions = state.borrow_mut::<NP>();
+    permissions.check_read(Path::new(&cert_file), &api)?;
+    permissions.check_read(Path::new(&key_file), &api)?;
+  }
+
+  let cert = load_certs_from_file(&cert_file)?;
+  let key = load_private_keys_from_file(&key_file)?
+    .into_iter()
+    .next()
+    .unwrap();
+  Ok(deno_core::cppgc::make_cppgc_object(
+    scope,
+    TlsKeys::Static(TlsKey(cert, key)),
+  ))
+}
+
 #[op2]
 #[serde]
 pub fn op_tls_start<NP>(
@@ -251,6 +303,7 @@ pub async fn op_net_connect_tls<NP>(
   state: Rc<RefCell<OpState>>,
   #[serde] addr: IpAddr,
   #[serde] args: ConnectTlsArgs,
+  #[cppgc] key_pair: &TlsKeys,
 ) -> Result<(ResourceId, IpAddr, IpAddr), AnyError>
 where
   NP: NetPermissions + 'static,
@@ -297,18 +350,10 @@ where
   let local_addr = tcp_stream.local_addr()?;
   let remote_addr = tcp_stream.peer_addr()?;
 
-  let cert_and_key = if args.cert.is_some() || args.key.is_some() {
+  let cert_and_key = match key_pair {
-    let cert = args
+    TlsKeys::Null => None,
-      .cert
+    TlsKeys::Static(key) => Some(key.clone()),
-      .ok_or_else(|| type_error("No certificate chain provided"))?;
-    let key = args
-      .key
-      .ok_or_else(|| type_error("No private key provided"))?;
-    Some((cert, key))
-  } else {
-    None
   };
-
   let mut tls_config = create_client_config(
     root_cert_store,
     ca_certs,
@@ -373,12 +418,6 @@ impl Resource for TlsListenerResource {
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct ListenTlsArgs {
-  cert: Option<String>,
-  // TODO(kt3k): Remove this option at v2.0.
-  cert_file: Option<String>,
-  key: Option<String>,
-  // TODO(kt3k): Remove this option at v2.0.
-  key_file: Option<String>,
   alpn_protocols: Option<Vec<String>>,
   reuse_port: bool,
 }
@@ -389,6 +428,7 @@ pub fn op_net_listen_tls<NP>(
   state: &mut OpState,
   #[serde] addr: IpAddr,
   #[serde] args: ListenTlsArgs,
+  #[cppgc] keys: &TlsKeys,
 ) -> Result<(ResourceId, IpAddr), AnyError>
 where
   NP: NetPermissions + 'static,
@@ -397,48 +437,24 @@ where
     super::check_unstable(state, "Deno.listenTls({ reusePort: true })");
   }
 
-  let cert_file = args.cert_file.as_deref();
-  let key_file = args.key_file.as_deref();
-  let cert = args.cert.as_deref();
-  let key = args.key.as_deref();
-
   {
     let permissions = state.borrow_mut::<NP>();
     permissions
       .check_net(&(&addr.hostname, Some(addr.port)), "Deno.listenTls()")?;
-    if let Some(path) = cert_file {
-      permissions.check_read(Path::new(path), "Deno.listenTls()")?;
-    }
-    if let Some(path) = key_file {
-      permissions.check_read(Path::new(path), "Deno.listenTls()")?;
-    }
   }
 
-  let cert_chain = if cert_file.is_some() && cert.is_some() {
+  let tls_config = ServerConfig::builder()
-    return Err(generic_error("Both cert and certFile is specified. You can specify either one of them."));
-  } else if let Some(path) = cert_file {
-    load_certs_from_file(path)?
-  } else if let Some(cert) = cert {
-    load_certs(&mut BufReader::new(cert.as_bytes()))?
-  } else {
-    return Err(generic_error("`cert` is not specified."));
-  };
-  let key_der = if key_file.is_some() && key.is_some() {
-    return Err(generic_error(
-      "Both key and keyFile is specified. You can specify either one of them.",
-    ));
-  } else if let Some(path) = key_file {
-    load_private_keys_from_file(path)?.remove(0)
-  } else if let Some(key) = key {
-    load_private_keys(key.as_bytes())?.remove(0)
-  } else {
-    return Err(generic_error("`key` is not specified."));
-  };
-
-  let mut tls_config = ServerConfig::builder()
     .with_safe_defaults()
-    .with_no_client_auth()
+    .with_no_client_auth();
-    .with_single_cert(cert_chain, key_der)
+
+  let mut tls_config = match keys {
+    TlsKeys::Null => {
+      unreachable!()
+    }
+    TlsKeys::Static(TlsKey(cert, key)) => {
+      tls_config.with_single_cert(cert.clone(), key.clone())
+    }
+  }
   .map_err(|e| {
     custom_error(
       "InvalidData",

ext/tls/lib.rs

@@ -174,19 +174,9 @@ pub fn create_client_config(
   root_cert_store: Option<RootCertStore>,
   ca_certs: Vec<Vec<u8>>,
   unsafely_ignore_certificate_errors: Option<Vec<String>>,
-  client_cert_chain_and_key: Option<(String, String)>,
+  maybe_cert_chain_and_key: Option<TlsKey>,
   socket_use: SocketUse,
 ) -> Result<ClientConfig, AnyError> {
-  let maybe_cert_chain_and_key =
-    if let Some((cert_chain, private_key)) = client_cert_chain_and_key {
-      // The `remove` is safe because load_private_keys checks that there is at least one key.
-      let private_key = load_private_keys(private_key.as_bytes())?.remove(0);
-      let cert_chain = load_certs(&mut cert_chain.as_bytes())?;
-      Some((cert_chain, private_key))
-    } else {
-      None
-    };
-
   if let Some(ic_allowlist) = unsafely_ignore_certificate_errors {
     let client_config = ClientConfig::builder()
       .with_safe_defaults()
@@ -199,7 +189,7 @@ pub fn create_client_config(
     // are not type-compatible - one wants "client cert", the other wants "transparency policy
     // or client cert".
     let mut client =
-      if let Some((cert_chain, private_key)) = maybe_cert_chain_and_key {
+      if let Some(TlsKey(cert_chain, private_key)) = maybe_cert_chain_and_key {
         client_config
           .with_client_auth_cert(cert_chain, private_key)
           .expect("invalid client key or certificate")
@@ -236,7 +226,7 @@ pub fn create_client_config(
     });
 
   let mut client =
-    if let Some((cert_chain, private_key)) = maybe_cert_chain_and_key {
+    if let Some(TlsKey(cert_chain, private_key)) = maybe_cert_chain_and_key {
       client_config
         .with_client_auth_cert(cert_chain, private_key)
         .expect("invalid client key or certificate")
@@ -270,8 +260,7 @@ pub fn load_certs(
     .map_err(|_| custom_error("InvalidData", "Unable to decode certificate"))?;
 
   if certs.is_empty() {
-    let e = custom_error("InvalidData", "No certificates found in cert file");
+    return Err(cert_not_found_err());
-    return Err(e);
   }
 
   Ok(certs.into_iter().map(Certificate).collect())
@@ -282,7 +271,11 @@ fn key_decode_err() -> AnyError {
 }
 
 fn key_not_found_err() -> AnyError {
-  custom_error("InvalidData", "No keys found in key file")
+  custom_error("InvalidData", "No keys found in key data")
+}
+
+fn cert_not_found_err() -> AnyError {
+  custom_error("InvalidData", "No certificates found in certificate data")
 }
 
 /// Starts with -----BEGIN RSA PRIVATE KEY-----
@@ -331,3 +324,15 @@ pub fn load_private_keys(bytes: &[u8]) -> Result<Vec<PrivateKey>, AnyError> {
 
   Ok(keys)
 }
+
+/// A loaded key.
+// FUTURE(mmastrac): add resolver enum value to support dynamic SNI
+pub enum TlsKeys {
+  // TODO(mmastrac): We need Option<&T> for cppgc -- this is a workaround
+  Null,
+  Static(TlsKey),
+}
+
+/// A TLS certificate/private key pair.
+#[derive(Clone, Debug)]
+pub struct TlsKey(pub Vec<Certificate>, pub PrivateKey);