From ebbaa22bc85f0cf424d17c787bc898894dbcc0a4 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 21 Nov 2017 12:00:21 -0500 Subject: [PATCH 01/36] bpo-32107 - Fix test_uuid The original check was erroneous in two ways. First, the documentation said that "47 bit will never be set in IEEE 802 addresses obtained from network cards". I think this is referring to the universally vs. locally administered MAC addresses. Network cards from hardware manufacturers will always be universally administered in order to guarantee global uniqueness. But from https://en.wikipedia.org/wiki/MAC_address it says that this flag is the "distinguished by setting the second-least-significant bit of the first octet of the address", where a 0 means universally administered and a 1 means it's locally administered. This works out to the 42nd bit when counting the LSB as bit 1, or 1<<41. The second bug is that the original bitmask value isn't right for either description: % ./python.exe -c "from math import log2; print(log2(0x010000000000))" 40.0 This causes the test to fail on valid MAC addresses. Fix this by improving the comment, with references, and using the correct bitmask. --- Lib/test/test_uuid.py | 13 ++++++++++--- Lib/uuid.py | 3 +-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 083c2aa8aab54d..5adba57e07d9f2 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -519,9 +519,16 @@ def check_node(self, node, requires=None, network=False): if support.verbose >= 2: print(hex, end=' ') if network: - # 47 bit will never be set in IEEE 802 addresses obtained - # from network cards. - self.assertFalse(node & 0x010000000000, hex) + # The second least significant bit of the first octet signifies + # whether the MAC (IEEE 802, or EUI-48) 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. This bit works + # out to be the 42nd bit counting from 1 being the least + # significant, or 1<<41. For a good, simple explanation, see the + # section on Universal vs. local in this page: + # https://en.wikipedia.org/wiki/MAC_address + self.assertFalse(node & (1 << 41), hex) self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) diff --git a/Lib/uuid.py b/Lib/uuid.py index 020c6e73c863d4..fee809dd1abfab 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -404,8 +404,7 @@ def _arp_getnode(): # This works on Linux, FreeBSD and NetBSD mac = _find_mac('arp', '-an', [os.fsencode('(%s)' % ip_addr)], lambda i: i+2) - if mac: - return mac + return mac def _lanscan_getnode(): """Get the hardware address on Unix by running lanscan.""" From 941b47db8e2cb1587951730f10ecdbfdc48585b5 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 21 Nov 2017 14:10:40 -0500 Subject: [PATCH 02/36] Return None instead of 0 --- Lib/uuid.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/uuid.py b/Lib/uuid.py index fee809dd1abfab..27335f89192909 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -404,7 +404,8 @@ 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 mac + # Return None instead of 0. + return mac if mac else None def _lanscan_getnode(): """Get the hardware address on Unix by running lanscan.""" From e529214489ed802468fab5ea41642bc1a38c3ad9 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 21 Nov 2017 23:08:06 -0500 Subject: [PATCH 03/36] Never return a locally administered MAC address Also, clean up some return sites. --- Lib/test/test_uuid.py | 9 --------- Lib/uuid.py | 45 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 5adba57e07d9f2..19564f2d548d28 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -519,15 +519,6 @@ def check_node(self, node, requires=None, network=False): if support.verbose >= 2: print(hex, end=' ') if network: - # The second least significant bit of the first octet signifies - # whether the MAC (IEEE 802, or EUI-48) 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. This bit works - # out to be the 42nd bit counting from 1 being the least - # significant, or 1<<41. For a good, simple explanation, see the - # section on Universal vs. local in this page: - # https://en.wikipedia.org/wiki/MAC_address self.assertFalse(node & (1 << 41), hex) self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) diff --git a/Lib/uuid.py b/Lib/uuid.py index 27335f89192909..0535433f2bffda 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -342,11 +342,26 @@ 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 skip over any locally administered MAC +# addresses, as it makes no sense to use those in UUID calculation. +# +# For a good, simple explanation, see the section on Universal vs. local in +# this page: https://en.wikipedia.org/wiki/MAC_address + def _find_mac(command, args, hw_identifiers, get_index): try: proc = _popen(command, *args.split()) if not proc: - return + return None with proc: for line in proc.stdout: words = line.lower().rstrip().split() @@ -355,7 +370,7 @@ def _find_mac(command, args, hw_identifiers, get_index): try: word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) - if mac: + if ~(mac & (1<<41)): return mac except (ValueError, IndexError): # Virtual interfaces, such as those provided by @@ -366,6 +381,7 @@ def _find_mac(command, args, hw_identifiers, get_index): pass except OSError: pass + return None def _ifconfig_getnode(): """Get the hardware address on Unix by running ifconfig.""" @@ -375,6 +391,7 @@ 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.""" @@ -382,6 +399,7 @@ def _ip_getnode(): 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.""" @@ -405,7 +423,9 @@ def _arp_getnode(): mac = _find_mac('arp', '-an', [os.fsencode('(%s)' % ip_addr)], lambda i: i+2) # Return None instead of 0. - return mac if mac else None + if mac: + return mac + return None def _lanscan_getnode(): """Get the hardware address on Unix by running lanscan.""" @@ -418,25 +438,26 @@ def _netstat_getnode(): try: proc = _popen('netstat', '-ia') if not proc: - return + return None with proc: words = proc.stdout.readline().rstrip().split() try: i = words.index(b'Address') except ValueError: - return + 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 mac: + if ~(mac & (1<<41)): return mac except (ValueError, IndexError): pass except OSError: pass + return None def _ipconfig_getnode(): """Get the hardware address on Windows by running ipconfig.exe.""" @@ -458,7 +479,10 @@ 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): - return int(value.replace('-', ''), 16) + mac = int(value.replace('-', ''), 16) + if ~(mac & (1<<41)): + return mac + return None def _netbios_getnode(): """Get the hardware address on Windows using NetBIOS calls. @@ -469,7 +493,7 @@ def _netbios_getnode(): ncb.Buffer = adapters = netbios.LANA_ENUM() adapters._pack() if win32wnet.Netbios(ncb) != 0: - return + return None adapters._unpack() for i in range(adapters.length): ncb.Reset() @@ -488,7 +512,10 @@ def _netbios_getnode(): bytes = status.adapter_address[:6] if len(bytes) != 6: continue - return int.from_bytes(bytes, 'big') + mac = int.from_bytes(bytes, 'big') + if ~(mac & (1<<41)): + return mac + return None _generate_time_safe = _UuidCreate = None From 24c4db29f03284821df326f71db35f190f49eb59 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 23 Nov 2017 10:16:21 -0500 Subject: [PATCH 04/36] Several sleep-improved fixes * Add a helper function to determine whether a MAC address is universally administered or not. * In the various MAC getter functions, keep track of the first locally administered MAC, and if no universally administered MAC is found, return the first local MAC. * In the ultimate fallback _random_getnode(), add a comment for the multicast bit being set, and use a better bitmask * In getnode(), just fall back to _random_getnode() explicitly if no other MAC has been found. --- Lib/test/test_uuid.py | 2 +- Lib/uuid.py | 45 +++++++++++++++++++++++++++++++------------ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 19564f2d548d28..97f34cdbe3310f 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -562,7 +562,7 @@ def test_netbios_getnode(self): def test_random_getnode(self): node = self.uuid._random_getnode() # Least significant bit of first octet must be set. - self.assertTrue(node & 0x010000000000, '%012x' % node) + self.assertTrue(node & (1 << 40), '%012x' % node) self.check_node(node) @unittest.skipUnless(os.name == 'posix', 'requires Posix') diff --git a/Lib/uuid.py b/Lib/uuid.py index 0535433f2bffda..3eec69f6f99d4b 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -354,10 +354,13 @@ def _popen(command, *args): # significant, or 1<<41. We'll skip over any locally administered MAC # addresses, as it makes no sense to use those in UUID calculation. # -# For a good, simple explanation, see the section on Universal vs. local in -# this page: https://en.wikipedia.org/wiki/MAC_address +# 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: @@ -370,8 +373,9 @@ def _find_mac(command, args, hw_identifiers, get_index): try: word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) - if ~(mac & (1<<41)): + 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 @@ -381,7 +385,7 @@ def _find_mac(command, args, hw_identifiers, get_index): pass except OSError: pass - return None + return first_local_mac or None def _ifconfig_getnode(): """Get the hardware address on Unix by running ifconfig.""" @@ -435,6 +439,7 @@ 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: @@ -451,17 +456,19 @@ def _netstat_getnode(): word = words[i] if len(word) == 17 and word.count(b':') == 5: mac = int(word.replace(b':', b''), 16) - if ~(mac & (1<<41)): + if _is_universal(mac): return mac + first_local_mac = first_local_mac or mac except (ValueError, IndexError): pass except OSError: pass - return None + return first_local_mac or None 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 @@ -480,14 +487,16 @@ def _ipconfig_getnode(): 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 ~(mac & (1<<41)): + if _is_universal(mac): return mac - return None + 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() @@ -513,8 +522,9 @@ def _netbios_getnode(): if len(bytes) != 6: continue mac = int.from_bytes(bytes, 'big') - if ~(mac & (1<<41)): + if _is_universal(mac): return mac + first_local_mac = first_local_mac or mac return None @@ -628,9 +638,19 @@ def _windll_getnode(): return UUID(bytes=bytes_(_buffer.raw)).node def _random_getnode(): - """Get a random node ID, with eighth bit set as suggested by RFC 4122.""" + """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." + # + # The "multicast bit" of a MAC address is defined to be "the least + # significant bit of the first octet". This worlds 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) | 0x010000000000 + return random.getrandbits(48) | (1 << 40) _node = None @@ -653,13 +673,14 @@ def getnode(): getters = [_unix_getnode, _ifconfig_getnode, _ip_getnode, _arp_getnode, _lanscan_getnode, _netstat_getnode] - for getter in getters + [_random_getnode]: + for getter in getters: try: _node = getter() except: continue if _node is not None: return _node + return _random_getnode() _last_timestamp = None From c056584d287cbd1cac1808fc1ca92e0704930bdf Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 23 Nov 2017 10:25:51 -0500 Subject: [PATCH 05/36] A missed return --- Lib/uuid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/uuid.py b/Lib/uuid.py index 3eec69f6f99d4b..364b5b0272b785 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -525,7 +525,7 @@ def _netbios_getnode(): if _is_universal(mac): return mac first_local_mac = first_local_mac or mac - return None + return first_local_mac or None _generate_time_safe = _UuidCreate = None From 7fd446e0d84ee8616b9449f0a1e924581d0fef03 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 23 Nov 2017 10:27:21 -0500 Subject: [PATCH 06/36] Two typos --- Lib/uuid.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/uuid.py b/Lib/uuid.py index 364b5b0272b785..9e7c672f7a0a6f 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -639,13 +639,13 @@ def _windll_getnode(): def _random_getnode(): """Get a random node ID.""" - # RFC 4122, $4.1.6. says "For systems with no IEEE address, a randomly or + # 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." # # The "multicast bit" of a MAC address is defined to be "the least - # significant bit of the first octet". This worlds out to be the 41st bit + # 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 From fb2c2f2ee3847c9e74d87a32876f26f0e1ce3304 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sat, 25 Nov 2017 10:10:12 -0500 Subject: [PATCH 07/36] DEBUGGING TRAVIS - DO NOT MERGE --- Lib/uuid.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/uuid.py b/Lib/uuid.py index 9e7c672f7a0a6f..c551bb60a0c9bd 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -374,7 +374,9 @@ def _find_mac(command, args, hw_identifiers, get_index): word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) if _is_universal(mac): + print('%012x' % mac, 'universal', command) return mac + print('%012x' % mac, 'local', command) first_local_mac = first_local_mac or mac except (ValueError, IndexError): # Virtual interfaces, such as those provided by From 99e86b2bd1c17cc3a4860a4d07abd39cd7c5df37 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sun, 26 Nov 2017 11:44:25 -0500 Subject: [PATCH 08/36] Two tests cannot succeed on Travis-CI --- Lib/test/test_uuid.py | 4 ++++ Lib/uuid.py | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 97f34cdbe3310f..05427968692d26 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -524,11 +524,15 @@ def check_node(self, node, requires=None, network=False): "%s is not an RFC 4122 node ID" % hex) @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipIf(os.getenv('TRAVIS'), + 'Travis-CI has no universally administered MAC addresses') def test_ifconfig_getnode(self): node = self.uuid._ifconfig_getnode() self.check_node(node, 'ifconfig', True) @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipIf(os.getenv('TRAVIS'), + 'Travis-CI has no universally administered MAC addresses') def test_ip_getnode(self): node = self.uuid._ip_getnode() self.check_node(node, 'ip', True) diff --git a/Lib/uuid.py b/Lib/uuid.py index c551bb60a0c9bd..9e7c672f7a0a6f 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -374,9 +374,7 @@ def _find_mac(command, args, hw_identifiers, get_index): word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) if _is_universal(mac): - print('%012x' % mac, 'universal', command) return mac - print('%012x' % mac, 'local', command) first_local_mac = first_local_mac or mac except (ValueError, IndexError): # Virtual interfaces, such as those provided by From 74702e3b6b12177cbabd49d6484a07c23ac379ce Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 26 Nov 2017 22:42:57 +0200 Subject: [PATCH 09/36] bpo-32107: Remove the check of the multicast bit in test_uuid. While it is required that the multicast bit (1<<40) should be set in random generated addresses, there is no requirement that it should be cleared in addresses obtained from network cards. --- Lib/test/test_uuid.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 083c2aa8aab54d..fa89d62aec2ab6 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -512,58 +512,55 @@ def test_find_mac(self): self.assertEqual(mac, 0x1234567890ab) - def check_node(self, node, requires=None, network=False): + def check_node(self, node, requires=None): if requires and node is None: self.skipTest('requires ' + requires) hex = '%012x' % node if support.verbose >= 2: print(hex, end=' ') - if network: - # 47 bit will never be set in IEEE 802 addresses obtained - # from network cards. - self.assertFalse(node & 0x010000000000, hex) self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_ifconfig_getnode(self): node = self.uuid._ifconfig_getnode() - self.check_node(node, 'ifconfig', True) + self.check_node(node, 'ifconfig') @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_ip_getnode(self): node = self.uuid._ip_getnode() - self.check_node(node, 'ip', True) + self.check_node(node, 'ip') @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_arp_getnode(self): node = self.uuid._arp_getnode() - self.check_node(node, 'arp', True) + self.check_node(node, 'arp') @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_lanscan_getnode(self): node = self.uuid._lanscan_getnode() - self.check_node(node, 'lanscan', True) + self.check_node(node, 'lanscan') @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_netstat_getnode(self): node = self.uuid._netstat_getnode() - self.check_node(node, 'netstat', True) + self.check_node(node, 'netstat') @unittest.skipUnless(os.name == 'nt', 'requires Windows') def test_ipconfig_getnode(self): node = self.uuid._ipconfig_getnode() - self.check_node(node, 'ipconfig', True) + self.check_node(node, 'ipconfig') @unittest.skipUnless(importable('win32wnet'), 'requires win32wnet') @unittest.skipUnless(importable('netbios'), 'requires netbios') def test_netbios_getnode(self): node = self.uuid._netbios_getnode() - self.check_node(node, network=True) + self.check_node(node) def test_random_getnode(self): node = self.uuid._random_getnode() # Least significant bit of first octet must be set. + # See RFC 4122, $4.1.6. self.assertTrue(node & 0x010000000000, '%012x' % node) self.check_node(node) From 41adbad8ee5b1d6c37892ae54062e1d53fc365ab Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 21 Nov 2017 12:00:21 -0500 Subject: [PATCH 10/36] bpo-32107 - Fix test_uuid The original check was erroneous in two ways. First, the documentation said that "47 bit will never be set in IEEE 802 addresses obtained from network cards". I think this is referring to the universally vs. locally administered MAC addresses. Network cards from hardware manufacturers will always be universally administered in order to guarantee global uniqueness. But from https://en.wikipedia.org/wiki/MAC_address it says that this flag is the "distinguished by setting the second-least-significant bit of the first octet of the address", where a 0 means universally administered and a 1 means it's locally administered. This works out to the 42nd bit when counting the LSB as bit 1, or 1<<41. The second bug is that the original bitmask value isn't right for either description: % ./python.exe -c "from math import log2; print(log2(0x010000000000))" 40.0 This causes the test to fail on valid MAC addresses. Fix this by improving the comment, with references, and using the correct bitmask. --- Lib/test/test_uuid.py | 13 ++++++++++--- Lib/uuid.py | 3 +-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 083c2aa8aab54d..5adba57e07d9f2 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -519,9 +519,16 @@ def check_node(self, node, requires=None, network=False): if support.verbose >= 2: print(hex, end=' ') if network: - # 47 bit will never be set in IEEE 802 addresses obtained - # from network cards. - self.assertFalse(node & 0x010000000000, hex) + # The second least significant bit of the first octet signifies + # whether the MAC (IEEE 802, or EUI-48) 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. This bit works + # out to be the 42nd bit counting from 1 being the least + # significant, or 1<<41. For a good, simple explanation, see the + # section on Universal vs. local in this page: + # https://en.wikipedia.org/wiki/MAC_address + self.assertFalse(node & (1 << 41), hex) self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) diff --git a/Lib/uuid.py b/Lib/uuid.py index 020c6e73c863d4..fee809dd1abfab 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -404,8 +404,7 @@ def _arp_getnode(): # This works on Linux, FreeBSD and NetBSD mac = _find_mac('arp', '-an', [os.fsencode('(%s)' % ip_addr)], lambda i: i+2) - if mac: - return mac + return mac def _lanscan_getnode(): """Get the hardware address on Unix by running lanscan.""" From d3d0465e4a64f24d5727553f1031302b29d3c849 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 21 Nov 2017 14:10:40 -0500 Subject: [PATCH 11/36] Return None instead of 0 --- Lib/uuid.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/uuid.py b/Lib/uuid.py index fee809dd1abfab..27335f89192909 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -404,7 +404,8 @@ 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 mac + # Return None instead of 0. + return mac if mac else None def _lanscan_getnode(): """Get the hardware address on Unix by running lanscan.""" From 7c17cd814f3a74c08566bb9386b4c03bce0a4eb2 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 21 Nov 2017 23:08:06 -0500 Subject: [PATCH 12/36] Never return a locally administered MAC address Also, clean up some return sites. --- Lib/test/test_uuid.py | 9 --------- Lib/uuid.py | 45 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 5adba57e07d9f2..19564f2d548d28 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -519,15 +519,6 @@ def check_node(self, node, requires=None, network=False): if support.verbose >= 2: print(hex, end=' ') if network: - # The second least significant bit of the first octet signifies - # whether the MAC (IEEE 802, or EUI-48) 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. This bit works - # out to be the 42nd bit counting from 1 being the least - # significant, or 1<<41. For a good, simple explanation, see the - # section on Universal vs. local in this page: - # https://en.wikipedia.org/wiki/MAC_address self.assertFalse(node & (1 << 41), hex) self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) diff --git a/Lib/uuid.py b/Lib/uuid.py index 27335f89192909..0535433f2bffda 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -342,11 +342,26 @@ 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 skip over any locally administered MAC +# addresses, as it makes no sense to use those in UUID calculation. +# +# For a good, simple explanation, see the section on Universal vs. local in +# this page: https://en.wikipedia.org/wiki/MAC_address + def _find_mac(command, args, hw_identifiers, get_index): try: proc = _popen(command, *args.split()) if not proc: - return + return None with proc: for line in proc.stdout: words = line.lower().rstrip().split() @@ -355,7 +370,7 @@ def _find_mac(command, args, hw_identifiers, get_index): try: word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) - if mac: + if ~(mac & (1<<41)): return mac except (ValueError, IndexError): # Virtual interfaces, such as those provided by @@ -366,6 +381,7 @@ def _find_mac(command, args, hw_identifiers, get_index): pass except OSError: pass + return None def _ifconfig_getnode(): """Get the hardware address on Unix by running ifconfig.""" @@ -375,6 +391,7 @@ 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.""" @@ -382,6 +399,7 @@ def _ip_getnode(): 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.""" @@ -405,7 +423,9 @@ def _arp_getnode(): mac = _find_mac('arp', '-an', [os.fsencode('(%s)' % ip_addr)], lambda i: i+2) # Return None instead of 0. - return mac if mac else None + if mac: + return mac + return None def _lanscan_getnode(): """Get the hardware address on Unix by running lanscan.""" @@ -418,25 +438,26 @@ def _netstat_getnode(): try: proc = _popen('netstat', '-ia') if not proc: - return + return None with proc: words = proc.stdout.readline().rstrip().split() try: i = words.index(b'Address') except ValueError: - return + 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 mac: + if ~(mac & (1<<41)): return mac except (ValueError, IndexError): pass except OSError: pass + return None def _ipconfig_getnode(): """Get the hardware address on Windows by running ipconfig.exe.""" @@ -458,7 +479,10 @@ 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): - return int(value.replace('-', ''), 16) + mac = int(value.replace('-', ''), 16) + if ~(mac & (1<<41)): + return mac + return None def _netbios_getnode(): """Get the hardware address on Windows using NetBIOS calls. @@ -469,7 +493,7 @@ def _netbios_getnode(): ncb.Buffer = adapters = netbios.LANA_ENUM() adapters._pack() if win32wnet.Netbios(ncb) != 0: - return + return None adapters._unpack() for i in range(adapters.length): ncb.Reset() @@ -488,7 +512,10 @@ def _netbios_getnode(): bytes = status.adapter_address[:6] if len(bytes) != 6: continue - return int.from_bytes(bytes, 'big') + mac = int.from_bytes(bytes, 'big') + if ~(mac & (1<<41)): + return mac + return None _generate_time_safe = _UuidCreate = None From 511fab0a6f980665e01f0cade38c7ecffb37257c Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 23 Nov 2017 10:16:21 -0500 Subject: [PATCH 13/36] Several sleep-improved fixes * Add a helper function to determine whether a MAC address is universally administered or not. * In the various MAC getter functions, keep track of the first locally administered MAC, and if no universally administered MAC is found, return the first local MAC. * In the ultimate fallback _random_getnode(), add a comment for the multicast bit being set, and use a better bitmask * In getnode(), just fall back to _random_getnode() explicitly if no other MAC has been found. --- Lib/test/test_uuid.py | 2 +- Lib/uuid.py | 45 +++++++++++++++++++++++++++++++------------ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 19564f2d548d28..97f34cdbe3310f 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -562,7 +562,7 @@ def test_netbios_getnode(self): def test_random_getnode(self): node = self.uuid._random_getnode() # Least significant bit of first octet must be set. - self.assertTrue(node & 0x010000000000, '%012x' % node) + self.assertTrue(node & (1 << 40), '%012x' % node) self.check_node(node) @unittest.skipUnless(os.name == 'posix', 'requires Posix') diff --git a/Lib/uuid.py b/Lib/uuid.py index 0535433f2bffda..3eec69f6f99d4b 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -354,10 +354,13 @@ def _popen(command, *args): # significant, or 1<<41. We'll skip over any locally administered MAC # addresses, as it makes no sense to use those in UUID calculation. # -# For a good, simple explanation, see the section on Universal vs. local in -# this page: https://en.wikipedia.org/wiki/MAC_address +# 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: @@ -370,8 +373,9 @@ def _find_mac(command, args, hw_identifiers, get_index): try: word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) - if ~(mac & (1<<41)): + 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 @@ -381,7 +385,7 @@ def _find_mac(command, args, hw_identifiers, get_index): pass except OSError: pass - return None + return first_local_mac or None def _ifconfig_getnode(): """Get the hardware address on Unix by running ifconfig.""" @@ -435,6 +439,7 @@ 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: @@ -451,17 +456,19 @@ def _netstat_getnode(): word = words[i] if len(word) == 17 and word.count(b':') == 5: mac = int(word.replace(b':', b''), 16) - if ~(mac & (1<<41)): + if _is_universal(mac): return mac + first_local_mac = first_local_mac or mac except (ValueError, IndexError): pass except OSError: pass - return None + return first_local_mac or None 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 @@ -480,14 +487,16 @@ def _ipconfig_getnode(): 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 ~(mac & (1<<41)): + if _is_universal(mac): return mac - return None + 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() @@ -513,8 +522,9 @@ def _netbios_getnode(): if len(bytes) != 6: continue mac = int.from_bytes(bytes, 'big') - if ~(mac & (1<<41)): + if _is_universal(mac): return mac + first_local_mac = first_local_mac or mac return None @@ -628,9 +638,19 @@ def _windll_getnode(): return UUID(bytes=bytes_(_buffer.raw)).node def _random_getnode(): - """Get a random node ID, with eighth bit set as suggested by RFC 4122.""" + """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." + # + # The "multicast bit" of a MAC address is defined to be "the least + # significant bit of the first octet". This worlds 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) | 0x010000000000 + return random.getrandbits(48) | (1 << 40) _node = None @@ -653,13 +673,14 @@ def getnode(): getters = [_unix_getnode, _ifconfig_getnode, _ip_getnode, _arp_getnode, _lanscan_getnode, _netstat_getnode] - for getter in getters + [_random_getnode]: + for getter in getters: try: _node = getter() except: continue if _node is not None: return _node + return _random_getnode() _last_timestamp = None From af7929c50060a09e6be3d6a6783318fb8697c816 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 23 Nov 2017 10:25:51 -0500 Subject: [PATCH 14/36] A missed return --- Lib/uuid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/uuid.py b/Lib/uuid.py index 3eec69f6f99d4b..364b5b0272b785 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -525,7 +525,7 @@ def _netbios_getnode(): if _is_universal(mac): return mac first_local_mac = first_local_mac or mac - return None + return first_local_mac or None _generate_time_safe = _UuidCreate = None From 51ba0b30bdbad94ca2c0c34d60d0527125be47d2 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 23 Nov 2017 10:27:21 -0500 Subject: [PATCH 15/36] Two typos --- Lib/uuid.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/uuid.py b/Lib/uuid.py index 364b5b0272b785..9e7c672f7a0a6f 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -639,13 +639,13 @@ def _windll_getnode(): def _random_getnode(): """Get a random node ID.""" - # RFC 4122, $4.1.6. says "For systems with no IEEE address, a randomly or + # 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." # # The "multicast bit" of a MAC address is defined to be "the least - # significant bit of the first octet". This worlds out to be the 41st bit + # 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 From 909ee79b0ff6f215167ea5af4694043ff8ae4b0b Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sat, 25 Nov 2017 10:10:12 -0500 Subject: [PATCH 16/36] DEBUGGING TRAVIS - DO NOT MERGE --- Lib/uuid.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/uuid.py b/Lib/uuid.py index 9e7c672f7a0a6f..c551bb60a0c9bd 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -374,7 +374,9 @@ def _find_mac(command, args, hw_identifiers, get_index): word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) if _is_universal(mac): + print('%012x' % mac, 'universal', command) return mac + print('%012x' % mac, 'local', command) first_local_mac = first_local_mac or mac except (ValueError, IndexError): # Virtual interfaces, such as those provided by From 6acb120cca296749a8744bf726aa1c58cbc5741d Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sun, 26 Nov 2017 11:44:25 -0500 Subject: [PATCH 17/36] Two tests cannot succeed on Travis-CI --- Lib/test/test_uuid.py | 4 ++++ Lib/uuid.py | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 97f34cdbe3310f..05427968692d26 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -524,11 +524,15 @@ def check_node(self, node, requires=None, network=False): "%s is not an RFC 4122 node ID" % hex) @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipIf(os.getenv('TRAVIS'), + 'Travis-CI has no universally administered MAC addresses') def test_ifconfig_getnode(self): node = self.uuid._ifconfig_getnode() self.check_node(node, 'ifconfig', True) @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipIf(os.getenv('TRAVIS'), + 'Travis-CI has no universally administered MAC addresses') def test_ip_getnode(self): node = self.uuid._ip_getnode() self.check_node(node, 'ip', True) diff --git a/Lib/uuid.py b/Lib/uuid.py index c551bb60a0c9bd..9e7c672f7a0a6f 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -374,9 +374,7 @@ def _find_mac(command, args, hw_identifiers, get_index): word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) if _is_universal(mac): - print('%012x' % mac, 'universal', command) return mac - print('%012x' % mac, 'local', command) first_local_mac = first_local_mac or mac except (ValueError, IndexError): # Virtual interfaces, such as those provided by From 18a96ce3df418755a670a34e1c42ce3c8529ee2b Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 21 Nov 2017 12:00:21 -0500 Subject: [PATCH 18/36] bpo-32107 - Fix test_uuid The original check was erroneous in two ways. First, the documentation said that "47 bit will never be set in IEEE 802 addresses obtained from network cards". I think this is referring to the universally vs. locally administered MAC addresses. Network cards from hardware manufacturers will always be universally administered in order to guarantee global uniqueness. But from https://en.wikipedia.org/wiki/MAC_address it says that this flag is the "distinguished by setting the second-least-significant bit of the first octet of the address", where a 0 means universally administered and a 1 means it's locally administered. This works out to the 42nd bit when counting the LSB as bit 1, or 1<<41. The second bug is that the original bitmask value isn't right for either description: % ./python.exe -c "from math import log2; print(log2(0x010000000000))" 40.0 This causes the test to fail on valid MAC addresses. Fix this by improving the comment, with references, and using the correct bitmask. --- Lib/test/test_uuid.py | 13 ++++++++++--- Lib/uuid.py | 3 +-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 083c2aa8aab54d..5adba57e07d9f2 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -519,9 +519,16 @@ def check_node(self, node, requires=None, network=False): if support.verbose >= 2: print(hex, end=' ') if network: - # 47 bit will never be set in IEEE 802 addresses obtained - # from network cards. - self.assertFalse(node & 0x010000000000, hex) + # The second least significant bit of the first octet signifies + # whether the MAC (IEEE 802, or EUI-48) 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. This bit works + # out to be the 42nd bit counting from 1 being the least + # significant, or 1<<41. For a good, simple explanation, see the + # section on Universal vs. local in this page: + # https://en.wikipedia.org/wiki/MAC_address + self.assertFalse(node & (1 << 41), hex) self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) diff --git a/Lib/uuid.py b/Lib/uuid.py index 020c6e73c863d4..fee809dd1abfab 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -404,8 +404,7 @@ def _arp_getnode(): # This works on Linux, FreeBSD and NetBSD mac = _find_mac('arp', '-an', [os.fsencode('(%s)' % ip_addr)], lambda i: i+2) - if mac: - return mac + return mac def _lanscan_getnode(): """Get the hardware address on Unix by running lanscan.""" From 83dc3a3b8ec25dcc70d97258827d3d0a00c77838 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 21 Nov 2017 14:10:40 -0500 Subject: [PATCH 19/36] Return None instead of 0 --- Lib/uuid.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/uuid.py b/Lib/uuid.py index fee809dd1abfab..27335f89192909 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -404,7 +404,8 @@ 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 mac + # Return None instead of 0. + return mac if mac else None def _lanscan_getnode(): """Get the hardware address on Unix by running lanscan.""" From fb156f3a5d46b92e5b79377936cd7225a4ffefe5 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 21 Nov 2017 23:08:06 -0500 Subject: [PATCH 20/36] Never return a locally administered MAC address Also, clean up some return sites. --- Lib/test/test_uuid.py | 9 --------- Lib/uuid.py | 45 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 5adba57e07d9f2..19564f2d548d28 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -519,15 +519,6 @@ def check_node(self, node, requires=None, network=False): if support.verbose >= 2: print(hex, end=' ') if network: - # The second least significant bit of the first octet signifies - # whether the MAC (IEEE 802, or EUI-48) 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. This bit works - # out to be the 42nd bit counting from 1 being the least - # significant, or 1<<41. For a good, simple explanation, see the - # section on Universal vs. local in this page: - # https://en.wikipedia.org/wiki/MAC_address self.assertFalse(node & (1 << 41), hex) self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) diff --git a/Lib/uuid.py b/Lib/uuid.py index 27335f89192909..0535433f2bffda 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -342,11 +342,26 @@ 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 skip over any locally administered MAC +# addresses, as it makes no sense to use those in UUID calculation. +# +# For a good, simple explanation, see the section on Universal vs. local in +# this page: https://en.wikipedia.org/wiki/MAC_address + def _find_mac(command, args, hw_identifiers, get_index): try: proc = _popen(command, *args.split()) if not proc: - return + return None with proc: for line in proc.stdout: words = line.lower().rstrip().split() @@ -355,7 +370,7 @@ def _find_mac(command, args, hw_identifiers, get_index): try: word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) - if mac: + if ~(mac & (1<<41)): return mac except (ValueError, IndexError): # Virtual interfaces, such as those provided by @@ -366,6 +381,7 @@ def _find_mac(command, args, hw_identifiers, get_index): pass except OSError: pass + return None def _ifconfig_getnode(): """Get the hardware address on Unix by running ifconfig.""" @@ -375,6 +391,7 @@ 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.""" @@ -382,6 +399,7 @@ def _ip_getnode(): 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.""" @@ -405,7 +423,9 @@ def _arp_getnode(): mac = _find_mac('arp', '-an', [os.fsencode('(%s)' % ip_addr)], lambda i: i+2) # Return None instead of 0. - return mac if mac else None + if mac: + return mac + return None def _lanscan_getnode(): """Get the hardware address on Unix by running lanscan.""" @@ -418,25 +438,26 @@ def _netstat_getnode(): try: proc = _popen('netstat', '-ia') if not proc: - return + return None with proc: words = proc.stdout.readline().rstrip().split() try: i = words.index(b'Address') except ValueError: - return + 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 mac: + if ~(mac & (1<<41)): return mac except (ValueError, IndexError): pass except OSError: pass + return None def _ipconfig_getnode(): """Get the hardware address on Windows by running ipconfig.exe.""" @@ -458,7 +479,10 @@ 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): - return int(value.replace('-', ''), 16) + mac = int(value.replace('-', ''), 16) + if ~(mac & (1<<41)): + return mac + return None def _netbios_getnode(): """Get the hardware address on Windows using NetBIOS calls. @@ -469,7 +493,7 @@ def _netbios_getnode(): ncb.Buffer = adapters = netbios.LANA_ENUM() adapters._pack() if win32wnet.Netbios(ncb) != 0: - return + return None adapters._unpack() for i in range(adapters.length): ncb.Reset() @@ -488,7 +512,10 @@ def _netbios_getnode(): bytes = status.adapter_address[:6] if len(bytes) != 6: continue - return int.from_bytes(bytes, 'big') + mac = int.from_bytes(bytes, 'big') + if ~(mac & (1<<41)): + return mac + return None _generate_time_safe = _UuidCreate = None From 3e8e17d6c38dc1abc9812e6985fd55f4a09b769f Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 23 Nov 2017 10:16:21 -0500 Subject: [PATCH 21/36] Several sleep-improved fixes * Add a helper function to determine whether a MAC address is universally administered or not. * In the various MAC getter functions, keep track of the first locally administered MAC, and if no universally administered MAC is found, return the first local MAC. * In the ultimate fallback _random_getnode(), add a comment for the multicast bit being set, and use a better bitmask * In getnode(), just fall back to _random_getnode() explicitly if no other MAC has been found. --- Lib/test/test_uuid.py | 2 +- Lib/uuid.py | 45 +++++++++++++++++++++++++++++++------------ 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 19564f2d548d28..97f34cdbe3310f 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -562,7 +562,7 @@ def test_netbios_getnode(self): def test_random_getnode(self): node = self.uuid._random_getnode() # Least significant bit of first octet must be set. - self.assertTrue(node & 0x010000000000, '%012x' % node) + self.assertTrue(node & (1 << 40), '%012x' % node) self.check_node(node) @unittest.skipUnless(os.name == 'posix', 'requires Posix') diff --git a/Lib/uuid.py b/Lib/uuid.py index 0535433f2bffda..3eec69f6f99d4b 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -354,10 +354,13 @@ def _popen(command, *args): # significant, or 1<<41. We'll skip over any locally administered MAC # addresses, as it makes no sense to use those in UUID calculation. # -# For a good, simple explanation, see the section on Universal vs. local in -# this page: https://en.wikipedia.org/wiki/MAC_address +# 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: @@ -370,8 +373,9 @@ def _find_mac(command, args, hw_identifiers, get_index): try: word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) - if ~(mac & (1<<41)): + 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 @@ -381,7 +385,7 @@ def _find_mac(command, args, hw_identifiers, get_index): pass except OSError: pass - return None + return first_local_mac or None def _ifconfig_getnode(): """Get the hardware address on Unix by running ifconfig.""" @@ -435,6 +439,7 @@ 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: @@ -451,17 +456,19 @@ def _netstat_getnode(): word = words[i] if len(word) == 17 and word.count(b':') == 5: mac = int(word.replace(b':', b''), 16) - if ~(mac & (1<<41)): + if _is_universal(mac): return mac + first_local_mac = first_local_mac or mac except (ValueError, IndexError): pass except OSError: pass - return None + return first_local_mac or None 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 @@ -480,14 +487,16 @@ def _ipconfig_getnode(): 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 ~(mac & (1<<41)): + if _is_universal(mac): return mac - return None + 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() @@ -513,8 +522,9 @@ def _netbios_getnode(): if len(bytes) != 6: continue mac = int.from_bytes(bytes, 'big') - if ~(mac & (1<<41)): + if _is_universal(mac): return mac + first_local_mac = first_local_mac or mac return None @@ -628,9 +638,19 @@ def _windll_getnode(): return UUID(bytes=bytes_(_buffer.raw)).node def _random_getnode(): - """Get a random node ID, with eighth bit set as suggested by RFC 4122.""" + """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." + # + # The "multicast bit" of a MAC address is defined to be "the least + # significant bit of the first octet". This worlds 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) | 0x010000000000 + return random.getrandbits(48) | (1 << 40) _node = None @@ -653,13 +673,14 @@ def getnode(): getters = [_unix_getnode, _ifconfig_getnode, _ip_getnode, _arp_getnode, _lanscan_getnode, _netstat_getnode] - for getter in getters + [_random_getnode]: + for getter in getters: try: _node = getter() except: continue if _node is not None: return _node + return _random_getnode() _last_timestamp = None From 668f4f6db80145cf9abb768dd2b01b8d8c15945c Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 23 Nov 2017 10:25:51 -0500 Subject: [PATCH 22/36] A missed return --- Lib/uuid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/uuid.py b/Lib/uuid.py index 3eec69f6f99d4b..364b5b0272b785 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -525,7 +525,7 @@ def _netbios_getnode(): if _is_universal(mac): return mac first_local_mac = first_local_mac or mac - return None + return first_local_mac or None _generate_time_safe = _UuidCreate = None From 6eecb80b85084c81d21aa9c276b5fc90f8b659e1 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Thu, 23 Nov 2017 10:27:21 -0500 Subject: [PATCH 23/36] Two typos --- Lib/uuid.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/uuid.py b/Lib/uuid.py index 364b5b0272b785..9e7c672f7a0a6f 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -639,13 +639,13 @@ def _windll_getnode(): def _random_getnode(): """Get a random node ID.""" - # RFC 4122, $4.1.6. says "For systems with no IEEE address, a randomly or + # 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." # # The "multicast bit" of a MAC address is defined to be "the least - # significant bit of the first octet". This worlds out to be the 41st bit + # 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 From 87712f3e89580912773a38c7ba6d54b645f75339 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sat, 25 Nov 2017 10:10:12 -0500 Subject: [PATCH 24/36] DEBUGGING TRAVIS - DO NOT MERGE --- Lib/uuid.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/uuid.py b/Lib/uuid.py index 9e7c672f7a0a6f..c551bb60a0c9bd 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -374,7 +374,9 @@ def _find_mac(command, args, hw_identifiers, get_index): word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) if _is_universal(mac): + print('%012x' % mac, 'universal', command) return mac + print('%012x' % mac, 'local', command) first_local_mac = first_local_mac or mac except (ValueError, IndexError): # Virtual interfaces, such as those provided by From 740393ebf3d1444259274f96a8f660292909ccfa Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sun, 26 Nov 2017 11:44:25 -0500 Subject: [PATCH 25/36] Two tests cannot succeed on Travis-CI --- Lib/test/test_uuid.py | 4 ++++ Lib/uuid.py | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 97f34cdbe3310f..05427968692d26 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -524,11 +524,15 @@ def check_node(self, node, requires=None, network=False): "%s is not an RFC 4122 node ID" % hex) @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipIf(os.getenv('TRAVIS'), + 'Travis-CI has no universally administered MAC addresses') def test_ifconfig_getnode(self): node = self.uuid._ifconfig_getnode() self.check_node(node, 'ifconfig', True) @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipIf(os.getenv('TRAVIS'), + 'Travis-CI has no universally administered MAC addresses') def test_ip_getnode(self): node = self.uuid._ip_getnode() self.check_node(node, 'ip', True) diff --git a/Lib/uuid.py b/Lib/uuid.py index c551bb60a0c9bd..9e7c672f7a0a6f 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -374,9 +374,7 @@ def _find_mac(command, args, hw_identifiers, get_index): word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) if _is_universal(mac): - print('%012x' % mac, 'universal', command) return mac - print('%012x' % mac, 'local', command) first_local_mac = first_local_mac or mac except (ValueError, IndexError): # Virtual interfaces, such as those provided by From 225b0638b58b9e94b75377b126467b1295a48946 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 26 Nov 2017 22:42:57 +0200 Subject: [PATCH 26/36] bpo-32107: Remove the check of the multicast bit in test_uuid. While it is required that the multicast bit (1<<40) should be set in random generated addresses, there is no requirement that it should be cleared in addresses obtained from network cards. --- Lib/test/test_uuid.py | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 05427968692d26..ca00bdb13534c1 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -512,62 +512,69 @@ def test_find_mac(self): self.assertEqual(mac, 0x1234567890ab) - def check_node(self, node, requires=None, network=False): + def check_node(self, node, requires=None, *, random=False): if requires and node is None: self.skipTest('requires ' + requires) hex = '%012x' % node if support.verbose >= 2: print(hex, end=' ') - if network: - self.assertFalse(node & (1 << 41), hex) + # The MAC address will be universally administered (i.e. the second + # least significant bit of the first octet must be unset) for any + # physical interface, such as an ethernet port or wireless adapter. + # There are some cases where this won't be the case. Randomly + # generated MACs may not be universally administered, but they must + # have their multicast bit set, though this is tested in the + # `test_random_getnode()` method specifically. Another case is the + # Travis-CI case, which apparently only has locally administered MAC + # addresses. + if not random and not os.getenv('TRAVIS'): + self.assertFalse(node & (1 << 41), '%012x' % node) self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) @unittest.skipUnless(os.name == 'posix', 'requires Posix') - @unittest.skipIf(os.getenv('TRAVIS'), - 'Travis-CI has no universally administered MAC addresses') def test_ifconfig_getnode(self): node = self.uuid._ifconfig_getnode() - self.check_node(node, 'ifconfig', True) + self.check_node(node, 'ifconfig') @unittest.skipUnless(os.name == 'posix', 'requires Posix') - @unittest.skipIf(os.getenv('TRAVIS'), - 'Travis-CI has no universally administered MAC addresses') def test_ip_getnode(self): node = self.uuid._ip_getnode() - self.check_node(node, 'ip', True) + self.check_node(node, 'ip') @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_arp_getnode(self): node = self.uuid._arp_getnode() - self.check_node(node, 'arp', True) + self.check_node(node, 'arp') @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_lanscan_getnode(self): node = self.uuid._lanscan_getnode() - self.check_node(node, 'lanscan', True) + self.check_node(node, 'lanscan') @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_netstat_getnode(self): node = self.uuid._netstat_getnode() - self.check_node(node, 'netstat', True) + self.check_node(node, 'netstat') @unittest.skipUnless(os.name == 'nt', 'requires Windows') def test_ipconfig_getnode(self): node = self.uuid._ipconfig_getnode() - self.check_node(node, 'ipconfig', True) + self.check_node(node, 'ipconfig') @unittest.skipUnless(importable('win32wnet'), 'requires win32wnet') @unittest.skipUnless(importable('netbios'), 'requires netbios') def test_netbios_getnode(self): node = self.uuid._netbios_getnode() - self.check_node(node, network=True) + self.check_node(node) def test_random_getnode(self): node = self.uuid._random_getnode() - # Least significant bit of first octet must be set. + # The multicast bit, i.e. the least significant bit of first octet, + # must be set for randomly generated MAC addresses. See RFC 4122, + # $4.1.6. self.assertTrue(node & (1 << 40), '%012x' % node) - self.check_node(node) + self.check_node(node, random=True) @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_unix_getnode(self): From 5b8ca9056d5a8e27d972fd53d542091880a70c77 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sat, 25 Nov 2017 10:10:12 -0500 Subject: [PATCH 27/36] DEBUGGING TRAVIS - DO NOT MERGE --- Lib/uuid.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/uuid.py b/Lib/uuid.py index 9e7c672f7a0a6f..c551bb60a0c9bd 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -374,7 +374,9 @@ def _find_mac(command, args, hw_identifiers, get_index): word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) if _is_universal(mac): + print('%012x' % mac, 'universal', command) return mac + print('%012x' % mac, 'local', command) first_local_mac = first_local_mac or mac except (ValueError, IndexError): # Virtual interfaces, such as those provided by From 2aa4f5082517aff7616c6bf792211ccb8aec443d Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sun, 26 Nov 2017 11:44:25 -0500 Subject: [PATCH 28/36] Two tests cannot succeed on Travis-CI --- Lib/test/test_uuid.py | 4 ++++ Lib/uuid.py | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index ca00bdb13534c1..00a69158ec3ad4 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -533,11 +533,15 @@ def check_node(self, node, requires=None, *, random=False): "%s is not an RFC 4122 node ID" % hex) @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipIf(os.getenv('TRAVIS'), + 'Travis-CI has no universally administered MAC addresses') def test_ifconfig_getnode(self): node = self.uuid._ifconfig_getnode() self.check_node(node, 'ifconfig') @unittest.skipUnless(os.name == 'posix', 'requires Posix') + @unittest.skipIf(os.getenv('TRAVIS'), + 'Travis-CI has no universally administered MAC addresses') def test_ip_getnode(self): node = self.uuid._ip_getnode() self.check_node(node, 'ip') diff --git a/Lib/uuid.py b/Lib/uuid.py index c551bb60a0c9bd..9e7c672f7a0a6f 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -374,9 +374,7 @@ def _find_mac(command, args, hw_identifiers, get_index): word = words[get_index(i)] mac = int(word.replace(b':', b''), 16) if _is_universal(mac): - print('%012x' % mac, 'universal', command) return mac - print('%012x' % mac, 'local', command) first_local_mac = first_local_mac or mac except (ValueError, IndexError): # Virtual interfaces, such as those provided by From efb6c07d0ff038d5c7b76c4189ad358e48743250 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sun, 26 Nov 2017 18:48:24 -0500 Subject: [PATCH 29/36] Add NEWS after all. --- .../next/Library/2017-11-26-18-48-17.bpo-32107.h2ph2K.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2017-11-26-18-48-17.bpo-32107.h2ph2K.rst diff --git a/Misc/NEWS.d/next/Library/2017-11-26-18-48-17.bpo-32107.h2ph2K.rst b/Misc/NEWS.d/next/Library/2017-11-26-18-48-17.bpo-32107.h2ph2K.rst new file mode 100644 index 00000000000000..fc4f5fe9f19e1e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-11-26-18-48-17.bpo-32107.h2ph2K.rst @@ -0,0 +1,4 @@ +Improve the private ``*_getnode()`` methods for UUID1 such that universally +administered MAC addresses are preferred over locally administered MAC +addresses. If only the latter is available, the first such one is returned. +Improve the related tests and fix some bugs there as well. From 17deb59f5a0ca52f719cbb4fa7cb796d94341798 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Mon, 27 Nov 2017 11:47:58 -0500 Subject: [PATCH 30/36] Respond to review comments. * Restored the check for the multicast bit. * Remove some now-extraneous skipIfs. --- Lib/test/test_uuid.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 00a69158ec3ad4..294e87c65bdf62 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -512,7 +512,9 @@ def test_find_mac(self): self.assertEqual(mac, 0x1234567890ab) - def check_node(self, node, requires=None, *, random=False): + def check_node(self, node, requires=None, *, + # Additional bitmask checks to perform. + local=False, multicast=False): if requires and node is None: self.skipTest('requires ' + requires) hex = '%012x' % node @@ -527,21 +529,22 @@ def check_node(self, node, requires=None, *, random=False): # `test_random_getnode()` method specifically. Another case is the # Travis-CI case, which apparently only has locally administered MAC # addresses. - if not random and not os.getenv('TRAVIS'): + if not local and not os.getenv('TRAVIS'): self.assertFalse(node & (1 << 41), '%012x' % node) + is_multicast = (node & (1 << 40)) + if multicast: + self.assertTrue(is_multicast, '%012x' % node) + else: + self.assertFalse(is_multicast, '%012x' % node) self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) @unittest.skipUnless(os.name == 'posix', 'requires Posix') - @unittest.skipIf(os.getenv('TRAVIS'), - 'Travis-CI has no universally administered MAC addresses') def test_ifconfig_getnode(self): node = self.uuid._ifconfig_getnode() self.check_node(node, 'ifconfig') @unittest.skipUnless(os.name == 'posix', 'requires Posix') - @unittest.skipIf(os.getenv('TRAVIS'), - 'Travis-CI has no universally administered MAC addresses') def test_ip_getnode(self): node = self.uuid._ip_getnode() self.check_node(node, 'ip') @@ -578,7 +581,7 @@ def test_random_getnode(self): # must be set for randomly generated MAC addresses. See RFC 4122, # $4.1.6. self.assertTrue(node & (1 << 40), '%012x' % node) - self.check_node(node, random=True) + self.check_node(node, local=True, multicast=True) @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_unix_getnode(self): From 34e4cb18ac8347d215e99a7fd24a0e39053b7a3d Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Mon, 27 Nov 2017 12:12:58 -0500 Subject: [PATCH 31/36] Tweak the tests one more time. * Rename `local` to `admin` and use this to control whether the local vs. universal admin bit is set. Generally we do want to test that, except for the random case, and the two cases where we don't know what the provenance of the MAC address is. * We only need to check the multicast bit in the random test. --- Lib/test/test_uuid.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 294e87c65bdf62..372d5d65064258 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -512,9 +512,7 @@ def test_find_mac(self): self.assertEqual(mac, 0x1234567890ab) - def check_node(self, node, requires=None, *, - # Additional bitmask checks to perform. - local=False, multicast=False): + def check_node(self, node, requires=None, *, admin=True): if requires and node is None: self.skipTest('requires ' + requires) hex = '%012x' % node @@ -529,13 +527,8 @@ def check_node(self, node, requires=None, *, # `test_random_getnode()` method specifically. Another case is the # Travis-CI case, which apparently only has locally administered MAC # addresses. - if not local and not os.getenv('TRAVIS'): + if admin and not os.getenv('TRAVIS'): self.assertFalse(node & (1 << 41), '%012x' % node) - is_multicast = (node & (1 << 40)) - if multicast: - self.assertTrue(is_multicast, '%012x' % node) - else: - self.assertFalse(is_multicast, '%012x' % node) self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) @@ -581,7 +574,7 @@ def test_random_getnode(self): # must be set for randomly generated MAC addresses. See RFC 4122, # $4.1.6. self.assertTrue(node & (1 << 40), '%012x' % node) - self.check_node(node, local=True, multicast=True) + self.check_node(node, admin=False) @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_unix_getnode(self): @@ -591,13 +584,17 @@ def test_unix_getnode(self): node = self.uuid._unix_getnode() except TypeError: self.skipTest('requires uuid_generate_time') - self.check_node(node, 'unix') + # Since we don't know the provenance of the MAC address, don't check + # whether it is locally or universally administered. + self.check_node(node, 'unix', admin=False) @unittest.skipUnless(os.name == 'nt', 'requires Windows') @unittest.skipUnless(importable('ctypes'), 'requires ctypes') def test_windll_getnode(self): node = self.uuid._windll_getnode() - self.check_node(node) + # Since we don't know the provenance of the MAC address, don't check + # whether it is locally or universally administered. + self.check_node(node, admin=False) class TestInternalsWithoutExtModule(BaseTestInternals, unittest.TestCase): From af4a656a05e60ec8439b60ed894453464a257ca8 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Mon, 27 Nov 2017 14:01:49 -0500 Subject: [PATCH 32/36] s/admin/check_bit/ --- Lib/test/test_uuid.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 372d5d65064258..54d73f7391e2c9 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -512,7 +512,7 @@ def test_find_mac(self): self.assertEqual(mac, 0x1234567890ab) - def check_node(self, node, requires=None, *, admin=True): + def check_node(self, node, requires=None, *, check_bit=True): if requires and node is None: self.skipTest('requires ' + requires) hex = '%012x' % node @@ -527,7 +527,7 @@ def check_node(self, node, requires=None, *, admin=True): # `test_random_getnode()` method specifically. Another case is the # Travis-CI case, which apparently only has locally administered MAC # addresses. - if admin and not os.getenv('TRAVIS'): + if check_bit and not os.getenv('TRAVIS'): self.assertFalse(node & (1 << 41), '%012x' % node) self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) @@ -574,7 +574,7 @@ def test_random_getnode(self): # must be set for randomly generated MAC addresses. See RFC 4122, # $4.1.6. self.assertTrue(node & (1 << 40), '%012x' % node) - self.check_node(node, admin=False) + self.check_node(node, check_bit=False) @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_unix_getnode(self): @@ -586,7 +586,7 @@ def test_unix_getnode(self): self.skipTest('requires uuid_generate_time') # Since we don't know the provenance of the MAC address, don't check # whether it is locally or universally administered. - self.check_node(node, 'unix', admin=False) + self.check_node(node, 'unix', check_bit=False) @unittest.skipUnless(os.name == 'nt', 'requires Windows') @unittest.skipUnless(importable('ctypes'), 'requires ctypes') @@ -594,7 +594,7 @@ def test_windll_getnode(self): node = self.uuid._windll_getnode() # Since we don't know the provenance of the MAC address, don't check # whether it is locally or universally administered. - self.check_node(node, admin=False) + self.check_node(node, check_bit=False) class TestInternalsWithoutExtModule(BaseTestInternals, unittest.TestCase): From 31ec4fdc98c31e770ba6369bb4d7d813345fee59 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Mon, 27 Nov 2017 19:02:20 -0500 Subject: [PATCH 33/36] Remove the bitmask check for universally administered MACs. This fails miserably on the buildbots in addition to Travis. --- Lib/test/test_uuid.py | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index 54d73f7391e2c9..e3b48afdf38d0d 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -512,23 +512,12 @@ def test_find_mac(self): self.assertEqual(mac, 0x1234567890ab) - def check_node(self, node, requires=None, *, check_bit=True): + def check_node(self, node, requires=None): if requires and node is None: self.skipTest('requires ' + requires) hex = '%012x' % node if support.verbose >= 2: print(hex, end=' ') - # The MAC address will be universally administered (i.e. the second - # least significant bit of the first octet must be unset) for any - # physical interface, such as an ethernet port or wireless adapter. - # There are some cases where this won't be the case. Randomly - # generated MACs may not be universally administered, but they must - # have their multicast bit set, though this is tested in the - # `test_random_getnode()` method specifically. Another case is the - # Travis-CI case, which apparently only has locally administered MAC - # addresses. - if check_bit and not os.getenv('TRAVIS'): - self.assertFalse(node & (1 << 41), '%012x' % node) self.assertTrue(0 < node < (1 << 48), "%s is not an RFC 4122 node ID" % hex) @@ -574,7 +563,7 @@ def test_random_getnode(self): # must be set for randomly generated MAC addresses. See RFC 4122, # $4.1.6. self.assertTrue(node & (1 << 40), '%012x' % node) - self.check_node(node, check_bit=False) + self.check_node(node) @unittest.skipUnless(os.name == 'posix', 'requires Posix') def test_unix_getnode(self): @@ -586,7 +575,7 @@ def test_unix_getnode(self): self.skipTest('requires uuid_generate_time') # Since we don't know the provenance of the MAC address, don't check # whether it is locally or universally administered. - self.check_node(node, 'unix', check_bit=False) + self.check_node(node, 'unix') @unittest.skipUnless(os.name == 'nt', 'requires Windows') @unittest.skipUnless(importable('ctypes'), 'requires ctypes') @@ -594,7 +583,7 @@ def test_windll_getnode(self): node = self.uuid._windll_getnode() # Since we don't know the provenance of the MAC address, don't check # whether it is locally or universally administered. - self.check_node(node, check_bit=False) + self.check_node(node) class TestInternalsWithoutExtModule(BaseTestInternals, unittest.TestCase): From eedce472aa9ba50bb1b32fb3485a26ef8c4cd5b6 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Mon, 27 Nov 2017 19:06:10 -0500 Subject: [PATCH 34/36] Remove some unnecessary comments --- Lib/test/test_uuid.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py index e3b48afdf38d0d..f113c551209c35 100644 --- a/Lib/test/test_uuid.py +++ b/Lib/test/test_uuid.py @@ -573,16 +573,12 @@ def test_unix_getnode(self): node = self.uuid._unix_getnode() except TypeError: self.skipTest('requires uuid_generate_time') - # Since we don't know the provenance of the MAC address, don't check - # whether it is locally or universally administered. self.check_node(node, 'unix') @unittest.skipUnless(os.name == 'nt', 'requires Windows') @unittest.skipUnless(importable('ctypes'), 'requires ctypes') def test_windll_getnode(self): node = self.uuid._windll_getnode() - # Since we don't know the provenance of the MAC address, don't check - # whether it is locally or universally administered. self.check_node(node) From 55f2da9cfa67b4edc12d064f032140766ed06b60 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 28 Nov 2017 09:52:02 -0500 Subject: [PATCH 35/36] Respond to review * Update the getnode() documentation. * Add a versionchanged note. * Elaborate a comment. --- Doc/library/uuid.rst | 16 ++++++++++++---- Lib/uuid.py | 5 +++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Doc/library/uuid.rst b/Doc/library/uuid.rst index ea9ea7dc7d9b82..8ec75a79acfa71 100644 --- a/Doc/library/uuid.rst +++ b/Doc/library/uuid.rst @@ -156,10 +156,18 @@ The :mod:`uuid` module defines the following functions: Get the hardware address as a 48-bit positive integer. The first time this runs, it may launch a separate program, which could be quite slow. If all - attempts to obtain the hardware address fail, we choose a random 48-bit number - with its eighth bit set to 1 as recommended in RFC 4122. "Hardware address" - means the MAC address of a network interface, and on a machine with multiple - network interfaces the MAC address of any one of them may be returned. + attempts to obtain the hardware address fail, we choose a random 48-bit + number with the multicast bit (least significant bit of the first octet) + set to 1 as recommended in RFC 4122. "Hardware address" means the MAC + address of a network interface. On a machine with multiple network + interfaces, universally administered MAC addresses (i.e. where the second + least significant bit of the first octet is *unset*) will be preferred over + locally administered MAC addresses, but with no other ordering guarantees. + + .. versionchanged:: 3.7 + Universally administered MAC addresses are preferred over locally + administered MAC addresses, since the former are guaranteed to be + globally unique, while the latter are not. .. index:: single: getnode diff --git a/Lib/uuid.py b/Lib/uuid.py index 9e7c672f7a0a6f..cb2bc092bdfcde 100644 --- a/Lib/uuid.py +++ b/Lib/uuid.py @@ -351,8 +351,9 @@ def _popen(command, *args): # 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 skip over any locally administered MAC -# addresses, as it makes no sense to use those in UUID calculation. +# 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 From db9d0cf0ec3866354d1528bba3d4f1f94310e916 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 28 Nov 2017 16:23:56 -0500 Subject: [PATCH 36/36] A better blurb --- .../Library/2017-11-26-18-48-17.bpo-32107.h2ph2K.rst | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2017-11-26-18-48-17.bpo-32107.h2ph2K.rst b/Misc/NEWS.d/next/Library/2017-11-26-18-48-17.bpo-32107.h2ph2K.rst index fc4f5fe9f19e1e..b26daa7b1b3bfa 100644 --- a/Misc/NEWS.d/next/Library/2017-11-26-18-48-17.bpo-32107.h2ph2K.rst +++ b/Misc/NEWS.d/next/Library/2017-11-26-18-48-17.bpo-32107.h2ph2K.rst @@ -1,4 +1,5 @@ -Improve the private ``*_getnode()`` methods for UUID1 such that universally -administered MAC addresses are preferred over locally administered MAC -addresses. If only the latter is available, the first such one is returned. -Improve the related tests and fix some bugs there as well. +``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.