[PR #5477] [management] Fix DNS peer IP corruption to ::1 during migration #23122

Open
opened 2026-08-05 06:06:49 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/5477
Author: @John-Dixon-IV
Created: 2/28/2026
Status: 🔄 Open

Base: mainHead: fix/dns-ipv6-loopback-corruption


📝 Commits (1)

  • ad4560d [management] Fix DNS peer IP corruption to ::1 during migration

📊 Changes

4 files changed (+87 additions, -2 deletions)

View changed files

📝 management/server/migration/migration.go (+1 -2)
📝 management/server/migration/migration_test.go (+30 -0)
📝 management/server/types/account.go (+5 -0)
📝 management/server/types/account_test.go (+51 -0)

📄 Description

Summary

  • Root cause fix: The blob-to-JSON migration for peer IPs silently fell back to net.IPv6loopback (::1) when parsing failed, permanently corrupting the peer's IP in the database. Replaced with an error return so invalid data is caught at migration time.
  • Defense-in-depth: Added IP validation in GetPeersCustomZone — peers with nil, IPv6, loopback, or unspecified IPs are now skipped with a logged error instead of generating invalid A records that miekg/dns silently rejects.
  • No client-side changes needed: The local resolver's AAAA handling already correctly returns NODATA when only A records exist.

Impact

Peers with corrupted IPs (e.g. from failed blob migration on hosts with IPv6 disabled) would lose their DNS records entirely. On Windows clients, this broke mapped drives because Windows would fall through to upstream DNS and prefer IPv6 loopback over the correct WireGuard IP.

Test plan

  • TestMigrateNetIPFieldFromBlobToJSON_WithInvalidBlobData — verifies migration fails on corrupt IP blobs instead of silently using ::1
  • TestGetPeersCustomZone_InvalidIPs — verifies peers with nil/IPv6/loopback/unspecified IPs are skipped
  • All existing migration tests pass (17/17)
  • All existing account types tests pass (78/78)

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling during IP field migration: invalid IP data now returns detailed error messages identifying the affected row instead of silently applying default values.
    • Added IP validation in DNS zone generation: peer IPs are now validated to ensure they are valid IPv4 addresses and properly formatted before DNS record creation.
  • Tests

    • Added test coverage for invalid IP data handling during migrations and DNS operations.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/5477 **Author:** [@John-Dixon-IV](https://github.com/John-Dixon-IV) **Created:** 2/28/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/dns-ipv6-loopback-corruption` --- ### 📝 Commits (1) - [`ad4560d`](https://github.com/netbirdio/netbird/commit/ad4560d8f0c9528cd4b720c1b40015148ec7f0a7) [management] Fix DNS peer IP corruption to ::1 during migration ### 📊 Changes **4 files changed** (+87 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `management/server/migration/migration.go` (+1 -2) 📝 `management/server/migration/migration_test.go` (+30 -0) 📝 `management/server/types/account.go` (+5 -0) 📝 `management/server/types/account_test.go` (+51 -0) </details> ### 📄 Description ## Summary - **Root cause fix:** The blob-to-JSON migration for peer IPs silently fell back to `net.IPv6loopback` (`::1`) when parsing failed, permanently corrupting the peer's IP in the database. Replaced with an error return so invalid data is caught at migration time. - **Defense-in-depth:** Added IP validation in `GetPeersCustomZone` — peers with nil, IPv6, loopback, or unspecified IPs are now skipped with a logged error instead of generating invalid A records that `miekg/dns` silently rejects. - **No client-side changes needed:** The local resolver's AAAA handling already correctly returns NODATA when only A records exist. ## Impact Peers with corrupted IPs (e.g. from failed blob migration on hosts with IPv6 disabled) would lose their DNS records entirely. On Windows clients, this broke mapped drives because Windows would fall through to upstream DNS and prefer IPv6 loopback over the correct WireGuard IP. ## Test plan - [x] `TestMigrateNetIPFieldFromBlobToJSON_WithInvalidBlobData` — verifies migration fails on corrupt IP blobs instead of silently using `::1` - [x] `TestGetPeersCustomZone_InvalidIPs` — verifies peers with nil/IPv6/loopback/unspecified IPs are skipped - [x] All existing migration tests pass (17/17) - [x] All existing account types tests pass (78/78) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved error handling during IP field migration: invalid IP data now returns detailed error messages identifying the affected row instead of silently applying default values. * Added IP validation in DNS zone generation: peer IPs are now validated to ensure they are valid IPv4 addresses and properly formatted before DNS record creation. * **Tests** * Added test coverage for invalid IP data handling during migrations and DNS operations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 06:06:49 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#23122