diff mbox

Patch for random mac address

Message ID CAFwXZv_b3hdEA4xOFKqAVfc4UZEBX=dL9DOY_mDHExsW+Qg6tQ@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

HacKurx May 24, 2017, 8:44 p.m. UTC
Hi all,

Firstly, I am sad that no major company has taken the trouble to
finance PaX / Grsecurity so they can continue their development in a
way that is accessible to all. This is regrettable because their work
is your main source of inspiration ...

In what brings me here. Brad had released an interesting hack for privacy:
https://www.grsecurity.net/~spender/random_mac.diff

I updated this patch and added a menu option. Can you examine it for
include it upstream?
Because this would be useful for distributions like Tails, Subgraph
OS, Kali Linux and other ...

Thanks. Best regards,

HacKurx (Loic)

Comments

Casey Schaufler May 24, 2017, 10:40 p.m. UTC | #1
On 5/24/2017 1:44 PM, HacKurx wrote:
> Hi all,
>
> Firstly, I am sad that no major company has taken the trouble to
> finance PaX / Grsecurity so they can continue their development in a
> way that is accessible to all. This is regrettable because their work
> is your main source of inspiration ...
>
> In what brings me here. Brad had released an interesting hack for privacy:
> https://www.grsecurity.net/~spender/random_mac.diff
>
> I updated this patch and added a menu option. Can you examine it for
> include it upstream?
> Because this would be useful for distributions like Tails, Subgraph
> OS, Kali Linux and other ...
>
> Thanks. Best regards,
>
> HacKurx (Loic)

Please submit patches inline for review.
Lukas Odzioba May 24, 2017, 11:05 p.m. UTC | #2
2017-05-25 0:40 GMT+02:00 Casey Schaufler <casey@schaufler-ca.com>:
>
> @@ -262,6 +262,10 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
>
>  case SIOCSIFHWADDR:
>  return dev_set_mac_address(dev, &ifr->ifr_hwaddr);
> +#ifdef CONFIG_RANDOM_MAC_ADDRESS
> + /* ignore userland MAC changes */
> + return 0;
> +#endif

I am not sure whether this code does what the comment advertises, but
I bet that Kali-like distro users still would like to be able to
change MAC, also such behavior should be documented in config section.
Thank you for your involvement, please take a look at this article
about preparing kernel patches in general (besides ml addresses most
of it could apply here as well):
https://www.kernel.org/doc/html/latest/process/submitting-patches.html

Thanks,
Lukas
intrigeri May 25, 2017, 7:31 a.m. UTC | #3
Hi,

HacKurx:
> Because this would be useful for distributions like Tails, Subgraph
> OS, Kali Linux and other ...

For what it's worth, it's unlikely that Tails ever uses this unless it
can be controlled at runtime from userspace: we need to give users an
option to disable MAC address randomization, because it breaks network
connectivity in some cases.

Cheers,
Rik van Riel May 25, 2017, 3:07 p.m. UTC | #4
On Thu, 2017-05-25 at 09:31 +0200, intrigeri wrote:
> Hi,
> 
> HacKurx:
> > Because this would be useful for distributions like Tails, Subgraph
> > OS, Kali Linux and other ...
> 
> For what it's worth, it's unlikely that Tails ever uses this unless
> it
> can be controlled at runtime from userspace: we need to give users an
> option to disable MAC address randomization, because it breaks
> network
> connectivity in some cases.

That suggests maybe this kind of functionality should
be implemented in userspace, instead?

Maybe in NetworkManager, or whatever Tails uses to
manage the network. After all, setting the MAC address
to something else is already supported by the kernel...
intrigeri May 25, 2017, 3:47 p.m. UTC | #5
Rik van Riel:
> That suggests maybe this kind of functionality should
> be implemented in userspace, instead?

> Maybe in NetworkManager, […]

It's already implemented in NetworkManager :)

Cheers,
Theodore Ts'o May 25, 2017, 3:48 p.m. UTC | #6
On Thu, May 25, 2017 at 09:31:15AM +0200, intrigeri wrote:
> 
> HacKurx:
> > Because this would be useful for distributions like Tails, Subgraph
> > OS, Kali Linux and other ...
> 
> For what it's worth, it's unlikely that Tails ever uses this unless it
> can be controlled at runtime from userspace: we need to give users an
> option to disable MAC address randomization, because it breaks network
> connectivity in some cases.

BTW, in case people aren't aware ---- you can set the MAC address from
userspace already:

Package: macchanger
Source: macchanger (1.7.0-5.3)
Version: 1.7.0-5.3+b1
Installed-Size: 636
Maintainer: David Paleino <dapal@debian.org>
Architecture: amd64
Depends: libc6 (>= 2.4), debconf (>= 0.5) | debconf-2.0, dpkg (>= 1.15.4) | install-info
Description-en: utility for manipulating the MAC address of network interfaces
 GNU MAC Changer is an utility that makes the maniputation of MAC addresses of
 network interfaces easier.  MAC addresses are unique identifiers on networks,
 they only need to be unique, they can be changed on most network hardware.
 MAC addresses have started to be abused by unscrupulous marketing firms,
 government agencies, and others to provide an easy way to track a computer
 across multiple networks.  By changing the MAC address regularly, this kind
 of tracking can be thwarted, or at least made a lot more difficult.
 .
 Features:
 .
   * set specific MAC address of a network interface
   * set the MAC randomly
   * set a MAC of another vendor
   * set another MAC of the same vendor
   * set a MAC of the same kind (eg: wireless card)
   * display a vendor MAC list (today, 6200 items) to choose from
Description-md5: b3958cf2d904ea6ecdbefc5cd46ec519
Homepage: https://github.com/alobbs/macchanger
Tag: network::configuration, role::program, scope::utility, suite::gnu,
 use::configuring
Section: net
Priority: extra
Filename: pool/main/m/macchanger/macchanger_1.7.0-5.3+b1_amd64.deb
Size: 192768
MD5sum: 35c51e01c0d778961eeda48faf4d884f
SHA256: 00cc5c1e6f2af37023315e5bb1473d3293acfdbf61fb4515c0cb094789d44147
Rik van Riel May 25, 2017, 3:59 p.m. UTC | #7
On Thu, 2017-05-25 at 17:47 +0200, intrigeri wrote:
> Rik van Riel:
> > That suggests maybe this kind of functionality should
> > be implemented in userspace, instead?
> > Maybe in NetworkManager, […]
> 
> It's already implemented in NetworkManager :)

So this kernel patch does not solve any problem,
because the solution has already been implemented
in userspace?
Kees Cook May 25, 2017, 5:28 p.m. UTC | #8
On Thu, May 25, 2017 at 8:59 AM, Rik van Riel <riel@redhat.com> wrote:
> On Thu, 2017-05-25 at 17:47 +0200, intrigeri wrote:
>> Rik van Riel:
>> > That suggests maybe this kind of functionality should
>> > be implemented in userspace, instead?
>> > Maybe in NetworkManager, […]
>>
>> It's already implemented in NetworkManager :)
>
> So this kernel patch does not solve any problem,
> because the solution has already been implemented
> in userspace?

It makes sure you can never not randomize the MAC, no matter what
userspace is doing. I'm not opposed to the idea, but it feels like
overkill to me.

BTW, the proposed patch is slightly wrong: it still allows userspace
to change the MAC address. The ifdef with the return 0 should be moved
up (and "return 0" seems like a bit of a lie: maybe -EINVAL or
-ENOTSUPPORTED?). How about sending a v2 with that fixed, inline, etc.
And see if other people chime in?

It might also be nice to have it be a kernel command line option as
well as a Kconfig, so that distros could include the Kconfig but not
enable it by default (interested users could set the command line
option to enable it).

Thanks!

-Kees
Anisse Astier May 25, 2017, 9:28 p.m. UTC | #9
Hi,

On Thu, May 25, 2017 at 10:28:19AM -0700, Kees Cook wrote:
> On Thu, May 25, 2017 at 8:59 AM, Rik van Riel <riel@redhat.com> wrote:
> > On Thu, 2017-05-25 at 17:47 +0200, intrigeri wrote:
> >> Rik van Riel:
> >> > That suggests maybe this kind of functionality should
> >> > be implemented in userspace, instead?
> >> > Maybe in NetworkManager, […]
> >>
> >> It's already implemented in NetworkManager :)
> >
> > So this kernel patch does not solve any problem,
> > because the solution has already been implemented
> > in userspace?
> 
> It makes sure you can never not randomize the MAC, no matter what
> userspace is doing. I'm not opposed to the idea, but it feels like
> overkill to me.
> 
> BTW, the proposed patch is slightly wrong: it still allows userspace
> to change the MAC address. The ifdef with the return 0 should be moved
> up (and "return 0" seems like a bit of a lie: maybe -EINVAL or
> -ENOTSUPPORTED?). How about sending a v2 with that fixed, inline, etc.
> And see if other people chime in?

Yes, the original grsec patch is slightly different.

> 
> It might also be nice to have it be a kernel command line option as
> well as a Kconfig, so that distros could include the Kconfig but not
> enable it by default (interested users could set the command line
> option to enable it).

Since it's still on the table, there's already a facility in the kernel
to generate a random mac in include/linux/etherdevice.h:
eth_random_addr. It's used by most network drivers when they can't fetch
the hardware address, so that there's still a functionning interface.

I'd be curious to know why this patch does not use it. The generation
looks slightly similar.

Regards,

Anisse Astier
Daniel Micay May 26, 2017, 8:23 a.m. UTC | #10
On Thu, 2017-05-25 at 23:28 +0200, Anisse Astier wrote:
> Hi,
> 
> On Thu, May 25, 2017 at 10:28:19AM -0700, Kees Cook wrote:
> > On Thu, May 25, 2017 at 8:59 AM, Rik van Riel <riel@redhat.com>
> > wrote:
> > > On Thu, 2017-05-25 at 17:47 +0200, intrigeri wrote:
> > > > Rik van Riel:
> > > > > That suggests maybe this kind of functionality should
> > > > > be implemented in userspace, instead?
> > > > > Maybe in NetworkManager, […]
> > > > 
> > > > It's already implemented in NetworkManager :)
> > > 
> > > So this kernel patch does not solve any problem,
> > > because the solution has already been implemented
> > > in userspace?
> > 
> > It makes sure you can never not randomize the MAC, no matter what
> > userspace is doing. I'm not opposed to the idea, but it feels like
> > overkill to me.
> > 
> > BTW, the proposed patch is slightly wrong: it still allows userspace
> > to change the MAC address. The ifdef with the return 0 should be
> > moved
> > up (and "return 0" seems like a bit of a lie: maybe -EINVAL or
> > -ENOTSUPPORTED?). How about sending a v2 with that fixed, inline,
> > etc.
> > And see if other people chime in?
> 
> Yes, the original grsec patch is slightly different.

It was never included in grsecurity, so it's not really a grsecurity
patch.
Matt Brown June 9, 2017, 1:11 p.m. UTC | #11
On 5/25/17 11:48 AM, Theodore Ts'o wrote:
> On Thu, May 25, 2017 at 09:31:15AM +0200, intrigeri wrote:
>>
>> HacKurx:
>>> Because this would be useful for distributions like Tails, Subgraph
>>> OS, Kali Linux and other ...
>>
>> For what it's worth, it's unlikely that Tails ever uses this unless it
>> can be controlled at runtime from userspace: we need to give users an
>> option to disable MAC address randomization, because it breaks network
>> connectivity in some cases.
> 
> BTW, in case people aren't aware ---- you can set the MAC address from
> userspace already:
> 
> Package: macchanger


Yeah I've used this program before. If you want it to always run at boot
you can write a service script for your init system of choice and set it
to run on start up.

In what way does this patch protect you more than a start up script as
described above?

Matt
HacKurx June 10, 2017, 7 a.m. UTC | #12
Le 09/06/2017 à 15:11, Matt Brown a écrit :

> On 5/25/17 11:48 AM, Theodore Ts'o wrote:
>> On Thu, May 25, 2017 at 09:31:15AM +0200, intrigeri wrote:
>>> HacKurx:
>>>> Because this would be useful for distributions like Tails, Subgraph
>>>> OS, Kali Linux and other ...
>>> For what it's worth, it's unlikely that Tails ever uses this unless it
>>> can be controlled at runtime from userspace: we need to give users an
>>> option to disable MAC address randomization, because it breaks network
>>> connectivity in some cases.
>> BTW, in case people aren't aware ---- you can set the MAC address from
>> userspace already:
>>
>> Package: macchanger
>
> Yeah I've used this program before. If you want it to always run at boot
> you can write a service script for your init system of choice and set it
> to run on start up.
>
> In what way does this patch protect you more than a start up script as
> described above?
>
> Matt
Because macchanger use the kernel...
It is loaded too late and increases the risk of the MAC address does not change. See:
https://github.com/alobbs/macchanger/issues

Does your startup script depend on systemd? Who it depends on udev and recommend dbus ...
Is the permanent MAC address stored in the system logs (boot, ipv6, firewall) ?
If a user use journalctl under ubuntu he could see this without sudo ...

For me randomize MAC in a kernel is be the best method to do this.

Loic
diff mbox

Patch

diff --git a/net/core/dev.c b/net/core/dev.c
index fca407b..3eeb42b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6669,6 +6669,26 @@  int dev_change_flags(struct net_device *dev, unsigned int flags)
 
 	changes = (old_flags ^ dev->flags) | (old_gflags ^ dev->gflags);
 	__dev_notify_flags(dev, old_flags, changes);
+
+#ifdef CONFIG_RANDOM_MAC_ADDRESS
+	if ((changes & IFF_UP) && !(old_flags & IFF_UP)) {
+		/* randomize MAC whenever interface is brought up */
+		struct sockaddr sa;
+		unsigned int mac4;
+		unsigned short mac2;
+
+		mac4 = prandom_u32();
+		mac2 = prandom_u32();
+		memcpy(sa.sa_data, &mac4, sizeof(mac4));
+		memcpy((char *)sa.sa_data + sizeof(mac4), &mac2, sizeof(mac2));
+		if (!is_valid_ether_addr(sa.sa_data))
+			sa.sa_data[5] = 1;
+		sa.sa_data[0] &= 0xFC;
+		sa.sa_family = dev->type;
+		dev_set_mac_address(dev, &sa);
+	}
+#endif
+
 	return ret;
 }
 EXPORT_SYMBOL(dev_change_flags);
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index b94b1d2..b020d15 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -262,6 +262,10 @@  static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
 
 	case SIOCSIFHWADDR:
 		return dev_set_mac_address(dev, &ifr->ifr_hwaddr);
+#ifdef CONFIG_RANDOM_MAC_ADDRESS
+		/* ignore userland MAC changes */
+		return 0;
+#endif
 
 	case SIOCSIFHWBROADCAST:
 		if (ifr->ifr_hwaddr.sa_family != dev->type)
diff --git a/security/Kconfig b/security/Kconfig
index 93027fd..6b7b6fc 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -67,6 +67,14 @@  config SECURITY_NETWORK_XFRM
 	  IPSec.
 	  If you are unsure how to answer this question, answer N.
 
+config RANDOM_MAC_ADDRESS
+	bool "Use random MAC adresses"
+	default n
+	help
+	  Say Y here for randomize the MAC addresses of network interfaces.
+	  This option is recommended for people who want to increase their privacy.
+	  If you are unsure how to answer this question, answer N.
+
 config SECURITY_PATH
 	bool "Security hooks for pathname based access control"
 	depends on SECURITY