Re: [pve-devel] [PATCH container] fix #5194: delete environment variables set by pve

[Folke Gleumes <f.gleumes@proxmox.com>]

On Tue, 2024-01-23 at 10:51 +0100, Fabian Grünbichler wrote:
> On January 22, 2024 11:12 am, Folke Gleumes wrote:
> > proxmox-perl-rs set's SSL_CERT_{DIR,FILE}, which can break ssl in
> > containers if their certificate store can't be found in the same
> > spot.
> > This patch explicitly unsets those variables before starting the
> > container.
> 
> after a short talk with Wolfgang - this patch is probably an okay
> stop-gap to fix the particular regression.
If I understood things correctly, setting the env variables won't be
necessary with the next Debian major release, so I'll add a notice to
remove the workaround with pve 9 [0].
> 
> but it might be nice to switch to `--clear-env` for lxc-attach with
> corresponding options for pct to either preserve the whole env, or
> particular variables? might be 9.0 material since it is a semantic
> change that possibly breaks scripted use cases that rely on env
> variables to pass along things from host to whatever they run inside
> the
> container.. we could introduce the options now though and also have a
> `--keep-env` that is the default for 8.x, and flip it to default to
> `--clear-env` with 9.0.
Seems like a good idea. I also noticed that the lxc-attach man page
currently states "[keep-env] is the current default behaviour  (as  of
version  0.9),  but is is likely to change in the future". By defining
it explicitly, we would be free to decide when to introduce the change.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=4979
> 
> > 
> > Signed-off-by: Folke Gleumes <f.gleumes@proxmox.com>
> > ---
> >  src/PVE/CLI/pct.pm | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/src/PVE/CLI/pct.pm b/src/PVE/CLI/pct.pm
> > index a0b9bce..53519e4 100755
> > --- a/src/PVE/CLI/pct.pm
> > +++ b/src/PVE/CLI/pct.pm
> > @@ -143,6 +143,15 @@ __PACKAGE__->register_method ({
> >         exec(@$cmd);
> >      }});
> >  
> > +sub clean_environment {
> > +    # These env variables are currently needed by PVE to work
> > correctly with rust libraries,
> > +    # but can break ssl inside of containers.
> > +    # An explanation why they are needed and the code that sets
> > them can be found here:
> > +    #
> > https://git.proxmox.com/?p=proxmox-perl-rs.git;a=blob;f=common/pkg/Proxmox/Lib/SslProbe.pm
> > +    delete $ENV{SSL_CERT_FILE};
> > +    delete $ENV{SSL_CERT_DIR};
> > +};
> > +
> >  __PACKAGE__->register_method ({
> >      name => 'enter',
> >      path => 'enter',
> > @@ -164,6 +173,7 @@ __PACKAGE__->register_method ({
> >         PVE::LXC::Config->load_config($vmid); # test if container
> > exists on this node
> >         die "container '$vmid' not running!\n" if
> > !PVE::LXC::check_running($vmid);
> >  
> > +       clean_environment();
> >         exec('lxc-attach', '-n',  $vmid);
> >      }});
> >  
> > @@ -189,6 +199,7 @@ __PACKAGE__->register_method ({
> >  
> >         die "missing command" if !@{$param->{'extra-args'}};
> >  
> > +       clean_environment();
> >         exec('lxc-attach', '-n', $vmid, '--', @{$param->{'extra-
> > args'}});
> >      }});
> >  
> > -- 
> > 2.39.2
> > 
> > 
> > 
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> > 
> > 
> > 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>