After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 691885 - [review:duid] DHCPv6 DUID permanence
[review:duid] DHCPv6 DUID permanence
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-01-16 18:34 UTC by Dan Williams
Modified: 2013-02-19 16:14 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Dan Williams 2013-01-16 18:34:29 UTC
DHCPv6 uses a DUID (DHCP Unique Identifier) as part of the DHCP exchanges, and that should stay the same for a given machine.  That typically gets stored in the leasefile.  But currently we don't read it, and we don't write it, so it may change every time.  Instead, find an existing DUID if we have one and use that for all DHCPv6 exchanges.  If we don't have one, generate one and write it to persistent storage so we can find it again next time.
Comment 1 Dan Winship 2013-01-21 20:15:49 UTC
Do we really want a global default DUID as opposed to only per-lease ones? Mumble mumble privacy concerns and all that?



In nm_dhcp_dhclient_escape_duid(), you could replace

		if (!isprint (*s) || !isascii (*s)) {

with

		if (!g_ascii_isprint (*s)) {

(although what you have should work, unless there are locales that consider some ASCII characters to be non-printable...)



+generate_duid (const GByteArray *hwaddr)

+	/* ARPHRD_ETHER or ARPHRD_INFINIBAND */
+	g_assert (arptype == 1 || arptype == 32);

any reason you don't use the constants?



+save_duid_to_leasefile (guint log_domain,

+		/* If the file already contains a DUID, leave it */
+		if (contents && strstr (contents, "default-duid")) {

That would trigger on commented-out default-duid lines too, etc. Not sure if that matters.


+	g_string_append_printf (s, "default-duid \"%s\";\n", escaped);
...
+			g_string_append (s, *iter);
+			g_string_append_c (s, '\r');

Trying to avoid picking favorites between line terminators? :)
Comment 2 Dan Williams 2013-01-24 21:22:25 UTC
(In reply to comment #1)
> Do we really want a global default DUID as opposed to only per-lease ones?
> Mumble mumble privacy concerns and all that?

Yeah: http://infonomics-society.org/IJISR/Privacy%20and%20Security%20of%20DHCP%20Unique%20Identifiers.pdf

However, the standards:

http://tools.ietf.org/html/rfc3315

"A DHCP Unique IDentifier for a DHCP participant; each DHCP client and server has exactly one DUID.  See section 9 for details of the ways in which a DUID may be constructed."

"The DUID is designed to be unique across all DHCP clients and servers, and stable for any specific client or server - that is, the DUID used by a client or server SHOULD NOT change over time if at all possible; for example, a device's DUID should not change as a result of a change in the device's network hardware."

http://tools.ietf.org/html/rfc6355

"DUIDs are intended to remain constant over time, so that they can be used as permanent identifiers for a device.  In the case of DUID-LLTs, they are intended to be generated once, stored in stable storage, and reused from that point forward."

So the standards are pretty clear.  But the privacy disclosures the paper talks about should apply to MAC addresses too, as almost no-one randomizes the MAC address on their machine every time they connect to a different network (wouldn't that be a cool NM plugin?).

So in short, the standards require something, and the downsides pretty much already exist, AFAICT.
Comment 3 Dan Williams 2013-01-24 23:00:52 UTC
Fixes pushed to branch, along with more testcases.  Re-review please!
Comment 4 Pavel Simerda 2013-01-25 00:58:23 UTC
(In reply to comment #1)
> Do we really want a global default DUID as opposed to only per-lease ones?
> Mumble mumble privacy concerns and all that?

Privacy? Do we want to re-generate MAC addresses, too? :)
Comment 5 Dan Winship 2013-01-25 14:47:25 UTC
(In reply to comment #4)
> (In reply to comment #1)
> > Do we really want a global default DUID as opposed to only per-lease ones?
> > Mumble mumble privacy concerns and all that?
> 
> Privacy? Do we want to re-generate MAC addresses, too? :)

Hence the vagueness of my question; I hadn't actually thought about it in detail, I just wanted to make sure *someone* had. :)
Comment 6 Dan Winship 2013-01-25 15:02:41 UTC
>+test_one_duid (const char *escaped, const guint8 *unescaped, guint len)

>+	g_assert_cmpint (strlen (escaped), ==, strlen (w));
>+	g_assert (strcmp (escaped, w) == 0);

You can just do: g_assert_cmpstr (escaped, ==, w);


>+save_duid_to_leasefile (guint log_domain,

>+		for (iter = lines; iter && *iter; iter++) {
>+			l = *iter;
>+			if (l[0] == '#' || !l[0])
>+				continue;
>+
>+			while (isblank (*l))
>+				l++;
>+			if (g_str_has_prefix (l, DUID_PREFIX)) {
>+				g_strfreev (lines);
>+				return TRUE;
>+			}
>+		}

You don't need the explicit comment/empty-line check. The other lines will rule out those cases anyway.

(Also, I'd use g_ascii_isspace(), not isblank()... the ctype functions should generally just be assumed to be evil and wrong :).

>+			/* dhclient uses only \n as the line ending */

that's not really comment-worthy; this is unix.



>dhclient: add testcases for reading DUIDs from leasefiles

> check-local: test-dhcp-dhclient
>-	$(abs_builddir)/test-dhcp-dhclient
>+	$(abs_builddir)/test-dhcp-dhclient $(abs_srcdir)/

I'd add -DSRCDIR=\""$(abs_srcdir)"\"" to test_dhcp_dhclient_CPPFLAGS, and then use SRCDIR rather than argv[1] from the code. That makes it easier to run the test by hand (or, eg, under gdb), because you don't have to worry about passing the right argument to it.
Comment 7 Pavel Simerda 2013-01-25 15:24:28 UTC
(In reply to comment #5)
> Hence the vagueness of my question; I hadn't actually thought about it in
> detail, I just wanted to make sure *someone* had. :)

Yeah. DUID is generally not considered sensitive. But it is expected to be as stable as possible.

Another sidenote I didn't add yesterday because of bugzilla outage...

On-disk DUID-LLT is pretty good in being stable over one installation of a system, but not over one piece of hardware.

If we want maximum stability between DUID and hadrware, we should use DUID-LL and we should *not* use a disk file to cache it over restarts.

Using DUID-LLT causes a serious regression with respect to both cloning software installations and network boot. It has been discussed a hundred times on various IPv6-related conferences and training courses.

I don't say we must necessarily fix it now. I just want to warn that such a problem exists and that it is considered serious by some (many?) system administrators.

On IRC, people were speking about the same behavior in Windows, but it is criticised there too. This is one of the rare times we can be better than Windows.
Comment 8 Dan Williams 2013-01-28 20:16:50 UTC
(In reply to comment #7)
> (In reply to comment #5)
> > Hence the vagueness of my question; I hadn't actually thought about it in
> > detail, I just wanted to make sure *someone* had. :)
> 
> Yeah. DUID is generally not considered sensitive. But it is expected to be as
> stable as possible.
> 
> Another sidenote I didn't add yesterday because of bugzilla outage...
> 
> On-disk DUID-LLT is pretty good in being stable over one installation of a
> system, but not over one piece of hardware.
> 
> If we want maximum stability between DUID and hadrware, we should use DUID-LL
> and we should *not* use a disk file to cache it over restarts.

The problem with DUID-LL is that the standards say:

"The DUID is designed to be unique across all DHCP clients and servers, and
stable for any specific client or server - that is, the DUID used by a client
or server SHOULD NOT change over time if at all possible; for example, a
device's DUID should not change as a result of a change in the device's network
hardware."

http://tools.ietf.org/html/rfc6355

which means that if we don't store the DUID we generate, and something on the system changes, like a network card, or the user adds another network card, how do we know which interface to regenerate the DUID from?  We don't, or at least we can't know which one we used last time.  So we can't assure that the DUID "should not change as a result of a change in the device's network hardware."

I don't really care if we use DUID-LL or DUID-LLT, but if we don't store the result, then we can't assure that it's stable as required by the standards.
Comment 9 Dan Williams 2013-01-28 20:52:00 UTC
Fixed up from comment 6 review and re-pushed.  Need another review or is it OK to merge?
Comment 10 Dan Winship 2013-01-28 21:37:27 UTC
ok to merge
Comment 11 Pavel Simerda 2013-01-29 10:08:49 UTC
(In reply to comment #8)
> The problem with DUID-LL is that the standards say:
> 
> "The DUID is designed to be unique across all DHCP clients and servers,

Correct.

> and stable for any specific client or server - that is, the DUID used by a client
> or server SHOULD NOT change over time if at all possible

Yeah, this is why I think DUID-LL should be preferred when globally unique MAC address is used.

> do we know which interface to regenerate the DUID from?

Yes, we do. We should use the first built-in interface in that case.

> I don't really care if we use DUID-LL or DUID-LLT, but if we don't store the
> result, then we can't assure that it's stable as required by the standards.

As above, if we use the same way each time, it is as stable as possible. An the problem with on-disk DUID is already well known.

We should be able to throw away the DUID upon system cloning, no matter how we do that. I currently see two possibilites:

1) When the system has at least one non-removable interface, take the first one's globally assinged MAC address and use it as DUID-LL. Don't store it.

2) Use DUID-LL when encountering a globally assigned MAC address. Store it on disk. Invalidate it at start when the interface is not found.

I recommend merging the current code (if it hasn't been done yet) and keeping this bug report open.
Comment 12 Dan Williams 2013-01-29 15:54:19 UTC
(In reply to comment #11)
> (In reply to comment #8)
> > The problem with DUID-LL is that the standards say:
> > 
> > "The DUID is designed to be unique across all DHCP clients and servers,
> 
> Correct.
> 
> > and stable for any specific client or server - that is, the DUID used by a client
> > or server SHOULD NOT change over time if at all possible
> 
> Yeah, this is why I think DUID-LL should be preferred when globally unique MAC
> address is used.
> 
> > do we know which interface to regenerate the DUID from?
> 
> Yes, we do. We should use the first built-in interface in that case.

But we have no idea which interface is the "first built-in interface".  There's simply no good way to determine that, because of interface re-ordering and hotplug enumeration.  Yes, we could get the same interface 90% of the time on a laptop with only one interface, but as soon as you plug in a USB ethernet device or a WWAN device (which have randomized addresses) then it doesn't work.

And it clearly doesn't work on servers where you may have multiple interfaces, and where it's much more likely that interfaces could be swapped out.  If we tried this, we'd continually be adding special-cases and workarounds and we'd still fail.

If you want DUID-LL, that's fine, but once we generate it, we need to store it somewhere.

> > I don't really care if we use DUID-LL or DUID-LLT, but if we don't store the
> > result, then we can't assure that it's stable as required by the standards.
> 
> As above, if we use the same way each time, it is as stable as possible. An the
> problem with on-disk DUID is already well known.

The problem is that we can't use it the same way each time, because we can't reliably determine what the first network interface is.

> We should be able to throw away the DUID upon system cloning, no matter how we
> do that. I currently see two possibilites:
> 
> 1) When the system has at least one non-removable interface, take the first
> one's globally assinged MAC address and use it as DUID-LL. Don't store it.

The next problem is, we can't reliably determine whether it's non-removable or not with 95% certainty.  Sometimes internal devices are actually USB connected, sometimes they are external.

> 2) Use DUID-LL when encountering a globally assigned MAC address. Store it on
> disk. Invalidate it at start when the interface is not found.
> 
> I recommend merging the current code (if it hasn't been done yet) and keeping
> this bug report open.

I'd rather have a new bug with this title:

"Generate new DUID when cloning a system"

That's the real problem, as I understand it.  It doesn't matter what we do with the DUID as long as when we clone a system, we don't use the same DUID as the old system.  Right?

That's a solvable problem.  Perhaps we save the DUID/machineID pair and if the machineID doesn't match we generate a new DUID.  Or something like that.  But I think it should be a new bug.
Comment 13 Dan Winship 2013-01-29 16:43:32 UTC
(In reply to comment #12)
> That's a solvable problem.  Perhaps we save the DUID/machineID pair and if the
> machineID doesn't match we generate a new DUID.  Or something like that.  But I
> think it should be a new bug.

Actually, can't we just generate the DUID programmatically from the machine-id? machine-id basically defines identity for a machine, so if we're on the "same machine", we have the same machine-id we did last time, so we'll end up with the same DUID, and if we have a different machine-id than we did last time then we're on a "different machine" and so should have a different DUID too.
Comment 14 Dan Williams 2013-01-29 17:23:01 UTC
merged
Comment 15 Dan Williams 2013-01-29 17:28:41 UTC
(In reply to comment #13)
> (In reply to comment #12)
> > That's a solvable problem.  Perhaps we save the DUID/machineID pair and if the
> > machineID doesn't match we generate a new DUID.  Or something like that.  But I
> > think it should be a new bug.
> 
> Actually, can't we just generate the DUID programmatically from the machine-id?
> machine-id basically defines identity for a machine, so if we're on the "same
> machine", we have the same machine-id we did last time, so we'll end up with
> the same DUID, and if we have a different machine-id than we did last time then
> we're on a "different machine" and so should have a different DUID too.

We could, though it would be a different mechanism that's not described by the standards.  I dont' think that's a big issue though, since AIUI the standards don't mandate a specific mechanism to generate the DUID, only suggestions.  They really only care about the behavior that I've stated above not how you get there.

I'd be fine with deriving the DUID from the machine-id, and maybe falling back to a MAC address if we don't have a machine-id for some reason.  Given that the machine-id already has the cloning problem, if it got fixed for machine-id then it would be fixed for duid.  That said, I think we still need to support using an administrator-defined DUID which might already exist in the config files.

So in this scenario, if we don't find a DUID anywhere, we generate one from the machine-id and we *don't* save it.  If we do find a DUID somewhere, then it's already stored on-disk and we use that DUID and we don't save it anywhere.  That way new installs would avoid the cloning problem, and we'd work correctly with existing installs by using the already-saved DUID.

What remains to be determined here is if this approach would be contrary to any of the standards, and I haven't read them as much as cyphermox and pavlix have.  So I'll defer to them on that determination.
Comment 16 Pavel Simerda 2013-01-30 17:39:46 UTC
(In reply to comment #15)
> So in this scenario, if we don't find a DUID anywhere, we generate one from the
> machine-id and we *don't* save it.

+1 for using machine-id and not saving it, with the following reference:

http://tools.ietf.org/html/rfc6355#section-8.1
Comment 17 Dan Williams 2013-02-19 16:14:51 UTC
It's been merged.