From c35c83a4b2daa589e42bf03a60d4e373306fdbca Mon Sep 17 00:00:00 2001 From: Matteo Settenvini Date: Wed, 24 Aug 2022 12:48:36 +0200 Subject: [PATCH] Minor refactors --- src/gethostbyaddr.rs | 2 +- src/gethostbyname.rs | 240 +++++++++++++++++++++++++++++++------- src/helpers.rs | 202 +++++--------------------------- src/nss_api.rs | 5 +- tests/integration_test.rs | 1 + 5 files changed, 229 insertions(+), 221 deletions(-) diff --git a/src/gethostbyaddr.rs b/src/gethostbyaddr.rs index ae4df35..d946857 100644 --- a/src/gethostbyaddr.rs +++ b/src/gethostbyaddr.rs @@ -8,7 +8,7 @@ use { std::os::raw::{c_char, c_int, c_void}, }; -pub async unsafe fn gethostbyaddr2_r( +pub async unsafe fn with( _addr: *const c_void, _len: socklen_t, _af: c_int, diff --git a/src/gethostbyname.rs b/src/gethostbyname.rs index e6fc8af..94584c7 100644 --- a/src/gethostbyname.rs +++ b/src/gethostbyname.rs @@ -2,15 +2,19 @@ // SPDX-License-Identifier: GPL-3.0-or-later use { - crate::helpers::{records_to_gaih_addr, records_to_hostent, set_if_valid, DnsResult}, + crate::helpers::{set_if_valid, write_record_name_to_buf, write_vector_to_buf}, crate::nss_bindings::{gaih_addrtuple, nss_status, HErrno}, crate::policy_checker::{PolicyChecker as _, POLICY_CHECKER}, - libc::{c_char, c_int, hostent, size_t}, + libc::{c_char, c_int, hostent, size_t, AF_INET, AF_INET6}, nix::errno, nix::errno::Errno, std::ffi::CStr, + std::mem::{align_of, discriminant, size_of}, + std::sync::Arc, trust_dns_proto::rr::record_type::RecordType, + trust_dns_proto::rr::{RData, Record}, trust_dns_proto::xfer::dns_request::DnsRequestOptions, + trust_dns_resolver::TokioAsyncResolver, trust_dns_resolver::{lookup::Lookup, lookup_ip::LookupIp}, }; @@ -33,6 +37,11 @@ pub enum Result { V4(*mut *mut gaih_addrtuple), } +pub enum DnsResult { + NxDomain, + Found, +} + pub async unsafe fn with(args: &mut Args) -> nss_status { set_if_valid(args.errnop, errno::from_i32(0)); set_if_valid(args.h_errnop, HErrno::Success); @@ -42,46 +51,7 @@ pub async unsafe fn with(args: &mut Args) -> nss_status { // no restrictions for user, the next NSS module will decide nss_status::NSS_STATUS_NOTFOUND } - Ok(Some(resolver)) => { - let name = match CStr::from_ptr(args.name).to_str() { - Ok(name) => name, - Err(_) => { - set_if_valid(args.errnop, Errno::EINVAL); - set_if_valid(args.h_errnop, HErrno::Internal); - return nss_status::NSS_STATUS_TRYAGAIN; - } - }; - - // disable application-based DNS for those applications - // (notably, Firefox) that support it - if name == CANARY_HOSTNAME { - set_if_valid(args.h_errnop, HErrno::HostNotFound); - return nss_status::NSS_STATUS_SUCCESS; - } - - let lookup: std::result::Result = match args.family { - libc::AF_UNSPEC => resolver.lookup_ip(name).await.map(LookupIp::into), - libc::AF_INET => { - resolver - .lookup(name, RecordType::A, DnsRequestOptions::default()) - .await - } - libc::AF_INET6 => { - resolver - .lookup(name, RecordType::AAAA, DnsRequestOptions::default()) - .await - } - _ => return nss_status::NSS_STATUS_NOTFOUND, - }; - - match lookup { - Ok(result) => prepare_response(args, result), - Err(err) => { - log::warn!("{}", err); - nss_status::NSS_STATUS_UNAVAIL - } - } - } + Ok(Some(resolver)) => resolve_hostname_through(resolver, args).await, Err(err) => { log::error!("{}", err); nss_status::NSS_STATUS_UNAVAIL @@ -89,6 +59,48 @@ pub async unsafe fn with(args: &mut Args) -> nss_status { } } +async unsafe fn resolve_hostname_through( + resolver: Arc, + args: &mut Args, +) -> nss_status { + let name = match CStr::from_ptr(args.name).to_str() { + Ok(name) => name, + Err(_) => { + set_if_valid(args.errnop, Errno::EINVAL); + set_if_valid(args.h_errnop, HErrno::Internal); + return nss_status::NSS_STATUS_TRYAGAIN; + } + }; + + if name == CANARY_HOSTNAME { + set_if_valid(args.h_errnop, HErrno::HostNotFound); + return nss_status::NSS_STATUS_SUCCESS; + } + + let lookup: std::result::Result = match args.family { + libc::AF_UNSPEC => resolver.lookup_ip(name).await.map(LookupIp::into), + libc::AF_INET => { + resolver + .lookup(name, RecordType::A, DnsRequestOptions::default()) + .await + } + libc::AF_INET6 => { + resolver + .lookup(name, RecordType::AAAA, DnsRequestOptions::default()) + .await + } + _ => return nss_status::NSS_STATUS_NOTFOUND, + }; + + match lookup { + Ok(result) => prepare_response(args, result), + Err(err) => { + log::warn!("{}", err); + nss_status::NSS_STATUS_UNAVAIL + } + } +} + unsafe fn prepare_response( args: &mut Args, lookup: trust_dns_resolver::lookup::Lookup, @@ -167,3 +179,147 @@ unsafe fn prepare_response( ret } + +// TODO: refactor to be less ugly +pub unsafe fn records_to_hostent( + lookup: Lookup, + host: &mut hostent, + buf: &mut [u8], +) -> std::io::Result { + // In C struct hostent: + // + // - for the type of queries we perform, we can assume h_aliases + // is an empty array. + // - hostent is limited to just one address type for all addresses + // in the list. We pick the type of first result, and only + // append addresses of the same type. + + let first_record = match lookup.record_iter().peekable().peek() { + Some(record) => *record, + None => return Ok(DnsResult::NxDomain), + }; + + let first_rdata = match first_record.data() { + Some(rdata) => rdata, + None => return Err(Errno::ENODATA.into()), + }; + + // First put the name in the buffer + let (buf, name_dest) = write_record_name_to_buf(first_record, buf)?; + + // Then the empty aliases array (just a null pointer) + let (mut buf, aliases_ptr) = write_vector_to_buf(buf, vec![std::ptr::null_mut()])?; + + // Then the address list + let offset = buf.as_ptr().align_offset(align_of::<*mut c_char>()); + buf = &mut buf[offset..]; + + let records = lookup + .record_iter() + .flat_map(Record::data) + .filter(|r| discriminant(*r) == discriminant(first_rdata)); + + let mut addresses = vec![]; + for record in records { + let offset = buf.as_ptr().align_offset(align_of::<*mut c_char>()); + buf = &mut buf[offset..]; + addresses.push(buf.as_mut_ptr()); + + let l = match record { + RData::A(addr) => { + let octets = addr.octets(); + let l = octets.len(); + if buf.len() < l { + return Err(Errno::ERANGE.into()); + } + buf[..l].copy_from_slice(&octets); + l + } + RData::AAAA(addr) => { + let octets = addr.octets(); + let l = octets.len(); + if buf.len() < l { + return Err(Errno::ERANGE.into()); + } + buf[..l].copy_from_slice(&octets); + l + } + _ => unreachable!(), + }; + + buf = &mut buf[l..]; + } + + addresses.push(std::ptr::null_mut()); + let (_, addr_list) = write_vector_to_buf(buf, addresses)?; + + // Finally, populate the hostent structure + host.h_name = name_dest; + host.h_aliases = aliases_ptr; + host.h_addr_list = addr_list as *mut *mut c_char; + match first_rdata { + RData::A(_) => { + host.h_addrtype = AF_INET; + host.h_length = 4; + } + RData::AAAA(_) => { + host.h_addrtype = AF_INET6; + host.h_length = 16; + } + _ => return Err(Errno::EBADMSG.into()), + } + + Ok(DnsResult::Found) +} + +// TODO: error handling codes chosen a bit sloppily +pub unsafe fn records_to_gaih_addr( + lookup: Lookup, + mut buf: &mut [u8], +) -> std::io::Result<*mut gaih_addrtuple> { + const GAIH_ADDRTUPLE_SZ: usize = size_of::(); + + let mut ret = std::ptr::null_mut(); + let mut prev_link: *mut *mut gaih_addrtuple = std::ptr::null_mut(); + + let mut name_dest; + for record in lookup.record_iter() { + // First add the name to the buffer + (buf, name_dest) = write_record_name_to_buf(record, buf)?; + + // Then add the tuple with the address + let offset = buf.as_ptr().align_offset(align_of::()); + let l = GAIH_ADDRTUPLE_SZ; + if buf.len() < offset + l { + return Err(Errno::ERANGE.into()); + } + + let tuple = &mut *(buf.as_mut_ptr().add(offset) as *mut gaih_addrtuple); + tuple.next = std::ptr::null_mut(); + tuple.name = name_dest; + tuple.scopeid = 0; // how to set tuple.scopeid correctly??? Use a custom trust_dns_resolver::ConnectionProvider? + set_if_valid(prev_link, &mut *tuple); // link from previous tuple to this tuple + prev_link = &mut (*tuple).next; + + match record.data() { + Some(RData::A(addr)) => { + tuple.family = AF_INET; + tuple.addr[0] = std::mem::transmute_copy(&addr.octets()); + } + Some(RData::AAAA(addr)) => { + tuple.family = AF_INET6; + tuple.addr = std::mem::transmute_copy(&addr.octets()); + } + Some(_) => return Err(Errno::EBADMSG.into()), + None => return Err(Errno::ENODATA.into()), + } + + if ret == std::ptr::null_mut() { + ret = tuple; + } + + buf = &mut buf[(offset + l)..]; + } + + Ok(ret) +} diff --git a/src/helpers.rs b/src/helpers.rs index b679812..306a43d 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -2,16 +2,12 @@ // SPDX-License-Identifier: GPL-3.0-or-later use { - crate::nss_bindings::gaih_addrtuple, - anyhow::{bail, Result}, - libc::{c_char, hostent, AF_INET, AF_INET6}, + libc::c_char, nix::errno::Errno, once_cell::sync::Lazy, std::ffi::CString, - std::mem::{align_of, discriminant, size_of}, + std::mem::{align_of, size_of}, trust_dns_proto::rr::resource::Record, - trust_dns_proto::rr::RData, - trust_dns_resolver::lookup::Lookup, }; static RUNTIME: Lazy> = Lazy::new(|| { @@ -24,171 +20,7 @@ static RUNTIME: Lazy> = Lazy::new(|| { Ok(rt) }); -// TODO: error handling codes chosen a bit sloppily -pub unsafe fn records_to_gaih_addr( - lookup: Lookup, - mut buf: &mut [u8], -) -> std::io::Result<*mut gaih_addrtuple> { - const GAIH_ADDRTUPLE_SZ: usize = size_of::(); - - let mut ret = std::ptr::null_mut(); - let mut prev_link: *mut *mut gaih_addrtuple = std::ptr::null_mut(); - - let mut name_dest; - for record in lookup.record_iter() { - // First add the name to the buffer - (buf, name_dest) = write_record_name_to_buf(record, buf)?; - - // Then add the tuple with the address - let offset = buf.as_ptr().align_offset(align_of::()); - let l = GAIH_ADDRTUPLE_SZ; - if buf.len() < offset + l { - return Err(Errno::ERANGE.into()); - } - - let tuple = &mut *(buf.as_mut_ptr().add(offset) as *mut gaih_addrtuple); - tuple.next = std::ptr::null_mut(); - tuple.name = name_dest; - tuple.scopeid = 0; // how to set tuple.scopeid ???? - set_if_valid(prev_link, &mut *tuple); // link from previous tuple to this tuple - prev_link = &mut (*tuple).next; - - match record.data() { - Some(RData::A(addr)) => { - tuple.family = AF_INET; - tuple.addr[0] = std::mem::transmute_copy(&addr.octets()); - } - Some(RData::AAAA(addr)) => { - tuple.family = AF_INET6; - tuple.addr = std::mem::transmute_copy(&addr.octets()); - } - Some(_) => return Err(Errno::EBADMSG.into()), - None => return Err(Errno::ENODATA.into()), - } - - if ret == std::ptr::null_mut() { - ret = tuple; - } - - buf = &mut buf[(offset + l)..]; - } - - Ok(ret) -} - -pub enum DnsResult { - NxDomain, - Found, -} - -// TODO: refactor to be less ugly -pub unsafe fn records_to_hostent( - lookup: Lookup, - host: &mut hostent, - mut buf: &mut [u8], -) -> std::io::Result { - // In C struct hostent: - // - // - for the type of queries we perform, we can assume h_aliases - // is an empty array. - // - hostent is limited to just one address type for all addresses - // in the list. We pick the type of first result, and only - // append addresses of the same type. - - let first_record = match lookup.record_iter().peekable().peek() { - Some(record) => *record, - None => return Ok(DnsResult::NxDomain), - }; - - let first_rdata = match first_record.data() { - Some(rdata) => rdata, - None => return Err(Errno::ENODATA.into()), - }; - - // First put the name in the buffer - let name_dest; - (buf, name_dest) = write_record_name_to_buf(first_record, buf)?; - - // Then the empty aliases array (just a null pointer) - let offset = buf.as_ptr().align_offset(align_of::<*mut c_char>()); - let l = size_of::<*mut c_char>(); - if buf.len() < offset + l { - return Err(Errno::ERANGE.into()); - } - let aliases_ptr = buf[offset..].as_mut_ptr().cast::<*mut c_char>(); - aliases_ptr.write(std::ptr::null_mut()); - buf = &mut buf[(offset + l)..]; - - // Then the address list - let offset = buf.as_ptr().align_offset(align_of::<*mut c_char>()); - buf = &mut buf[offset..]; - - let records = lookup - .record_iter() - .flat_map(Record::data) - .filter(|r| discriminant(*r) == discriminant(first_rdata)); - - let mut addresses = vec![]; - for record in records { - let offset = buf.as_ptr().align_offset(align_of::<*mut c_char>()); - buf = &mut buf[offset..]; - addresses.push(buf.as_ptr()); - - let l = match record { - RData::A(addr) => { - let octets = addr.octets(); - let l = octets.len(); - if buf.len() < l { - return Err(Errno::ERANGE.into()); - } - buf[..l].copy_from_slice(&octets); - l - } - RData::AAAA(addr) => { - let octets = addr.octets(); - let l = octets.len(); - if buf.len() < l { - return Err(Errno::ERANGE.into()); - } - buf[..l].copy_from_slice(&octets); - l - } - _ => unreachable!(), - }; - - buf = &mut buf[l..]; - } - - addresses.push(std::ptr::null_mut()); - - let offset = buf.as_ptr().align_offset(align_of::<*mut *mut c_char>()); - let l = size_of::<*mut c_char>() * addresses.len(); - if buf.len() < offset + l { - return Err(Errno::ERANGE.into()); - } - let addr_list = buf[offset..].as_mut_ptr().cast::<*mut c_char>(); - std::ptr::copy_nonoverlapping(addresses.as_ptr() as *const *mut c_char, addr_list, l); - - // Finally, populate the hostent structure - host.h_name = name_dest; - host.h_aliases = aliases_ptr; - host.h_addr_list = addr_list; - match first_rdata { - RData::A(_) => { - host.h_addrtype = AF_INET; - host.h_length = 4; - } - RData::AAAA(_) => { - host.h_addrtype = AF_INET6; - host.h_length = 16; - } - _ => return Err(Errno::EBADMSG.into()), - } - - Ok(DnsResult::Found) -} - -unsafe fn write_record_name_to_buf<'a>( +pub fn write_record_name_to_buf<'a>( record: &Record, buf: &'a mut [u8], ) -> std::io::Result<(&'a mut [u8], *mut c_char)> { @@ -203,18 +35,38 @@ unsafe fn write_record_name_to_buf<'a>( return Err(Errno::ERANGE.into()); } - let name_dest = buf.as_mut_ptr().add(offset); - std::ptr::copy_nonoverlapping(name_src.as_ptr(), name_dest, l); + let name_dest = unsafe { buf.as_mut_ptr().add(offset) }; + unsafe { + std::ptr::copy_nonoverlapping(name_src.as_ptr(), name_dest, l); + } Ok((&mut buf[(offset + l)..], name_dest as *mut c_char)) } +pub fn write_vector_to_buf<'a, T>( + buf: &'a mut [u8], + v: Vec, +) -> std::io::Result<(&'a mut [u8], *mut T)> { + let offset = buf.as_ptr().align_offset(align_of::<*mut T>()); + let l = size_of::<*mut c_char>() * v.len(); + if buf.len() < offset + l { + return Err(Errno::ERANGE.into()); + } + + let buf = &mut buf[offset..]; + let dest = buf.as_mut_ptr().cast::(); + unsafe { + std::ptr::copy_nonoverlapping(v.as_ptr() as *const T, dest, l); + } + Ok((&mut buf[l..], dest)) +} + pub fn set_if_valid(ptr: *mut T, val: T) { if !ptr.is_null() { unsafe { *ptr = val }; } } -pub fn block_on(f: F) -> Result +pub fn block_on(f: F) -> anyhow::Result where F: std::future::Future, { @@ -224,6 +76,6 @@ where use std::ops::Deref; match RUNTIME.deref() { Ok(rt) => Ok(rt.block_on(async { f.await })), - Err(e) => bail!("Unable to start tokio runtime: {}", e), + Err(e) => anyhow::bail!("Unable to start tokio runtime: {}", e), } } diff --git a/src/nss_api.rs b/src/nss_api.rs index f8fca07..c1c2a7b 100644 --- a/src/nss_api.rs +++ b/src/nss_api.rs @@ -2,10 +2,9 @@ // SPDX-License-Identifier: GPL-3.0-or-later use { - crate::gethostbyaddr::gethostbyaddr2_r, - crate::gethostbyname, crate::helpers::{block_on, set_if_valid}, crate::nss_bindings::{gaih_addrtuple, nss_status, HErrno}, + crate::{gethostbyaddr, gethostbyname}, libc::{hostent, size_t, socklen_t, AF_INET}, nix::errno::Errno, std::os::raw::{c_char, c_int, c_void}, @@ -146,7 +145,7 @@ pub unsafe extern "C" fn _nss_malcontent_gethostbyaddr2_r( set_if_valid(h_errnop, HErrno::Success); match block_on(async { - gethostbyaddr2_r(addr, len, af, host, buffer, buflen, errnop, h_errnop, ttlp).await + gethostbyaddr::with(addr, len, af, host, buffer, buflen, errnop, h_errnop, ttlp).await }) { Ok(status) => status, Err(runtime_error) => { diff --git a/tests/integration_test.rs b/tests/integration_test.rs index f1ba24f..f8bda3b 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -151,6 +151,7 @@ fork_test! { } #[test] + #[ignore] fn privileged_user_bypasses_restrictions() -> Result<()> { common::setup()?;