[pve-devel] [PATCH container] fix #3443: setup: clear /etc/machine-id in post-create hook

[pve-devel] [PATCH container] fix #3443: setup: clear /etc/machine-id in post-create hook

[Oguz Bektas]

this way when new containers are created the will have a unique
/etc/machine-id

note that post_create_hook doesn't run for cloned containers so that
will need to be handled separately


Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 src/PVE/LXC/Setup/Base.pm | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm
index be41874..75a7f74 100644
--- a/src/PVE/LXC/Setup/Base.pm
+++ b/src/PVE/LXC/Setup/Base.pm
@@ -476,6 +476,18 @@ sub set_timezone {
     }
 }
 
+sub clear_machine_id {
+    my ($self, $conf) = @_;
+
+    my $dbus_machine_id_path = "/var/lib/dbus/machine-id";
+    my $machine_id_path = "/etc/machine-id";
+    if ($self->ct_file_exists($dbus_machine_id_path)) {
+        $self->ct_unlink($dbus_machine_id_path);
+    }
+    $self->ct_unlink($machine_id_path);
+    $self->ct_file_set_contents($machine_id_path, "uninitialized\n");
+}
+
 sub pre_start_hook {
     my ($self, $conf) = @_;
 
@@ -491,6 +503,7 @@ sub pre_start_hook {
 sub post_create_hook {
     my ($self, $conf, $root_password, $ssh_keys) = @_;
 
+    $self->clear_machine_id($conf);
     $self->template_fixup($conf);
 
     $self->randomize_crontab($conf);
-- 
2.20.1

Re: [pve-devel] [PATCH container] fix #3443: setup: clear /etc/machine-id in post-create hook

[Thomas Lamprecht]

On 25.05.21 15:17, Oguz Bektas wrote:
> this way when new containers are created the will have a unique
> /etc/machine-id
> 

why the dbus then? systemd talks explicitly only about /etc/machine-id
in the docs and also in their CT interface [0], the thematic is only touched
there though.

The dbus one is most often a symlink to /etc/machine-id, removing that can
break things...

[0] https://systemd.io/CONTAINER_INTERFACE/

> note that post_create_hook doesn't run for cloned containers so that
> will need to be handled separately
> 

If you read my post you also read that we must not remove the file in the
clone case.

We currently always generate a new random MAC-address for all netX devies of
a CT on clone, that suggests that we always want to truncate in the clone case,
to ensure that IPv6 SLAAC, among other things, can work OK.

We could add a "unique" param to the clone call, but until now this was never
requested to be configurable.

[1]: https://forum.proxmox.com/threads/bug-machine-id-etc-machine-id-not-unique-in-lxc-containers.89708/post-392258

> 
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
>  src/PVE/LXC/Setup/Base.pm | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/PVE/LXC/Setup/Base.pm b/src/PVE/LXC/Setup/Base.pm
> index be41874..75a7f74 100644
> --- a/src/PVE/LXC/Setup/Base.pm
> +++ b/src/PVE/LXC/Setup/Base.pm
> @@ -476,6 +476,18 @@ sub set_timezone {
>      }
>  }
>  
> +sub clear_machine_id {
> +    my ($self, $conf) = @_;
> +
> +    my $dbus_machine_id_path = "/var/lib/dbus/machine-id";
> +    my $machine_id_path = "/etc/machine-id";
> +    if ($self->ct_file_exists($dbus_machine_id_path)) {
> +        $self->ct_unlink($dbus_machine_id_path);
> +    }
> +    $self->ct_unlink($machine_id_path);
> +    $self->ct_file_set_contents($machine_id_path, "uninitialized\n");
> +}
> +
>  sub pre_start_hook {
>      my ($self, $conf) = @_;
>  
> @@ -491,6 +503,7 @@ sub pre_start_hook {
>  sub post_create_hook {
>      my ($self, $conf, $root_password, $ssh_keys) = @_;
>  
> +    $self->clear_machine_id($conf);

only relevant for systemd envs. so it should be only called then, if we must call this
in such a general place.

Either called in in the respective distro setup or check if any of "/lib/systemd/systemd"
or "/usr/lib/systemd/system" is executable for a heuristic to find out if the CT is
systemd managed.

>      $self->template_fixup($conf);
>  
>      $self->randomize_crontab($conf);
> 

this now depends on the patch which changed the private randomize_crontab helper
to a plugin method as the change is visible in the context, but that isn't mentioned
anywhere...

Re: [pve-devel] [PATCH container] fix #3443: setup: clear /etc/machine-id in post-create hook

[Oguz Bektas]

hi,

thanks for checking

On Tue, May 25, 2021 at 03:45:53PM +0200, Thomas Lamprecht wrote:
> On 25.05.21 15:17, Oguz Bektas wrote:
> > this way when new containers are created the will have a unique
> > /etc/machine-id
> > 
> 
> why the dbus then? systemd talks explicitly only about /etc/machine-id
> in the docs and also in their CT interface [0], the thematic is only touched
> there though.
> 
> The dbus one is most often a symlink to /etc/machine-id, removing that can
> break things...


in our ubuntu templates it exists as a regular file (not symlinked).

so if that's not removed before first boot, it's copied to /etc/machine-id
from there when the container boots.

`man machine-id` tells me:

       When a machine is booted with systemd(1) the ID of the machine will be established. If systemd.machine_id= or
       --machine-id= options (see first section) are specified, this value will be used. Otherwise, the value in
       /etc/machine-id will be used. If this file is empty or missing, systemd will attempt to use the D-Bus machine
       ID from /var/lib/dbus/machine-id, the value of the kernel command line option container_uuid, the KVM DMI
       product_uuid (on KVM systems), and finally a randomly generated UUID.

so without removing the dbus file we don't get a unique machine-id on container
creation, since the templates seem to have a hardcoded id in the dbus path by
default. we can also remove that but then we will have to make sure to do that
for all the relevant templates

> 
> [0] https://systemd.io/CONTAINER_INTERFACE/
> 
> > note that post_create_hook doesn't run for cloned containers so that
> > will need to be handled separately
> > 
> 
> If you read my post you also read that we must not remove the file in the
> clone case.


yes this hook doesn't run at clone so that's fine.


> 
> We currently always generate a new random MAC-address for all netX devies of
> a CT on clone, that suggests that we always want to truncate in the clone case,
> to ensure that IPv6 SLAAC, among other things, can work OK.
> 
> We could add a "unique" param to the clone call, but until now this was never
> requested to be configurable.

looking at the clone_vm api call i wasn't sure where to modify the file during
clone.

would it make sense to add this as a config option? we could set this to
"uninitialized" in the container config by default. the "unique" param can then
decide if the machine-id would be copied or truncated at clone.

i'm open to ideas

> 
> [1]: https://forum.proxmox.com/threads/bug-machine-id-etc-machine-id-not-unique-in-lxc-containers.89708/post-392258
> 
> > @@ -491,6 +503,7 @@ sub pre_start_hook {
> >  sub post_create_hook {
> >      my ($self, $conf, $root_password, $ssh_keys) = @_;
> >  
> > +    $self->clear_machine_id($conf);
> 
> only relevant for systemd envs. so it should be only called then, if we must call this
> in such a general place.
> 
> Either called in in the respective distro setup or check if any of "/lib/systemd/systemd"
> or "/usr/lib/systemd/system" is executable for a heuristic to find out if the CT is
> systemd managed.

okay that makes sense i'll add that

> 
> >      $self->template_fixup($conf);
> >  
> >      $self->randomize_crontab($conf);
> > 
> 
> this now depends on the patch which changed the private randomize_crontab helper
> to a plugin method as the change is visible in the context, but that isn't mentioned
> anywhere...


guess i'll also remove this part based on your other email

Re: [pve-devel] [PATCH container] fix #3443: setup: clear /etc/machine-id in post-create hook

[Oguz Bektas]

On Wed, May 26, 2021 at 11:40:23AM +0200, Oguz Bektas wrote:
> hi,
> 
> thanks for checking
> 
> On Tue, May 25, 2021 at 03:45:53PM +0200, Thomas Lamprecht wrote:
> > On 25.05.21 15:17, Oguz Bektas wrote:
> > > this way when new containers are created the will have a unique
> > > /etc/machine-id
> > > 
> > 
> > why the dbus then? systemd talks explicitly only about /etc/machine-id
> > in the docs and also in their CT interface [0], the thematic is only touched
> > there though.
> > 
> > The dbus one is most often a symlink to /etc/machine-id, removing that can
> > break things...
> 
> 
> in our ubuntu templates it exists as a regular file (not symlinked).
> 
> so if that's not removed before first boot, it's copied to /etc/machine-id
> from there when the container boots.
> 
 

 also in the debian buster template. but not in the archlinux (there
 it's symlinked)

Re: [pve-devel] [PATCH container] fix #3443: setup: clear /etc/machine-id in post-create hook

[Thomas Lamprecht]

On 26.05.21 11:40, Oguz Bektas wrote:
> so without removing the dbus file we don't get a unique machine-id on container
> creation, since the templates seem to have a hardcoded id in the dbus path by
> default. we can also remove that but then we will have to make sure to do that
> for all the relevant templates

Yes, but you must check if the dbus one is a symlinlk and if that's the case you
must not remove it.

> 
>>
>> [0] https://systemd.io/CONTAINER_INTERFACE/
>>
>>> note that post_create_hook doesn't run for cloned containers so that
>>> will need to be handled separately
>>>
>>
>> If you read my post you also read that we must not remove the file in the
>> clone case.
> 
> 
> yes this hook doesn't run at clone so that's fine.
> 
> 
>>
>> We currently always generate a new random MAC-address for all netX devies of
>> a CT on clone, that suggests that we always want to truncate in the clone case,
>> to ensure that IPv6 SLAAC, among other things, can work OK.
>>
>> We could add a "unique" param to the clone call, but until now this was never
>> requested to be configurable.
> 
> looking at the clone_vm api call i wasn't sure where to modify the file during
> clone.
> 
> would it make sense to add this as a config option? we could set this to
> "uninitialized" in the container config by default. the "unique" param can then
> decide if the machine-id would be copied or truncated at clone.
> 
> i'm open to ideas
> 

no, this would never be a config option as it's a flag for a one time action on
clone, not a permanent configuration relevant for the CT.

The unique flag, if added, would be a basically a copy of the restore "unique"
flag we already have, but it would default to true for the clone case as we
basically have that assumption already (for the MAC regeneration).

But as said, this is optional, the assumption is now already to make relevant
characteristics like MAC unique on clone, so we could default to that.

Re: [pve-devel] [PATCH container] fix #3443: setup: clear /etc/machine-id in post-create hook

[Oguz Bektas]

On Wed, May 26, 2021 at 12:00:13PM +0200, Thomas Lamprecht wrote:
> On 26.05.21 11:40, Oguz Bektas wrote:
> > so without removing the dbus file we don't get a unique machine-id on container
> > creation, since the templates seem to have a hardcoded id in the dbus path by
> > default. we can also remove that but then we will have to make sure to do that
> > for all the relevant templates
> 
> Yes, but you must check if the dbus one is a symlinlk and if that's the case you
> must not remove it.
> 
> > 
> >>
> >> [0] https://systemd.io/CONTAINER_INTERFACE/
> >>
> >>> note that post_create_hook doesn't run for cloned containers so that
> >>> will need to be handled separately
> >>>
> >>
> >> If you read my post you also read that we must not remove the file in the
> >> clone case.
> > 
> > 
> > yes this hook doesn't run at clone so that's fine.
> > 
> > 
> >>
> >> We currently always generate a new random MAC-address for all netX devies of
> >> a CT on clone, that suggests that we always want to truncate in the clone case,
> >> to ensure that IPv6 SLAAC, among other things, can work OK.
> >>
> >> We could add a "unique" param to the clone call, but until now this was never
> >> requested to be configurable.
> > 
> > looking at the clone_vm api call i wasn't sure where to modify the file during
> > clone.
> > 
> > would it make sense to add this as a config option? we could set this to
> > "uninitialized" in the container config by default. the "unique" param can then
> > decide if the machine-id would be copied or truncated at clone.
> > 
> > i'm open to ideas
> > 
> 
> no, this would never be a config option as it's a flag for a one time action on
> clone, not a permanent configuration relevant for the CT.

sorry i meant here the machine-id as a container config option, and that
the "unique" param could decide the action taken at clone

> 
> The unique flag, if added, would be a basically a copy of the restore "unique"
> flag we already have, but it would default to true for the clone case as we
> basically have that assumption already (for the MAC regeneration).
> 
> But as said, this is optional, the assumption is now already to make relevant
> characteristics like MAC unique on clone, so we could default to that.

Re: [pve-devel] [PATCH container] fix #3443: setup: clear /etc/machine-id in post-create hook

[Thomas Lamprecht]

On 26.05.21 12:03, Oguz Bektas wrote:
> On Wed, May 26, 2021 at 12:00:13PM +0200, Thomas Lamprecht wrote:
>> On 26.05.21 11:40, Oguz Bektas wrote:
>>> so without removing the dbus file we don't get a unique machine-id on container
>>> creation, since the templates seem to have a hardcoded id in the dbus path by
>>> default. we can also remove that but then we will have to make sure to do that
>>> for all the relevant templates
>>
>> Yes, but you must check if the dbus one is a symlinlk and if that's the case you
>> must not remove it.
>>
>>>
>>>>
>>>> [0] https://systemd.io/CONTAINER_INTERFACE/
>>>>
>>>>> note that post_create_hook doesn't run for cloned containers so that
>>>>> will need to be handled separately
>>>>>
>>>>
>>>> If you read my post you also read that we must not remove the file in the
>>>> clone case.
>>>
>>>
>>> yes this hook doesn't run at clone so that's fine.
>>>
>>>
>>>>
>>>> We currently always generate a new random MAC-address for all netX devies of
>>>> a CT on clone, that suggests that we always want to truncate in the clone case,
>>>> to ensure that IPv6 SLAAC, among other things, can work OK.
>>>>
>>>> We could add a "unique" param to the clone call, but until now this was never
>>>> requested to be configurable.
>>>
>>> looking at the clone_vm api call i wasn't sure where to modify the file during
>>> clone.
>>>
>>> would it make sense to add this as a config option? we could set this to
>>> "uninitialized" in the container config by default. the "unique" param can then
>>> decide if the machine-id would be copied or truncated at clone.
>>>
>>> i'm open to ideas
>>>
>>
>> no, this would never be a config option as it's a flag for a one time action on
>> clone, not a permanent configuration relevant for the CT.
> 
> sorry i meant here the machine-id as a container config option, and that
> the "unique" param could decide the action taken at clone

The machine-id config is already /etc/machine-id which we have full access to, and
there's no general need to set it to fixed values, can only break things.

The unique flag needs no such thin in the PCT config.