[PRODENG-3935] Warn on insecure WinRM with an inline password

.changes/PRODENG-3935.md

@@ -0,0 +1,9 @@
+**Breaking: WinRM hosts without a complete credential are now rejected at config load.**
+
+`launchpad` now runs rig's own connection validation for every host when the configuration is loaded. A `winRM` host with a `user` but no `password`, or with a password that is empty after environment substitution, now fails with `user and password must both be set for password authentication` instead of loading. A host entry with no connection block at all fails with `no protocol configuration`. Certificate authentication (`certPath` plus `keyPath`) is unaffected.
+
+Such a configuration could not authenticate before this change either — it sent an HTTP Basic header with an empty password and failed at connect. The change is that it now fails at load, with a message naming the cause.
+
+Rejection was chosen over substituting a default or warning and continuing, because the failure it replaces was silent: a password that vanished during YAML parsing left launchpad authenticating as the configured user with **no password at all**, against a host whose TLS certificate it may also not have verified. A warning would have been missed in exactly the runs where it mattered.
+
+If you are affected, supply the password. Note that values are substituted into the configuration and then parsed as YAML, so a literal `$` must be written `$$` and a value containing an unquoted `#` is truncated at the `#` — which is how a password silently becomes empty. Full guidance: [Keeping credentials out of launchpad.yaml](https://github.com/Mirantis/launchpad/blob/main/docs/usage/getting-started.md).

docs/usage/getting-started.md

@@ -29,6 +29,94 @@ Create a `launchpad.yaml` to define your cluster. A minimal configuration includ
 
 For a full reference, see the [public documentation](https://docs.mirantis.com/mke/3.8/launchpad.html).
 
+### Keeping credentials out of `launchpad.yaml`
+
+Do not write passwords into `launchpad.yaml`. The file sits in plaintext on
+the disk of whichever machine runs Launchpad, is read by every `apply` and
+`reset`, and is easy to paste into a ticket or a chat window by accident.
+
+Launchpad expands environment variables in the configuration file before
+parsing it, so reference the credential instead of inlining it:
+
+```yaml
+spec:
+  hosts:
+    - role: manager
+      winRM:
+        address: 10.0.0.1
+        user: Administrator
+        password: ${WINRM_PASSWORD}
+        useHTTPS: true
+  mke:
+    adminUsername: admin
+    adminPassword: ${MKE_ADMIN_PASSWORD}
+```
+
+Supply the values from your shell, a secret manager, or a CI secret store:
+
+```bash
+export WINRM_PASSWORD="$(my-secret-tool read winrm/administrator)"
+export MKE_ADMIN_PASSWORD="$(my-secret-tool read mke/admin)"
+launchpad apply --config launchpad.yaml
+```
+
+Both `${VAR}` and `$VAR` are recognised. Note the substitution rules, because
+they bite in exactly this situation:
+
+- **An unset variable is an error, not an empty string.** If
+  `WINRM_PASSWORD` is not exported, `launchpad apply` fails with a
+  substitution error rather than quietly trying to authenticate with an empty
+  password. An exported-but-empty variable is accepted as empty.
+- **A literal `$` in a value must be escaped as `$$`.** A password of
+  `Pa$$word` has to be written `Pa$$$$word`, and a password of `abc$def` has
+  to be written `abc$$def` — otherwise `$def` is read as a variable
+  reference. This applies to any value in the file, not just passwords.
+- **`$` followed by a digit is left alone.** `$5986` is never a valid
+  variable name, so it stays literal and needs no escaping.
+- **The substituted value is parsed as YAML afterwards.** Substitution
+  rewrites the file's text and the result is then re-parsed, so YAML
+  metacharacters inside the value are interpreted rather than taken
+  literally. A password of `abc #def` is truncated to `abc`, because
+  everything from the unquoted `#` is a comment; a password of `abc: def`
+  aborts the load with `yaml: line 11: mapping values are not allowed in
+  this context`. Quoting the placeholder in your own file does not help:
+  the configuration is re-marshalled before substitution, which discards
+  that quoting.
+
+A value containing `#`, `:`, `"`, `'`, a leading `{`, `[`, `*`, `&` or `%`,
+or a trailing space therefore needs care. Two ways to stay safe:
+
+- Prefer generated passwords restricted to alphanumerics and `-`, `_`, `.`,
+  which have no YAML meaning in any position.
+- If the value is outside your control, pass it base64-encoded and decode it
+  when you set the variable, so the text that reaches the configuration file
+  is always inert.
+
+Sourcing the credential from the environment is still better than inlining
+it, because the value lives in the process environment rather than on disk —
+but it is not a way to smuggle arbitrary bytes past the parser.
+
+### WinRM over TLS
+
+Launchpad warns at config load for every host whose WinRM password is exposed
+by the transport it is configured to use. There are two separate failures.
+
+**`winRM.useHTTPS: false` with a password sends it in clear.** Without TLS,
+and without `winRM.useNTLM: true`, the password is placed in an HTTP
+`Authorization` header as base64 — an encoding, not encryption. Anyone on the
+network path reads the password and every command sent to the host. This is
+the default transport: `useHTTPS` defaults to `false`.
+
+**`winRM.insecure: true` accepts any certificate the host presents.** The
+channel is encrypted but the peer is not authenticated, so an attacker on the
+network path can impersonate the host, capture the password and read every
+command sent to it. `useHTTPS: true` does not protect you on its own.
+
+Treat either warning as a finding. Set `winRM.useHTTPS: true`, install a
+certificate the client trusts, pin it with `winRM.caCertPath`, and leave
+`winRM.insecure` unset. Leaving `insecure` set is only defensible on a
+trusted network you control, and never with a password you would mind losing.
+
 ## Running Launchpad
 
 Use the `apply` command to start the installation:

pkg/product/mke/config/cluster.go

@@ -12,6 +12,7 @@ import (
 	validator "github.com/go-playground/validator/v10"
 	rig "github.com/k0sproject/rig/v2"
 	"github.com/k0sproject/rig/v2/protocol/ssh"
+	log "github.com/sirupsen/logrus"
 )
 
 // ClusterMeta defines cluster metadata.
@@ -62,6 +63,8 @@ func (c *ClusterConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {
 // Validate validates that everything in the config makes sense
 // Currently we do only very "static" validation using https://github.com/go-playground/validator
 func (c *ClusterConfig) Validate() error {
+	c.warnWinRMPasswordExposure()
+
 	validator := validator.New(validator.WithRequiredStructEnabled())
 	validator.RegisterStructValidation(roleChecks, ClusterSpec{})
 	// Defence in depth, not the control: the command sites quote. Validation
@@ -82,9 +85,110 @@ func (c *ClusterConfig) Validate() error {
 	if err := validator.Struct(c); err != nil {
 		return fmt.Errorf("cluster config validation failed: %w", err)
 	}
+
+	if err := c.validateConnections(); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// validateConnections runs rig's own connection validation for every host.
+//
+// rig already enforces that a password-authenticated connection has both a
+// user and a password (protocol/winrm/config.go:66-68 and the SSH
+// equivalent), but launchpad never reached that guard: it connects through
+// CompositeConfig.Connection(), which does not validate, so the check was
+// dead code on this path. Without it a password that is present in the file
+// but empty by the time it is parsed -- a value YAML-truncated to nothing,
+// for instance -- authenticated as the configured user with no password at
+// all, with no warning and no error.
+//
+// This also closes the gap in warnWinRMPasswordExposure, which fires where
+// the password survives and was silent where it vanished entirely.
+func (c *ClusterConfig) validateConnections() error {
+	if c.Spec == nil {
+		return nil
+	}
+
+	for _, h := range c.Spec.Hosts {
+		if h == nil {
+			continue
+		}
+		if err := h.Validate(); err != nil {
+			return fmt.Errorf("host connection configuration is invalid: %w", err)
+		}
+	}
+
 	return nil
 }
 
+// warnWinRMPasswordExposure warns about every host whose WinRM password is
+// exposed by the transport it is configured to use. Two distinct failures are
+// reported separately, because the remedy differs:
+//
+//   - plain HTTP with a password. rig installs a transport decorator only for
+//     NTLM, or for HTTPS with a loaded client certificate
+//     (protocol/winrm/connection.go:242-250). With neither, go-winrm's default
+//     transport calls req.SetBasicAuth (masterzen/winrm/http.go:96), so the
+//     password crosses the network base64-encoded in an Authorization header
+//     with no encryption at all.
+//   - HTTPS with winRM.insecure. The channel is encrypted but the peer is
+//     never authenticated, so an on-path attacker can impersonate the host,
+//     recover the password and read every command issued to it.
+//
+// Both are warnings rather than errors: an operator may have accepted either
+// risk knowingly and must still be able to apply. The messages name the host
+// and the settings only -- never the credential, nor its length or any digest.
+func (c *ClusterConfig) warnWinRMPasswordExposure() {
+	if c.Spec == nil {
+		return
+	}
+
+	for _, h := range c.Spec.Hosts {
+		if h == nil || h.WinRM == nil {
+			continue
+		}
+		wrm := h.WinRM
+		if wrm.Password == "" {
+			continue
+		}
+
+		// No message-level protection either: NTLM negotiates its own
+		// encryption, so it is not cleartext even over plain HTTP.
+		if !wrm.UseHTTPS && !wrm.UseNTLM {
+			log.Warnf(
+				"%s: winRM.useHTTPS is false, so the WinRM password is sent in clear. "+
+					"It is base64-encoded in an HTTP Authorization header, which is encoding, not encryption, "+
+					"and anyone on the network path can read it along with every command sent to this host. "+
+					"Set winRM.useHTTPS and give the host a certificate your client trusts "+
+					"(see docs/usage/getting-started.md, \"WinRM over TLS\").",
+				wrm.Address,
+			)
+
+			continue
+		}
+
+		// The message cannot tell whether the password was inlined or came
+		// from an environment variable: substitution happens at
+		// pkg/config/config.go:79, before the config is parsed, so the field
+		// is populated identically either way. It therefore leads with the
+		// certificate problem, which is always actionable, and mentions
+		// sourcing the password from the environment as secondary advice
+		// rather than implying the user has inlined it.
+		if wrm.UseHTTPS && wrm.Insecure {
+			log.Warnf(
+				"%s: winRM.insecure is true, so the host's TLS certificate is not verified. "+
+					"An on-path attacker can impersonate this host, capture the WinRM password and read every command sent to it. "+
+					"Remove winRM.insecure once the host presents a trusted certificate (pin it with winRM.caCertPath), "+
+					"and source the password from the environment rather than inlining it "+
+					"(see docs/usage/getting-started.md, \"Keeping credentials out of launchpad.yaml\").",
+				wrm.Address,
+			)
+		}
+	}
+}
+
 func isValidMCRChannel(fl validator.FieldLevel) bool {
 	return common.IsValidMCRChannel(fl.Field().String())
 }