containers / image

Work with containers' images
Apache License 2.0
862 stars 375 forks source link

Support more arbitrary credential helper executable names? #2449

Open tony-sol opened 3 months ago

tony-sol commented 3 months ago

I was looking how to use custom credentials helper along with podman, e.g. using shell script wrapper for access to macos keychain via security.

After some research i found out, that calling credentials helper looks like this

helperName := fmt.Sprintf("docker-credential-%s", credHelper)
p := helperclient.NewShellProgramFunc(helperName)
creds, err := helperclient.Get(p, registry)

which is, technically, allows to use any executable file with docker-credential- prefix if it is in $PATH and supports defined cli.

So, is it possible to get rid of the need for this prefix, like, use fallback from "docker-credential-%s" to "%s"?

This behavior would allow to use any executable, regardless of its name, in credential-helpers list, for example:

credential-helpers = ["security", "containers-auth.json"]

this would provide helpers chain:

  1. exec docker-credential-security
  2. exec security
  3. read auth.json

Possible downside of this, if existence of docker-credential-security executable in $PATH would change between credStore operations, like store would call docker-credential-security and get would call security due to docker-credential-* renamed or removed. But looks like this is convenient behavior now.

So, is it possible (or am i allowed😉) to implement that or there are more downside, which i don't see?

mtrmac commented 3 months ago

Thanks for reaching out!

It seems to me that adding that lookup would just be confusing. Even in this motivating example, creating a binary named security: security alone means nothing, whereas docker-credential-security gives a fairly strong idea of what the command is for. Why is this namespacing a bad thing?

This example looks bad especially on macOS, where /usr/bin/security actually exists, as you know. And the helpers are being looked up in PATH. AFAICS there would just not be any practical way to set up a workstation: if the credential helper were first in PATH, scripts that expect the ordinary systemwide security command would start failing; if /usr/bin/security were first in PATH, the credential helper would not get executed.

The physical cost of an extra syscall is not all that much, but it is still non-zero, and I don’t think there really is a benefit.


One possible alternative might be to allow an absolute path (starting with /) to be included in the helper list.

I think that would work well in the registries.conf credential-helpers option.

In .docker/config.json, it’s possible that other tools might be reading the helper list, and they would not understand the new convention. (In fact using the .docker/config.json file is typically motivated by interoperability with such tools.) So adding the new semantics to that field is not obviously attractive.

tony-sol commented 3 months ago

About security it is just an example, it more like would be security-wrapper or keychain-wrapper or any else, which, ofc, won't shadowing binaries in $PATH.

I thought about absolute path, but didn't sure about a couple moments, like:

Also trying to find struct of ~/.docker/config.json to figure out how credsStore processed in docker, but can't for now

mtrmac commented 3 months ago

So you do want to have some prefix/suffix of the executable name, just for some reason not the default docker-credential- one? This is mostly the aesthetics of the thing?

tony-sol commented 3 months ago

@mtrmac kinda, yes