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>
refreshImage is the only function used as a reducer and it doesn't use
the `filter *listContext`.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Aggregate same images into one object and add the list of tags pointing
to it to the RepoTags array
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
- use assert.Check to continue the test even if a check fails
- assert the total number of images returned, not only their RepoTags
- use subtests
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Check for generic `errdefs.NotFound` rather than specific error helper
struct when checking if the error is caused by the image not being
present.
It still works for `ErrImageDoesNotExist` because it
implements the NotFound errdefs interface too.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This variable was only accessed from within LocalRegistry methods, but
due to being a package-level variable, tests had to deal with setting
and resetting it.
Move it to be a field scoped to the LocalRegistry. This simplifies the
tests, and to make this more transparent, also removing the "Setup()"
helper (which, wasn't marked as a t.Helper()).
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The client's transport can only be set by newClientWithTransport, which
is not exported, and always uses a transport.HTTPTransport.
However, requestFactory is mocked in one of the tests, so keep the interface,
but make it a local, non-exported one.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The interface is not consumed anywhere, and only non-exported functions
produced one, so we can remove it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This field was exported, but never mutated outside of the package, and
effectively a rather "creative" way to define a method on LocalRegistry.
While un-exporting also store these paths in a field, instead of constructing
them on every call, as the results won't change during the lifecycle of the
LocalRegistry.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Split the exported SpecsPaths from the platform-specific implementations,
so that documentation can be maintained in a single location.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Since 0e5eaf8ee3, these implementations
were fully identical, so removing the duplicate, and move it to a
platform-agnostic file.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
- use gotest.tools assertions
- use consts and struct-literals where possible
- use assert.Check instead of t.Fatal() where possible
- fix some unhandled errors
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function was used by the postNetworkConnect() handler, but is handled
by the backend itself, starting with d63a5a1ff5.
Since that commit, this function was no longer used, so we can remove it.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The rootChain variable that the Key function references is a
package-global slice. As the append() built-in may append to the slice's
backing array in place, it is theoretically possible for the temporary
slices in concurrent Key() calls to share the same backing array, which
would be a data race. Thankfully in my tests (on Go 1.20.6)
cap(rootChain) == len(rootChain)
held true, so in practice a new slice is always allocated and there is
no race. But that is a very brittle assumption to depend upon, which
could blow up in our faces at any time without warning. Rewrite the
implementation in a way which cannot lead to data races.
Signed-off-by: Cory Snider <csnider@mirantis.com>
Before this change, integration test would fail fast and not execute all
test suites when one suite fails.
Change this behavior into opt-in enabled by TEST_INTEGRATION_FAIL_FAST
variable.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
It only had a single implementation, so let's remove the interface.
While changing, also renaming;
- datastore.DataStore -> datastore.Store
- datastore.NewDataStore -> datastore.New
- datastore.NewDataStoreFromConfig -> datastore.FromConfig
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
