mirror of
https://github.com/outbackdingo/firezone.git
synced 2026-01-27 10:18:54 +00:00
4ae64f0257
When forwarding DNS queries, we need to remember the original source socket in order to send the response back. Previously, this mapping was indexed by the DNS query ID. As it turns out, at least Windows doesn't have a global DNS query ID counter and may reuse them across different DNS servers. If that happens and two of these queries overlap, then we match the wrong responses together. In the best case, this produces bad DNS results on the client. In the worst case, those queries were for DNS servers with different IP versions in which case we triggered a panic in connlib further down the stack where we created the IP packet for the response. To fix this, we first and foremost remove the explicit `panic!` from the `make::` functions in `ip-packet`. Originally, these functions were only used in tests but we started to use them in production code too and unfortunately forgot about this panic. By introducing a `Result`, all call-sites are made aware that this can fail. Second, we fix the actual indexing into the data structure for forwarded DNS queries to also include the DNS server's socket. This ensures we don't treat the DNS query IDs as globally unique. Third, we replace the panicking path in `try_handle_forwarded_dns_response` with a log statement, meaning if the above assumption turns out wrong for some reason, we still don't panic and simply don't handle the packet.