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

Jump to bottom

net: IPNet missing network mask on Windows #12811

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

net: IPNet missing network mask on Windows #12811

akevinbailey opened this issue Oct 1, 2015 · 10 comments
Labels
FrozenDueToAge OS-Windows
Milestone
Go1.6

Comments

@akevinbailey
Copy link

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 this to the Go1.5.2 milestone Oct 1, 2015
@bradfitz
Copy link
Contributor

bradfitz commented Oct 1, 2015

/cc @alexbrainman

@akevinbailey
Copy link
Author

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.

@akevinbailey akevinbailey mentioned this issue Oct 8, 2015
Closed
@alexbrainman
Copy link
Member

@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
Copy link

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

@rsc
Copy link
Contributor

rsc 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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link

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

@gopherbot
Copy link

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

alexbrainman added a commit that referenced this issue Dec 9, 2015
@alexbrainman
net: add TestInterfaceAddrsWithNetsh
1b7a227 
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 mentioned this issue Dec 9, 2015
Closed
@mikioh mikioh closed this as completed 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.
Assignees
No one assigned
Labels
FrozenDueToAge OS-Windows
Projects
None yet
Milestone
Go1.6
Development

No branches or pull requests
5 participants
@bradfitz @rsc @gopherbot @alexbrainman @akevinbailey