Commit Graph

37 Commits

Author SHA1 Message Date
Sebastiaan van Stijn 0dd6f7f1b3
cil/config/credentials: remove newStore() test-utility
This function was names slightly confusing, as it returns a fakeStore,
and it didn't do any constructing, so didn't provide value above just
constructing the type.

I'm planning to add more functionality to the fakeStore, but don't want
to maintain a full-fledged constructor for all of that, so let's remove
this utility.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-22 10:58:29 +02:00
Sebastiaan van Stijn d3f6867e4d
cli/config/credentials: skip saving config-file if credentials didn't change
Before this change, the config-file was always updated, even if there
were no changes to save. This could cause issues when the config-file
already had credentials set and was read-only for the current user.

For example, on NixOS, this poses a problem because `config.json` is a
symlink to a write-protected file;

    $ readlink ~/.docker/config.json
    /home/username/.config/sops-nix/secrets/ghcr_auth

    $ readlink -f ~/.docker/config.json
    /run/user/1000/secrets.d/28/ghcr_auth

Which causes `docker login` to fail, even if no changes were to be made;

    Error saving credentials: rename /home/derek/.docker/config.json2180380217 /home/username/.config/sops-nix/secrets/ghcr_auth: invalid cross-device link

This patch updates the code to only update the config file if changes
were detected. It there's nothing to save, it skips updating the file,
as well as skips printing the warning about credentials being stored
insecurely.

With this patch applied:

    $ docker login -u yourname
    Password:

    WARNING! Your credentials are stored unencrypted in '/root/.docker/config.json'.
    Configure a credential helper to remove this warning. See
    https://docs.docker.com/go/credential-store/

    Login Succeeded

    $ docker login -u yourname
    Password:
    Login Succeeded

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-21 00:19:52 +02:00
Laura Brehm 5eb3275c28
filestore: don't print warning multiple times
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
2024-08-14 19:48:08 +01:00
Sebastiaan van Stijn ab80ea355f
cli/config/credentials: move warning to fileStore
The fileStore itself is aware that it's insecure, so we can make it
responsible for printing the warning. It's not "perfect", as we use
`os.Stderr` unconditionally (not `dockerCli.Err()`), but probably won't
make a difference in _most_ cases.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-07-19 01:42:01 +02:00
Sebastiaan van Stijn 8b0a7b025d
cli/config/credentials: ConvertToHostname: handle IP-addresses
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-06-26 13:43:02 +02:00
Carston Schilds 2380481609 re-introduced support for port numbers in docker registry URL
Signed-off-by: Carston Schilds <Carston.Schilds@visier.com>
2024-06-25 15:36:31 -07:00
Sebastiaan van Stijn 8376b3e428
use local ConvertToHostname() implementation
Commit 27b2797f7d added a local implementation
of this function, so let's use the local variant to (slightly) reduce the
dependency on moby's registry package.

Also made some minor cleanups.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-06-14 00:25:21 +02:00
Eric Bode b24e7f85a4
Fix setting ServerAddress property in NativeStore
This will return the ServerAddress property when using the NativeStore.
This happens when you use docker credential helpers, not the credential
store.

The reason this fix is needed is because it needs to be propagated
properly down towards `moby/moby` project in the following logic:

```golang
func authorizationCredsFromAuthConfig(authConfig registrytypes.AuthConfig) docker.AuthorizerOpt {
	cfgHost := registry.ConvertToHostname(authConfig.ServerAddress)
	if cfgHost == "" || cfgHost == registry.IndexHostname {
		cfgHost = registry.DefaultRegistryHost
	}

	return docker.WithAuthCreds(func(host string) (string, string, error) {
		if cfgHost != host {
			logrus.WithFields(logrus.Fields{
				"host":    host,
				"cfgHost": cfgHost,
			}).Warn("Host doesn't match")
			return "", "", nil
		}
		if authConfig.IdentityToken != "" {
			return "", authConfig.IdentityToken, nil
		}
		return authConfig.Username, authConfig.Password, nil
	})
}
```
This logic resides in the following file :
`daemon/containerd/resolver.go` .

In the case when using the containerd storage feature when setting the
`cfgHost` variable from the `authConfig.ServerAddress` it will always be
empty. Since it will never be returned from the NativeStore currently.
Therefore Docker Hub images will work fine, but anything else will fail
since the `cfgHost` will always be the `registry.DefaultRegistryHost`.

Signed-off-by: Eric Bode <eric.bode@foundries.io>
2023-11-11 14:22:23 +01:00
Danial ad43df5e86
configfile: Initialize nil AuthConfigs
Initialize AuthConfigs map if it's nil before returning it.
This fixes fileStore.Store nil dereference panic when adding a new key
to the map.

Signed-off-by: Danial Gharib <danial.mail.gh@gmail.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2023-07-19 15:36:10 +02:00
Sebastiaan van Stijn 4cf04988ae
remove uses of golang.org/x/sys/execabs
the "golang.org/x/sys/execabs" package was introduced to address a security
issue on Windows, and changing the default behavior of os/exec was considered
a breaking change. go1.19 applied the behavior that was previously implemented
in the execabs package;

from the release notes: https://go.dev/doc/go1.19#os-exec-path

> Command and LookPath no longer allow results from a PATH search to be found
> relative to the current directory. This removes a common source of security
> problems but may also break existing programs that depend on using, say,
> exec.Command("prog") to run a binary named prog (or, on Windows, prog.exe)
> in the current directory. See the os/exec package documentation for information
> about how best to update such programs.
>
> On Windows, Command and LookPath now respect the NoDefaultCurrentDirectoryInExePath
> environment variable, making it possible to disable the default implicit search
> of “.” in PATH lookups on Windows systems.

With those changes, we no longer need to use the execabs package, and we can
switch back to os/exec.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-05-26 02:03:45 +02:00
Sebastiaan van Stijn 10bade23e1
Merge pull request #4261 from thaJeztah/remove_old_buildtags
remove pre-go1.17 build-tags
2023-05-16 18:12:50 +01:00
Sebastiaan van Stijn ce11b28d83
cli/config/credentials: skip unneeded exec.LookPath()
defaultCredentialsStore() on Linux does an exec.LookPath() for "pass", but
if a custom credential-store is passed to DetectDefaultStore, the result
of that won't be used.

This patch changes the logic to return early if a custom credential-store
is passed.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-05-05 21:10:18 +02:00
Sebastiaan van Stijn 2ae223038c
remove pre-go1.17 build-tags
Removed pre-go1.17 build-tags with go fix;

    go mod init
    go fix -mod=readonly ./...
    rm go.mod

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2023-05-05 18:23:03 +02:00
Sebastiaan van Stijn c8bd8932a1
cli/config: use strings.Cut
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-12-29 15:19:31 +01:00
Sebastiaan van Stijn d7c1fb9112
linting: ignore some "G101: Potential hardcoded credentials" warnings
cli/config/credentials/native_store.go:10:2: G101: Potential hardcoded credentials (gosec)
        remoteCredentialsPrefix = "docker-credential-"
        ^
    cli/command/service/opts.go:917:2: G101: Potential hardcoded credentials (gosec)
        flagCredentialSpec          = "credential-spec"
        ^

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-03-28 10:37:22 +02:00
Sebastiaan van Stijn a0f0578299
gofmt with go1.17
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-03-26 20:21:00 +01:00
Sebastiaan van Stijn 71575ab3b5
cli/config: remove deprecated io/ioutil and use t.TempDir()
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2022-02-25 15:42:15 +01:00
Sebastiaan van Stijn f3886f354a
Use designated test domains (RFC2606) in tests
Some tests were using domain names that were intended to be "fake", but are
actually registered domain names (such as mycorp.com).

Even though we were not actually making connections to these domains, it's
better to use domains that are designated for testing/examples in RFC2606:
https://tools.ietf.org/html/rfc2606

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2021-04-30 10:03:45 +02:00
Tibor Vass 8d199d5bba Use golang.org/x/sys/execabs
On Windows, the os/exec.{Command,CommandContext,LookPath} functions
resolve command names that have neither path separators nor file extension
(e.g., "git") by first looking in the current working directory before
looking in the PATH environment variable.
Go maintainers intended to match cmd.exe's historical behavior.

However, this is pretty much never the intended behavior and as an abundance of precaution
this patch prevents that when executing commands.
Example of commands that docker.exe may execute: `git`, `docker-buildx` (or other cli plugin), `docker-credential-wincred`, `docker`.

Note that this was prompted by the [Go 1.15.7 security fixes](https://blog.golang.org/path-security), but unlike in `go.exe`,
the windows path lookups in docker are not in a code path allowing remote code execution, thus there is no security impact on docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
2021-01-26 17:18:04 +00:00
Sebastiaan van Stijn 2c0e93063b
bump gotest.tools v3.0.1 for compatibility with Go 1.14
full diff: https://github.com/gotestyourself/gotest.tools/compare/v2.3.0...v3.0.1

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-02-23 00:28:55 +01:00
Tonis Tiigi 27b2797f7d Remove docker api dependency from cli/config
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
2019-01-31 21:25:43 +00:00
Euan Kemp 056015c3d8 config/credentials: don't run 'pass' to detect it
'CheckInitialized' in the credential-helper library actually invokes
`pass`, which isn't desirable (see #699).

This moves the check to be simpler, and then pass will only be invoked
when it's needed (such as for `docker login` or when pulling from a
private registry).

This logic could also reasonably live in the credential-helper library,
but it's simple enough it seems fine in either location.

Signed-off-by: Euan Kemp <euank@euank.com>
2018-06-29 11:38:39 -07:00
Vincent Demeester d9741fc96b
Update docker-credential-helpers dependency
This is mainly for the `pass` helper ; `pass` shouldn't be called
every docker command anymore ;).

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
2018-06-28 14:44:53 +02:00
Vincent Demeester 2c4de4fb5e
Update tests to use gotest.tools 👼
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
2018-06-08 18:24:26 +02:00
Vincent Demeester 4290df3958 print a big warning about cleartext passwords
Signed-off-by: Tycho Andersen <tycho@docker.com>
2018-03-26 16:18:32 +02:00
Daniel Nephin baf65a5502 Convert to assert.NilError
Using:

  git grep -l '^\s\+assert\.Check(t, err)$' | \
    xargs sed -i -e 's/^\(\s\+assert\)\.Check(t, err)$/\1.NilError(t, err)/'

Signed-off-by: Daniel Nephin <dnephin@docker.com>
2018-03-06 15:27:34 -05:00
Daniel Nephin 681c921528 Remove testutil
Signed-off-by: Daniel Nephin <dnephin@docker.com>
2018-03-06 14:38:35 -05:00
Daniel Nephin 39c2ca57c1 Automated migration
Signed-off-by: Daniel Nephin <dnephin@docker.com>
2018-03-05 19:41:17 -05:00
Ivan Markin 57c51c8af3 Fix compilation of defaultCredentialStore() on unsupported platforms
Signed-off-by: Ivan Markin <sw@nogoegst.net>
2018-02-10 15:30:32 +00:00
Tycho Andersen 713e87e780 prefer `pass` backend if it is available
Signed-off-by: Tycho Andersen <tycho@docker.com>
2017-09-20 11:15:49 -06:00
Tycho Andersen 4cf1849418 defaultCredentialStore: make this a function
In the next patch, we'll use this to implement some logic about which
password backend to use.

Signed-off-by: Tycho Andersen <tycho@docker.com>
2017-09-20 11:14:27 -06:00
Daniel Nephin 846a31aa50 Use new internal testutil.ErrorContains()
Signed-off-by: Daniel Nephin <dnephin@docker.com>
2017-08-22 10:14:25 -04:00
Daniel Nephin 33cbb70270
Cleanup config/credentials, remove dependency on config file.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
2017-06-27 13:45:50 +02:00
Daniel Nephin a8c70e43a3
Move config file loading to more appropriate packages.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
2017-06-27 13:45:50 +02:00
Daniel Nephin 3724fb7f37 Add gosimple lint
Signed-off-by: Daniel Nephin <dnephin@docker.com>
2017-06-14 16:55:08 -07:00
Daniel Nephin 10641c2aae Update imports.
Signed-off-by: Daniel Nephin <dnephin@docker.com>
2017-04-17 18:07:56 -04:00
Daniel Nephin 1630fc40f8 Import docker/docker/cli
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
2017-04-17 17:40:59 -04:00