Review Request 124747: Fix random MAC generation on Wired/WiFi

[Boudhayan Gupta]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124747/
-----------------------------------------------------------

Review request for Network Management.


Repository: plasma-nm


Description
-------

Randomly generated MAC addresses aren't truly random; they should have an even first octet (LSB set to 0) to signify that this is an Unicast address, and the second least-significant-bit should be set to 1 to specify that it is a locally administered MAC, not a globally enforced one.

Setting the Multicast bit (current behaviour half the time) will result in the kernel refusing to set the randomly generated MAC address, with a small entry in Journalctl/Syslog:

[platform/nm-linux-platform.c:2782] do_change_link(): platform-linux: do-change-link: failure changing link 3: Invalid address for specified address family (19)

This small patch fixes random MAC generation for both Wired and WiFi connections


Diffs
-----

libs/editor/settings/wificonnectionwidget.cpp d22fda0
libs/editor/settings/wiredconnectionwidget.cpp 80523ef

Diff: https://git.reviewboard.kde.org/r/124747/diff/


Testing
-------

Builds, correctly generates random MACs with last two bits of first octet set to 10, saves said MAC.


Thanks,

Boudhayan Gupta

[Lamarque Souza]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124747/#review83839
-----------------------------------------------------------



libs/editor/settings/wificonnectionwidget.cpp (line 132)
<https://git.reviewboard.kde.org/r/124747/#comment58086>

Comments starts with capital letter and ends with a period.



libs/editor/settings/wificonnectionwidget.cpp (line 133)
<https://git.reviewboard.kde.org/r/124747/#comment58085>

You can use:

mac[0] &= ~0x1;
mac[0] |= 0x2;

It is more common to specify bits in hexadecimal than decimal. I know, in this particular case they use the same digits.


- Lamarque Souza

Post by Boudhayan Gupta
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/124747/
-----------------------------------------------------------
(Updated Aug. 15, 2015, 6:45 p.m.)
Review request for Network Management.
Repository: plasma-nm
Description
-------
Randomly generated MAC addresses aren't truly random; they should have an even first octet (LSB set to 0) to signify that this is an Unicast address, and the second least-significant-bit should be set to 1 to specify that it is a locally administered MAC, not a globally enforced one.
[platform/nm-linux-platform.c:2782] do_change_link(): platform-linux: do-change-link: failure changing link 3: Invalid address for specified address family (19)
This small patch fixes random MAC generation for both Wired and WiFi connections
Diffs
-----
libs/editor/settings/wificonnectionwidget.cpp d22fda0
libs/editor/settings/wiredconnectionwidget.cpp 80523ef
Diff: https://git.reviewboard.kde.org/r/124747/diff/
Testing
-------
Builds, correctly generates random MACs with last two bits of first octet set to 10, saves said MAC.
Thanks,
Boudhayan Gupta

[Boudhayan Gupta]

Post by Boudhayan Gupta

libs/editor/settings/wificonnectionwidget.cpp, line 133
<https://git.reviewboard.kde.org/r/124747/diff/1/?file=394828#file394828line133>
mac[0] &= ~0x1;
mac[0] |= 0x2;
It is more common to specify bits in hexadecimal than decimal. I know, in this particular case they use the same digits.

I'll fix it to use hex, but using the &= and |= shorthand gives a very weird build error:

```
/home/bg14ina/Projects/plasma-nm/libs/editor/settings/wificonnectionwidget.cpp: In member function ‘void WifiConnectionWidget::generateRandomClonedMac()’:
/home/bg14ina/Projects/plasma-nm/libs/editor/settings/wificonnectionwidget.cpp:133:12: error: no match for ‘operator&=’ (operand types are ‘QByteRef’ and ‘int’)
mac[0] &= ~1;
^
/home/bg14ina/Projects/plasma-nm/libs/editor/settings/wificonnectionwidget.cpp:134:12: error: no match for ‘operator|=’ (operand types are ‘QByteRef’ and ‘int’)
mac[0] |= 2;
^
In file included from /usr/include/qt/QtCore/QtCore:48:0,
from /usr/include/qt/QtDBus/QtDBusDepends:3,
from /usr/include/qt/QtDBus/QtDBus:3,
from /usr/include/KF5/ModemManagerQt/generictypes.h:30,
from /usr/include/KF5/ModemManagerQt/bearer.h:30,
from /usr/include/KF5/ModemManagerQt/modem.h:37,
from /home/bg14ina/Projects/plasma-nm/libs/uiutils.h:41,
from /home/bg14ina/Projects/plasma-nm/libs/editor/settings/wificonnectionwidget.cpp:28:
/usr/include/qt/QtCore/qurl.h:377:33: note: candidate: QUrl::FormattingOptions& operator|=(QUrl::FormattingOptions&, QUrl::ComponentFormattingOptions)
inline QUrl::FormattingOptions &operator|=(QUrl::FormattingOptions &i, QUrl::ComponentFormattingOptions f)
^
/usr/include/qt/QtCore/qurl.h:377:33: note: no known conversion for argument 1 from ‘QByteRef’ to ‘QUrl::FormattingOptions& {aka QUrlTwoFlags<QUrl::UrlFormattingOption, QUrl::ComponentFormattingOption>&}’
```

Post by Boudhayan Gupta

libs/editor/settings/wificonnectionwidget.cpp, line 132
<https://git.reviewboard.kde.org/r/124747/diff/1/?file=394828#file394828line132>
Comments starts with capital letter and ends with a period.

fixing


- Boudhayan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124747/#review83839
-----------------------------------------------------------

Post by Boudhayan Gupta
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/124747/
-----------------------------------------------------------
(Updated Aug. 16, 2015, 12:15 a.m.)
Review request for Network Management.
Repository: plasma-nm
Description
-------
Randomly generated MAC addresses aren't truly random; they should have an even first octet (LSB set to 0) to signify that this is an Unicast address, and the second least-significant-bit should be set to 1 to specify that it is a locally administered MAC, not a globally enforced one.
[platform/nm-linux-platform.c:2782] do_change_link(): platform-linux: do-change-link: failure changing link 3: Invalid address for specified address family (19)
This small patch fixes random MAC generation for both Wired and WiFi connections
Diffs
-----
libs/editor/settings/wificonnectionwidget.cpp d22fda0
libs/editor/settings/wiredconnectionwidget.cpp 80523ef
Diff: https://git.reviewboard.kde.org/r/124747/diff/
Testing
-------
Builds, correctly generates random MACs with last two bits of first octet set to 10, saves said MAC.
Thanks,
Boudhayan Gupta

[Lamarque Souza]

Post by Boudhayan Gupta

libs/editor/settings/wificonnectionwidget.cpp, line 133
<https://git.reviewboard.kde.org/r/124747/diff/1/?file=394828#file394828line133>
mac[0] &= ~0x1;
mac[0] |= 0x2;
It is more common to specify bits in hexadecimal than decimal. I know, in this particular case they use the same digits.

```
/home/bg14ina/Projects/plasma-nm/libs/editor/settings/wificonnectionwidget.cpp:133:12: error: no match for ‘operator&=’ (operand types are ‘QByteRef’ and ‘int’)
mac[0] &= ~1;
^
/home/bg14ina/Projects/plasma-nm/libs/editor/settings/wificonnectionwidget.cpp:134:12: error: no match for ‘operator|=’ (operand types are ‘QByteRef’ and ‘int’)
mac[0] |= 2;
^
In file included from /usr/include/qt/QtCore/QtCore:48:0,
from /usr/include/qt/QtDBus/QtDBusDepends:3,
from /usr/include/qt/QtDBus/QtDBus:3,
from /usr/include/KF5/ModemManagerQt/generictypes.h:30,
from /usr/include/KF5/ModemManagerQt/bearer.h:30,
from /usr/include/KF5/ModemManagerQt/modem.h:37,
from /home/bg14ina/Projects/plasma-nm/libs/uiutils.h:41,
/usr/include/qt/QtCore/qurl.h:377:33: note: candidate: QUrl::FormattingOptions& operator|=(QUrl::FormattingOptions&, QUrl::ComponentFormattingOptions)
inline QUrl::FormattingOptions &operator|=(QUrl::FormattingOptions &i, QUrl::ComponentFormattingOptions f)
^
/usr/include/qt/QtCore/qurl.h:377:33: note: no known conversion for argument 1 from ‘QByteRef’ to ‘QUrl::FormattingOptions& {aka QUrlTwoFlags<QUrl::UrlFormattingOption, QUrl::ComponentFormattingOption>&}’
```

Ok, let's use the old version then. Do you have a developer account or do you need me to push this patch for you?


- Lamarque


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124747/#review83839
-----------------------------------------------------------

Post by Boudhayan Gupta
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/124747/
-----------------------------------------------------------
(Updated Aug. 15, 2015, 7:03 p.m.)
Review request for Network Management.
Repository: plasma-nm
Description
-------
Randomly generated MAC addresses aren't truly random; they should have an even first octet (LSB set to 0) to signify that this is an Unicast address, and the second least-significant-bit should be set to 1 to specify that it is a locally administered MAC, not a globally enforced one.
[platform/nm-linux-platform.c:2782] do_change_link(): platform-linux: do-change-link: failure changing link 3: Invalid address for specified address family (19)
This small patch fixes random MAC generation for both Wired and WiFi connections
Diffs
-----
libs/editor/settings/wificonnectionwidget.cpp d22fda0
libs/editor/settings/wiredconnectionwidget.cpp 80523ef
Diff: https://git.reviewboard.kde.org/r/124747/diff/
Testing
-------
Builds, correctly generates random MACs with last two bits of first octet set to 10, saves said MAC.
Thanks,
Boudhayan Gupta

[Boudhayan Gupta]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124747/
-----------------------------------------------------------

(Updated Aug. 16, 2015, 12:33 a.m.)


Review request for Network Management.


Changes
-------

Implement changes as per comments


Repository: plasma-nm


Description
-------

Randomly generated MAC addresses aren't truly random; they should have an even first octet (LSB set to 0) to signify that this is an Unicast address, and the second least-significant-bit should be set to 1 to specify that it is a locally administered MAC, not a globally enforced one.

Setting the Multicast bit (current behaviour half the time) will result in the kernel refusing to set the randomly generated MAC address, with a small entry in Journalctl/Syslog:

[platform/nm-linux-platform.c:2782] do_change_link(): platform-linux: do-change-link: failure changing link 3: Invalid address for specified address family (19)

This small patch fixes random MAC generation for both Wired and WiFi connections


Diffs (updated)
-----

libs/editor/settings/wificonnectionwidget.cpp d22fda0
libs/editor/settings/wiredconnectionwidget.cpp 80523ef

Diff: https://git.reviewboard.kde.org/r/124747/diff/


Testing
-------

Builds, correctly generates random MACs with last two bits of first octet set to 10, saves said MAC.


Thanks,

Boudhayan Gupta

[Lamarque Souza]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124747/#review83854
-----------------------------------------------------------

Ship it!


Ship It!

- Lamarque Souza

Post by Boudhayan Gupta
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/124747/
-----------------------------------------------------------
(Updated Aug. 15, 2015, 7:03 p.m.)
Review request for Network Management.
Repository: plasma-nm
Description
-------
Randomly generated MAC addresses aren't truly random; they should have an even first octet (LSB set to 0) to signify that this is an Unicast address, and the second least-significant-bit should be set to 1 to specify that it is a locally administered MAC, not a globally enforced one.
[platform/nm-linux-platform.c:2782] do_change_link(): platform-linux: do-change-link: failure changing link 3: Invalid address for specified address family (19)
This small patch fixes random MAC generation for both Wired and WiFi connections
Diffs
-----
libs/editor/settings/wificonnectionwidget.cpp d22fda0
libs/editor/settings/wiredconnectionwidget.cpp 80523ef
Diff: https://git.reviewboard.kde.org/r/124747/diff/
Testing
-------
Builds, correctly generates random MACs with last two bits of first octet set to 10, saves said MAC.
Thanks,
Boudhayan Gupta

[Boudhayan Gupta]

Post by Boudhayan Gupta

Post by Lamarque Souza
Ship It!

Done. Can you backport it to the 5.4 branch?


- Boudhayan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124747/#review83854
-----------------------------------------------------------

Post by Boudhayan Gupta
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/124747/
-----------------------------------------------------------
(Updated Aug. 16, 2015, 5:03 a.m.)
Review request for Network Management.
Repository: plasma-nm
Description
-------
Randomly generated MAC addresses aren't truly random; they should have an even first octet (LSB set to 0) to signify that this is an Unicast address, and the second least-significant-bit should be set to 1 to specify that it is a locally administered MAC, not a globally enforced one.
[platform/nm-linux-platform.c:2782] do_change_link(): platform-linux: do-change-link: failure changing link 3: Invalid address for specified address family (19)
This small patch fixes random MAC generation for both Wired and WiFi connections
Diffs
-----
libs/editor/settings/wificonnectionwidget.cpp d22fda0
libs/editor/settings/wiredconnectionwidget.cpp 80523ef
Diff: https://git.reviewboard.kde.org/r/124747/diff/
Testing
-------
Builds, correctly generates random MACs with last two bits of first octet set to 10, saves said MAC.
Thanks,
Boudhayan Gupta

[Boudhayan Gupta]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124747/
-----------------------------------------------------------

(Updated Aug. 15, 2015, 11:33 p.m.)


Status
------

This change has been marked as submitted.


Review request for Network Management.


Changes
-------

Submitted with commit d66c6bd1ca0954ac7c4268f4f0538078ea6b6a29 by Boudhayan Gupta to branch master.


Repository: plasma-nm


Description
-------

Randomly generated MAC addresses aren't truly random; they should have an even first octet (LSB set to 0) to signify that this is an Unicast address, and the second least-significant-bit should be set to 1 to specify that it is a locally administered MAC, not a globally enforced one.

Setting the Multicast bit (current behaviour half the time) will result in the kernel refusing to set the randomly generated MAC address, with a small entry in Journalctl/Syslog:

[platform/nm-linux-platform.c:2782] do_change_link(): platform-linux: do-change-link: failure changing link 3: Invalid address for specified address family (19)

This small patch fixes random MAC generation for both Wired and WiFi connections


Diffs
-----

libs/editor/settings/wificonnectionwidget.cpp d22fda0
libs/editor/settings/wiredconnectionwidget.cpp 80523ef

Diff: https://git.reviewboard.kde.org/r/124747/diff/


Testing
-------

Builds, correctly generates random MACs with last two bits of first octet set to 10, saves said MAC.


Thanks,

Boudhayan Gupta