nzrdhcp: make it actually work

* Check the DHCP options for the requested IPv4 address
* Update yiaddr, not siaddr or ciaddr
* Read the RFC a tenth time. I think I've got it now

nzrd/src/ctx.rs

@@ -3,6 +3,7 @@ use diesel::{
     SqliteConnection,
 };
 use diesel_migrations::{embed_migrations, EmbeddedMigrations, MigrationHarness};
+use log::trace;
 use nzr_virt::{vol, Connection};
 use std::ops::Deref;
 use thiserror::Error;
@@ -64,6 +65,7 @@ impl InnerCtx {
             baseimg: conn.get_pool(&config.storage.base_image_pool).await?,
         };
 
+        trace!("Connecting to database");
         let db_uri = config.db_uri.clone();
         let sqldb = tokio::task::spawn_blocking(|| {
             let manager = ConnectionManager::<SqliteConnection>::new(db_uri);
@@ -74,6 +76,7 @@ impl InnerCtx {
         .unwrap()?;
 
         {
+            trace!("Running pending migrations");
             let mut conn = sqldb.get()?;
             tokio::task::spawn_blocking(move || {
                 conn.run_pending_migrations(MIGRATIONS)

nzrdhcp/src/main.rs

@@ -5,7 +5,7 @@ use std::{net::Ipv4Addr, process::ExitCode};
 use ctx::Context;
 use dhcproto::{
     v4::{DhcpOption, Message, MessageType, Opcode, OptionCode},
-    Decodable, Decoder,
+    Decodable, Decoder, Encodable, Encoder,
 };
 use nzr_api::{config::Config, net::mac::MacAddr};
 use std::net::SocketAddr;
@@ -16,9 +16,9 @@ const DEFAULT_LEASE: u32 = 86400;
 
 fn make_reply(msg: &Message, msg_type: MessageType, lease_addr: Option<Ipv4Addr>) -> Message {
     let mut resp = Message::new(
-        EMPTY_V4,
         EMPTY_V4,
         lease_addr.unwrap_or(EMPTY_V4),
+        EMPTY_V4,
         msg.giaddr(),
         msg.chaddr(),
     );
@@ -46,6 +46,7 @@ async fn handle_message(ctx: &Context, from: SocketAddr, msg: &Message) {
         tracing::info!("Received DHCP payload with invalid addr (different media type?)");
         return;
     };
+    tracing::debug!("Client MAC is {client_mac}!!!");
 
     let instance = match ctx.instance_by_mac(client_mac).await {
         Ok(Some(i)) => i,
@@ -59,6 +60,11 @@ async fn handle_message(ctx: &Context, from: SocketAddr, msg: &Message) {
         }
     };
 
+    tracing::info!(
+        "Recieved {msg_type:?} from {client_mac} (assuming {})",
+        &instance.name
+    );
+
     let mut lease_time = None;
     let mut nak = false;
 
@@ -68,12 +74,18 @@ async fn handle_message(ctx: &Context, from: SocketAddr, msg: &Message) {
             make_reply(msg, MessageType::Offer, Some(instance.lease.addr.addr))
         }
         MessageType::Request => {
-            if msg.ciaddr() != instance.lease.addr.addr {
+            if let Some(DhcpOption::RequestedIpAddress(addr)) =
+                msg.opts().get(OptionCode::RequestedIpAddress)
+            {
+                if *addr == instance.lease.addr.addr {
+                    make_reply(msg, MessageType::Ack, Some(instance.lease.addr.addr))
+                } else {
+                    nak = true;
+                    make_reply(msg, MessageType::Nak, None)
+                }
+            } else {
                 nak = true;
                 make_reply(msg, MessageType::Nak, None)
-            } else {
-                lease_time = Some(DEFAULT_LEASE);
-                make_reply(msg, MessageType::Ack, Some(instance.lease.addr.addr))
             }
         }
         MessageType::Decline => {
@@ -86,57 +98,80 @@ async fn handle_message(ctx: &Context, from: SocketAddr, msg: &Message) {
         }
         MessageType::Release => {
             // We only provide static leases
-            tracing::trace!("Ignoring DHCPRELEASE");
+            tracing::debug!("Ignoring DHCPRELEASE");
             return;
         }
         MessageType::Inform => make_reply(msg, MessageType::Ack, None),
         other => {
-            tracing::trace!("Received unhandled message {other:?}");
+            tracing::info!("Received unhandled message {other:?}");
             return;
         }
     };
 
-    let opts = response.opts_mut();
+    {
-    let giaddr = if msg.giaddr().is_unspecified() {
+        let opts = response.opts_mut();
-        todo!("no relay??")
+        let giaddr = if msg.giaddr().is_unspecified() {
-    } else {
+            todo!("no relay??")
-        msg.giaddr()
+        } else {
-    };
+            msg.giaddr()
-
-    opts.insert(DhcpOption::ServerIdentifier(giaddr));
-    if let Some(time) = lease_time {
-        opts.insert(DhcpOption::AddressLeaseTime(time));
-    }
-
-    if !nak {
-        // Get general networking info
-        let subnet = match ctx.get_subnet(&instance.lease.subnet).await {
-            Ok(Some(net)) => net,
-            Ok(None) => {
-                tracing::error!("nzrd says '{}' isn't a subnet", &instance.lease.subnet);
-                return;
-            }
-            Err(err) => {
-                tracing::error!("Error getting subnet: {err}");
-                return;
-            }
         };
 
-        opts.insert(DhcpOption::Hostname(instance.name.clone()));
+        opts.insert(DhcpOption::ServerIdentifier(giaddr));
-
+        if let Some(time) = lease_time {
-        if !subnet.dns.is_empty() {
+            opts.insert(DhcpOption::AddressLeaseTime(time));
-            opts.insert(DhcpOption::DomainNameServer(subnet.dns));
         }
 
-        if let Some(name) = subnet.domain_name {
+        if !nak {
-            opts.insert(DhcpOption::DomainName(name.to_utf8()));
+            // Get general networking info
-        }
+            let subnet = match ctx.get_subnet(&instance.lease.subnet).await {
+                Ok(Some(net)) => net,
+                Ok(None) => {
+                    tracing::error!("nzrd says '{}' isn't a subnet", &instance.lease.subnet);
+                    return;
+                }
+                Err(err) => {
+                    tracing::error!("Error getting subnet: {err}");
+                    return;
+                }
+            };
 
-        if let Some(gw) = subnet.gateway4 {
+            opts.insert(DhcpOption::Hostname(instance.name.clone()));
-            opts.insert(DhcpOption::Router(Vec::from(&[gw])));
-        }
 
-        opts.insert(DhcpOption::SubnetMask(instance.lease.addr.netmask()));
+            if !subnet.dns.is_empty() {
+                opts.insert(DhcpOption::DomainNameServer(subnet.dns));
+            }
+
+            if let Some(name) = subnet.domain_name {
+                opts.insert(DhcpOption::DomainName(name.to_utf8()));
+            }
+
+            if let Some(gw) = subnet.gateway4 {
+                opts.insert(DhcpOption::Router(Vec::from(&[gw])));
+            }
+
+            opts.insert(DhcpOption::SubnetMask(instance.lease.addr.netmask()));
+        }
+    }
+
+    tracing::info!(
+        "Sending message {:?} with yiaddr {}",
+        response
+            .opts()
+            .get(OptionCode::MessageType)
+            .unwrap_or(&DhcpOption::End),
+        response.yiaddr()
+    );
+
+    // unicast it back
+    let mut resp_buf = Vec::new();
+    let mut enc = Encoder::new(&mut resp_buf);
+    if let Err(err) = response.encode(&mut enc) {
+        tracing::error!("Couldn't encode response: {err}");
+        return;
+    }
+
+    if let Err(err) = ctx.sock().send_to(&resp_buf, from).await {
+        tracing::error!("Couldn't send response: {err}");
     }
 }