Bug 2217079

Reading https://pkg.go.dev/io/ioutil#TempFile the ioutil.TempFile function is deprecated and os.CreateTemp is now the new thing. https://pkg.go.dev/os#CreateTemp states:

> If dir is the empty string, CreateTemp uses the default directory for temporary files, as returned by TempDir.

https://pkg.go.dev/os#TempDir states:

On Unix systems, it returns $TMPDIR if non-empty, else /tmp.

So that should be used instead of hardcoding /tmp. Then it can be configured using a commonly used environment variable.

What is the expectation? Is it that it should honor the remote_working_dir tunable as described in KCS 2419861 or that one should be able to configure this on each client separately?

From my point of view ( or the case engineers point of view ):

* foreman_ygg_worker/yggdrasil should allow setting a different temporary directory than /tmp ( if CU has hardened their client systems ) [ Perhaps something via the config.toml ]

* At satellite side:
    * remote_working_dir should be honored by pull_mqtt mode 
    * Using the value of remote_working_dir, Something additional should happen to generate and deploy a new config.toml on the client systems and restart yggdrasild on them.

    If this has to be a manual work, we will need to document that in the same KB or somewhere in the docs itself.

> Using the value of remote_working_dir, Something additional should happen to generate and deploy a new config.toml on the client systems and restart yggdrasild on them.

I'd do one or the other. Changing a config file on a capsule and then having to propagate the changes to all the clients, which would also mean the clients would have to modify their own configuration and restart themselves feels overly complicated and fragile. Especially when considering you don't have to do anything else for non-pull rex.

Which means if remote_working_dir should be honored then it would get sent to the client as a parameter of the job and then the client would honor it.

My expectation is to have one way to configure the tmp dir used by the client.

I really like this approach of sending the remote_working_dir as a parameter of the job and the client honoring it.

(In reply to Sayan Das from comment #5)
> * At satellite side:
>     * remote_working_dir should be honored by pull_mqtt mode 
>     * Using the value of remote_working_dir, Something additional should
> happen to generate and deploy a new config.toml on the client systems and
> restart yggdrasild on them.
> 
>     If this has to be a manual work, we will need to document that in the
> same KB or somewhere in the docs itself.

It wouldn't be the smoothest thing, but if pull_mqtt mode honors remote_working_dir, can't you force a re-register of the clients to update the config.toml? I know this isn't an option for everyone, just a thought.


Just to chip in my two cents this is affecting a hardened system I manage. In order to use the pull_mqtt mode currently we actually have to adjust our settings on every system we want to manage. Fixing this would be extremely helpful.

I'm not sure remote_working_dir even makes sense. In SSH context it may make sense but with the ygg worker you have a daemon that runs persistent. I'd prefer that it respects TMPDIR and be hardened using systemd's PrivateTmp=yes. I think https://github.com/theforeman/foreman_ygg_worker/pull/13 would address at least the TMPDIR part. As for hardening I think that needs to happen in https://github.com/RedHatInsights/yggdrasil/blob/main/data/systemd/yggdrasil.service.in but I can't really oversee the implications.

I'm afraid your patch will not really change anything, apart from us not using a now deprecated function. Yggdrasil severely reduces the environment[1] before it kicks of the workers so it won't really matter what you set the TMPDIR env var to for the service as it will not reach the worker.

[1] - https://github.com/RedHatInsights/yggdrasil/blob/0.2.0/cmd/yggd/main.go#L319-L322

Moving back to MODIFIED.

After investigation with @aruzicka it has been found out that PR https://github.com/theforeman/foreman-packaging/pull/9817/
somehow didn't make it to *downstream* yggdrasil-0.2.0-3 package. It is included in the upstream yggdrasil-0.2.0-3 though.

So far, I've verified with foreman_ygg_worker-0.2.1, that '/run' directory is used by default now, so it doesn't fail on hardened systems with noexec /tmp directory.

Waiting for updated yggdrasil package for re-verification, that also a custom directory for task execution can be used via new environment variable FOREMAN_YGG_WORKER_WORKDIR.

(In reply to Pavel Novotny from comment #15)
> Waiting for updated yggdrasil package for re-verification, that also a
> custom directory for task execution can be used via new environment variable
> FOREMAN_YGG_WORKER_WORKDIR.

Is this something that can come out as as it's passed QA/QC? Or does it need to wait until the next full release in 6.14?
This is an issue that affecting us in prod as we harden systems.

Verified in
foreman_ygg_worker-0.2.2-1.el8sat.x86_64
yggdrasil-0.2.3-1.el8sat.x86_64


As mentioned in comment 15, '/run' directory is now the default, so running a remote job on clients with noexec /tmp directory should not fail.

A custom temp directory can be now specified via environment variable(*) FOREMAN_YGG_WORKER_WORKDIR.
(*) environment variable in a systemd service context

Example - using custom temp directory '/tmp_custom':

$ mkdir /tmp_custom
$ mount -t tmpfs tmpfs /tmp_custom
$ echo -e "[Service]\nEnvironment=FOREMAN_YGG_WORKER_WORKDIR=/tmp_custom" > /etc/systemd/system/yggdrasild.service.d/override.conf
$ systemctl daemon-reload
$ systemctl restart yggdrasild

Running a job with `printenv` command returns:
```
   1: OLDPWD=/
   2: FOREMAN_YGG_WORKER_WORKDIR=/tmp_custom
   3: PWD=/root
   4: HOME=/root
   5: SHLVL=1
   6: PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
   7: YGG_SOCKET_ADDR=unix:@yggd-dispatcher-nhpQDR
   8: _=/usr/bin/printenv
   9: Exit status: 0
```

Ensuring that the client indeed uses the custom directory, re-mounting /tmp_custom with *noexec* and running the job again now returns error:

```
   1: /bin/sh: /tmp_custom/ygg_rex2042744520: Permission denied
   2: /bin/sh: line 0: exec: /tmp_custom/ygg_rex2042744520: cannot execute: Permission denied
   3: Exit status: 126
   4: StandardError: Job execution failed
```

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Important: Red Hat Satellite Client security and bug fix update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2023:5982