Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Podman machine wrongfully escapes special characters in proxy env #23277

Closed
feloy opened this issue Jul 15, 2024 · 8 comments · Fixed by #23674
Closed

Podman machine wrongfully escapes special characters in proxy env #23277

feloy opened this issue Jul 15, 2024 · 8 comments · Fixed by #23674
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. machine

Comments

@feloy
Copy link
Contributor

feloy commented Jul 15, 2024

Issue Description

% cat ~/.config/containers/containers.conf

[containers]

[engine]

env = [
	'https_proxy=http://192.168.1.21:3128',
	'http_proxy=http://192.168.1.21:3128',
	'no_proxy=localhost,127.0.0.0/24',
]

Steps to reproduce the issue

Steps to reproduce the issue

  1. Set env variables in config file:
% cat ~/.config/containers/containers.conf

[containers]

[engine]

env = [
	'https_proxy=http://192.168.1.21:3128',
	'http_proxy=http://192.168.1.21:3128',
	'no_proxy=locahost,127.0.0.1/24',
]
  1. create and start podman machine
  2. check settings in VM:
$ podman machine ssh
$ cat /etc/systemd/system.conf.d/default-env.conf 
[Manager]
DefaultEnvironment=http_proxy=http://192.168.1.21:3128
DefaultEnvironment=https_proxy=http://192.168.1.21:3128
DefaultEnvironment=no_proxy=localhost\,127.0.0.0/24

Describe the results you received

The value of the env is escaped: no_proxy=localhost\,127.0.0.0/24

Describe the results you expected

Not escaped value (, instead of \,): no_proxy=localhost,127.0.0.0/24

podman info output

host:
  arch: arm64
  buildahVersion: 1.36.0
  cgroupControllers:
  - cpu
  - io
  - memory
  - pids
  cgroupManager: systemd
  cgroupVersion: v2
  conmon:
    package: conmon-2.1.10-1.fc40.aarch64
    path: /usr/bin/conmon
    version: 'conmon version 2.1.10, commit: '
  cpuUtilization:
    idlePercent: 98.85
    systemPercent: 0.63
    userPercent: 0.52
  cpus: 6
  databaseBackend: sqlite
  distribution:
    distribution: fedora
    variant: coreos
    version: "40"
  eventLogger: journald
  freeLocks: 2048
  hostname: localhost.localdomain
  idMappings:
    gidmap:
    - container_id: 0
      host_id: 1000
      size: 1
    - container_id: 1
      host_id: 100000
      size: 1000000
    uidmap:
    - container_id: 0
      host_id: 501
      size: 1
    - container_id: 1
      host_id: 100000
      size: 1000000
  kernel: 6.8.11-300.fc40.aarch64
  linkmode: dynamic
  logDriver: journald
  memFree: 1507635200
  memTotal: 2044387328
  networkBackend: netavark
  networkBackendInfo:
    backend: netavark
    dns:
      package: aardvark-dns-1.11.0-1.20240628130058229856.main.10.g5ad6420.fc40.aarch64
      path: /usr/libexec/podman/aardvark-dns
      version: aardvark-dns 1.12.0-dev
    package: netavark-1.11.0-1.20240702123536284903.main.32.g49fb0c2.fc40.aarch64
    path: /usr/libexec/podman/netavark
    version: netavark 1.12.0-dev
  ociRuntime:
    name: crun
    package: crun-1.15-1.20240708144150212138.main.51.g6c158dd.fc40.aarch64
    path: /usr/bin/crun
    version: |-
      crun version UNKNOWN
      commit: 54f958d21c4e2299eae6b0f4d8b742304540dce6
      rundir: /run/user/501/crun
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +LIBKRUN +WASM:wasmedge +YAJL
  os: linux
  pasta:
    executable: /usr/bin/pasta
    package: passt-0^20240624.g1ee2eca-1.fc40.aarch64
    version: |
      pasta 0^20240624.g1ee2eca-1.fc40.aarch64-pasta
      Copyright Red Hat
      GNU General Public License, version 2 or later
        <https://www.gnu.org/licenses/old-licenses/gpl-2.0.html>
      This is free software: you are free to change and redistribute it.
      There is NO WARRANTY, to the extent permitted by law.
  remoteSocket:
    exists: true
    path: /run/user/501/podman/podman.sock
  rootlessNetworkCmd: pasta
  security:
    apparmorEnabled: false
    capabilities: CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
    rootless: true
    seccompEnabled: true
    seccompProfilePath: /usr/share/containers/seccomp.json
    selinuxEnabled: true
  serviceIsRemote: false
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: slirp4netns-1.2.2-2.fc40.aarch64
    version: |-
      slirp4netns version 1.2.2
      commit: 0ee2d87523e906518d34a6b423271e4826f71faf
      libslirp: 4.7.0
      SLIRP_CONFIG_VERSION_MAX: 4
      libseccomp: 2.5.5
  swapFree: 0
  swapTotal: 0
  uptime: 0h 1m 20.00s
  variant: v8
plugins:
  authorization: null
  log:
  - k8s-file
  - none
  - passthrough
  - journald
  network:
  - bridge
  - macvlan
  - ipvlan
  volume:
  - local
registries:
  search:
  - docker.io
store:
  configFile: /var/home/core/.config/containers/storage.conf
  containerStore:
    number: 0
    paused: 0
    running: 0
    stopped: 0
  graphDriverName: overlay
  graphOptions: {}
  graphRoot: /var/home/core/.local/share/containers/storage
  graphRootAllocated: 106769133568
  graphRootUsed: 4496261120
  graphStatus:
    Backing Filesystem: xfs
    Native Overlay Diff: "true"
    Supports d_type: "true"
    Supports shifting: "false"
    Supports volatile: "true"
    Using metacopy: "false"
  imageCopyTmpDir: /var/tmp
  imageStore:
    number: 0
  runRoot: /run/user/501/containers
  transientStore: false
  volumePath: /var/home/core/.local/share/containers/storage/volumes
version:
  APIVersion: 5.1.2
  Built: 1720569600
  BuiltTime: Wed Jul 10 02:00:00 2024
  GitCommit: ""
  GoVersion: go1.22.5
  Os: linux
  OsArch: linux/arm64
  Version: 5.1.2

Podman in a container

No

Privileged Or Rootless

Rootless

Upstream Latest Release

Yes

Additional environment details

Additional environment details

Additional information

First reported in podman-desktop/podman-desktop#7894

@TheConen
Copy link

TheConen commented Jul 15, 2024

I think this might happen because of the use of %q here:

envs = append(envs, fmt.Sprintf("%q", key+"="+value))

and/or the use of %%q here:

printf "DefaultEnvironment=%%q\n" "$proxy" >> $SYSTEMD_CONF
printf "%%q\n" "$proxy" >> $ENVD_CONF
printf "export %%q\n" "$proxy" >> $PROFILE_CONF

The fmt documentation describes %q as

%q	a double-quoted string safely escaped with Go syntax

Unfortunately, safely escaping the proxy strings in this case seems to break them, e.g. when NO_PROXY is a comma-separated list or username/password of HTTP(S)_PROXY contain special characters.

I am not very familiar with go and was not yet able to setup a working development environment with podman and test this. Maybe someone with a working environment can have a look at this (@Luap99 possibly?).

@Luap99
Copy link
Member

Luap99 commented Jul 17, 2024

The problem is not the go escaping, printf "%%q\n" uses the shell escape logic, i.e. echo test\,a
If the shell escape logic is not correct for the systemd env we have to skip it there. But this is tricky because some chars must be escaped, in general this whole thing is super ugly.

@Luap99 Luap99 added the machine label Jul 17, 2024
@Luap99 Luap99 changed the title Podman wrongfully escapes special characters in env values Podman machine wrongfully escapes special characters proxy env Jul 17, 2024
@Luap99 Luap99 changed the title Podman machine wrongfully escapes special characters proxy env Podman machine wrongfully escapes special characters in proxy env Jul 17, 2024
@feloy
Copy link
Contributor Author

feloy commented Jul 18, 2024

The problem is not the go escaping, printf "%%q\n" uses the shell escape logic, i.e. echo test\,a If the shell escape logic is not correct for the systemd env we have to skip it there. But this is tricky because some chars must be escaped, in general this whole thing is super ugly.

@Luap99 Could we quote the value, to avoid having to espace it? For example:

key='a b c'

instead of:

key=a\ b\ c

@Luap99
Copy link
Member

Luap99 commented Jul 18, 2024

The problem is not the go escaping, printf "%%q\n" uses the shell escape logic, i.e. echo test\,a If the shell escape logic is not correct for the systemd env we have to skip it there. But this is tricky because some chars must be escaped, in general this whole thing is super ugly.

@Luap99 Could we quote the value, to avoid having to espace it? For example:

key='a b c'

instead of:

key=a\ b\ c

Well that just changes how to escape. If the given input also contains a quote (I know much less likely but still) we still have to take care of it. But yes you are right that might be much easier to do.

@TheConen
Copy link

TheConen commented Jul 18, 2024

I think simply quoting the value with " or ' and mentioning in the documentation (https://podman-desktop.io/docs/proxy) that you could run into issues if you use these characters in your proxy settings (maybe show a hint in the Podman Desktop GUI if someone enters it) is preferable to the current situation.

Right now, Podman is pretty much unusable if you need to have any special characters in your proxy settings - most common case probably being a comma-separated list of domains in NO_PROXY.

How was this handled previously? It was not an issue in Podman 4 - maybe that logic could be reused.

Copy link

A friendly reminder that this issue had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Aug 19, 2024

@feloy interested in opening a PR?

@feloy
Copy link
Contributor Author

feloy commented Aug 19, 2024

@rhatdan Yes, I started to work on a fix today. A PR should come soon

@stale-locking-app stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Nov 25, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Nov 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. machine
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants