[PRODENG-3917] Escape MSR bootstrapper credentials and host-reported facts

pkg/msr/msr.go

@@ -10,6 +10,7 @@ import (
 	mkeconfig "github.com/Mirantis/launchpad/pkg/product/mke/config"
 	"github.com/Mirantis/launchpad/pkg/util/stringutil"
 	"github.com/avast/retry-go"
+	"github.com/k0sproject/rig/v2/sh/shellescape"
 	log "github.com/sirupsen/logrus"
 )
 
@@ -134,9 +135,9 @@ func BuildMKEFlags(config *mkeconfig.ClusterConfig) commonconfig.Flags {
 	}
 
 	return commonconfig.Flags{
-		fmt.Sprintf("--ucp-url=\"%s\"", mkeURLHost(config)),
-		fmt.Sprintf("--ucp-username=\"%s\"", mkeUser),
-		fmt.Sprintf("--ucp-password=\"%s\"", mkePass),
+		fmt.Sprintf("--ucp-url %s", shellescape.Quote(mkeURLHost(config))),
+		fmt.Sprintf("--ucp-username %s", shellescape.Quote(mkeUser)),
+		fmt.Sprintf("--ucp-password %s", shellescape.Quote(mkePass)),
 	}
 }
 
@@ -151,13 +152,22 @@ func mkeURLHost(config *mkeconfig.ClusterConfig) string {
 // installer if it fails.
 func Destroy(h *mkeconfig.Host, config *mkeconfig.ClusterConfig) error {
 	mkeFlags := BuildMKEFlags(config)
+	// BuildMKEFlags returns shell-quoted flag values. Recover the raw values
+	// before handing them to DockerCmd, which applies its own quoting.
+	unquote := func(flag string) string {
+		if raw, err := shellescape.Unquote(mkeFlags.GetValue(flag)); err == nil {
+			return raw
+		}
+
+		return mkeFlags.GetValue(flag)
+	}
 	destroyCmd := h.Configurer.DockerCmd().
 		Raw("run -it --rm").
 		Arg(fmt.Sprintf("mirantis/dtr:%s", h.MSRMetadata.InstalledVersion)).
 		Raw("destroy --ucp-insecure-tls").
-		Raw("--ucp-url").Arg(mkeFlags.GetValue("--ucp-url")).
-		Raw("--ucp-username").Arg(mkeFlags.GetValue("--ucp-username")).
-		Raw("--ucp-password").Arg(mkeFlags.GetValue("--ucp-password")).
+		Raw("--ucp-url").Arg(unquote("--ucp-url")).
+		Raw("--ucp-username").Arg(unquote("--ucp-username")).
+		Raw("--ucp-password").Arg(unquote("--ucp-password")).
 		Raw("--replica-id").Arg(h.MSRMetadata.ReplicaID).
 		String()
 	if err := h.Exec(destroyCmd); err != nil {

pkg/msr/msr_test.go

@@ -3,10 +3,13 @@ package msr
 import (
 	"reflect"
 	"sort"
+	"strings"
 	"testing"
 
+	"github.com/Mirantis/launchpad/pkg/configurer"
 	commonconfig "github.com/Mirantis/launchpad/pkg/product/common/config"
 	mkeconfig "github.com/Mirantis/launchpad/pkg/product/mke/config"
+	"github.com/k0sproject/rig/v2/sh/shellescape"
 	"github.com/stretchr/testify/require"
 )
 
@@ -63,12 +66,12 @@ func TestBuildMKEFlags(t *testing.T) {
 		},
 	}
 
-	t.Run("MKE flags are built when --san is provided", func(t *testing.T) {
+	t.Run("MKE flags are built shell-quoted when --san is provided", func(t *testing.T) {
 		actual := BuildMKEFlags(config)
 		expected := commonconfig.Flags{
-			"--ucp-url=\"ucp.acme.com\"",
-			"--ucp-username=\"admin\"",
-			"--ucp-password=\"password1234\"",
+			"--ucp-url ucp.acme.com",
+			"--ucp-username admin",
+			"--ucp-password password1234",
 		}
 		sort.Strings(actual)
 		sort.Strings(expected)
@@ -78,6 +81,63 @@ func TestBuildMKEFlags(t *testing.T) {
 	})
 }
 
+// TestBuildMKEFlagsQuotesCredentials pins the property every MSR bootstrapper
+// phase depends on: the flag values are single shell-safe words, so a
+// password containing $(...), backticks or $VAR cannot execute when the flag
+// list is joined and run through the remote shell.
+func TestBuildMKEFlagsQuotesCredentials(t *testing.T) {
+	t.Parallel()
+
+	config := &mkeconfig.ClusterConfig{Spec: &mkeconfig.ClusterSpec{}}
+	config.Spec.MKE = mkeconfig.MKEConfig{
+		AdminUsername: `ad'min"$(touch /tmp/pwned)`,
+		AdminPassword: "p$(touch /tmp/pwned)`id`$HOME",
+	}
+	config.Spec.MSR = &mkeconfig.MSRConfig{
+		InstallFlags: commonconfig.Flags{"--ucp-url 10.0.0.1"},
+	}
+	flags := BuildMKEFlags(config)
+
+	for _, flag := range []string{"--ucp-username", "--ucp-password"} {
+		value := flags.GetValue(flag)
+		require.Equal(t, value, strings.TrimSpace(value), "%s value is a single word: %q", flag, value)
+	}
+
+	require.Equal(t,
+		`ad'min"$(touch /tmp/pwned)`,
+		mustUnquote(t, flags.GetValue("--ucp-username")),
+	)
+	require.Equal(t,
+		"p$(touch /tmp/pwned)`id`$HOME",
+		mustUnquote(t, flags.GetValue("--ucp-password")),
+	)
+}
+
+// TestDestroyQuotesCredentials pins the single-word property the remote
+// shell depends on: DockerCmd quoting of a metacharacter payload must
+// survive shellescape.Unquote unchanged and stay wrapped, so the shell sees
+// one literal argument instead of expanding it.
+func TestDestroyQuotesCredentials(t *testing.T) {
+	t.Parallel()
+
+	var c configurer.LinuxConfigurer
+	const pass = "p`id`$HOME"
+
+	quotedPass := c.DockerCmd().Quote(pass)
+
+	require.Contains(t, quotedPass, `'`, "quoting must wrap the value")
+	require.Equal(t, pass, mustUnquote(t, quotedPass), "quoting must be reversible without payload loss")
+}
+
+func mustUnquote(t *testing.T, s string) string {
+	t.Helper()
+
+	unquoted, err := shellescape.Unquote(s)
+	require.NoError(t, err)
+
+	return unquoted
+}
+
 func TestFormatReplicaID(t *testing.T) {
 	require.Equal(t, "000000000001", FormatReplicaID(1))
 	require.Equal(t, "00000000000a", FormatReplicaID(10))

pkg/product/mke/phase/gather_facts.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io/fs"
 	"net"
+	"regexp"
 
 	// needed to load the build func in package init.
 	_ "github.com/Mirantis/launchpad/pkg/configurer/centos"
@@ -100,7 +101,15 @@ func (p *GatherFacts) Run() error {
 	return nil
 }
 
-var errInvalidIP = errors.New("invalid IP address")
+var (
+	errInvalidIP       = errors.New("invalid IP address")
+	errInvalidHostname = errors.New("invalid hostname")
+)
+
+// validHostnameRegexp matches a RFC 1123 hostname: letters, digits, hyphens
+// and dots only. The value is host-reported and is later interpolated into
+// MSR bootstrapper flags, so anything looser is an injection vector.
+var validHostnameRegexp = regexp.MustCompile(`^[a-zA-Z0-9.-]+$`)
 
 func (p *GatherFacts) investigateHost(h *mkeconfig.Host, _ *mkeconfig.ClusterConfig) error {
 	log.Infof("%s: gathering host facts", h)
@@ -133,7 +142,11 @@ func (p *GatherFacts) investigateHost(h *mkeconfig.Host, _ *mkeconfig.ClusterCon
 
 	h.Metadata.MCRVersion = version
 
-	h.Metadata.Hostname, _ = h.FS().Hostname()
+	hostname, err := h.FS().Hostname()
+	if err == nil && hostname != "" && !validHostnameRegexp.MatchString(hostname) {
+		return fmt.Errorf("%s: %w: %q", h, errInvalidHostname, hostname)
+	}
+	h.Metadata.Hostname = hostname
 	h.Metadata.LongHostname, _ = h.FS().LongHostname()
 
 	if h.PrivateInterface == "" {

pkg/product/mke/phase/install_msr.go

@@ -50,7 +50,7 @@ func (p *InstallMSR) Run() error {
 	mkeFlags := msr.BuildMKEFlags(p.Config)
 	// Conduct the install passing the --ucp-node flag for the host provided in
 	// msrLeader.
-	mkeFlags.AddOrReplace(fmt.Sprintf("--ucp-node %s", h.Metadata.Hostname))
+	mkeFlags.AddOrReplace(fmt.Sprintf("--ucp-node %s", shellescape.Quote(h.Metadata.Hostname)))
 
 	installFlags.Merge(mkeFlags)
 
@@ -80,7 +80,7 @@ func (p *InstallMSR) Run() error {
 
 	if h.MSRMetadata.ReplicaID != "" {
 		log.Infof("%s: installing MSR with replica id %s", h, h.MSRMetadata.ReplicaID)
-		installFlags.AddOrReplace(fmt.Sprintf("--replica-id %s", h.MSRMetadata.ReplicaID))
+		installFlags.AddOrReplace(fmt.Sprintf("--replica-id %s", shellescape.Quote(h.MSRMetadata.ReplicaID)))
 	} else {
 		log.Infof("%s: installing MSR version %s", h, p.Config.Spec.MSR.Version)
 	}

pkg/product/mke/phase/join_msr_replicas.go

@@ -72,8 +72,8 @@ func (p *JoinMSRReplicas) Run() error {
 		}
 		joinFlags := commonconfig.Flags{}
 		redacts := []string{}
-		joinFlags.Add(fmt.Sprintf("--ucp-node %s", h.Metadata.Hostname))
-		joinFlags.Add(fmt.Sprintf("--existing-replica-id %s", msrLeader.MSRMetadata.ReplicaID))
+		joinFlags.Add(fmt.Sprintf("--ucp-node %s", shellescape.Quote(h.Metadata.Hostname)))
+		joinFlags.Add(fmt.Sprintf("--existing-replica-id %s", shellescape.Quote(msrLeader.MSRMetadata.ReplicaID)))
 		joinFlags.MergeOverwrite(mkeFlags)
 		// We can't just append the installFlags to joinFlags because they
 		// differ, so we have to selectively pluck the ones that are shared
@@ -87,7 +87,7 @@ func (p *JoinMSRReplicas) Run() error {
 		}
 		if h.MSRMetadata != nil && h.MSRMetadata.ReplicaID != "" {
 			log.Infof("%s: joining MSR replica to cluster with replica id: %s", h, h.MSRMetadata.ReplicaID)
-			joinFlags.AddOrReplace(fmt.Sprintf("--replica-id %s", h.MSRMetadata.ReplicaID))
+			joinFlags.AddOrReplace(fmt.Sprintf("--replica-id %s", shellescape.Quote(h.MSRMetadata.ReplicaID)))
 		} else {
 			log.Infof("%s: joining MSR replica to cluster", h)
 		}

pkg/product/mke/phase/remove_nodes.go

@@ -18,6 +18,7 @@ import (
 	"github.com/Mirantis/launchpad/pkg/swarm"
 	"github.com/Mirantis/launchpad/pkg/util/stringutil"
 	"github.com/k0sproject/rig/v2/cmd"
+	"github.com/k0sproject/rig/v2/sh/shellescape"
 	log "github.com/sirupsen/logrus"
 )
 
@@ -228,8 +229,8 @@ func (p *RemoveNodes) removemsrNode(config *mkeconfig.ClusterConfig, replicaID s
 	}
 
 	removeFlags := commonconfig.Flags{
-		fmt.Sprintf("--replica-ids %s", replicaID),
-		fmt.Sprintf("--existing-replica-id %s", msrLeader.MSRMetadata.ReplicaID),
+		fmt.Sprintf("--replica-ids %s", shellescape.Quote(replicaID)),
+		fmt.Sprintf("--existing-replica-id %s", shellescape.Quote(msrLeader.MSRMetadata.ReplicaID)),
 	}
 	removeFlags.MergeOverwrite(mkeFlags)
 	for _, f := range msr.PluckSharedInstallFlags(config.Spec.MSR.InstallFlags, msr.SharedInstallRemoveFlags) {

pkg/product/mke/phase/upgrade_msr.go

@@ -7,6 +7,7 @@ import (
 	"github.com/Mirantis/launchpad/pkg/msr"
 	"github.com/Mirantis/launchpad/pkg/phase"
 	commonconfig "github.com/Mirantis/launchpad/pkg/product/common/config"
+	"github.com/k0sproject/rig/v2/sh/shellescape"
 	log "github.com/sirupsen/logrus"
 )
 
@@ -41,7 +42,7 @@ func (p *UpgradeMSR) Run() error {
 		return nil
 	}
 
-	upgradeFlags := commonconfig.Flags{fmt.Sprintf("--existing-replica-id %s", h.MSRMetadata.ReplicaID)}
+	upgradeFlags := commonconfig.Flags{fmt.Sprintf("--existing-replica-id %s", shellescape.Quote(h.MSRMetadata.ReplicaID))}
 
 	upgradeFlags.MergeOverwrite(msr.BuildMKEFlags(p.Config))
 	for _, f := range msr.PluckSharedInstallFlags(p.Config.Spec.MSR.InstallFlags, msr.SharedInstallUpgradeFlags) {