Go 1.15.7 contained a security fix for CVE-2021-3115, which allowed arbitrary
code to be executed at build time when using cgo on Windows.
This issue was not limited to the go command itself, and could also affect binaries
that use `os.Command`, `os.LookPath`, etc.
From the related blogpost (https://blog.golang.org/path-security):
> Are your own programs affected?
>
> If you use exec.LookPath or exec.Command in your own programs, you only need to
> be concerned if you (or your users) run your program in a directory with untrusted
> contents. If so, then a subprocess could be started using an executable from dot
> instead of from a system directory. (Again, using an executable from dot happens
> always on Windows and only with uncommon PATH settings on Unix.)
>
> If you are concerned, then we’ve published the more restricted variant of os/exec
> as golang.org/x/sys/execabs. You can use it in your program by simply replacing
At time of the go1.15 release, the Go team considered changing the behavior of
`os.LookPath()` and `exec.LookPath()` to be a breaking change, and made the
behavior "opt-in" by providing the `golang.org/x/sys/execabs` package as a
replacement.
However, for the go1.19 release, this changed, and the default behavior of
`os.LookPath()` and `exec.LookPath()` was changed. 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.
A result of this change was that registering the daemon as a Windows service
no longer worked when done from within the directory of the binary itself:
C:\> cd "Program Files\Docker\Docker\resources"
C:\Program Files\Docker\Docker\resources> dockerd --register-service
exec: "dockerd": cannot run executable found relative to current directory
Note that using an absolute path would work around the issue:
C:\Program Files\Docker\Docker>resources\dockerd.exe --register-service
This patch changes `registerService()` to use `os.Executable()`, instead of
depending on `os.Args[0]` and `exec.LookPath()` for resolving the absolute
path of the binary.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 3e8fda0a70)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The image store sends events when a new image is created/tagged, using
it instead of the reference store makes sure we send the "tag" event
when a new image is built using buildx.
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
GRPC is logging a *lot* of garbage at info level.
This configures the GRPC logger such that it is only giving us logs when
at debug level and also adds a log field indicating where the logs are
coming from.
containerd is still currently spewing these same log messages and needs
a separate update.
Without this change `docker build` is extremely noisy in the daemon
logs.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit c7ccc68b15)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
dockerd handles SIGQUIT by dumping all goroutine stacks to standard
error and exiting. In contrast, the Go runtime's default SIGQUIT
behaviour... dumps all goroutine stacks to standard error and exits.
The default SIGQUIT behaviour is implemented directly in the runtime's
signal handler, and so is both more robust to bugs in the Go runtime and
does not perturb the state of the process to anywhere near same degree
as dumping goroutine stacks from a user goroutine. The only notable
difference from a user's perspective is that the process exits with
status 2 instead of 128+SIGQUIT.
Signed-off-by: Cory Snider <csnider@mirantis.com>
(cherry picked from commit 0867d3173c)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This centralizes more defaults, to be part of the config struct that's
created, instead of interweaving the defaults with other code in various
places.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit b28e66cf4f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This is only used for tests, and the key is not verified anymore, so
instead of creating a key and storing it, we can just use an ad-hoc
one.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 8feeaecb84)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Turned out that the loadOrCreateTrustKey() utility was doing exactly the
same as libtrust.LoadOrCreateTrustKey(), so making it a thin wrapped. I kept
the tests to verify the behavior, but we could remove them as we only need this
for our integration tests.
The storage location for the generated key was changed (again as we only need
this for some integration tests), so we can remove the TrustKeyPath from the
config.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 5cdd6ab7cd)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This fix tries to address issues raised in #44346.
The max-concurrent-downloads and max-concurrent-uploads limits are applied for the whole engine and not for each pull/push command.
Signed-off-by: Luis Henrique Mulinari <luis.mulinari@gmail.com>
(cherry picked from commit 6c0aa5b00a)
Signed-off-by: Cory Snider <csnider@mirantis.com>
Previously we waited for 60 seconds after the service faults to restart
it. However, there isn't much benefit to waiting this long. We expect
15 seconds to be a more reasonable delay.
Co-Authored-by: Kevin Parsons <kevpar@microsoft.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 624daf8d9e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This is the equivalent of the local implementation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 3c585e6567)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
golang.org/x/sys/windows now implements this, so we can use that
instead of a local implementation.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 6176ab5901)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `IsAnInteractiveSession` was deprecated, and `IsWindowsService` is marked
as the recommended replacement.
For details, see 280f808b4a
> CL 244958 includes isWindowsService function that determines if a
> process is running as a service. The code of the function is based on
> public .Net implementation.
>
> IsAnInteractiveSession function implements similar functionality, but
> is based on an old Stackoverflow post., which is not as authoritative
> as code written by Microsoft for their official product.
>
> This change copies CL 244958 isWindowsService function into svc package
> and makes it public. The intention is that future users will prefer
> IsWindowsService to IsAnInteractiveSession.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit ffcddc908e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cmd/dockerd/trap/trap_linux_test.go:29:29: empty-lines: extra empty line at the end of a block (revive)
cmd/dockerd/daemon.go:327:35: empty-lines: extra empty line at the start of a block (revive)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit f63dea4337)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
After discussing in the maintainers meeting, we concluded that Slowloris attacks
are not a real risk other than potentially having some additional goroutines
lingering around, so setting a long timeout to satisfy the linter, and to at
least have "some" timeout.
libnetwork/diagnostic/server.go:96:10: G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)
srv := &http.Server{
Addr: net.JoinHostPort(ip, strconv.Itoa(port)),
Handler: s,
}
api/server/server.go:60:10: G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)
srv: &http.Server{
Addr: addr,
},
daemon/metrics_unix.go:34:13: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
if err := http.Serve(l, mux); err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
^
cmd/dockerd/metrics.go:27:13: G114: Use of net/http serve function that has no support for setting timeouts (gosec)
if err := http.Serve(l, mux); err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
^
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 55fd77f724)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The `-g` / `--graph` options were soft deprecated in favor of `--data-root` in
261ef1fa27 (v17.05.0) and at the time considered
to not be removed. However, with the move towards containerd snapshotters, having
these options around adds additional complexity to handle fallbacks for deprecated
(and hidden) flags, so completing the deprecation.
With this patch:
dockerd --graph=/var/lib/docker --validate
Flag --graph has been deprecated, Use --data-root instead
unable to configure the Docker daemon with file /etc/docker/daemon.json: merged configuration validation from file and command line flags failed: the "graph" config file option is deprecated; use "data-root" instead
mkdir -p /etc/docker
echo '{"graph":"/var/lib/docker"}' > /etc/docker/daemon.json
dockerd --validate
unable to configure the Docker daemon with file /etc/docker/daemon.json: merged configuration validation from file and command line flags failed: the "graph" config file option is deprecated; use "data-root" instead
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit b58de39ca7)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Older versions of Go don't format comments, so committing this as
a separate commit, so that we can already make these changes before
we upgrade to Go 1.19.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 52c1a2fae8)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Set the defaults when constructing the config, instead of setting them
indirectly through the command-line flags.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function depends on flags having been parsed before it's used;
add a safety-net in case this function would be called before that.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This prevents creating a socket and touching the filesystem before
trying to use a port that was already in use by a container.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The LoopkupImage method is only used by the inspect image route and
returns an api/type struct. The depenency to api/types of the
daemon/images package is wrong, the daemon doesn't need to know about
the api types.
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Use the default (0) value to indicate "not set", which simplifies
working with these configuration options, preventing the need to
use intermediate variables etc.
While changing this code, also making some small cleanups, such
as replacing "fmt.Sprintf()" for "strconv" variants.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
spf13/pflag now provides this out of the box, so no need to implement
and use our own value-type for this.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This file was originally part of the work to support Solaris, and
there's nothing "not common unix" anymmore, so merging the files.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
installConfigFlags already has separate implementations for Linux and
Windows, so no need to further differentiate.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The installCommonConfigFlags() function is meant for flags that are
supported by all platforms, so removing it from that function.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Previously, the API server configuration would be initialized and
validated when starting the API. Because of this, invalid configuration
(e.g. missing or invalid TLS certificates) would not be detected
when using `dockerd --validate`.
This patch moves creation of the validation earlier, so that it's
validated as part of `dockerd --validate`.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Previously, hosts were de-duplicated and normalized when starting
the API server (in `loadListeners()`), which meant that errors could
occur in that step (but not detected when using `dockerd --validate`),
as well as the list of hosts in the config not matching what would
actually be used (i.e., if duplicates were present).
This patch extracts the de-duplicating to a separate function, and
executes it as part of loading the daemon configuration, so that we
can fail early.
Moving this code also showed that some of this validation depended
on `newAPIServerConfig()` modifying the configuration (adding an
empty host if none was set) in order to have the parsing set a
default. This code was moved elsewhere, but a TODO comment added
as this logic is somewhat sketchy.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- un-export `daemonOptions.InstallFlags()`; `daemonOptions` itself isn't exported,
not exported, and `InstallFlags()` isn't matching any interface and only used
internally.
- un-export `daemonOptions.SetDefaultOptions()` and remove the `flags` argument
as we were passing `daemonOptions.flags` as argument on a method attached to
`daemonOptions`, which was somewhat backwards. While at it, also removing an
intermediate variable that wasn't needed.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Log-level validation was previously performed when configuring the daemon-logs;
this moves the validation to config.Validate() so that we can catch invalid
settings when running dockerd --validate.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This was introduced in 85572cac14, where I
probably forgot to remove this code from an earlier iteration (I decided
that having an explicit `configureCertsDir()` function call for this would
make it more transparent that we're re-configuring a default).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The Logging boolean was unconditionally set to true and ignored in all locations,
except for enabling the debugging middleware, which was also gated by the active
logrus logging level.
While it could make sense to have a Loglevel option configured on the API server,
we don't have this currently, and to make that actually useful, that config would
need to be tollerated by all locations that produce logs (which isn't the case
either).
Looking at the history of this option; a boolean to disable logging was originally
added in commit c423a790d6, which hard-coded it to
"disabled" in a test, and "enabled" for the API server outside of tests (before
that commit, logging was always enabled).
02ddaad5d9 and 5c42b2b512
changed the hard-coded values to be configurable through a `Logging` env-var (env-
vars were used _internally_ at the time to pass on options), which later became
a configuration struct in a0bf80fe03.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
