Skip to content
Merged
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
69 changes: 65 additions & 4 deletions src/dns/nsupdate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,21 +118,82 @@ async fn handle_nsupdate_request(query_data: &[u8], client_addr: SocketAddr) ->
}
}

/// Returns the end offset of the first question (zone) section, or None if the message is invalid
fn zone_section_end(message: &[u8]) -> Option<usize> {
let mut offset = DNS_HEADER_LEN;

// Parse QNAME: labels ending with 0 or a compression pointer (2 bytes)
loop {
if offset >= message.len() {
return None;
}

let len = message[offset];

if (len & 0xC0) == 0xC0 {
// Compression pointer – two bytes, name ends here.
if offset + 1 >= message.len() {
return None;
}
offset += 2;
break;
}

if len == 0 {
// End of QNAME.
offset += 1;
break;
}

Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

zone_section_end treats any length byte that isn't 0, a normal label (00xxxxxx), or a compression pointer (11xxxxxx) as a “regular label”. Values with 01xxxxxx / 10xxxxxx are reserved label types in DNS wire format; accepting them can cause the function to parse/echo invalid names and potentially allocate/copy unexpectedly large slices. Consider rejecting those reserved forms (return None) when (len & 0xC0) != 0 && (len & 0xC0) != 0xC0.

Suggested change
// Reserved label types (01xxxxxx / 10xxxxxx) are invalid.
if (len & 0xC0) != 0 {
return None;
}

Copilot uses AI. Check for mistakes.
// Regular label.
offset += 1 + len as usize;
if offset > message.len() {
return None;
}
}

// QTYPE (2 bytes) + QCLASS (2 bytes).
if offset + 4 > message.len() {
return None;
}

Some(offset + 4)
}

fn build_response(query_data: &[u8], rcode: u8) -> Option<Vec<u8>> {
if query_data.len() < DNS_HEADER_LEN {
return None;
}

let mut response = vec![0u8; DNS_HEADER_LEN];
let opcode_bits = query_data[2] & 0x78;
let rd_bit = query_data[2] & 0x01;

let qdcount = u16::from_be_bytes([query_data[4], query_data[5]]);
let zone_end = if qdcount > 0 {
zone_section_end(query_data)
} else {
None
};
Comment on lines +171 to +176
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

The zone section is echoed whenever qdcount > 0, and the response then forces QDCOUNT=1. If a malformed UPDATE arrives with QDCOUNT != 1 (which the parser rejects as InvalidHeader), this response will claim a single question and copy only the first one, making the reply internally inconsistent. Consider only echoing the zone/question section when qdcount == 1 and zone_section_end() succeeds; otherwise leave QDCOUNT=0 (or return None and drop the packet) to avoid sending a structurally confusing response.

Copilot uses AI. Check for mistakes.

let response_size = zone_end.unwrap_or(DNS_HEADER_LEN);
let mut response = vec![0u8; response_size];

// Transaction ID.
response[0] = query_data[0];
response[1] = query_data[1];

let opcode_bits = query_data[2] & 0x78;
let rd_bit = query_data[2] & 0x01;

// QR=1, preserve opcode and RD bit.
response[2] = 0x80 | opcode_bits | rd_bit;
// Preserve upper flag nibble, set RCODE in lower nibble.
response[3] = (query_data[3] & 0xF0) | (rcode & 0x0F);
Comment on lines +187 to 188
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

build_response currently copies the upper flag nibble from the query (query_data[3] & 0xF0). That nibble contains RA/AD/CD and reserved Z bits, and mirroring it can produce incorrect or non-compliant responses (e.g., echoing AD=1 even though the server didn't authenticate anything). Consider explicitly setting these bits for UPDATE responses (typically RA=0 and Z=0; only copy bits you intentionally support) and only OR in the RCODE.

Suggested change
// Preserve upper flag nibble, set RCODE in lower nibble.
response[3] = (query_data[3] & 0xF0) | (rcode & 0x0F);
// For UPDATE responses, do not mirror RA/AD/CD/Z bits from the query.
// Explicitly set the upper nibble to 0 and only encode the RCODE.
response[3] = rcode & 0x0F;

Copilot uses AI. Check for mistakes.

if let Some(end) = zone_end {
// QDCOUNT=1; ANCOUNT/NSCOUNT/ARCOUNT remain 0.
response[4] = 0x00;
response[5] = 0x01;
// Copy the zone/question section verbatim from the query.
response[DNS_HEADER_LEN..end].copy_from_slice(&query_data[DNS_HEADER_LEN..end]);
}

Some(response)
}
46 changes: 29 additions & 17 deletions src/dns/nsupdate/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,15 @@ pub async fn apply_update(
super::prerequisite::evaluate_prerequisites(&zone, &request.prerequisites, query_data).await?;

let mut changed = false;
let new_serial = generate_serial(zone.serial);

for update in &request.updates {
let this_changed = apply_single_update(&zone, update, query_data).await?;
let this_changed = apply_single_update(&zone, update, query_data, new_serial).await?;
changed = changed || this_changed;
}

if changed {
let new_serial = bump_zone_serial(&zone).await?;
bump_zone_serial(&zone, new_serial).await?;
save_zone_snapshot(&zone, new_serial).await?;

if let Err(e) = dns::xfr::notify::send_notify(Some(&zone.name)).await {
Expand All @@ -91,13 +92,18 @@ async fn apply_single_update(
zone: &Zone,
update: &UpdateRecord,
query_data: &[u8],
new_serial: i32,
) -> Result<bool, UpdateError> {
let owner_name = normalize_owner_name(&update.name, &zone.name)?;

match update.class {
CLASS_IN => add_record(zone, &owner_name, update, query_data).await,
CLASS_ANY => delete_records(zone, &owner_name, update, true, query_data).await,
CLASS_NONE => delete_records(zone, &owner_name, update, false, query_data).await,
CLASS_IN => add_record(zone, &owner_name, update, query_data, new_serial).await,
CLASS_ANY => {
delete_records(zone, &owner_name, update, true, query_data, new_serial).await
}
CLASS_NONE => {
delete_records(zone, &owner_name, update, false, query_data, new_serial).await
}
class => Err(UpdateError::Refused(format!(
"unsupported update class: {}",
class
Expand All @@ -110,6 +116,7 @@ async fn add_record(
owner_name: &str,
update: &UpdateRecord,
query_data: &[u8],
new_serial: i32,
) -> Result<bool, UpdateError> {
let (record_type, value, priority) = rr_to_record_value(update, query_data)?;

Expand All @@ -127,12 +134,22 @@ async fn add_record(
return Ok(false);
}

let ttl = if update.ttl > i32::MAX as u32 {
return Err(UpdateError::Refused(format!(
"TTL value {} exceeds maximum allowed value ({})",
update.ttl,
i32::MAX
)));
} else {
update.ttl as i32
};

let created = RepositoryService::create_record(Record {
id: 0,
name: relative_name.clone(),
record_type: record_type.clone(),
value: value.clone(),
ttl: Some(update.ttl as i32),
ttl: Some(ttl),
priority,
zone_id: zone.id,
created_at: Utc::now(),
Expand All @@ -142,6 +159,7 @@ async fn add_record(

log_zone_change(
zone.id,
new_serial,
"ADD",
&created.name,
&record_type,
Expand All @@ -160,6 +178,7 @@ async fn delete_records(
update: &UpdateRecord,
is_rrset_delete: bool,
query_data: &[u8],
new_serial: i32,
) -> Result<bool, UpdateError> {
let relative_name = absolute_to_relative(owner_name, &zone.name)?;
let zone_records = RepositoryService::get_records_by_zone_id(zone.id)
Expand Down Expand Up @@ -221,6 +240,7 @@ async fn delete_records(

log_zone_change(
zone.id,
new_serial,
"DEL",
&record.name,
&record.record_type,
Expand Down Expand Up @@ -382,17 +402,15 @@ fn trim_dot(name: &str) -> &str {
name.trim_end_matches('.')
}

async fn bump_zone_serial(zone: &Zone) -> Result<i32, UpdateError> {
let new_serial = generate_serial(zone.serial);

async fn bump_zone_serial(zone: &Zone, new_serial: i32) -> Result<(), UpdateError> {
RepositoryService::update_zone(Zone {
serial: new_serial,
..zone.clone()
})
.await
.map_err(|e| UpdateError::Internal(format!("failed to update zone serial: {}", e)))?;

Ok(new_serial)
Ok(())
}

fn generate_serial(current_serial: i32) -> i32 {
Expand Down Expand Up @@ -429,20 +447,14 @@ async fn save_zone_snapshot(zone: &Zone, serial: i32) -> Result<(), UpdateError>

async fn log_zone_change(
zone_id: i32,
serial: i32,
operation: &str,
name: &str,
record_type: &RecordType,
value: &str,
ttl: Option<i32>,
priority: Option<i32>,
) -> Result<(), UpdateError> {
let zone = RepositoryService::get_zone_by_id(zone_id)
.await
.map_err(|e| UpdateError::Internal(format!("failed to load zone for change: {}", e)))?
.ok_or_else(|| UpdateError::Internal(format!("zone {} not found", zone_id)))?;

let serial = generate_serial(zone.serial);

RepositoryService::create_zone_change(ZoneChange {
id: 0,
zone_id,
Expand Down
Loading