internal-dns: NXDOMAIN/SERVFAIL where appropriate

internal-dns/Cargo.toml

@@ -24,6 +24,7 @@ tokio = { version = "1.18", features = [ "full" ] }
 toml = "0.5"
 trust-dns-proto = "0.21"
 trust-dns-server = "0.21"
+trust-dns-client = "0.21"
 
 [dev-dependencies]
 expectorate = "1.0.5"

internal-dns/src/bin/apigen.rs

@@ -7,7 +7,7 @@ use internal_dns::dropshot_server::api;
 use std::fs::File;
 use std::io;
 
-fn usage(args: &Vec<String>) -> String {
+fn usage(args: &[String]) -> String {
     format!("{} [output path]", args[0])
 }
 

internal-dns/src/bin/dns-server.rs

@@ -27,13 +27,17 @@ struct Args {
 
     #[clap(long)]
     dns_address: SocketAddrV6,
+
+    #[clap(long)]
+    dns_zone: String,
 }
 
 #[tokio::main]
 async fn main() -> Result<(), anyhow::Error> {
     let args = Args::parse();
     let config_file = &args.config_file;
     let dns_address = &args.dns_address;
+    let zone = &args.dns_zone;
     let config_file_contents = std::fs::read_to_string(config_file)
         .with_context(|| format!("read config file {:?}", config_file))?;
     let mut config: internal_dns::Config =
@@ -55,6 +59,7 @@ async fn main() -> Result<(), anyhow::Error> {
         let log = log.clone();
         let dns_config = internal_dns::dns_server::Config {
             bind_address: dns_address.to_string(),
+            zone: zone.to_string(),
         };
         tokio::spawn(async move {
             internal_dns::dns_server::run(log, db, dns_config).await

internal-dns/src/dns_server.rs

@@ -12,7 +12,9 @@ use pretty_hex::*;
 use serde::Deserialize;
 use slog::{error, Logger};
 use tokio::net::UdpSocket;
+use trust_dns_client::rr::LowerName;
 use trust_dns_proto::op::header::Header;
+use trust_dns_proto::op::response_code::ResponseCode;
 use trust_dns_proto::rr::rdata::SRV;
 use trust_dns_proto::rr::record_data::RData;
 use trust_dns_proto::rr::record_type::RecordType;
@@ -27,6 +29,9 @@ use trust_dns_server::authority::{MessageRequest, MessageResponseBuilder};
 pub struct Config {
     /// The address to listen for DNS requests on
     pub bind_address: String,
+
+    /// The DNS zone this server presides over. This must be a valid DNS name
+    pub zone: String,
 }
 
 pub async fn run(log: Logger, db: Arc<sled::Db>, config: Config) -> Result<()> {
@@ -40,10 +45,11 @@ pub async fn run(log: Logger, db: Arc<sled::Db>, config: Config) -> Result<()> {
         let socket = socket.clone();
         let log = log.clone();
         let db = db.clone();
+        let zone = config.zone.clone();
 
-        tokio::spawn(
-            async move { handle_req(log, db, socket, src, buf).await },
-        );
+        tokio::spawn(async move {
+            handle_req(log, db, socket, src, buf, zone).await
+        });
     }
 }
 
@@ -53,6 +59,7 @@ async fn handle_req<'a, 'b, 'c>(
     socket: Arc<UdpSocket>,
     src: SocketAddr,
     buf: Vec<u8>,
+    zone: String,
 ) {
     println!("{:?}", buf.hex_dump());
 
@@ -67,21 +74,51 @@ async fn handle_req<'a, 'b, 'c>(
 
     println!("{:#?}", mr);
 
-    let rb = MessageResponseBuilder::from_message_request(&mr);
     let header = Header::response_from_request(mr.header());
+    let zone = LowerName::from(Name::from_str(&zone).unwrap());
+
+    // Ensure the query is for this zone, otherwise bail with servfail. This
+    // will cause resolvers to look to other DNS servers for this query.
+    let name = mr.query().name();
+    if !zone.zone_of(name) {
+        nack(&log, &mr, &socket, &header, &src).await;
+        return;
+    }
 
     let name = mr.query().original().name().clone();
     let key = name.to_string();
     let key = key.trim_end_matches('.');
 
+    let rb = MessageResponseBuilder::from_message_request(&mr);
+
     let bits = match db.get(key.as_bytes()) {
         Ok(Some(bits)) => bits,
-        Err(e) => {
-            error!(log, "db get: {}", e);
-            nack(&log, &mr, &socket, &header, &src).await;
+
+        // If no record is found bail with NXDOMAIN.
+        Ok(None) => {
+            let mresp = rb.error_msg(&header, ResponseCode::NXDomain);
+            let mut resp_data = Vec::new();
+            let mut enc = BinEncoder::new(&mut resp_data);
+            match mresp.destructive_emit(&mut enc) {
+                Ok(_) => {}
+                Err(e) => {
+                    error!(log, "NXDOMAIN destructive emit: {}", e);
+                    nack(&log, &mr, &socket, &header, &src).await;
+                    return;
+                }
+            }
+            match socket.send_to(&resp_data, &src).await {
+                Ok(_) => {}
+                Err(e) => {
+                    error!(log, "NXDOMAIN send: {}", e);
+                }
+            }
             return;
         }
-        _ => {
+
+        // If we encountered an error bail with SERVFAIL.
+        Err(e) => {
+            error!(log, "db get: {}", e);
             nack(&log, &mr, &socket, &header, &src).await;
             return;
         }
@@ -92,6 +129,7 @@ async fn handle_req<'a, 'b, 'c>(
             Ok(r) => r,
             Err(e) => {
                 error!(log, "deserialize record: {}", e);
+                nack(&log, &mr, &socket, &header, &src).await;
                 return;
             }
         };
@@ -166,7 +204,7 @@ async fn nack(
     src: &SocketAddr,
 ) {
     let rb = MessageResponseBuilder::from_message_request(mr);
-    let mresp = rb.build_no_records(*header);
+    let mresp = rb.error_msg(header, ResponseCode::ServFail);
     let mut resp_data = Vec::new();
     let mut enc = BinEncoder::new(&mut resp_data);
     match mresp.destructive_emit(&mut enc) {

internal-dns/tests/basic_test.rs

@@ -10,14 +10,16 @@ use internal_dns_client::{
     types::{DnsKv, DnsRecord, DnsRecordKey, Srv},
     Client,
 };
+use trust_dns_proto::op::response_code::ResponseCode;
 use trust_dns_resolver::config::{
     NameServerConfig, Protocol, ResolverConfig, ResolverOpts,
 };
+use trust_dns_resolver::error::ResolveErrorKind;
 use trust_dns_resolver::TokioAsyncResolver;
 
 #[tokio::test]
 pub async fn aaaa_crud() -> Result<(), anyhow::Error> {
-    let test_ctx = init_client_server().await?;
+    let test_ctx = init_client_server("oxide.internal".into()).await?;
     let client = &test_ctx.client;
     let resolver = &test_ctx.resolver;
 
@@ -26,7 +28,7 @@ pub async fn aaaa_crud() -> Result<(), anyhow::Error> {
     assert!(records.is_empty());
 
     // add an aaaa record
-    let name = DnsRecordKey { name: "devron.system".into() };
+    let name = DnsRecordKey { name: "devron.oxide.internal".into() };
     let addr = Ipv6Addr::new(0xfd, 0, 0, 0, 0, 0, 0, 0x1);
     let aaaa = DnsRecord::Aaaa(addr);
     client
@@ -60,7 +62,7 @@ pub async fn aaaa_crud() -> Result<(), anyhow::Error> {
 
 #[tokio::test]
 pub async fn srv_crud() -> Result<(), anyhow::Error> {
-    let test_ctx = init_client_server().await?;
+    let test_ctx = init_client_server("oxide.internal".into()).await?;
     let client = &test_ctx.client;
     let resolver = &test_ctx.resolver;
 
@@ -69,7 +71,7 @@ pub async fn srv_crud() -> Result<(), anyhow::Error> {
     assert!(records.is_empty());
 
     // add a srv record
-    let name = DnsRecordKey { name: "hromi.cluster".into() };
+    let name = DnsRecordKey { name: "hromi.oxide.internal".into() };
     let srv =
         Srv { prio: 47, weight: 74, port: 99, target: "outpost47".into() };
     let rec = DnsRecord::Srv(srv.clone());
@@ -108,6 +110,78 @@ pub async fn srv_crud() -> Result<(), anyhow::Error> {
     Ok(())
 }
 
+#[tokio::test]
+pub async fn nxdomain() -> Result<(), anyhow::Error> {
+    let test_ctx = init_client_server("oxide.internal".into()).await?;
+    let resolver = &test_ctx.resolver;
+
+    // asking for a nonexistent record within the domain of the internal DNS
+    // server should result in an NXDOMAIN
+    match resolver.lookup_ip("unicorn.oxide.internal").await {
+        Ok(unexpected) => {
+            panic!("Expected NXDOMAIN, got record {:?}", unexpected);
+        }
+        Err(e) => match e.kind() {
+            ResolveErrorKind::NoRecordsFound {
+                response_code,
+                query: _,
+                soa: _,
+                negative_ttl: _,
+                trusted: _,
+            } => match response_code {
+                ResponseCode::NXDomain => {}
+                unexpected => {
+                    panic!(
+                        "Expected NXDOMAIN, got response code {:?}",
+                        unexpected
+                    );
+                }
+            },
+            unexpected => {
+                panic!("Expected NXDOMAIN, got error {:?}", unexpected);
+            }
+        },
+    };
+
+    Ok(())
+}
+
+#[tokio::test]
+pub async fn servfail() -> Result<(), anyhow::Error> {
+    let test_ctx = init_client_server("oxide.internal".into()).await?;
+    let resolver = &test_ctx.resolver;
+
+    // asking for a record outside the domain of the internal DNS
+    // server should result in a SERVFAIL.
+    match resolver.lookup_ip("oxide.computer").await {
+        Ok(unexpected) => {
+            panic!("Expected SERVFAIL, got record {:?}", unexpected);
+        }
+        Err(e) => match e.kind() {
+            ResolveErrorKind::NoRecordsFound {
+                response_code,
+                query: _,
+                soa: _,
+                negative_ttl: _,
+                trusted: _,
+            } => match response_code {
+                ResponseCode::ServFail => {}
+                unexpected => {
+                    panic!(
+                        "Expected SERVFAIL, got response code {:?}",
+                        unexpected
+                    );
+                }
+            },
+            unexpected => {
+                panic!("Expected SERVFAIL, got error {:?}", unexpected);
+            }
+        },
+    };
+
+    Ok(())
+}
+
 struct TestContext {
     client: Client,
     resolver: TokioAsyncResolver,
@@ -122,7 +196,9 @@ impl TestContext {
     }
 }
 
-async fn init_client_server() -> Result<TestContext, anyhow::Error> {
+async fn init_client_server(
+    zone: String,
+) -> Result<TestContext, anyhow::Error> {
     // initialize dns server config
     let (tmp, config, dropshot_port, dns_port) = test_config()?;
     let log = config
@@ -160,6 +236,7 @@ async fn init_client_server() -> Result<TestContext, anyhow::Error> {
         let log = log.clone();
         let dns_config = internal_dns::dns_server::Config {
             bind_address: format!("[::1]:{}", dns_port),
+            zone,
         };
 
         tokio::spawn(async move {

smf/internal-dns/manifest.xml

@@ -13,13 +13,14 @@
   </dependency>
 
   <exec_method type='method' name='start'
-    exec='ctrun -l child -o noorphan,regent /opt/oxide/internal-dns/bin/dns-server --config-file /var/svc/manifest/site/internal-dns/config.toml --server-address %{config/server_address} --dns-address %{config/dns_address} &amp;'
+      exec='ctrun -l child -o noorphan,regent /opt/oxide/internal-dns/bin/dns-server --config-file /var/svc/manifest/site/internal-dns/config.toml --server-address %{config/server_address} --dns-address %{config/dns_address} --dns-zone %{config/dns_zone} &amp;'
     timeout_seconds='0' />
   <exec_method type='method' name='stop' exec=':kill' timeout_seconds='0' />
 
   <property_group name='config' type='application'>
     <propval name='server_address' type='astring' value='unknown' />
     <propval name='dns_address' type='astring' value='unknown' />
+    <propval name='dns_zone' type='astring' value='oxide.internal' />
   </property_group>
 
   <property_group name='startd' type='framework'>