From a609277cf5e17e13b8a7b963a41bb2c86592bddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 6 Feb 2026 22:52:25 +0100 Subject: [PATCH 01/18] Add image/pkg/cli/basetls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/docs/containers-tls-details.yaml.5.md | 84 +++++++ image/pkg/cli/basetls/basetls.go | 219 ++++++++++++++++++ image/pkg/cli/basetls/basetls_test.go | 138 +++++++++++ .../tlsdetails/testdata/all-fields.yaml | 8 + .../tlsdetails/testdata/empty-fields.yaml | 3 + .../basetls/tlsdetails/testdata/empty.yaml | 1 + .../tlsdetails/testdata/invalid-values.yaml | 5 + .../pkg/cli/basetls/tlsdetails/tlsdetails.go | 59 +++++ .../cli/basetls/tlsdetails/tlsdetails_test.go | 114 +++++++++ 9 files changed, 631 insertions(+) create mode 100644 image/docs/containers-tls-details.yaml.5.md create mode 100644 image/pkg/cli/basetls/basetls.go create mode 100644 image/pkg/cli/basetls/basetls_test.go create mode 100644 image/pkg/cli/basetls/tlsdetails/testdata/all-fields.yaml create mode 100644 image/pkg/cli/basetls/tlsdetails/testdata/empty-fields.yaml create mode 100644 image/pkg/cli/basetls/tlsdetails/testdata/empty.yaml create mode 100644 image/pkg/cli/basetls/tlsdetails/testdata/invalid-values.yaml create mode 100644 image/pkg/cli/basetls/tlsdetails/tlsdetails.go create mode 100644 image/pkg/cli/basetls/tlsdetails/tlsdetails_test.go diff --git a/image/docs/containers-tls-details.yaml.5.md b/image/docs/containers-tls-details.yaml.5.md new file mode 100644 index 0000000000..a5fcfc9d5b --- /dev/null +++ b/image/docs/containers-tls-details.yaml.5.md @@ -0,0 +1,84 @@ +% CONTAINERS-TLS-DETAILS.YAML 5 container-libs TLS details file format +% Miloslav Trmač +% February 2026 + +# NAME +containers-tls-details.yaml - syntax for the container-libs TLS details parameter file + +# DESCRIPTION + +The TLS details parameter file is accepted by various projects using the go.podman.io/* libraries. +There is no default location for these files; they are user-managed, and a path is provided on the CLI, +e.g. `skopeo --tls-details=`_details-file_`.yaml copy …`. + +# WARNINGS + +The `--tls-details` options, and this file format, should only rarely be used. +If this mechanism is not used, the software is expected to use appropriate defaults which will vary over time, +depending on version of the software, version of the Go standard library, +or platform’s configuration (e.g. `GODEBUG` values; or, not as of early 2026, but potentially, **crypto-policies**(7)). + +These options _only_ affect the programs which provide the `--tls-details` option; +they don't affect other executables (e.g. **git**(1), **ssh**(1)) that may be executed internally to perform another operation. + +There are some known gaps in the implementation of these options. +We hope to fix that over time, but in the meantime, careful testing feature by feature is recommended. +Known gaps include network operations performed while creating sigstore signatures (communicating with Rekor, OIDC servers, Fulcio). + +# FORMAT + +The TLS details files use YAML. All fields are optional. + +- `minVersion` + + The minimum TLS version to use throughout the program. + If not set, defaults to a reasonable default that may change over time. + + Users should generally not use this option and hard-code a version unless they have a process + to ensure that the value will be kept up to date. + +- `cipherSuites` + + The allowed TLS cipher suites to use throughout the program. + The value is an array of IANA TLS Cipher Suites names. + + If not set, defaults to a reasonable default that may change over time; + if set to an empty array, prohibits using all cipher suites. + + **Warning:** Almost no-one should ever use this option. + Use it only if you have a bureaucracy that requires a specific list, + and if you are confident that this bureaucracy will still exist, + and will bring you an updated list when necessary, + many years from now. + + **Warning:** The effectiveness of this option is limited by capabilities of the Go standard library; + e.g., as of Go 1.25, it is not possible to change which cipher suites are used in TLS 1.3. + +- `namedGroups` + + The allowed TLS named groups to use throughout the program. + The value is an array of IANA TLS Supported Groups names. + + If not set, defaults to a reasonable default that may change over time. + + **Warning:** Almost no-one should ever use this option. + Use it only if you have a bureaucracy that requires a specific list, + and if you are confident that this bureaucracy will still exist, + and will bring you an updated list when necessary, + many years from now. + +# EXAMPLE + +```yaml +minVersion: "1.2" +cipherSuites: + - "TLS_AES_128_GCM_SHA256" + - "TLS_CHACHA20_POLY1305_SHA256" +namedGroups: + - "secp256r1" + - "secp384r1" + - "x25519" +``` + +# SEE ALSO +buildah(1), podman(1), skopeo(1) diff --git a/image/pkg/cli/basetls/basetls.go b/image/pkg/cli/basetls/basetls.go new file mode 100644 index 0000000000..1558e8f4ea --- /dev/null +++ b/image/pkg/cli/basetls/basetls.go @@ -0,0 +1,219 @@ +// Package basetls encapsulates a set of base TLS settings (not keys/certificates) +// configured via containers-tls-details.yaml(5). +// +// CLI integration should generally be done using c/image/pkg/cli/basetls/tlsdetails instead +// of using the TLSDetailsFile directly. +package basetls + +import ( + "bytes" + "crypto/tls" + "encoding/json" + "errors" + "fmt" + "slices" +) + +// Config encapsulates user’s choices about base TLS settings, typically +// configured via containers-tls-details.yaml(5). +// +// Most codebases should pass around the resulting *tls.Config, without depending on this subpackage; +// this primarily exists as a separate type to allow passing the configuration around within (version-matched) RPC systems, +// using the MarshalText/UnmarshalText methods. +type Config struct { + // We keep the text representation because we start with it, and this way we don't have + // to implement formatting back to text. This is an internal detail, so we can change that later. + text TLSDetailsFile + config *tls.Config // Parsed from .text, both match +} + +// TLSDetailsFile contains a set of TLS options. +// +// To consume such a file, most callers should use c/image/pkg/cli/basetls/tlsdetails instead +// of dealing with this type explicitly. +// +// This type is exported primarily to allow creating parameter files programmatically +// (and eventually the tlsdetails subpackage should provide an API to convert this type into +// the appropriate file contents, so that callers don't need to do that manually). +type TLSDetailsFile struct { + // Keep this in sync with docs/containers-tls-details.yaml.5.md ! + + MinVersion string `yaml:"minVersion,omitempty"` // If set, minimum version to use throughout the program. + CipherSuites []string `yaml:"cipherSuites,omitempty"` // If set, allowed TLS cipher suites to use throughout the program. + NamedGroups []string `yaml:"namedGroups,omitempty"` // If set, allowed TLS named groups to use throughout the program. +} + +// NewFromTLSDetails creates a Config from a TLSDetailsFile. +func NewFromTLSDetails(details *TLSDetailsFile) (*Config, error) { + res := Config{ + text: TLSDetailsFile{}, + config: &tls.Config{}, + } + configChanged := false + for _, fn := range []func(input *TLSDetailsFile) (bool, error){ + res.parseMinVersion, + res.parseCipherSuites, + res.parseNamedGroups, + } { + changed, err := fn(details) + if err != nil { + return nil, err + } + if changed { + configChanged = true + } + } + + if !configChanged { + res.config = nil + } + return &res, nil +} + +// tlsVersions maps TLS version strings to their crypto/tls constants. +// We could use the `tls.VersionName` names, but those are verbose and contain spaces; +// similarly the OpenShift enum values (“VersionTLS11”) are unergonomic. +var tlsVersions = map[string]uint16{ + "1.0": tls.VersionTLS10, + "1.1": tls.VersionTLS11, + "1.2": tls.VersionTLS12, + "1.3": tls.VersionTLS13, +} + +func (c *Config) parseMinVersion(input *TLSDetailsFile) (bool, error) { + if input.MinVersion == "" { + return false, nil + } + v, ok := tlsVersions[input.MinVersion] + if !ok { + return false, fmt.Errorf("unrecognized TLS minimum version %q", input.MinVersion) + } + c.text.MinVersion = input.MinVersion + c.config.MinVersion = v + return true, nil +} + +// cipherSuitesByName returns a map from cipher suite name to its ID. +func cipherSuitesByName() map[string]uint16 { + // The Go standard library uses IANA names and already contains the mapping (for relevant values) + // sadly we still need to turn it into a lookup map. + suites := make(map[string]uint16) + for _, cs := range tls.CipherSuites() { + suites[cs.Name] = cs.ID + } + for _, cs := range tls.InsecureCipherSuites() { + suites[cs.Name] = cs.ID + } + return suites +} + +func (c *Config) parseCipherSuites(input *TLSDetailsFile) (bool, error) { + if input.CipherSuites == nil { + return false, nil + } + suitesByName := cipherSuitesByName() + ids := []uint16{} + for _, name := range input.CipherSuites { + id, ok := suitesByName[name] + if !ok { + return false, fmt.Errorf("unrecognized TLS cipher suite %q", name) + } + ids = append(ids, id) + } + c.text.CipherSuites = slices.Clone(input.CipherSuites) + c.config.CipherSuites = ids + return true, nil +} + +// groupsByName maps curve/group names to their tls.CurveID. +// The names match IANA TLS Supported Groups registry. +// +// Yes, the x25519 names differ in capitalization. +// Go’s tls.CurveID has a .String() method, but it +// uses the Go names. +var groupsByName = map[string]tls.CurveID{ + "secp256r1": tls.CurveP256, + "secp384r1": tls.CurveP384, + "secp521r1": tls.CurveP521, + "x25519": tls.X25519, + "X25519MLKEM768": tls.X25519MLKEM768, +} + +func (c *Config) parseNamedGroups(input *TLSDetailsFile) (bool, error) { + if input.NamedGroups == nil { + return false, nil + } + ids := []tls.CurveID{} + for _, name := range input.NamedGroups { + id, ok := groupsByName[name] + if !ok { + return false, fmt.Errorf("unrecognized TLS named group %q", name) + } + ids = append(ids, id) + } + c.text.NamedGroups = slices.Clone(input.NamedGroups) + c.config.CurvePreferences = ids + return true, nil +} + +// TLSConfig returns a *tls.Config matching the provided settings. +// If c contains no settings, it returns nil. +// Otherwise, the returned *tls.Config is freshly allocated and the caller can modify it as needed. +func (c *Config) TLSConfig() *tls.Config { + if c.config == nil { + return nil + } + return c.config.Clone() +} + +// marshaledSerialization is the data we use in MarshalText/UnmarshalText, +// marshaled using JSON. +// +// Note that the file format is using YAML, but we use JSON, to minimize dependencies +// in backend code where we don't need comments and the brackets are not annoying users. +type marshaledSerialization struct { + Version int + Data TLSDetailsFile +} + +const marshaledSerializationVersion1 = 1 + +// MarshalText serializes c to a text representation. +// +// The representation is intended to be reasonably stable across updates to c/image, +// but the consumer must not be older than the producer. +func (c Config) MarshalText() ([]byte, error) { + data := marshaledSerialization{ + Version: marshaledSerializationVersion1, + Data: c.text, + } + return json.Marshal(data) +} + +// UnmarshalText parses the output of MarshalText. +// +// The format is otherwise undocumented and we do not promise ongoing compatibility with producers external to this package. +func (c *Config) UnmarshalText(text []byte) error { + var data marshaledSerialization + + // In the future, this should be an even stricter parser, e.g. refusing duplicate fields + // and requiring a case-sensitive field name match. + decoder := json.NewDecoder(bytes.NewReader(text)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&data); err != nil { + return err + } + if decoder.More() { + return errors.New("unexpected extra data after a JSON object") + } + + if data.Version != marshaledSerializationVersion1 { + return fmt.Errorf("unsupported version %d", data.Version) + } + v, err := NewFromTLSDetails(&data.Data) + if err != nil { + return err + } + *c = *v + return nil +} diff --git a/image/pkg/cli/basetls/basetls_test.go b/image/pkg/cli/basetls/basetls_test.go new file mode 100644 index 0000000000..bb1840a4ea --- /dev/null +++ b/image/pkg/cli/basetls/basetls_test.go @@ -0,0 +1,138 @@ +package basetls + +import ( + "crypto/tls" + "encoding" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var ( + _ encoding.TextMarshaler = Config{} + _ encoding.TextMarshaler = (*Config)(nil) + _ encoding.TextUnmarshaler = (*Config)(nil) +) + +func TestNewFromTLSDetails(t *testing.T) { + // This tests both the initial parsing, and marshaling+unmarshaling. + for _, tc := range []struct { + details TLSDetailsFile + expected *tls.Config + }{ + { + details: TLSDetailsFile{MinVersion: "1.2", CipherSuites: []string{"TLS_AES_128_GCM_SHA256", "TLS_CHACHA20_POLY1305_SHA256"}, NamedGroups: []string{"secp256r1", "secp384r1", "x25519"}}, + expected: &tls.Config{MinVersion: tls.VersionTLS12, CipherSuites: []uint16{tls.TLS_AES_128_GCM_SHA256, tls.TLS_CHACHA20_POLY1305_SHA256}, CurvePreferences: []tls.CurveID{tls.CurveP256, tls.CurveP384, tls.X25519}}, + }, + } { + baseTLS, err := NewFromTLSDetails(&tc.details) + require.NoError(t, err) + assert.Equal(t, tc.expected, baseTLS.TLSConfig()) + + marshaled, err := baseTLS.MarshalText() + require.NoError(t, err) + var unmarshaled Config + err = unmarshaled.UnmarshalText(marshaled) + require.NoError(t, err) + assert.Equal(t, tc.expected, unmarshaled.TLSConfig()) + + marshaled2, err := unmarshaled.MarshalText() + require.NoError(t, err) + assert.Equal(t, marshaled, marshaled2) // NOT an API promise, and this assumes JSON marshaling is deterministic + var unmarshaled2 Config + err = unmarshaled2.UnmarshalText(marshaled2) + require.NoError(t, err) + assert.Equal(t, baseTLS.TLSConfig(), unmarshaled2.TLSConfig()) + } +} + +func TestBaseTLSParseMinVersion(t *testing.T) { + for _, tc := range []struct { + version string + expected *tls.Config + expectError bool + }{ + {"", nil, false}, + {"1.0", &tls.Config{MinVersion: tls.VersionTLS10}, false}, + {"1.1", &tls.Config{MinVersion: tls.VersionTLS11}, false}, + {"1.2", &tls.Config{MinVersion: tls.VersionTLS12}, false}, + {"1.3", &tls.Config{MinVersion: tls.VersionTLS13}, false}, + {"this_is_invalid", nil, true}, + } { + baseTLS, err := NewFromTLSDetails(&TLSDetailsFile{MinVersion: tc.version}) + if tc.expectError { + assert.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tc.expected, baseTLS.TLSConfig()) + } + } +} + +func TestBaseTLSParseCipherSuites(t *testing.T) { + validSuites := tls.CipherSuites() + require.True(t, len(validSuites) >= 2) + suite0, suite1 := validSuites[0], validSuites[1] + + for _, tc := range []struct { + suites []string + expected *tls.Config + expectError bool + }{ + {nil, nil, false}, // empty + {[]string{}, &tls.Config{CipherSuites: []uint16{}}, false}, // no cipher suites + {[]string{suite0.Name}, &tls.Config{CipherSuites: []uint16{suite0.ID}}, false}, + {[]string{suite0.Name, suite1.Name}, &tls.Config{CipherSuites: []uint16{suite0.ID, suite1.ID}}, false}, + {[]string{"this_is_invalid"}, nil, true}, + } { + baseTLS, err := NewFromTLSDetails(&TLSDetailsFile{CipherSuites: tc.suites}) + if tc.expectError { + assert.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tc.expected, baseTLS.TLSConfig()) + } + } +} + +func TestBaseTLSParseNamedGroups(t *testing.T) { + for _, tc := range []struct { + groups []string + expected *tls.Config + expectError bool + }{ + {nil, nil, false}, // empty + {[]string{}, &tls.Config{CurvePreferences: []tls.CurveID{}}, false}, // no named groups + {[]string{"x25519"}, &tls.Config{CurvePreferences: []tls.CurveID{tls.X25519}}, false}, + {[]string{"secp256r1"}, &tls.Config{CurvePreferences: []tls.CurveID{tls.CurveP256}}, false}, + {[]string{"secp384r1"}, &tls.Config{CurvePreferences: []tls.CurveID{tls.CurveP384}}, false}, + {[]string{"secp521r1"}, &tls.Config{CurvePreferences: []tls.CurveID{tls.CurveP521}}, false}, + {[]string{"x25519", "secp256r1"}, &tls.Config{CurvePreferences: []tls.CurveID{tls.X25519, tls.CurveP256}}, false}, + {[]string{"this_is_invalid"}, nil, true}, + } { + baseTLS, err := NewFromTLSDetails(&TLSDetailsFile{NamedGroups: tc.groups}) + if tc.expectError { + assert.Error(t, err) + } else { + require.NoError(t, err) + assert.Equal(t, tc.expected, baseTLS.TLSConfig()) + } + } +} + +func TestBaseTLSUnmarshalText(t *testing.T) { + // General correctness and round-trip safety is tested in TestNewFromTLSDetails. + for _, json := range []string{ + `¬ a valid JSON`, + `{}`, // missing version + `{"Version": 9999, "Data":{}}`, + `{"Version": 1, "Data":{}}{"Version": 1, "Data":{}}`, + `{"Version": 1, "Data":{ "MinVersion": "this_is_invalid" }}`, + } { + var baseTLS Config + err := baseTLS.UnmarshalText([]byte(json)) + assert.Error(t, err, json) + t.Logf("error: %v", err) + } +} diff --git a/image/pkg/cli/basetls/tlsdetails/testdata/all-fields.yaml b/image/pkg/cli/basetls/tlsdetails/testdata/all-fields.yaml new file mode 100644 index 0000000000..8a63ffde73 --- /dev/null +++ b/image/pkg/cli/basetls/tlsdetails/testdata/all-fields.yaml @@ -0,0 +1,8 @@ +minVersion: "1.2" +cipherSuites: + - "TLS_AES_128_GCM_SHA256" + - "TLS_CHACHA20_POLY1305_SHA256" +namedGroups: + - "secp256r1" + - "secp384r1" + - "x25519" diff --git a/image/pkg/cli/basetls/tlsdetails/testdata/empty-fields.yaml b/image/pkg/cli/basetls/tlsdetails/testdata/empty-fields.yaml new file mode 100644 index 0000000000..7e8f36c1d8 --- /dev/null +++ b/image/pkg/cli/basetls/tlsdetails/testdata/empty-fields.yaml @@ -0,0 +1,3 @@ +minVersion: "" +cipherSuites: [] +namedGroups: [] diff --git a/image/pkg/cli/basetls/tlsdetails/testdata/empty.yaml b/image/pkg/cli/basetls/tlsdetails/testdata/empty.yaml new file mode 100644 index 0000000000..9e26dfeeb6 --- /dev/null +++ b/image/pkg/cli/basetls/tlsdetails/testdata/empty.yaml @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/image/pkg/cli/basetls/tlsdetails/testdata/invalid-values.yaml b/image/pkg/cli/basetls/tlsdetails/testdata/invalid-values.yaml new file mode 100644 index 0000000000..f22f919a83 --- /dev/null +++ b/image/pkg/cli/basetls/tlsdetails/testdata/invalid-values.yaml @@ -0,0 +1,5 @@ +minVersion: "this is not a valid version" +cipherSuites: + - "this is not a valid cipher suite" +namedGroups: + - "this is not a valid named group" diff --git a/image/pkg/cli/basetls/tlsdetails/tlsdetails.go b/image/pkg/cli/basetls/tlsdetails/tlsdetails.go new file mode 100644 index 0000000000..fca6bed917 --- /dev/null +++ b/image/pkg/cli/basetls/tlsdetails/tlsdetails.go @@ -0,0 +1,59 @@ +// Package tlsdetails implements the containers-tls-details.yaml(5) file format. +// +// Recommended CLI integration is by a --tls-details flag parsed using BaseTLSFromOptionalFile, with the following documentation: +// +// --tls-details is a path to a containers-tls-details.yaml(5) file, affecting TLS behavior throughout the program. +// +// If not set, defaults to a reasonable default that may change over time (depending on system’s global policy, +// version of the program, version of the Go language, and the like). +// +// Users should generally not use this option unless they have a process to ensure that the configuration will be kept up to date. +package tlsdetails + +import ( + "bytes" + "fmt" + "os" + + "go.podman.io/image/v5/pkg/cli/basetls" + "gopkg.in/yaml.v3" +) + +// BaseTLSFromOptionalFile returns a basetls.Config matching a containers-tls-details.yaml file at the specified path. +// If path is "", it returns a valid basetls.Config with no settings (where config.TLSConfig() will return nil). +func BaseTLSFromOptionalFile(path string) (*basetls.Config, error) { + if path == "" { + return basetls.NewFromTLSDetails(&basetls.TLSDetailsFile{}) + } + return BaseTLSFromFile(path) +} + +// BaseTLSFromFile returns a basetls.Config matching a containers-tls-details.yaml file at the specified path. +func BaseTLSFromFile(path string) (*basetls.Config, error) { + details, err := ParseFile(path) + if err != nil { + return nil, err + } + res, err := basetls.NewFromTLSDetails(details) + if err != nil { + return nil, fmt.Errorf("parsing TLS details %q: %w", path, err) + } + return res, nil +} + +// ParseFile parses a basetls.TLSDetailsFile at the specified path. +// +// Most consumers of the parameter file should use BaseTLSFromFile or BaseTLSFromOptionalFile instead. +func ParseFile(path string) (*basetls.TLSDetailsFile, error) { + var res basetls.TLSDetailsFile + source, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("reading %q: %w", path, err) + } + dec := yaml.NewDecoder(bytes.NewReader(source)) + dec.KnownFields(true) + if err = dec.Decode(&res); err != nil { + return nil, fmt.Errorf("parsing %q: %w", path, err) + } + return &res, nil +} diff --git a/image/pkg/cli/basetls/tlsdetails/tlsdetails_test.go b/image/pkg/cli/basetls/tlsdetails/tlsdetails_test.go new file mode 100644 index 0000000000..7af7cede7b --- /dev/null +++ b/image/pkg/cli/basetls/tlsdetails/tlsdetails_test.go @@ -0,0 +1,114 @@ +package tlsdetails + +import ( + "crypto/tls" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.podman.io/image/v5/pkg/cli/basetls" +) + +func TestBaseTLSFromOptionalFile(t *testing.T) { + for _, tc := range []struct { + path string + expected *tls.Config + }{ + {path: "", expected: nil}, + { + path: "testdata/all-fields.yaml", + expected: &tls.Config{ + MinVersion: tls.VersionTLS12, + CipherSuites: []uint16{tls.TLS_AES_128_GCM_SHA256, tls.TLS_CHACHA20_POLY1305_SHA256}, + CurvePreferences: []tls.CurveID{tls.CurveP256, tls.CurveP384, tls.X25519}, + }, + }, + {path: "testdata/empty.yaml", expected: nil}, + } { + baseTLS, err := BaseTLSFromOptionalFile(tc.path) + require.NoError(t, err) + require.NotNil(t, baseTLS) + assert.Equal(t, tc.expected, baseTLS.TLSConfig()) + } +} + +func TestBaseTLSFromFile(t *testing.T) { + for _, tc := range []struct { + path string + expected *tls.Config + }{ + { + path: "testdata/all-fields.yaml", + expected: &tls.Config{ + MinVersion: tls.VersionTLS12, + CipherSuites: []uint16{tls.TLS_AES_128_GCM_SHA256, tls.TLS_CHACHA20_POLY1305_SHA256}, + CurvePreferences: []tls.CurveID{tls.CurveP256, tls.CurveP384, tls.X25519}, + }, + }, + {path: "testdata/empty.yaml", expected: nil}, + } { + baseTLS, err := BaseTLSFromFile(tc.path) + require.NoError(t, err) + assert.Equal(t, tc.expected, baseTLS.TLSConfig()) + } + + for _, path := range []string{ + "/dev/null/this/does/not/exist", + "testdata/invalid-values.yaml", + } { + _, err := BaseTLSFromFile(path) + assert.Error(t, err, path) + t.Logf("error: %v", err) + } +} + +func TestParseFile(t *testing.T) { + // A minimal test of parsing; field validation and handling is a responsibility of pkg/cli/basetls. + for _, tc := range []struct { + path string + expected basetls.TLSDetailsFile + }{ + { + path: "testdata/all-fields.yaml", + expected: basetls.TLSDetailsFile{ + MinVersion: "1.2", + CipherSuites: []string{"TLS_AES_128_GCM_SHA256", "TLS_CHACHA20_POLY1305_SHA256"}, + NamedGroups: []string{"secp256r1", "secp384r1", "x25519"}, + }, + }, + { + path: "testdata/empty-fields.yaml", + expected: basetls.TLSDetailsFile{ + MinVersion: "", + CipherSuites: []string{}, + NamedGroups: []string{}, + }, + }, + { + path: "testdata/empty.yaml", + expected: basetls.TLSDetailsFile{}, + }, + } { + f, err := ParseFile(tc.path) + require.NoError(t, err) + assert.Equal(t, tc.expected, *f) + } + + _, err := ParseFile("/dev/null/this/does/not/exist") + assert.Error(t, err) + + for _, yaml := range []string{ + `minVersion: {}`, // not a string; (number-like text is auto-converted) + `cipherSuites: "not-an-array"`, // not a list of strings + `this-field-does-not-exist: true`, // unknown field + "minVersion: 1\nminVersion: 1.2", // duplicate field + } { + path := filepath.Join(t.TempDir(), "bad.yaml") + err := os.WriteFile(path, []byte(yaml), 0o600) + require.NoError(t, err) + _, err = ParseFile(path) + assert.Error(t, err, yaml) + } +} From 0b8be4bf5081dec9faf009f2058b4cce50983f07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 30 Jan 2026 16:21:24 +0100 Subject: [PATCH 02/18] Remove an obsolete comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "parsing again" was removed in commit 35256ec83e3ef89c20501abb6fffbf4bdcd664c3 . Should not change behavior. Signed-off-by: Miloslav Trmač --- image/openshift/openshift.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/image/openshift/openshift.go b/image/openshift/openshift.go index 8e73de9937..e8e894eade 100644 --- a/image/openshift/openshift.go +++ b/image/openshift/openshift.go @@ -30,12 +30,6 @@ type openshiftClient struct { // newOpenshiftClient creates a new openshiftClient for the specified reference. func newOpenshiftClient(ref openshiftReference) (*openshiftClient, error) { - // We have already done this parsing in ParseReference, but thrown away - // httpClient. So, parse again. - // (We could also rework/split restClientFor to "get base URL" to be done - // in ParseReference, and "get httpClient" to be done here. But until/unless - // we support non-default clusters, this is good enough.) - // Overall, this is modelled on openshift/origin/pkg/cmd/util/clientcmd.New().ClientConfig() and openshift/origin/pkg/client. cmdConfig := defaultClientConfig() logrus.Debugf("cmdConfig: %#v", cmdConfig) From c039d64cc6b26877882163e82ff1be9bb21cbf60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 30 Jan 2026 17:10:28 +0100 Subject: [PATCH 03/18] Update code references after the module rename MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- image/docker/daemon/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/image/docker/daemon/client.go b/image/docker/daemon/client.go index 004a95fe77..e5776904f8 100644 --- a/image/docker/daemon/client.go +++ b/image/docker/daemon/client.go @@ -83,7 +83,7 @@ func tlsConfig(sys *types.SystemContext) (*http.Client, error) { Transport: &http.Transport{ Proxy: http.ProxyFromEnvironment, TLSClientConfig: tlsc, - // In general we want to follow docker/daemon/client.defaultHTTPClient , as long as it doesn’t affect compatibility. + // In general we want to follow github.com/moby/moby/client.defaultHTTPClient , as long as it doesn’t affect compatibility. // These idle connection limits really only apply to long-running clients, which is not our case here; // we include the same values purely for symmetry. MaxIdleConns: 6, @@ -98,7 +98,7 @@ func httpConfig() *http.Client { Transport: &http.Transport{ Proxy: http.ProxyFromEnvironment, TLSClientConfig: nil, - // In general we want to follow docker/daemon/client.defaultHTTPClient , as long as it doesn’t affect compatibility. + // In general we want to follow github.com/moby/moby/client.defaultHTTPClient , as long as it doesn’t affect compatibility. // These idle connection limits really only apply to long-running clients, which is not our case here; // we include the same values purely for symmetry. MaxIdleConns: 6, From a195a3cf38b9e8fd54b12407b150b2b699ae2f29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 10 Feb 2026 01:01:07 +0100 Subject: [PATCH 04/18] Remove the testDir function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There's really no benefit to it. Should not change (test) behavior. Signed-off-by: Miloslav Trmač --- image/docker/daemon/client_test.go | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/image/docker/daemon/client_test.go b/image/docker/daemon/client_test.go index 74cd1eac55..cbbe7457ef 100644 --- a/image/docker/daemon/client_test.go +++ b/image/docker/daemon/client_test.go @@ -2,7 +2,6 @@ package daemon import ( "net/http" - "os" "path/filepath" "testing" @@ -23,11 +22,9 @@ func TestDockerClientFromNilSystemContext(t *testing.T) { } func TestDockerClientFromCertContext(t *testing.T) { - testDir := testDir(t) - host := "tcp://127.0.0.1:2376" systemCtx := &types.SystemContext{ - DockerDaemonCertPath: filepath.Join(testDir, "testdata", "certs"), + DockerDaemonCertPath: filepath.Join("testdata", "certs"), DockerDaemonHost: host, DockerDaemonInsecureSkipTLSVerify: true, } @@ -52,10 +49,8 @@ func TestTlsConfigFromInvalidCertPath(t *testing.T) { } func TestTlsConfigFromCertPath(t *testing.T) { - testDir := testDir(t) - ctx := &types.SystemContext{ - DockerDaemonCertPath: filepath.Join(testDir, "testdata", "certs"), + DockerDaemonCertPath: filepath.Join("testdata", "certs"), DockerDaemonInsecureSkipTLSVerify: true, } @@ -69,8 +64,6 @@ func TestTlsConfigFromCertPath(t *testing.T) { } func TestSkipTLSVerifyOnly(t *testing.T) { - // testDir := testDir(t) - ctx := &types.SystemContext{ DockerDaemonInsecureSkipTLSVerify: true, } @@ -98,11 +91,3 @@ func TestSpecifyPlainHTTPViaHostScheme(t *testing.T) { assert.Equal(t, host, client.DaemonHost()) assert.NoError(t, client.Close()) } - -func testDir(t *testing.T) string { - testDir, err := os.Getwd() - if err != nil { - t.Fatal("Unable to determine the current test directory") - } - return testDir -} From a7d525e1f085b42cd0a894947fd9113de5697652 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 2 Feb 2026 14:35:54 +0100 Subject: [PATCH 05/18] Delete the temporary file on error in download.FromURL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- common/pkg/download/download.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/common/pkg/download/download.go b/common/pkg/download/download.go index 777a115220..187af4a614 100644 --- a/common/pkg/download/download.go +++ b/common/pkg/download/download.go @@ -15,6 +15,12 @@ func FromURL(tmpdir, source string) (string, error) { return "", fmt.Errorf("creating temporary download file: %w", err) } defer tmp.Close() + succeeded := false + defer func() { + if !succeeded { + os.Remove(tmp.Name()) + } + }() response, err := http.Get(source) // nolint:noctx if err != nil { @@ -27,5 +33,6 @@ func FromURL(tmpdir, source string) (string, error) { return "", fmt.Errorf("copying %s to %s: %w", source, tmp.Name(), err) } + succeeded = true return tmp.Name(), nil } From 9a9048b2c8e0ee3c6dab1f9a8250cc2676afb594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 28 Jan 2026 19:31:33 +0100 Subject: [PATCH 06/18] Add SystemContext.BaseTLSConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/types/types.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/image/types/types.go b/image/types/types.go index de25dabcdc..0b53301ca9 100644 --- a/image/types/types.go +++ b/image/types/types.go @@ -2,6 +2,7 @@ package types import ( "context" + "crypto/tls" "io" "net/url" "time" @@ -619,6 +620,11 @@ type SystemContext struct { DockerArchiveAdditionalTags []reference.NamedTagged // If not "", overrides the temporary directory to use for storing big files BigFilesTemporaryDir string + // If not nil, may contain TLS _algorithm_ options (e.g. TLS version, cipher suites, “curves”, etc.) + // The effect of setting any other options (cryptographic keys, InsecureSkipTLSVerify, callbacks, etc.) is UNDEFINED, + // may be inconsistent in various use cases, and may change over time. + // Consumers of this value are expected to .Clone() the config and then apply other options. + BaseTLSConfig *tls.Config // === OCI.Transport overrides === // If not "", a directory containing a CA certificate (ending with ".crt"), From b11d876b60ac4d5f584dae30dd7f5d62faf3dea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 30 Jan 2026 14:34:35 +0100 Subject: [PATCH 07/18] Implement BaseTLS in c/image/docker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/docker/docker_client.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/image/docker/docker_client.go b/image/docker/docker_client.go index 2f257076f5..6b916e89cf 100644 --- a/image/docker/docker_client.go +++ b/image/docker/docker_client.go @@ -240,13 +240,18 @@ func newDockerClient(sys *types.SystemContext, registry, reference string) (*doc if registry == dockerHostname { registry = dockerRegistry } - tlsClientConfig := &tls.Config{ - // As of 2025-08, tlsconfig.ClientDefault() differs from Go 1.23 defaults only in CipherSuites; - // so, limit us to only using that value. If go-connections/tlsconfig changes its policy, we - // will want to consider that and make a decision whether to follow suit. - // There is some chance that eventually the Go default will be to require TLS 1.3, and that point - // we might want to drop the dependency on go-connections entirely. - CipherSuites: tlsconfig.ClientDefault().CipherSuites, + var tlsClientConfig *tls.Config + if sys != nil && sys.BaseTLSConfig != nil { + tlsClientConfig = sys.BaseTLSConfig.Clone() + } else { + tlsClientConfig = &tls.Config{ + // As of 2025-08, tlsconfig.ClientDefault() differs from Go 1.23 defaults only in CipherSuites; + // so, limit us to only using that value. If go-connections/tlsconfig changes its policy, we + // will want to consider that and make a decision whether to follow suit. + // There is some chance that eventually the Go default will be to require TLS 1.3, and that point + // we might want to drop the dependency on go-connections entirely. + CipherSuites: tlsconfig.ClientDefault().CipherSuites, + } } // It is undefined whether the host[:port] string for dockerHostname should be dockerHostname or dockerRegistry, From 4077ca298a2f56baf875c9f8104b90b6ad8e889f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 30 Jan 2026 14:42:17 +0100 Subject: [PATCH 08/18] Implement BaseTLSConfig in c/image/oci/layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/oci/layout/oci_src.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/image/oci/layout/oci_src.go b/image/oci/layout/oci_src.go index f265a21d70..69954c4f8c 100644 --- a/image/oci/layout/oci_src.go +++ b/image/oci/layout/oci_src.go @@ -51,13 +51,17 @@ type ociImageSource struct { // newImageSource returns an ImageSource for reading from an existing directory. func newImageSource(sys *types.SystemContext, ref ociReference) (private.ImageSource, error) { tr := tlsclientconfig.NewTransport() - tr.TLSClientConfig = &tls.Config{ - // As of 2025-08, tlsconfig.ClientDefault() differs from Go 1.23 defaults only in CipherSuites; - // so, limit us to only using that value. If go-connections/tlsconfig changes its policy, we - // will want to consider that and make a decision whether to follow suit. - // There is some chance that eventually the Go default will be to require TLS 1.3, and that point - // we might want to drop the dependency on go-connections entirely. - CipherSuites: tlsconfig.ClientDefault().CipherSuites, + if sys != nil && sys.BaseTLSConfig != nil { + tr.TLSClientConfig = sys.BaseTLSConfig.Clone() + } else { + tr.TLSClientConfig = &tls.Config{ + // As of 2025-08, tlsconfig.ClientDefault() differs from Go 1.23 defaults only in CipherSuites; + // so, limit us to only using that value. If go-connections/tlsconfig changes its policy, we + // will want to consider that and make a decision whether to follow suit. + // There is some chance that eventually the Go default will be to require TLS 1.3, and that point + // we might want to drop the dependency on go-connections entirely. + CipherSuites: tlsconfig.ClientDefault().CipherSuites, + } } if sys != nil && sys.OCICertPath != "" { From 1117fef8d50ef659e5daf2f3fd61e7dc1d871d03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 30 Jan 2026 16:24:21 +0100 Subject: [PATCH 09/18] Implement BaseTLSConfig for image/openshift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/openshift/openshift-copies.go | 30 +++++++++++++++++------------ image/openshift/openshift.go | 5 +++-- image/openshift/openshift_dest.go | 2 +- image/openshift/openshift_src.go | 2 +- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/image/openshift/openshift-copies.go b/image/openshift/openshift-copies.go index 2799038644..84a46a9836 100644 --- a/image/openshift/openshift-copies.go +++ b/image/openshift/openshift-copies.go @@ -21,6 +21,7 @@ import ( "dario.cat/mergo" "github.com/sirupsen/logrus" "go.podman.io/image/v5/internal/multierr" + "go.podman.io/image/v5/types" "go.podman.io/storage/pkg/homedir" "gopkg.in/yaml.v3" ) @@ -715,7 +716,7 @@ func resolvePaths(refs []*string, base string) error { // object. Note that a RESTClient may require fields that are optional when initializing a Client. // A RESTClient created by this method is generic - it expects to operate on an API that follows // the Kubernetes conventions, but may not be the Kubernetes API. -func restClientFor(config *restConfig) (*url.URL, *http.Client, error) { +func restClientFor(sys *types.SystemContext, config *restConfig) (*url.URL, *http.Client, error) { // REMOVED: Configurable GroupVersion, Codec // REMOVED: Configurable versionedAPIPath baseURL, err := defaultServerURLFor(config) @@ -723,7 +724,7 @@ func restClientFor(config *restConfig) (*url.URL, *http.Client, error) { return nil, nil, err } - transport, err := transportFor(config) + transport, err := transportFor(sys, config) if err != nil { return nil, nil, err } @@ -791,9 +792,9 @@ func defaultServerURLFor(config *restConfig) (*url.URL, error) { // TransportFor returns an http.RoundTripper that will provide the authentication // or transport level security defined by the provided Config. Will return the // default http.DefaultTransport if no special case behavior is needed. -func transportFor(config *restConfig) (http.RoundTripper, error) { +func transportFor(sys *types.SystemContext, config *restConfig) (http.RoundTripper, error) { // REMOVED: separation between restclient.Config and transport.Config, Transport, WrapTransport support - return transportNew(config) + return transportNew(sys, config) } // isConfigTransportTLS is a modified copy of k8s.io/kubernetes/pkg/client/restclient.IsConfigTransportTLS. @@ -815,7 +816,7 @@ func isConfigTransportTLS(config restConfig) bool { // transportNew is a modified copy of k8s.io/kubernetes/pkg/client/transport.New. // New returns an http.RoundTripper that will provide the authentication // or transport level security defined by the provided Config. -func transportNew(config *restConfig) (http.RoundTripper, error) { +func transportNew(sys *types.SystemContext, config *restConfig) (http.RoundTripper, error) { // REMOVED: custom config.Transport support. // Set transport level security @@ -824,7 +825,7 @@ func transportNew(config *restConfig) (http.RoundTripper, error) { err error ) - rt, err = tlsCacheGet(config) + rt, err = tlsCacheGet(sys, config) if err != nil { return nil, err } @@ -883,11 +884,11 @@ func newProxierWithNoProxyCIDR(delegate func(req *http.Request) (*url.URL, error } // tlsCacheGet is a modified copy of k8s.io/kubernetes/pkg/client/transport.tlsTransportCache.get. -func tlsCacheGet(config *restConfig) (http.RoundTripper, error) { +func tlsCacheGet(sys *types.SystemContext, config *restConfig) (http.RoundTripper, error) { // REMOVED: any actual caching // Get the TLS options for this client config - tlsConfig, err := tlsConfigFor(config) + tlsConfig, err := tlsConfigFor(sys, config) if err != nil { return nil, err } @@ -918,8 +919,8 @@ func tlsCacheGet(config *restConfig) (http.RoundTripper, error) { // tlsConfigFor is a modified copy of k8s.io/kubernetes/pkg/client/transport.TLSConfigFor. // TLSConfigFor returns a tls.Config that will provide the transport level security defined // by the provided Config. Will return nil if no transport level security is requested. -func tlsConfigFor(c *restConfig) (*tls.Config, error) { - if !c.HasCA() && !c.HasCertAuth() && !c.Insecure { +func tlsConfigFor(sys *types.SystemContext, c *restConfig) (*tls.Config, error) { + if !c.HasCA() && !c.HasCertAuth() && !c.Insecure && (sys == nil || sys.BaseTLSConfig == nil) { return nil, nil } if c.HasCA() && c.Insecure { @@ -929,10 +930,15 @@ func tlsConfigFor(c *restConfig) (*tls.Config, error) { return nil, err } - tlsConfig := &tls.Config{ - InsecureSkipVerify: c.Insecure, + var tlsConfig *tls.Config + if sys != nil && sys.BaseTLSConfig != nil { + tlsConfig = sys.BaseTLSConfig.Clone() + } else { + tlsConfig = &tls.Config{} } + tlsConfig.InsecureSkipVerify = c.Insecure + if c.HasCA() { tlsConfig.RootCAs = rootCertPool(c.TLSClientConfig.CAData) } diff --git a/image/openshift/openshift.go b/image/openshift/openshift.go index e8e894eade..52a3f29272 100644 --- a/image/openshift/openshift.go +++ b/image/openshift/openshift.go @@ -14,6 +14,7 @@ import ( "github.com/sirupsen/logrus" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/internal/iolimits" + "go.podman.io/image/v5/types" "go.podman.io/image/v5/version" ) @@ -29,7 +30,7 @@ type openshiftClient struct { } // newOpenshiftClient creates a new openshiftClient for the specified reference. -func newOpenshiftClient(ref openshiftReference) (*openshiftClient, error) { +func newOpenshiftClient(sys *types.SystemContext, ref openshiftReference) (*openshiftClient, error) { // Overall, this is modelled on openshift/origin/pkg/cmd/util/clientcmd.New().ClientConfig() and openshift/origin/pkg/client. cmdConfig := defaultClientConfig() logrus.Debugf("cmdConfig: %#v", cmdConfig) @@ -39,7 +40,7 @@ func newOpenshiftClient(ref openshiftReference) (*openshiftClient, error) { } // REMOVED: SetOpenShiftDefaults (values are not overridable in config files, so hard-coded these defaults.) logrus.Debugf("restConfig: %#v", restConfig) - baseURL, httpClient, err := restClientFor(restConfig) + baseURL, httpClient, err := restClientFor(sys, restConfig) if err != nil { return nil, err } diff --git a/image/openshift/openshift_dest.go b/image/openshift/openshift_dest.go index 7c901d8e7d..f61a7407a9 100644 --- a/image/openshift/openshift_dest.go +++ b/image/openshift/openshift_dest.go @@ -37,7 +37,7 @@ type openshiftImageDestination struct { // newImageDestination creates a new ImageDestination for the specified reference. func newImageDestination(ctx context.Context, sys *types.SystemContext, ref openshiftReference) (private.ImageDestination, error) { - client, err := newOpenshiftClient(ref) + client, err := newOpenshiftClient(sys, ref) if err != nil { return nil, err } diff --git a/image/openshift/openshift_src.go b/image/openshift/openshift_src.go index 4842ba3018..a3ab084516 100644 --- a/image/openshift/openshift_src.go +++ b/image/openshift/openshift_src.go @@ -37,7 +37,7 @@ type openshiftImageSource struct { // newImageSource creates a new ImageSource for the specified reference. // The caller must call .Close() on the returned ImageSource. func newImageSource(sys *types.SystemContext, ref openshiftReference) (private.ImageSource, error) { - client, err := newOpenshiftClient(ref) + client, err := newOpenshiftClient(sys, ref) if err != nil { return nil, err } From ea1b24388b82a1db6ff4cdc97e39280cfefffadb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 10 Feb 2026 01:19:05 +0100 Subject: [PATCH 10/18] Consolidate TestTlsConfig into table-driven tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/docker/daemon/client_test.go | 58 ++++++++++++++---------------- 1 file changed, 26 insertions(+), 32 deletions(-) diff --git a/image/docker/daemon/client_test.go b/image/docker/daemon/client_test.go index cbbe7457ef..fec9702bdb 100644 --- a/image/docker/daemon/client_test.go +++ b/image/docker/daemon/client_test.go @@ -7,6 +7,7 @@ import ( dockerclient "github.com/moby/moby/client" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.podman.io/image/v5/types" ) @@ -39,42 +40,35 @@ func TestDockerClientFromCertContext(t *testing.T) { assert.NoError(t, client.Close()) } -func TestTlsConfigFromInvalidCertPath(t *testing.T) { - ctx := &types.SystemContext{ - DockerDaemonCertPath: "/foo/bar", +func TestTlsConfig(t *testing.T) { + tests := []struct { + ctx *types.SystemContext + wantInsecure bool + wantCerts int + }{ + {&types.SystemContext{ + DockerDaemonCertPath: filepath.Join("testdata", "certs"), + DockerDaemonInsecureSkipTLSVerify: true, + }, true, 1}, + {&types.SystemContext{DockerDaemonInsecureSkipTLSVerify: true}, true, 0}, } - - _, err := tlsConfig(ctx) - assert.ErrorContains(t, err, "could not read CA certificate") -} - -func TestTlsConfigFromCertPath(t *testing.T) { - ctx := &types.SystemContext{ - DockerDaemonCertPath: filepath.Join("testdata", "certs"), - DockerDaemonInsecureSkipTLSVerify: true, + for _, c := range tests { + httpClient, err := tlsConfig(c.ctx) + require.NoError(t, err) + tlsCfg := httpClient.Transport.(*http.Transport).TLSClientConfig + assert.Equal(t, c.wantInsecure, tlsCfg.InsecureSkipVerify) + assert.Len(t, tlsCfg.Certificates, c.wantCerts) } - httpClient, err := tlsConfig(ctx) - - assert.NoError(t, err, "There should be no error creating the HTTP client") - - tlsConfig := httpClient.Transport.(*http.Transport).TLSClientConfig - assert.True(t, tlsConfig.InsecureSkipVerify, "TLS verification should be skipped") - assert.Len(t, tlsConfig.Certificates, 1, "There should be one certificate") -} - -func TestSkipTLSVerifyOnly(t *testing.T) { - ctx := &types.SystemContext{ - DockerDaemonInsecureSkipTLSVerify: true, + for _, c := range []struct { + ctx *types.SystemContext + pathFragment string + }{ + {&types.SystemContext{DockerDaemonCertPath: "/dev/null/this/does/not/exist"}, "dev/null/this/does/not/exist"}, + } { + _, err := tlsConfig(c.ctx) + assert.ErrorContains(t, err, c.pathFragment) } - - httpClient, err := tlsConfig(ctx) - - assert.NoError(t, err, "There should be no error creating the HTTP client") - - tlsConfig := httpClient.Transport.(*http.Transport).TLSClientConfig - assert.True(t, tlsConfig.InsecureSkipVerify, "TLS verification should be skipped") - assert.Len(t, tlsConfig.Certificates, 0, "There should be no certificate") } func TestSpecifyPlainHTTPViaHostScheme(t *testing.T) { From 229ffb1bed2cc5611816146ff0eed7b846534fea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Tue, 10 Feb 2026 01:34:34 +0100 Subject: [PATCH 11/18] Expand TestTlsConfig MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We will replace the implementation Signed-off-by: Miloslav Trmač --- image/docker/daemon/client_test.go | 64 +++++++++++++++++-- .../daemon/testdata/certs-no-ca/cert.pem | 1 + .../daemon/testdata/certs-no-ca/key.pem | 1 + .../daemon/testdata/certs-no-cert/ca.pem | 1 + .../daemon/testdata/certs-no-cert/key.pem | 1 + .../daemon/testdata/certs-no-key/ca.pem | 1 + .../daemon/testdata/certs-no-key/cert.pem | 1 + .../testdata/certs-unreadable-ca/ca.pem | 1 + .../testdata/certs-unreadable-ca/cert.pem | 1 + .../testdata/certs-unreadable-ca/key.pem | 1 + .../testdata/certs-unreadable-cert/ca.pem | 1 + .../testdata/certs-unreadable-cert/cert.pem | 1 + .../testdata/certs-unreadable-cert/key.pem | 1 + .../testdata/certs-unreadable-key/ca.pem | 1 + .../testdata/certs-unreadable-key/cert.pem | 1 + .../testdata/certs-unreadable-key/key.pem | 1 + 16 files changed, 74 insertions(+), 5 deletions(-) create mode 120000 image/docker/daemon/testdata/certs-no-ca/cert.pem create mode 120000 image/docker/daemon/testdata/certs-no-ca/key.pem create mode 120000 image/docker/daemon/testdata/certs-no-cert/ca.pem create mode 120000 image/docker/daemon/testdata/certs-no-cert/key.pem create mode 120000 image/docker/daemon/testdata/certs-no-key/ca.pem create mode 120000 image/docker/daemon/testdata/certs-no-key/cert.pem create mode 120000 image/docker/daemon/testdata/certs-unreadable-ca/ca.pem create mode 120000 image/docker/daemon/testdata/certs-unreadable-ca/cert.pem create mode 120000 image/docker/daemon/testdata/certs-unreadable-ca/key.pem create mode 120000 image/docker/daemon/testdata/certs-unreadable-cert/ca.pem create mode 120000 image/docker/daemon/testdata/certs-unreadable-cert/cert.pem create mode 120000 image/docker/daemon/testdata/certs-unreadable-cert/key.pem create mode 120000 image/docker/daemon/testdata/certs-unreadable-key/ca.pem create mode 120000 image/docker/daemon/testdata/certs-unreadable-key/cert.pem create mode 120000 image/docker/daemon/testdata/certs-unreadable-key/key.pem diff --git a/image/docker/daemon/client_test.go b/image/docker/daemon/client_test.go index fec9702bdb..9d3f30d414 100644 --- a/image/docker/daemon/client_test.go +++ b/image/docker/daemon/client_test.go @@ -1,6 +1,9 @@ package daemon import ( + "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" "net/http" "path/filepath" "testing" @@ -8,6 +11,7 @@ import ( dockerclient "github.com/moby/moby/client" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.podman.io/image/v5/internal/set" "go.podman.io/image/v5/types" ) @@ -42,21 +46,65 @@ func TestDockerClientFromCertContext(t *testing.T) { func TestTlsConfig(t *testing.T) { tests := []struct { - ctx *types.SystemContext - wantInsecure bool - wantCerts int + ctx *types.SystemContext + wantInsecure bool + wantCAIncluded bool + wantCerts int }{ {&types.SystemContext{ DockerDaemonCertPath: filepath.Join("testdata", "certs"), DockerDaemonInsecureSkipTLSVerify: true, - }, true, 1}, - {&types.SystemContext{DockerDaemonInsecureSkipTLSVerify: true}, true, 0}, + }, true, false, 1}, + {&types.SystemContext{DockerDaemonInsecureSkipTLSVerify: true}, true, false, 0}, + {&types.SystemContext{ + DockerDaemonCertPath: filepath.Join("testdata", "certs"), + }, false, true, 1}, } for _, c := range tests { httpClient, err := tlsConfig(c.ctx) require.NoError(t, err) tlsCfg := httpClient.Transport.(*http.Transport).TLSClientConfig assert.Equal(t, c.wantInsecure, tlsCfg.InsecureSkipVerify) + + if c.wantCAIncluded { + // SystemCertPool is implemented natively, and .Subjects() does not + // return raw certificates, on some systems (as of Go 1.18, + // Windows, macOS, iOS); so, .Subjects() is deprecated. + // We still use .Subjects() in these tests, because they work + // acceptably even in the native case, and they work fine on Linux, + // which we care about the most. + + // On systems where SystemCertPool is not special-cased, RootCAs include SystemCertPool; + // On systems where SystemCertPool is special cased, this compares two empty sets + // and succeeds. + // There isn’t a plausible alternative to calling .Subjects() here. + loadedSubjectBytes := set.New[string]() + for _, s := range tlsCfg.RootCAs.Subjects() { //nolint:staticcheck // SA1019: Receiving no data for system roots is acceptable. + loadedSubjectBytes.Add(string(s)) + } + systemCertPool, err := x509.SystemCertPool() + require.NoError(t, err) + for _, s := range systemCertPool.Subjects() { //nolint:staticcheck // SA1019: Receiving no data for system roots is acceptable. + assert.True(t, loadedSubjectBytes.Contains(string(s))) + } + // RootCAs include our certificates. + // We could possibly test this without .Subjects() by validating certificates + // signed by our test CAs. + loadedSubjectOrgs := set.New[string]() + for _, s := range tlsCfg.RootCAs.Subjects() { //nolint:staticcheck // SA1019: We only care about non-system roots here. + subjectRDN := pkix.RDNSequence{} + rest, err := asn1.Unmarshal(s, &subjectRDN) + require.NoError(t, err) + require.Empty(t, rest) + subject := pkix.Name{} + subject.FillFromRDNSequence(&subjectRDN) + for _, org := range subject.Organization { + loadedSubjectOrgs.Add(org) + } + } + assert.True(t, loadedSubjectOrgs.Contains("hardy"), "RootCAs should include the testdata CA (O=hardy)") + } + assert.Len(t, tlsCfg.Certificates, c.wantCerts) } @@ -65,6 +113,12 @@ func TestTlsConfig(t *testing.T) { pathFragment string }{ {&types.SystemContext{DockerDaemonCertPath: "/dev/null/this/does/not/exist"}, "dev/null/this/does/not/exist"}, + {&types.SystemContext{DockerDaemonCertPath: filepath.Join("testdata", "certs-no-ca")}, "ca.pem"}, + {&types.SystemContext{DockerDaemonCertPath: filepath.Join("testdata", "certs-no-cert")}, "cert.pem"}, + {&types.SystemContext{DockerDaemonCertPath: filepath.Join("testdata", "certs-no-key")}, "key.pem"}, + {&types.SystemContext{DockerDaemonCertPath: filepath.Join("testdata", "certs-unreadable-ca")}, "ca.pem"}, + {&types.SystemContext{DockerDaemonCertPath: filepath.Join("testdata", "certs-unreadable-cert")}, "cert.pem"}, + {&types.SystemContext{DockerDaemonCertPath: filepath.Join("testdata", "certs-unreadable-key")}, "key.pem"}, } { _, err := tlsConfig(c.ctx) assert.ErrorContains(t, err, c.pathFragment) diff --git a/image/docker/daemon/testdata/certs-no-ca/cert.pem b/image/docker/daemon/testdata/certs-no-ca/cert.pem new file mode 120000 index 0000000000..ef7322efd1 --- /dev/null +++ b/image/docker/daemon/testdata/certs-no-ca/cert.pem @@ -0,0 +1 @@ +../certs/cert.pem \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-no-ca/key.pem b/image/docker/daemon/testdata/certs-no-ca/key.pem new file mode 120000 index 0000000000..8bfbc333aa --- /dev/null +++ b/image/docker/daemon/testdata/certs-no-ca/key.pem @@ -0,0 +1 @@ +../certs/key.pem \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-no-cert/ca.pem b/image/docker/daemon/testdata/certs-no-cert/ca.pem new file mode 120000 index 0000000000..e4e83c4777 --- /dev/null +++ b/image/docker/daemon/testdata/certs-no-cert/ca.pem @@ -0,0 +1 @@ +../certs/ca.pem \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-no-cert/key.pem b/image/docker/daemon/testdata/certs-no-cert/key.pem new file mode 120000 index 0000000000..8bfbc333aa --- /dev/null +++ b/image/docker/daemon/testdata/certs-no-cert/key.pem @@ -0,0 +1 @@ +../certs/key.pem \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-no-key/ca.pem b/image/docker/daemon/testdata/certs-no-key/ca.pem new file mode 120000 index 0000000000..e4e83c4777 --- /dev/null +++ b/image/docker/daemon/testdata/certs-no-key/ca.pem @@ -0,0 +1 @@ +../certs/ca.pem \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-no-key/cert.pem b/image/docker/daemon/testdata/certs-no-key/cert.pem new file mode 120000 index 0000000000..ef7322efd1 --- /dev/null +++ b/image/docker/daemon/testdata/certs-no-key/cert.pem @@ -0,0 +1 @@ +../certs/cert.pem \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-unreadable-ca/ca.pem b/image/docker/daemon/testdata/certs-unreadable-ca/ca.pem new file mode 120000 index 0000000000..c1c714fdd6 --- /dev/null +++ b/image/docker/daemon/testdata/certs-unreadable-ca/ca.pem @@ -0,0 +1 @@ +/dev/null/this/does/not/exist \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-unreadable-ca/cert.pem b/image/docker/daemon/testdata/certs-unreadable-ca/cert.pem new file mode 120000 index 0000000000..ef7322efd1 --- /dev/null +++ b/image/docker/daemon/testdata/certs-unreadable-ca/cert.pem @@ -0,0 +1 @@ +../certs/cert.pem \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-unreadable-ca/key.pem b/image/docker/daemon/testdata/certs-unreadable-ca/key.pem new file mode 120000 index 0000000000..8bfbc333aa --- /dev/null +++ b/image/docker/daemon/testdata/certs-unreadable-ca/key.pem @@ -0,0 +1 @@ +../certs/key.pem \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-unreadable-cert/ca.pem b/image/docker/daemon/testdata/certs-unreadable-cert/ca.pem new file mode 120000 index 0000000000..e4e83c4777 --- /dev/null +++ b/image/docker/daemon/testdata/certs-unreadable-cert/ca.pem @@ -0,0 +1 @@ +../certs/ca.pem \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-unreadable-cert/cert.pem b/image/docker/daemon/testdata/certs-unreadable-cert/cert.pem new file mode 120000 index 0000000000..c1c714fdd6 --- /dev/null +++ b/image/docker/daemon/testdata/certs-unreadable-cert/cert.pem @@ -0,0 +1 @@ +/dev/null/this/does/not/exist \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-unreadable-cert/key.pem b/image/docker/daemon/testdata/certs-unreadable-cert/key.pem new file mode 120000 index 0000000000..8bfbc333aa --- /dev/null +++ b/image/docker/daemon/testdata/certs-unreadable-cert/key.pem @@ -0,0 +1 @@ +../certs/key.pem \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-unreadable-key/ca.pem b/image/docker/daemon/testdata/certs-unreadable-key/ca.pem new file mode 120000 index 0000000000..e4e83c4777 --- /dev/null +++ b/image/docker/daemon/testdata/certs-unreadable-key/ca.pem @@ -0,0 +1 @@ +../certs/ca.pem \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-unreadable-key/cert.pem b/image/docker/daemon/testdata/certs-unreadable-key/cert.pem new file mode 120000 index 0000000000..ef7322efd1 --- /dev/null +++ b/image/docker/daemon/testdata/certs-unreadable-key/cert.pem @@ -0,0 +1 @@ +../certs/cert.pem \ No newline at end of file diff --git a/image/docker/daemon/testdata/certs-unreadable-key/key.pem b/image/docker/daemon/testdata/certs-unreadable-key/key.pem new file mode 120000 index 0000000000..c1c714fdd6 --- /dev/null +++ b/image/docker/daemon/testdata/certs-unreadable-key/key.pem @@ -0,0 +1 @@ +/dev/null/this/does/not/exist \ No newline at end of file From b55bbb94fcd1f6cb55a36bf15af26ec3937af2bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 30 Jan 2026 17:30:02 +0100 Subject: [PATCH 12/18] Implement BaseTLSConfig for image/docker/daemon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/docker/daemon/client.go | 63 ++++++++++++++++++++++++------ image/docker/daemon/client_test.go | 11 ++++++ 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/image/docker/daemon/client.go b/image/docker/daemon/client.go index e5776904f8..433c0c9778 100644 --- a/image/docker/daemon/client.go +++ b/image/docker/daemon/client.go @@ -1,7 +1,11 @@ package daemon import ( + "crypto/tls" + "crypto/x509" + "fmt" "net/http" + "os" "path/filepath" "time" @@ -63,26 +67,61 @@ func newDockerClient(sys *types.SystemContext) (*dockerclient.Client, error) { } func tlsConfig(sys *types.SystemContext) (*http.Client, error) { - options := tlsconfig.Options{} - if sys != nil && sys.DockerDaemonInsecureSkipTLSVerify { - options.InsecureSkipVerify = true - } + // This is intended to follow github.com/moby/moby/client.defaultHTTPClient + // and approximately github.com/moby/moby/client.WithTLSClientConfigFromEnv; + // we can’t use WithTLSClientConfig because 1) it uses ExclusiveRootPools:true, + // and 2) there is no clean way to override the crypto choices of tlsconfig.Client. + // So, we need to load the certs and keys ourselves in the BaseTLSConfig case + // anyway, we might just as well do it in the other case as well. - if sys != nil && sys.DockerDaemonCertPath != "" { - options.CAFile = filepath.Join(sys.DockerDaemonCertPath, "ca.pem") - options.CertFile = filepath.Join(sys.DockerDaemonCertPath, "cert.pem") - options.KeyFile = filepath.Join(sys.DockerDaemonCertPath, "key.pem") + var tlsConfig *tls.Config + if sys != nil && sys.BaseTLSConfig != nil { + tlsConfig = sys.BaseTLSConfig.Clone() + } else { + tlsConfig = &tls.Config{ + // As of 2025-08, tlsconfig.ClientDefault() differs from Go 1.23 defaults only in CipherSuites; + // so, limit us to only using that value. If go-connections/tlsconfig changes its policy, we + // will want to consider that and make a decision whether to follow suit. + // There is some chance that eventually the Go default will be to require TLS 1.3, and that point + // we might want to drop the dependency on go-connections entirely. + CipherSuites: tlsconfig.ClientDefault().CipherSuites, + } } - tlsc, err := tlsconfig.Client(options) - if err != nil { - return nil, err + if sys != nil { + if sys.DockerDaemonInsecureSkipTLSVerify { + tlsConfig.InsecureSkipVerify = true + } else if sys.DockerDaemonCertPath != "" { + caFilePath := filepath.Join(sys.DockerDaemonCertPath, "ca.pem") + caData, err := os.ReadFile(caFilePath) + if err != nil { + return nil, fmt.Errorf("reading CA file %q: %w", caFilePath, err) + } + caPool, err := x509.SystemCertPool() + if err != nil { + return nil, fmt.Errorf("reading systemwide CAs: %w", err) + } + if !caPool.AppendCertsFromPEM(caData) { + return nil, fmt.Errorf("no certificate found in %q", caFilePath) + } + tlsConfig.RootCAs = caPool + } + + if sys.DockerDaemonCertPath != "" { + cert, err := tls.LoadX509KeyPair( + filepath.Join(sys.DockerDaemonCertPath, "cert.pem"), + filepath.Join(sys.DockerDaemonCertPath, "key.pem")) + if err != nil { + return nil, fmt.Errorf("loading X509 key pair: %w", err) + } + tlsConfig.Certificates = []tls.Certificate{cert} + } } return &http.Client{ Transport: &http.Transport{ Proxy: http.ProxyFromEnvironment, - TLSClientConfig: tlsc, + TLSClientConfig: tlsConfig, // In general we want to follow github.com/moby/moby/client.defaultHTTPClient , as long as it doesn’t affect compatibility. // These idle connection limits really only apply to long-running clients, which is not our case here; // we include the same values purely for symmetry. diff --git a/image/docker/daemon/client_test.go b/image/docker/daemon/client_test.go index 9d3f30d414..53c35c7264 100644 --- a/image/docker/daemon/client_test.go +++ b/image/docker/daemon/client_test.go @@ -1,6 +1,7 @@ package daemon import ( + "crypto/tls" "crypto/x509" "crypto/x509/pkix" "encoding/asn1" @@ -59,6 +60,12 @@ func TestTlsConfig(t *testing.T) { {&types.SystemContext{ DockerDaemonCertPath: filepath.Join("testdata", "certs"), }, false, true, 1}, + {&types.SystemContext{ + DockerDaemonCertPath: filepath.Join("testdata", "certs"), + BaseTLSConfig: &tls.Config{ + MinVersion: tls.VersionTLS13, + }, + }, false, true, 1}, } for _, c := range tests { httpClient, err := tlsConfig(c.ctx) @@ -106,6 +113,10 @@ func TestTlsConfig(t *testing.T) { } assert.Len(t, tlsCfg.Certificates, c.wantCerts) + + if c.ctx.BaseTLSConfig != nil { + assert.Equal(t, c.ctx.BaseTLSConfig.MinVersion, tlsCfg.MinVersion) + } } for _, c := range []struct { From 0e5b2f99b7d25660599f9558eed10d593018c1fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 30 Jan 2026 18:49:54 +0100 Subject: [PATCH 13/18] INCOMPLETE Implement BaseTLS in Rekor signing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The underlying implementation is there, but it is not exposed in the API; we need an implementation for Fulcio + OIDC as well, and then we should decide whether the two get separate BaseTLS options, or whether we will do signature/sigstore.WithBaseTLS that applies to both. Signed-off-by: Miloslav Trmač --- image/signature/sigstore/rekor/rekor.go | 13 ++++++++++--- image/signature/sigstore/rekor/rekor_test.go | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/image/signature/sigstore/rekor/rekor.go b/image/signature/sigstore/rekor/rekor.go index a064c454c9..5e497da7fc 100644 --- a/image/signature/sigstore/rekor/rekor.go +++ b/image/signature/sigstore/rekor/rekor.go @@ -3,6 +3,7 @@ package rekor import ( "context" "crypto/sha256" + "crypto/tls" "encoding/base64" "encoding/hex" "encoding/json" @@ -29,7 +30,8 @@ const ( func WithRekor(rekorURL *url.URL) signerInternal.Option { return func(s *signerInternal.SigstoreSigner) error { logrus.Debugf("Using Rekor server at %s", rekorURL.Redacted()) - client := newRekorClient(rekorURL) + baseTLSConfig := (*tls.Config)(nil) // FIXME: decide on an API — should it be shared with Fulcio? + client := newRekorClient(rekorURL, baseTLSConfig) s.RekorUploader = client.uploadKeyOrCert return nil } @@ -43,9 +45,14 @@ type rekorClient struct { } // newRekorClient creates a rekorClient for rekorURL. -func newRekorClient(rekorURL *url.URL) *rekorClient { +// If baseTLSConfig is not nil, it may contain TLS _algorithm_ options (e.g. TLS version, cipher suites, “curves”, etc.). +func newRekorClient(rekorURL *url.URL, baseTLSConfig *tls.Config) *rekorClient { retryableClient := retryablehttp.NewClient() - retryableClient.HTTPClient = cleanhttp.DefaultClient() + transport := cleanhttp.DefaultTransport() + if baseTLSConfig != nil { + transport.TLSClientConfig = baseTLSConfig.Clone() + } + retryableClient.HTTPClient = &http.Client{Transport: transport} retryableClient.RetryMax = defaultRetryCount retryableClient.Logger = leveledLoggerForLogrus(logrus.StandardLogger()) basePath := rekorURL.Path diff --git a/image/signature/sigstore/rekor/rekor_test.go b/image/signature/sigstore/rekor/rekor_test.go index 3048f0e864..1b236415b7 100644 --- a/image/signature/sigstore/rekor/rekor_test.go +++ b/image/signature/sigstore/rekor/rekor_test.go @@ -96,7 +96,7 @@ func Test_rekorUploadKeyOrCert(t *testing.T) { require.NoError(t, err) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cl := newRekorClient(u) + cl := newRekorClient(u, nil) signatureBytes, err := base64.StdEncoding.DecodeString(tt.args.signatureBase64) require.NoError(t, err) From 785cf04ded2cabdc66a7bd50b3ecebf76f458d38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 2 Feb 2026 14:33:44 +0100 Subject: [PATCH 14/18] API change: Use a context, and a private client, in download.FromURL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use a context to allow aborting, instead of silencing a relevant lint warning - Use a separate http.Client so that we don't share cookies with the rest of the process Signed-off-by: Miloslav Trmač --- common/libimage/import.go | 2 +- common/pkg/download/download.go | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/common/libimage/import.go b/common/libimage/import.go index 54a77e4e95..3718785bc4 100644 --- a/common/libimage/import.go +++ b/common/libimage/import.go @@ -69,7 +69,7 @@ func (r *Runtime) Import(ctx context.Context, path string, options *ImportOption if err == nil && u.Scheme != "" { // If source is a URL, download the file. fmt.Printf("Downloading from %q\n", path) //nolint:forbidigo - file, err := download.FromURL(r.systemContext.BigFilesTemporaryDir, path) + file, err := download.FromURL(ctx, r.systemContext.BigFilesTemporaryDir, path) if err != nil { return "", err } diff --git a/common/pkg/download/download.go b/common/pkg/download/download.go index 187af4a614..c960ec494a 100644 --- a/common/pkg/download/download.go +++ b/common/pkg/download/download.go @@ -1,6 +1,7 @@ package download import ( + "context" "fmt" "io" "net/http" @@ -9,7 +10,7 @@ import ( // FromURL downloads the specified source to a file in tmpdir (OS defaults if // empty). -func FromURL(tmpdir, source string) (string, error) { +func FromURL(ctx context.Context, tmpdir, source string) (string, error) { tmp, err := os.CreateTemp(tmpdir, "") if err != nil { return "", fmt.Errorf("creating temporary download file: %w", err) @@ -22,7 +23,12 @@ func FromURL(tmpdir, source string) (string, error) { } }() - response, err := http.Get(source) // nolint:noctx + client := &http.Client{} + req, err := http.NewRequestWithContext(ctx, http.MethodGet, source, nil) + if err != nil { + return "", fmt.Errorf("preparing to download %q: %w", source, err) + } + response, err := client.Do(req) if err != nil { return "", fmt.Errorf("downloading %s: %w", source, err) } From 2223072f9bd22d3164afb22e18471c4013b9dc15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 2 Feb 2026 14:36:22 +0100 Subject: [PATCH 15/18] API change: Add an Options parameter to download.FromURL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... and allow configuring BaseTLS. Signed-off-by: Miloslav Trmač --- common/libimage/import.go | 4 +++- common/pkg/download/download.go | 20 ++++++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/common/libimage/import.go b/common/libimage/import.go index 3718785bc4..766dc8cc8d 100644 --- a/common/libimage/import.go +++ b/common/libimage/import.go @@ -69,7 +69,9 @@ func (r *Runtime) Import(ctx context.Context, path string, options *ImportOption if err == nil && u.Scheme != "" { // If source is a URL, download the file. fmt.Printf("Downloading from %q\n", path) //nolint:forbidigo - file, err := download.FromURL(ctx, r.systemContext.BigFilesTemporaryDir, path) + file, err := download.FromURL(ctx, r.systemContext.BigFilesTemporaryDir, path, download.Options{ + BaseTLSConfig: r.systemContext.BaseTLSConfig, + }) if err != nil { return "", err } diff --git a/common/pkg/download/download.go b/common/pkg/download/download.go index c960ec494a..41a106c39f 100644 --- a/common/pkg/download/download.go +++ b/common/pkg/download/download.go @@ -2,15 +2,22 @@ package download import ( "context" + "crypto/tls" "fmt" "io" "net/http" "os" ) +// Options holds named options for FromURL. +type Options struct { + // If not nil, may contain TLS _algorithm_ options (e.g. TLS version, cipher suites, “curves”, etc.). + BaseTLSConfig *tls.Config +} + // FromURL downloads the specified source to a file in tmpdir (OS defaults if // empty). -func FromURL(ctx context.Context, tmpdir, source string) (string, error) { +func FromURL(ctx context.Context, tmpdir, source string, options Options) (string, error) { tmp, err := os.CreateTemp(tmpdir, "") if err != nil { return "", fmt.Errorf("creating temporary download file: %w", err) @@ -23,7 +30,16 @@ func FromURL(ctx context.Context, tmpdir, source string) (string, error) { } }() - client := &http.Client{} + var transport *http.Transport // nil means http.DefaultTransport + if options.BaseTLSConfig != nil { + transport = &http.Transport{ + TLSClientConfig: options.BaseTLSConfig, + } + defer transport.CloseIdleConnections() + } + client := &http.Client{ + Transport: transport, + } req, err := http.NewRequestWithContext(ctx, http.MethodGet, source, nil) if err != nil { return "", fmt.Errorf("preparing to download %q: %w", source, err) From c56a4fe8fef98f6f72f7cb57e491aba3ffa3695a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 5 Feb 2026 19:40:04 +0100 Subject: [PATCH 16/18] Fix Deprecated: comments on functions which don't accept SystemContext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- image/docker/tarfile/dest.go | 1 + image/docker/tarfile/src.go | 2 ++ image/oci/archive/oci_src.go | 1 + 3 files changed, 4 insertions(+) diff --git a/image/docker/tarfile/dest.go b/image/docker/tarfile/dest.go index aca89bf2e0..92af4d155d 100644 --- a/image/docker/tarfile/dest.go +++ b/image/docker/tarfile/dest.go @@ -18,6 +18,7 @@ type Destination struct { } // NewDestination returns a tarfile.Destination for the specified io.Writer. +// // Deprecated: please use NewDestinationWithContext instead func NewDestination(dest io.Writer, ref reference.NamedTagged) *Destination { return NewDestinationWithContext(nil, dest, ref) diff --git a/image/docker/tarfile/src.go b/image/docker/tarfile/src.go index c9d75c07b5..1f3e59ac4f 100644 --- a/image/docker/tarfile/src.go +++ b/image/docker/tarfile/src.go @@ -16,6 +16,7 @@ type Source struct { } // NewSourceFromFile returns a tarfile.Source for the specified path. +// // Deprecated: Please use NewSourceFromFileWithContext which will allows you to configure temp directory // for big files through SystemContext.BigFilesTemporaryDir func NewSourceFromFile(path string) (*Source, error) { @@ -35,6 +36,7 @@ func NewSourceFromFileWithContext(sys *types.SystemContext, path string) (*Sourc // NewSourceFromStream returns a tarfile.Source for the specified inputStream, // which can be either compressed or uncompressed. The caller can close the // inputStream immediately after NewSourceFromFile returns. +// // Deprecated: Please use NewSourceFromStreamWithSystemContext which will allows you to configure // temp directory for big files through SystemContext.BigFilesTemporaryDir func NewSourceFromStream(inputStream io.Reader) (*Source, error) { diff --git a/image/oci/archive/oci_src.go b/image/oci/archive/oci_src.go index cb15c06f03..fcfe6d19ac 100644 --- a/image/oci/archive/oci_src.go +++ b/image/oci/archive/oci_src.go @@ -77,6 +77,7 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref ociArchiv } // LoadManifestDescriptor loads the manifest +// // Deprecated: use LoadManifestDescriptorWithContext instead func LoadManifestDescriptor(imgRef types.ImageReference) (imgspecv1.Descriptor, error) { return LoadManifestDescriptorWithContext(nil, imgRef) From 6f9f516648ac95845b19c2da0885519be36815e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 5 Feb 2026 20:21:55 +0100 Subject: [PATCH 17/18] Remove CopyOptions.SystemContext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It has no real users (one caller in Podman hard-codes it to nil), and it's unclear how it interacts with RuntimeOptions.SystemContext. Instead, it might make more sense to add more specific override options to CopyOptions, falling back on the runtime's SystemContext if unset. Signed-off-by: Miloslav Trmač --- common/libimage/copier.go | 3 --- common/libimage/push_test.go | 5 +---- common/libimage/runtime_test.go | 10 ++++++++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/common/libimage/copier.go b/common/libimage/copier.go index 3c7d538104..89711b337e 100644 --- a/common/libimage/copier.go +++ b/common/libimage/copier.go @@ -36,9 +36,6 @@ const ( // CopyOptions allow for customizing image-copy operations. type CopyOptions struct { - // If set, will be used for copying the image. Fields below may - // override certain settings. - SystemContext *types.SystemContext // Allows for customizing the source reference lookup. This can be // used to use custom blob caches. SourceLookupReferenceFunc LookupReferenceFunc diff --git a/common/libimage/push_test.go b/common/libimage/push_test.go index f2bc90d971..f9ed77f589 100644 --- a/common/libimage/push_test.go +++ b/common/libimage/push_test.go @@ -12,7 +12,6 @@ import ( "github.com/stretchr/testify/require" "go.podman.io/common/pkg/config" "go.podman.io/image/v5/pkg/compression" - "go.podman.io/image/v5/types" ) func TestPush(t *testing.T) { @@ -99,7 +98,7 @@ func TestPushOtherPlatform(t *testing.T) { } func TestPushWithForceCompression(t *testing.T) { - runtime := testNewRuntime(t) + runtime := testNewRuntime(t, testNewRuntimeOptions{dirForceDecompress: true}) ctx := context.Background() // Prefetch alpine. @@ -116,8 +115,6 @@ func TestPushWithForceCompression(t *testing.T) { // Push newly pulled alpine to directory with uncompressed blobs pushOptions := &PushOptions{} - pushOptions.SystemContext = &types.SystemContext{} - pushOptions.SystemContext.DirForceDecompress = true pushOptions.Writer = os.Stdout dirDest := t.TempDir() _, err = runtime.Push(ctx, "quay.io/libpod/alpine:latest", "dir:"+dirDest, pushOptions) diff --git a/common/libimage/runtime_test.go b/common/libimage/runtime_test.go index 60ae6ed110..5c1d24106a 100644 --- a/common/libimage/runtime_test.go +++ b/common/libimage/runtime_test.go @@ -24,6 +24,7 @@ func TestMain(m *testing.M) { type testNewRuntimeOptions struct { registriesConfPath string + dirForceDecompress bool } // Create a new Runtime that can be used for testing. @@ -41,8 +42,13 @@ func testNewRuntime(t *testing.T, options ...testNewRuntimeOptions) *Runtime { SystemRegistriesConfDirPath: "/dev/null", } - if len(options) == 1 && options[0].registriesConfPath != "" { - systemContext.SystemRegistriesConfPath = options[0].registriesConfPath + if len(options) == 1 { + if options[0].registriesConfPath != "" { + systemContext.SystemRegistriesConfPath = options[0].registriesConfPath + } + if options[0].dirForceDecompress { + systemContext.DirForceDecompress = true + } } runtime, err := RuntimeFromStoreOptions(&RuntimeOptions{SystemContext: systemContext}, storeOptions) From 89ef0bbb7431de10899ea6bd5f601135fd4e4d7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 5 Feb 2026 20:24:30 +0100 Subject: [PATCH 18/18] Pass Runtime.systemContext whenever possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- common/libimage/image.go | 2 +- common/libimage/manifest_list.go | 12 ++++++++---- common/libimage/pull.go | 6 +++--- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/common/libimage/image.go b/common/libimage/image.go index 32995968e7..ea102f81ca 100644 --- a/common/libimage/image.go +++ b/common/libimage/image.go @@ -97,7 +97,7 @@ func (i *Image) isCorrupted(ctx context.Context, name string) error { return err } - img, err := ref.NewImage(ctx, nil) + img, err := ref.NewImage(ctx, &i.runtime.systemContext) if err != nil { if name == "" { name = i.ID()[:12] diff --git a/common/libimage/manifest_list.go b/common/libimage/manifest_list.go index f084bbe55f..eaea9ad60b 100644 --- a/common/libimage/manifest_list.go +++ b/common/libimage/manifest_list.go @@ -235,6 +235,9 @@ func (i *Image) ConvertToManifestList(ctx context.Context) (*ManifestList, error // Copy from the OCI layout into the same image record, so that it gets // both its own manifest and the image index. copyOptions := imageCopy.Options{ + SourceCtx: &i.runtime.systemContext, + DestinationCtx: &i.runtime.systemContext, + ForceManifestMIMEType: imageManifestType, } if _, err := imageCopy.Image(ctx, policyContext, i.storageReference, bundle, ©Options); err != nil { @@ -561,6 +564,9 @@ func (m *ManifestList) AddArtifact(ctx context.Context, options *ManifestListAdd if options == nil { options = &ManifestListAddArtifactOptions{} } + + systemContext := m.image.runtime.systemContextCopy() + opts := manifests.AddArtifactOptions{ ManifestArtifactType: options.Type, Annotations: maps.Clone(options.Annotations), @@ -592,7 +598,7 @@ func (m *ManifestList) AddArtifact(ctx context.Context, options *ManifestListAdd opts.LayerMediaType = &options.LayerType } if options.Subject != "" { - ref, err := m.parseNameToExtantReference(ctx, nil, options.Subject, true, "subject for artifact manifest") + ref, err := m.parseNameToExtantReference(ctx, systemContext, options.Subject, true, "subject for artifact manifest") if err != nil { return "", err } @@ -607,8 +613,6 @@ func (m *ManifestList) AddArtifact(ctx context.Context, options *ManifestListAdd locker.Lock() defer locker.Unlock() - systemContext := m.image.runtime.systemContextCopy() - // Make sure to reload the image from the containers storage to fetch // the latest data (e.g., new or delete digests). if err := m.reload(); err != nil { @@ -709,7 +713,7 @@ func (m *ManifestList) AnnotateInstance(d digest.Digest, options *ManifestListAn } } if options.Subject != "" { - ref, err := m.parseNameToExtantReference(ctx, nil, options.Subject, true, "subject for image index") + ref, err := m.parseNameToExtantReference(ctx, &m.image.runtime.systemContext, options.Subject, true, "subject for image index") if err != nil { return err } diff --git a/common/libimage/pull.go b/common/libimage/pull.go index 1183311f4c..02bde0c630 100644 --- a/common/libimage/pull.go +++ b/common/libimage/pull.go @@ -257,7 +257,7 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, if !ok || refName == "" { // Same trick as for the dir transport: we cannot use // the path to a directory as the name. - storageName, err = getImageID(ctx, ref, nil) + storageName, err = getImageID(ctx, ref, &r.systemContext) if err != nil { return nil, nil, err } @@ -276,7 +276,7 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, switch len(storageName) { case 0: // If there's no reference name in the annotations, compute an ID. - storageName, err = getImageID(ctx, ref, nil) + storageName, err = getImageID(ctx, ref, &r.systemContext) if err != nil { return nil, nil, err } @@ -302,7 +302,7 @@ func (r *Runtime) copyFromDefault(ctx context.Context, ref types.ImageReference, // Path-based transports (e.g., dir) may include invalid // characters, so we should pessimistically generate an ID // instead of looking at the StringWithinTransport(). - storageName, err = getImageID(ctx, ref, nil) + storageName, err = getImageID(ctx, ref, &r.systemContext) if err != nil { return nil, nil, err }