Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4c684971 authored by Mike Yu's avatar Mike Yu
Browse files

Make max_idle_timeout configurable 

Add a new flag, doh_idle_timeout_ms, so as to configure the QUIC
transport parameter max_idle_timeout.

Like the other flags, the DnsResolver doesn't use the new timeout
immediately until either createNetworkCache() or destroyNetworkCache()
is called.

After the DnsResolver starts using the new timeout, the new timeout
will be applied to new networks. The existing networks will still use
the old timeout before users change private DNS settings.

Bug: 205922706
Test: cd packages/modules/DnsResolver && atest
Test: 1. Turned on mobile data and sent some DoH queries
      2. Set doh_idle_timeout_ms to 5000
      3. Turned on/off wifi to make the flag effective
      4. Checked that DnsResolve is using default timeout on the
         cellular network.
      5. Change private DNS settings
      6. Checked that DnsResolver started using 5000ms for new
         DoH connections on the cellular network.
Change-Id: I29997164067c190f6e1d07b5815e2170ef425ddb
parent 549be5d7

Experiments.h

+1 −0
Original line number Diff line number Diff line
@@ -64,6 +64,7 @@ class Experiments { 
            "doh",

            "doh_query_timeout_ms",

            "doh_probe_timeout_ms",

            "doh_idle_timeout_ms",

            "mdns_resolution",

    };

    // This value is used in updateInternal as the default value if any flags can't be found.

PrivateDnsConfiguration.cpp

+15 −6
Original line number Diff line number Diff line
@@ -444,6 +444,15 @@ int PrivateDnsConfiguration::setDoh(int32_t netId, uint32_t mark, 
        return 0;

    }



    const auto getTimeoutFromFlag = [&](const std::string_view key, int defaultValue) -> uint64_t {

        static constexpr int kMinTimeoutMs = 1000;

        uint64_t timeout = Experiments::getInstance()->getFlag(key, defaultValue);

        if (timeout < kMinTimeoutMs) {

            timeout = kMinTimeoutMs;

        }

        return timeout;

    };



    // Sort the input servers to ensure that we could get the server vector at the same order.

    std::vector<std::string> sortedServers = servers;

    // Prefer ipv6.
@@ -481,13 +490,13 @@ int PrivateDnsConfiguration::setDoh(int32_t netId, uint32_t mark, 
        LOG(INFO) << __func__ << ": Upgrading server to DoH: " << name;

        resolv_stats_set_addrs(netId, PROTO_DOH, {dohId.ipAddr}, kDohPort);



        int probeTimeout = Experiments::getInstance()->getFlag("doh_probe_timeout_ms",

                                                               kDohProbeDefaultTimeoutMs);

        if (probeTimeout < 1000) {

            probeTimeout = 1000;

        }

        auto probeTimeout = getTimeoutFromFlag("doh_probe_timeout_ms", kDohProbeDefaultTimeoutMs);

        auto idleTimeout = getTimeoutFromFlag("doh_idle_timeout_ms", kDohIdleDefaultTimeoutMs);

        LOG(DEBUG) << __func__ << ": probeTimeout " << probeTimeout << ", idleTimeout "

                   << idleTimeout;



        return doh_net_new(mDohDispatcher, netId, dohId.httpsTemplate.c_str(), dohId.host.c_str(),

                           dohId.ipAddr.c_str(), mark, caCert.c_str(), probeTimeout);

                           dohId.ipAddr.c_str(), mark, caCert.c_str(), probeTimeout, idleTimeout);

    }



    LOG(INFO) << __func__ << ": No suitable DoH server found";

PrivateDnsConfiguration.h

+3 −0
Original line number Diff line number Diff line
@@ -63,6 +63,9 @@ class PrivateDnsConfiguration { 
    static constexpr int kDohQueryDefaultTimeoutMs = 30000;

    static constexpr int kDohProbeDefaultTimeoutMs = 60000;



    // The default value for QUIC max_idle_timeout.

    static constexpr int kDohIdleDefaultTimeoutMs = 55000;



    struct ServerIdentity {

        const netdutils::IPSockAddr sockaddr;

        const std::string provider;

doh.h

+1 −1
Original line number Diff line number Diff line
@@ -87,7 +87,7 @@ void doh_dispatcher_delete(DohDispatcher* doh); 
/// `url`, `domain`, `ip_addr`, `cert_path` are null terminated strings.

int32_t doh_net_new(DohDispatcher* doh, uint32_t net_id, const char* url, const char* domain,

                    const char* ip_addr, uint32_t sk_mark, const char* cert_path,

                    uint64_t timeout_ms);

                    uint64_t probe_timeout_ms, uint64_t idle_timeout_ms);



/// Sends a DNS query via the network associated to the given |net_id| and waits for the response.

/// The return code should be either one of the public constant RESULT_* to indicate the error or

doh/config.rs

+60 −25
Original line number Diff line number Diff line
@@ -41,8 +41,6 @@ const MAX_INCOMING_BUFFER_SIZE_EACH: u64 = 1000000; 
const MAX_CONCURRENT_STREAM_SIZE: u64 = 100;

/// Maximum datagram size we will accept.

pub const MAX_DATAGRAM_SIZE: usize = 1350;

/// How long with no packets before we assume a connection is dead, in milliseconds.

pub const QUICHE_IDLE_TIMEOUT_MS: u64 = 55000;



impl Config {

    fn from_weak(weak: &WeakConfig) -> Option<Self> {
@@ -55,10 +53,10 @@ impl Config { 


    /// Construct a `Config` object from certificate path. If no path

    /// is provided, peers will not be verified.

    pub fn from_cert_path(cert_path: Option<&str>) -> Result<Self> {

    pub fn from_key(key: &Key) -> Result<Self> {

        let mut config = quiche::Config::new(quiche::PROTOCOL_VERSION)?;

        config.set_application_protos(h3::APPLICATION_PROTOCOL)?;

        match cert_path {

        match key.cert_path.as_deref() {

            Some(path) => {

                config.verify_peer(true);

                config.load_verify_locations_from_directory(path)?;
@@ -67,7 +65,7 @@ impl Config { 
        }



        // Some of these configs are necessary, or the server can't respond the HTTP/3 request.

        config.set_max_idle_timeout(QUICHE_IDLE_TIMEOUT_MS);

        config.set_max_idle_timeout(key.max_idle_timeout);

        config.set_max_recv_udp_payload_size(MAX_DATAGRAM_SIZE);

        config.set_initial_max_data(MAX_INCOMING_BUFFER_SIZE_WHOLE);

        config.set_initial_max_stream_data_bidi_local(MAX_INCOMING_BUFFER_SIZE_EACH);
@@ -89,15 +87,15 @@ impl Config { 
#[derive(Clone, Default)]

struct State {

    // Mapping from cert_path to configs

    path_to_config: HashMap<Option<String>, WeakConfig>,

    key_to_config: HashMap<Key, WeakConfig>,

    // Keep latest config alive to minimize reparsing when flapping

    // If more keep-alive is needed, replace with a LRU LinkedList

    latest: Option<Config>,

}



impl State {

    fn get_config(&self, cert_path: &Option<String>) -> Option<Config> {

        self.path_to_config.get(cert_path).and_then(Config::from_weak)

    fn get_config(&self, key: &Key) -> Option<Config> {

        self.key_to_config.get(key).and_then(Config::from_weak)

    }



    fn keep_alive(&mut self, config: Config) {
@@ -105,7 +103,7 @@ impl State { 
    }



    fn garbage_collect(&mut self) {

        self.path_to_config.retain(|_, config| config.strong_count() != 0)

        self.key_to_config.retain(|_, config| config.strong_count() != 0)

    }

}
@@ -123,6 +121,13 @@ pub struct Cache { 
    state: Arc<RwLock<State>>,

}



/// Key used for getting an associated Quiche Config from Cache.

#[derive(Clone, PartialEq, Eq, Hash)]

pub struct Key {

    pub cert_path: Option<String>,

    pub max_idle_timeout: u64,

}



impl Cache {

    /// Creates a fresh empty cache

    pub fn new() -> Self {
@@ -132,9 +137,9 @@ impl Cache { 
    /// Behaves as `Config::from_cert_path`, but with a cache.

    /// If any object previously given out by this cache is still live,

    /// a duplicate will not be made.

    pub fn from_cert_path(&self, cert_path: &Option<String>) -> Result<Config> {

    pub fn from_key(&self, key: &Key) -> Result<Config> {

        // Fast path - read-only access to state retrieves config

        if let Some(config) = self.state.read().unwrap().get_config(cert_path) {

        if let Some(config) = self.state.read().unwrap().get_config(key) {

            return Ok(config);

        }
@@ -142,21 +147,21 @@ impl Cache { 
        // the cert path, we'll arbitrate that in the next step, but this

        // makes sure loading a new cert path doesn't block other loads to

        // refresh connections.

        let config = Config::from_cert_path(cert_path.as_deref())?;

        let config = Config::from_key(key)?;



        let mut state = self.state.write().unwrap();

        // We now have exclusive access to the state.

        // If someone else calculated a config at the same time as us, we

        // want to discard ours and use theirs, since it will result in

        // less total memory used.

        if let Some(config) = state.get_config(cert_path) {

        if let Some(config) = state.get_config(key) {

            return Ok(config);

        }



        // We have exclusive access and a fresh config. Install it into

        // the cache.

        state.keep_alive(config.clone());

        state.path_to_config.insert(cert_path.to_owned(), config.to_weak());

        state.key_to_config.insert(key.clone(), config.to_weak());

        Ok(config)

    }
@@ -168,9 +173,16 @@ impl Cache { 


#[test]

fn create_quiche_config() {

    assert!(Config::from_cert_path(None).is_ok(), "quiche config without cert creating failed");

    assert!(

        Config::from_cert_path(Some("data/local/tmp/")).is_ok(),

        Config::from_key(&Key { cert_path: None, max_idle_timeout: 1000 }).is_ok(),

        "quiche config without cert creating failed"

    );

    assert!(

        Config::from_key(&Key {

            cert_path: Some("data/local/tmp/".to_string()),

            max_idle_timeout: 1000

        })

        .is_ok(),

        "quiche config with cert creating failed"

    );

}
@@ -179,25 +191,48 @@ fn create_quiche_config() { 
fn shared_cache() {

    let cache_a = Cache::new();

    let cache_b = cache_a.clone();

    let config_a = cache_a.from_cert_path(&None).unwrap();

    let config_a = cache_a.from_key(&Key { cert_path: None, max_idle_timeout: 1000 }).unwrap();

    assert_eq!(Arc::strong_count(&config_a.0), 2);

    let _config_b = cache_b.from_cert_path(&None).unwrap();

    let _config_b = cache_b.from_key(&Key { cert_path: None, max_idle_timeout: 1000 }).unwrap();

    assert_eq!(Arc::strong_count(&config_a.0), 3);

}



#[test]

fn different_keys() {

    let cache = Cache::new();

    let key_a = Key { cert_path: None, max_idle_timeout: 1000 };

    let key_b = Key { cert_path: Some("a".to_string()), max_idle_timeout: 1000 };

    let key_c = Key { cert_path: Some("a".to_string()), max_idle_timeout: 5000 };

    let config_a = cache.from_key(&key_a).unwrap();

    let config_b = cache.from_key(&key_b).unwrap();

    let _config_b = cache.from_key(&key_b).unwrap();

    let config_c = cache.from_key(&key_c).unwrap();

    let _config_c = cache.from_key(&key_c).unwrap();



    assert_eq!(Arc::strong_count(&config_a.0), 1);

    assert_eq!(Arc::strong_count(&config_b.0), 2);



    // config_c was most recently created, so it should have an extra strong reference due to

    // keep-alive in the cache.

    assert_eq!(Arc::strong_count(&config_c.0), 3);

}



#[test]

fn lifetimes() {

    let cache = Cache::new();

    let config_none = cache.from_cert_path(&None).unwrap();

    let config_a = cache.from_cert_path(&Some("a".to_string())).unwrap();

    let config_b = cache.from_cert_path(&Some("b".to_string())).unwrap();

    let key_a = Key { cert_path: Some("a".to_string()), max_idle_timeout: 1000 };

    let key_b = Key { cert_path: Some("b".to_string()), max_idle_timeout: 1000 };

    let config_none = cache.from_key(&Key { cert_path: None, max_idle_timeout: 1000 }).unwrap();

    let config_a = cache.from_key(&key_a).unwrap();

    let config_b = cache.from_key(&key_b).unwrap();



    // The first two we created should have a strong count of one - those handles are the only

    // thing keeping them alive.

    assert_eq!(Arc::strong_count(&config_none.0), 1);

    assert_eq!(Arc::strong_count(&config_a.0), 1);



    // If we try to get another handle we already have, it should be the same one.

    let _config_a2 = cache.from_cert_path(&Some("a".to_string())).unwrap();

    let _config_a2 = cache.from_key(&key_a).unwrap();

    assert_eq!(Arc::strong_count(&config_a.0), 2);



    // config_b was most recently created, so it should have a keep-alive
@@ -221,19 +256,19 @@ fn lifetimes() { 


    // If we try to get a config which is still kept alive by the cache, we should get the same

    // one.

    let _config_b2 = cache.from_cert_path(&Some("b".to_string())).unwrap();

    let _config_b2 = cache.from_key(&key_b).unwrap();

    assert_eq!(config_b_weak.strong_count(), 2);



    // We broke None, but "a" and "b" should still both be alive. Check that

    // this is still the case in the mapping after garbage collection.

    cache.garbage_collect();

    assert_eq!(cache.state.read().unwrap().path_to_config.len(), 2);

    assert_eq!(cache.state.read().unwrap().key_to_config.len(), 2);

}



#[tokio::test]

async fn quiche_connect() {

    use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4};

    let mut config = Config::from_cert_path(None).unwrap();

    let mut config = Config::from_key(&Key { cert_path: None, max_idle_timeout: 10 }).unwrap();

    let socket_addr = SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::new(127, 0, 0, 1), 42));

    let conn_id = quiche::ConnectionId::from_ref(&[]);

    quiche::connect(None, &conn_id, socket_addr, config.take().await.deref_mut()).unwrap();