Repository: containers/podman
Pull request #21125 (2024-01-02 14:41:51)

machine: Use a single ssh key for all machines
Changes SSH key behavior such that there is a single persisted key for all
machines across all providers. If there is no key that is located at
`.local/share/containers/podman/machine/` then it is created. The keys are
not deleted when the last machine on the host is removed.

The main motivation for this change is it leads to fewer files created on the
host as a result of vm configuration. Having `n` machines on your system doesn't
result in `2n` machine-related files in `.ssh` on your system anymore.

As a result of ssh keys being persisted by default, the `--save-keys` flag on `podman machine rm` will no longer be supported.

Also fixes a bug where if a machine failed during init due to a bad ignition path,
it would not be properly torn down.

<!--
Thanks for sending a pull request!

Please make sure you've read our contributing guidelines and how to submit a pull request (https://github.com/containers/podman/blob/main/CONTRIBUTING.md#submitting-pull-requests).

In case you're only changing docs, make sure to prefix the pull-request title with "[CI:DOCS]". That will prevent functional tests from running and save time and energy.

Finally, be sure to sign commits with your real name. Since by opening
a PR you already have commits, you can add signatures if needed with
something like `git commit -s --amend`.
-->

#### Does this PR introduce a user-facing change?

<!--
If no, just write `None` in the release-note block below. If yes, a release note
is required: Enter your extended release note in the block below. If the PR
requires additional action from users switching to the new release, include the
string "action required".

For more information on release notes, please follow the Kubernetes model:
https://git.k8s.io/community/contributors/guide/release-notes.md
-->

```release-note
All future machines created on the system will utilize the same persisted ssh key across all providers. The key will be located at `.local/share/containers/podman/machine/machine`. If the ssh key does not exist during machine initialization, it will be created. It is important to note when a machine is being removed, it will no longer remove the ssh keys. As a result, the `--save-keys` flag on `podman machine rm` will no longer be supported since the ssh keys are being persisted by default. If the user would like to override the default ssh keys with their own, they must place their key at `.local/share/containers/podman/machine/` and name it `machine`. 
```


openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/containers/podman/pull/21125#" title="Author self-approved">jakecorrenti</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign [tomsweeneyredhat](https://github.com/tomsweeneyredhat) for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=containers%2Fpodman).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/containers/podman/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["tomsweeneyredhat"]} -->
packit-as-a-service[bot]: Ephemeral COPR build failed. @containers/packit-build please check.
cgwalters: Can you add more information as to *why* to make this change?  I can guess but it'd be good to write down explicitly.


packit-as-a-service[bot]: Cockpit tests failed for commit df4cc3b48eea2cdbefc76db124f90106493efa9f. @martinpitt, @jelly, @mvollmer please check.
jakecorrenti: > Can you add more information as to _why_ to make this change? I can guess but it'd be good to write down explicitly.

Does this sound ok?

> The main motivation for this change is it leads to fewer files created on the host as a result of vm configuration. Having n machines on your system doesn't result in 2n machine-related files in ~/.ssh/ on your system anymore, just 2.
cgwalters: Yes, that makes sense.  That said, another possible change here is to store the key in `~/.config/containers/podman/machine` instead, and change `machine/ssh.go` to explicitly pass it via `ssh -i ~/.config/containers/podman/machine/ssh.key -o IdentitiesOnly=true` (the latter disables other keys which we want here too).
jakecorrenti: I can give that a shot
cgwalters: Thinking about this a bit more, it's not user configuration actually so `~/.local/share/containers/podman/machine` is probably better.
baude: i like the recommended filepath as well.  
cgwalters: Just noting these are orthogonal, but also complementary changes.  If we moved the existing ssh key*s* (i.e. multiple) into a private podman place, we'd at least avoid polluting the user's other "normal" ssh keys.  

But clearly it's better to do both: use a private path, and only use one key.  

packit-as-a-service[bot]: Ephemeral COPR build failed. @containers/packit-build please check.
jakecorrenti: Temporarily turned to a draft to take advantage of `[CI:MACHINE]` with the hope that CI moves a bit quicker

Patch:
diff --git a/cmd/podman/machine/rm.go b/cmd/podman/machine/rm.go
index ca1128e5d4..d25922be11 100644
--- a/cmd/podman/machine/rm.go
+++ b/cmd/podman/machine/rm.go
@@ -41,9 +41,6 @@ func init() {
 	formatFlagName := "force"
 	flags.BoolVarP(&destroyOptions.Force, formatFlagName, "f", false, "Stop and do not prompt before rming")
 
-	keysFlagName := "save-keys"
-	flags.BoolVar(&destroyOptions.SaveKeys, keysFlagName, false, "Do not delete SSH keys")
-
 	ignitionFlagName := "save-ignition"
 	flags.BoolVar(&destroyOptions.SaveIgnition, ignitionFlagName, false, "Do not delete ignition file")
 
diff --git a/docs/source/locale/ja/LC_MESSAGES/markdown.po b/docs/source/locale/ja/LC_MESSAGES/markdown.po
index d625c9c3e5..daba666e6d 100644
--- a/docs/source/locale/ja/LC_MESSAGES/markdown.po
+++ b/docs/source/locale/ja/LC_MESSAGES/markdown.po
@@ -18478,10 +18478,6 @@ msgstr ""
 msgid "Do not delete the VM image."
 msgstr ""
 
-#: ../../source/markdown/podman-machine-rm.1.md:43
-msgid "**--save-keys**"
-msgstr ""
-
 #: ../../source/markdown/podman-machine-rm.1.md:45
 msgid ""
 "Do not delete the SSH keys for the VM.  The system connection is always "
diff --git a/docs/source/markdown/podman-machine-rm.1.md b/docs/source/markdown/podman-machine-rm.1.md
index 1224adbe1d..9aa90c35ad 100644
--- a/docs/source/markdown/podman-machine-rm.1.md
+++ b/docs/source/markdown/podman-machine-rm.1.md
@@ -10,7 +10,7 @@ podman\-machine\-rm - Remove a virtual machine
 
 Remove a virtual machine and its related files.  What is actually deleted
 depends on the virtual machine type.  For all virtual machines, the generated
-SSH keys and the podman system connection are deleted.  The ignition files
+podman system connections are deleted.  The ignition files
 generated for that VM are also removed as is its image file on the filesystem.
 
 Users get a display of what is deleted and are required to confirm unless the option `--force`
@@ -39,11 +39,6 @@ Do not delete the generated ignition file.
 
 Do not delete the VM image.
 
-#### **--save-keys**
-
-Do not delete the SSH keys for the VM.  The system connection is always
-deleted.
-
 ## EXAMPLES
 
 Remove a VM named "test1":
@@ -53,8 +48,6 @@ $ podman machine rm test1
 
 The following files will be deleted:
 
-/home/user/.ssh/test1
-/home/user/.ssh/test1.pub
 /home/user/.config/containers/podman/machine/qemu/test1.ign
 /home/user/.local/share/containers/podman/machine/qemu/test1_fedora-coreos-33.20210315.1.0-qemu.x86_64.qcow2
 /home/user/.config/containers/podman/machine/qemu/test1.json
diff --git a/hack/xref-helpmsgs-manpages b/hack/xref-helpmsgs-manpages
index 7230057208..575f3daecc 100755
--- a/hack/xref-helpmsgs-manpages
+++ b/hack/xref-helpmsgs-manpages
@@ -9,7 +9,6 @@ use utf8;
 
 use strict;
 use warnings;
-use Clone                       qw(clone);
 use FindBin;
 
 (our $ME = $0) =~ s|.*/||;
@@ -461,25 +460,12 @@ sub podman_help {
 ################
 #  podman_man  #  Parse contents of podman-*.1.md
 ################
-our %Man_Seen;
 sub podman_man {
     my $command = shift;
     my $subpath = "$Markdown_Path/$command.1.md";
     print "** $subpath \n"                              if $debug;
 
     my %man = (_path => $subpath);
-
-    # We often get called multiple times on the same man page,
-    # because (e.g.) podman-container-list == podman-ps. It's the
-    # same man page text, though, and we don't know which subcommand
-    # we're being called for, so there's nothing to be gained by
-    # rereading the man page or by dumping yet more warnings
-    # at the user. So, keep a cache of what we've done.
-    if (my $seen = $Man_Seen{$subpath}) {
-        return clone($seen);
-    }
-    $Man_Seen{$subpath} = \%man;
-
     open my $fh, '<', $subpath
         or die "$ME: Cannot read $subpath: $!\n";
     my $section = '';
@@ -529,7 +515,7 @@ sub podman_man {
             if ($line =~ /^\|\s*\[podman-(\S+?)\(\d\)\]/) {
                 # $1 will be changed by recursion _*BEFORE*_ left-hand assignment
                 my $subcmd = $1;
-                $man{$subcmd} = podman_man("podman-$subcmd");
+                $man{$subcmd} = podman_man("podman-$1");
             }
 
             # In podman-<subcommand>.1.md
diff --git a/hack/xref-helpmsgs-manpages.t b/hack/xref-helpmsgs-manpages.t
index ae2587033f..e85d598316 100644
--- a/hack/xref-helpmsgs-manpages.t
+++ b/hack/xref-helpmsgs-manpages.t
@@ -15,7 +15,7 @@ use FindBin;
 use Test::More;
 use Test::Differences;
 
-plan tests => 10;
+plan tests => 9;
 
 require_ok "$FindBin::Bin/xref-helpmsgs-manpages";
 
@@ -40,10 +40,9 @@ my @warnings_seen;
 $SIG{__WARN__} = sub {
     my $msg = shift;
     chomp $msg;
-    $msg =~ s/^xref-\S+?:\s+//;         # strip "$ME: "
-    $msg =~ s!(^|\s+)$doc_path/!$1!g;   # strip "doc/source/markdown"
-    $msg =~ s!:\d+:!:NNN:!;             # file line numbers can change
-   push @warnings_seen, $msg;
+    $msg =~ s/^xref-\S+?:\s+//;
+    $msg =~ s|\s+$doc_path/| |g;
+    push @warnings_seen, $msg;
 };
 
 # When we get errors (hopefully only when adding new functionality
@@ -158,221 +157,10 @@ test_xref("xref_by_help() injection", $hclone, $mclone,
 # BEGIN fault injection tests on podman_man()
 #
 # This function has a ton of sanity checks. To test them we need to
-# perform minor surgery on lots of .md files: reordering lines,
-# adding inconsistencies.
+# perform minor surgery on lots of .md files.
 #
+# FIXME: TBD. This PR has grown big enough as it is.
 
-# Ordered list of the warnings we expect to see
-my @expect_warnings;
-
-# Helper function: given a filename and a function, reads filename
-# line by line, invoking filter on each line and writing out the
-# results.
-sub sed {
-    my $path   = shift;                 # in: filename (something.md)
-    my $action = shift;                 # in: filter function
-
-    # The rest of our arguments are the warnings introduced into this man page
-    push @expect_warnings, @_;
-
-    open my $fh_in, '<', "$doc_path/$path"
-        or die "Cannot read $doc_path/$path: $!";
-    my $tmpfile = "$doc_path/$path.tmp.$$";
-    open my $fh_out, '>', $tmpfile
-        or die "Cannot create $doc_path/$tmpfile: $!";
-
-    while (my $line = <$fh_in>) {
-        # This is what does all the magic
-        print { $fh_out } $action->($line);
-    }
-    close $fh_in;
-    close $fh_out
-        or die "Error writing $doc_path/$tmpfile: $!";
-    rename "$tmpfile" => "$doc_path/$path"
-        or die "Could not rename $doc_path/$tmpfile: $!";
-}
-
-# Start filtering.
-
-# podman-attach is a deliberate choice here: it also serves as the man page
-# for podman-container-attach. Prior to 2023-12-20 we would read the file
-# twice, issuing two warnings, which is anti-helpful. Here we confirm that
-# the dup-removing code works.
-sed('podman-attach.1.md', sub {
-        my $line = shift;
-        $line =~ s/^(%\s+podman)-(attach\s+1)/$1 $2/;
-        $line;
-    },
-
-    "podman-attach.1.md:NNN: wrong title line '% podman attach 1'; should be '% podman-attach 1'",
-);
-
-
-# Tests for broken command-line options
-# IMPORTANT NOTE: podman-exec precedes podman-container (below),
-# because podman-exec.1.md is actually read while podman-container.1.md
-# is still processing; so these messages are printed earlier:
-#   podman-container.1.md  -> list of subcommands -> exec -> read -exec.1.md
-# Sorry for the confusion.
-sed('podman-exec.1.md', sub {
-        my $line = shift;
-
-        if ($line =~ /^#### \*\*--env\*\*/) {
-            $line = $line . "\ndup dup dup\n\n" . $line;
-        }
-        elsif ($line =~ /^#### \*\*--privileged/) {
-            $line = "#### \*\*--put-me-back-in-order\*\*\n\nbogus option\n\n" . $line;
-        }
-        elsif ($line =~ /^#### \*\*--tty\*\*/) {
-            chomp $line;
-            $line .= " xyz\n";
-        }
-        elsif ($line =~ /^#### \*\*--workdir\*\*/) {
-            $line = <<"END_FOO";
-#### **--workdir**=*dir*, **-w**
-
-blah blah bogus description
-
-#### **--yucca**=*cactus*|*starch*|*both*
-
-blah blah
-
-#### **--zydeco**=*true* | *false*
-
-END_FOO
-        }
-
-        return $line;
-    },
-
-    "podman-exec.1.md:NNN: flag '--env' is a dup",
-    "podman-exec.1.md:NNN: --privileged should precede --put-me-back-in-order",
-    "podman-exec.1.md:NNN: could not parse ' xyz' in option description",
-    "podman-exec.1.md:NNN: please rewrite as ', **-w**=*dir*'",
-    "podman-exec.1.md:NNN: values must be space-separated: '=*cactus*|*starch*|*both*'",
-    "podman-exec.1.md:NNN: Do not enumerate true/false for boolean-only options",
-);
-
-
-# Tests for subcommands in a table
-sed('podman-container.1.md', sub {
-        my $line = shift;
-
-        # "podman container diff": force an out-of-order error
-        state $diff;
-        if ($line =~ /^\|\s+diff\s+\|/) {
-            $diff = $line;
-            return '';
-        }
-        if ($diff) {
-            $line .= $diff;
-            $diff = undef;
-        }
-
-        # "podman init": force a duplicate-command error
-        if ($line =~ /^\|\s+init\s+\|/) {
-            $line .= $line;
-        }
-
-        # "podman container port": force a wrong-man-page error
-        if ($line =~ /^\|\s+port\s+\|/) {
-            $line =~ s/-port\.1\.md/-top.1.md/;
-        }
-
-        return $line;
-    },
-
-    "podman-container.1.md:NNN: 'exec' and 'diff' are out of order",
-    "podman-container.1.md:NNN: duplicate subcommand 'init'",
-    # FIXME: this is not technically correct; it could be the other way around.
-    "podman-container.1.md:NNN: 'podman-port' should be 'podman-top' in '[podman-port(1)](podman-top.1.md)'",
-);
-
-
-# Tests for --format specifiers in a table
-sed('podman-image-inspect.1.md', sub {
-        my $line = shift;
-
-        state $digest;
-        if ($line =~ /^\|\s+\.Digest\s+\|/) {
-            $digest = $line;
-            return '';
-        }
-        if ($digest) {
-            $line .= $digest;
-            $digest = undef;
-        }
-
-        if ($line =~ /^\|\s+\.ID\s+\|/) {
-            $line = $line . $line;
-        }
-
-        return $line;
-    },
-
-    "podman-image-inspect.1.md:NNN: format specifier '.Digest' should precede '.GraphDriver'",
-    "podman-image-inspect.1.md:NNN: format specifier '.ID' is a dup",
-);
-
-
-# Tests for SEE ALSO section
-sed('podman-version.1.md', sub {
-        my $line = shift;
-
-        if ($line =~ /^## SEE ALSO/) {
-            $line .= "**foo**,**bar**"
-                . ", **baz**baz**"
-                . ", missingstars"
-                . ", **[podman-info(1)](podman-cp.1.md)**"
-                . ", **[podman-foo(1)](podman-wait.1.md)**"
-                . ", **[podman-x](podman-bar.1.md)**"
-                . ", **podman-logs(1)**"
-                . ", **podman-image-rm(1)**"
-                . ", **sdfsdf**"
-                . "\n";
-        }
-
-        return $line;
-    },
-
-    "podman-version.1.md:NNN: please add space after comma: '**foo**,**bar**'",
-    "podman-version.1.md:NNN: invalid token 'baz**baz'",
-    "podman-version.1.md:NNN: 'missingstars' should be bracketed by '**'",
-    "podman-version.1.md:NNN: inconsistent link podman-info(1) -> podman-cp.1.md, expected podman-info.1.md",
-    "podman-version.1.md:NNN: invalid link podman-foo(1) -> podman-wait.1.md",
-    "podman-version.1.md:NNN: could not parse 'podman-x' as 'manpage(N)'",
-    "podman-version.1.md:NNN: 'podman-logs(1)' should be '[podman-logs(1)](podman-logs.1.md)'",
-    "podman-version.1.md:NNN: 'podman-image-rm(1)' refers to a command alias; please use the canonical command name instead",
-    "podman-version.1.md:NNN: invalid token 'sdfsdf'"
-);
-
-
-# Tests for --filter specifiers
-sed('podman-volume-prune.1.md', sub {
-        my $line = shift;
-
-        if ($line =~ /^\|\s+driver\s+\|/) {
-            $line = "| name! | sdfsdf |\n" . $line;
-        }
-        if ($line =~ /^\|\s+opt\s+\|/) {
-            $line .= $line;
-        }
-
-        return $line;
-    },
-
-    "podman-volume-prune.1.md:NNN: filter 'name!' only allowed immediately after its positive",
-    "podman-volume-prune.1.md:NNN: filter specifier 'opt' is a dup",
-);
-
-# DONE with fault injection. Reread man pages and verify warnings.
-@warnings_seen = ();
-{
-    no warnings 'once';
-    %LibPod::CI::XrefHelpmsgsManpages::Man_Seen = ();
-}
-$man = LibPod::CI::XrefHelpmsgsManpages::podman_man('podman');
-eq_or_diff_text \@warnings_seen, \@expect_warnings, "podman_man() with faults";
 
 # END   fault injection tests on podman_man()
 ###############################################################################
diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go
index 280ada1e84..91feb8e29a 100644
--- a/pkg/machine/applehv/machine.go
+++ b/pkg/machine/applehv/machine.go
@@ -22,9 +22,9 @@ import (
 	"github.com/containers/podman/v4/pkg/machine"
 	"github.com/containers/podman/v4/pkg/machine/applehv/vfkit"
 	"github.com/containers/podman/v4/pkg/machine/define"
+	"github.com/containers/podman/v4/pkg/machine/ignition"
 	"github.com/containers/podman/v4/pkg/machine/sockets"
 	"github.com/containers/podman/v4/pkg/machine/vmconfigs"
-    "github.com/containers/podman/v4/pkg/machine/ignition"
 	"github.com/containers/podman/v4/pkg/strongunits"
 	"github.com/containers/podman/v4/pkg/systemd/parser"
 	"github.com/containers/podman/v4/pkg/util"
@@ -202,7 +202,7 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 		return false, err
 	}
 
-	m.IdentityPath = util.GetIdentityPath(m.Name)
+	m.IdentityPath = util.GetIdentityPath(define.DefaultIdentityName)
 	m.Rootful = opts.Rootful
 	m.RemoteUsername = opts.Username
 
@@ -241,11 +241,10 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 	}
 
 	if len(opts.IgnitionPath) < 1 {
-		key, err = machine.CreateSSHKeys(m.IdentityPath)
+		key, err = machine.GetSSHKeys(m.IdentityPath)
 		if err != nil {
 			return false, err
 		}
-		callbackFuncs.Add(m.removeSSHKeys)
 	}
 
 	builder := ignition.NewIgnitionBuilder(ignition.DynamicIgnition{
@@ -260,7 +259,8 @@ func (m *MacMachine) Init(opts machine.InitOptions) (bool, error) {
 	})
 
 	if len(opts.IgnitionPath) > 0 {
-		return false, builder.BuildWithIgnitionFile(opts.IgnitionPath)
+		err = builder.BuildWithIgnitionFile(opts.IgnitionPath)
+		return false, err
 	}
 
 	if err := builder.GenerateIgnitionConfig(); err != nil {
@@ -293,13 +293,6 @@ func createReadyUnitFile() (string, error) {
 	return readyUnit.ToString()
 }
 
-func (m *MacMachine) removeSSHKeys() error {
-	if err := os.Remove(fmt.Sprintf("%s.pub", m.IdentityPath)); err != nil {
-		logrus.Error(err)
-	}
-	return os.Remove(m.IdentityPath)
-}
-
 func (m *MacMachine) removeSystemConnections() error {
 	return machine.RemoveConnections(m.Name, fmt.Sprintf("%s-root", m.Name))
 }
@@ -344,9 +337,6 @@ func (m *MacMachine) Inspect() (*machine.InspectInfo, error) {
 // collectFilesToDestroy retrieves the files that will be destroyed by `Remove`
 func (m *MacMachine) collectFilesToDestroy(opts machine.RemoveOptions) []string {
 	files := []string{}
-	if !opts.SaveKeys {
-		files = append(files, m.IdentityPath, m.IdentityPath+".pub")
-	}
 	if !opts.SaveIgnition {
 		files = append(files, m.IgnitionFile.GetPath())
 	}
diff --git a/pkg/machine/config.go b/pkg/machine/config.go
index 57491ab94f..9698e20d7c 100644
--- a/pkg/machine/config.go
+++ b/pkg/machine/config.go
@@ -113,7 +113,6 @@ type StopOptions struct{}
 
 type RemoveOptions struct {
 	Force        bool
-	SaveKeys     bool
 	SaveImage    bool
 	SaveIgnition bool
 }
diff --git a/pkg/machine/define/config.go b/pkg/machine/define/config.go
index ba98908be1..f7a8bbd48d 100644
--- a/pkg/machine/define/config.go
+++ b/pkg/machine/define/config.go
@@ -1,3 +1,4 @@
 package define
 
 const UserCertsTargetPath = "/etc/containers/certs.d"
+const DefaultIdentityName = "machine"
diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go
index 8ff0791358..ea8d522144 100644
--- a/pkg/machine/hyperv/machine.go
+++ b/pkg/machine/hyperv/machine.go
@@ -21,8 +21,8 @@ import (
 	"github.com/containers/podman/v4/pkg/machine"
 	"github.com/containers/podman/v4/pkg/machine/define"
 	"github.com/containers/podman/v4/pkg/machine/hyperv/vsock"
-	"github.com/containers/podman/v4/pkg/machine/vmconfigs"
 	"github.com/containers/podman/v4/pkg/machine/ignition"
+	"github.com/containers/podman/v4/pkg/machine/vmconfigs"
 	"github.com/containers/podman/v4/pkg/strongunits"
 	"github.com/containers/podman/v4/pkg/systemd/parser"
 	"github.com/containers/podman/v4/pkg/util"
@@ -178,7 +178,7 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 		return nil
 	})
 
-	m.IdentityPath = util.GetIdentityPath(m.Name)
+	m.IdentityPath = util.GetIdentityPath(define.DefaultIdentityName)
 
 	if m.UID == 0 {
 		m.UID = 1000
@@ -205,11 +205,10 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 	callbackFuncs.Add(m.removeSystemConnections)
 
 	if len(opts.IgnitionPath) < 1 {
-		key, err = machine.CreateSSHKeys(m.IdentityPath)
+		key, err = machine.GetSSHKeys(m.IdentityPath)
 		if err != nil {
 			return false, err
 		}
-		callbackFuncs.Add(m.removeSSHKeys)
 	}
 
 	m.ResourceConfig = vmconfigs.ResourceConfig{
@@ -233,7 +232,8 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 	// If the user provides an ignition file, we need to
 	// copy it into the conf dir
 	if len(opts.IgnitionPath) > 0 {
-		return false, builder.BuildWithIgnitionFile(opts.IgnitionPath)
+		err = builder.BuildWithIgnitionFile(opts.IgnitionPath)
+		return false, err
 	}
 	callbackFuncs.Add(m.IgnitionFile.Delete)
 
@@ -319,13 +319,6 @@ func (m *HyperVMachine) unregisterMachine() error {
 	return vm.Remove("")
 }
 
-func (m *HyperVMachine) removeSSHKeys() error {
-	if err := os.Remove(fmt.Sprintf("%s.pub", m.IdentityPath)); err != nil {
-		logrus.Error(err)
-	}
-	return os.Remove(m.IdentityPath)
-}
-
 func (m *HyperVMachine) removeSystemConnections() error {
 	return machine.RemoveConnections(m.Name, fmt.Sprintf("%s-root", m.Name))
 }
@@ -378,10 +371,6 @@ func (m *HyperVMachine) Inspect() (*machine.InspectInfo, error) {
 // collectFilesToDestroy retrieves the files that will be destroyed by `Remove`
 func (m *HyperVMachine) collectFilesToDestroy(opts machine.RemoveOptions, diskPath *string) []string {
 	files := []string{}
-
-	if !opts.SaveKeys {
-		files = append(files, m.IdentityPath, m.IdentityPath+".pub")
-	}
 	if !opts.SaveIgnition {
 		files = append(files, m.IgnitionFile.GetPath())
 	}
diff --git a/pkg/machine/keys.go b/pkg/machine/keys.go
index d46f759f71..e151aa1174 100644
--- a/pkg/machine/keys.go
+++ b/pkg/machine/keys.go
@@ -36,6 +36,20 @@ func CreateSSHKeys(writeLocation string) (string, error) {
 	return strings.TrimSuffix(string(b), "\n"), nil
 }
 
+// GetSSHKeys checks to see if there is a ssh key at the provided location.
+// If not, we create the priv and pub keys. The ssh key is then returned.
+func GetSSHKeys(identityPath string) (string, error) {
+	if _, err := os.Stat(identityPath); err == nil {
+		b, err := os.ReadFile(identityPath + ".pub")
+		if err != nil {
+			return "", err
+		}
+		return strings.TrimSuffix(string(b), "\n"), nil
+	}
+
+	return CreateSSHKeys(identityPath)
+}
+
 func CreateSSHKeysPrefix(identityPath string, passThru bool, skipExisting bool, prefix ...string) (string, error) {
 	_, e := os.Stat(identityPath)
 	if !skipExisting || errors.Is(e, os.ErrNotExist) {
diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go
index 0845626ae6..14ab01d790 100644
--- a/pkg/machine/qemu/machine.go
+++ b/pkg/machine/qemu/machine.go
@@ -124,7 +124,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	defer callbackFuncs.CleanIfErr(&err)
 	go callbackFuncs.CleanOnSignal()
 
-	v.IdentityPath = util.GetIdentityPath(v.Name)
+	v.IdentityPath = util.GetIdentityPath(define.DefaultIdentityName)
 	v.Rootful = opts.Rootful
 
 	imagePath, strm, err := machine.Pull(opts.ImagePath, opts.Name, VirtualizationProvider())
@@ -169,11 +169,10 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	// User has provided ignition file so keygen
 	// will be skipped.
 	if len(opts.IgnitionPath) < 1 {
-		key, err = machine.CreateSSHKeys(v.IdentityPath)
+		key, err = machine.GetSSHKeys(v.IdentityPath)
 		if err != nil {
 			return false, err
 		}
-		callbackFuncs.Add(v.removeSSHKeys)
 	}
 	// Run arch specific things that need to be done
 	if err = v.prepare(); err != nil {
@@ -206,7 +205,8 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	// If the user provides an ignition file, we need to
 	// copy it into the conf dir
 	if len(opts.IgnitionPath) > 0 {
-		return false, builder.BuildWithIgnitionFile(opts.IgnitionPath)
+		err = builder.BuildWithIgnitionFile(opts.IgnitionPath)
+		return false, err
 	}
 
 	if err := builder.GenerateIgnitionConfig(); err != nil {
@@ -238,13 +238,6 @@ func createReadyUnitFile() (string, error) {
 	return readyUnit.ToString()
 }
 
-func (v *MachineVM) removeSSHKeys() error {
-	if err := os.Remove(fmt.Sprintf("%s.pub", v.IdentityPath)); err != nil {
-		logrus.Error(err)
-	}
-	return os.Remove(v.IdentityPath)
-}
-
 func (v *MachineVM) removeSystemConnections() error {
 	return machine.RemoveConnections(v.Name, fmt.Sprintf("%s-root", v.Name))
 }
@@ -923,9 +916,6 @@ func NewQMPMonitor(network, name string, timeout time.Duration) (command.Monitor
 func (v *MachineVM) collectFilesToDestroy(opts machine.RemoveOptions) ([]string, error) {
 	files := []string{}
 	// Collect all the files that need to be destroyed
-	if !opts.SaveKeys {
-		files = append(files, v.IdentityPath, v.IdentityPath+".pub")
-	}
 	if !opts.SaveIgnition {
 		files = append(files, v.getIgnitionFile())
 	}
@@ -968,7 +958,7 @@ func (v *MachineVM) removeQMPMonitorSocketAndVMPidFile() {
 	}
 }
 
-// Remove deletes all the files associated with a machine including ssh keys, the image itself
+// Remove deletes all the files associated with a machine including the image itself
 func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func() error, error) {
 	var (
 		files []string
diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go
index efb5485239..ee0301070e 100644
--- a/pkg/machine/wsl/machine.go
+++ b/pkg/machine/wsl/machine.go
@@ -19,8 +19,8 @@ import (
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/podman/v4/pkg/machine"
 	"github.com/containers/podman/v4/pkg/machine/define"
-	"github.com/containers/podman/v4/pkg/machine/vmconfigs"
 	"github.com/containers/podman/v4/pkg/machine/ignition"
+	"github.com/containers/podman/v4/pkg/machine/vmconfigs"
 	"github.com/containers/podman/v4/pkg/machine/wsl/wutil"
 	"github.com/containers/podman/v4/pkg/util"
 	"github.com/containers/podman/v4/utils"
@@ -411,7 +411,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	}
 
 	_ = setupWslProxyEnv()
-	v.IdentityPath = util.GetIdentityPath(v.Name)
+	v.IdentityPath = util.GetIdentityPath(define.DefaultIdentityName)
 	v.Rootful = opts.Rootful
 	v.Version = currentMachineVersion
 
@@ -452,7 +452,6 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	if err = createKeys(v, dist); err != nil {
 		return false, err
 	}
-	callbackFuncs.Add(v.removeSSHKeys)
 
 	// Cycle so that user change goes into effect
 	_ = terminateDist(dist)
@@ -494,13 +493,6 @@ func (v *MachineVM) removeMachineImage() error {
 	return utils.GuardedRemoveAll(v.ImagePath)
 }
 
-func (v *MachineVM) removeSSHKeys() error {
-	if err := utils.GuardedRemoveAll(fmt.Sprintf("%s.pub", v.IdentityPath)); err != nil {
-		logrus.Error(err)
-	}
-	return utils.GuardedRemoveAll(v.IdentityPath)
-}
-
 func (v *MachineVM) removeSystemConnections() error {
 	return machine.RemoveConnections(v.Name, fmt.Sprintf("%s-root", v.Name))
 }
@@ -1057,7 +1049,7 @@ func wslPipe(input string, dist string, arg ...string) error {
 }
 
 func wslCreateKeys(identityPath string, dist string) (string, error) {
-	return machine.CreateSSHKeysPrefix(identityPath, true, false, "wsl", "-u", "root", "-d", dist)
+	return machine.CreateSSHKeysPrefix(identityPath, true, true, "wsl", "-u", "root", "-d", dist)
 }
 
 func runCmdPassThrough(name string, arg ...string) error {
@@ -1605,9 +1597,6 @@ func (v *MachineVM) Remove(name string, opts machine.RemoveOptions) (string, fun
 	defer v.lock.Unlock()
 
 	// Collect all the files that need to be destroyed
-	if !opts.SaveKeys {
-		files = append(files, v.IdentityPath, v.IdentityPath+".pub")
-	}
 	if !opts.SaveImage {
 		files = append(files, v.ImagePath)
 	}
diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index 04abffa36f..b3ccbb4c23 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -1184,7 +1184,7 @@ func ExitCode(err error) int {
 }
 
 func GetIdentityPath(name string) string {
-	return filepath.Join(homedir.Get(), ".ssh", name)
+	return filepath.Join(homedir.Get(), ".local", "share", "containers", "podman", "machine", name)
 }
 
 func Tmpdir() string {


Test patch:
diff --git a/pkg/machine/e2e/config_rm_test.go b/pkg/machine/e2e/config_rm_test.go
index f0cf15a06f..c638e6ac05 100644
--- a/pkg/machine/e2e/config_rm_test.go
+++ b/pkg/machine/e2e/config_rm_test.go
@@ -5,13 +5,11 @@ type rmMachine struct {
 	  -f, --force           Stop and do not prompt before rming
 	      --save-ignition   Do not delete ignition file
 	      --save-image      Do not delete the image file
-	      --save-keys       Do not delete SSH keys
 
 	*/
 	force        bool
 	saveIgnition bool
 	saveImage    bool
-	saveKeys     bool
 
 	cmd []string
 }
@@ -27,9 +25,6 @@ func (i *rmMachine) buildCmd(m *machineTestBuilder) []string {
 	if i.saveImage {
 		cmd = append(cmd, "--save-image")
 	}
-	if i.saveKeys {
-		cmd = append(cmd, "--save-keys")
-	}
 	cmd = append(cmd, m.name)
 	i.cmd = cmd
 	return cmd
@@ -49,8 +44,3 @@ func (i *rmMachine) withSaveImage() *rmMachine {
 	i.saveImage = true
 	return i
 }
-
-func (i *rmMachine) withSaveKeys() *rmMachine {
-	i.saveKeys = true
-	return i
-}
diff --git a/pkg/machine/e2e/init_test.go b/pkg/machine/e2e/init_test.go
index 84f064777b..27b0b3d060 100644
--- a/pkg/machine/e2e/init_test.go
+++ b/pkg/machine/e2e/init_test.go
@@ -303,7 +303,7 @@ var _ = Describe("podman machine init", func() {
 		img := inspectSession.outputToString()
 
 		rm := rmMachine{}
-		removeSession, err := mb.setCmd(rm.withForce().withSaveKeys()).run()
+		removeSession, err := mb.setCmd(rm.withForce()).run()
 		Expect(err).ToNot(HaveOccurred())
 		Expect(removeSession).To(Exit(0))
 
@@ -313,20 +313,21 @@ var _ = Describe("podman machine init", func() {
 		Expect(err).ToNot(HaveOccurred())
 		Expect(ec).To(Equal(125))
 
-		// Clashing keys - init fails
-		i = new(initMachine)
-		session, err = mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run()
-		Expect(err).ToNot(HaveOccurred())
-		Expect(session).To(Exit(125))
-
-		// ensure files created by init are cleaned up on init failure
-		_, err = os.Stat(img)
-		Expect(err).To(HaveOccurred())
-		_, err = os.Stat(cfgpth)
-		Expect(err).To(HaveOccurred())
-
 		// WSL does not use ignition
 		if testProvider.VMType() != define.WSLVirt {
+			// Bad ignition path - init fails
+			i = new(initMachine)
+			i.ignitionPath = "/bad/path"
+			session, err = mb.setName(name).setCmd(i.withImagePath(mb.imagePath)).run()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(session).To(Exit(125))
+
+			// ensure files created by init are cleaned up on init failure
+			_, err = os.Stat(img)
+			Expect(err).To(HaveOccurred())
+			_, err = os.Stat(cfgpth)
+			Expect(err).To(HaveOccurred())
+
 			_, err = os.Stat(ign)
 			Expect(err).To(HaveOccurred())
 		}
diff --git a/pkg/machine/e2e/rm_test.go b/pkg/machine/e2e/rm_test.go
index a3a1ab6ef0..11f76de25b 100644
--- a/pkg/machine/e2e/rm_test.go
+++ b/pkg/machine/e2e/rm_test.go
@@ -3,6 +3,7 @@ package e2e_test
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 
 	"github.com/containers/podman/v4/pkg/machine/define"
 	. "github.com/onsi/ginkgo/v2"
@@ -80,7 +81,7 @@ var _ = Describe("podman machine rm", func() {
 		Expect(ec).To(Equal(125))
 	})
 
-	It("machine rm --save-keys, --save-ignition, --save-image", func() {
+	It("machine rm --save-ignition --save-image", func() {
 		i := new(initMachine)
 		session, err := mb.setCmd(i.withImagePath(mb.imagePath)).run()
 		Expect(err).ToNot(HaveOccurred())
@@ -104,7 +105,7 @@ var _ = Describe("podman machine rm", func() {
 		img := inspectSession.outputToString()
 
 		rm := rmMachine{}
-		removeSession, err := mb.setCmd(rm.withForce().withSaveIgnition().withSaveImage().withSaveKeys()).run()
+		removeSession, err := mb.setCmd(rm.withForce().withSaveIgnition().withSaveImage()).run()
 		Expect(err).ToNot(HaveOccurred())
 		Expect(removeSession).To(Exit(0))
 
@@ -127,4 +128,71 @@ var _ = Describe("podman machine rm", func() {
 		_, err = os.Stat(img)
 		Expect(err).ToNot(HaveOccurred())
 	})
+
+	It("Remove machine sharing ssh key with another machine", func() {
+		expectedIdentityPathSuffix := filepath.Join(".local", "share", "containers", "podman", "machine", define.DefaultIdentityName)
+
+		fooName := "foo"
+		foo := new(initMachine)
+		session, err := mb.setName(fooName).setCmd(foo.withImagePath(mb.imagePath)).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(session).To(Exit(0))
+
+		barName := "bar"
+		bar := new(initMachine)
+		session, err = mb.setName(barName).setCmd(bar.withImagePath(mb.imagePath).withNow()).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(session).To(Exit(0))
+
+		inspectFoo := new(inspectMachine)
+		inspectFoo = inspectFoo.withFormat("{{.SSHConfig.IdentityPath}}")
+		inspectSession, err := mb.setName(fooName).setCmd(inspectFoo).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(inspectSession).To(Exit(0))
+		Expect(inspectSession.outputToString()).To(ContainSubstring(expectedIdentityPathSuffix))
+		fooIdentityPath := inspectSession.outputToString()
+
+		inspectBar := new(inspectMachine)
+		inspectBar = inspectBar.withFormat("{{.SSHConfig.IdentityPath}}")
+		inspectSession, err = mb.setName(barName).setCmd(inspectBar).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(inspectSession).To(Exit(0))
+		Expect(inspectSession.outputToString()).To(Equal(fooIdentityPath))
+
+		rmFoo := new(rmMachine)
+		stop, err := mb.setName(fooName).setCmd(rmFoo.withForce()).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(stop).To(Exit(0))
+
+		// removal of foo should not affect the ability to ssh into the bar machine
+		sshBar := new(sshMachine)
+		sshSession, err := mb.setName(barName).setCmd(sshBar.withSSHCommand([]string{"echo", "foo"})).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(sshSession).To(Exit(0))
+	})
+
+	It("Removing all machines doesn't delete ssh keys", func() {
+		fooName := "foo"
+		foo := new(initMachine)
+		session, err := mb.setName(fooName).setCmd(foo.withImagePath(mb.imagePath)).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(session).To(Exit(0))
+
+		inspectFoo := new(inspectMachine)
+		inspectFoo = inspectFoo.withFormat("{{.SSHConfig.IdentityPath}}")
+		inspectSession, err := mb.setName(fooName).setCmd(inspectFoo).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(inspectSession).To(Exit(0))
+		fooIdentityPath := inspectSession.outputToString()
+
+		rmFoo := new(rmMachine)
+		stop, err := mb.setName(fooName).setCmd(rmFoo.withForce()).run()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(stop).To(Exit(0))
+
+		_, err = os.Stat(fooIdentityPath)
+		Expect(err).ToNot(HaveOccurred())
+		_, err = os.Stat(fooIdentityPath + ".pub")
+		Expect(err).ToNot(HaveOccurred())
+	})
 })
diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go
index e3c303a6e9..38a65f453f 100644
--- a/pkg/util/utils_test.go
+++ b/pkg/util/utils_test.go
@@ -531,7 +531,7 @@ func TestValidateSysctlBadSysctlWithExtraSpaces(t *testing.T) {
 func TestGetIdentityPath(t *testing.T) {
 	name := "p-test"
 	identityPath := GetIdentityPath(name)
-	assert.Equal(t, identityPath, filepath.Join(homedir.Get(), ".ssh", name))
+	assert.Equal(t, identityPath, filepath.Join(homedir.Get(), ".local", "share", "containers", "podman", "machine", name))
 }
 
 func TestCoresToPeriodAndQuota(t *testing.T) {