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/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/import.go b/common/libimage/import.go index 54a77e4e95..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(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/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 } 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) diff --git a/common/pkg/download/download.go b/common/pkg/download/download.go index 777a115220..41a106c39f 100644 --- a/common/pkg/download/download.go +++ b/common/pkg/download/download.go @@ -1,22 +1,50 @@ 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(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) } defer tmp.Close() + succeeded := false + defer func() { + if !succeeded { + os.Remove(tmp.Name()) + } + }() - response, err := http.Get(source) // nolint:noctx + 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) + } + response, err := client.Do(req) if err != nil { return "", fmt.Errorf("downloading %s: %w", source, err) } @@ -27,5 +55,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 } diff --git a/image/docker/daemon/client.go b/image/docker/daemon/client.go index 004a95fe77..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,27 +67,62 @@ 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, - // In general we want to follow docker/daemon/client.defaultHTTPClient , as long as it doesn’t affect compatibility. + 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. MaxIdleConns: 6, @@ -98,7 +137,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, diff --git a/image/docker/daemon/client_test.go b/image/docker/daemon/client_test.go index 74cd1eac55..53c35c7264 100644 --- a/image/docker/daemon/client_test.go +++ b/image/docker/daemon/client_test.go @@ -1,13 +1,18 @@ package daemon import ( + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" "net/http" - "os" "path/filepath" "testing" 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" ) @@ -23,11 +28,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, } @@ -42,46 +45,95 @@ 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 + wantCAIncluded bool + wantCerts int + }{ + {&types.SystemContext{ + DockerDaemonCertPath: filepath.Join("testdata", "certs"), + DockerDaemonInsecureSkipTLSVerify: true, + }, true, false, 1}, + {&types.SystemContext{DockerDaemonInsecureSkipTLSVerify: true}, true, false, 0}, + {&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}, } - - _, err := tlsConfig(ctx) - assert.ErrorContains(t, err, "could not read CA certificate") -} - -func TestTlsConfigFromCertPath(t *testing.T) { - testDir := testDir(t) - - ctx := &types.SystemContext{ - DockerDaemonCertPath: filepath.Join(testDir, "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) + + 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) + + if c.ctx.BaseTLSConfig != nil { + assert.Equal(t, c.ctx.BaseTLSConfig.MinVersion, tlsCfg.MinVersion) + } } - 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) { - // testDir := testDir(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"}, + {&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) } - - 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) { @@ -98,11 +150,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 -} 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 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, 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/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/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) 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 != "" { 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 8e73de9937..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,13 +30,7 @@ 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.) - +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) @@ -45,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 } 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) + } +} 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) 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"),