diff --git a/jetsocat/src/doctor/cert_inspect.rs b/jetsocat/src/doctor/cert_inspect.rs new file mode 100644 index 000000000..8b8fd52e8 --- /dev/null +++ b/jetsocat/src/doctor/cert_inspect.rs @@ -0,0 +1,192 @@ +//! Minimal DER walker for X.509 certificate extension inspection. +//! +//! Navigates the ASN.1 DER structure of an X.509 certificate to check +//! for the presence of specific extensions (SAN, EKU) without requiring +//! a full X.509 parsing library. +//! +//! # Design rationale +//! +//! We intentionally use a hand-written DER walker instead of pulling in a crate +//! such as `x509-cert` or `x509-parser`. +//! Jetsocat is meant to stay lean: adding an X.509 crate would pull a transitive +//! dependency tree (`der`, `spki`, `const-oid`, … or `asn1-rs`, `nom`, `oid-registry`, …), +//! increasing compile times and binary size for what amounts to two boolean questions +//! ("does the cert have a SAN extension?" and "does the EKU contain serverAuth?"). +//! +//! Because the X.509 structure and the OIDs we inspect are standardised and frozen, +//! this code carries virtually no maintenance burden. + +/// OID for Subject Alternative Name (2.5.29.17). +const OID_SUBJECT_ALT_NAME: &[u8] = &[0x55, 0x1D, 0x11]; + +/// OID for Extended Key Usage (2.5.29.37). +const OID_EXTENDED_KEY_USAGE: &[u8] = &[0x55, 0x1D, 0x25]; + +/// OID for id-kp-serverAuth (1.3.6.1.5.5.7.3.1). +const OID_KP_SERVER_AUTH: &[u8] = &[0x2B, 0x06, 0x01, 0x05, 0x05, 0x07, 0x03, 0x01]; + +/// Checks if a DER-encoded X.509 certificate has the Subject Alternative Name extension. +pub(super) fn cert_has_san_extension(cert_der: &[u8]) -> anyhow::Result { + cert_has_extension(cert_der, OID_SUBJECT_ALT_NAME) +} + +/// Checks if a DER-encoded X.509 certificate includes the serverAuth Extended Key Usage. +pub(super) fn cert_has_server_auth_eku(cert_der: &[u8]) -> anyhow::Result { + let Some(eku_value) = cert_find_extension_value(cert_der, OID_EXTENDED_KEY_USAGE)? else { + return Ok(false); + }; + + // EKU value is a SEQUENCE of KeyPurposeId OIDs. + let (tag, content, _) = der_read_tlv(eku_value)?; + anyhow::ensure!(tag == 0x30, "expected EKU SEQUENCE, got tag {tag:#04X}"); + + let mut pos = 0; + while pos < content.len() { + let (tag, oid_bytes, consumed) = der_read_tlv(&content[pos..])?; + anyhow::ensure!(tag == 0x06, "expected OID in EKU SEQUENCE, got tag {tag:#04X}"); + if oid_bytes == OID_KP_SERVER_AUTH { + return Ok(true); + } + pos += consumed; + } + + Ok(false) +} + +fn cert_has_extension(cert_der: &[u8], target_oid: &[u8]) -> anyhow::Result { + Ok(cert_find_extension_value(cert_der, target_oid)?.is_some()) +} + +/// Finds an extension by OID and returns the content of its `extnValue` OCTET STRING. +fn cert_find_extension_value<'a>(cert_der: &'a [u8], target_oid: &[u8]) -> anyhow::Result> { + let Some(extensions) = cert_find_extensions(cert_der)? else { + return Ok(None); + }; + + // Extensions is a SEQUENCE of Extension. + let (tag, exts_content, _) = der_read_tlv(extensions)?; + anyhow::ensure!(tag == 0x30, "expected Extensions SEQUENCE, got tag {tag:#04X}"); + + let mut pos = 0; + while pos < exts_content.len() { + let (tag, ext_bytes, consumed) = der_read_tlv(&exts_content[pos..])?; + anyhow::ensure!(tag == 0x30, "expected Extension SEQUENCE, got tag {tag:#04X}"); + + // Extension ::= SEQUENCE { extnID OID, critical BOOLEAN OPTIONAL, extnValue OCTET STRING } + let (oid_tag, oid_bytes, mut inner_pos) = der_read_tlv(ext_bytes)?; + anyhow::ensure!(oid_tag == 0x06, "expected extension OID, got tag {oid_tag:#04X}"); + + if oid_bytes == target_oid { + // Walk remaining fields to find the OCTET STRING value. + while inner_pos < ext_bytes.len() { + let (inner_tag, inner_bytes, next_inner) = der_read_tlv(&ext_bytes[inner_pos..])?; + if inner_tag == 0x04 { + return Ok(Some(inner_bytes)); + } + inner_pos += next_inner; + } + } + + pos += consumed; + } + + Ok(None) +} + +/// Locates the extensions block in a DER-encoded X.509 certificate. +/// +/// Returns the raw bytes of the `[3] EXPLICIT` wrapper content (which contains +/// the Extensions SEQUENCE), or `None` if extensions are absent. +fn cert_find_extensions(cert_der: &[u8]) -> anyhow::Result> { + // Certificate ::= SEQUENCE { tbsCertificate, signatureAlgorithm, signature } + let (tag, cert_content, _) = der_read_tlv(cert_der)?; + anyhow::ensure!(tag == 0x30, "expected Certificate SEQUENCE, got tag {tag:#04X}"); + + // TBSCertificate ::= SEQUENCE { version, serial, sig, issuer, validity, subject, spki, ... } + let (tag, tbs_content, _) = der_read_tlv(cert_content)?; + anyhow::ensure!(tag == 0x30, "expected TBSCertificate SEQUENCE, got tag {tag:#04X}"); + + let mut pos = 0; + + // version [0] EXPLICIT (optional) + if pos < tbs_content.len() && tbs_content[pos] == 0xA0 { + let (_, _, consumed) = der_read_tlv(&tbs_content[pos..])?; + pos += consumed; + } + + // Skip fixed fields: serialNumber, signature, issuer, validity, subject, subjectPublicKeyInfo. + for field_name in [ + "serialNumber", + "signature", + "issuer", + "validity", + "subject", + "subjectPublicKeyInfo", + ] { + anyhow::ensure!( + pos < tbs_content.len(), + "unexpected end of TBSCertificate before {field_name}" + ); + let (_, _, consumed) = der_read_tlv(&tbs_content[pos..])?; + pos += consumed; + } + + // issuerUniqueID [1] IMPLICIT (optional) + if pos < tbs_content.len() && tbs_content[pos] == 0x81 { + let (_, _, consumed) = der_read_tlv(&tbs_content[pos..])?; + pos += consumed; + } + + // subjectUniqueID [2] IMPLICIT (optional) + if pos < tbs_content.len() && tbs_content[pos] == 0x82 { + let (_, _, consumed) = der_read_tlv(&tbs_content[pos..])?; + pos += consumed; + } + + // extensions [3] EXPLICIT (optional) + if pos < tbs_content.len() && tbs_content[pos] == 0xA3 { + let (_, exts_wrapper, _) = der_read_tlv(&tbs_content[pos..])?; + return Ok(Some(exts_wrapper)); + } + + Ok(None) +} + +/// Reads a DER TLV (Tag-Length-Value) at the start of `data`. +/// +/// Returns `(tag, value_bytes, total_bytes_consumed)`. +fn der_read_tlv(data: &[u8]) -> anyhow::Result<(u8, &[u8], usize)> { + anyhow::ensure!(!data.is_empty(), "unexpected end of DER data"); + + let tag = data[0]; + let (length, length_size) = der_read_length(&data[1..])?; + let header_size = 1 + length_size; + let end = header_size + length; + + anyhow::ensure!(end <= data.len(), "DER value extends beyond available data"); + + Ok((tag, &data[header_size..end], end)) +} + +/// Decodes a DER length field. Returns `(length_value, bytes_consumed)`. +fn der_read_length(data: &[u8]) -> anyhow::Result<(usize, usize)> { + anyhow::ensure!(!data.is_empty(), "unexpected end of DER data reading length"); + + let first = data[0]; + + if first < 0x80 { + // Short form. + Ok((first as usize, 1)) + } else if first == 0x80 { + anyhow::bail!("indefinite-length encoding is not valid DER"); + } else { + // Long form. + let num_bytes = (first & 0x7F) as usize; + anyhow::ensure!(num_bytes <= 4 && num_bytes < data.len(), "invalid DER length encoding"); + let mut length = 0usize; + for i in 0..num_bytes { + length = (length << 8) | data[1 + i] as usize; + } + Ok((length, 1 + num_bytes)) + } +} diff --git a/jetsocat/src/doctor/help.rs b/jetsocat/src/doctor/help.rs index c052bb461..2e6c9613a 100644 --- a/jetsocat/src/doctor/help.rs +++ b/jetsocat/src/doctor/help.rs @@ -69,6 +69,26 @@ You need to generate a separate certificate valid for server authentication." ) } +pub(crate) fn cert_missing_san(ctx: &mut DiagnosticCtx) { + ctx.attach_help( + "The certificate is missing the Subject Alternative Name (SAN) extension. +Modern clients (e.g., Chrome, macOS) require certificates to include SAN entries instead of relying on the Common Name (CN) field. +The Devolutions Gateway will reject this certificate when the TlsVerifyStrict option is enabled. +To resolve this, generate a new certificate that includes the appropriate SAN entries for your domain." + .to_owned(), + ); +} + +pub(crate) fn cert_missing_server_auth_eku(ctx: &mut DiagnosticCtx) { + ctx.attach_help( + "The certificate does not include the serverAuth Extended Key Usage (EKU). +The serverAuth purpose indicates that the certificate is valid for TLS server authentication. +The Devolutions Gateway will reject this certificate when the TlsVerifyStrict option is enabled. +To resolve this, generate a new certificate that includes the serverAuth Extended Key Usage." + .to_owned(), + ); +} + pub(crate) fn x509_io_link(ctx: &mut DiagnosticCtx, certs: C) where C: Iterator, diff --git a/jetsocat/src/doctor/mod.rs b/jetsocat/src/doctor/mod.rs index 2ff48701d..087f2340b 100644 --- a/jetsocat/src/doctor/mod.rs +++ b/jetsocat/src/doctor/mod.rs @@ -1,3 +1,4 @@ +mod cert_inspect; mod common; mod help; mod macros; diff --git a/jetsocat/src/doctor/native_tls.rs b/jetsocat/src/doctor/native_tls.rs index 6a63f5e85..1ec19fc8d 100644 --- a/jetsocat/src/doctor/native_tls.rs +++ b/jetsocat/src/doctor/native_tls.rs @@ -155,6 +155,8 @@ mod openssl { } diagnostic!(callback, openssl_check_chain(&server_certificates)); + diagnostic!(callback, openssl_check_san_extension(&server_certificates)); + diagnostic!(callback, openssl_check_server_auth_eku(&server_certificates)); } } @@ -348,6 +350,54 @@ To resolve this issue, you can: - Obtain and use a certificate signed by a legitimate certification authority."); } + fn openssl_check_san_extension(ctx: &mut DiagnosticCtx, server_certificates: &[X509]) -> anyhow::Result<()> { + let certificate = server_certificates + .first() + .context("end entity certificate is missing")?; + let cert_der = certificate.to_der().context("failed to encode certificate as DER")?; + + info!("Check for Subject Alternative Name extension"); + + let has_san = crate::doctor::cert_inspect::cert_has_san_extension(&cert_der)?; + + if !has_san { + ctx.attach_warning( + "when TlsVerifyStrict is enabled in the Devolutions Gateway configuration, this certificate will be rejected" + .to_owned(), + ); + help::cert_missing_san(ctx); + anyhow::bail!("the end entity certificate is missing the Subject Alternative Name (SAN) extension"); + } + + info!("Subject Alternative Name extension found"); + + Ok(()) + } + + fn openssl_check_server_auth_eku(ctx: &mut DiagnosticCtx, server_certificates: &[X509]) -> anyhow::Result<()> { + let certificate = server_certificates + .first() + .context("end entity certificate is missing")?; + let cert_der = certificate.to_der().context("failed to encode certificate as DER")?; + + info!("Check for serverAuth Extended Key Usage"); + + let has_server_auth = crate::doctor::cert_inspect::cert_has_server_auth_eku(&cert_der)?; + + if !has_server_auth { + ctx.attach_warning( + "when TlsVerifyStrict is enabled in the Devolutions Gateway configuration, this certificate will be rejected" + .to_owned(), + ); + help::cert_missing_server_auth_eku(ctx); + anyhow::bail!("the end entity certificate does not include the serverAuth Extended Key Usage (EKU)"); + } + + info!("serverAuth Extended Key Usage found"); + + Ok(()) + } + impl InspectCert for X509 { fn der(&self) -> anyhow::Result> { let der = self.to_der()?; diff --git a/jetsocat/src/doctor/rustls.rs b/jetsocat/src/doctor/rustls.rs index 28dd4bb9d..28a74c551 100644 --- a/jetsocat/src/doctor/rustls.rs +++ b/jetsocat/src/doctor/rustls.rs @@ -31,6 +31,8 @@ pub(super) fn run(args: &Args, callback: &mut dyn FnMut(Diagnostic) -> bool) { rustls_check_end_entity_cert(&server_certificates, args.subject_name.as_deref()) ); diagnostic!(callback, rustls_check_chain(&root_store, &server_certificates)); + diagnostic!(callback, rustls_check_san_extension(&server_certificates)); + diagnostic!(callback, rustls_check_server_auth_eku(&server_certificates)); } } @@ -206,6 +208,54 @@ fn rustls_check_chain( Ok(()) } +fn rustls_check_san_extension( + ctx: &mut DiagnosticCtx, + server_certificates: &[pki_types::CertificateDer<'static>], +) -> anyhow::Result<()> { + let end_entity_cert = server_certificates.first().context("empty chain")?; + + info!("Check for Subject Alternative Name extension"); + + let has_san = crate::doctor::cert_inspect::cert_has_san_extension(end_entity_cert)?; + + if !has_san { + ctx.attach_warning( + "when TlsVerifyStrict is enabled in the Devolutions Gateway configuration, this certificate will be rejected" + .to_owned(), + ); + help::cert_missing_san(ctx); + anyhow::bail!("the end entity certificate is missing the Subject Alternative Name (SAN) extension"); + } + + info!("Subject Alternative Name extension found"); + + Ok(()) +} + +fn rustls_check_server_auth_eku( + ctx: &mut DiagnosticCtx, + server_certificates: &[pki_types::CertificateDer<'static>], +) -> anyhow::Result<()> { + let end_entity_cert = server_certificates.first().context("empty chain")?; + + info!("Check for serverAuth Extended Key Usage"); + + let has_server_auth = crate::doctor::cert_inspect::cert_has_server_auth_eku(end_entity_cert)?; + + if !has_server_auth { + ctx.attach_warning( + "when TlsVerifyStrict is enabled in the Devolutions Gateway configuration, this certificate will be rejected" + .to_owned(), + ); + help::cert_missing_server_auth_eku(ctx); + anyhow::bail!("the end entity certificate does not include the serverAuth Extended Key Usage (EKU)"); + } + + info!("serverAuth Extended Key Usage found"); + + Ok(()) +} + impl InspectCert for pki_types::CertificateDer<'_> { fn der(&self) -> anyhow::Result> { Ok(Cow::Borrowed(self)) diff --git a/jetsocat/src/doctor/schannel.rs b/jetsocat/src/doctor/schannel.rs index ede0616dd..c15bb8776 100644 --- a/jetsocat/src/doctor/schannel.rs +++ b/jetsocat/src/doctor/schannel.rs @@ -12,50 +12,60 @@ use wrapper::ScopeGuard; use crate::doctor::macros::diagnostic; use crate::doctor::{Args, CertInspectProxy, Diagnostic, DiagnosticCtx, help}; -struct ChainCtx { - store: wrapper::CertStore, - end_entity_info: wrapper::CertInfo, -} - pub(super) fn run(args: &Args, callback: &mut dyn FnMut(Diagnostic) -> bool) { - let mut chain_ctx = None; + let mut store = None; + + diagnostic!(callback, schannel_open_in_memory_cert_store(&mut store)); - diagnostic!(callback, schannel_open_in_memory_cert_store(&mut chain_ctx)); + let Some(mut store) = store else { return }; - let Some(mut chain_ctx) = chain_ctx else { return }; + let mut end_entity_info = None; if let Some(chain_path) = &args.chain_path { - diagnostic!(callback, schannel_read_chain(&chain_path, &mut chain_ctx)); + diagnostic!( + callback, + schannel_read_chain(chain_path, &mut store, &mut end_entity_info) + ); } else if let Some(subject_name) = args.subject_name.as_deref() && args.allow_network { diagnostic!( callback, - schannel_fetch_chain(&mut chain_ctx, subject_name, args.server_port) + schannel_fetch_chain(&mut store, &mut end_entity_info, subject_name, args.server_port) ); } + let Some(end_entity_info) = &end_entity_info else { + return; + }; + if let Some(subject_name) = args.subject_name.as_deref() { - diagnostic!(callback, schannel_check_end_entity_cert(&chain_ctx, subject_name)); + diagnostic!( + callback, + schannel_check_end_entity_cert(&store, end_entity_info, subject_name) + ); } - diagnostic!(callback, schannel_check_chain(&chain_ctx)); + diagnostic!(callback, schannel_check_chain(&store, end_entity_info)); + diagnostic!(callback, schannel_check_san_extension(&store, end_entity_info)); + diagnostic!(callback, schannel_check_server_auth_eku(&store, end_entity_info)); } -fn schannel_open_in_memory_cert_store(_: &mut DiagnosticCtx, chain_ctx: &mut Option) -> anyhow::Result<()> { +fn schannel_open_in_memory_cert_store( + _: &mut DiagnosticCtx, + store: &mut Option, +) -> anyhow::Result<()> { let opened = wrapper::CertStore::open_in_memory().context("failed to open in-memory certificate store")?; - *chain_ctx = Some(ChainCtx { - store: opened, - end_entity_info: wrapper::CertInfo::default(), - }); + *store = Some(opened); Ok(()) } fn schannel_fetch_chain( ctx: &mut DiagnosticCtx, - chain_ctx: &mut ChainCtx, + store: &mut wrapper::CertStore, + end_entity_info: &mut Option, subject_name: &str, port: Option, ) -> anyhow::Result<()> { @@ -304,8 +314,8 @@ fn schannel_fetch_chain( let remote_end_entity_cert = wrapper::CertContext::schannel_remote_cert(ctx_handle.as_ref()).context("failed to retrieve remote cert")?; - // Update the end entity info of the chain context. - chain_ctx.end_entity_info = remote_end_entity_cert.to_info(); + // Update the end entity info. + *end_entity_info = Some(remote_end_entity_cert.to_info()); let remote_chain = remote_end_entity_cert .chain() @@ -315,7 +325,7 @@ fn schannel_fetch_chain( let mut warning_messages = Vec::new(); remote_chain.for_each(|cert_idx, element| { - if let Err(error) = chain_ctx.store.add_x509_encoded_certificate(element.cert.as_x509_der()) { + if let Err(error) = store.add_x509_encoded_certificate(element.cert.as_x509_der()) { let error_msg = format!("failed to add certificate {cert_idx} to the store: {error}"); warn!("{}", error_msg); warning_messages.push(error_msg); @@ -359,7 +369,12 @@ fn schannel_fetch_chain( } } -fn schannel_read_chain(ctx: &mut DiagnosticCtx, chain_path: &Path, chain_ctx: &mut ChainCtx) -> anyhow::Result<()> { +fn schannel_read_chain( + ctx: &mut DiagnosticCtx, + chain_path: &Path, + store: &mut wrapper::CertStore, + end_entity_info: &mut Option, +) -> anyhow::Result<()> { info!("Read file at {}", chain_path.display()); let mut file = std::fs::File::open(chain_path) @@ -371,17 +386,23 @@ fn schannel_read_chain(ctx: &mut DiagnosticCtx, chain_path: &Path, chain_ctx: &m for (idx, cert_der) in rustls_pemfile::certs(&mut file).enumerate() { let cert_der = cert_der.with_context(|| format!("failed to read certificate number {idx}"))?; - let cert_ctx = chain_ctx - .store + let cert_ctx = store .add_x509_encoded_certificate(&cert_der) .with_context(|| format!("failed to add certificate number {idx} to the store"))?; + // The first certificate in the PEM chain is the end entity (leaf) certificate. + if idx == 0 { + *end_entity_info = Some(cert_ctx.to_info()); + } + certificates.push(CertInspectProxy { friendly_name: cert_ctx.subject_friendly_name().ok(), der: cert_ctx.as_x509_der().to_owned(), }); } + anyhow::ensure!(!certificates.is_empty(), "no certificate found in the chain file"); + crate::doctor::log_chain(certificates.iter()); help::x509_io_link(ctx, certificates.iter()); @@ -390,12 +411,12 @@ fn schannel_read_chain(ctx: &mut DiagnosticCtx, chain_path: &Path, chain_ctx: &m fn schannel_check_end_entity_cert( ctx: &mut DiagnosticCtx, - chain_ctx: &ChainCtx, + store: &wrapper::CertStore, + end_entity_info: &wrapper::CertInfo, subject_name_to_verify: &str, ) -> anyhow::Result<()> { - let end_entity_cert = chain_ctx - .store - .fetch_certificate(&chain_ctx.end_entity_info) + let end_entity_cert = store + .fetch_certificate(end_entity_info) .context("failed to fetch end entity cert from in-memory store")?; info!("Inspect the end entity certificate"); @@ -426,10 +447,13 @@ fn schannel_check_end_entity_cert( Ok(()) } -fn schannel_check_chain(ctx: &mut DiagnosticCtx, chain_ctx: &ChainCtx) -> anyhow::Result<()> { - let end_entity_cert = chain_ctx - .store - .fetch_certificate(&chain_ctx.end_entity_info) +fn schannel_check_chain( + ctx: &mut DiagnosticCtx, + store: &wrapper::CertStore, + end_entity_info: &wrapper::CertInfo, +) -> anyhow::Result<()> { + let end_entity_cert = store + .fetch_certificate(end_entity_info) .context("failed to fetch end entity cert from in-memory store")?; let chain = end_entity_cert.chain().context("failed to get certificate chain")?; @@ -538,6 +562,64 @@ fn schannel_check_chain(ctx: &mut DiagnosticCtx, chain_ctx: &ChainCtx) -> anyhow } } +fn schannel_check_san_extension( + ctx: &mut DiagnosticCtx, + store: &wrapper::CertStore, + end_entity_info: &wrapper::CertInfo, +) -> anyhow::Result<()> { + let end_entity_cert = store + .fetch_certificate(end_entity_info) + .context("failed to fetch end entity cert from in-memory store")?; + + let cert_der = end_entity_cert.as_x509_der(); + + info!("Check for Subject Alternative Name extension"); + + let has_san = crate::doctor::cert_inspect::cert_has_san_extension(cert_der)?; + + if !has_san { + ctx.attach_warning( + "when TlsVerifyStrict is enabled in the Devolutions Gateway configuration, this certificate will be rejected" + .to_owned(), + ); + help::cert_missing_san(ctx); + anyhow::bail!("the end entity certificate is missing the Subject Alternative Name (SAN) extension"); + } + + info!("Subject Alternative Name extension found"); + + Ok(()) +} + +fn schannel_check_server_auth_eku( + ctx: &mut DiagnosticCtx, + store: &wrapper::CertStore, + end_entity_info: &wrapper::CertInfo, +) -> anyhow::Result<()> { + let end_entity_cert = store + .fetch_certificate(end_entity_info) + .context("failed to fetch end entity cert from in-memory store")?; + + let cert_der = end_entity_cert.as_x509_der(); + + info!("Check for serverAuth Extended Key Usage"); + + let has_server_auth = crate::doctor::cert_inspect::cert_has_server_auth_eku(cert_der)?; + + if !has_server_auth { + ctx.attach_warning( + "when TlsVerifyStrict is enabled in the Devolutions Gateway configuration, this certificate will be rejected" + .to_owned(), + ); + help::cert_missing_server_auth_eku(ctx); + anyhow::bail!("the end entity certificate does not include the serverAuth Extended Key Usage (EKU)"); + } + + info!("serverAuth Extended Key Usage found"); + + Ok(()) +} + mod wrapper { use super::*; @@ -955,7 +1037,6 @@ mod wrapper { pub trust_status: Cryptography::CERT_TRUST_STATUS, } - #[derive(Default)] pub(super) struct CertInfo { issuer: Vec, serial: Vec, diff --git a/testsuite/tests/cli/jetsocat.rs b/testsuite/tests/cli/jetsocat.rs index 97f2436d6..247d54912 100644 --- a/testsuite/tests/cli/jetsocat.rs +++ b/testsuite/tests/cli/jetsocat.rs @@ -292,18 +292,19 @@ fn jmux_proxy_write_hello_world() { } #[test] -#[cfg_attr(windows, ignore = "does not pass on Windows")] // FIXME fn doctor_no_args_is_valid() { jetsocat_assert_cmd().arg("doctor").assert().success(); } #[test] -#[cfg_attr(windows, ignore = "does not pass on Windows")] // FIXME fn doctor_verify_chain_with_json_output() { - #[cfg(unix)] - const CHAIN_CHECK_NAME: &str = "rustls_check_chain"; - #[cfg(windows)] - const CHAIN_CHECK_NAME: &str = "schannel_check_chain"; + // Chain checks that are expected to fail because the leaf certificate is expired. + // On Windows, both the rustls and schannel backends run and detect the expiry. + let expected_chain_failures: &[&str] = if cfg!(windows) { + &["rustls_check_chain", "schannel_check_chain"] + } else { + &["rustls_check_chain"] + }; let tempdir = tempfile::tempdir().unwrap(); let chain_file_path = tempdir.path().join("expired-devolutions-net-chain.pem"); @@ -338,6 +339,7 @@ fn doctor_verify_chain_with_json_output() { "name" | "success" => { /* verified above */ } "output" => assert!(value.is_string()), "error" => assert!(value.is_string()), + "warning" => assert!(value.is_string()), "help" => assert!(value.is_string()), "links" => assert!(value.is_array()), @@ -346,12 +348,14 @@ fn doctor_verify_chain_with_json_output() { } } - if entry["name"].as_str().unwrap() == CHAIN_CHECK_NAME { - // Since the leaf certificate is expired, this check should fail. - assert!(!entry["success"].as_bool().unwrap()); + let name = entry["name"].as_str().unwrap(); + + if expected_chain_failures.contains(&name) { + // Since the leaf certificate is expired, chain checks should fail. + assert!(!entry["success"].as_bool().unwrap(), "{name} should have failed"); } else { // All the other checks should succeed. - assert!(entry["success"].as_bool().unwrap()); + assert!(entry["success"].as_bool().unwrap(), "{name} should have succeeded"); } } @@ -456,6 +460,112 @@ CAUw7C29C79Fv1C5qfPrmAESrciIxpg0X40KPMbp1ZWVbd4= -----END CERTIFICATE-----"; } +#[test] +fn doctor_missing_san_and_eku() { + // Checks that are expected to fail because the certificate is missing SAN and EKU. + // The rustls end entity cert check also fails because rustls requires SAN for hostname verification. + // The schannel end entity cert check succeeds because schannel falls back to CN matching. + let expected_failures: &[&str] = if cfg!(windows) { + &[ + "rustls_check_end_entity_cert", + "rustls_check_chain", + "rustls_check_san_extension", + "rustls_check_server_auth_eku", + "schannel_check_chain", + "schannel_check_san_extension", + "schannel_check_server_auth_eku", + ] + } else { + &[ + "rustls_check_end_entity_cert", + "rustls_check_chain", + "rustls_check_san_extension", + "rustls_check_server_auth_eku", + ] + }; + + // Checks expected to carry a warning about TlsVerifyStrict. + let expected_warnings: &[&str] = if cfg!(windows) { + &[ + "rustls_check_san_extension", + "rustls_check_server_auth_eku", + "schannel_check_san_extension", + "schannel_check_server_auth_eku", + ] + } else { + &["rustls_check_san_extension", "rustls_check_server_auth_eku"] + }; + + let tempdir = tempfile::tempdir().unwrap(); + let chain_file_path = tempdir.path().join("no-san-no-eku.pem"); + std::fs::write(&chain_file_path, CERT_WITHOUT_SAN_OR_EKU).unwrap(); + + let output = jetsocat_assert_cmd() + .args([ + "doctor", + "--chain", + chain_file_path.to_str().unwrap(), + "--subject-name", + "test.example.com", + "--format", + "json", + ]) + .assert() + .failure(); + + let stdout = std::str::from_utf8(&output.get_output().stdout).unwrap(); + + for line in stdout.lines() { + let entry: serde_json::Value = serde_json::from_str(line).unwrap(); + + let name = entry["name"].as_str().unwrap(); + let success = entry["success"].as_bool().unwrap(); + + if expected_failures.contains(&name) { + assert!(!success, "{name} should have failed"); + } else { + assert!(success, "{name} should have succeeded"); + } + + if expected_warnings.contains(&name) { + assert!( + entry["warning"].is_string(), + "{name} should have a warning about TlsVerifyStrict" + ); + let warning = entry["warning"].as_str().unwrap(); + assert!( + warning.contains("TlsVerifyStrict"), + "{name}: warning should mention TlsVerifyStrict, got: {warning}" + ); + } + } + + // This certificate is a basic self-signed cert generated with: + // openssl req -x509 -newkey rsa:2048 -keyout /dev/null -nodes -subj "/CN=test.example.com" -days 3650 + // It has no Subject Alternative Name (SAN) extension and no Extended Key Usage (EKU) extension. + // The doctor should report warnings for both missing properties. + const CERT_WITHOUT_SAN_OR_EKU: &str = " +-----BEGIN CERTIFICATE----- +MIIDFzCCAf+gAwIBAgIUfFwD6PaeQUaW/b9RAunIbruFiOMwDQYJKoZIhvcNAQEL +BQAwGzEZMBcGA1UEAwwQdGVzdC5leGFtcGxlLmNvbTAeFw0yNjAzMTkwMjQ4MTda +Fw0zNjAzMTYwMjQ4MTdaMBsxGTAXBgNVBAMMEHRlc3QuZXhhbXBsZS5jb20wggEi +MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCVm3m0BFIhYJmxs1yt+QhHVz+T +YdJlY7tsJmaqR9mtV5BW3rLRL1W+dQJ3cSuBLAtE6BWWhec/guRMxR2nwsJ+8GFo +G83AE+ulehST+Ymh7LTTD4jHex6bZM0L132wWUkskkFhYzqQiXHBqKkuAmmcOe9l +gy+tvV4nIlta2vKC7U+1W3cixJS1Anu/BOvy08XctUwRtAxQsSbvFawE1sqGYD95 +sGNraE3w74kc0jRsldv0zGclsW463GQhVlHRu/56SW5DGSXdxhsZXKsChEubphZ5 +u7s0OB8XEqyr4kAJVfmFsdr2xavxkuwW6vuO6CDhrV6bZRYgNnFbVKf1HLjDAgMB +AAGjUzBRMB0GA1UdDgQWBBQsVvQ0YMe8qMcT1L/XSV4q78G8lzAfBgNVHSMEGDAW +gBQsVvQ0YMe8qMcT1L/XSV4q78G8lzAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3 +DQEBCwUAA4IBAQAIE5Fy7xFodiOGSSRwhXL90bMa477nONobhF6rdeRaH47H0Sru +Nj0WvwWgv6QYWvMk40xvCGcOJl8ZO18KxrHV3tKAWv92VWhKcSXXYIVJdrEdi5z1 +qRjFhOl8Bk6jlUomjk2CwbaBjxZctUSM/bpc+szOipSPf7VYA340iWVpb1frmZMW +Oz1dDMCILaSldUlmPXL9g5VntW6Rr7zfLqyeUwq0BV22O9l349Kbu3i9EifWerAf +D7Evd6eXm50umoqlchupHZFRmIJCiHrg7vWwXdJQtgP8zYqh7uZIIbHsLHBJAlR3 +4p6zIygy/wRS/nQb/Y+kFRN+uRdfVB7eftRJ +-----END CERTIFICATE-----"; +} + #[test] fn doctor_invalid_server_port() { let output = jetsocat_assert_cmd()