Skip to content

Commit 412a1c0

Browse files
committed
fix(docker): add usage instructions for rootless Podman and service ports
fix(cache): sync question section with current request to prevent mismatches fix(forward): validate response types against request types in concurrent queries
1 parent 827d7b2 commit 412a1c0

3 files changed

Lines changed: 225 additions & 5 deletions

File tree

docker/docker-compose.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@
99
# To make persistent across reboots, create a sysctl config file:
1010
# echo "net.ipv4.ip_unprivileged_port_start=53" | sudo tee /etc/sysctl.d/99-unprivileged-ports.conf
1111
# sudo sysctl --system
12+
#
13+
# Run with `--service-ports` to ensure all ports are exposed, especially for rootless Podman.
14+
# Example usage:
15+
# podman compose -f docker-compose.yml run --rm --service-ports -it lazydns
16+
# podman compose -f docker-compose.yml up -d
17+
1218

1319
services:
1420
lazydns:

src/plugins/cache.rs

Lines changed: 96 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,14 @@ impl CachePlugin {
645645
q.qclass().to_u16()
646646
);
647647

648+
trace!(
649+
"Generated cache key: {} | qname={} | qtype={:?} | qclass={:?}",
650+
key,
651+
qname_lower,
652+
q.qtype(),
653+
q.qclass()
654+
);
655+
648656
// TODO: Add EDNS0 flags if message has EDNS0
649657
// This would ensure DNSSEC queries are cached separately from non-DNSSEC
650658
// Currently, we focus on the main fix: domain name normalization
@@ -857,6 +865,11 @@ impl Plugin for CachePlugin {
857865
let response_ref = Arc::make_mut(&mut response_arc);
858866
Self::update_ttls(response_ref, STALE_RESPONSE_TTL_SECS); // stale response TTL is fixed to 5s (matches upstream)
859867
response_ref.set_id(context.request().id());
868+
// Sync question section with current request to fix question/answer mismatch
869+
response_ref.clear_questions();
870+
for q in context.request().questions() {
871+
response_ref.add_question(q.clone());
872+
}
860873
context.set_response_arc(Some(response_arc));
861874

862875
// Mark that response came from cache to prevent Phase 2 re-execution
@@ -1012,6 +1025,11 @@ impl Plugin for CachePlugin {
10121025
let response_ref = Arc::make_mut(&mut response_arc);
10131026
Self::update_ttls(response_ref, remaining_ttl);
10141027
response_ref.set_id(context.request().id());
1028+
// Sync question section with current request to fix question/answer mismatch
1029+
response_ref.clear_questions();
1030+
for q in context.request().questions() {
1031+
response_ref.add_question(q.clone());
1032+
}
10151033
context.set_response_arc(Some(response_arc));
10161034

10171035
// Mark that response came from cache to prevent Phase 2 re-execution
@@ -1125,6 +1143,11 @@ impl Plugin for CachePlugin {
11251143
let response_ref = Arc::make_mut(&mut response_arc);
11261144
Self::update_ttls(response_ref, remaining_ttl);
11271145
response_ref.set_id(context.request().id());
1146+
// Sync question section with current request to fix question/answer mismatch
1147+
response_ref.clear_questions();
1148+
for q in context.request().questions() {
1149+
response_ref.add_question(q.clone());
1150+
}
11281151
context.set_response_arc(Some(response_arc));
11291152

11301153
// Mark that response came from cache to prevent Phase 2 re-execution
@@ -1179,9 +1202,32 @@ impl Plugin for CachePlugin {
11791202
if ttl > 0 {
11801203
// Determine cache TTL: if cache_ttl is set, use it for positive answers
11811204
let cache_ttl = self.cache_ttl.unwrap_or(ttl);
1205+
1206+
// Verify cache key matches request question type
1207+
let request_qtype = context
1208+
.request()
1209+
.questions()
1210+
.first()
1211+
.map(|q| format!("{:?}", q.qtype()))
1212+
.unwrap_or_else(|| "N/A".to_string());
1213+
1214+
// Get response answer record types
1215+
let answer_types: Vec<String> = response
1216+
.answers()
1217+
.iter()
1218+
.map(|r| r.rtype())
1219+
.collect::<std::collections::HashSet<_>>()
1220+
.iter()
1221+
.map(|rt| format!("{:?}", rt))
1222+
.collect();
1223+
11821224
debug!(
1183-
"Storing response in cache: {} (message TTL: {}s, cache TTL: {}s)",
1184-
key, ttl, cache_ttl
1225+
"Storing response in cache: {} | request_qtype={} | answer_types={} | message_ttl={}s | cache_ttl={}s",
1226+
key,
1227+
request_qtype,
1228+
answer_types.join(","),
1229+
ttl,
1230+
cache_ttl
11851231
);
11861232

11871233
// Always create/replace with new entry
@@ -1909,4 +1955,52 @@ plugins:
19091955
// because we're testing with a different cache instance. But we've verified the task
19101956
// spawns and runs without errors.
19111957
}
1958+
1959+
#[tokio::test]
1960+
async fn test_cache_hit_syncs_question_section() {
1961+
// Test the critical fix: cache hit should sync question section with current request
1962+
let cache = CachePlugin::new(100);
1963+
1964+
// Create initial A query response with A question
1965+
let mut a_response = create_test_response();
1966+
a_response.clear_questions();
1967+
a_response.add_question(Question::new("example.com", RecordType::A, RecordClass::IN));
1968+
1969+
// Store in cache with key "example.com:1:1" (A query)
1970+
let entry = CacheEntry::new(a_response, 300, 300);
1971+
cache
1972+
.cache
1973+
.write()
1974+
.push("example.com:1:1".to_string(), entry);
1975+
1976+
// Verify the cached response has A question
1977+
let cached = cache.cache.write().get("example.com:1:1").cloned();
1978+
assert!(cached.is_some());
1979+
let cached_resp = cached.unwrap().response;
1980+
assert_eq!(cached_resp.questions()[0].qtype(), RecordType::A);
1981+
1982+
// Simulate what cache does: prepare response for AAAA request
1983+
// by syncing question section (this is the fix)
1984+
let mut aaaa_request = Message::new();
1985+
aaaa_request.add_question(Question::new(
1986+
"example.com",
1987+
RecordType::AAAA,
1988+
RecordClass::IN,
1989+
));
1990+
1991+
if let Some(entry) = cache.cache.write().get("example.com:1:1").cloned() {
1992+
let mut response_arc = Arc::clone(&entry.response);
1993+
let response_ref = Arc::make_mut(&mut response_arc);
1994+
1995+
// This is the fix: sync question section with current request
1996+
response_ref.clear_questions();
1997+
for q in aaaa_request.questions() {
1998+
response_ref.add_question(q.clone());
1999+
}
2000+
2001+
// Verify the question section was updated
2002+
assert_eq!(response_ref.question_count(), 1);
2003+
assert_eq!(response_ref.questions()[0].qtype(), RecordType::AAAA);
2004+
}
2005+
}
19122006
}

src/plugins/forward.rs

Lines changed: 123 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -847,11 +847,51 @@ impl ForwardPlugin {
847847
tasks.push(task);
848848
}
849849

850-
// Wait for first success
850+
// Get request qtype for validation
851+
let request_qtype = request.questions().first().map(|q| q.qtype());
852+
853+
// Wait for first success with response validation
851854
for task in tasks {
852855
if let Ok(Ok(response)) = task.await {
853-
trace!(answers = ?response.answers(), "Got fastest response in concurrent mode");
854-
return Ok(response);
856+
// CRITICAL FIX: Validate response answers match request query type
857+
// This prevents mismatched responses from concurrent queries being returned
858+
let response_valid = match request_qtype {
859+
Some(req_qtype) => {
860+
// Check if response has answers and if ANY answer matches the request type
861+
if response.answers().is_empty() {
862+
// Empty answer is valid (NODATA response)
863+
true
864+
} else {
865+
// At least one answer must match the requested type
866+
response
867+
.answers()
868+
.iter()
869+
.any(|record| record.rtype() == req_qtype)
870+
}
871+
}
872+
None => true, // No request question, accept any response
873+
};
874+
875+
if response_valid {
876+
debug!(
877+
answers = ?response.answers(),
878+
"Got fastest response in concurrent mode (validated)"
879+
);
880+
return Ok(response);
881+
} else {
882+
// Response doesn't match request type - skip and continue
883+
warn!(
884+
"Skipping concurrent response with mismatched answer type. \
885+
Request: {:?}, Response answers: {:?}",
886+
request_qtype,
887+
response
888+
.answers()
889+
.iter()
890+
.map(|r| format!("{}:{:?}", r.name(), r.rtype()))
891+
.collect::<Vec<_>>()
892+
);
893+
continue;
894+
}
855895
}
856896
}
857897

@@ -1684,4 +1724,84 @@ mod tests {
16841724
let url = format!("https://localhost:{}/dns-query", local_addr.port());
16851725
(url, handle)
16861726
}
1727+
1728+
#[test]
1729+
fn test_concurrent_response_validation_filters_mismatched_types() {
1730+
// This test verifies that execute_concurrent validates response types match request types
1731+
// Scenario: Two concurrent requests (A and AAAA) should each get correct response type
1732+
1733+
let mut req_a = Message::new();
1734+
req_a.add_question(Question::new("example.com", RecordType::A, RecordClass::IN));
1735+
1736+
let mut req_aaaa = Message::new();
1737+
req_aaaa.add_question(Question::new(
1738+
"example.com",
1739+
RecordType::AAAA,
1740+
RecordClass::IN,
1741+
));
1742+
1743+
// Create response with AAAA record when A was requested
1744+
let mut wrong_response = Message::new();
1745+
wrong_response.add_answer(ResourceRecord::new(
1746+
"example.com",
1747+
RecordType::AAAA, // Response has AAAA but request is A
1748+
RecordClass::IN,
1749+
300,
1750+
RData::AAAA("2001:db8::1".parse().unwrap()),
1751+
));
1752+
1753+
// The concurrent validator should detect this mismatch
1754+
// When request is for A type but response contains only AAAA records,
1755+
// it should be marked as invalid and skipped
1756+
let request_qtype = req_a.questions().first().map(|q| q.qtype());
1757+
1758+
let response_valid = match request_qtype {
1759+
Some(req_qtype) => {
1760+
if wrong_response.answers().is_empty() {
1761+
true
1762+
} else {
1763+
wrong_response
1764+
.answers()
1765+
.iter()
1766+
.any(|record| record.rtype() == req_qtype)
1767+
}
1768+
}
1769+
None => true,
1770+
};
1771+
1772+
// Should be invalid: A request getting AAAA response
1773+
assert!(
1774+
!response_valid,
1775+
"Response with wrong answer type should be rejected"
1776+
);
1777+
1778+
// Now test correct response is accepted
1779+
let mut correct_response = Message::new();
1780+
correct_response.add_answer(ResourceRecord::new(
1781+
"example.com",
1782+
RecordType::A, // Correct type for A request
1783+
RecordClass::IN,
1784+
300,
1785+
RData::A("192.0.2.1".parse().unwrap()),
1786+
));
1787+
1788+
let response_valid = match request_qtype {
1789+
Some(req_qtype) => {
1790+
if correct_response.answers().is_empty() {
1791+
true
1792+
} else {
1793+
correct_response
1794+
.answers()
1795+
.iter()
1796+
.any(|record| record.rtype() == req_qtype)
1797+
}
1798+
}
1799+
None => true,
1800+
};
1801+
1802+
assert!(
1803+
response_valid,
1804+
"Response with matching answer type should be accepted"
1805+
);
1806+
}
16871807
}

0 commit comments

Comments
 (0)