Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: IPNet missing network mask on Windows #12811

Closed
akevinbailey opened this issue Oct 1, 2015 · 10 comments

Comments

Projects
None yet
5 participants
@akevinbailey
Copy link

commented Oct 1, 2015

Go 1.5.1 on Windows the IPNet structure retuned by net.Interfaces().Addrs() does not return the actual network mask of the IP address.

The current code uses the puni.Address.SockaddrLength, but this only returns the default mast of the network and NOT the actual mask! Instead you need to use puni.OnLinkPrefixLength

Fix:
In the interfaceAddrTable function in interface_windows.go change:
puni.Address.SockaddrLength to puni.OnLinkPrefixLength
in lines 167, 171, 182, 196

example:

ifa := &net.IPNet{IP: make(net.IP, net.IPv4len), Mask: net.CIDRMask(int(puni.Address.SockaddrLength), 8 * net.IPv4len)}

change to

ifa := &net.IPNet{IP: make(net.IP, net.IPv4len), Mask: net.CIDRMask(int(puni.OnLinkPrefixLength), 8 * net.IPv4len)}

This will only work on Widows Vista/Windows2008 and above. For Windows XP/Windows 2003 you need to implement the following C code in Go to get the correct prefix length (network mask):

/\* Prior to Windows Vista the FirstPrefix pointed to the list with single prefix for each IP address assigned to the adapter. Order of FirstPrefix does not match order of FirstUnicastAddress, so we need to find corresponding prefix */
    IP_ADAPTER_PREFIX* prefix;
    prefix_len = 0;

    for (prefix = adapter->FirstPrefix; prefix; prefix = prefix->Next) {
      /* We want the longest matching prefix. */
      if (prefix->Address.lpSockaddr->sa_family != sa->sa_family ||
          prefix->PrefixLength <= prefix_len)
        continue;

      if (address_prefix_match(sa->sa_family, sa,
          prefix->Address.lpSockaddr, prefix->PrefixLength)) {
        prefix_len = prefix->PrefixLength;
      }
    }

    /* If there is no matching prefix information, return a single-host
     * subnet mask (e.g. 255.255.255.255 for IPv4).
     */
    if (!prefix_len)
      prefix_len = (sa->sa_family == AF_INET6) ? 128 : 32;

I'll leave this exercise up to the Go guys.

@bradfitz bradfitz changed the title Go 1.5.1 -- Bug in interface_windows.go -- Fix found net: IPNet missing network mask on Windows Oct 1, 2015

@bradfitz bradfitz added the OS-Windows label Oct 1, 2015

@bradfitz bradfitz added this to the Go1.5.2 milestone Oct 1, 2015

@bradfitz

This comment has been minimized.

Copy link
Member

commented Oct 1, 2015

@akevinbailey

This comment has been minimized.

Copy link
Author

commented Oct 1, 2015

I found another issue: The interfaceAddrTable returns Interfaces that are not active. Need to implement the follow code after for for ; paddr != nil; paddr = paddr.Next {:

if paddr.OperStatus == windows.IfOperStatusUp {
...
}

This will exclude inactive interfaces.

@alexbrainman

This comment has been minimized.

Copy link
Member

commented Nov 13, 2015

@akevinbailey please review https://go-review.googlesource.com/16854. I just followed your advice. I really don't know much about the subject. Thank you.

Alex

@gopherbot

This comment has been minimized.

Copy link

commented Nov 13, 2015

CL https://golang.org/cl/16854 mentions this issue.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2015

Is this a regression from Go 1.4? If not, I think it should be released in Go 1.6, not Go 1.5.2.

@akevinbailey

This comment has been minimized.

Copy link
Author

commented Nov 14, 2015

The network mask code looks good, but I don't have a Windows XP image to test the first prefix code. However, you left out a way to filter out the non active interfaces. See #12812

I think it would be best to merge the fix for this issue (12811) with 12812. Then test and release them together as part of Go 1.6.

@alexbrainman

This comment has been minimized.

Copy link
Member

commented Nov 16, 2015

Is this a regression from Go 1.4? ...

It is not regression from go1.4. It was introduced by CL 3024, which is part of go1.5.

The network mask code looks good, but I don't have a Windows XP image to test the first prefix code.

I have Windows XP computer. I visually checked my new netmasks looks good.

However, you left out a way to filter out the non active interfaces. See #12812

CL 16854 fixes issue #12811. It has nothing to do with issue #12812. One thing at a time. Do you have a suggestion for a test I can use for issue #12812?

I think it would be best to merge the fix for this issue (12811) with 12812. Then test and release them together as part of Go 1.6.

I disagee. These bugs are not related in any ways. It will be much easier (and quicker) to have 2 simple fixes approved, then 1 combined - bigger and more complicated. Both fixes can go into go1.6 (if they make deadline).

Alex

@akevinbailey

This comment has been minimized.

Copy link
Author

commented Nov 16, 2015

Several months ago I combined the two "fixes" in my code to get the desired functionality for my application. However, since you are the Go expert, you would know better than I what is the best way to bring this to general availability.

I found issue 12812 because my laptop has WiFi, 1000base-T, and several VPN connections as shown in the image below. All of the interfaces that have an X are not active but show up in paddr list. The ones that are actually active will have a paddr.OperStatus == windows.IfOperStatusUp.

networksettings

I can send you the code for my "fix" if you like, but if you are separating the solutions, it might not do you much good.

Regards,

Kevin

@rsc rsc modified the milestones: Go1.6, Go1.5.2 Nov 17, 2015

@gopherbot

This comment has been minimized.

Copy link

commented Dec 4, 2015

CL https://golang.org/cl/17412 mentions this issue.

@gopherbot

This comment has been minimized.

Copy link

commented Dec 8, 2015

CL https://golang.org/cl/17478 mentions this issue.

alexbrainman added a commit that referenced this issue Dec 9, 2015

net: add TestInterfaceAddrsWithNetsh
Use windows netsh command to verify interface
addresses and netmasks net package returns.

The test is to be enabled once issue #12811
is fixed.

Updates #12811

Change-Id: I191e350a1403e5133791d4ec59561fefa24f5c61
Reviewed-on: https://go-review.googlesource.com/17478
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

@mikioh mikioh closed this in e05b48e Dec 10, 2015

@golang golang locked and limited conversation to collaborators Dec 14, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.