Skip to content

Commit 1d8465f

Browse files
nathanwhitbotnathanwhitbartlomieju
authored
fix(ext/node): improve node:tls test compatibility (#34067)
## Summary - Improves `node:tls` CA certificate handling, including default CA overrides, system CA flags, CA list validation/deduping, and `getCACertificates()` default/system/bundled/extra behavior. - Adds SecureContext CA mutation support and verifier error tracking so explicit CA options and `rejectUnauthorized: false` align more closely with Node. - Fills selected TLS compatibility gaps for protocol-method errors, ALPN `setKeyCert()`, ticket key APIs, and related TLS option plumbing. - Enables newly passing TLS compatibility tests in `config.jsonc`. --------- Co-authored-by: Nathan Whitaker <nathan@deno.com> Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
1 parent 1bf75fb commit 1d8465f

17 files changed

Lines changed: 633 additions & 150 deletions

File tree

ext/node/ops/node_cli_parser.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,14 @@ fn translate_to_deno_args(
6666
let use_system_ca = parsed_args.options.use_system_ca;
6767
let use_bundled_ca = parsed_args.options.use_bundled_ca;
6868
let result = translate_to_deno_args_impl(parsed_args, &options);
69-
let ca_stores = match (use_system_ca, use_bundled_ca) {
70-
(true, true) => Some(vec!["system".to_string(), "mozilla".to_string()]),
71-
(true, false) => Some(vec!["system".to_string()]),
72-
(false, true) => Some(vec!["mozilla".to_string()]),
73-
(false, false) => None,
69+
let ca_stores = if use_openssl_ca {
70+
Some(vec!["system".to_string()])
71+
} else {
72+
match (use_system_ca, use_bundled_ca) {
73+
(true, _) => Some(vec!["mozilla".to_string(), "system".to_string()]),
74+
(false, true) => Some(vec!["mozilla".to_string()]),
75+
(false, false) => None,
76+
}
7477
};
7578

7679
TranslatedArgs {

ext/node/ops/tls.rs

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,16 @@ fn parse_extra_ca_certs(sys: &(impl EnvVar + FsRead)) -> Vec<String> {
133133
.collect()
134134
}
135135

136+
fn load_system_ca_certificates() -> Result<Vec<String>, CaCertificatesError> {
137+
let mut certs = load_native_certs()
138+
.map_err(|err| CaCertificatesError::Other(err.to_string()))?
139+
.into_iter()
140+
.map(|cert| cert_der_to_pem(&cert.0))
141+
.collect::<Vec<_>>();
142+
certs.sort_unstable();
143+
Ok(certs)
144+
}
145+
136146
#[derive(Debug, thiserror::Error, deno_error::JsError)]
137147
pub enum CaCertificatesError {
138148
#[class(type)]
@@ -173,14 +183,7 @@ pub fn op_node_get_ca_certificates<TSys: ExtNodeSys + 'static>(
173183
.map(|cert| cert_der_to_pem(cert))
174184
.collect(),
175185
),
176-
"system" => load_native_certs()
177-
.map(|roots| {
178-
roots
179-
.into_iter()
180-
.map(|cert| cert_der_to_pem(&cert.0))
181-
.collect()
182-
})
183-
.map_err(|err| CaCertificatesError::Other(err.to_string())),
186+
"system" => load_system_ca_certificates(),
184187
"extra" => Ok(parse_extra_ca_certs(sys)),
185188
"default" => {
186189
let mut certs = Vec::new();
@@ -201,12 +204,7 @@ pub fn op_node_get_ca_certificates<TSys: ExtNodeSys + 'static>(
201204
);
202205
}
203206
if stores.contains(&"system") {
204-
certs.extend(
205-
load_native_certs()
206-
.map_err(|err| CaCertificatesError::Other(err.to_string()))?
207-
.into_iter()
208-
.map(|cert| cert_der_to_pem(&cert.0)),
209-
);
207+
certs.extend(load_system_ca_certificates()?);
210208
}
211209
certs.extend(parse_extra_ca_certs(sys));
212210
Ok(certs)
@@ -220,19 +218,13 @@ pub fn op_set_default_ca_certificates(
220218
state: &mut OpState,
221219
#[serde] certs: Vec<String>,
222220
) {
223-
// Treat `setDefaultCACertificates([])` as "use defaults" (None) rather
224-
// than "use no custom CAs" (Some(vec![])). The two are semantically
225-
// identical for cert validation, but `Some(vec![])` would force the
226-
// default-path verifier cache off in `build_client_config` and silently
227-
// disable session resumption.
228-
let normalized = if certs.is_empty() { None } else { Some(certs) };
229221
if let Some(tls_state) = state.try_borrow_mut::<NodeTlsState>() {
230-
tls_state.custom_ca_certs = normalized;
222+
tls_state.custom_ca_certs = Some(certs);
231223
// Custom CA list changed; previously cached verifier no longer matches.
232224
tls_state.cached_default_verifier = None;
233225
} else {
234226
state.put(NodeTlsState {
235-
custom_ca_certs: normalized,
227+
custom_ca_certs: Some(certs),
236228
client_session_store: Arc::new(
237229
deno_tls::rustls::client::ClientSessionMemoryCache::new(256),
238230
),

ext/node/ops/tls_wrap.rs

Lines changed: 63 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -872,29 +872,10 @@ fn rustls_error_to_node_error(
872872
format!("{e} (wrong version number)"),
873873
"ERR_SSL_WRONG_VERSION_NUMBER".to_string(),
874874
),
875-
E::InvalidCertificate(cert_err) => {
876-
let reason = format!("{cert_err}");
877-
// Map common rustls certificate errors to OpenSSL error codes
878-
let code = if reason.contains("UnknownIssuer") {
879-
"UNABLE_TO_VERIFY_LEAF_SIGNATURE"
880-
} else if reason.contains("NotValidYet") {
881-
"CERT_NOT_YET_VALID"
882-
} else if reason.contains("Expired") {
883-
"CERT_HAS_EXPIRED"
884-
} else if reason.contains("NotValidForName") {
885-
"ERR_TLS_CERT_ALTNAME_INVALID"
886-
} else if reason.contains("CaUsedAsEndEntity")
887-
|| reason.contains("IssuerNotCrlSigner")
888-
|| reason.contains("InvalidPurpose")
889-
{
890-
"UNABLE_TO_VERIFY_LEAF_SIGNATURE"
891-
} else if reason.contains("SelfSigned") {
892-
"DEPTH_ZERO_SELF_SIGNED_CERT"
893-
} else {
894-
"ERR_SSL_SSLV3_ALERT_CERTIFICATE_UNKNOWN"
895-
};
896-
(format!("{e}"), format!("ERR_SSL_{code}"))
897-
}
875+
E::InvalidCertificate(cert_err) => (
876+
format!("{e}"),
877+
cert_error_to_node_code(cert_err).to_string(),
878+
),
898879
E::NoCertificatesPresented => (
899880
format!("{e}"),
900881
"ERR_SSL_PEER_DID_NOT_RETURN_A_CERTIFICATE".to_string(),
@@ -3100,6 +3081,11 @@ fn store_verify_error(fallback: &VerifyErrorStore, code: String) {
31003081
struct NodeServerCertVerifier {
31013082
inner: Arc<rustls::client::WebPkiServerVerifier>,
31023083
verify_error: VerifyErrorStore,
3084+
/// True for an explicit `ca: []`. rustls/webpki cannot build its verifier
3085+
/// with no roots, so `inner` uses fallback roots only to classify
3086+
/// certificate-specific failures. Otherwise-valid chains are still recorded
3087+
/// as unauthorized below.
3088+
empty_explicit_ca: bool,
31033089
/// Raw DER bytes of every root certificate so we can check whether a
31043090
/// `CaUsedAsEndEntity` cert is actually trusted.
31053091
root_cert_ders: Vec<Vec<u8>>,
@@ -3286,6 +3272,7 @@ fn verify_chain_structure(
32863272

32873273
// Walk the chain from end entity upward.
32883274
let mut current_issuer = ee.0;
3275+
let end_entity_subject = ee.1;
32893276

32903277
// Limit iterations to prevent cycles.
32913278
for _ in 0..(intermediates.len() + 2) {
@@ -3308,13 +3295,12 @@ fn verify_chain_structure(
33083295
}
33093296

33103297
// Chain doesn't reach a trusted root.
3311-
if root_cert_ders.is_empty() {
3312-
// No explicit CA was provided (only system/default roots which
3313-
// didn't match). OpenSSL reports this as "unable to get local
3314-
// issuer certificate".
3315-
Err("UNABLE_TO_GET_ISSUER_CERT_LOCALLY")
3316-
} else {
3298+
if current_issuer == end_entity_subject {
3299+
Err("DEPTH_ZERO_SELF_SIGNED_CERT")
3300+
} else if intermediates.is_empty() {
33173301
Err("UNABLE_TO_VERIFY_LEAF_SIGNATURE")
3302+
} else {
3303+
Err("UNABLE_TO_GET_ISSUER_CERT_LOCALLY")
33183304
}
33193305
}
33203306

@@ -3361,7 +3347,21 @@ impl rustls::client::danger::ServerCertVerifier for NodeServerCertVerifier {
33613347
ocsp,
33623348
now,
33633349
) {
3364-
Ok(v) => Ok(v),
3350+
Ok(v) => {
3351+
if self.empty_explicit_ca {
3352+
let code = verify_chain_structure(
3353+
end_entity.as_ref(),
3354+
intermediates,
3355+
&self.root_cert_ders,
3356+
)
3357+
.err()
3358+
.unwrap_or("UNABLE_TO_VERIFY_LEAF_SIGNATURE");
3359+
store_verify_error(&self.verify_error, code.to_string());
3360+
Ok(rustls::client::danger::ServerCertVerified::assertion())
3361+
} else {
3362+
Ok(v)
3363+
}
3364+
}
33653365
Err(rustls::Error::InvalidCertificate(ref cert_error)) => {
33663366
// Server-name checks are handled by JS (checkServerIdentity).
33673367
if matches!(
@@ -3409,6 +3409,17 @@ impl rustls::client::danger::ServerCertVerifier for NodeServerCertVerifier {
34093409
}
34103410
}
34113411
}
3412+
if matches!(cert_error, rustls::CertificateError::UnknownIssuer) {
3413+
let code = verify_chain_structure(
3414+
end_entity.as_ref(),
3415+
intermediates,
3416+
&self.root_cert_ders,
3417+
)
3418+
.err()
3419+
.unwrap_or("UNABLE_TO_VERIFY_LEAF_SIGNATURE");
3420+
store_verify_error(&self.verify_error, code.to_string());
3421+
return Ok(rustls::client::danger::ServerCertVerified::assertion());
3422+
}
34123423
if let rustls::CertificateError::Other(other) = cert_error
34133424
&& let Some(webpki_err) = other.0.downcast_ref::<webpki::Error>()
34143425
{
@@ -3524,6 +3535,7 @@ fn build_client_config(
35243535

35253536
let _reject_unauthorized =
35263537
get_js_bool(scope, context, "rejectUnauthorized", true);
3538+
let use_default_ca = get_js_bool(scope, context, "useDefaultCA", true);
35273539
let protocol_versions = match get_protocol_versions(scope, context) {
35283540
ProtocolVersionSelection::Default => {
35293541
&[&rustls::version::TLS13, &rustls::version::TLS12][..]
@@ -3560,17 +3572,18 @@ fn build_client_config(
35603572
.flatten();
35613573

35623574
// Use custom CA certs from setDefaultCACertificates() only when the
3563-
// SecureContext doesn't provide its own CA. This matches Node.js
3564-
// behavior where explicit `ca` in options takes precedence.
3565-
if ca_certs.is_empty()
3575+
// SecureContext is on the default CA path. Explicit `ca` replaces the
3576+
// root store, while context.addCACert() extends whatever default CA store
3577+
// is active.
3578+
if use_default_ca
35663579
&& let Some(node_tls_state) = op_state.try_borrow::<NodeTlsState>()
35673580
&& let Some(custom_ca_certs) = &node_tls_state.custom_ca_certs
35683581
{
35693582
root_cert_store = Some(rustls::RootCertStore::empty());
3570-
ca_certs = custom_ca_certs
3571-
.iter()
3572-
.map(|cert| cert.clone().into_bytes())
3573-
.collect();
3583+
ca_certs
3584+
.extend(custom_ca_certs.iter().map(|cert| cert.clone().into_bytes()));
3585+
} else if !use_default_ca {
3586+
root_cert_store = Some(rustls::RootCertStore::empty());
35743587
}
35753588

35763589
// Build client key/cert if provided
@@ -3602,6 +3615,7 @@ fn build_client_config(
36023615
// every TLS connection without explicit CA options to fail verification.
36033616
let mut root_cert_store =
36043617
root_cert_store.unwrap_or_else(deno_tls::create_default_root_cert_store);
3618+
let empty_explicit_ca = !use_default_ca && ca_certs.is_empty();
36053619

36063620
// Collect raw DER bytes of root certs so NodeServerCertVerifier can
36073621
// check CaUsedAsEndEntity certs against the trust store.
@@ -3637,6 +3651,7 @@ fn build_client_config(
36373651
// same `Arc`s and session resumption is allowed to proceed (rustls keys
36383652
// its `compatible_config` check on `Arc::downgrade(&verifier)` identity).
36393653
let is_default_path = ca_certs.is_empty()
3654+
&& use_default_ca
36403655
&& op_state
36413656
.try_borrow::<NodeTlsState>()
36423657
.is_none_or(|s| s.custom_ca_certs.is_none())
@@ -3713,6 +3728,7 @@ fn build_client_config(
37133728
Arc::new(NodeServerCertVerifier {
37143729
inner,
37153730
verify_error: store.clone(),
3731+
empty_explicit_ca: false,
37163732
root_cert_ders,
37173733
});
37183734
state.cached_default_verifier = Some((v.clone(), store.clone()));
@@ -3723,14 +3739,21 @@ fn build_client_config(
37233739
}
37243740
} else {
37253741
let store: VerifyErrorStore = Default::default();
3726-
let verifier_result =
3727-
rustls::client::WebPkiServerVerifier::builder(Arc::new(root_cert_store))
3728-
.build();
3742+
let verifier_root_store = if empty_explicit_ca {
3743+
deno_tls::create_default_root_cert_store()
3744+
} else {
3745+
root_cert_store.clone()
3746+
};
3747+
let verifier_result = rustls::client::WebPkiServerVerifier::builder(
3748+
Arc::new(verifier_root_store),
3749+
)
3750+
.build();
37293751
let v: Option<Arc<dyn rustls::client::danger::ServerCertVerifier>> =
37303752
verifier_result.ok().map(|inner| {
37313753
Arc::new(NodeServerCertVerifier {
37323754
inner,
37333755
verify_error: store.clone(),
3756+
empty_explicit_ca,
37343757
root_cert_ders,
37353758
}) as Arc<dyn rustls::client::danger::ServerCertVerifier>
37363759
});

0 commit comments

Comments
 (0)