Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
ebbaa22
bpo-32107 - Fix test_uuid
warsaw Nov 21, 2017
941b47d
Return None instead of 0
warsaw Nov 21, 2017
e529214
Never return a locally administered MAC address
warsaw Nov 22, 2017
24c4db2
Several sleep-improved fixes
warsaw Nov 23, 2017
c056584
A missed return
warsaw Nov 23, 2017
7fd446e
Two typos
warsaw Nov 23, 2017
fb2c2f2
DEBUGGING TRAVIS - DO NOT MERGE
warsaw Nov 25, 2017
99e86b2
Two tests cannot succeed on Travis-CI
warsaw Nov 26, 2017
74702e3
bpo-32107: Remove the check of the multicast bit in test_uuid.
serhiy-storchaka Nov 26, 2017
41adbad
bpo-32107 - Fix test_uuid
warsaw Nov 21, 2017
d3d0465
Return None instead of 0
warsaw Nov 21, 2017
7c17cd8
Never return a locally administered MAC address
warsaw Nov 22, 2017
511fab0
Several sleep-improved fixes
warsaw Nov 23, 2017
af7929c
A missed return
warsaw Nov 23, 2017
51ba0b3
Two typos
warsaw Nov 23, 2017
909ee79
DEBUGGING TRAVIS - DO NOT MERGE
warsaw Nov 25, 2017
6acb120
Two tests cannot succeed on Travis-CI
warsaw Nov 26, 2017
b7d647c
Merge branch 'test_uuid' of github.com:warsaw/cpython into test_uuid
warsaw Nov 26, 2017
26db2f5
Merge branch 'test-uuid-multicast-bit' of https://github.com/serhiy-s…
warsaw Nov 26, 2017
18a96ce
bpo-32107 - Fix test_uuid
warsaw Nov 21, 2017
83dc3a3
Return None instead of 0
warsaw Nov 21, 2017
fb156f3
Never return a locally administered MAC address
warsaw Nov 22, 2017
3e8e17d
Several sleep-improved fixes
warsaw Nov 23, 2017
668f4f6
A missed return
warsaw Nov 23, 2017
6eecb80
Two typos
warsaw Nov 23, 2017
87712f3
DEBUGGING TRAVIS - DO NOT MERGE
warsaw Nov 25, 2017
740393e
Two tests cannot succeed on Travis-CI
warsaw Nov 26, 2017
225b063
bpo-32107: Remove the check of the multicast bit in test_uuid.
serhiy-storchaka Nov 26, 2017
5b8ca90
DEBUGGING TRAVIS - DO NOT MERGE
warsaw Nov 25, 2017
2aa4f50
Two tests cannot succeed on Travis-CI
warsaw Nov 26, 2017
71ab3b5
Merge branch 'test_uuid' of github.com:warsaw/cpython into test_uuid
warsaw Nov 26, 2017
efb6c07
Add NEWS after all.
warsaw Nov 26, 2017
17deb59
Respond to review comments.
warsaw Nov 27, 2017
34e4cb1
Tweak the tests one more time.
warsaw Nov 27, 2017
af4a656
s/admin/check_bit/
warsaw Nov 27, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 30 additions & 17 deletions Lib/test/test_uuid.py
Original file line number Diff line number Diff line change
Expand Up @@ -512,60 +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, *, check_bit=True):
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)
# 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)

@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.
self.assertTrue(node & 0x010000000000, '%012x' % node)
self.check_node(node)
# 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, check_bit=False)

@unittest.skipUnless(os.name == 'posix', 'requires Posix')
def test_unix_getnode(self):
Expand All @@ -575,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', check_bit=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, check_bit=False)


class TestInternalsWithoutExtModule(BaseTestInternals, unittest.TestCase):
Expand Down
70 changes: 59 additions & 11 deletions Lib/uuid.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,11 +342,29 @@ 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.
#
# 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
return None
with proc:
for line in proc.stdout:
words = line.lower().rstrip().split()
Expand All @@ -355,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:
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
Expand All @@ -366,6 +385,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."""
Expand All @@ -375,13 +395,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."""
Expand All @@ -404,8 +426,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."""
Expand All @@ -415,32 +439,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
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 _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

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
Expand All @@ -458,18 +486,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):
return int(value.replace('-', ''), 16)
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
return None
adapters._unpack()
for i in range(adapters.length):
ncb.Reset()
Expand All @@ -488,7 +521,11 @@ 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 _is_universal(mac):
return mac
first_local_mac = first_local_mac or mac
return first_local_mac or None


_generate_time_safe = _UuidCreate = None
Expand Down Expand Up @@ -601,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 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) | 0x010000000000
return random.getrandbits(48) | (1 << 40)


_node = None
Expand All @@ -626,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:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The difference is that exceptions in _random_getnode() no longer ignored. Ignoring all exceptions is not a good idea, but what exceptions can raise _random_getnode()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think the only possibility is an import error. Ultimately, random.getrandbits() boils down to os.urandom() which I think can block but AFAICT won't raise an exception. And it bothers me (as I think it bothers you) that all exceptions in all those getters are just swallowed. Here's my thinking: since I'm not sure of the best way to report any errors in the other getters, and since the last getter is _random_getnode(), and since it's pretty unlikely that the function will experience an ImportError, and if it does then I think it's better to report that than to silently swallow the exception and return None from getnode(), since that will lead to even more mysterious bugs, I think letting any rare exceptions in _random_getnode() not be swallowed is the better choice.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's LGTM. I just wanted to make sure that this is an intentional change.

try:
_node = getter()
except:
continue
if _node is not None:
return _node
return _random_getnode()


_last_timestamp = None
Expand Down
Original file line number Diff line number Diff line change
@@ -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.