Skip to content

buildkitd: Frontend restriction support#4899

Merged
tonistiigi merged 1 commit intomoby:masterfrom
dancysoft:frontend-restriction
May 13, 2024
Merged

buildkitd: Frontend restriction support#4899
tonistiigi merged 1 commit intomoby:masterfrom
dancysoft:frontend-restriction

Conversation

@dancysoft
Copy link
Copy Markdown
Contributor

@dancysoft dancysoft commented May 3, 2024

This commit adds buildkitd configuration options allowed-frontends and allowed-gateway-source. These options enable restricting the allowed frontends or gateways sources to enforce local policy.

If allowed-frontends is empty (the default), all frontends (e.g, "dockerfile.v0" and "gateway.v0") are allowed. Otherwise, only those listed are allowed

If allowed-gateway-sources is empty (the default), all gateway sources are allowed. Otherwise, only sources that match the patterns in this list will be allowed. Patterns are matched using
https://pkg.go.dev/github.com/moby/buildkit/util/wildcard. Note that implicit references to docker.io should not be used in the patterns since matching occurs on a fully expanded image name (for example "docker/dockerfile" expands to "docker.io/docker/dockerfile").

@dancysoft dancysoft force-pushed the frontend-restriction branch from 1f243d0 to 60e9209 Compare May 3, 2024 16:17
Comment thread docs/buildkitd.toml.md Outdated
@dancysoft dancysoft force-pushed the frontend-restriction branch from 60e9209 to 1824a34 Compare May 6, 2024 16:16
Copy link
Copy Markdown
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the use case behind this change? Frontends run in sandbox so there shouldn't be a security difference in the context of access to host. For gateway sources, we already have a policy rules support for restricting access to specific sources.

@dancysoft
Copy link
Copy Markdown
Contributor Author

What's the use case behind this change? Frontends run in sandbox so there shouldn't be a security difference in the context of access to host.

Hi Tõnis. At Wikimedia Foundation we have a policy that container images running in production must be built using a specific frontend which enforces consistent image build patterns and policies. We use this change to make sure that frontend it used.

For gateway sources, we already have a policy rules support for restricting access to specific sources.

I'm more than happy to use existing functionality. Can you point me to the documentation on this subject?

@tonistiigi
Copy link
Copy Markdown
Member

The difference in policy and this is that policy is set with the top build request, not with daemon config. https://github.com/moby/buildkit/blob/master/docs/build-repro.md#build-reproducibility #3332

Not against config option for such specific use case. I wonder though if we should add more structure to the config fields rather than adding new global keys. Smth like:

	Frontends struct {
		Gateway       GatewayFrontendConfig        `toml:"gateway"`
	} `toml:"frontend"`


type GatewayFrontendConfig struct {
   Disabled bool
   AllowedSources []string
}

@dancysoft
Copy link
Copy Markdown
Contributor Author

dancysoft commented May 7, 2024

@tonistiigi

Something like this?

...
# Frontend control
[frontend."dockerfile.v0"]
 enabled = true

[frontend."gateway.v0"]
 enabled = true

 # If allowedSources is empty, all gateway sources are allowed.
 # Otherwise, only sources that match the patterns in this list will
 # be allowed.
 #
 # NOTES:
 # * Only the image name (without tag) is compared.
 # * Patterns are matched using <https://pkg.go.dev/github.com/moby/buildkit/util/wildcard>.
 # * Implicit references to docker.io should not be used in
 #   the patterns since matching occurs on a fully expanded image name
 #   (for example "docker/dockerfile" expands to "docker.io/docker/dockerfile").
 #
 # Example:
 # allowedSources = [ "docker-registry.wikimedia.org/repos/releng/blubber/buildkit" ]
 allowedSources = []

@tonistiigi
Copy link
Copy Markdown
Member

@dancysoft Yeah, but maybe enabled -> disabled to make the zero value the default.

@AkihiroSuda wdyt?

@dancysoft
Copy link
Copy Markdown
Contributor Author

@dancysoft Yeah, but maybe enabled -> disabled to make the zero value the default.

If you don't have a strong preference, I would like to use enabled (defaulting to true) for consistency with the worker.oci and worker.containerd sections in the example buildkitd.toml:

...
[worker.oci]
  enabled = true
 ...
[worker.containerd]
  address = "/run/containerd/containerd.sock"
  enabled = true
...

I understand that I'll have to write code to handle this.

@dancysoft dancysoft force-pushed the frontend-restriction branch from 1824a34 to 85a6178 Compare May 8, 2024 18:42
Comment thread frontend/gateway/gateway.go
This commit adds [frontend."dockerfile.v0"] and
[frontend."gateway.v0"] buildkitd.toml configuration sections.  Each
frontend can individually be disabled by setting `enabled = false`
(both frontends are enabled by default).

The [frontend."gateway.v0"] section has an `allowedRepositories`
setting.  If `allowedRepositories` is empty (the default), all gateway
sources are allowed.  Otherwise, only repositories in the list will be
allowed.  NOTE: Only the repository name (without tag) is compared.

Change-Id: Ia484401709ef6c13cf3e5a2e4d0e1c6bd0c47d13
Signed-off-by: Ahmon Dancy <adancy@wikimedia.org>
@dancysoft dancysoft force-pushed the frontend-restriction branch from 85a6178 to b5c50af Compare May 9, 2024 15:52
@dancysoft
Copy link
Copy Markdown
Contributor Author

I don't think the most recent test failure is caused by my changes:
https://github.com/moby/buildkit/actions/runs/9019916817/job/24784047347?pr=4899

== Failed
=== FAIL: client TestIntegration (0.35s)
    run.go:165: 
        	Error Trace:	/src/util/testutil/integration/run.go:165
        	            				/src/client/client_test.go:236
        	            				/src/client/client_test.go:225
        	Error:      	Received unexpected error:
        	            	unexpected status from HEAD request to https://registry-1.docker.io/v2/cpuguy83/buildkit-foreign/manifests/latest: 503 Service Unavailable
        	Test:       	TestIntegration

@tonistiigi tonistiigi merged commit a17cbfd into moby:master May 13, 2024
@dancysoft
Copy link
Copy Markdown
Contributor Author

Thanks for merging!

@dancysoft dancysoft deleted the frontend-restriction branch May 14, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants