Prototype: Add --tls-details

[mtrmac]

This exposes containers/container-libs#623 , looking for ways to do that.

Absolutely untested at this point.

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

• Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
  commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
  the sign-off email address. See CONTRIBUTING.md
  for more information.
• Referenced issues using Fixes: #00000 in commit message (if applicable)
• Tests have been added/updated (or no tests are needed)
• Documentation has been updated (or no documentation changes are needed)
• All commits pass make validatepr (format/lint checks)
• Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

Many subcommands now expose a `--tls-details` option, allowing to tune TLS settings using a `containers-tls-details.yaml(5)` file.

[packit-as-a-service]

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

[mtrmac]

I think this highlights the basic design trade-offs:

• We can add --tls-details as a global option.
  • That automatically forwards it to libimage, and hopefully affects all image operations (but not any other operations), for fairly little work.
  • We will still need to scope/audit all other TLS users, and use the --tls-details option.
  • At a first glance it’s a bit unsatisfactory to have registry.PodmanConfig().TLSDetailsFile contain only the path, not the parsed data. The parsing code would have to be repeated in many users. There is probably a trivially way to do better here, I didn’t spend much time looking.
  • This doesn’t affect remote processes at all; how would the option get to the remote server??!
• We can add it as a per-operation option
  • Rather more tedious, but we would ~clearly delineate where we expect it to work and where it isn’t ready yet.
  • Clearly generalizes to remote operation: add the basetls.Config.MarshalText format as an option field.
  • Right now libimage.CopyOptions ignores the SystemContext field AFAICT. We don’t need to change that, we can add a new libimage.CopyOptions.BaseTLSConfig (perhaps preferring CopyOptions but falling back to the initialization-time runtime SystemContext).

[mtrmac]

Does this really need to be manually, separately from pkg/bindings/images.PullOptions? it It seems tedious and error-prone.

[mheon]

We can't guarantee the bindings are always used with the API? But maybe I'm misunderstanding the question

[mtrmac]

I was expecting some kind of

var query pkg/bindings/images.PullOptions
if err := query.FromParams(r.URL.Query()); …

using exactly the same type on the client and server side, not a duplicate re-declaration; and mirroring the existing query.ToParams.

[mtrmac]

OK a minimal smoke test is promising:

# cat tls-details-pqc-only.yaml
minVersion: "1.3"
namedGroups:
  - "X25519MLKEM768"
# bin/podman --tls-details tls-details-pqc-only.yaml pull --retry 0 quay.io/foo
Trying to pull quay.io/foo:latest...
Error: unable to copy from source docker://quay.io/foo:latest: initializing source docker://quay.io/foo:latest: pinging container registry quay.io: Get "https://quay.io/v2/": EOF
# bin/podman  --tls-details tls-details-pqc-only.yaml push --retry 0 quay.io/libpod/alpine
Getting image source signatures
Error: trying to reuse blob sha256:03901b4a2ea88eeaad62dbe59b072b28b6efa00491962b8741081c5df50c65e0 at destination: pinging container registry quay.io: Get "https://quay.io/v2/": EOF

("EOF" is how the quay.io server reacts to the inability to negotiate a PQC group.)

Note that this does not yet include even login, which is critical to protect (but hopefully irrelevant to OpenShift).

[mtrmac]

@containers/podman-maintainers RFC on the approach. (In containers/container-libs#619 , Paul suggests not supporting the option for remote at all, or maybe exposing it as an option for the remote server only. That would certainly simplify things.)

[Luap99]

or maybe exposing it as an option for the remote server only.

Well my point is that it is already exposed for the server because it is a global option, podman --tls-details blah system service should work with that logic as it creates the same libpod (and thus libimage) runtime that a podman pull/run would also use. So in that sense this should work?

[mtrmac]

podman --tls-details blah system service should work with that logic

I think so, the way the current version of this PR initializes the runtime.

I suppose machine creation would still need extra code to copy the file + set the flag, or maybe we would leave that for users to do manually.

[mtrmac]

Updated:

• Analysis still in progress (Buildah, and looking for SystemContext in Podman TBD)
• Inherited from container-libs: known to miss handling of Fulcio, Rekor, OIDC when creating sigstore signatures
• All absolutely untested, other than the one smoke test quoted above.
• Assumes that we will configure libimage once, and not add an option to every single remote API

At this point I think this is valuable to understand the scope of the option — perhaps just by titles of the commits .

[mtrmac]

xref-helpmsgs-manpages: 'podman build --help' lists '--tls-details', which is not in docs/source/markdown/podman-build.1.md

I’d welcome comments on the Try to handle (build) with --tls-details commit. I think that should handle the option without surprises to the user, but it is very clumsy and I can’t help thinking similar sharing of configuration between Buildah and Podman must have been an issue previously. How is it handled elsewhere?

[mheon]

Do we need to add proxy info here or does that carry over from the default transport if not explicitly overridden?

[mtrmac]

Oops, yes we do. I’ll need to fix that everywhere.

[mtrmac]

Fixed, along with nil dereferences due to Go’s interface type weirdness.

[mheon]

Approach LGTM but this looks like a real pain to bring back to earlier versions which I really do not like.

[mheon]

(Though I also see no real alternative...)

[mtrmac]

Approach LGTM

The approach where we set --tls-details at process level, and don’t have it in the API, or the approach where we individually add a new parameter to many API operations?

(Paul preferred the latter, and I do agree that would be much easier.)

[mtrmac]

This is now the full scope of everything I could find.