Remove "Docker Content Trust"

[tianon]

As an opener, I want to make it clear that this is 100% a self-motivated change; I am not making this change on behalf of Docker Inc, nor does my opinion here represent Docker Inc in any official capacity (ie, if you're writing a splashy news article about this, you're barking up the wrong tree by attributing it officially to Docker Inc; I'm "rogue" here / doing this on my own time as a personally interested party).

My biggest motivation for making this proposal is frankly the state of the upstream Notary (v1) project. It has been completely unmaintained for at least a full year, and mostly unmaintained for quite a few years (ref https://github.com/notaryproject/notary/pulls?q=is%3Apr + notaryproject/.github#70). No matter what value this feature might have once had, it currently is vastly overshadowed by mass bitrot, and it is my argument/opinion that we are actively doing the community a very large disservice (and even harm) by continuing to "support" the feature in the Docker CLI.

Given the state of the upstream project, it is my belief that this should qualify for an exception to our regular "deprecation" process such that we remove the feature immediately, and IMO we could very reasonably even consider backporting this deprecation to any past supported branches.

Arguably, we should have some official means of integrating other "trusted image" solutions into the Docker platform, but IMO those belong in the Engine (unlike DCT which is entirely implemented in the CLI), and I see that (more complex) discussion as orthogonal to removing this bitrot.

There are still quite a few TODO items here (most notably that we probably need some period of time with no-op/warning/erroring --disable-content-trust=xxx flags and deprecation documentation). I'm also certain I missed a few things, as I was mostly doing a pretty serious hack job to see how difficult this would be, not focused on creating a 100% optimal change (and this touches so many parts of the codebase that it's frankly more than I'm comfortable determining by myself whether I've made the changes correctly anyways).

[tianon]

Heh, well, proof's in the pudding (I've lit CI up nice and red to prove that I've clearly missed some things).

I'm also definitely confused by go mod tidy with this change, because it wants to add github.com/docker/libtrust to the list of "required" modules after I remove all this, apparently thanks to github.com/docker/distribution/registry/client.test, which is certainly annoying, to say the least, but extremely odd at best.

[thaJeztah]

❤️ I think I had a branch similar like this at some point.

The "trust" code is definitely interweaved in many places; while no decision is made yet on the fate of DCT (still waiting on direction), I did start to somewhat untangle the trust code from "non-trust", with the potential to add a compile-time build-tag to disable it (replacing DCT code with stubs possibly). Some are merged, but I have some WIP changes stashed locally;

• cli/command: internalize and deprecate Cli.NotaryClient #5885
• cli/command: deprecate Cli.ManifestStore, Cli.RegistryClient #5889
• move some trust-related code to trust package #5894
• cli/command, cil/command/image: remove deprecated methods and functions #5876

As to libtrust (and friends); yeah, it's quite possible that notary implicitly bumped the version (or similar), and removing it now makes go mod re-resolve (minimum) versions; sometimes that can result in dependencies being "added", and sometimes dependencies are left behind, because with the (indirect) dependency that forced a higher minimum version "gone", go modules now considers our own go.mod to be the source of truth, which means that it considers dependencies listed to be a "manual bump" ("explicit") for an (in)direct dependency.

Sometimes removing those lines from go.mod (/ vendor.mod), then letting go modules re-resolve versions can help to make them dissolve, but sometimes that is troublesome, as it may now have to re-resolve using legacy dependencies it finds in the tree. Those dependencies may not have a go.mod, so now it will try "latest!", which can result in things failing (I recall notary -> github.com/docker/go-metrics -> prometheus -> otel, and github.com/docker/distribution -> github.com/docker/go-metrics -> prometheus -> otel (or grpc?)) being problematic paths in those.

[tianon]

the potential to add a compile-time build-tag to disable it (replacing DCT code with stubs possibly)

Can you elaborate on this? Why would we want to keep the code / add even more complexity here? (Whatever new things we implement, they're not very likely to match the usage patterns of DCT, nor do we want them to, because we'd want something that's actually enforced in the daemon, not just a CLI toggle like all this is, right?)

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 81.39535% with 8 lines in your changes missing coverage. Please review.

Project coverage is 60.10%. Comparing base (23eadcd) to head (89559da).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5896      +/-   ##
==========================================
+ Coverage   59.43%   60.10%   +0.66%     
==========================================
  Files         358      337      -21     
  Lines       29762    27848    -1914     
==========================================
- Hits        17690    16738     -952     
+ Misses      11107    10188     -919     
+ Partials      965      922      -43     

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tianon]

It's wild that it's no longer static -- I'm not sure what to make of that.

This PR:

 > [build 2/2] RUN --mount=type=bind,target=.,ro     --mount=type=cache,target=/root/.cache     --mount=type=tmpfs,target=cli/winresources     xx-go --wrap &&     TARGET=/out ./scripts/build/binary &&     xx-verify $([ "static" = "static" ] && echo "--static") /out/docker:
0.116 Building static docker-linux-amd64
0.119 + go build -o /out/docker-linux-amd64 -tags ' osusergo' -ldflags ' -X "github.com/docker/cli/cli/version.GitCommit=329f3ef" -X "github.com/docker/cli/cli/version.BuildTime=2025-03-07T08:07:09Z" -X "github.com/docker/cli/cli/version.Version=pr-5896" -extldflags -static' '-buildmode=pie' github.com/docker/cli/cmd/docker
30.03 file /out/docker is not statically linked: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib/ld-musl-x86_64.so.1, Go BuildID=0tBrrCP3FiHoexzL_l-B/PXXtsAeztugHiuX3XRMi/cXimdL1dgW63WVVLy9mO/_PAXMfK1hwrvKusQVO0k, with debug_info, not stripped

vs master:

#18 [build 2/2] RUN --mount=type=bind,target=.,ro     --mount=type=cache,target=/root/.cache     --mount=type=tmpfs,target=cli/winresources     xx-go --wrap &&     TARGET=/out ./scripts/build/binary &&     xx-verify $([ "static" = "static" ] && echo "--static") /out/docker
#18 0.122 Building static docker-linux-amd64
#18 0.125 + go build -o /out/docker-linux-amd64 -tags ' osusergo pkcs11' -ldflags ' -X "github.com/docker/cli/cli/version.GitCommit=ceef542" -X "github.com/docker/cli/cli/version.BuildTime=2025-03-06T17:11:33Z" -X "github.com/docker/cli/cli/version.Version=master" -extldflags -static' '-buildmode=pie' github.com/docker/cli/cmd/docker
#18 DONE 33.3s

The only difference is the pkcs11 build tag, which is literally deleted completely from the codebase with this PR, so there's something odder going on here and I'm missing it. 😭

[tianon]

I guess we don't actually need cgo with this change, thanks to the removal of pkcs11/yubikey "support", which would mean we get static binaries by default without so many (attempted) backflips. 🤔

[tianon]

Error response from daemon: toomanyrequests: You have reached your unauthenticated pull rate limit. https://www.docker.com/increase-rate-limit

womp womp

(but, mostly green otherwise!)

[thaJeztah]

I guess we don't actually need cgo with this change, thanks to the removal of pkcs11/yubikey "support", which would mean we get static binaries by default without so many (attempted) backflips. 🤔

Quite possible yes! I recall at least that most of the CGO-related bits were for content trust; I think most of the other code would be just run of the mill Go.

Not sure if we still need the osusergo build-tag as well with that 🤔 (ISTR it was only needed when compiling statically with CGO enabled), but may not do harm. (also not sure if that applies to macOS, which I think will always have some CGO component?)

[tianon]

Yeah, I managed to fix static+cgo in 81e57b2, but we can probably flip the "default to cgo" code to not do that anymore (and leave the rest so other builders can choose cgo if they have a reason).

[thaJeztah]

After this, we can probably inline this pullImage now, and get rid of this closure.

[tianon]

pullAndTagImage is used twice below - would you rather inline pullImage or ditch pullAndTagImage and use pullImage twice? (the latter sounds cleaner IMO)

[tianon]

(I just pushed the latter in https://github.com/docker/cli/compare/81e57b2d5fc1a95aef77545e182e466aa97c25ce..a5312466d6359558cee6bba555bcbc7e592fb149, but of course I'm happy to adjust further)

[tianon]

To reiterate what I've alluded to in #5920 (comment), with this change, I get a fully static and fully functional binary from just go build -v -trimpath -modfile=vendor.mod -mod=vendor -o ./build/docker ./cmd/docker (and a go.mod stub thanks to -modfile requiring it, but the contents are simply module github.com/docker/cli).

Edit: ok, ok, fine, it technically reports itself as Docker version unknown-version, build unknown-commit, but we could "fix" that via seeding those variables from "buildinfo" if they don't get set at compile-time.

Edit 2: one gross little init function later:

$ ./build/docker -v
Docker version v28.0.2-0.20250312023411-34d5dfa14659+incompatible+dirty, build 34d5dfa1465947d84dde80e5eb27e6ce2f3857ac

I'm sure this could be better, but for the curious/adventurous:

SEE BELOW, THERE'S A SECOND BETTER PATCH!

diff --git a/cli/version/version.go b/cli/version/version.go
index a263b9a73..56db10127 100644
--- a/cli/version/version.go
+++ b/cli/version/version.go
@@ -1,5 +1,9 @@
 package version
 
+import (
+	"runtime/debug"
+)
+
 // Default build-time variable.
 // These values are overridden via ldflags
 var (
@@ -8,3 +12,36 @@
 	GitCommit    = "unknown-commit"
 	BuildTime    = "unknown-buildtime"
 )
+
+func init() {
+	missingVersion := Version == "unknown-version"
+	missingGitCommit := GitCommit == "unknown-commit"
+	missingBuildTime := BuildTime == "unknown-buildtime"
+	if missingVersion || missingGitCommit || missingBuildTime {
+		if bi, ok := debug.ReadBuildInfo(); ok {
+			if missingVersion {
+				Version = bi.Main.Version
+			}
+			if missingGitCommit || missingBuildTime {
+				for _, bs := range bi.Settings {
+					if missingGitCommit && bs.Key == "vcs.revision" {
+						GitCommit = bs.Value
+						missingGitCommit = false
+						if !missingBuildTime {
+							break
+						}
+						continue
+					}
+					if missingBuildTime && bs.Key == "vcs.time" {
+						BuildTime = bs.Value
+						missingBuildTime = false
+						if !missingGitCommit {
+							break
+						}
+						continue
+					}
+				}
+			}
+		}
+	}
+}

Edit 3: here's a cleaner and better implementation of that that ends up more consistent (Docker version 28.0.2-0.20250312023411-34d5dfa14659.m, build 34d5dfa14):

Better Patch:

diff --git a/cli/version/version.go b/cli/version/version.go
index a263b9a73..5145e9410 100644
--- a/cli/version/version.go
+++ b/cli/version/version.go
@@ -1,10 +1,78 @@
 package version
 
+import (
+	"runtime/debug"
+	"strings"
+)
+
 // Default build-time variable.
 // These values are overridden via ldflags
 var (
 	PlatformName = ""
-	Version      = "unknown-version"
-	GitCommit    = "unknown-commit"
-	BuildTime    = "unknown-buildtime"
+	Version      = ""
+	GitCommit    = ""
+	BuildTime    = ""
 )
+
+func init() {
+	if Version == "" || GitCommit == "" || BuildTime == "" {
+		var (
+			biVersion  string
+			biCommit   string
+			biTime     string
+			biModified string
+		)
+		if bi, ok := debug.ReadBuildInfo(); ok {
+			if bi.Main.Path == "github.com/docker/cli" || strings.HasPrefix(bi.Main.Path, "github.com/docker/cli/") {
+				biVersion = bi.Main.Version
+				for _, bs := range bi.Settings {
+					switch bs.Key {
+					case "vcs.revision":
+						biCommit = bs.Value
+					case "vcs.time":
+						// TODO technically, this is probably a good value for BuildTime regardless of whether vcs.revision and vcs.modified are valid/meaningful for this build
+						biTime = bs.Value
+					case "vcs.modified":
+						biModified = bs.Value
+					}
+				}
+			} else {
+				for _, m := range bi.Deps {
+					if m != nil && (m.Path == "github.com/docker/cli" || strings.HasPrefix(bi.Main.Path, "github.com/docker/cli/")) {
+						biVersion = m.Version
+						break
+					}
+				}
+			}
+		}
+		if Version == "" {
+			if biVersion != "" && biVersion != "(devel)" {
+				Version = biVersion
+				Version = strings.TrimPrefix(Version, "v")
+				// TODO should these be Replace instead of TrimSuffix?
+				Version = strings.TrimSuffix(Version, "+dirty")
+				Version = strings.TrimSuffix(Version, "+incompatible")
+			} else {
+				Version = "unknown-version"
+			}
+		}
+		if GitCommit == "" {
+			if biCommit != "" {
+				// TODO it seems production builds currently use 7, but my local git chooses 9 (git rev-parse --short HEAD), so I'm matching 9
+				GitCommit = biCommit[:9]
+			} else {
+				GitCommit = "unknown-commit"
+			}
+		}
+		if BuildTime == "" {
+			if biTime != "" {
+				BuildTime = biTime
+			} else {
+				BuildTime = "unknown-buildtime"
+			}
+		}
+		if (biModified == "true" || strings.Contains(biVersion, "+dirty")) && !strings.HasSuffix(Version, ".m") {
+			Version += ".m"
+		}
+	}
+}

When using that straight off the v28.0.1 tag, I get Docker version 28.0.1.m, build 068a01ea9 which is very close and related in all the ways you might expect to the Docker version 28.0.1, build 068a01e of the production build.

[tianon]

Just highlighting all the TODO points with comments, which are actually all the same exact TODO (and maybe there's a clean place to put a single implementation that could be used for all of them so they're done consistently?)

// TODO add a (hidden) --disable-content-trust flag that throws a deprecation/removal warning and does nothing

I suppose we should also add something for docker trust so it prints a similar warning, but I don't think I've got a spot in the code anywhere that adds a TODO for that. 🤔

(Do we have any stats on whether anyone actually still invokes docker trust XXX? I've certainly never personally used it, and honestly forgot it even existed until I went to create this PR.)

[tianon]

TODO

[tianon]

TODO

[tianon]

TODO

[tianon]

TODO

[tianon]

TODO

[tianon]

TODO

[tianon]

TODO