From d97e33a2aba62df3347c316efe96257c992d9f80 Mon Sep 17 00:00:00 2001 From: Arish Anwar Date: Thu, 30 Jul 2026 06:33:47 +0000 Subject: [PATCH 1/3] support custom ca certificates without disabling verification --- lib/wreq_ruby/client.rb | 38 +++++ src/client.rs | 98 ++++++++++- test/custom_ca_test.rb | 367 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 501 insertions(+), 2 deletions(-) create mode 100644 test/custom_ca_test.rb diff --git a/lib/wreq_ruby/client.rb b/lib/wreq_ruby/client.rb index 9e62269..25d2187 100644 --- a/lib/wreq_ruby/client.rb +++ b/lib/wreq_ruby/client.rb @@ -144,6 +144,35 @@ class Client # {Wreq::TlsInfo} object. Disabled by default because retaining # certificate data uses additional memory. # + # @param ca_file [String, #to_path, nil] Path to a PEM-encoded CA bundle + # that **replaces** the default system trust store. Only certificates + # signed by CAs in this file will be trusted. Accepts any object + # responding to +to_path+ (e.g. +Pathname+). The file is read during + # client construction; a missing or unreadable file raises immediately. + # Mutually exclusive with +ca_pem+, +additional_ca_file+, and + # +additional_ca_pem+. + # + # @param ca_pem [String, nil] Raw PEM-encoded certificate content that + # **replaces** the default system trust store. Useful when certificate + # material comes from a secret store or environment variable rather + # than a file on disk. Must contain at least one + # +-----BEGIN CERTIFICATE-----+ block. + # Mutually exclusive with +ca_file+, +additional_ca_file+, and + # +additional_ca_pem+. + # + # @param additional_ca_file [String, #to_path, nil] Path to a PEM-encoded + # CA bundle loaded **alongside** the default system trust store. + # Public roots remain available; the supplied certificates are added + # on top. Accepts any object responding to +to_path+ (e.g. +Pathname+). + # Mutually exclusive with +ca_file+, +ca_pem+, and +additional_ca_pem+. + # + # @param additional_ca_pem [String, nil] Raw PEM-encoded certificate + # content loaded **alongside** the default system trust store. Public + # roots remain available; the supplied certificates are added on top. + # Must contain at least one +-----BEGIN CERTIFICATE-----+ block. + # Mutually exclusive with +ca_file+, +ca_pem+, and + # +additional_ca_file+. + # # @param no_proxy [Boolean, nil] Disable use of any configured proxy # for this client, even if proxy settings are detected from the # environment. @@ -254,6 +283,15 @@ class Client # verify: false, # WARNING: Do not use in production! # timeout: 5 # ) + # @example Client with custom CA (replace system roots) + # client = Wreq::Client.new( + # ca_file: "/etc/ssl/private/internal-ca.pem" + # ) + # + # @example Client with additional CA (augment system roots) + # client = Wreq::Client.new( + # additional_ca_pem: File.binread("/etc/ssl/certs/extra-ca.pem") + # ) def self.new(**options) end diff --git a/src/client.rs b/src/client.rs index e42b488..0a1a67a 100644 --- a/src/client.rs +++ b/src/client.rs @@ -7,7 +7,10 @@ pub mod resp; use std::{net::IpAddr, time::Duration}; use ::serde::Deserialize; -use magnus::{Module, Object, RModule, Ruby, TryConvert, Value, function, method, typed_data::Obj}; +use magnus::{ + Module, Object, RModule, RString, Ruby, TryConvert, Value, function, method, typed_data::Obj, + value::ReprValue, +}; use wreq::Proxy; use crate::{ @@ -15,7 +18,7 @@ use crate::{ client::{req::execute_request, resp::Response}, cookie::Jar, emulate::Emulation, - error::wreq_error, + error::{argument_error, option_value_error, wreq_error}, extractor::Extractor, gvl, header::{Headers, OrigHeaders, UserAgent}, @@ -96,6 +99,16 @@ struct Builder { verify: Option, /// Whether to retain peer certificate data on responses. tls_info: Option, + /// Path to a PEM CA bundle that replaces the default trust store. + #[serde(default)] + ca_file: NativeOption, + /// Raw PEM certificate content that replaces the default trust store. + ca_pem: Option, + /// Path to a PEM CA bundle added alongside the default trust store. + #[serde(default)] + additional_ca_file: NativeOption, + /// Raw PEM certificate content added alongside the default trust store. + additional_ca_pem: Option, // ========= Network options ========= /// Whether to disable the proxy for the client. @@ -152,6 +165,18 @@ impl Builder { (stringify!(proxy), options.is_non_nil(stringify!(proxy))), (stringify!(no_proxy), builder.no_proxy == Some(true)), ]) + .reject_conflicts([ + (stringify!(ca_file), options.is_non_nil(stringify!(ca_file))), + (stringify!(ca_pem), builder.ca_pem.is_some()), + ( + stringify!(additional_ca_file), + options.is_non_nil(stringify!(additional_ca_file)), + ), + ( + stringify!(additional_ca_pem), + builder.additional_ca_pem.is_some(), + ), + ]) .require_when_present( stringify!(max_redirects), builder.max_redirects.is_some(), @@ -160,6 +185,22 @@ impl Builder { ) .finish()?; + // Convert path-like objects (Pathname, etc.) for CA file options. + if let Some(value) = options.convert_present::("ca_file")? { + if !value.is_nil() { + builder.ca_file.set(Some( + convert_path(value).map_err(|e| option_value_error("ca_file", e))?, + )); + } + } + if let Some(value) = options.convert_present::("additional_ca_file")? { + if !value.is_nil() { + builder.additional_ca_file.set(Some( + convert_path(value).map_err(|e| option_value_error("additional_ca_file", e))?, + )); + } + } + extract_native_option!( options, builder, @@ -178,6 +219,16 @@ impl Builder { } } +/// Convert a Ruby value to a file-system path `String`. +/// +/// Accepts a plain `String` or any object responding to `to_path` (e.g. `Pathname`). +fn convert_path(value: Value) -> Result { + if let Ok(path) = value.funcall::<_, _, RString>("to_path", ()) { + return path.to_string().map_err(|e| e.into()); + } + String::try_convert(value) +} + // ===== impl Client ===== impl Client { @@ -221,6 +272,39 @@ impl Client { .take() .map(|jar| jar.clone_store(ruby)) .transpose()?; + + // Resolve the CA option (at most one is set, enforced by reject_conflicts). + // Read file contents before releasing the GVL so I/O errors become ArgumentError. + // The bool indicates whether to augment system defaults (true) or replace them (false). + let ca_pem_data: Option<(Vec, bool)> = if let Some(path) = params.ca_file.take() { + let data = std::fs::read(&path) + .map_err(|e| argument_error(ruby, format!("ca_file: cannot read {path}: {e}")))?; + Some((data, false)) + } else if let Some(pem) = params.ca_pem.take() { + Some((pem.into_bytes(), false)) + } else if let Some(path) = params.additional_ca_file.take() { + let data = std::fs::read(&path).map_err(|e| { + argument_error(ruby, format!("additional_ca_file: cannot read {path}: {e}")) + })?; + Some((data, true)) + } else if let Some(pem) = params.additional_ca_pem.take() { + Some((pem.into_bytes(), true)) + } else { + None + }; + + // Validate that PEM data contains at least one certificate. + if let Some((ref pem_bytes, _)) = ca_pem_data { + if !pem_bytes + .windows(27) + .any(|w| w == b"-----BEGIN CERTIFICATE-----") + { + return Err(argument_error( + ruby, + "PEM data does not contain any certificates", + )); + } + } let result = gvl::nogvl(|| { let mut builder = wreq::Client::builder(); @@ -351,6 +435,16 @@ impl Client { apply_option!(set_if_some, builder, params.verify, tls_cert_verification); apply_option!(set_if_some, builder, params.tls_info, tls_info); + // Custom CA certificate store. + if let Some((pem_bytes, augment)) = ca_pem_data { + let mut store_builder = wreq::tls::trust::CertStore::builder(); + if augment { + store_builder = store_builder.set_default_paths(); + } + let store = store_builder.add_stack_pem_certs(&pem_bytes).build()?; + builder = builder.tls_cert_store(store); + } + // Network options. apply_option!(set_if_some, builder, params.proxy, proxy); apply_option!(set_if_true, builder, params.no_proxy, no_proxy, false); diff --git a/test/custom_ca_test.rb b/test/custom_ca_test.rb new file mode 100644 index 0000000..38a6114 --- /dev/null +++ b/test/custom_ca_test.rb @@ -0,0 +1,367 @@ +# frozen_string_literal: true + +require "test_helper" +require "pathname" +require "openssl" +require "socket" +require "tempfile" + +class CustomCaTest < Minitest::Test + # Shared test CA and server certificate, generated once per suite. + CA_KEY = OpenSSL::PKey::RSA.new(2048) + CA_CERT = OpenSSL::X509::Certificate.new.tap do |cert| + cert.version = 2 + cert.serial = 1 + cert.subject = OpenSSL::X509::Name.parse("/CN=Test CA") + cert.issuer = cert.subject + cert.public_key = CA_KEY.public_key + cert.not_before = Time.now - 60 + cert.not_after = Time.now + 3600 + + ef = OpenSSL::X509::ExtensionFactory.new + ef.subject_certificate = cert + ef.issuer_certificate = cert + cert.add_extension(ef.create_extension("basicConstraints", "CA:TRUE", true)) + cert.add_extension(ef.create_extension("subjectKeyIdentifier", "hash")) + + cert.sign(CA_KEY, OpenSSL::Digest::SHA256.new) + end + + SERVER_KEY = OpenSSL::PKey::RSA.new(2048) + SERVER_CERT = OpenSSL::X509::Certificate.new.tap do |cert| + cert.version = 2 + cert.serial = 2 + cert.subject = OpenSSL::X509::Name.parse("/CN=localhost") + cert.issuer = CA_CERT.subject + cert.public_key = SERVER_KEY.public_key + cert.not_before = Time.now - 60 + cert.not_after = Time.now + 3600 + + ef = OpenSSL::X509::ExtensionFactory.new + ef.subject_certificate = cert + ef.issuer_certificate = CA_CERT + cert.add_extension(ef.create_extension("subjectAltName", "DNS:localhost,IP:127.0.0.1")) + + cert.sign(CA_KEY, OpenSSL::Digest::SHA256.new) + end + + CA_PEM = CA_CERT.to_pem + + # A second unrelated CA that did NOT sign the server cert. + OTHER_KEY = OpenSSL::PKey::RSA.new(2048) + OTHER_CERT = OpenSSL::X509::Certificate.new.tap do |cert| + cert.version = 2 + cert.serial = 3 + cert.subject = OpenSSL::X509::Name.parse("/CN=Other CA") + cert.issuer = cert.subject + cert.public_key = OTHER_KEY.public_key + cert.not_before = Time.now - 60 + cert.not_after = Time.now + 3600 + cert.sign(OTHER_KEY, OpenSSL::Digest::SHA256.new) + end + + OTHER_PEM = OTHER_CERT.to_pem + + # Bundle containing both CAs. + BUNDLE_PEM = CA_PEM + OTHER_PEM + + @server_started = false + + def self.start_server + return if @server_started + @server_started = true + + ctx = OpenSSL::SSL::SSLContext.new + ctx.cert = SERVER_CERT + ctx.key = SERVER_KEY + + tcp = TCPServer.new("127.0.0.1", 0) + @port = tcp.addr[1] + @ssl_server = OpenSSL::SSL::SSLServer.new(tcp, ctx) + @server_url = "https://localhost:#{@port}" + + @thread = Thread.new do + loop do + begin + client = @ssl_server.accept + rescue OpenSSL::SSL::SSLError + next + rescue IOError + break + end + Thread.new(client) do |c| + begin + c.gets + c.print "HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok" + rescue + ensure + c.close rescue nil + end + end + end + end + @thread.abort_on_exception = true + + Minitest.after_run do + @ssl_server&.close + @thread&.kill + @thread&.join(2) + end + end + + def self.server_url + @server_url + end + + def setup + self.class.start_server + end + + def server_url + self.class.server_url + end + + # ---- Helper: write PEM to a tempfile and return its path ---- + + def with_pem_file(content) + file = Tempfile.new(["ca", ".pem"]) + file.write(content) + file.flush + yield file.path + ensure + file&.close! + end + + # ================================================================= + # Replace semantics: ca_file / ca_pem + # ================================================================= + + def test_ca_pem_trusts_server_signed_by_that_ca + client = Wreq::Client.new(ca_pem: CA_PEM, timeout: 3) + resp = client.get(server_url) + assert_equal 200, resp.code + end + + def test_ca_file_trusts_server_signed_by_that_ca + with_pem_file(CA_PEM) do |path| + client = Wreq::Client.new(ca_file: path, timeout: 3) + resp = client.get(server_url) + assert_equal 200, resp.code + end + end + + def test_ca_file_rejects_server_not_signed_by_that_ca + with_pem_file(OTHER_PEM) do |path| + client = Wreq::Client.new(ca_file: path, timeout: 3) + assert_raises(Wreq::ConnectionError, Wreq::TimeoutError) { client.get(server_url) } + end + end + + def test_ca_pem_replaces_system_roots + client = Wreq::Client.new(ca_pem: CA_PEM, timeout: 3) + assert_raises(Wreq::ConnectionError) do + client.get("https://www.google.com") + end + end + + def test_ca_file_does_not_set_verify_false + client = Wreq::Client.new(ca_pem: OTHER_PEM, timeout: 3) + assert_raises(Wreq::ConnectionError, Wreq::TimeoutError) { client.get(server_url) } + end + + # ================================================================= + # Augment semantics: additional_ca_file / additional_ca_pem + # ================================================================= + + def test_additional_ca_pem_trusts_custom_ca_and_system_roots + client = Wreq::Client.new(additional_ca_pem: CA_PEM, timeout: 3) + resp = client.get(server_url) + assert_equal 200, resp.code + + resp = client.get("https://www.google.com") + assert_equal 200, resp.code + end + + def test_additional_ca_file_trusts_custom_ca_and_system_roots + with_pem_file(CA_PEM) do |path| + client = Wreq::Client.new(additional_ca_file: path, timeout: 3) + resp = client.get(server_url) + assert_equal 200, resp.code + + resp = client.get("https://www.google.com") + assert_equal 200, resp.code + end + end + + # ================================================================= + # Bundled PEM (multiple certificates in one file/string) + # ================================================================= + + def test_ca_pem_accepts_bundled_certificates + client = Wreq::Client.new(ca_pem: BUNDLE_PEM, timeout: 3) + resp = client.get(server_url) + assert_equal 200, resp.code + end + + def test_ca_file_accepts_bundled_certificates + with_pem_file(BUNDLE_PEM) do |path| + client = Wreq::Client.new(ca_file: path, timeout: 3) + resp = client.get(server_url) + assert_equal 200, resp.code + end + end + + # ================================================================= + # Path-like objects (to_path protocol) + # ================================================================= + + def test_ca_file_accepts_pathname + with_pem_file(CA_PEM) do |path| + client = Wreq::Client.new(ca_file: Pathname.new(path), timeout: 3) + resp = client.get(server_url) + assert_equal 200, resp.code + end + end + + def test_additional_ca_file_accepts_pathname + with_pem_file(CA_PEM) do |path| + client = Wreq::Client.new(additional_ca_file: Pathname.new(path), timeout: 3) + resp = client.get(server_url) + assert_equal 200, resp.code + end + end + + def test_ca_file_accepts_custom_to_path_object + with_pem_file(CA_PEM) do |path| + path_like = Object.new + path_like.define_singleton_method(:to_path) { path } + + client = Wreq::Client.new(ca_file: path_like, timeout: 3) + resp = client.get(server_url) + assert_equal 200, resp.code + end + end + + # ================================================================= + # Invalid inputs fail during construction + # ================================================================= + + def test_missing_ca_file_raises_argument_error + error = assert_raises(ArgumentError) do + Wreq::Client.new(ca_file: "/nonexistent/ca.pem") + end + assert_includes error.message, "ca_file" + assert_includes error.message, "cannot read" + refute_includes error.message, "BEGIN CERTIFICATE" + end + + def test_missing_additional_ca_file_raises_argument_error + error = assert_raises(ArgumentError) do + Wreq::Client.new(additional_ca_file: "/nonexistent/extra.pem") + end + assert_includes error.message, "additional_ca_file" + end + + def test_empty_ca_pem_raises_argument_error + assert_raises(ArgumentError) do + Wreq::Client.new(ca_pem: "") + end + end + + def test_garbage_ca_pem_raises_argument_error + error = assert_raises(ArgumentError) do + Wreq::Client.new(ca_pem: "not valid pem") + end + assert_includes error.message, "does not contain any certificates" + refute_includes error.message, "not valid pem" + end + + def test_malformed_base64_pem_raises_tls_error + bad_pem = "-----BEGIN CERTIFICATE-----\nthis-is-not-valid-base64!!!\n-----END CERTIFICATE-----\n" + assert_raises(Wreq::TlsError) do + Wreq::Client.new(ca_pem: bad_pem) + end + end + + def test_malformed_ca_file_raises_tls_error + bad_pem = "-----BEGIN CERTIFICATE-----\nthis-is-not-valid-base64!!!\n-----END CERTIFICATE-----\n" + with_pem_file(bad_pem) do |path| + assert_raises(Wreq::TlsError) do + Wreq::Client.new(ca_file: path) + end + end + end + + # ================================================================= + # Mutual exclusion + # ================================================================= + + def test_ca_file_and_ca_pem_are_mutually_exclusive + error = assert_raises(ArgumentError) do + Wreq::Client.new(ca_file: "/a", ca_pem: "b") + end + assert_includes error.message, ":ca_file" + assert_includes error.message, ":ca_pem" + end + + def test_ca_file_and_additional_ca_pem_are_mutually_exclusive + error = assert_raises(ArgumentError) do + Wreq::Client.new(ca_file: "/a", additional_ca_pem: "b") + end + assert_includes error.message, ":ca_file" + assert_includes error.message, ":additional_ca_pem" + end + + def test_all_four_ca_options_are_mutually_exclusive + error = assert_raises(ArgumentError) do + Wreq::Client.new(ca_file: "/a", ca_pem: "b", additional_ca_pem: "c") + end + assert_includes error.message, ":ca_file" + assert_includes error.message, ":ca_pem" + assert_includes error.message, ":additional_ca_pem" + end + + # ================================================================= + # nil values are treated as absent + # ================================================================= + + def test_nil_ca_options_are_ignored + client = Wreq::Client.new(ca_pem: nil, ca_file: nil, timeout: 3) + assert_instance_of Wreq::Client, client + end + + # ================================================================= + # verify: false with CA options + # ================================================================= + + def test_verify_false_with_valid_ca_still_builds + with_pem_file(CA_PEM) do |path| + client = Wreq::Client.new(verify: false, ca_file: path) + assert_instance_of Wreq::Client, client + end + end + + def test_verify_false_does_not_skip_pem_validation + assert_raises(ArgumentError) do + Wreq::Client.new(verify: false, ca_pem: "not valid pem") + end + end + + # ================================================================= + # Inspect does not leak CA configuration + # ================================================================= + + def test_inspect_does_not_leak_ca_file_path + with_pem_file(CA_PEM) do |path| + client = Wreq::Client.new(ca_file: path) + refute_includes client.inspect, path + refute_includes client.inspect, "BEGIN CERTIFICATE" + end + end + + def test_inspect_does_not_leak_ca_pem_content + client = Wreq::Client.new(ca_pem: CA_PEM) + refute_includes client.inspect, "BEGIN CERTIFICATE" + assert_equal "#", client.inspect + end +end \ No newline at end of file From 6a5a097caa8284a9686da60a36f410b10c939256 Mon Sep 17 00:00:00 2001 From: Arish Anwar Date: Thu, 30 Jul 2026 18:56:27 +0000 Subject: [PATCH 2/3] lint + skip some tests on windows --- src/client.rs | 48 +++++++++++++++++++++--------------------- test/custom_ca_test.rb | 21 +++++++++++++++--- 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/src/client.rs b/src/client.rs index 0a1a67a..f52789e 100644 --- a/src/client.rs +++ b/src/client.rs @@ -186,19 +186,19 @@ impl Builder { .finish()?; // Convert path-like objects (Pathname, etc.) for CA file options. - if let Some(value) = options.convert_present::("ca_file")? { - if !value.is_nil() { - builder.ca_file.set(Some( - convert_path(value).map_err(|e| option_value_error("ca_file", e))?, - )); - } + if let Some(value) = options.convert_present::("ca_file")? + && !value.is_nil() + { + builder.ca_file.set(Some( + convert_path(value).map_err(|e| option_value_error("ca_file", e))?, + )); } - if let Some(value) = options.convert_present::("additional_ca_file")? { - if !value.is_nil() { - builder.additional_ca_file.set(Some( - convert_path(value).map_err(|e| option_value_error("additional_ca_file", e))?, - )); - } + if let Some(value) = options.convert_present::("additional_ca_file")? + && !value.is_nil() + { + builder.additional_ca_file.set(Some( + convert_path(value).map_err(|e| option_value_error("additional_ca_file", e))?, + )); } extract_native_option!( @@ -224,7 +224,7 @@ impl Builder { /// Accepts a plain `String` or any object responding to `to_path` (e.g. `Pathname`). fn convert_path(value: Value) -> Result { if let Ok(path) = value.funcall::<_, _, RString>("to_path", ()) { - return path.to_string().map_err(|e| e.into()); + return path.to_string(); } String::try_convert(value) } @@ -287,23 +287,23 @@ impl Client { argument_error(ruby, format!("additional_ca_file: cannot read {path}: {e}")) })?; Some((data, true)) - } else if let Some(pem) = params.additional_ca_pem.take() { - Some((pem.into_bytes(), true)) } else { - None + params + .additional_ca_pem + .take() + .map(|pem| (pem.into_bytes(), true)) }; // Validate that PEM data contains at least one certificate. - if let Some((ref pem_bytes, _)) = ca_pem_data { - if !pem_bytes + if let Some((ref pem_bytes, _)) = ca_pem_data + && !pem_bytes .windows(27) .any(|w| w == b"-----BEGIN CERTIFICATE-----") - { - return Err(argument_error( - ruby, - "PEM data does not contain any certificates", - )); - } + { + return Err(argument_error( + ruby, + "PEM data does not contain any certificates", + )); } let result = gvl::nogvl(|| { let mut builder = wreq::Client::builder(); diff --git a/test/custom_ca_test.rb b/test/custom_ca_test.rb index 38a6114..dda1d73 100644 --- a/test/custom_ca_test.rb +++ b/test/custom_ca_test.rb @@ -7,6 +7,8 @@ require "tempfile" class CustomCaTest < Minitest::Test + SKIP_LOCAL_TLS = Gem.win_platform? + # Shared test CA and server certificate, generated once per suite. CA_KEY = OpenSSL::PKey::RSA.new(2048) CA_CERT = OpenSSL::X509::Certificate.new.tap do |cert| @@ -68,7 +70,7 @@ class CustomCaTest < Minitest::Test @server_started = false def self.start_server - return if @server_started + return if @server_started || SKIP_LOCAL_TLS @server_started = true ctx = OpenSSL::SSL::SSLContext.new @@ -137,12 +139,14 @@ def with_pem_file(content) # ================================================================= def test_ca_pem_trusts_server_signed_by_that_ca + skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS client = Wreq::Client.new(ca_pem: CA_PEM, timeout: 3) resp = client.get(server_url) assert_equal 200, resp.code end def test_ca_file_trusts_server_signed_by_that_ca + skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS with_pem_file(CA_PEM) do |path| client = Wreq::Client.new(ca_file: path, timeout: 3) resp = client.get(server_url) @@ -151,6 +155,7 @@ def test_ca_file_trusts_server_signed_by_that_ca end def test_ca_file_rejects_server_not_signed_by_that_ca + skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS with_pem_file(OTHER_PEM) do |path| client = Wreq::Client.new(ca_file: path, timeout: 3) assert_raises(Wreq::ConnectionError, Wreq::TimeoutError) { client.get(server_url) } @@ -165,8 +170,11 @@ def test_ca_pem_replaces_system_roots end def test_ca_file_does_not_set_verify_false - client = Wreq::Client.new(ca_pem: OTHER_PEM, timeout: 3) - assert_raises(Wreq::ConnectionError, Wreq::TimeoutError) { client.get(server_url) } + skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS + with_pem_file(OTHER_PEM) do |path| + client = Wreq::Client.new(ca_file: path, timeout: 3) + assert_raises(Wreq::ConnectionError, Wreq::TimeoutError) { client.get(server_url) } + end end # ================================================================= @@ -174,6 +182,7 @@ def test_ca_file_does_not_set_verify_false # ================================================================= def test_additional_ca_pem_trusts_custom_ca_and_system_roots + skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS client = Wreq::Client.new(additional_ca_pem: CA_PEM, timeout: 3) resp = client.get(server_url) assert_equal 200, resp.code @@ -183,6 +192,7 @@ def test_additional_ca_pem_trusts_custom_ca_and_system_roots end def test_additional_ca_file_trusts_custom_ca_and_system_roots + skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS with_pem_file(CA_PEM) do |path| client = Wreq::Client.new(additional_ca_file: path, timeout: 3) resp = client.get(server_url) @@ -198,12 +208,14 @@ def test_additional_ca_file_trusts_custom_ca_and_system_roots # ================================================================= def test_ca_pem_accepts_bundled_certificates + skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS client = Wreq::Client.new(ca_pem: BUNDLE_PEM, timeout: 3) resp = client.get(server_url) assert_equal 200, resp.code end def test_ca_file_accepts_bundled_certificates + skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS with_pem_file(BUNDLE_PEM) do |path| client = Wreq::Client.new(ca_file: path, timeout: 3) resp = client.get(server_url) @@ -216,6 +228,7 @@ def test_ca_file_accepts_bundled_certificates # ================================================================= def test_ca_file_accepts_pathname + skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS with_pem_file(CA_PEM) do |path| client = Wreq::Client.new(ca_file: Pathname.new(path), timeout: 3) resp = client.get(server_url) @@ -224,6 +237,7 @@ def test_ca_file_accepts_pathname end def test_additional_ca_file_accepts_pathname + skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS with_pem_file(CA_PEM) do |path| client = Wreq::Client.new(additional_ca_file: Pathname.new(path), timeout: 3) resp = client.get(server_url) @@ -232,6 +246,7 @@ def test_additional_ca_file_accepts_pathname end def test_ca_file_accepts_custom_to_path_object + skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS with_pem_file(CA_PEM) do |path| path_like = Object.new path_like.define_singleton_method(:to_path) { path } From c88e1d16bdd43abc9c01b5ae75d0a276a1c9d217 Mon Sep 17 00:00:00 2001 From: Arish Anwar Date: Sun, 16 Aug 2026 08:08:17 +0000 Subject: [PATCH 3/3] refactor for better design --- lib/wreq_ruby/client.rb | 11 +- src/client.rs | 91 ++++----------- src/client/ca.rs | 70 ++++++++++++ src/lib.rs | 1 + src/macros.rs | 13 +++ src/utils.rs | 13 +++ test/custom_ca_test.rb | 202 +++++----------------------------- test/support/ca_tls_server.rb | 129 ++++++++++++++++++++++ 8 files changed, 283 insertions(+), 247 deletions(-) create mode 100644 src/client/ca.rs create mode 100644 src/utils.rs create mode 100644 test/support/ca_tls_server.rb diff --git a/lib/wreq_ruby/client.rb b/lib/wreq_ruby/client.rb index 25d2187..0ceae71 100644 --- a/lib/wreq_ruby/client.rb +++ b/lib/wreq_ruby/client.rb @@ -155,8 +155,8 @@ class Client # @param ca_pem [String, nil] Raw PEM-encoded certificate content that # **replaces** the default system trust store. Useful when certificate # material comes from a secret store or environment variable rather - # than a file on disk. Must contain at least one - # +-----BEGIN CERTIFICATE-----+ block. + # than a file on disk. Invalid PEM that the native store rejects raises + # {Wreq::TlsError} during client construction. # Mutually exclusive with +ca_file+, +additional_ca_file+, and # +additional_ca_pem+. # @@ -164,12 +164,15 @@ class Client # CA bundle loaded **alongside** the default system trust store. # Public roots remain available; the supplied certificates are added # on top. Accepts any object responding to +to_path+ (e.g. +Pathname+). - # Mutually exclusive with +ca_file+, +ca_pem+, and +additional_ca_pem+. + # The file is read during client construction; a missing or unreadable + # file raises immediately. Mutually exclusive with +ca_file+, +ca_pem+, + # and +additional_ca_pem+. # # @param additional_ca_pem [String, nil] Raw PEM-encoded certificate # content loaded **alongside** the default system trust store. Public # roots remain available; the supplied certificates are added on top. - # Must contain at least one +-----BEGIN CERTIFICATE-----+ block. + # Invalid PEM that the native store rejects raises + # {Wreq::TlsError} during client construction. # Mutually exclusive with +ca_file+, +ca_pem+, and # +additional_ca_file+. # diff --git a/src/client.rs b/src/client.rs index f52789e..641a0d0 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1,4 +1,5 @@ mod body; +mod ca; mod param; mod query; mod req; @@ -7,10 +8,7 @@ pub mod resp; use std::{net::IpAddr, time::Duration}; use ::serde::Deserialize; -use magnus::{ - Module, Object, RModule, RString, Ruby, TryConvert, Value, function, method, typed_data::Obj, - value::ReprValue, -}; +use magnus::{Module, Object, RModule, Ruby, TryConvert, Value, function, method, typed_data::Obj}; use wreq::Proxy; use crate::{ @@ -18,12 +16,13 @@ use crate::{ client::{req::execute_request, resp::Response}, cookie::Jar, emulate::Emulation, - error::{argument_error, option_value_error, wreq_error}, + error::{option_value_error, wreq_error}, extractor::Extractor, gvl, header::{Headers, OrigHeaders, UserAgent}, http::Method, options::{NativeOption, Options}, + utils::convert_path, }; /// A builder for `Client`. @@ -185,22 +184,14 @@ impl Builder { ) .finish()?; - // Convert path-like objects (Pathname, etc.) for CA file options. - if let Some(value) = options.convert_present::("ca_file")? - && !value.is_nil() - { - builder.ca_file.set(Some( - convert_path(value).map_err(|e| option_value_error("ca_file", e))?, - )); - } - if let Some(value) = options.convert_present::("additional_ca_file")? - && !value.is_nil() - { - builder.additional_ca_file.set(Some( - convert_path(value).map_err(|e| option_value_error("additional_ca_file", e))?, - )); - } - + extract_native_option!( + options, builder, ca_file, + Value =>? convert_path + ); + extract_native_option!( + options, builder, additional_ca_file, + Value =>? convert_path + ); extract_native_option!( options, builder, @@ -219,16 +210,6 @@ impl Builder { } } -/// Convert a Ruby value to a file-system path `String`. -/// -/// Accepts a plain `String` or any object responding to `to_path` (e.g. `Pathname`). -fn convert_path(value: Value) -> Result { - if let Ok(path) = value.funcall::<_, _, RString>("to_path", ()) { - return path.to_string(); - } - String::try_convert(value) -} - // ===== impl Client ===== impl Client { @@ -273,38 +254,7 @@ impl Client { .map(|jar| jar.clone_store(ruby)) .transpose()?; - // Resolve the CA option (at most one is set, enforced by reject_conflicts). - // Read file contents before releasing the GVL so I/O errors become ArgumentError. - // The bool indicates whether to augment system defaults (true) or replace them (false). - let ca_pem_data: Option<(Vec, bool)> = if let Some(path) = params.ca_file.take() { - let data = std::fs::read(&path) - .map_err(|e| argument_error(ruby, format!("ca_file: cannot read {path}: {e}")))?; - Some((data, false)) - } else if let Some(pem) = params.ca_pem.take() { - Some((pem.into_bytes(), false)) - } else if let Some(path) = params.additional_ca_file.take() { - let data = std::fs::read(&path).map_err(|e| { - argument_error(ruby, format!("additional_ca_file: cannot read {path}: {e}")) - })?; - Some((data, true)) - } else { - params - .additional_ca_pem - .take() - .map(|pem| (pem.into_bytes(), true)) - }; - - // Validate that PEM data contains at least one certificate. - if let Some((ref pem_bytes, _)) = ca_pem_data - && !pem_bytes - .windows(27) - .any(|w| w == b"-----BEGIN CERTIFICATE-----") - { - return Err(argument_error( - ruby, - "PEM data does not contain any certificates", - )); - } + let mut ca = ca::resolve(ruby, &mut params)?; let result = gvl::nogvl(|| { let mut builder = wreq::Client::builder(); @@ -436,14 +386,13 @@ impl Client { apply_option!(set_if_some, builder, params.tls_info, tls_info); // Custom CA certificate store. - if let Some((pem_bytes, augment)) = ca_pem_data { - let mut store_builder = wreq::tls::trust::CertStore::builder(); - if augment { - store_builder = store_builder.set_default_paths(); - } - let store = store_builder.add_stack_pem_certs(&pem_bytes).build()?; - builder = builder.tls_cert_store(store); - } + apply_option!( + set_if_some_try_map, + builder, + ca, + tls_cert_store, + ca::into_cert_store + ); // Network options. apply_option!(set_if_some, builder, params.proxy, proxy); diff --git a/src/client/ca.rs b/src/client/ca.rs new file mode 100644 index 0000000..014a932 --- /dev/null +++ b/src/client/ca.rs @@ -0,0 +1,70 @@ +//! Custom CA trust-store resolution for [`super::Client`] construction. + +use magnus::Ruby; + +use crate::error::argument_error; + +use super::Builder; + +/// PEM material used to build a custom [`wreq::tls::trust::CertStore`]. +pub(super) struct CaPem { + /// PEM-encoded certificate bytes (one cert or a bundle). + pub(super) pem: Vec, + /// When `true`, keep system roots and add these certs. + /// When `false`, replace the default trust store. + pub(super) augment: bool, +} + +/// Resolve the mutually exclusive CA client options into PEM bytes. +/// +/// Reads `*_file` options here (with the GVL held) so missing/unreadable +/// files raise `ArgumentError` before native client construction. +pub(super) fn resolve(ruby: &Ruby, params: &mut Builder) -> Result, magnus::Error> { + let mut pem: Option> = None; + let mut augment_flag: Option = None; + + // ---- path options: ca_file / additional_ca_file ---- + if let Some((path, additional)) = params + .ca_file + .take() + .map(|path| (path, false)) + .or_else(|| params.additional_ca_file.take().map(|path| (path, true))) + { + let name = if additional { + "additional_ca_file" + } else { + "ca_file" + }; + pem = + Some(std::fs::read(&path).map_err(|_| { + argument_error(ruby, format!("{name}: cannot read certificate file")) + })?); + augment_flag = Some(additional); + } + + // ---- string options: ca_pem / additional_ca_pem ---- + if let Some((value, additional)) = params + .ca_pem + .take() + .map(|value| (value, false)) + .or_else(|| params.additional_ca_pem.take().map(|value| (value, true))) + { + pem = Some(value.into_bytes()); + augment_flag = Some(additional); + } + + if let (Some(pem), Some(augment)) = (pem, augment_flag) { + return Ok(Some(CaPem { pem, augment })); + } + + Ok(None) +} + +/// Attach a custom certificate store to the native client builder. +pub(super) fn into_cert_store(ca: CaPem) -> wreq::Result { + let mut store_builder = wreq::tls::trust::CertStore::builder(); + if ca.augment { + store_builder = store_builder.set_default_paths(); + } + store_builder.add_stack_pem_certs(&ca.pem).build() +} diff --git a/src/lib.rs b/src/lib.rs index 0503557..6b40180 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -16,6 +16,7 @@ mod options; mod rt; mod serde; mod tls; +mod utils; use magnus::{Error, Module, Ruby, Value}; diff --git a/src/macros.rs b/src/macros.rs index 18d97c6..f4db55d 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -34,6 +34,11 @@ macro_rules! apply_option { $builder = $builder.$method(); } }; + (set_if_some_try_map, $builder:expr, $option:expr, $method:ident, $transform:expr) => { + if let Some(value) = $option.take() { + $builder = $builder.$method($transform(value)?); + } + }; } /// Convert a Ruby-native option whose field name is also its keyword name. @@ -51,6 +56,14 @@ macro_rules! extract_native_option { .$field .set($options.convert::<$source>(stringify!($field))?.map($map)); }}; + ($options:expr, $target:expr, $field:ident, $source:ty =>? $map:expr) => {{ + $target.$field.set( + $options + .convert::<$source>(stringify!($field))? + .map(|v| ($map)(v).map_err(|e| option_value_error(stringify!($field), e))) + .transpose()?, + ); + }}; } macro_rules! define_ruby_enum { diff --git a/src/utils.rs b/src/utils.rs new file mode 100644 index 0000000..1295aa7 --- /dev/null +++ b/src/utils.rs @@ -0,0 +1,13 @@ +//! Shared helpers + +use magnus::{RString, TryConvert, Value, value::ReprValue}; + +/// Convert a Ruby value to a file-system path `String`. +/// +/// Accepts a plain `String` or any object responding to `to_path` (e.g. `Pathname`). +pub(crate) fn convert_path(value: Value) -> Result { + if let Ok(path) = value.funcall::<_, _, RString>("to_path", ()) { + return path.to_string(); + } + String::try_convert(value) +} diff --git a/test/custom_ca_test.rb b/test/custom_ca_test.rb index dda1d73..297c110 100644 --- a/test/custom_ca_test.rb +++ b/test/custom_ca_test.rb @@ -2,136 +2,14 @@ require "test_helper" require "pathname" -require "openssl" -require "socket" -require "tempfile" +require_relative "support/ca_tls_server" class CustomCaTest < Minitest::Test SKIP_LOCAL_TLS = Gem.win_platform? - # Shared test CA and server certificate, generated once per suite. - CA_KEY = OpenSSL::PKey::RSA.new(2048) - CA_CERT = OpenSSL::X509::Certificate.new.tap do |cert| - cert.version = 2 - cert.serial = 1 - cert.subject = OpenSSL::X509::Name.parse("/CN=Test CA") - cert.issuer = cert.subject - cert.public_key = CA_KEY.public_key - cert.not_before = Time.now - 60 - cert.not_after = Time.now + 3600 - - ef = OpenSSL::X509::ExtensionFactory.new - ef.subject_certificate = cert - ef.issuer_certificate = cert - cert.add_extension(ef.create_extension("basicConstraints", "CA:TRUE", true)) - cert.add_extension(ef.create_extension("subjectKeyIdentifier", "hash")) - - cert.sign(CA_KEY, OpenSSL::Digest::SHA256.new) - end - - SERVER_KEY = OpenSSL::PKey::RSA.new(2048) - SERVER_CERT = OpenSSL::X509::Certificate.new.tap do |cert| - cert.version = 2 - cert.serial = 2 - cert.subject = OpenSSL::X509::Name.parse("/CN=localhost") - cert.issuer = CA_CERT.subject - cert.public_key = SERVER_KEY.public_key - cert.not_before = Time.now - 60 - cert.not_after = Time.now + 3600 - - ef = OpenSSL::X509::ExtensionFactory.new - ef.subject_certificate = cert - ef.issuer_certificate = CA_CERT - cert.add_extension(ef.create_extension("subjectAltName", "DNS:localhost,IP:127.0.0.1")) - - cert.sign(CA_KEY, OpenSSL::Digest::SHA256.new) - end - - CA_PEM = CA_CERT.to_pem - - # A second unrelated CA that did NOT sign the server cert. - OTHER_KEY = OpenSSL::PKey::RSA.new(2048) - OTHER_CERT = OpenSSL::X509::Certificate.new.tap do |cert| - cert.version = 2 - cert.serial = 3 - cert.subject = OpenSSL::X509::Name.parse("/CN=Other CA") - cert.issuer = cert.subject - cert.public_key = OTHER_KEY.public_key - cert.not_before = Time.now - 60 - cert.not_after = Time.now + 3600 - cert.sign(OTHER_KEY, OpenSSL::Digest::SHA256.new) - end - - OTHER_PEM = OTHER_CERT.to_pem - - # Bundle containing both CAs. - BUNDLE_PEM = CA_PEM + OTHER_PEM - - @server_started = false - - def self.start_server - return if @server_started || SKIP_LOCAL_TLS - @server_started = true - - ctx = OpenSSL::SSL::SSLContext.new - ctx.cert = SERVER_CERT - ctx.key = SERVER_KEY - - tcp = TCPServer.new("127.0.0.1", 0) - @port = tcp.addr[1] - @ssl_server = OpenSSL::SSL::SSLServer.new(tcp, ctx) - @server_url = "https://localhost:#{@port}" - - @thread = Thread.new do - loop do - begin - client = @ssl_server.accept - rescue OpenSSL::SSL::SSLError - next - rescue IOError - break - end - Thread.new(client) do |c| - begin - c.gets - c.print "HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok" - rescue - ensure - c.close rescue nil - end - end - end - end - @thread.abort_on_exception = true - - Minitest.after_run do - @ssl_server&.close - @thread&.kill - @thread&.join(2) - end - end - - def self.server_url - @server_url - end - def setup - self.class.start_server - end - - def server_url - self.class.server_url - end - - # ---- Helper: write PEM to a tempfile and return its path ---- - - def with_pem_file(content) - file = Tempfile.new(["ca", ".pem"]) - file.write(content) - file.flush - yield file.path - ensure - file&.close! + return if SKIP_LOCAL_TLS + CaTlsServer.start_server! end # ================================================================= @@ -140,30 +18,30 @@ def with_pem_file(content) def test_ca_pem_trusts_server_signed_by_that_ca skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS - client = Wreq::Client.new(ca_pem: CA_PEM, timeout: 3) - resp = client.get(server_url) + client = Wreq::Client.new(ca_pem: CaTlsServer::CA_PEM, timeout: 3) + resp = client.get(CaTlsServer.server_url) assert_equal 200, resp.code end def test_ca_file_trusts_server_signed_by_that_ca skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS - with_pem_file(CA_PEM) do |path| + CaTlsServer.with_pem_file(CaTlsServer::CA_PEM) do |path| client = Wreq::Client.new(ca_file: path, timeout: 3) - resp = client.get(server_url) + resp = client.get(CaTlsServer.server_url) assert_equal 200, resp.code end end def test_ca_file_rejects_server_not_signed_by_that_ca skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS - with_pem_file(OTHER_PEM) do |path| + CaTlsServer.with_pem_file(CaTlsServer::OTHER_PEM) do |path| client = Wreq::Client.new(ca_file: path, timeout: 3) - assert_raises(Wreq::ConnectionError, Wreq::TimeoutError) { client.get(server_url) } + assert_raises(Wreq::ConnectionError, Wreq::TimeoutError) { client.get(CaTlsServer.server_url) } end end def test_ca_pem_replaces_system_roots - client = Wreq::Client.new(ca_pem: CA_PEM, timeout: 3) + client = Wreq::Client.new(ca_pem: CaTlsServer::CA_PEM, timeout: 3) assert_raises(Wreq::ConnectionError) do client.get("https://www.google.com") end @@ -171,9 +49,9 @@ def test_ca_pem_replaces_system_roots def test_ca_file_does_not_set_verify_false skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS - with_pem_file(OTHER_PEM) do |path| + CaTlsServer.with_pem_file(CaTlsServer::OTHER_PEM) do |path| client = Wreq::Client.new(ca_file: path, timeout: 3) - assert_raises(Wreq::ConnectionError, Wreq::TimeoutError) { client.get(server_url) } + assert_raises(Wreq::ConnectionError, Wreq::TimeoutError) { client.get(CaTlsServer.server_url) } end end @@ -183,8 +61,8 @@ def test_ca_file_does_not_set_verify_false def test_additional_ca_pem_trusts_custom_ca_and_system_roots skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS - client = Wreq::Client.new(additional_ca_pem: CA_PEM, timeout: 3) - resp = client.get(server_url) + client = Wreq::Client.new(additional_ca_pem: CaTlsServer::CA_PEM, timeout: 3) + resp = client.get(CaTlsServer.server_url) assert_equal 200, resp.code resp = client.get("https://www.google.com") @@ -193,9 +71,9 @@ def test_additional_ca_pem_trusts_custom_ca_and_system_roots def test_additional_ca_file_trusts_custom_ca_and_system_roots skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS - with_pem_file(CA_PEM) do |path| + CaTlsServer.with_pem_file(CaTlsServer::CA_PEM) do |path| client = Wreq::Client.new(additional_ca_file: path, timeout: 3) - resp = client.get(server_url) + resp = client.get(CaTlsServer.server_url) assert_equal 200, resp.code resp = client.get("https://www.google.com") @@ -209,16 +87,16 @@ def test_additional_ca_file_trusts_custom_ca_and_system_roots def test_ca_pem_accepts_bundled_certificates skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS - client = Wreq::Client.new(ca_pem: BUNDLE_PEM, timeout: 3) - resp = client.get(server_url) + client = Wreq::Client.new(ca_pem: CaTlsServer::BUNDLE_PEM, timeout: 3) + resp = client.get(CaTlsServer.server_url) assert_equal 200, resp.code end def test_ca_file_accepts_bundled_certificates skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS - with_pem_file(BUNDLE_PEM) do |path| + CaTlsServer.with_pem_file(CaTlsServer::BUNDLE_PEM) do |path| client = Wreq::Client.new(ca_file: path, timeout: 3) - resp = client.get(server_url) + resp = client.get(CaTlsServer.server_url) assert_equal 200, resp.code end end @@ -229,30 +107,30 @@ def test_ca_file_accepts_bundled_certificates def test_ca_file_accepts_pathname skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS - with_pem_file(CA_PEM) do |path| + CaTlsServer.with_pem_file(CaTlsServer::CA_PEM) do |path| client = Wreq::Client.new(ca_file: Pathname.new(path), timeout: 3) - resp = client.get(server_url) + resp = client.get(CaTlsServer.server_url) assert_equal 200, resp.code end end def test_additional_ca_file_accepts_pathname skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS - with_pem_file(CA_PEM) do |path| + CaTlsServer.with_pem_file(CaTlsServer::CA_PEM) do |path| client = Wreq::Client.new(additional_ca_file: Pathname.new(path), timeout: 3) - resp = client.get(server_url) + resp = client.get(CaTlsServer.server_url) assert_equal 200, resp.code end end def test_ca_file_accepts_custom_to_path_object skip "Local TLS server not supported on Windows" if SKIP_LOCAL_TLS - with_pem_file(CA_PEM) do |path| + CaTlsServer.with_pem_file(CaTlsServer::CA_PEM) do |path| path_like = Object.new path_like.define_singleton_method(:to_path) { path } client = Wreq::Client.new(ca_file: path_like, timeout: 3) - resp = client.get(server_url) + resp = client.get(CaTlsServer.server_url) assert_equal 200, resp.code end end @@ -277,20 +155,6 @@ def test_missing_additional_ca_file_raises_argument_error assert_includes error.message, "additional_ca_file" end - def test_empty_ca_pem_raises_argument_error - assert_raises(ArgumentError) do - Wreq::Client.new(ca_pem: "") - end - end - - def test_garbage_ca_pem_raises_argument_error - error = assert_raises(ArgumentError) do - Wreq::Client.new(ca_pem: "not valid pem") - end - assert_includes error.message, "does not contain any certificates" - refute_includes error.message, "not valid pem" - end - def test_malformed_base64_pem_raises_tls_error bad_pem = "-----BEGIN CERTIFICATE-----\nthis-is-not-valid-base64!!!\n-----END CERTIFICATE-----\n" assert_raises(Wreq::TlsError) do @@ -300,7 +164,7 @@ def test_malformed_base64_pem_raises_tls_error def test_malformed_ca_file_raises_tls_error bad_pem = "-----BEGIN CERTIFICATE-----\nthis-is-not-valid-base64!!!\n-----END CERTIFICATE-----\n" - with_pem_file(bad_pem) do |path| + CaTlsServer.with_pem_file(bad_pem) do |path| assert_raises(Wreq::TlsError) do Wreq::Client.new(ca_file: path) end @@ -350,24 +214,18 @@ def test_nil_ca_options_are_ignored # ================================================================= def test_verify_false_with_valid_ca_still_builds - with_pem_file(CA_PEM) do |path| + CaTlsServer.with_pem_file(CaTlsServer::CA_PEM) do |path| client = Wreq::Client.new(verify: false, ca_file: path) assert_instance_of Wreq::Client, client end end - def test_verify_false_does_not_skip_pem_validation - assert_raises(ArgumentError) do - Wreq::Client.new(verify: false, ca_pem: "not valid pem") - end - end - # ================================================================= # Inspect does not leak CA configuration # ================================================================= def test_inspect_does_not_leak_ca_file_path - with_pem_file(CA_PEM) do |path| + CaTlsServer.with_pem_file(CaTlsServer::CA_PEM) do |path| client = Wreq::Client.new(ca_file: path) refute_includes client.inspect, path refute_includes client.inspect, "BEGIN CERTIFICATE" @@ -375,7 +233,7 @@ def test_inspect_does_not_leak_ca_file_path end def test_inspect_does_not_leak_ca_pem_content - client = Wreq::Client.new(ca_pem: CA_PEM) + client = Wreq::Client.new(ca_pem: CaTlsServer::CA_PEM) refute_includes client.inspect, "BEGIN CERTIFICATE" assert_equal "#", client.inspect end diff --git a/test/support/ca_tls_server.rb b/test/support/ca_tls_server.rb new file mode 100644 index 0000000..04838f6 --- /dev/null +++ b/test/support/ca_tls_server.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +require "openssl" +require "socket" +require "tempfile" + +# A CA-signed HTTPS server for testing custom trust-store options. +# +# Unlike TlsTestServer (self-signed, ephemeral), this module provides +# a proper CA → leaf chain so tests can verify replace/augment trust +# semantics with ca_file / ca_pem / additional_ca_*. +module CaTlsServer + # --- PKI (generated once at load time) --- + + CA_KEY = OpenSSL::PKey::RSA.new(2048) + CA_CERT = OpenSSL::X509::Certificate.new.tap do |cert| + cert.version = 2 + cert.serial = 1 + cert.subject = OpenSSL::X509::Name.parse("/CN=Test CA") + cert.issuer = cert.subject + cert.public_key = CA_KEY.public_key + cert.not_before = Time.now - 60 + cert.not_after = Time.now + 3600 + + ef = OpenSSL::X509::ExtensionFactory.new + ef.subject_certificate = cert + ef.issuer_certificate = cert + cert.add_extension(ef.create_extension("basicConstraints", "CA:TRUE", true)) + cert.add_extension(ef.create_extension("subjectKeyIdentifier", "hash")) + + cert.sign(CA_KEY, OpenSSL::Digest::SHA256.new) + end + + SERVER_KEY = OpenSSL::PKey::RSA.new(2048) + SERVER_CERT = OpenSSL::X509::Certificate.new.tap do |cert| + cert.version = 2 + cert.serial = 2 + cert.subject = OpenSSL::X509::Name.parse("/CN=localhost") + cert.issuer = CA_CERT.subject + cert.public_key = SERVER_KEY.public_key + cert.not_before = Time.now - 60 + cert.not_after = Time.now + 3600 + + ef = OpenSSL::X509::ExtensionFactory.new + ef.subject_certificate = cert + ef.issuer_certificate = CA_CERT + cert.add_extension(ef.create_extension("subjectAltName", "DNS:localhost,IP:127.0.0.1")) + + cert.sign(CA_KEY, OpenSSL::Digest::SHA256.new) + end + + CA_PEM = CA_CERT.to_pem + + OTHER_KEY = OpenSSL::PKey::RSA.new(2048) + OTHER_CERT = OpenSSL::X509::Certificate.new.tap do |cert| + cert.version = 2 + cert.serial = 3 + cert.subject = OpenSSL::X509::Name.parse("/CN=Other CA") + cert.issuer = cert.subject + cert.public_key = OTHER_KEY.public_key + cert.not_before = Time.now - 60 + cert.not_after = Time.now + 3600 + cert.sign(OTHER_KEY, OpenSSL::Digest::SHA256.new) + end + + OTHER_PEM = OTHER_CERT.to_pem + BUNDLE_PEM = CA_PEM + OTHER_PEM + + # --- Long-lived server (started once, cleaned up at suite end) --- + + @server_started = false + + module_function + + def start_server! + return if @server_started + @server_started = true + + ctx = OpenSSL::SSL::SSLContext.new + ctx.cert = SERVER_CERT + ctx.key = SERVER_KEY + + tcp = TCPServer.new("127.0.0.1", 0) + @port = tcp.addr[1] + @ssl_server = OpenSSL::SSL::SSLServer.new(tcp, ctx) + @server_url = "https://localhost:#{@port}" + + @thread = Thread.new do + loop do + begin + client = @ssl_server.accept + rescue OpenSSL::SSL::SSLError + next + rescue IOError + break + end + Thread.new(client) do |c| + begin + c.gets + c.print "HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok" + rescue + ensure + c.close rescue nil + end + end + end + end + @thread.abort_on_exception = true + + Minitest.after_run do + @ssl_server&.close + @thread&.kill + @thread&.join(2) + end + end + + def server_url + @server_url + end + + def with_pem_file(content) + file = Tempfile.new(["ca", ".pem"]) + file.write(content) + file.flush + yield file.path + ensure + file&.close! + end +end \ No newline at end of file