Consider adding machine provider prefix to ssh keys

[arixmkii]

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind feature

Description

For machine settings they are stored under provider directory. E.g. for QEMU it will have /qemu/ part.

podman machine inspect | grep .json
               "Path": "~/.config/containers/podman/machine/qemu/podman-machine-default.json"

But ssh keys are named with only machine name. This will become an issue if multiple providers are available for the same platform: like Applehv and QEMU for MacOS or QEMU and WSL2 for Windows.

Even if there will be different binaries/builds to support only single provider per build, this could cause issues when backup restore happens as it might overwrite keys (the data), which actually belongs to another provider and should be isolated.

Adding provider prefix would not make it 100% error prone, but will mitigate at least some cases. And it will also allow to use same named machines for different providers (like calling podman machine init with defaults for either of them w/o chances to break something for another provider).

Example of this collision is described in #13006 (comment)

Preventing such name collision is difficult as every provider will maintain own set of data (list of machines and their states) w/o knowing about other providers.

Steps to reproduce the issue:

1. podman machine init

2. ls ~/.ssh | grep podman

Describe the results you received:

podman-machine-default and podman-machine-default.pub are created.

Describe the results you expected:

qemu-podman-machine-default and qemu-podman-machine-default.pub

Additional information you deem important (e.g. issue happens only occasionally):

Output of podman version:

Built from latest main

Output of podman info:

Skipped.

Package info (e.g. output of rpm -q podman or apt list podman or brew info podman):

Skipped.

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide?

Yes

Additional environment details (AWS, VirtualBox, physical, etc.):

Podman machine on darwin

[github-actions]

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

[rhatdan]

@ashley-cui @cdoern PTAL.

[github-actions]

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

[rhatdan]

@arixmkii Any chance you could open a PR to handle this?

[arixmkii]

@rhatdan I will sketch a solution. At least it would be possible to have a more focused discussion in the PR about cons/pros of this.

But this is lower priority than #17116 and #15852

[github-actions]

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

[rhatdan]

@arixmkii Any time to work on this?

[arixmkii]

@rhatdan I will have some time for OSS projects next week. Will try implementing this.

[rhatdan]

Great

[arixmkii]

Some more updates after a bit of sketching and taking into account #17521 and #17209

First of all at present Podman has 2 different behaviors on how existing key affects the init. In QEMU case it will result in error, but in WSL case it will just use the key. The latter obviously needs an improvement to have the flag, which would mark that preexisting key was used, so, rm command will not delete what was not owned by the machine. I'm yet to check if this is actually the current behavior.

#17521 suggested to store keys within dedicated storage directory of the VM. Being the most simple solution it is probably the most breaking one. Recursively emptying storage tends to be error prone and also introducing that change might be tricky as some of the directory definitions are split between common (and then also other projects) and Podman. It also involves additional ownership/permission handling for the priv/pub keys as they have special requirements comparing to other assets. Cross platform support also contributes to this complexity.

My original proposal doesn't hit the mark either. For example creating machine named "podman" would result in identity files being named "qemu-podman" on QEMU, which could be a username for running the machine as a service and having associated list of identity files.

Keeping in mind a desire to continue using ".ssh" folder there is another take on it. Remembering, that IdentityPath is stored in the machine config anyway and remembering, that it could be read using this command:

podman machine inspect --format {{.SSHConfig.IdentityPath}}

There is an opportunity open to use generated filenames (something similar to free port selector for SSH). Current suggestion is to use "podman-machine-XYZW", where all XYZW, are HEX letters. This reserves a range for 65536 machines per user per host, which is well beyond what average storage could handle (taking into account minimal space required for a machine). 65k step (at most) iteration on init to find an available slot seems acceptable. Range start point could even be randomized (or could be initially set to ssh port number) and then the mod 2^16 would be applied during loop.

To preserve the current behavior as an opt in init command should start accepting IdentityFilename parameter and use it instead of generated one if one is not specified. This would cover needs of those who relied on a predictable IdentityPath for a machine and a corner case of WSL, where one wants to reuse the predefined key.

Follow up improvement would be to make this choice significant and apply to all machine providers. Something like AllowReusingIdentityFiles flag for the init command. This would be also a good moment to extend SSHConfig structure to preserve value if the key was created by init command or existed, so, that rm would know if it should delete the keys as well as other assets.

There are some obvious races for concurent inits or concurrent init and rm on the same host, which could only be mitigated through file locking or using system wide mutex, but the added complexity can't be really justified, because the current init implementation doesn't look like fully concurrent safe either.

[rhatdan]

@arixmkii are you going ahead with this?

[arixmkii]

@rhatdan yes, I plan work on this. There is a working solution for this specific issue in the PR (I might need to rebase, though), but there are comments from Podman team about it and I want to get understanding what would be the acceptable implementation. The discussion in the PR moved towards a larger scope, where other potential collisions are outlined (not only the private and public keys).

[arixmkii]

Should be not an issue anymore with machine 5 refactoring.