bpo-32107 - Improve MAC address calculation and fix test_uuid.py #4600
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -342,11 +342,30 @@ def _popen(command, *args): | |
| env=env) | ||
| return proc | ||
|
|
||
| # For MAC (a.k.a. IEEE 802, or EUI-48) addresses, the second least significant | ||
| # bit of the first octet signifies whether the MAC address is universally (0) | ||
| # or locally (1) administered. Network cards from hardware manufacturers will | ||
| # always be universally administered to guarantee global uniqueness of the MAC | ||
| # address, but any particular machine may have other interfaces which are | ||
| # locally administered. An example of the latter is the bridge interface to | ||
| # the Touch Bar on MacBook Pros. | ||
| # | ||
| # This bit works out to be the 42nd bit counting from 1 being the least | ||
| # significant, or 1<<41. We'll prefer universally administered MAC addresses | ||
| # over locally administered ones since the former are globally unique, but | ||
| # we'll return the first of the latter found if that's all the machine has. | ||
| # | ||
| # See https://en.wikipedia.org/wiki/MAC_address#Universal_vs._local | ||
|
|
||
| def _is_universal(mac): | ||
| return not (mac & (1 << 41)) | ||
|
|
||
| def _find_mac(command, args, hw_identifiers, get_index): | ||
| first_local_mac = None | ||
| try: | ||
| proc = _popen(command, *args.split()) | ||
| if not proc: | ||
| return None | ||
| with proc: | ||
| for line in proc.stdout: | ||
| words = line.lower().rstrip().split() | ||
|
|
@@ -355,8 +374,9 @@ def _find_mac(command, args, hw_identifiers, get_index): | |
| try: | ||
| word = words[get_index(i)] | ||
| mac = int(word.replace(b':', b''), 16) | ||
| if _is_universal(mac): | ||
| return mac | ||
| first_local_mac = first_local_mac or mac | ||
| except (ValueError, IndexError): | ||
| # Virtual interfaces, such as those provided by | ||
| # VPNs, do not have a colon-delimited MAC address | ||
|
|
@@ -366,6 +386,7 @@ def _find_mac(command, args, hw_identifiers, get_index): | |
| pass | ||
| except OSError: | ||
| pass | ||
| return first_local_mac or None | ||
|
|
||
| def _ifconfig_getnode(): | ||
| """Get the hardware address on Unix by running ifconfig.""" | ||
|
|
@@ -375,13 +396,15 @@ def _ifconfig_getnode(): | |
| mac = _find_mac('ifconfig', args, keywords, lambda i: i+1) | ||
| if mac: | ||
| return mac | ||
| return None | ||
|
|
||
| def _ip_getnode(): | ||
| """Get the hardware address on Unix by running ip.""" | ||
| # This works on Linux with iproute2. | ||
| mac = _find_mac('ip', 'link list', [b'link/ether'], lambda i: i+1) | ||
| if mac: | ||
| return mac | ||
| return None | ||
|
|
||
| def _arp_getnode(): | ||
| """Get the hardware address on Unix by running arp.""" | ||
|
|
@@ -404,8 +427,10 @@ def _arp_getnode(): | |
| # This works on Linux, FreeBSD and NetBSD | ||
| mac = _find_mac('arp', '-an', [os.fsencode('(%s)' % ip_addr)], | ||
| lambda i: i+2) | ||
| # Return None instead of 0. | ||
| if mac: | ||
| return mac | ||
| return None | ||
|
|
||
| def _lanscan_getnode(): | ||
| """Get the hardware address on Unix by running lanscan.""" | ||
|
|
@@ -415,32 +440,36 @@ def _lanscan_getnode(): | |
| def _netstat_getnode(): | ||
| """Get the hardware address on Unix by running netstat.""" | ||
| # This might work on AIX, Tru64 UNIX. | ||
| first_local_mac = None | ||
| try: | ||
| proc = _popen('netstat', '-ia') | ||
| if not proc: | ||
| return None | ||
| with proc: | ||
| words = proc.stdout.readline().rstrip().split() | ||
| try: | ||
| i = words.index(b'Address') | ||
| except ValueError: | ||
| return None | ||
| for line in proc.stdout: | ||
| try: | ||
| words = line.rstrip().split() | ||
| word = words[i] | ||
| if len(word) == 17 and word.count(b':') == 5: | ||
| mac = int(word.replace(b':', b''), 16) | ||
| if _is_universal(mac): | ||
| return mac | ||
| first_local_mac = first_local_mac or mac | ||
| except (ValueError, IndexError): | ||
| pass | ||
| except OSError: | ||
| pass | ||
| return first_local_mac or None | ||
|
Member
There was a problem hiding this comment. "or None" seems useless here. Same comment above. |
||
|
|
||
| def _ipconfig_getnode(): | ||
| """Get the hardware address on Windows by running ipconfig.exe.""" | ||
| import os, re | ||
| first_local_mac = None | ||
| dirs = ['', r'c:\windows\system32', r'c:\winnt\system32'] | ||
| try: | ||
| import ctypes | ||
|
|
@@ -458,18 +487,23 @@ def _ipconfig_getnode(): | |
| for line in pipe: | ||
| value = line.split(':')[-1].strip().lower() | ||
| if re.match('([0-9a-f][0-9a-f]-){5}[0-9a-f][0-9a-f]', value): | ||
| mac = int(value.replace('-', ''), 16) | ||
| if _is_universal(mac): | ||
| return mac | ||
| first_local_mac = first_local_mac or mac | ||
| return first_local_mac or None | ||
|
|
||
| def _netbios_getnode(): | ||
| """Get the hardware address on Windows using NetBIOS calls. | ||
| See http://support.microsoft.com/kb/118623 for details.""" | ||
| import win32wnet, netbios | ||
| first_local_mac = None | ||
| ncb = netbios.NCB() | ||
| ncb.Command = netbios.NCBENUM | ||
| ncb.Buffer = adapters = netbios.LANA_ENUM() | ||
| adapters._pack() | ||
| if win32wnet.Netbios(ncb) != 0: | ||
| return None | ||
| adapters._unpack() | ||
| for i in range(adapters.length): | ||
| ncb.Reset() | ||
|
|
@@ -488,7 +522,11 @@ def _netbios_getnode(): | |
| bytes = status.adapter_address[:6] | ||
| if len(bytes) != 6: | ||
| continue | ||
| mac = int.from_bytes(bytes, 'big') | ||
| if _is_universal(mac): | ||
| return mac | ||
| first_local_mac = first_local_mac or mac | ||
| return first_local_mac or None | ||
|
|
||
|
|
||
| _generate_time_safe = _UuidCreate = None | ||
|
|
@@ -601,9 +639,19 @@ def _windll_getnode(): | |
| return UUID(bytes=bytes_(_buffer.raw)).node | ||
|
|
||
| def _random_getnode(): | ||
| """Get a random node ID.""" | ||
| # RFC 4122, $4.1.6 says "For systems with no IEEE address, a randomly or | ||
| # pseudo-randomly generated value may be used; see Section 4.5. The | ||
| # multicast bit must be set in such addresses, in order that they will | ||
| # never conflict with addresses obtained from network cards." | ||
|
Member
There was a problem hiding this comment. I prefer "the multicast bit" rather than "eight bit". Please update also the documentation: |
||
| # | ||
| # The "multicast bit" of a MAC address is defined to be "the least | ||
| # significant bit of the first octet". This works out to be the 41st bit | ||
| # counting from 1 being the least significant bit, or 1<<40. | ||
| # | ||
| # See https://en.wikipedia.org/wiki/MAC_address#Unicast_vs._multicast | ||
| import random | ||
| return random.getrandbits(48) | (1 << 40) | ||
|
|
||
|
|
||
| _node = None | ||
|
|
@@ -626,13 +674,14 @@ def getnode(): | |
| getters = [_unix_getnode, _ifconfig_getnode, _ip_getnode, | ||
| _arp_getnode, _lanscan_getnode, _netstat_getnode] | ||
|
|
||
| for getter in getters: | ||
| try: | ||
| _node = getter() | ||
| except: | ||
| continue | ||
| if _node is not None: | ||
| return _node | ||
| return _random_getnode() | ||
|
|
||
|
|
||
| _last_timestamp = None | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| ``uuid.getnode()`` now preferentially returns universally administered MAC | ||
| addresses if available, over locally administered MAC addresses. This makes a | ||
| better guarantee for global uniqueness of UUIDs returned from | ||
| ``uuid.uuid1()``. If only locally administered MAC addresses are available, | ||
| the first such one found is returned. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change seems like a regression, why removing the following test? Maybe added it back here?
self.assertFalse(node & 0x010000000000, hex)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the original report. This check never failed before on developer's machines and buildbots, but Barry has a machine on which it is failed. While the initial diagnostic of this failure was wrong, actually there are no guaranties that this check always passed.