TLS configuration options #619

mtrmac · 2026-01-30T12:52:50Z

This is sufficient, and if users use …/tlscobra, we can change the details without changing the callers again; but we might want to do something else.

mtrmac · 2026-01-30T12:53:13Z

image/AGENTS.md

@@ -0,0 +1,78 @@
+# Commitments


This probably should be a separate PR.

yeah let's move this elsehwere

Yes, it makes sense to put in a separate PR.

Moved to #636 .

mtrmac · 2026-01-30T12:58:02Z

image/pkg/cli/tlsopts/tlscobra/cobra.go

@@ -0,0 +1,98 @@
+// Package tlscobra implements Cobra CLI flags for tlsopts.
+//
+// Recommended flag documentation:


Perhaps we don’t want individual options at all, and instead ask users to point at a config file. (That would also somewhat disincentivize unnecessary and overzealous use of --tls-min-version.)

Perhaps we should read a system-wide config file by default? Probably not; the long-term future should be Go following /etc/crypto-policies (any decade now?), and we don’t want to establish, and start shipping, a config file, only to have to incompatibly remove it later.

Perhaps we should just put this into containers.conf. Skopeo doesn’t currently use that file at all.

Either way, the API of this package can support all of those scenarios, we “only” have to make a decision before tagging a release of any of the users.

If we believe the long-term future should be go following /etc/crypto-policies I agree that putting the tls options in a config file is better than exposing them as flags.

Luap99

I think overall the abstraction is nice, now whenever we actually want all this options or not is a separate conversation I don't feel like pushing back on.

Ultimately I feel like this is wrong no matter how we implement it and should be handled by the go std lib instead but here we are so I am fine merging this with the AGENTS file dropped.

Luap99 · 2026-01-30T15:40:17Z

image/pkg/cli/tlsopts/tlscobra/cobra.go

+
+func (os *onceStringValue) Set(val string) error {
+	if os.present {
+		return errors.New("flag already set")


non blocking

I had to double check since I was not sure if cobra/pflag wraps the right option name becaus ewithout this it would be hard to kjnow for the user which flag was meant but it seems the wrapped message is fine
invalid argument "1.2" for "--tls-min-version" flag: flag already set

so maybe a small comment that cobra wraps the flag name to the error here would be helpful

Luap99 · 2026-01-30T15:42:25Z

image/pkg/cli/tlsopts/tlscobra/cobra_test.go

+	for _, tc := range []struct {
+		args        []string
+		expected    *tls.Config
+		expectError bool


non blocking:
I like to either have the true error type or a string here that actual matches the correct error message via assert.ErrorContains() because only then you can be sure the correct error condition is triggered and we don;t cover the wrong thing

same for the other tls test cases

mtrmac · 2026-02-06T22:34:46Z

OK, a complete rewrite:

Renamed the primary config object from tlsopts.BaseTLSConfig to basetls.Config
To support Podman’s remote operation, the object now exposes MarshalText and UnmarshalText: the CLI can provide a basetls.Config to the engine abstraction, and the engine can either consume the config directly, or send it over the wire. (Alternatively, the config could be set at Podman machine creation, and when starting the remote server; this API just allows for passing the data around, with a decision to be made later.)
The configuration is now primarily in a file, using YAML (matching sigstore-params, more or less to allow comments). Added a centralized man page.
This makes the tlscobra wrappers less necessary, it’s just one option now. And with the way buildah bud is wired into podman build, where the Cobra flag definitions and their consumption are split across different subpackages (I don’t like that much but I have no ambition to change that now), requiring the tlscobra abstraction would be difficult. So, now, programs are expected to define their own CLI flags, and consume them via basetls.NewFromOptionalFile.

PTAL again.

image/pkg/cli/basetls/basetls.go

Luap99 · 2026-02-09T18:49:47Z

To support Podman’s remote operation, the object now exposes MarshalText and UnmarshalText: the CLI can provide a basetls.Config to the engine abstraction, and the engine can either consume the config directly, or send it over the wire. (Alternatively, the config could be set at Podman machine creation, and when starting the remote server; this API just allows for passing the data around, with a decision to be made later.)

I think we are better off not accepting those via remote API. In the case of podman system service you could just add the option to that command to cover all of the service actions.
And in terms of podman buildah I really think we need the containers.conf option anyway.

Looking at your podman PR you have the option defined globally which seem right but then you pass it to remote only on pull? That is broken design wise because both play kube and podman build both send the content via file and the pull is triggered on the remote not via the /pull endpoint

So IMO the only sensible thing never send this thing via remote and get rid of the custom marshal format. Everything else will just end up a security issue if we forget some code path that triggers a remote pull without sending that config.

This makes the tlscobra wrappers less necessary, it’s just one option now. And with the way buildah bud is wired into podman build, where the Cobra flag definitions and their consumption are split across different subpackage

It this not defined once globally for buildah? I don't think adding it on bud would help there either, you have to expose this to buildah from and buildah add (if we care regular about https downloads) so then I don't think this inter dependency should matter.

mtrmac · 2026-02-09T20:16:04Z

I think we are better off not accepting those via remote API.

I’m open to that, it would certainly simplify things a lot. (Maybe we wouldn’t need to worry about ciphers in ssh at all.)

The marshaling would then be remain unused, but I don’t think it hurts to have it (there are fairly comprehensive unit tests).

but then you pass it to remote only on pull?

That part of the Podman PR is really a proof of concept, I was assuming that we’d have to wire every single Podman operation individually. (And it would certainly be easy to forget something, but that’s always going to be the case with the various operation that don’t go through libimage.)

And with the way buildah bud is wired into podman build, where the Cobra flag definitions and their consumption are split across different subpackage

It this not defined once globally for buildah?

It is ~centralized but not encapsulated: compare https://github.com/containers/buildah/blob/0e32de077a526d7e2bfd4f552136357b2f0a1502/pkg/cli/common.go#L251 and https://github.com/containers/buildah/blob/0e32de077a526d7e2bfd4f552136357b2f0a1502/pkg/parse/parse.go#L446 , connected just by a string literal.

I have chosen this flag entirely randomly, and it seems that it might actually be parsed three ways:

For buildah bud, from the variable defined in pkg/cli via BoolVarP
For commit and push, from a variable defined in cmd/buildah/… via BoolVarP
From pkg/parse setting up a SystemContext, by matching a string literal to either of the above

That’s an even stronger argument why the original version of the PR, where a single function returned both a FlagSet and a struct holding a field where the value is to be written, would be harder to integrate into the existing Buildah structure.

mtrmac · 2026-02-09T23:37:18Z

Restructed a bit more: TLSDetailsFile is now in basetls, and the CLI-consuming NewFrom[Optional]File are now tlsdetails.BaseTLSFrom[Optional]File, so that basetls only depends on the standard library.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

github-actions bot added the image Related to "image" package label Jan 30, 2026

mtrmac commented Jan 30, 2026

View reviewed changes

mtrmac force-pushed the tls-options branch from cf886b4 to 66e7ed6 Compare January 30, 2026 13:01

Luap99 reviewed Jan 30, 2026

View reviewed changes

This was referenced Jan 30, 2026

Re-evaluate TLS (and possibly HTTP) settings and defaults used in the codebase #277

Open

Add "BaseTLS" options #623

Draft

mtrmac force-pushed the tls-options branch from 66e7ed6 to 284a3ff Compare February 6, 2026 22:27

mtrmac marked this pull request as ready for review February 6, 2026 22:45

mtrmac force-pushed the tls-options branch from 284a3ff to 690478f Compare February 6, 2026 23:16

mtrmac commented Feb 6, 2026

View reviewed changes

image/pkg/cli/basetls/basetls.go Show resolved Hide resolved

mtrmac mentioned this pull request Feb 9, 2026

Prototype: Add --tls-details, only for non-remote pulling for now containers/podman#28043

Draft

6 tasks

mtrmac force-pushed the tls-options branch from 690478f to 25cb073 Compare February 9, 2026 23:35

mtrmac force-pushed the tls-options branch from 25cb073 to a609277 Compare February 9, 2026 23:38

Add image/pkg/cli/basetls

789ace0

Signed-off-by: Miloslav Trmač <mitr@redhat.com>

mtrmac force-pushed the tls-options branch from a609277 to 789ace0 Compare February 10, 2026 19:22

TLS configuration options #619

Are you sure you want to change the base?

TLS configuration options #619

Uh oh!

Conversation

mtrmac commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Luap99 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtrmac commented Feb 6, 2026

Uh oh!

Uh oh!

Luap99 commented Feb 9, 2026

Uh oh!

mtrmac commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrmac commented Feb 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mtrmac commented Jan 30, 2026 •

edited

Loading

mtrmac commented Feb 9, 2026 •

edited

Loading