[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Xavier de Gaye  added the comment:

Thanks Serhiy and Barry for your comments and reviews :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Barry A. Warsaw]

Barry A. Warsaw  added the comment:

LGTM, and thanks!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Change by Xavier de Gaye :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Xavier de Gaye  added the comment:


New changeset 03031fbc7d44106d652756462db34eae67de9568 by xdegaye (Miss 
Islington (bot)) in branch '3.6':
bpo-32199:  The getnode() ip getter now uses 'ip link' instead of 'ip link 
list' (GH-4696) (#4747)
https://github.com/python/cpython/commit/03031fbc7d44106d652756462db34eae67de9568


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Roundup Robot]

Change by Roundup Robot :


--
pull_requests: +4650

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Xavier de Gaye  added the comment:


New changeset 961dbe0548e26394b7716d41423c61b1e2e58ef7 by xdegaye in branch 
'master':
bpo-32199:  The getnode() ip getter now uses 'ip link' instead of 'ip link 
list' (GH-4696)
https://github.com/python/cpython/commit/961dbe0548e26394b7716d41423c61b1e2e58ef7


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Xavier de Gaye  added the comment:

On archlinux it is easy to know precisely what patches are applied to iproute2 
and how it is built (see 
https://git.archlinux.org/svntogit/packages.git/tree/trunk?h=packages/iproute2).

The attached two files, archlinux-ip_link.strace and 
archlinux-ip_link_list.strace, contain the output of strace run on the commands 
'ip link' and 'ip link list' on archlinux.
* For 'ip link', the sendto() syscall uses RTM_GETLINK to get information about 
a specific network interface.
* For 'ip link list', this sendto() syscall is preceded by another sendto() 
syscall using RTM_NEWLINK to *create* information about a specific network 
interface.

Conclusions:
1) Both commands are not equivalent, this seems to be a bug in iproute2 or its 
documentation (I did not read the whole iproute2 documentation).
2) By using RTM_NEWLINK, 'ip link list' requests a write-like operation that 
may be denied by SELinux if there is no policy that allows netlink_route_socket 
(nlmsg_write). I may be wrong but on Android API 26 it seems that only few 
processes get that permission: dhcp, clatd, logd, netd, rild, ...
3) From Python perspective it is more robust to call 'ip link' to handle 
platforms where SELinux is run in enforcing mode.

I will update the PR to do only that change: s/ip link list/ip link/

--
versions: +Python 3.6
Added file: https://bugs.python.org/file47327/archlinux-ip_link_list.strace

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Change by Xavier de Gaye :


Added file: https://bugs.python.org/file47326/archlinux-ip_link.strace

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Barry A. Warsaw]

Barry A. Warsaw  added the comment:

> Maybe we should just close this issue as 'wont fix' then.

I would be okay with any of these resolutions:

* Close as `wont fix`
* Just call `ip link` (without list)
* Add a new getter such that both `ip link` and `ip link list` are called.

How to handle exceptions in the getters should be addressed in a new issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Barry A. Warsaw]

Barry A. Warsaw  added the comment:

On Dec 6, 2017, at 02:06, Xavier de Gaye  wrote:
> 
> Whatever the change made to fix this issue, it is not possible to add a test 
> case for this change.

Even with say, exception raising mocks for the getters?

> So following the suggestion made by Barry in PR 4696, we can add (in another 
> issue) a new keyword parameter to getnode() named 'methods' whose value may 
> be None (the default, meaning try all the known methods) or a tuple 
> containing a subset of the following methods ('unix', 'ifconfig', 'ip', 
> 'arp', 'lanscan', 'netstat',  'random') that would raise an exception if the 
> value cannot be obtained using one of the requested method tried in the 
> requested order. This would also improve the documentation on the methods 
> getnode() is using. Then if we decide to make the change for 'ip link' in the 
> current issue, one can add a test case that would first test for the 
> avaibility of the ip command and if the command exists would fail if 
> getnode(methods=('ip',)) raises an exception.

I am thinking about this slightly differently.

What if getnode() accepted a `handler` argument and the code was changed to 
something like this:

1 file changed, 4 insertions(+), 2 deletions(-)
Lib/uuid.py | 6 --

modified   Lib/uuid.py
@@ -656,7 +656,7 @@ def _random_getnode():

 _node = None

-def getnode():
+def getnode(handler=None):
 """Get the hardware address as a 48-bit positive integer.

 The first time this runs, it may launch a separate program, which could
@@ -677,7 +677,9 @@ def getnode():
 for getter in getters + [_random_getnode]:
 try:
 _node = getter()
-except:
+except Exception as error:
+if handler is not None:
+handler(getter, error)
 continue
 if _node is not None:
 return _node

`handler` could log some diagnostics, reraise the exception, raise 
StopIteration, etc.  Then we could use that in the test suite too, because we 
could mock a getter to raise an exception and then pass in a handler that 
verified the exception was raised with the expected getter.

(Maybe we spell `handler` as `error_handler`.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Xavier de Gaye  added the comment:

Attached ip_link.strace and ip_link_list.strace, the output of strace for the 
'ip link' and 'ip link list' commands.

At the end of the process, both commands create an PF_NETLINK socket to receive 
from the kernel link information through the NETLINK_ROUTE group. The sendto() 
function fails with EACCES for 'ip link list' and its SELinux avc record 
relates to this event (the netlink prefix used throughout the documentation [1] 
is 'nlmsg' and the resource denied in the avc record is 'nlmsg_write'). The 
SELinux avc record for 'ip link' relates to a failed attempt to open 
"/data/misc/net/group" (it does not exist).

[1] http://man7.org/linux/man-pages/man7/netlink.7.html

--
Added file: https://bugs.python.org/file47323/ip_link_list.strace

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Change by Xavier de Gaye :


Added file: https://bugs.python.org/file47322/ip_link.strace

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Xavier de Gaye  added the comment:

It is also possible that rather than an oversight in Android, it is a side 
effect of SELinux on the implementation of iproute2 if the 'ip link list' 
command does a little bit more than the 'ip link' command and if this 'little 
bit more' is prohibited by a SELinux policy. I guess this means diving into the 
source of iproute2 to confirm that :-(

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Xavier de Gaye  added the comment:

Oops, the second failed command is 'ip link list' of course.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Xavier de Gaye  added the comment:

'adb logcat' is a tool that monitors many kind of events on Android. Both ip 
commands prints a SElinux record on logcat:

Upon the successfull 'ip link' command, logcat prints:
12-06 09:17:24.119  2460  2460 W ip  : type=1400 audit(0.0:8): avc: denied 
{ search } for name="net" dev="vdc" ino=91 scontext=u:r:shell:s0 
tcontext=u:object_r:net_data_file:s0 tclass=dir permissive=0

Upon the failed 'ip link' command, logcat prints:
12-06 09:17:42.109  2461  2461 W ip  : type=1400 audit(0.0:9): avc: denied 
{ nlmsg_write } for scontext=u:r:shell:s0 tcontext=u:r:shell:s0 
tclass=netlink_route_socket permissive=0

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Xavier de Gaye  added the comment:

You may be right Serhiy. Those tests have been run on the emulator at API 24 
(Android 7.0 Nougat, the first API version where SELinux is run in enforced 
mode) where 'ip link list' fails, but on my device (a Samsung API 21, Android 
5.1 Lollipop) running the 'ip link list' (using the termux package installed 
from google PlayStore) the command is ok.

The Android source of iproute2 can be:
* browsed at: 
https://android.googlesource.com/platform/external/iproute2/+/master
* cloned from: https://android.googlesource.com/platform/external/iproute2/
They are using iproute2-3.4.0 released in 2012, see README.version.
I checked that the ip/ subdirectory on the Android master branch is unchanged 
from upstream 3.4.0 at https://www.kernel.org/pub/linux/utils/net/iproute2/.

The Android SELinux policies are at:
https://android.googlesource.com/platform/system/sepolicy/
Does someone know how to read them ?

Maybe we should just close this issue as 'wont fix' then.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Xavier de Gaye  added the comment:

Whatever the change made to fix this issue, it is not possible to add a test 
case for this change.

So following the suggestion made by Barry in PR 4696, we can add (in another 
issue) a new keyword parameter to getnode() named 'methods' whose value may be 
None (the default, meaning try all the known methods) or a tuple containing a 
subset of the following methods ('unix', 'ifconfig', 'ip', 'arp', 'lanscan', 
'netstat',  'random') that would raise an exception if the value cannot be 
obtained using one of the requested method tried in the requested order. This 
would also improve the documentation on the methods getnode() is using. Then if 
we decide to make the change for 'ip link' in the current issue, one can add a 
test case that would first test for the avaibility of the ip command and if the 
command exists would fail if getnode(methods=('ip',)) raises an exception.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

What if "ip link list" was intentionally prohibited "for security reasons", and 
"ip link" works just due to oversight? Xavier, could you please inspect the 
sources of the ip command on Android? Is it the standard iproute2 with 
additional patches prohibiting the part of the functionality?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Barry A. Warsaw]

Barry A. Warsaw  added the comment:

On Dec 5, 2017, at 16:28, Xavier de Gaye  wrote:
> 
> The result of various 'ip' commands on Android, the last 'ip link list' 
> command is run as root and succeeds (did not think about trying that before):
> 
> generic_x86_64:/data/local/tmp/python $ ip link list
> request send failed: Permission denied
> 
> 1|generic_x86_64:/data/local/tmp/python $ ip link help
> request send failed: Permission denied
> 
> 1|generic_x86_64:/data/local/tmp/python $ ip link
…[output]…

Well, that’s weird!
-B

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Xavier de Gaye  added the comment:

The result of various 'ip' commands on Android, the last 'ip link list' command 
is run as root and succeeds (did not think about trying that before):

generic_x86_64:/data/local/tmp/python $ ip link list
request send failed: Permission denied

1|generic_x86_64:/data/local/tmp/python $ ip link help
request send failed: Permission denied

1|generic_x86_64:/data/local/tmp/python $ ip link
1: lo:  mtu 65536 qdisc noqueue state UNKNOWN mode 
DEFAULT group default 
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
3: wlan0:  mtu 1500 qdisc mq state UP mode 
DORMANT group default qlen 1000
link/ether 02:00:00:44:55:66 brd ff:ff:ff:ff:ff:ff
5: hwsim0:  mtu 1500 qdisc noop state DOWN mode DEFAULT 
group default 
link/ieee802.11/radiotap 12:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
6: sit0:  mtu 1480 qdisc noop state DOWN mode DEFAULT group default 
link/sit 0.0.0.0 brd 0.0.0.0
8: radio0:  mtu 1500 qdisc pfifo_fast state UP 
mode DEFAULT group default qlen 1000
link/ether 22:d5:92:86:1a:d8 brd ff:ff:ff:ff:ff:ff

generic_x86_64:/data/local/tmp/python # ip link list
1: lo:  mtu 65536 qdisc noqueue state UNKNOWN mode 
DEFAULT group default 
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
3: wlan0:  mtu 1500 qdisc mq state UP mode 
DORMANT group default qlen 1000
link/ether 02:00:00:44:55:66 brd ff:ff:ff:ff:ff:ff
5: hwsim0:  mtu 1500 qdisc noop state DOWN mode DEFAULT 
group default 
link/ieee802.11/radiotap 12:00:00:00:00:00 brd ff:ff:ff:ff:ff:ff
6: sit0:  mtu 1480 qdisc noop state DOWN mode DEFAULT group default 
link/sit 0.0.0.0 brd 0.0.0.0
8: radio0:  mtu 1500 qdisc pfifo_fast state DOWN mode 
DEFAULT group default qlen 1000
link/ether 22:d5:92:86:1a:d8 brd ff:ff:ff:ff:ff:ff

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Why the 'ip link list' command fails on Android at first place? Does Android 
use its own independent implementation? Or its version is based on the fork of 
very old version of iproute2 that didn't supported the list command (if there 
was such version)?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Barry A. Warsaw]

Barry A. Warsaw  added the comment:

Over in the PR I suggested:

Here's another thought: what if you just added another getter that calls ip 
link list and placed that after one that calls ip link. Wouldn't that 
accomplish both goals? Then if ip link fails, we fall back to the old behavior, 
so nothing changes. It's uglier, but it doesn't special case for the Android 
platform, and eventually we can decide to remove ip link list altogether.

--
nosy: +barry

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

Change by Xavier de Gaye :


--
keywords: +patch
pull_requests: +4609
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
keywords: +easy
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32199] uuid.getnode() should return the MAC address on Android

[Xavier de Gaye]

New submission from Xavier de Gaye :

Currently uuid.getnode() returns a random 48-bit number and so the UUIDs are 
not persistent across time. The reason is that on Android the 'ip link list' 
command fails.

uuid._ip_getnode() should invoke the 'ip link' command instead.

--
components: Library (Lib)
messages: 307432
nosy: xdegaye
priority: normal
severity: normal
stage: needs patch
status: open
title: uuid.getnode() should return the MAC address on Android
type: behavior
versions: Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com