From 214a1d3e2dbe4ad253f68ca767ed78fa3f724b18 Mon Sep 17 00:00:00 2001 From: link2xt Date: Thu, 16 Oct 2025 23:52:20 +0000 Subject: [PATCH] feat: do not resolve MX records during configuration MX record lookup was only used to detect Google Workspace domains. They can still be configured manually. We anyway do not want to encourage creating new profiles with Google Workspace as we don't have Gmail OAUTH2 token anymore and new users can more easily onboard with a chatmail relay. --- Cargo.lock | 1 - Cargo.toml | 1 - deltachat-ffi/deltachat.h | 7 +- deltachat-ffi/src/lib.rs | 34 ++--- deltachat-jsonrpc/src/api.rs | 15 +- deltachat-repl/src/cmdline.rs | 5 +- deltachat-rpc-client/tests/test_something.py | 5 +- src/configure.rs | 4 +- src/oauth2.rs | 27 ++-- src/provider.rs | 149 ++----------------- src/sql/migrations.rs | 4 +- 11 files changed, 47 insertions(+), 205 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 82e9cffec..d574d22f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1316,7 +1316,6 @@ dependencies = [ "futures", "futures-lite", "hex", - "hickory-resolver", "http-body-util", "humansize", "hyper", diff --git a/Cargo.toml b/Cargo.toml index f25f16678..43bd54326 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -61,7 +61,6 @@ fd-lock = "4" futures-lite = { workspace = true } futures = { workspace = true } hex = "0.4.0" -hickory-resolver = "0.25.2" http-body-util = "0.1.3" humansize = "2" hyper = "1" diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 053f5fc12..581a7df2b 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -5350,11 +5350,9 @@ dc_provider_t* dc_provider_new_from_email (const dc_context_t* conte /** - * Create a provider struct for the given e-mail address by local and DNS lookup. + * Create a provider struct for the given e-mail address by local lookup. * - * First lookup is done from the local database as of dc_provider_new_from_email(). - * If the first lookup fails, an additional DNS lookup is done, - * trying to figure out the provider belonging to custom domains. + * DNS lookup is not used anymore and this function is deprecated. * * @memberof dc_provider_t * @param context The context object. @@ -5362,6 +5360,7 @@ dc_provider_t* dc_provider_new_from_email (const dc_context_t* conte * @return A dc_provider_t struct which can be used with the dc_provider_get_* * accessor functions. If no provider info is found, NULL will be * returned. + * @deprecated 2025-10-17 use dc_provider_new_from_email() instead. */ dc_provider_t* dc_provider_new_from_email_with_dns (const dc_context_t* context, const char* email); diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 96157e4bb..ae32bd169 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -4661,13 +4661,9 @@ pub unsafe extern "C" fn dc_provider_new_from_email( let ctx = &*context; - match block_on(provider::get_provider_info_by_addr( - ctx, - addr.as_str(), - true, - )) - .log_err(ctx) - .unwrap_or_default() + match provider::get_provider_info_by_addr(addr.as_str()) + .log_err(ctx) + .unwrap_or_default() { Some(provider) => provider, None => ptr::null_mut(), @@ -4686,25 +4682,13 @@ pub unsafe extern "C" fn dc_provider_new_from_email_with_dns( let addr = to_string_lossy(addr); let ctx = &*context; - let proxy_enabled = block_on(ctx.get_config_bool(config::Config::ProxyEnabled)) - .context("Can't get config") - .log_err(ctx); - match proxy_enabled { - Ok(proxy_enabled) => { - match block_on(provider::get_provider_info_by_addr( - ctx, - addr.as_str(), - proxy_enabled, - )) - .log_err(ctx) - .unwrap_or_default() - { - Some(provider) => provider, - None => ptr::null_mut(), - } - } - Err(_) => ptr::null_mut(), + match provider::get_provider_info_by_addr(addr.as_str()) + .log_err(ctx) + .unwrap_or_default() + { + Some(provider) => provider, + None => ptr::null_mut(), } } diff --git a/deltachat-jsonrpc/src/api.rs b/deltachat-jsonrpc/src/api.rs index bc2a8f285..92c9fe59a 100644 --- a/deltachat-jsonrpc/src/api.rs +++ b/deltachat-jsonrpc/src/api.rs @@ -336,21 +336,10 @@ impl CommandApi { /// instead of the domain. async fn get_provider_info( &self, - account_id: u32, + _account_id: u32, email: String, ) -> Result> { - let ctx = self.get_context(account_id).await?; - - let proxy_enabled = ctx - .get_config_bool(deltachat::config::Config::ProxyEnabled) - .await?; - - let provider_info = get_provider_info( - &ctx, - email.split('@').next_back().unwrap_or(""), - proxy_enabled, - ) - .await; + let provider_info = get_provider_info(email.split('@').next_back().unwrap_or("")); Ok(ProviderInfo::from_dc_type(provider_info)) } diff --git a/deltachat-repl/src/cmdline.rs b/deltachat-repl/src/cmdline.rs index b8db71e41..9ea813e2d 100644 --- a/deltachat-repl/src/cmdline.rs +++ b/deltachat-repl/src/cmdline.rs @@ -1266,10 +1266,7 @@ pub async fn cmdline(context: Context, line: &str, chat_id: &mut ChatId) -> Resu } "providerinfo" => { ensure!(!arg1.is_empty(), "Argument missing."); - let proxy_enabled = context - .get_config_bool(config::Config::ProxyEnabled) - .await?; - match provider::get_provider_info(&context, arg1, proxy_enabled).await { + match provider::get_provider_info(arg1) { Some(info) => { println!("Information for provider belonging to {arg1}:"); println!("status: {}", info.status as u32); diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index f7c144a4a..7f437f14a 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -570,8 +570,11 @@ def test_provider_info(rpc) -> None: assert provider_info is None # Test MX record resolution. + # This previously resulted in Gmail provider + # because MX record pointed to google.com domain, + # but MX record resolution has been removed. provider_info = rpc.get_provider_info(account_id, "github.com") - assert provider_info["id"] == "gmail" + assert provider_info is None # Disable MX record resolution. rpc.set_config(account_id, "proxy_enabled", "1") diff --git a/src/configure.rs b/src/configure.rs index 31d81200b..91aff2c5c 100644 --- a/src/configure.rs +++ b/src/configure.rs @@ -300,8 +300,6 @@ async fn get_configured_param( param.smtp.password.clone() }; - let proxy_enabled = ctx.get_config_bool(Config::ProxyEnabled).await?; - let mut addr = param.addr.clone(); if param.oauth2 { // the used oauth2 addr may differ, check this. @@ -343,7 +341,7 @@ async fn get_configured_param( "checking internal provider-info for offline autoconfig" ); - provider = provider::get_provider_info(ctx, ¶m_domain, proxy_enabled).await; + provider = provider::get_provider_info(¶m_domain); if let Some(provider) = provider { if provider.server.is_empty() { info!(ctx, "Offline autoconfig found, but no servers defined."); diff --git a/src/oauth2.rs b/src/oauth2.rs index 4592a19b7..5c0b6a00e 100644 --- a/src/oauth2.rs +++ b/src/oauth2.rs @@ -53,7 +53,7 @@ pub async fn get_oauth2_url( addr: &str, redirect_uri: &str, ) -> Result> { - if let Some(oauth2) = Oauth2::from_address(context, addr).await { + if let Some(oauth2) = Oauth2::from_address(addr) { context .sql .set_raw_config("oauth2_pending_redirect_uri", Some(redirect_uri)) @@ -73,7 +73,7 @@ pub(crate) async fn get_oauth2_access_token( code: &str, regenerate: bool, ) -> Result> { - if let Some(oauth2) = Oauth2::from_address(context, addr).await { + if let Some(oauth2) = Oauth2::from_address(addr) { let lock = context.oauth2_mutex.lock().await; // read generated token @@ -221,7 +221,7 @@ pub(crate) async fn get_oauth2_addr( addr: &str, code: &str, ) -> Result> { - let oauth2 = match Oauth2::from_address(context, addr).await { + let oauth2 = match Oauth2::from_address(addr) { Some(o) => o, None => return Ok(None), }; @@ -256,15 +256,13 @@ pub(crate) async fn get_oauth2_addr( } impl Oauth2 { - async fn from_address(context: &Context, addr: &str) -> Option { + fn from_address(addr: &str) -> Option { let addr_normalized = normalize_addr(addr); - let skip_mx = true; if let Some(domain) = addr_normalized .find('@') .map(|index| addr_normalized.split_at(index + 1).1) { - if let Some(oauth2_authorizer) = provider::get_provider_info(context, domain, skip_mx) - .await + if let Some(oauth2_authorizer) = provider::get_provider_info(domain) .and_then(|provider| provider.oauth2_authorizer.as_ref()) { return Some(match oauth2_authorizer { @@ -354,21 +352,16 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_oauth_from_address() { - let t = TestContext::new().await; - // Delta Chat does not have working Gmail client ID anymore. - assert_eq!(Oauth2::from_address(&t, "hello@gmail.com").await, None); - assert_eq!(Oauth2::from_address(&t, "hello@googlemail.com").await, None); + assert_eq!(Oauth2::from_address("hello@gmail.com"), None); + assert_eq!(Oauth2::from_address("hello@googlemail.com"), None); assert_eq!( - Oauth2::from_address(&t, "hello@yandex.com").await, + Oauth2::from_address("hello@yandex.com"), Some(OAUTH2_YANDEX) ); - assert_eq!( - Oauth2::from_address(&t, "hello@yandex.ru").await, - Some(OAUTH2_YANDEX) - ); - assert_eq!(Oauth2::from_address(&t, "hello@web.de").await, None); + assert_eq!(Oauth2::from_address("hello@yandex.ru"), Some(OAUTH2_YANDEX)); + assert_eq!(Oauth2::from_address("hello@web.de"), None); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/src/provider.rs b/src/provider.rs index fb9eb369e..9d5ee63c0 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -4,13 +4,9 @@ pub(crate) mod data; use anyhow::Result; use deltachat_contact_tools::EmailAddress; -use hickory_resolver::name_server::TokioConnectionProvider; -use hickory_resolver::{Resolver, TokioResolver, config}; use serde::{Deserialize, Serialize}; use crate::config::Config; -use crate::context::Context; -use crate::log::warn; use crate::provider::data::{PROVIDER_DATA, PROVIDER_IDS}; /// Provider status according to manual testing. @@ -171,61 +167,17 @@ impl ProviderOptions { } } -/// Get resolver to query MX records. -/// -/// We first try to read the system's resolver from `/etc/resolv.conf`. -/// This does not work at least on some Androids, therefore we fallback -/// to the default `ResolverConfig` which uses eg. to google's `8.8.8.8` or `8.8.4.4`. -fn get_resolver() -> Result { - if let Ok(resolver) = TokioResolver::builder_tokio() { - return Ok(resolver.build()); - } - let resolver = Resolver::builder_with_config( - config::ResolverConfig::default(), - TokioConnectionProvider::default(), - ); - Ok(resolver.build()) -} - /// Returns provider for the given an e-mail address. /// /// Returns an error if provided address is not valid. -pub async fn get_provider_info_by_addr( - context: &Context, - addr: &str, - skip_mx: bool, -) -> Result> { +pub fn get_provider_info_by_addr(addr: &str) -> Result> { let addr = EmailAddress::new(addr)?; - let provider = get_provider_info(context, &addr.domain, skip_mx).await; - Ok(provider) -} - -/// Returns provider for the given domain. -/// -/// This function looks up domain in offline database first. If not -/// found, it queries MX record for the domain and looks up offline -/// database for MX domains. -pub async fn get_provider_info( - context: &Context, - domain: &str, - skip_mx: bool, -) -> Option<&'static Provider> { - if let Some(provider) = get_provider_by_domain(domain) { - return Some(provider); - } - - if !skip_mx { - if let Some(provider) = get_provider_by_mx(context, domain).await { - return Some(provider); - } - } - - None + Ok(get_provider_info(&addr.domain)) } /// Finds a provider in offline database based on domain. -pub fn get_provider_by_domain(domain: &str) -> Option<&'static Provider> { +pub fn get_provider_info(domain: &str) -> Option<&'static Provider> { let domain = domain.to_lowercase(); for (pattern, provider) in PROVIDER_DATA { if let Some(suffix) = pattern.strip_prefix('*') { @@ -243,51 +195,6 @@ pub fn get_provider_by_domain(domain: &str) -> Option<&'static Provider> { None } -/// Finds a provider based on MX record for the given domain. -/// -/// For security reasons, only Gmail can be configured this way. -pub async fn get_provider_by_mx(context: &Context, domain: &str) -> Option<&'static Provider> { - let Ok(resolver) = get_resolver() else { - warn!(context, "Cannot get a resolver to check MX records."); - return None; - }; - - let mut fqdn: String = domain.to_string(); - if !fqdn.ends_with('.') { - fqdn.push('.'); - } - - let Ok(mx_domains) = resolver.mx_lookup(fqdn).await else { - warn!(context, "Cannot resolve MX records for {domain:?}."); - return None; - }; - - for (provider_domain_pattern, provider) in PROVIDER_DATA { - if provider.id != "gmail" { - // MX lookup is limited to Gmail for security reasons - continue; - } - - if provider_domain_pattern.starts_with('*') { - // Skip wildcard patterns. - continue; - } - - let provider_fqdn = provider_domain_pattern.to_string() + "."; - let provider_fqdn_dot = ".".to_string() + &provider_fqdn; - - for mx_domain in mx_domains.iter() { - let mx_domain = mx_domain.exchange().to_lowercase().to_utf8(); - - if mx_domain == provider_fqdn || mx_domain.ends_with(&provider_fqdn_dot) { - return Some(provider); - } - } - } - - None -} - /// Returns a provider with the given ID from the database. pub fn get_provider_by_id(id: &str) -> Option<&'static Provider> { if let Some(provider) = PROVIDER_IDS.get(id) { @@ -300,24 +207,23 @@ pub fn get_provider_by_id(id: &str) -> Option<&'static Provider> { #[cfg(test)] mod tests { use super::*; - use crate::test_utils::TestContext; #[test] fn test_get_provider_by_domain_unexistant() { - let provider = get_provider_by_domain("unexistant.org"); + let provider = get_provider_info("unexistant.org"); assert!(provider.is_none()); } #[test] fn test_get_provider_by_domain_mixed_case() { - let provider = get_provider_by_domain("nAUta.Cu").unwrap(); + let provider = get_provider_info("nAUta.Cu").unwrap(); assert!(provider.status == Status::Ok); } #[test] - fn test_get_provider_by_domain() { + fn test_get_provider_info() { let addr = "nauta.cu"; - let provider = get_provider_by_domain(addr).unwrap(); + let provider = get_provider_info(addr).unwrap(); assert!(provider.status == Status::Ok); let server = &provider.server[0]; assert_eq!(server.protocol, Protocol::Imap); @@ -332,13 +238,17 @@ mod tests { assert_eq!(server.port, 25); assert_eq!(server.username_pattern, UsernamePattern::Email); - let provider = get_provider_by_domain("gmail.com").unwrap(); + let provider = get_provider_info("gmail.com").unwrap(); assert!(provider.status == Status::Preparation); assert!(!provider.before_login_hint.is_empty()); assert!(!provider.overview_page.is_empty()); - let provider = get_provider_by_domain("googlemail.com").unwrap(); + let provider = get_provider_info("googlemail.com").unwrap(); assert!(provider.status == Status::Preparation); + + assert!(get_provider_info("").is_none()); + assert!(get_provider_info("google.com").unwrap().id == "gmail"); + assert!(get_provider_info("example@google.com").is_none()); } #[test] @@ -347,39 +257,10 @@ mod tests { assert!(provider.id == "gmail"); } - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_get_provider_info() { - let t = TestContext::new().await; - assert!(get_provider_info(&t, "", false).await.is_none()); - assert!(get_provider_info(&t, "google.com", false).await.unwrap().id == "gmail"); - assert!( - get_provider_info(&t, "example@google.com", false) - .await - .is_none() - ); - } - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_get_provider_info_by_addr() -> Result<()> { - let t = TestContext::new().await; - assert!( - get_provider_info_by_addr(&t, "google.com", false) - .await - .is_err() - ); - assert!( - get_provider_info_by_addr(&t, "example@google.com", false) - .await? - .unwrap() - .id - == "gmail" - ); - Ok(()) - } - - #[test] - fn test_get_resolver() -> Result<()> { - assert!(get_resolver().is_ok()); + assert!(get_provider_info_by_addr("google.com").is_err()); + assert!(get_provider_info_by_addr("example@google.com")?.unwrap().id == "gmail"); Ok(()) } } diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index 4a7c40e91..01d4df286 100644 --- a/src/sql/migrations.rs +++ b/src/sql/migrations.rs @@ -19,7 +19,7 @@ use crate::key::DcKey; use crate::log::{info, warn}; use crate::login_param::ConfiguredLoginParam; use crate::message::MsgId; -use crate::provider::get_provider_by_domain; +use crate::provider::get_provider_info; use crate::sql::Sql; use crate::tools::{Time, inc_and_check, time_elapsed}; @@ -382,7 +382,7 @@ UPDATE chats SET protected=1, type=120 WHERE type=130;"#, context .set_config_internal( Config::ConfiguredProvider, - get_provider_by_domain(&domain).map(|provider| provider.id), + get_provider_info(&domain).map(|provider| provider.id), ) .await?; } else {