From 752aa2693ea22b9aec5972021d99d6bd8625c894 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 19 Mar 2026 11:02:45 +0900 Subject: [PATCH 1/3] feat(jetsocat): add SAN and EKU diagnostics to doctor module Add certificate extension checks mirroring the gateway's TlsVerifyStrict validation. Each TLS backend (rustls, openssl, schannel) now runs two new diagnostics on the end-entity certificate: - check_san_extension: verifies the Subject Alternative Name extension is present - check_server_auth_eku: verifies the Extended Key Usage includes serverAuth Issue: DGW-350 --- jetsocat/src/doctor/cert_inspect.rs | 192 ++++++++++++++++++++++++++++ jetsocat/src/doctor/help.rs | 20 +++ jetsocat/src/doctor/mod.rs | 1 + jetsocat/src/doctor/native_tls.rs | 50 ++++++++ jetsocat/src/doctor/rustls.rs | 50 ++++++++ jetsocat/src/doctor/schannel.rs | 54 ++++++++ 6 files changed, 367 insertions(+) create mode 100644 jetsocat/src/doctor/cert_inspect.rs 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..0d1cba762 100644 --- a/jetsocat/src/doctor/schannel.rs +++ b/jetsocat/src/doctor/schannel.rs @@ -40,6 +40,8 @@ pub(super) fn run(args: &Args, callback: &mut dyn FnMut(Diagnostic) -> bool) { } diagnostic!(callback, schannel_check_chain(&chain_ctx)); + diagnostic!(callback, schannel_check_san_extension(&chain_ctx)); + diagnostic!(callback, schannel_check_server_auth_eku(&chain_ctx)); } fn schannel_open_in_memory_cert_store(_: &mut DiagnosticCtx, chain_ctx: &mut Option) -> anyhow::Result<()> { @@ -538,6 +540,58 @@ fn schannel_check_chain(ctx: &mut DiagnosticCtx, chain_ctx: &ChainCtx) -> anyhow } } +fn schannel_check_san_extension(ctx: &mut DiagnosticCtx, chain_ctx: &ChainCtx) -> anyhow::Result<()> { + let end_entity_cert = chain_ctx + .store + .fetch_certificate(&chain_ctx.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, chain_ctx: &ChainCtx) -> anyhow::Result<()> { + let end_entity_cert = chain_ctx + .store + .fetch_certificate(&chain_ctx.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::*; From caacdf146aebb8d25625e8af6a88f73f4c544a91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 19 Mar 2026 11:41:37 +0900 Subject: [PATCH 2/3] fix(jetsocat): fix schannel doctor backend Set end_entity_info in schannel_read_chain for the leaf certificate, so that cert checks work when loading a chain from a PEM file. Remove ChainCtx struct in favor of separate store and end_entity_info locals, using the same Option-unwrap pattern already established for the store. This naturally gates cert checks on chain availability and makes schannel_read_chain fail explicitly when the chain file contains no certificates. Remove the #[cfg_attr(windows, ignore)] attributes from doctor tests. --- jetsocat/src/doctor/schannel.rs | 113 ++++++++++++++++++++------------ testsuite/tests/cli/jetsocat.rs | 24 ++++--- 2 files changed, 84 insertions(+), 53 deletions(-) diff --git a/jetsocat/src/doctor/schannel.rs b/jetsocat/src/doctor/schannel.rs index 0d1cba762..c15bb8776 100644 --- a/jetsocat/src/doctor/schannel.rs +++ b/jetsocat/src/doctor/schannel.rs @@ -12,52 +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_san_extension(&chain_ctx)); - diagnostic!(callback, schannel_check_server_auth_eku(&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<()> { @@ -306,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() @@ -317,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); @@ -361,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) @@ -373,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()); @@ -392,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"); @@ -428,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")?; @@ -540,10 +562,13 @@ fn schannel_check_chain(ctx: &mut DiagnosticCtx, chain_ctx: &ChainCtx) -> anyhow } } -fn schannel_check_san_extension(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_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(); @@ -566,10 +591,13 @@ fn schannel_check_san_extension(ctx: &mut DiagnosticCtx, chain_ctx: &ChainCtx) - Ok(()) } -fn schannel_check_server_auth_eku(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_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(); @@ -1009,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..2882d6cbe 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"); } } From a9689501d156287ab286a7d7d4ffd20dc9e3745f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 19 Mar 2026 11:58:25 +0900 Subject: [PATCH 3/3] test(jetsocat): add test for missing SAN and EKU diagnostics Adds a doctor test using a self-signed certificate that has no Subject Alternative Name extension and no Extended Key Usage extension. Verifies that the SAN and EKU checks fail with TlsVerifyStrict warnings on all backends, while unrelated checks still succeed. --- testsuite/tests/cli/jetsocat.rs | 106 ++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/testsuite/tests/cli/jetsocat.rs b/testsuite/tests/cli/jetsocat.rs index 2882d6cbe..247d54912 100644 --- a/testsuite/tests/cli/jetsocat.rs +++ b/testsuite/tests/cli/jetsocat.rs @@ -460,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()