From 69ec45eac76c63ea973c68479ea4f0bbf58b29e9 Mon Sep 17 00:00:00 2001 From: Andreu Botella Date: Tue, 17 Jan 2023 16:18:24 -0800 Subject: [PATCH] refactor(cli): Integrate standalone mode cert handling into `Flags` (#17419) The way the standalone mode handles the `--cert` flag is different to all other modes. This is because `--cert` takes a path to the certificate file, which is directly added to the root cert store; except for compile mode, where its byte contents are stored in the standalone metadata, and they are added to the root cert store after the `ProcState` is created. This change instead changes `Flags::ca_file` (an `Option`) into `Flags::ca_data`, which can represent a `String` file path or a `Vec` with the certificate contents. That way, standalone mode can create a `ProcState` whose root cert store alreay contains the certificate. This change also adds a tests for certificates in standalone mode, since there weren't any before. This refactor will help with implementing web workers in standalone mode in the future. --- cli/args/flags.rs | 34 +++++++++++++++++--------- cli/args/mod.rs | 37 +++++++++++++++++++---------- cli/lsp/language_server.rs | 5 ++-- cli/standalone.rs | 23 +++--------------- cli/tests/integration/cert_tests.rs | 34 ++++++++++++++++++++++++++ cli/tools/installer.rs | 3 ++- cli/tools/standalone.rs | 6 +++-- 7 files changed, 93 insertions(+), 49 deletions(-) diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 903f936390..c6c922bd6f 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -291,6 +291,15 @@ impl Default for ConfigFlag { } } +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum CaData { + /// The string is a file path + File(String), + /// This variant is not exposed as an option in the CLI, it is used internally + /// for standalone binaries. + Bytes(Vec), +} + #[derive(Clone, Debug, Eq, PartialEq, Default)] pub struct Flags { /// Vector of CLI arguments - these are user script arguments, all Deno @@ -308,7 +317,7 @@ pub struct Flags { pub allow_sys: Option>, pub allow_write: Option>, pub ca_stores: Option>, - pub ca_file: Option, + pub ca_data: Option, pub cache_blocklist: Vec, /// This is not exposed as an option in the CLI, it is used internally when /// the language server is configured with an explicit cache option. @@ -3091,7 +3100,10 @@ fn reload_arg_parse(flags: &mut Flags, matches: &ArgMatches) { } fn ca_file_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) { - flags.ca_file = matches.value_of("cert").map(ToOwned::to_owned); + flags.ca_data = matches + .value_of("cert") + .map(ToOwned::to_owned) + .map(CaData::File); } fn enable_testing_features_arg_parse( @@ -4276,7 +4288,7 @@ mod tests { reload: true, lock: Some(PathBuf::from("lock.json")), lock_write: true, - ca_file: Some("example.crt".to_string()), + ca_data: Some(CaData::File("example.crt".to_string())), cached_only: true, location: Some(Url::parse("https://foo/").unwrap()), v8_flags: svec!["--help", "--random-seed=1"], @@ -4370,7 +4382,7 @@ mod tests { reload: true, lock: Some(PathBuf::from("lock.json")), lock_write: true, - ca_file: Some("example.crt".to_string()), + ca_data: Some(CaData::File("example.crt".to_string())), cached_only: true, location: Some(Url::parse("https://foo/").unwrap()), v8_flags: svec!["--help", "--random-seed=1"], @@ -5036,7 +5048,7 @@ mod tests { reload: true, lock: Some(PathBuf::from("lock.json")), lock_write: true, - ca_file: Some("example.crt".to_string()), + ca_data: Some(CaData::File("example.crt".to_string())), cached_only: true, v8_flags: svec!["--help", "--random-seed=1"], seed: Some(1), @@ -5608,7 +5620,7 @@ mod tests { subcommand: DenoSubcommand::Run(RunFlags { script: "script.ts".to_string(), }), - ca_file: Some("example.crt".to_owned()), + ca_data: Some(CaData::File("example.crt".to_owned())), ..Flags::default() } ); @@ -5856,7 +5868,7 @@ mod tests { out_file: None, }), type_check_mode: TypeCheckMode::Local, - ca_file: Some("example.crt".to_owned()), + ca_data: Some(CaData::File("example.crt".to_owned())), ..Flags::default() } ); @@ -5875,7 +5887,7 @@ mod tests { version: None, output: None, }), - ca_file: Some("example.crt".to_owned()), + ca_data: Some(CaData::File("example.crt".to_owned())), ..Flags::default() } ); @@ -5897,7 +5909,7 @@ mod tests { subcommand: DenoSubcommand::Cache(CacheFlags { files: svec!["script.ts", "script_two.ts"], }), - ca_file: Some("example.crt".to_owned()), + ca_data: Some(CaData::File("example.crt".to_owned())), ..Flags::default() } ); @@ -5919,7 +5931,7 @@ mod tests { json: false, file: Some("https://example.com".to_string()), }), - ca_file: Some("example.crt".to_owned()), + ca_data: Some(CaData::File("example.crt".to_owned())), ..Flags::default() } ); @@ -6093,7 +6105,7 @@ mod tests { reload: true, lock: Some(PathBuf::from("lock.json")), lock_write: true, - ca_file: Some("example.crt".to_string()), + ca_data: Some(CaData::File("example.crt".to_string())), cached_only: true, location: Some(Url::parse("https://foo/").unwrap()), allow_read: Some(vec![]), diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 0f60d09c37..b604d3ab5c 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -42,6 +42,7 @@ use deno_runtime::permissions::PermissionsOptions; use std::collections::BTreeMap; use std::env; use std::io::BufReader; +use std::io::Cursor; use std::net::SocketAddr; use std::num::NonZeroUsize; use std::path::PathBuf; @@ -370,7 +371,7 @@ fn resolve_lint_rules_options( pub fn get_root_cert_store( maybe_root_path: Option, maybe_ca_stores: Option>, - maybe_ca_file: Option, + maybe_ca_data: Option, ) -> Result { let mut root_cert_store = RootCertStore::empty(); let ca_stores: Vec = maybe_ca_stores @@ -413,17 +414,27 @@ pub fn get_root_cert_store( } } - let ca_file = maybe_ca_file.or_else(|| env::var("DENO_CERT").ok()); - if let Some(ca_file) = ca_file { - let ca_file = if let Some(root) = &maybe_root_path { - root.join(&ca_file) - } else { - PathBuf::from(ca_file) + let ca_data = + maybe_ca_data.or_else(|| env::var("DENO_CERT").ok().map(CaData::File)); + if let Some(ca_data) = ca_data { + let result = match ca_data { + CaData::File(ca_file) => { + let ca_file = if let Some(root) = &maybe_root_path { + root.join(&ca_file) + } else { + PathBuf::from(ca_file) + }; + let certfile = std::fs::File::open(ca_file)?; + let mut reader = BufReader::new(certfile); + rustls_pemfile::certs(&mut reader) + } + CaData::Bytes(data) => { + let mut reader = BufReader::new(Cursor::new(data)); + rustls_pemfile::certs(&mut reader) + } }; - let certfile = std::fs::File::open(ca_file)?; - let mut reader = BufReader::new(certfile); - match rustls_pemfile::certs(&mut reader) { + match result { Ok(certs) => { root_cert_store.add_parsable_certificates(&certs); } @@ -576,7 +587,7 @@ impl CliOptions { get_root_cert_store( None, self.flags.ca_stores.clone(), - self.flags.ca_file.clone(), + self.flags.ca_data.clone(), ) } @@ -722,8 +733,8 @@ impl CliOptions { &self.flags.argv } - pub fn ca_file(&self) -> &Option { - &self.flags.ca_file + pub fn ca_data(&self) -> &Option { + &self.flags.ca_data } pub fn ca_stores(&self) -> &Option> { diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index ec31fea4c7..b90f613e9f 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -58,6 +58,7 @@ use super::tsc::AssetsSnapshot; use super::tsc::TsServer; use super::urls; use crate::args::get_root_cert_store; +use crate::args::CaData; use crate::args::CacheSetting; use crate::args::CliOptions; use crate::args::ConfigFile; @@ -579,7 +580,7 @@ impl Inner { let root_cert_store = Some(get_root_cert_store( maybe_root_path, workspace_settings.certificate_stores, - workspace_settings.tls_certificate, + workspace_settings.tls_certificate.map(CaData::File), )?); let client = HttpClient::new( root_cert_store, @@ -2952,7 +2953,7 @@ impl Inner { Flags { cache_path: self.maybe_cache_path.clone(), ca_stores: None, - ca_file: None, + ca_data: None, unsafely_ignore_certificate_errors: None, // this is to allow loading npm specifiers, so we can remove this // once stabilizing them diff --git a/cli/standalone.rs b/cli/standalone.rs index 18d134293f..593cff7ce2 100644 --- a/cli/standalone.rs +++ b/cli/standalone.rs @@ -1,5 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use crate::args::CaData; use crate::args::Flags; use crate::colors; use crate::file_fetcher::get_source_from_data_url; @@ -7,7 +8,6 @@ use crate::ops; use crate::proc_state::ProcState; use crate::version; use crate::CliResolver; -use deno_core::anyhow::anyhow; use deno_core::anyhow::Context; use deno_core::error::type_error; use deno_core::error::AnyError; @@ -26,7 +26,6 @@ use deno_core::ModuleSpecifier; use deno_core::ResolutionKind; use deno_graph::source::Resolver; use deno_runtime::deno_broadcast_channel::InMemoryBroadcastChannel; -use deno_runtime::deno_tls::rustls_pemfile; use deno_runtime::deno_web::BlobStore; use deno_runtime::fmt_errors::format_js_error; use deno_runtime::permissions::Permissions; @@ -38,8 +37,6 @@ use deno_runtime::BootstrapOptions; use import_map::parse_from_json; use log::Level; use std::env::current_exe; -use std::io::BufReader; -use std::io::Cursor; use std::io::SeekFrom; use std::iter::once; use std::pin::Pin; @@ -217,6 +214,7 @@ fn metadata_to_flags(metadata: &Metadata) -> Flags { v8_flags: metadata.v8_flags.clone(), log_level: metadata.log_level, ca_stores: metadata.ca_stores.clone(), + ca_data: metadata.ca_data.clone().map(CaData::Bytes), ..Default::default() } } @@ -257,22 +255,7 @@ pub async fn run( .collect::>(), ); - let mut root_cert_store = ps.root_cert_store.clone(); - - if let Some(cert) = metadata.ca_data { - let reader = &mut BufReader::new(Cursor::new(cert)); - match rustls_pemfile::certs(reader) { - Ok(certs) => { - root_cert_store.add_parsable_certificates(&certs); - } - Err(e) => { - return Err(anyhow!( - "Unable to add pem file to certificate store: {}", - e - )); - } - } - } + let root_cert_store = ps.root_cert_store.clone(); let options = WorkerOptions { bootstrap: BootstrapOptions { diff --git a/cli/tests/integration/cert_tests.rs b/cli/tests/integration/cert_tests.rs index 0a403c39a2..9e0e810b02 100644 --- a/cli/tests/integration/cert_tests.rs +++ b/cli/tests/integration/cert_tests.rs @@ -112,6 +112,40 @@ fn cafile_fetch() { assert_eq!(out, ""); } +#[test] +fn cafile_compile() { + let _g = util::http_server(); + let dir = TempDir::new(); + let exe = if cfg!(windows) { + dir.path().join("cert.exe") + } else { + dir.path().join("cert") + }; + let output = util::deno_cmd() + .current_dir(util::testdata_path()) + .arg("compile") + .arg("--cert") + .arg("./tls/RootCA.pem") + .arg("--allow-net") + .arg("--output") + .arg(&exe) + .arg("./cert/cafile_ts_fetch.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + let output = Command::new(exe) + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + assert_eq!(output.stdout, b"[WILDCARD]\nHello\n") +} + #[flaky_test::flaky_test] fn cafile_install_remote_module() { let _g = util::http_server(); diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index 344758590d..2d2584f548 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -1,6 +1,7 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use crate::args::resolve_no_prompt; +use crate::args::CaData; use crate::args::ConfigFlag; use crate::args::Flags; use crate::args::InstallFlags; @@ -325,7 +326,7 @@ fn resolve_shim_data( executable_args.push("--location".to_string()); executable_args.push(url.to_string()); } - if let Some(ca_file) = &flags.ca_file { + if let Some(CaData::File(ca_file)) = &flags.ca_data { executable_args.push("--cert".to_string()); executable_args.push(ca_file.to_owned()) } diff --git a/cli/tools/standalone.rs b/cli/tools/standalone.rs index b5738654a0..8b99b8874f 100644 --- a/cli/tools/standalone.rs +++ b/cli/tools/standalone.rs @@ -1,5 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use crate::args::CaData; use crate::args::CompileFlags; use crate::args::Flags; use crate::cache::DenoDir; @@ -158,10 +159,11 @@ async fn create_standalone_binary( ) -> Result, AnyError> { let mut eszip_archive = eszip.into_bytes(); - let ca_data = match ps.options.ca_file() { - Some(ca_file) => { + let ca_data = match ps.options.ca_data() { + Some(CaData::File(ca_file)) => { Some(fs::read(ca_file).with_context(|| format!("Reading: {}", ca_file))?) } + Some(CaData::Bytes(bytes)) => Some(bytes.clone()), None => None, }; let maybe_import_map: Option<(Url, String)> =