[GH-ISSUE #3340] reflect dnssec_ok from request to response #1170

New issue

Open

opened 2026-03-16 01:48:01 +03:00 by kerem · 2 comments

kerem commented

2026-03-16 01:48:01 +03:00

Owner

Originally created by @bryanlarsen on GitHub (Oct 31, 2025).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/3340

Describe the bug
This feels like more correct behaviour:

diff --git a/crates/server/src/zone_handler/catalog.rs b/crates/server/src/zone_handler/catalog.rs
index 394d3b69f..39bcb8a60 100644
--- a/crates/server/src/zone_handler/catalog.rs
+++ b/crates/server/src/zone_handler/catalog.rs
@@ -72,7 +72,8 @@ impl RequestHandler for Catalog {
             // check our version against the request
             // TODO: what version are we?
             let our_version = 0;
-            resp_edns.set_dnssec_ok(true);
+            resp_edns.set_dnssec_ok(req_edns.flags().dnssec_ok);
             resp_edns.set_max_payload(req_edns.max_payload().max(512));
             resp_edns.set_version(our_version);

To Reproduce
conformance test:

use dns_test::client::{Client, DigSettings};
use dns_test::name_server::NameServer;
use dns_test::record::RecordType;
use dns_test::zone_file::SignSettings;
use dns_test::{Error, FQDN, Network};

#[test]
fn do_bit_not_set_in_request() -> Result<(), Error> {
    let network = Network::new()?;

    let ns = NameServer::new(&dns_test::SUBJECT, FQDN::ROOT, &network)?
        .sign(SignSettings::default())?
        .start()?;

    let client = Client::new(&network)?;

    // Query WITHOUT DO bit set
    let settings = DigSettings::default();
    let ans = client.dig(
        settings,
        ns.ipv4_addr(),
        RecordType::A,
        ns.fqdn(),
    )?;

    assert!(ans.status.is_noerror());

    // The response should NOT have the DO bit set since the request didn't have it
    assert!(!ans.dnssec_ok_flag,
        "Server should not set DO=1 in response when client sent DO=0");

    Ok(())
}

#[test]
fn do_bit_set_in_request() -> Result<(), Error> {
    let network = Network::new()?;

    let ns = NameServer::new(&dns_test::SUBJECT, FQDN::ROOT, &network)?
        .sign(SignSettings::default())?
        .start()?;

    let client = Client::new(&network)?;

    // Query WITH DO bit set
    let settings = *DigSettings::default().dnssec();
    let ans = client.dig(
        settings,
        ns.ipv4_addr(),
        RecordType::A,
        ns.fqdn(),
    )?;

    assert!(ans.status.is_noerror());

    // The response SHOULD have the DO bit set since the request had it
    assert!(ans.dnssec_ok_flag,
        "Server should set DO=1 in response when client sent DO=1");

    Ok(())
}

Expected behavior
The above was added to !3310, but it's not appropriate nor necessary for !3310. Marcus0x62 commented https://github.com/hickory-dns/hickory-dns/pull/3310#discussion_r2479689437 on appropriate next steps.

Version:
!3310

Originally created by @bryanlarsen on GitHub (Oct 31, 2025). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/3340 **Describe the bug** This feels like more correct behaviour: ``` diff --git a/crates/server/src/zone_handler/catalog.rs b/crates/server/src/zone_handler/catalog.rs index 394d3b69f..39bcb8a60 100644 --- a/crates/server/src/zone_handler/catalog.rs +++ b/crates/server/src/zone_handler/catalog.rs @@ -72,7 +72,8 @@ impl RequestHandler for Catalog { // check our version against the request // TODO: what version are we? let our_version = 0; - resp_edns.set_dnssec_ok(true); + resp_edns.set_dnssec_ok(req_edns.flags().dnssec_ok); resp_edns.set_max_payload(req_edns.max_payload().max(512)); resp_edns.set_version(our_version); ``` **To Reproduce** conformance test: ``` use dns_test::client::{Client, DigSettings}; use dns_test::name_server::NameServer; use dns_test::record::RecordType; use dns_test::zone_file::SignSettings; use dns_test::{Error, FQDN, Network}; #[test] fn do_bit_not_set_in_request() -> Result<(), Error> { let network = Network::new()?; let ns = NameServer::new(&dns_test::SUBJECT, FQDN::ROOT, &network)? .sign(SignSettings::default())? .start()?; let client = Client::new(&network)?; // Query WITHOUT DO bit set let settings = DigSettings::default(); let ans = client.dig( settings, ns.ipv4_addr(), RecordType::A, ns.fqdn(), )?; assert!(ans.status.is_noerror()); // The response should NOT have the DO bit set since the request didn't have it assert!(!ans.dnssec_ok_flag, "Server should not set DO=1 in response when client sent DO=0"); Ok(()) } #[test] fn do_bit_set_in_request() -> Result<(), Error> { let network = Network::new()?; let ns = NameServer::new(&dns_test::SUBJECT, FQDN::ROOT, &network)? .sign(SignSettings::default())? .start()?; let client = Client::new(&network)?; // Query WITH DO bit set let settings = *DigSettings::default().dnssec(); let ans = client.dig( settings, ns.ipv4_addr(), RecordType::A, ns.fqdn(), )?; assert!(ans.status.is_noerror()); // The response SHOULD have the DO bit set since the request had it assert!(ans.dnssec_ok_flag, "Server should set DO=1 in response when client sent DO=1"); Ok(()) } ``` **Expected behavior** The above was added to !3310, but it's not appropriate nor necessary for !3310. Marcus0x62 commented https://github.com/hickory-dns/hickory-dns/pull/3310#discussion_r2479689437 on appropriate next steps. **Version:** !3310

kerem commented

2026-03-16 01:48:07 +03:00

Author

Owner

@divergentdave commented on GitHub (Oct 31, 2025):

I couldn't find any normative text in RFC 4035 about what to do with the DO bit in responses, but the example responses in Appendix B do have the DO bit set. RFC 3225 does say "The DO bit of the query MUST be copied in the response".

@divergentdave commented on GitHub (Oct 31, 2025): I couldn't find any normative text in RFC 4035 about what to do with the DO bit in responses, but the example responses in Appendix B do have the DO bit set. RFC 3225 does say "The DO bit of the query MUST be copied in the response".

kerem commented

2026-03-16 01:48:12 +03:00

Author

Owner

@bryanlarsen commented on GitHub (Oct 31, 2025):

I've found evidence of contradictory behavior in another place in Hickory:

https://github.com/hickory-dns/hickory-dns/blob/main/crates/recursor/src/recursor.rs#L496 explicitly sets it to true despite it being passed in as a parameter to the resolve function.

Context: the maybe_strip_dnssec_records function takes record_type and query_dnssec_ok as parameters as well as the Message. Those two parameters seem redundant, since a Message should contain the query and the dnssec_ok bit. But if the response and query have different bit settings, I cannot elide it from the function.

@bryanlarsen commented on GitHub (Oct 31, 2025): I've found evidence of contradictory behavior in another place in Hickory: https://github.com/hickory-dns/hickory-dns/blob/main/crates/recursor/src/recursor.rs#L496 explicitly sets it to true despite it being passed in as a parameter to the resolve function. Context: the maybe_strip_dnssec_records function takes record_type and query_dnssec_ok as parameters as well as the Message. Those two parameters *seem* redundant, since a Message should contain the query and the dnssec_ok bit. But if the response and query have different bit settings, I cannot elide it from the function.