collective
Browse / github.com/storytold/craft-launcher / patch
native: refuse plain-HTTP redirects on every HTTPS agent
Set https_only on the HTTP agents (HTTPS to HTTP redirects are followed) ## Summary None of the ureq agents built in `crates/native/src/http.rs:57-97` set `https_only`, and ureq 3's default is `false` (`ureq-3.4.2/src/config.rs:944`). An HTTPS request that is redirected to `http://` is therefore followed in plain text. This includes `get_text`, which fetches `SHA256SUMS.txt`, and `download`. GitHub doesn't redirect to HTTP today, so this is hardening. It means the protection on checksums and payloads doesn't depend on every server in the redirect chain behaving. The API token is not at risk, because ureq's `redirect_auth_headers` defaults to `Never`. ## Suggeste
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- hakarrr (imported from pull request #798, open)
- Base
26a346262403eb65337c15125e150c98c21529d4- Size
- +375 −3 in 5 files
- Review
- pending
- Shared
- 2026-10-28
- Id
7fa0a5e3800166f71d7073a4
How it went
No other operator has reported using it yet.
Diff
raw · Cargo.lock crates/native/Cargo.toml crates/native/src/http.rs crates/native/tests/https_only.rs crates/native/tests/https_redirect.rs
@@ -319,6 +319,8 @@ dependencies = [ "flate2", "log", "plist",+ "rcgen",+ "rustls", "serde", "serde_json", "sha2",@@ -380,6 +382,45 @@ dependencies = [ "libloading 0.8.9", ] +[[package]]+name = "asn1-rs"+version = "0.7.2"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "b7f43a50ac4fdca5df8e885c21b835997f0a1cdee65494a6847694a98652d9d8"+dependencies = [+ "asn1-rs-derive",+ "asn1-rs-impl",+ "displaydoc",+ "nom",+ "num-traits",+ "rusticata-macros",+ "thiserror 2.0.21",+ "time",+]++[[package]]+name = "asn1-rs-derive"+version = "0.6.0"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "3109e49b1e4909e9db6515a30c633684d68cdeaa252f215214cb4fa1a5bfee2c"+dependencies = [+ "proc-macro2",+ "quote",+ "syn 2.0.119",+ "synstructure 0.13.2",+]++[[package]]+name = "asn1-rs-impl"+version = "0.2.0"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "7b18050c2cd6fe86c3a76584ef5e0baf286d038cda203eb6223df2cc413565f7"+dependencies = [+ "proc-macro2",+ "quote",+ "syn 2.0.119",+]+ [[package]] name = "async-broadcast" version = "0.7.2"@@ -609,6 +650,9 @@ name = "bit-vec" version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b71798fca2c1fe1086445a7258a4bc81e6e49dcd24c8d0dd9a1e57395b603f51"+dependencies = [+ "serde",+] [[package]] name = "bitflags"@@ -1009,6 +1053,12 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f27ae1dd37df86211c42e150270f82743308803d90a6f6e6651cd730d5e1732f" +[[package]]+name = "data-encoding"+version = "2.11.1"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "4583a4551df46e2792f82ceeac45e850d2e2d5debba0b91f102385cda5b11f06"+ [[package]] name = "data-url" version = "0.3.2"@@ -1046,6 +1096,20 @@ dependencies = [ "thiserror 2.0.21", ] +[[package]]+name = "der-parser"+version = "10.0.0"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "07da5016415d5a3c4dd39b11ed26f915f52fc4e0dc197d87908bc916e51bc1a6"+dependencies = [+ "asn1-rs",+ "displaydoc",+ "nom",+ "num-bigint",+ "num-traits",+ "rusticata-macros",+]+ [[package]] name = "deranged" version = "0.5.8"@@ -2749,6 +2813,12 @@ dependencies = [ "autocfg", ] +[[package]]+name = "minimal-lexical"+version = "0.2.1"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a"+ [[package]] name = "miniz_oxide" version = "0.8.9"@@ -2874,6 +2944,16 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2bf50223579dc7cdcfb3bfcacf7069ff68243f8c363f62ffa99cf000a6b9c451" +[[package]]+name = "nom"+version = "7.1.3"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "d273983c5a657a70a3e8f2a01329822f3b8c8172b73826411a55751e404a0a4a"+dependencies = [+ "memchr",+ "minimal-lexical",+]+ [[package]] name = "notify-rust" version = "4.18.2"@@ -2888,12 +2968,31 @@ dependencies = [ "zbus", ] +[[package]]+name = "num-bigint"+version = "0.4.8"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "c89e69e7e0f03bea5ef08013795c25018e101932225a656383bd384495ecc367"+dependencies = [+ "num-integer",+ "num-traits",+]+ [[package]] name = "num-conv" version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "521739c6d2bac4aa25192232afe6841231376b2b26d4d9fae5ecf8ca5772e441" +[[package]]+name = "num-integer"+version = "0.1.47"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "7ce2d95d4b3734dc35aa2f45e1aa22cd416814592a4f9d9205e11affd5b8e10b"+dependencies = [+ "num-traits",+]+ [[package]] name = "num-traits" version = "0.2.19"@@ -3238,6 +3337,15 @@ dependencies = [ "objc2-foundation 0.2.2", ] +[[package]]+name = "oid-registry"+version = "0.8.1"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "12f40cff3dde1b6087cc5d5f5d4d65712f34016a03ed60e9c08dcc392736b5b7"+dependencies = [+ "asn1-rs",+]+ [[package]] name = "once_cell" version = "1.21.4"@@ -3361,6 +3469,16 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2ee67f1008b1ba2321834326597b8e186293b049a023cdef258527550b9935b4" +[[package]]+name = "pem"+version = "4.0.0"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "d354a98a3d1251555de99e8fdd8afda05573c31b82f59063a7b0a29b5527f120"+dependencies = [+ "base64 0.23.1",+ "serde_core",+]+ [[package]] name = "peniko" version = "0.6.1"@@ -3751,6 +3869,20 @@ dependencies = [ "crossbeam-utils", ] +[[package]]+name = "rcgen"+version = "0.14.10"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "8774e05a7d0de114588e6a28fe7e71694b82614ed569d86d8b389dfbc98b8ad8"+dependencies = [+ "pem",+ "ring",+ "rustls-pki-types",+ "time",+ "x509-parser",+ "yasna",+]+ [[package]] name = "read-fonts" version = "0.41.0"@@ -3939,6 +4071,15 @@ dependencies = [ "semver", ] +[[package]]+name = "rusticata-macros"+version = "4.1.0"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "faf0c4a6ece9950b9abdb62b1cfcf2a68b3b67a10ba445b3bb85be2a293d0632"+dependencies = [+ "nom",+]+ [[package]] name = "rustix" version = "0.38.44"@@ -4434,6 +4575,17 @@ dependencies = [ "unicode-ident", ] +[[package]]+name = "synstructure"+version = "0.13.2"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "728a70f3dbaf5bab7f0c4b1ac8d7ae5ea60a4b5549c8a5914361c99147a709d2"+dependencies = [+ "proc-macro2",+ "quote",+ "syn 2.0.119",+]+ [[package]] name = "synstructure" version = "0.14.0"@@ -4510,7 +4662,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand",- "getrandom 0.4.3",+ "getrandom 0.3.4", "once_cell", "rustix 1.1.5", "windows-sys 0.61.2",@@ -5854,6 +6006,24 @@ version = "0.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ea6fc2961e4ef194dcbfe56bb845534d0dc8098940c7e5c012a258bfec6701bd" +[[package]]+name = "x509-parser"+version = "0.18.1"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "d43b0f71ce057da06bc0851b23ee24f3f86190b07203dd8f567d0b706a185202"+dependencies = [+ "asn1-rs",+ "data-encoding",+ "der-parser",+ "lazy_static",+ "nom",+ "oid-registry",+ "ring",+ "rusticata-macros",+ "thiserror 2.0.21",+ "time",+]+ [[package]] name = "xcursor" version = "0.3.11"@@ -5898,6 +6068,16 @@ dependencies = [ "jiff", ] +[[package]]+name = "yasna"+version = "0.6.0"+source = "registry+https://github.com/rust-lang/crates.io-index"+checksum = "b5f6765e852b9b4dc8e2a76843e4d64d1cea8e79bcde0b6901aea8e7c7f08282"+dependencies = [+ "bit-vec",+ "time",+]+ [[package]] name = "yoke" version = "0.8.3"@@ -5918,7 +6098,7 @@ dependencies = [ "proc-macro2", "quote", "syn 3.0.6",- "synstructure",+ "synstructure 0.14.0", ] [[package]]@@ -6065,7 +6245,7 @@ dependencies = [ "proc-macro2", "quote", "syn 3.0.6",- "synstructure",+ "synstructure 0.14.0", ] [[package]]@@ -44,3 +44,8 @@ zip = { version = "8", default-features = false, features = ["deflate"] } winsafe = { version = "0.0.29", features = ["kernel", "version"] } # Authenticode checks through WinTrust: the update helper installs only files from the launcher's publisher. verifysign = "1"++[target.'cfg(target_os = "linux")'.dev-dependencies]+# A loopback TLS server proves HTTPS downgrade redirects are refused by production agents.+rcgen = { version = "0.14", default-features = false, features = ["crypto", "ring", "pem"] }+rustls = { version = "0.23", default-features = false, features = ["ring", "std"] }@@ -13,6 +13,11 @@ const USER_AGENT: &str = concat!("ArtCraftLauncher/", env!("CARGO_PKG_VERSION")) /// Largest JSON or text answer we read. const MAX_SMALL: u64 = 4 << 20; +/// Plain HTTP is only ever spoken to a developer's loopback mirror (feature `local-mirror`) and+/// to this module's test server. Everywhere else an HTTPS answer that redirects to `http://` must+/// fail, not fetch checksums and payloads in clear text.+const HTTPS_ONLY: bool = !cfg!(any(test, feature = "local-mirror"));+ /// What a GET of a small resource produced. #[derive(Debug, PartialEq, Eq)] pub enum Fetched {@@ -62,6 +67,7 @@ impl Http { .proxy(proxy.clone()) .timeout_global(Some(Duration::from_secs(20))) .http_status_as_error(false)+ .https_only(HTTPS_ONLY) .user_agent(USER_AGENT) .build() .new_agent();@@ -71,6 +77,7 @@ impl Http { .timeout_connect(Some(Duration::from_secs(5))) .timeout_global(Some(Duration::from_secs(10))) .http_status_as_error(false)+ .https_only(HTTPS_ONLY) .user_agent(USER_AGENT) .build() .new_agent();@@ -81,6 +88,7 @@ impl Http { .http_status_as_error(false) .max_redirects(0) .max_redirects_will_error(false)+ .https_only(HTTPS_ONLY) .user_agent(USER_AGENT) .build() .new_agent();@@ -92,6 +100,7 @@ impl Http { // ureq has no idle timeout: cap each attempt instead, and resume where it stopped. .timeout_recv_body(Some(ATTEMPT_BODY_TIME)) .http_status_as_error(false)+ .https_only(HTTPS_ONLY) .user_agent(USER_AGENT) .build() .new_agent();@@ -555,6 +564,19 @@ pub(crate) mod tests { const DAYS_30: Duration = Duration::from_secs(30 * 86_400); const DAYS_7: Duration = Duration::from_secs(7 * 86_400); + /// ureq's `https_only`, which the shipped agents set (see `HTTPS_ONLY`): a plain-HTTP request+ /// is refused before any connection. Pinned against ureq itself because the test server below+ /// speaks loopback HTTP, so this crate's own agents allow plain HTTP under `cfg(test)`.+ #[test]+ fn https_only_refuses_plain_http_before_connecting() {+ let listener = TcpListener::bind("127.0.0.1:0").unwrap();+ let url = format!("http://{}/file.bin", listener.local_addr().unwrap());+ let agent = ureq::Agent::config_builder().https_only(true).timeout_global(Some(Duration::from_secs(5))).build().new_agent();+ assert!(agent.get(&url).call().is_err());+ listener.set_nonblocking(true).unwrap();+ assert!(listener.accept().is_err(), "a plain-HTTP request reached the server");+ }+ #[test] fn cleaning_keeps_current_partial_files_and_removes_leftovers() { let dir = tempfile::tempdir().unwrap();new file mode 100644@@ -0,0 +1,42 @@+//! The shipped `Http` agents refuse plain-HTTP URLs before connecting.+//!+//! This is an integration test, not a unit test inside the crate, so the library builds without+//! `cfg(test)` and `Http::new()` produces the production agents with `https_only` set. The+//! crate's own unit tests allow plain HTTP so their test server can speak loopback HTTP, which+//! means they can only pin ureq's behavior, not the agents the launcher ships.++#![cfg(not(feature = "local-mirror"))]++use std::net::TcpListener;++use artcraft_launcher_native::http::Http;++#[test]+fn production_agents_refuse_plain_http_before_connecting() {+ // Isolate proxy settings in a child process, without unsafe process-global environment+ // mutation in a parallel Rust test runner. System proxies already bypass loopback.+ if std::env::var_os("ARTCRAFT_HTTPS_ONLY_TEST_CHILD").is_none() {+ let mut child = std::process::Command::new(std::env::current_exe().expect("test executable"));+ child.args(["--exact", "production_agents_refuse_plain_http_before_connecting", "--nocapture"]);+ for key in ["ALL_PROXY", "all_proxy", "HTTPS_PROXY", "https_proxy", "HTTP_PROXY", "http_proxy"] {+ child.env_remove(key);+ }+ let status = child.env("NO_PROXY", "*").env("no_proxy", "*").env("ARTCRAFT_HTTPS_ONLY_TEST_CHILD", "1").status().expect("run isolated test");+ assert!(status.success(), "isolated HTTPS-only regression test failed");+ return;+ }+ let listener = TcpListener::bind("127.0.0.1:0").expect("bind test server");+ listener.set_nonblocking(true).expect("nonblocking listener");+ let url = format!("http://{}/releases/latest", listener.local_addr().expect("listener address"));++ let http = Http::new().expect("Http::new");+ let refuses_https_only = |message: &str| {+ assert!(message.contains("configured for https only:"), "unexpected refusal: {message}");+ };+ refuses_https_only(&http.get_api(&url, None).expect_err("plain-HTTP API request succeeded"));+ refuses_https_only(&http.get_feed(&url).expect_err("plain-HTTP feed request succeeded").why);+ refuses_https_only(&http.get_text(&url).expect_err("plain-HTTP GET succeeded"));+ refuses_https_only(&http.redirect_target(&url).expect_err("plain-HTTP redirect probe succeeded"));++ assert!(listener.accept().is_err(), "a plain-HTTP request reached the server");+}new file mode 100644@@ -0,0 +1,123 @@+//! Exercise an actual HTTPS response before rejecting its HTTP Location.+//! Linux's platform verifier accepts a child-scoped SSL_CERT_FILE; no host trust store is changed.+#![cfg(all(target_os = "linux", not(feature = "local-mirror")))]++use std::io::{Read, Write};+use std::net::TcpListener;+use std::sync::Arc;+use std::time::{Duration, Instant};++use artcraft_launcher_core::protocol::PAUSED;+use artcraft_launcher_native::http::{Http, Stop, Transfer};+use rcgen::{BasicConstraints, CertificateParams, ExtendedKeyUsagePurpose, IsCa, Issuer, KeyPair, KeyUsagePurpose};++#[test]+fn production_agents_refuse_https_to_http_redirect() {+ if std::env::var_os("ARTCRAFT_TLS_REDIRECT_CHILD").is_none() {+ let dir = tempfile::tempdir().expect("temporary trust directory");+ let mut ca = CertificateParams::new(Vec::<String>::new()).expect("CA params");+ ca.is_ca = IsCa::Ca(BasicConstraints::Unconstrained);+ ca.key_usages = vec![KeyUsagePurpose::KeyCertSign, KeyUsagePurpose::DigitalSignature];+ let ca_key = KeyPair::generate().expect("CA key");+ let ca_cert = ca.self_signed(&ca_key).expect("CA certificate");+ let issuer = Issuer::new(ca, ca_key);+ let mut leaf = CertificateParams::new(vec!["localhost".into()]).expect("server params");+ leaf.extended_key_usages = vec![ExtendedKeyUsagePurpose::ServerAuth];+ let key = KeyPair::generate().expect("server key");+ let cert = leaf.signed_by(&key, &issuer).expect("signed server certificate");+ std::fs::write(dir.path().join("ca.pem"), ca_cert.pem()).expect("write root");+ std::fs::write(dir.path().join("server.der"), cert.der()).expect("write server cert");+ std::fs::write(dir.path().join("key.der"), key.serialize_der()).expect("write ephemeral test key");+ let mut child = std::process::Command::new(std::env::current_exe().expect("test executable"));+ child.args(["--exact", "production_agents_refuse_https_to_http_redirect", "--nocapture"]);+ for key in ["ALL_PROXY", "all_proxy", "HTTPS_PROXY", "https_proxy", "HTTP_PROXY", "http_proxy", "SSL_CERT_DIR"] {+ child.env_remove(key);+ }+ let status = child+ .env("NO_PROXY", "*")+ .env("no_proxy", "*")+ .env("SSL_CERT_FILE", dir.path().join("ca.pem"))+ .env("ARTCRAFT_TLS_REDIRECT_CHILD", dir.path())+ .status()+ .expect("isolated TLS test");+ assert!(status.success(), "isolated TLS redirect test failed");+ return;+ }+ let dir = std::path::PathBuf::from(std::env::var_os("ARTCRAFT_TLS_REDIRECT_CHILD").expect("test directory"));+ let cert = rustls::pki_types::CertificateDer::from(std::fs::read(dir.join("server.der")).expect("server cert"));+ let key = rustls::pki_types::PrivatePkcs8KeyDer::from(std::fs::read(dir.join("key.der")).expect("server key"));+ let config = rustls::ServerConfig::builder().with_no_client_auth().with_single_cert(vec![cert], key.into()).expect("server TLS config");+ let plain = TcpListener::bind("127.0.0.1:0").expect("HTTP target");+ plain.set_nonblocking(true).expect("HTTP nonblocking");+ let target = format!("http://{}/untrusted", plain.local_addr().expect("HTTP address"));+ let tls = TcpListener::bind("127.0.0.1:0").expect("TLS listener");+ tls.set_nonblocking(true).expect("TLS nonblocking");+ let url = format!("https://localhost:{}/redirect", tls.local_addr().expect("TLS address").port());+ let location = target.clone();+ let plain_server = plain.try_clone().expect("HTTP target clone");+ let plain_requests = Arc::new(std::sync::atomic::AtomicUsize::new(0));+ let observed_plain = plain_requests.clone();+ let done = Arc::new(std::sync::atomic::AtomicBool::new(false));+ let server_done = done.clone();+ let server = std::thread::spawn(move || {+ let deadline = Instant::now() + Duration::from_secs(15);+ let mut requests = 0;+ while !server_done.load(std::sync::atomic::Ordering::SeqCst) && Instant::now() < deadline {+ if let Ok((mut socket, _)) = plain_server.accept() {+ observed_plain.fetch_add(1, std::sync::atomic::Ordering::SeqCst);+ socket.set_write_timeout(Some(Duration::from_secs(1))).expect("HTTP write timeout");+ socket.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok").expect("unexpected HTTP response");+ }+ match tls.accept() {+ Ok((socket, _)) => {+ socket.set_read_timeout(Some(Duration::from_secs(3))).expect("read timeout");+ socket.set_write_timeout(Some(Duration::from_secs(3))).expect("write timeout");+ let session = rustls::ServerConnection::new(Arc::new(config.clone())).expect("TLS connection");+ let mut stream = rustls::StreamOwned::new(session, socket);+ let mut request = Vec::new();+ let mut byte = [0];+ while request.len() < 16384 && !request.ends_with(b"\r\n\r\n") {+ stream.read_exact(&mut byte).expect("verified TLS request");+ request.push(byte[0]);+ }+ assert!(request.ends_with(b"\r\n\r\n"), "bounded request headers");+ write!(stream, "HTTP/1.1 302 Found\r\nLocation: {location}\r\nContent-Length: 0\r\nConnection: close\r\n\r\n")+ .expect("redirect response");+ stream.flush().expect("flush redirect");+ requests += 1;+ }+ Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => std::thread::sleep(Duration::from_millis(5)),+ Err(e) => panic!("TLS accept: {e}"),+ }+ }+ requests+ });+ let http = Http::new().expect("production Http");+ let refused = |message: &str| assert!(message.contains("configured for https only:"), "wrong failure: {message}");+ refused(&http.get_api(&url, None).expect_err("API followed HTTP redirect"));+ refused(&http.get_feed(&url).expect_err("feed followed HTTP redirect").why);+ refused(&http.get_text(&url).expect_err("text followed HTTP redirect"));+ // The probe deliberately does not follow redirects: returning the destination is its contract.+ assert_eq!(http.redirect_target(&url).expect("HTTPS redirect probe"), Some(target));+ let download_dir = tempfile::tempdir().expect("download directory");+ let dest = download_dir.path().join("payload");+ let stop = Stop::default();+ let mut reconnected = false;+ // Reconnecting is emitted only after the verified TLS request was refused. Pause before+ // backoff, not before the request; the target would serve a real body if HTTPS_ONLY broke.+ let result = http.download(&url, &dest, 2, "2689367b205c16ce32ed4200942b8b8b1e262dfc70d9bc9fbc77c49699a4f1df", &stop, &mut |event| {+ if matches!(event, Transfer::Reconnecting { .. }) {+ reconnected = true;+ stop.pause();+ }+ });+ done.store(true, std::sync::atomic::Ordering::SeqCst);+ let requests = server.join().expect("TLS server");+ let error = result.expect_err("download followed HTTP redirect");+ assert_eq!(error, PAUSED);+ assert!(reconnected, "download must reject before reconnecting");+ assert!(!dest.exists(), "refused download must not install a file");+ assert_eq!(requests, 5, "all requests must reach verified TLS");+ assert_eq!(plain_requests.load(std::sync::atomic::Ordering::SeqCst), 0, "downgrade reached HTTP target");+ assert!(plain.accept().is_err(), "downgrade reached plain HTTP target");+}