[GH-ISSUE #2252] infinite recursion in `DnssecDnsHandle` when nameservers use an unsupported algorithm #939

New issue

Closed

opened 2026-03-16 01:02:01 +03:00 by kerem · 1 comment

kerem commented

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

Owner

Originally created by @japaric on GitHub (Jun 20, 2024).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2252

Describe the bug
what the title says

To Reproduce

Use dns-test set up a network of nameservers whose zone files have been signed using the algorithm RSASHA1NSEC3SHA1.
Build hickory-dns with the feature dnssec-ring (+)
Tell hickory-dns to perform DNSSEC validation (#2234)
Send a query like NS com. with the AD flag set
hickory-dns will enter an infinite recursion loop that results in a stack overflow:

(..)
1718886687:DEBUG:hickory_proto::xfer::dns_handle:64:querying: . DS
1718886687:DEBUG:hickory_recursor::recursor:513:found zone . for . IN DS
(..)
1718886687:DEBUG:hickory_proto::xfer::dns_handle:64:querying: . DS
1718886687:DEBUG:hickory_recursor::recursor:513:found zone . for . IN DS
(..)
1718886687:DEBUG:hickory_proto::xfer::dns_handle:64:querying: . DS
1718886687:DEBUG:hickory_recursor::recursor:513:found zone . for . IN DS
(..)
1718886687:DEBUG:hickory_proto::xfer::dns_handle:64:querying: . DS

thread 'hickory-server-runtime' has overflowed its stack
fatal runtime error: stack overflow
Aborted (core dumped)

Once the PR for #2234 is up I think we'll have at least one test that hits this bug.

I'm not sure if this bug also affects Resolver as I have not tested that in a dns-test environment

(+) As a workaround, you can use the dnssec-openssl implementation to avoid this bug. ring does not support RSASHA1NSEC3SHA1 but openssl does.

Expected behavior
hickory-dns should not crash and instead it should respond with an error status like SERVFAIL

System:

OS: Debian
Architecture: x86_64
Version bookworm
rustc version: 1.79

Version:
Crate: proto, recursor, server, hickory-dns
Version: 02a8668fd5

Originally created by @japaric on GitHub (Jun 20, 2024). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/2252 **Describe the bug** what the title says **To Reproduce** 1. Use `dns-test` set up a network of nameservers whose zone files have been signed using the algorithm `RSASHA1NSEC3SHA1`. 2. Build `hickory-dns` with the feature `dnssec-ring` (+) 3. Tell `hickory-dns` to perform DNSSEC validation (#2234) 4. Send a query like `NS com.` with the AD flag set 5. `hickory-dns` will enter an infinite recursion loop that results in a stack overflow: ``` console (..) 1718886687:DEBUG:hickory_proto::xfer::dns_handle:64:querying: . DS 1718886687:DEBUG:hickory_recursor::recursor:513:found zone . for . IN DS (..) 1718886687:DEBUG:hickory_proto::xfer::dns_handle:64:querying: . DS 1718886687:DEBUG:hickory_recursor::recursor:513:found zone . for . IN DS (..) 1718886687:DEBUG:hickory_proto::xfer::dns_handle:64:querying: . DS 1718886687:DEBUG:hickory_recursor::recursor:513:found zone . for . IN DS (..) 1718886687:DEBUG:hickory_proto::xfer::dns_handle:64:querying: . DS thread 'hickory-server-runtime' has overflowed its stack fatal runtime error: stack overflow Aborted (core dumped) ``` Once the PR for #2234 is up I think we'll have at least one test that hits this bug. I'm not sure if this bug also affects `Resolver` as I have not tested that in a `dns-test` environment (+) As a workaround, you can use the `dnssec-openssl` implementation to avoid this bug. [*ring* does not support RSASHA1NSEC3SHA1](https://github.com/hickory-dns/hickory-dns/blob/02a8668fd54be5c3b1a9f8d8e79393229ffea2be/crates/proto/src/rr/dnssec/public_key.rs#L412) but openssl does. **Expected behavior** `hickory-dns` should not crash and instead it should respond with an error status like SERVFAIL **System:** - OS: Debian - Architecture: x86_64 - Version bookworm - rustc version: 1.79 **Version:** Crate: `proto`, `recursor`, `server`, `hickory-dns` Version: 02a8668fd54be5c3b1a9f8d8e79393229ffea2be

kerem closed this issue

2026-03-16 01:02:06 +03:00

kerem commented

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

Author

Owner

@japaric commented on GitHub (Jul 1, 2024):

I found the source of the bug in this function

github.com/hickory-dns/hickory-dns@898f60fbeb/crates/proto/src/xfer/dnssec_dns_handle.rs (L490-L495)

this line specifically

github.com/hickory-dns/hickory-dns@898f60fbeb/crates/proto/src/xfer/dnssec_dns_handle.rs (L545)

if zone is . (root) then zone.base_name() also returns . and that causes find_ds_record to be executed with the exact same arguments as its parent. That results in infinite recursion. The request_depth mechanism in DnssecDnsHandle does not prevent the problem because even when there are lookup errors the recursive call in line 545 is executed.

I think the other condition to trigger this bug is that the query DS . must return no records which is the case in the conformance tests but I believe it's not the case in the public DNS network.

IMO, the function Zone::base_name should return an Option instead of returning . for .. That might have made this infinite recursion path more obvious when this code was authored.

I think the other condition to trigger this bug is that the query DS . must return no records

The fix is easy but to add a regression test along the fix I would need to have #2253 in place as that lets me test this path via the Recursor API. I'll prepare a separate PR that builds on top of #2253

@japaric commented on GitHub (Jul 1, 2024): I found the source of the bug in this function https://github.com/hickory-dns/hickory-dns/blob/898f60fbebf62416a49922e9490a6394d6ec8735/crates/proto/src/xfer/dnssec_dns_handle.rs#L490-L495 this line specifically https://github.com/hickory-dns/hickory-dns/blob/898f60fbebf62416a49922e9490a6394d6ec8735/crates/proto/src/xfer/dnssec_dns_handle.rs#L545 if `zone` is `.` (root) then `zone.base_name()` also returns `.` and that causes `find_ds_record` to be executed with the exact same arguments as its parent. That results in infinite recursion. The `request_depth` mechanism in `DnssecDnsHandle` does not prevent the problem because even when there are `lookup` errors the recursive call in line 545 is executed. I think the other condition to trigger this bug is that the query `DS .` must return no records which is the case in the conformance tests but I believe it's not the case in the public DNS network. IMO, the function `Zone::base_name` should return an `Option` instead of returning `.` for `.`. That might have made this infinite recursion path more obvious when this code was authored. > I think the other condition to trigger this bug is that the query `DS .` must return no records The fix is easy but to add a regression test along the fix I would need to have #2253 in place as that lets me test this path via the `Recursor` API. I'll prepare a separate PR that builds on top of #2253

kerem referenced this issue

2026-03-16 02:26:01 +03:00

[PR #932] [MERGED] update to tokio 0.2 release #1800