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

Commit 6b17842f authored by Matthew Maurer's avatar Matthew Maurer
Browse files

DoH: Refactor FFI 

* Factor out constants to level enum conversion
* Document property that doh_init_logger will have no effect on future calls
* Change `doh_set_log_level` to default to `Error` logging on an invalid
  value rather than disabling all logging. This matches the behavior of
  `doh_init_logger`.
* Prefix constants with `DOH_`. `cbindgen`-exported constants are not in
  a namespace; they are raw C constants. Prepending the library name
  will help avoid future collisions.
* `android_logger::init_once` explicitly states in the documentation that
  calling it more than once is safe, and will simply not change the logger
  on future calls.

Bug: 202081046
Change-Id: I91a8a7f40363ee2dcf6211fb595e1c6e7f6084a8
parent 29c917d9

DnsResolver.cpp

+1 −1
Original line number Diff line number Diff line
@@ -33,7 +33,7 @@ bool resolv_init(const ResolverNetdCallbacks* callbacks) { 
    // TODO(b/170539625): restore log level to WARNING after clarifying flaky tests.

    const bool isDebug = isUserDebugBuild();

    resolv_set_log_severity(isDebug ? android::base::DEBUG : android::base::WARNING);

    doh_init_logger(isDebug ? LOG_LEVEL_DEBUG : LOG_LEVEL_WARN);

    doh_init_logger(isDebug ? DOH_LOG_LEVEL_DEBUG : DOH_LOG_LEVEL_WARN);

    using android::net::gApiLevel;

    gApiLevel = getApiLevel();

    using android::net::gResNetdCallbacks;

PrivateDnsConfiguration.cpp

+1 −1
Original line number Diff line number Diff line
@@ -510,7 +510,7 @@ ssize_t PrivateDnsConfiguration::dohQuery(unsigned netId, const Slice query, con 
    {

        std::lock_guard guard(mPrivateDnsLock);

        // It's safe because mDohDispatcher won't be deleted after initializing.

        if (mDohDispatcher == nullptr) return RESULT_CAN_NOT_SEND;

        if (mDohDispatcher == nullptr) return DOH_RESULT_CAN_NOT_SEND;

    }

    return doh_query(mDohDispatcher, netId, query.base(), query.size(), answer.base(),

                     answer.size(), timeoutMs);

doh.h

+11 −8
Original line number Diff line number Diff line
@@ -26,28 +26,28 @@ 
#include <sys/types.h>



/// The return code of doh_query means that there is no answer.

static const ssize_t RESULT_INTERNAL_ERROR = -1;

static const ssize_t DOH_RESULT_INTERNAL_ERROR = -1;



/// The return code of doh_query means that query can't be sent.

static const ssize_t RESULT_CAN_NOT_SEND = -2;

static const ssize_t DOH_RESULT_CAN_NOT_SEND = -2;



/// The return code of doh_query to indicate that the query timed out.

static const ssize_t RESULT_TIMEOUT = -255;

static const ssize_t DOH_RESULT_TIMEOUT = -255;



/// The error log level.

static const uint32_t LOG_LEVEL_ERROR = 0;

static const uint32_t DOH_LOG_LEVEL_ERROR = 0;



/// The warning log level.

static const uint32_t LOG_LEVEL_WARN = 1;

static const uint32_t DOH_LOG_LEVEL_WARN = 1;



/// The info log level.

static const uint32_t LOG_LEVEL_INFO = 2;

static const uint32_t DOH_LOG_LEVEL_INFO = 2;



/// The debug log level.

static const uint32_t LOG_LEVEL_DEBUG = 3;

static const uint32_t DOH_LOG_LEVEL_DEBUG = 3;



/// The trace log level.

static const uint32_t LOG_LEVEL_TRACE = 4;

static const uint32_t DOH_LOG_LEVEL_TRACE = 4;



/// Context for a running DoH engine.

struct DohDispatcher;
@@ -60,9 +60,12 @@ using TagSocketCallback = void (*)(int32_t sock); 
extern "C" {



/// Performs static initialization for android logger.

/// If an invalid level is passed, defaults to logging errors only.

/// If called more than once, it will have no effect on subsequent calls.

void doh_init_logger(uint32_t level);



/// Set the log level.

/// If an invalid level is passed, defaults to logging errors only.

void doh_set_log_level(uint32_t level);



/// Performs the initialization for the DoH engine.

doh/ffi.rs

+39 −32
Original line number Diff line number Diff line
@@ -42,45 +42,52 @@ impl DohDispatcher { 
const SYSTEM_CERT_PATH: &str = "/system/etc/security/cacerts";



/// The return code of doh_query means that there is no answer.

pub const RESULT_INTERNAL_ERROR: ssize_t = -1;

pub const DOH_RESULT_INTERNAL_ERROR: ssize_t = -1;

/// The return code of doh_query means that query can't be sent.

pub const RESULT_CAN_NOT_SEND: ssize_t = -2;

pub const DOH_RESULT_CAN_NOT_SEND: ssize_t = -2;

/// The return code of doh_query to indicate that the query timed out.

pub const RESULT_TIMEOUT: ssize_t = -255;

pub const DOH_RESULT_TIMEOUT: ssize_t = -255;



/// The error log level.

pub const LOG_LEVEL_ERROR: u32 = 0;

pub const DOH_LOG_LEVEL_ERROR: u32 = 0;

/// The warning log level.

pub const LOG_LEVEL_WARN: u32 = 1;

pub const DOH_LOG_LEVEL_WARN: u32 = 1;

/// The info log level.

pub const LOG_LEVEL_INFO: u32 = 2;

pub const DOH_LOG_LEVEL_INFO: u32 = 2;

/// The debug log level.

pub const LOG_LEVEL_DEBUG: u32 = 3;

pub const DOH_LOG_LEVEL_DEBUG: u32 = 3;

/// The trace log level.

pub const LOG_LEVEL_TRACE: u32 = 4;

pub const DOH_LOG_LEVEL_TRACE: u32 = 4;



fn level_from_u32(level: u32) -> Option<log::Level> {

    use log::Level::*;

    match level {

        DOH_LOG_LEVEL_ERROR => Some(Error),

        DOH_LOG_LEVEL_WARN => Some(Warn),

        DOH_LOG_LEVEL_INFO => Some(Info),

        DOH_LOG_LEVEL_DEBUG => Some(Debug),

        DOH_LOG_LEVEL_TRACE => Some(Trace),

        _ => None,

    }

}



/// Performs static initialization for android logger.

/// If an invalid level is passed, defaults to logging errors only.

/// If called more than once, it will have no effect on subsequent calls.

#[no_mangle]

pub extern "C" fn doh_init_logger(level: u32) {

    let level = match level {

        LOG_LEVEL_WARN => log::Level::Warn,

        LOG_LEVEL_DEBUG => log::Level::Debug,

        _ => log::Level::Error,

    };

    android_logger::init_once(android_logger::Config::default().with_min_level(level));

    let log_level = level_from_u32(level).unwrap_or(log::Level::Error);

    android_logger::init_once(android_logger::Config::default().with_min_level(log_level));

}



/// Set the log level.

/// If an invalid level is passed, defaults to logging errors only.

#[no_mangle]

pub extern "C" fn doh_set_log_level(level: u32) {

    let level = match level {

        LOG_LEVEL_ERROR => log::LevelFilter::Error,

        LOG_LEVEL_WARN => log::LevelFilter::Warn,

        LOG_LEVEL_INFO => log::LevelFilter::Info,

        LOG_LEVEL_DEBUG => log::LevelFilter::Debug,

        LOG_LEVEL_TRACE => log::LevelFilter::Trace,

        _ => log::LevelFilter::Off,

    };

    log::set_max_level(level);

    let level_filter = level_from_u32(level)

        .map(|level| level.to_level_filter())

        .unwrap_or(log::LevelFilter::Error);

    log::set_max_level(level_filter);

}



/// Performs the initialization for the DoH engine.
@@ -177,8 +184,8 @@ pub unsafe extern "C" fn doh_net_new( 
}



/// 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

/// the size of the answer.

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

/// or the size of the answer.

/// # Safety

/// `doh` must be a non-null pointer previously created by `doh_dispatcher_new()`

/// and not yet deleted by `doh_dispatcher_delete()`.
@@ -208,11 +215,11 @@ pub unsafe extern "C" fn doh_query( 


        if let Err(e) = doh.lock().send_cmd(cmd) {

            error!("Failed to send the query: {:?}", e);

            return RESULT_CAN_NOT_SEND;

            return DOH_RESULT_CAN_NOT_SEND;

        }

    } else {

        error!("Bad timeout parameter: {}", timeout_ms);

        return RESULT_CAN_NOT_SEND;

        return DOH_RESULT_CAN_NOT_SEND;

    }



    if let Ok(rt) = Runtime::new() {
@@ -222,26 +229,26 @@ pub unsafe extern "C" fn doh_query( 
                Ok(v) => match v {

                    Response::Success { answer } => {

                        if answer.len() > response_len || answer.len() > isize::MAX as usize {

                            return RESULT_INTERNAL_ERROR;

                            return DOH_RESULT_INTERNAL_ERROR;

                        }

                        let response = slice::from_raw_parts_mut(response, answer.len());

                        response.copy_from_slice(&answer);

                        answer.len() as ssize_t

                    }

                    _ => RESULT_CAN_NOT_SEND,

                    _ => DOH_RESULT_CAN_NOT_SEND,

                },

                Err(e) => {

                    error!("no result {}", e);

                    RESULT_CAN_NOT_SEND

                    DOH_RESULT_CAN_NOT_SEND

                }

            },

            Err(e) => {

                error!("timeout: {}", e);

                RESULT_TIMEOUT

                DOH_RESULT_TIMEOUT

            }

        }

    } else {

        RESULT_CAN_NOT_SEND

        DOH_RESULT_CAN_NOT_SEND

    }

}

res_debug.cpp

+6 −6
Original line number Diff line number Diff line
@@ -495,11 +495,11 @@ int resolv_set_log_severity(uint32_t logSeverity) { 
    switch (logSeverity) {

        case aidl::android::net::IDnsResolver::DNS_RESOLVER_LOG_VERBOSE:

            logSeverity = android::base::VERBOSE;

            doh_set_log_level(LOG_LEVEL_TRACE);

            doh_set_log_level(DOH_LOG_LEVEL_TRACE);

            // *** enable verbose logging only when DBG is set. It prints sensitive data ***

            if (RESOLV_ALLOW_VERBOSE_LOGGING == false) {

                logSeverity = android::base::DEBUG;

                doh_set_log_level(LOG_LEVEL_DEBUG);

                doh_set_log_level(DOH_LOG_LEVEL_DEBUG);

                LOG(ERROR) << "Refusing to set VERBOSE logging in non-debuggable build";

                // TODO: Return EACCES then callers could know if the log

                // severity is acceptable
@@ -507,19 +507,19 @@ int resolv_set_log_severity(uint32_t logSeverity) { 
            break;

        case aidl::android::net::IDnsResolver::DNS_RESOLVER_LOG_DEBUG:

            logSeverity = android::base::DEBUG;

            doh_set_log_level(LOG_LEVEL_DEBUG);

            doh_set_log_level(DOH_LOG_LEVEL_DEBUG);

            break;

        case aidl::android::net::IDnsResolver::DNS_RESOLVER_LOG_INFO:

            logSeverity = android::base::INFO;

            doh_set_log_level(LOG_LEVEL_INFO);

            doh_set_log_level(DOH_LOG_LEVEL_INFO);

            break;

        case aidl::android::net::IDnsResolver::DNS_RESOLVER_LOG_WARNING:

            logSeverity = android::base::WARNING;

            doh_set_log_level(LOG_LEVEL_WARN);

            doh_set_log_level(DOH_LOG_LEVEL_WARN);

            break;

        case aidl::android::net::IDnsResolver::DNS_RESOLVER_LOG_ERROR:

            logSeverity = android::base::ERROR;

            doh_set_log_level(LOG_LEVEL_ERROR);

            doh_set_log_level(DOH_LOG_LEVEL_ERROR);

            break;

        default:

            LOG(ERROR) << __func__ << ": invalid log severity: " << logSeverity;