Skip to content

Commit 4e2cb80

Browse files
lexfreisircthulhu
andcommitted
chore(deps): drop cozystack/talos fork for upstream v1.13.7
The fork existed solely to carry the --skip-verify flag, which upstream declined (siderolabs/talos#12652) over the risk of leaking data to an actor impersonating the Talos API. Reimplement the flag locally in pkg/commands: a package-level SkipVerify bound to --skip-verify, and a WithClientSkipVerify that opens the talosconfig, builds a TLS config skipping server-certificate verification while preserving client-cert authentication, and dials the node directly. With the flag no longer depending on fork-only API, the replace directive is dropped and talm tracks stock upstream Talos v1.13.7. The rendered machine-config is unchanged — golden snapshots stay byte-identical; only the client-connection path is affected. Co-authored-by: Kirill Ilin <[email protected]> Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent e13774b commit 4e2cb80

10 files changed

Lines changed: 852 additions & 257 deletions

File tree

‎docs/manual-test-plan.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -986,6 +986,28 @@ Expected (per node): `node 192.0.2.10: talm: drift verification unavailable on m
986986

987987
Regression anchor: a refactor that always-prefixes (`node : talm: ...` on single-node) is a UX regression. The `nodePrefix("")` helper must collapse to empty for the bare-line single-node case.
988988

989+
### D4. TLS `--skip-verify` — native honored, wrapped passthrough warned
990+
991+
`--skip-verify` skips server-certificate verification while preserving client-certificate authentication, for connecting to a node whose IP is absent from the server cert SANs. It is reimplemented in talm (`WithClientSkipVerify` in `pkg/commands/root.go`) now that the cozystack/talos fork is dropped, and its coverage differs by command kind.
992+
993+
Native command (apply / template / upgrade / rotate-ca) — the flag is honored. Run against a node reachable by an IP that is NOT in the server cert SANs:
994+
995+
```bash
996+
talm apply --dry-run --skip-verify --nodes 192.0.2.10 -f nodes/node0.yaml
997+
```
998+
999+
Expected: the connection succeeds (no `x509: certificate is valid for … not 192.0.2.10` TLS SAN error) and the dry-run render proceeds.
1000+
1001+
Wrapped talosctl passthrough command (health / get / dashboard / …) — the flag is NOT supported, and the command says so instead of failing opaquely:
1002+
1003+
```bash
1004+
talm health --skip-verify --nodes 192.0.2.10
1005+
```
1006+
1007+
Expected (stderr): `Warning: --skip-verify is not supported for the wrapped talosctl command "health"; it applies only to talm-native commands (apply, template, upgrade, rotate-ca). Connecting with full TLS verification.` — the command then connects with full verification (and fails against an out-of-SAN IP, as it would without the flag).
1008+
1009+
Regression anchor: `pkg/commands/skip_verify_test.go` pins the TLS config (skip server verify + client cert preserved), the native routing (`WithClientNoNodes` / `WithClient` reach `WithClientSkipVerify`), the `--cluster` and dial-option threading, and the passthrough warning. A change that silently no-ops `--skip-verify` on native commands, or drops the passthrough warning, is a regression.
1010+
9891011
## E. Upgrade
9901012

9911013
### E1. Stage an upgrade to the same image

‎go.mod‎

Lines changed: 65 additions & 83 deletions
Large diffs are not rendered by default.

‎go.sum‎

Lines changed: 122 additions & 153 deletions
Large diffs are not rendered by default.

‎main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func registerRootFlags(cmd *cobra.Command) {
136136
cmd.PersistentFlags().StringSliceVar(&commands.GlobalArgs.Nodes, "nodes", []string{}, "target the specified nodes")
137137
cmd.PersistentFlags().StringSliceVarP(&commands.GlobalArgs.Endpoints, "endpoints", "e", []string{}, "override default endpoints in Talos configuration")
138138
cmd.PersistentFlags().StringVar(&commands.GlobalArgs.Cluster, "cluster", "", "Cluster to connect to if a proxy endpoint is used.")
139-
cmd.PersistentFlags().BoolVar(&commands.GlobalArgs.SkipVerify, "skip-verify", false, "skip TLS certificate verification (keeps client authentication)")
139+
cmd.PersistentFlags().BoolVar(&commands.SkipVerify, "skip-verify", false, "skip TLS certificate verification (keeps client authentication)")
140140
cmd.PersistentFlags().Bool("version", false, "Print the version number of the application")
141141
// No backticks in this usage string: pflag's UnquoteUsage treats the
142142
// first backtick-quoted word as the flag's value-placeholder name, which

‎pkg/commands/apply.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -797,7 +797,7 @@ func withApplyClientBare(action func(ctx context.Context, c *client.Client) erro
797797
return WithClientMaintenance(applyCmdFlags.certFingerprints, action)
798798
}
799799

800-
if GlobalArgs.SkipVerify {
800+
if SkipVerify {
801801
return WithClientSkipVerify(action)
802802
}
803803

‎pkg/commands/root.go‎

Lines changed: 147 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,12 @@ package commands
1616

1717
import (
1818
"context"
19+
"crypto/tls"
20+
"encoding/base64"
1921
"os"
22+
"os/signal"
2023
"path/filepath"
24+
"syscall"
2125
"time"
2226

2327
"github.com/cockroachdb/errors"
@@ -27,13 +31,67 @@ import (
2731

2832
"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/global"
2933
"github.com/siderolabs/talos/pkg/machinery/client"
34+
clientconfig "github.com/siderolabs/talos/pkg/machinery/client/config"
3035
)
3136

3237
// GlobalArgs is the common arguments for the root command.
3338
//
3439
//nolint:gochecknoglobals // cobra CLI architecture: persistent flags bind to package-level state shared across all subcommands; refactoring out the global would require threading state through every command's RunE.
3540
var GlobalArgs global.Args
3641

42+
// SkipVerify, when set via --skip-verify, disables TLS certificate verification
43+
// while preserving client-certificate authentication. Upstream global.Args has no
44+
// such field (the cozystack/talos fork added it for siderolabs/talos#12652, which
45+
// upstream declined); it is reimplemented here so talm can drop the fork and track
46+
// stock upstream Talos.
47+
//
48+
//nolint:gochecknoglobals // cobra CLI architecture: persistent flag binds to package-level state.
49+
var SkipVerify bool
50+
51+
// errContextNotFound is returned when the requested context is absent from the
52+
// talosconfig.
53+
var errContextNotFound = errors.New("context not found in talosconfig")
54+
55+
// signalContext returns a context cancelled on SIGINT/SIGTERM so a --skip-verify
56+
// client connection can be interrupted cleanly, mirroring talosctl's own wrappers.
57+
func signalContext() (context.Context, context.CancelFunc) {
58+
return signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
59+
}
60+
61+
// skipVerifyTLSConfig builds a TLS config that skips server-certificate
62+
// verification while preserving client-certificate authentication taken from the
63+
// talosconfig context.
64+
func skipVerifyTLSConfig(configContext *clientconfig.Context) (*tls.Config, error) {
65+
// InsecureSkipVerify is the whole point of --skip-verify (connect to nodes
66+
// whose IP is absent from the cert SANs); client-cert auth is preserved below.
67+
tlsConfig := &tls.Config{
68+
InsecureSkipVerify: true, //nolint:gosec // intentional: --skip-verify
69+
}
70+
71+
if configContext.Crt == "" || configContext.Key == "" {
72+
return tlsConfig, nil
73+
}
74+
75+
crtBytes, err := base64.StdEncoding.DecodeString(configContext.Crt)
76+
if err != nil {
77+
return nil, errors.Wrap(err, "decoding client certificate from talosconfig context")
78+
}
79+
80+
keyBytes, err := base64.StdEncoding.DecodeString(configContext.Key)
81+
if err != nil {
82+
return nil, errors.Wrap(err, "decoding client key from talosconfig context")
83+
}
84+
85+
cert, err := tls.X509KeyPair(crtBytes, keyBytes)
86+
if err != nil {
87+
return nil, errors.Wrap(err, "loading client key pair from talosconfig context")
88+
}
89+
90+
tlsConfig.Certificates = []tls.Certificate{cert}
91+
92+
return tlsConfig, nil
93+
}
94+
3795
// Config is the package-level configuration populated from Chart.yaml and
3896
// CLI persistent flags. Mirrors GlobalArgs for project-root-relative path
3997
// resolution across every subcommand.
@@ -84,7 +142,18 @@ var Config struct {
84142
// WithClientNoNodes wraps common code to initialize Talos client and provide cancellable context.
85143
//
86144
// WithClientNoNodes doesn't set any node information on the request context.
145+
//
146+
// This is the single choke point every talm-native command funnels through
147+
// (directly or via WithClient), so routing --skip-verify here restores the
148+
// coverage the dropped cozystack/talos fork used to provide at the library
149+
// level for the whole CLI. Wrapped talosctl passthrough commands run upstream
150+
// RunE code that never reaches this function, so they are handled separately
151+
// (and cannot honor --skip-verify without the fork — see talosctl_wrapper.go).
87152
func WithClientNoNodes(action func(context.Context, *client.Client) error, dialOptions ...grpc.DialOption) error {
153+
if SkipVerify {
154+
return WithClientSkipVerify(action, dialOptions...)
155+
}
156+
88157
//nolint:wrapcheck // thin pass-through to talos global.Args; error already carries Talos context
89158
return GlobalArgs.WithClientNoNodes(action, dialOptions...)
90159
}
@@ -119,22 +188,90 @@ func WithClientMaintenance(enforceFingerprints []string, action func(context.Con
119188
return GlobalArgs.WithClientMaintenance(enforceFingerprints, action)
120189
}
121190

191+
// skipVerifyClientOptions assembles the client options for a --skip-verify
192+
// connection. It mirrors upstream global.Args.WithClientNoNodes so the skip
193+
// path does not silently lose behavior the normal path has: it pins the
194+
// already-resolved config context (so client.GetConfigContext honors
195+
// --talosconfig / --context instead of falling back to the default config),
196+
// forwards caller dial options, threads the --cluster proxy override when set,
197+
// and selects endpoints from flags or the talosconfig context — all carrying
198+
// the verification-skipping TLS config built from that context.
199+
func skipVerifyClientOptions(configContext *clientconfig.Context, tlsConfig *tls.Config, dialOptions []grpc.DialOption) []client.OptionFunc {
200+
opts := []client.OptionFunc{
201+
client.WithConfigContext(configContext),
202+
client.WithTLSConfig(tlsConfig),
203+
client.WithDefaultGRPCDialOptions(),
204+
// Kept for structural parity with upstream WithClientNoNodes. It is
205+
// inert on the skip-verify path — getConn short-circuits on the explicit
206+
// TLS config before the SideroV1 interceptor — so it never actually
207+
// consumes the keys dir here; skip-verify + Omni SaaS-key auth is not a
208+
// real combination.
209+
client.WithSideroV1KeysDir(clientconfig.CustomSideroV1KeysDirPath(GlobalArgs.SideroV1KeysDir)),
210+
}
211+
212+
if len(dialOptions) > 0 {
213+
opts = append(opts, client.WithGRPCDialOptions(dialOptions...))
214+
}
215+
216+
// Preserve the --cluster proxy header the non-skip path sets, so
217+
// `--skip-verify --cluster X` still reaches nodes behind an Omni/Sidero proxy.
218+
if GlobalArgs.Cluster != "" {
219+
opts = append(opts, client.WithCluster(GlobalArgs.Cluster))
220+
}
221+
222+
if len(GlobalArgs.Endpoints) > 0 {
223+
opts = append(opts, client.WithEndpoints(GlobalArgs.Endpoints...))
224+
} else if len(configContext.Endpoints) > 0 {
225+
opts = append(opts, client.WithEndpoints(configContext.Endpoints...))
226+
}
227+
228+
return opts
229+
}
230+
122231
// WithClientSkipVerify wraps common code to initialize Talos client with TLS verification disabled
123232
// but with client certificate authentication preserved.
124233
// This is useful when connecting to nodes via IP addresses not listed in the server certificate's SANs.
125-
func WithClientSkipVerify(action func(context.Context, *client.Client) error) error {
126-
//nolint:wrapcheck // thin pass-through to talos global.Args; error already carries Talos context
127-
return GlobalArgs.WithClientSkipVerify(action)
128-
}
234+
func WithClientSkipVerify(action func(context.Context, *client.Client) error, dialOptions ...grpc.DialOption) error {
235+
ctx, stop := signalContext()
236+
defer stop()
129237

130-
// WithClientAuto automatically selects the appropriate client wrapper based on GlobalArgs.SkipVerify.
131-
// If SkipVerify is true, uses WithClientSkipVerify, otherwise uses WithClientNoNodes.
132-
func WithClientAuto(action func(context.Context, *client.Client) error) error {
133-
if GlobalArgs.SkipVerify {
134-
return WithClientSkipVerify(action)
238+
cfg, err := clientconfig.Open(GlobalArgs.Talosconfig)
239+
if err != nil {
240+
return errors.Wrapf(err, "opening talosconfig %q", GlobalArgs.Talosconfig)
241+
}
242+
243+
contextName := GlobalArgs.CmdContext
244+
if contextName == "" {
245+
contextName = cfg.Context
246+
}
247+
248+
configContext, ok := cfg.Contexts[contextName]
249+
if !ok {
250+
//nolint:wrapcheck // cockroachdb/errors.WithHint at boundary.
251+
return errors.WithHint(
252+
errors.Wrapf(errContextNotFound, "%q", contextName),
253+
"verify the context name against `talosctl config contexts`",
254+
)
255+
}
256+
257+
tlsConfig, err := skipVerifyTLSConfig(configContext)
258+
if err != nil {
259+
return err
135260
}
136261

137-
return WithClientNoNodes(action)
262+
c, err := client.New(ctx, skipVerifyClientOptions(configContext, tlsConfig, dialOptions)...)
263+
if err != nil {
264+
return errors.Wrap(err, "constructing Talos client")
265+
}
266+
defer func() { _ = c.Close() }()
267+
268+
// Deliberately no client.WithNodes here: this is the skip-verify backing
269+
// for the no-nodes constructors (WithClientNoNodes, withApplyClientBare),
270+
// mirroring upstream where WithClientNoNodes never sets node metadata.
271+
// Callers that want nodes (WithClient, the per-node apply loop) inject
272+
// them in their own wrapper layer. Injecting here would attach a plural
273+
// `nodes` key that apid's director rejects for COSI reads (e.g. rotate-ca).
274+
return action(ctx, c)
138275
}
139276

140277
// Commands is a list of commands published by the package.

‎pkg/commands/rotate_ca_handler.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ func discoverClusterNodes() (controlPlane, workers []string, err error) {
312312
func getKubeconfigFromTalos() ([]byte, error) {
313313
var kubeconfigData []byte
314314

315-
err := GlobalArgs.WithClient(func(ctx context.Context, c *client.Client) error {
315+
err := WithClient(func(ctx context.Context, c *client.Client) error {
316316
var err error
317317

318318
kubeconfigData, err = c.Kubeconfig(ctx)
@@ -323,7 +323,6 @@ func getKubeconfigFromTalos() ([]byte, error) {
323323
return nil
324324
})
325325

326-
//nolint:wrapcheck // forwarding talos/cobra error verbatim per the wrapper contract.
327326
return kubeconfigData, err
328327
}
329328

0 commit comments

Comments
 (0)