Skip to content

desktop discovery: unmap IPv6 addresses#31227

Merged
zmb3 merged 3 commits intomasterfrom
zmb3/dns-ipv4-mapping
Sep 5, 2023
Merged

desktop discovery: unmap IPv6 addresses#31227
zmb3 merged 3 commits intomasterfrom
zmb3/dns-ipv4-mapping

Conversation

@zmb3
Copy link
Copy Markdown
Collaborator

@zmb3 zmb3 commented Aug 30, 2023

We request only IPv4 addresses when doing DNS queries for Windows desktops. In some circumstances, we can get addresses that are represented ("mapped") as IPv6 addresses. Unmap them so that Teleport always stores the IPv4 format.

We request only IPv4 addresses when doing DNS queries for Windows
desktops. In some circumstances, we can get addresses that are
represented ("mapped") as IPv6 addresses. Unmap them so that Teleport
always stores the IPv4 format.
result := make([]netip.Addr, 0, len(addrs))
for _, addr := range addrs {
if addr.Is4() || addr.Is4In6() {
result = append(result, addr.Unmap())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we worried about actual ipv6 addresses being returned here? We could just do addrs[i] = addrs[i].Unmap() if that's not the case, Unmap is a noop for ipv4 addresses.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't worried about them showing up before and then we ran in to this bug, so I'd rather be extra defensive.

@zmb3 zmb3 added this pull request to the merge queue Sep 5, 2023
Merged via the queue into master with commit e7d3358 Sep 5, 2023
@zmb3 zmb3 deleted the zmb3/dns-ipv4-mapping branch September 5, 2023 01:16
@public-teleport-github-review-bot
Copy link
Copy Markdown

@zmb3 See the table below for backport results.

Branch Result
branch/v11 Create PR
branch/v12 Create PR
branch/v13 Create PR
branch/v14 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants