Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: captive portal dns (+lwip apis) #20

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
305 changes: 305 additions & 0 deletions src/dns.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,305 @@
use core::mem;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must say, I LOVE the idea of having a Captive Portal!
What I don't love that much:

  • Overall, the code feels very low level (as if we are coding in C, via Rust) - I'll comment more below
  • It is part of esp-idf-svc but I'm not sure what it is doing here? This is a generic DNS server, which can - just as well - be coded against Rust's STD APIs (correct me if I'm wrong!) - or better yet - against the embedded-nal so that it does have a no_std story as well. (embedded-nal traits do have STD implementations too, and if these are lacking, we can implement / patch / upstream our own changes.). So I'm very much in favor of moving this into embedded-svc (which depends on no-std-net already anyway) - possibly behind a feature toggle.
  • As for what would be the use-case of that? Well, I might want to use a captive dns portal on the Raspberry Pi too?

Copy link
Contributor Author

@ForsakenHarmony ForsakenHarmony Nov 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is indeed C converted to Rust, but not quite at the point where I'm super happy with the "Rustyness" of it yet

Sure, it would be nice to have it generic.
But my use case wasn't quite as generic, so I implemented it specifically for the esp right now.

embedded-nal seems nice, hadn't heard of it before.

I feel like someone else must have already implemented a captive portal dns in rust that's more generic, though. (not that I checked and not that I'm opposed to make this more generic)

This PR also doesn't have to be merged, it was more of an "are you interested in having this here", because I had already built it


use anyhow::*;
use log::*;

use esp_idf_sys::c_types::*;
use esp_idf_sys::*;

use crate::private::cstr::{CStr, CString};
use crate::task::{TaskConfig, TaskHandle};
use crate::lwip;

/// 0.0.0.0
const IPADDR_ANY: u32 = 0x00000000;

const DNS_PORT: c_ushort = 53;
const DNS_MAX_LEN: usize = 256;

const OPCODE_MASK: u16 = 0x7800;
const QR_FLAG: u16 = 1 << 7;
const QD_TYPE_A: u16 = 0x0001;
const ANS_TTL_SEC: u32 = 300;

pub struct CaptivePortalDns {
task_handle: Option<TaskHandle>,
}

impl CaptivePortalDns {
pub fn new() -> Self {
CaptivePortalDns { task_handle: None }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the desire to stay no_std compatible is what brought you into exposing bits and pieces for FreeRTOS? How about we take a simple trait instead, that would allow us to spawn a thread? We can even provide an implementation of the trait when Rust STD is enabled, and provide additional constructor which does not take the trait, for STD mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the desire to stay no_std compatible is what brought you into exposing bits and pieces for FreeRTOS?
Yes

How about we take a simple trait instead, that would allow us to spawn a thread? We can even provide an implementation of the trait when Rust STD is enabled, and provide additional constructor which does not take the trait, for STD mode.

As in, implement it with std::thread when we're using std and otherwise add an impl with FreeRTOS?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in implement it with std::thread when we are using std and don't provide an implementation at all for no_std. With no_std, user would be on their own to provide an implementation in whatever way they feel like.

It boils down to the following: we have to keep the esp-idf-svc surface small, clean and easy to explain by exposing Rust-safe, self-contained pieces of functionality, - ideally - behind generic traits. The emphasis being on safe and self-contained. We cannot just go there and expose random bits and pieces of all the raw level unsafe APIs that are out there in the ESP-IDF. For example, in your "basic freertos task api" what you have exposed from FreeRTOS is just a way to start a task (and that is always pinned to a core, if I'm not mistaken). Is that API generic enough so that folks can use it without calling other, unsafe Freertos APIs? Well, no. For one, the queueing and synchronization primitives of freertos are not exposed as safe APIs, because you did not need them for your particular case. But very likely people will need these to do something meaningful with freertos. So, how are we evolving your Freertos code? Is it going to - at some point in time - reach a feature parity with the unsafe freertos APIs?

  • If yes, then we are basically creating a whole Freertos wrapper, and shouldn't we then instead work towards making some of the Freertos Rust wrappers interop-ing with esp-idf-svc interop-ing with esp-idf-sys?
  • If no, aren't we in a better position by saying to folks that there are NO freertos safe APIs, and they should just use the unsafe bindings which are out there in esp-idf-sys?

}

pub fn start(&mut self) -> Result<()> {
if self.task_handle.is_some() {
bail!("dns server is already running");
}

let handle = TaskConfig::default()
.priority(5)
.spawn("dns_server", dns_server_task)?;

self.task_handle = Some(handle);
Ok(())
}

pub fn stop(&mut self) -> Result<()> {
if let Some(handle) = self.task_handle.take() {
handle.stop();
Ok(())
} else {
Err(anyhow!("dns task already stopped or was never started"))
}
}
}

impl Drop for CaptivePortalDns {
fn drop(&mut self) {
if self.task_handle.is_some() {
self.stop().unwrap();
}
}
}

/// DNS Header Packet
#[repr(C, packed)]
struct DnsHeader {
id: u16,
flags: u16,
qd_count: u16,
an_count: u16,
ns_count: u16,
ar_count: u16,
}

/// DNS Question Packet
#[repr(C)]
struct DnsQuestion {
typ: u16,
class: u16,
}

/// DNS Answer Packet
#[repr(C, packed)]
struct DnsAnswer {
name_ptr: u16,
typ: u16,
class: u16,
ttl: u32,
addr_len: u16,
ip_addr: u32,
}

fn parse_dns_name(raw_name: *mut u8, parsed_name: &mut [u8]) -> *mut u8 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the next function are littered with unsafe statements. Would it be possible to stay in Rust safe-land for parsing this stuff? Only if possible?

let mut label = raw_name;
let parsed_name_max_len = parsed_name.len();
let mut name_itr = parsed_name.iter_mut();
let mut name_len: usize = 0;

loop {
let sub_name_len = unsafe { *label as c_int };
// (len + 1) since we are adding a '.'
name_len += (sub_name_len + 1) as usize;
if name_len > parsed_name_max_len {
return core::ptr::null_mut();
}

// Copy the sub name that follows the the label
for i in 0..sub_name_len {
let ptr = name_itr.next().unwrap();
*ptr = unsafe { *label.offset((i + 1) as isize) };
}
*name_itr.next().unwrap() = '.' as u8;
label = unsafe { label.offset((sub_name_len + 1) as isize) };

if unsafe { *label == 0 } {
break;
}
}

// Terminate the final string, replacing the last '.'
parsed_name[name_len - 1] = '\0' as u8;
// Return pointer to first char after the name
return unsafe { label.offset(1) };
}

fn parse_dns_request(
req: &mut [u8],
req_len: usize,
dns_reply: &mut [u8],
dns_reply_max_len: usize,
) -> Option<usize> {
if req_len > dns_reply_max_len {
return None;
}

// Prepare the reply
dns_reply.fill(0);
(&mut dns_reply[0..req_len]).copy_from_slice(&req[0..req_len]);

let header_len = mem::size_of::<DnsHeader>();
let (header_bytes, rest) = dns_reply.split_at_mut(header_len);

// Endianess of NW packet different from chip
let header = unsafe {
header_bytes
.as_mut_ptr()
.cast::<DnsHeader>()
.as_mut()
.unwrap()
};

debug!(
"DNS query with header id: 0x{:X}, flags: 0x{:X}, qd_count: {}",
ntohs(header.id),
ntohs(header.flags),
ntohs(header.qd_count)
);

// Not a standard query
if (header.flags & OPCODE_MASK) != 0 {
return None;
}

// Set question response flag
header.flags |= QR_FLAG;

let qd_count = ntohs(header.qd_count);
header.an_count = htons(qd_count);

let reply_len = qd_count as usize * mem::size_of::<DnsAnswer>() + req_len;
if reply_len > dns_reply_max_len {
return None;
}

// Pointer to current answer and question
let (questions, answers) = rest.split_at_mut(req_len - header_len);
let cur_qd_ptr = questions.as_mut_ptr();
let mut cur_ans_ptr = answers.as_mut_ptr();
let mut name: [u8; 128] = [0; 128];

// Respond to all questions with the ESP32's IP address
for i in 0..qd_count {
debug!("answering question {}", i);
let name_end_ptr = parse_dns_name(cur_qd_ptr, &mut name);
if name_end_ptr.is_null() {
error!("failed to parse DNS question: {:?}", unsafe {
CStr::from_ptr(cur_qd_ptr as _)
});
return None;
}

let question = unsafe { name_end_ptr.cast::<DnsQuestion>().as_mut().unwrap() };
let qd_type = ntohs(question.typ);
let qd_class = ntohs(question.class);

info!(
"received type: {} | class: {} | question for: {:?}",
qd_type,
qd_class,
unsafe { CStr::from_ptr(name.as_ptr() as _) }
);

if qd_type == QD_TYPE_A {
let answer = unsafe { cur_ans_ptr.cast::<DnsAnswer>().as_mut().unwrap() };

let ptr_offset = unsafe { cur_qd_ptr.offset_from(dns_reply.as_ptr()) };
answer.name_ptr = htons((0xC000 | ptr_offset) as u16);
answer.typ = htons(qd_type);
answer.class = htons(qd_class);
answer.ttl = htonl(ANS_TTL_SEC);

let mut ip_info = esp_netif_ip_info_t::default();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an implementation which is independent of ESP-IDF, we would want the IP address to be abstracted away, via a trait, or passed at construction/start time.

let c_if_key = CString::new("WIFI_AP_DEF").unwrap();
unsafe {
esp_netif_get_ip_info(
esp_netif_get_handle_from_ifkey(c_if_key.as_ptr()),
&mut ip_info,
)
};

info!(
"answer with PTR offset: 0x{:X} (0x{:X}) and IP 0x{:X}",
ntohs(answer.name_ptr),
ptr_offset,
ip_info.ip.addr,
);

answer.addr_len = htons(mem::size_of_val(&ip_info.ip.addr) as u16);
answer.ip_addr = ip_info.ip.addr;

cur_ans_ptr = unsafe { cur_ans_ptr.offset(mem::size_of::<DnsAnswer>() as isize) };
}
}

Some(reply_len)
}

fn dns_server_task() -> Result<()> {
use lwip::*;

let mut rx_buffer = [0; 128];

loop {
let dest_addr = sockaddr_in {
sin_addr: in_addr {
s_addr: htonl(IPADDR_ANY),
},
sin_family: AF_INET as u8,
sin_port: htons(DNS_PORT),
..Default::default()
};

let mut sock = Socket::open(AddressFamily::Ipv4, SocketType::Dgram, Protocol::Ip)
.context("creating socket")?;
info!("socket created");

sock.bind(&dest_addr as *const sockaddr_in as _)
.context("binding socket")?;
info!("socket bound, port {}", DNS_PORT);

loop {
info!("waiting for data");

let (len, source_addr) = sock.recv_from(&mut rx_buffer).context("recv_from")?;

// Null-terminate whatever we received
rx_buffer[len] = 0;

let mut reply = [0; DNS_MAX_LEN];
let reply_len = parse_dns_request(&mut rx_buffer, len, &mut reply, DNS_MAX_LEN);

info!(
"received {} bytes from {} | DNS reply with len: {:?}",
len, source_addr, reply_len
);

if let Some(reply_len) = reply_len {
sock.send_to(&mut reply[0..reply_len], source_addr)
.context("send_to")?;
} else {
error!("failed to prepare a DNS reply");
}
}
}
}

/// host to network byte order
fn htonl(n: u32) -> u32 {
n.to_be()
}

/// host to network byte order
fn htons(n: u16) -> u16 {
n.to_be()
}

/// network to host byte order
fn ntohl(n: u32) -> u32 {
u32::from_be(n)
}

/// network to host byte order
fn ntohs(n: u16) -> u16 {
u16::from_be(n)
}
9 changes: 8 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
#![cfg_attr(feature = "experimental", feature(generic_associated_types))] // for http, http::client, http::server, ota
#![feature(const_btree_new)]

#[cfg(any(feature = "alloc"))]
#[cfg(feature = "alloc")]
#[macro_use]
extern crate alloc;

#[cfg(feature = "alloc")]
pub mod dns;
#[cfg(feature = "alloc")]
#[cfg(any(
all(esp32, esp_idf_eth_use_esp32_emac),
Expand All @@ -25,6 +27,8 @@ pub mod httpd;
#[cfg(feature = "alloc")]
// TODO: Ideally should not need "alloc" (also for performance reasons)
pub mod log;
pub mod lwip;
pub mod misc;
#[cfg(esp_idf_config_lwip_ipv4_napt)]
pub mod napt;
pub mod netif;
Expand All @@ -43,6 +47,9 @@ pub mod nvs_storage;
pub mod ota;
pub mod ping;
pub mod sysloop;
#[cfg(feature = "alloc")]
pub mod task;
pub mod time;
#[cfg(feature = "alloc")] // TODO: Expose a subset which does not require "alloc"
pub mod wifi;

Expand Down
Loading