Skip to content

Expose https_proxy env variable to ssh-import-id cmd#1333

Merged
TheRealFalcon merged 2 commits into
canonical:mainfrom
michaelrommel:expose_proxy_sshimportid
Apr 4, 2022
Merged

Expose https_proxy env variable to ssh-import-id cmd#1333
TheRealFalcon merged 2 commits into
canonical:mainfrom
michaelrommel:expose_proxy_sshimportid

Conversation

@michaelrommel
Copy link
Copy Markdown
Contributor

The import of ssh keys for users does not work in corporate networks
behind proxies.

The ssh-import-id command does not support cmd line arguments or other
configurations for specifying a proxy. The sudo command in the module
executes the ssh-import-id command directly, so any proxy settings of
shells will not work. The only option would be either to specify the
proxy setting on the command line, but this wouldn't be easily user
editable.

So the only solution is to selectively forward any already set
https_proxy variable to the executed command. Since only one env
variable is used and forwarded, the security impact should be
acceptable.

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

The import of ssh keys for users does not work in corporate networks
behind proxies.

The ssh-import-id command does not support cmd line arguments or other
configurations for specifying a proxy. The sudo command in the module
executes the ssh-import-id command directly, so any proxy settings of
shells will not work. The only option would be either to specify the
proxy setting on the command line, but this wouldn't be easily user
editable.

So the only solution is to selectively forward any already set
https_proxy variable to the executed command. Since only one env
variable is used and forwarded, the security impact should be
acceptable.
@TheRealFalcon
Copy link
Copy Markdown
Contributor

@michaelrommel I don't completely understand how this would work. Can you provide some more context? Where would the https_proxy env var come from? Who would set it and where?

@github-actions
Copy link
Copy Markdown

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.)

@github-actions github-actions Bot added the stale-pr Pull request is stale; will be auto-closed soon label Mar 30, 2022
@michaelrommel
Copy link
Copy Markdown
Contributor Author

michaelrommel commented Apr 1, 2022

Hello @TheRealFalcon,

I have a user-data file that includes a bootcmd section as follows:

bootcmd:
 - |
   cloud-init-per once env sh -c "mkdir -p /etc/systemd/system/cloud-config.service.d &&
   mkdir -p /etc/systemd/system/cloud-final.service.d && { cat > /etc/cloud/env <<-EOF
   http_proxy=http://192.168.10.212:3128/
   https_proxy=http://192.168.10.212:3128/
   EOF
   } && { cat > /etc/systemd/system/cloud-config.service.d/override.conf <<-EOF
   [Service]
   EnvironmentFile=/etc/cloud/env
   PassEnvironment=https_proxy http_proxy
   EOF
   } && { cat > /etc/systemd/system/cloud-final.service.d/override.conf <<-EOF
   [Service]
   EnvironmentFile=/etc/cloud/env
   PassEnvironment=https_proxy http_proxy
   EOF
   } && systemctl daemon-reload"

This leads to the required environment variables for that particular environment being exported as part of the two standard cloud-init systemd services. These could then be passed on to the import-ssh-id command as I described above.

The creation of those files could also happen at some other place in the user-data file.

Hope that helps.

Maybe one further comment: how can one actually check, whether a intended change works? I could only track that down to commands that I can change, but the proposed change in the PR is untested. Because even if I could compile cloud-init myself, how would I inject that into a standard debian preseeded installation? That only ever pulls the original unmodified version from the debian repos. I could only verify what would be needed on the command line, but I am unsure if the modification should look like:

cmd = ["sudo", "--preserve-env=https_proxy -Hu", user, "ssh-import-id"] + ids

or rather

cmd = ["sudo", "--preserve-env=https_proxy", "-Hu", user, "ssh-import-id"] + ids

so if it needs to be split into two array members?

@TheRealFalcon TheRealFalcon removed the stale-pr Pull request is stale; will be auto-closed soon label Apr 1, 2022
Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @michaelrommel for the explanation. This is a valid use case, though I think we should probably handle it in a more holistic manner. Given that that may be a fair bit of work, I'm using your solution, but also pushed a big TODO comment for us in the future.

Assuming CI passes, I'll go ahead and merge it.

@TheRealFalcon TheRealFalcon merged commit bb93952 into canonical:main Apr 4, 2022
@michaelrommel michaelrommel deleted the expose_proxy_sshimportid branch April 5, 2022 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants