Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

buildkitd: Frontend restriction support #4899

Merged
merged 1 commit into from May 13, 2024

Conversation

dancysoft
Copy link
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").

docs/buildkitd.toml.md Outdated Show resolved Hide resolved
Copy link
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
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
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
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
Member

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

@AkihiroSuda wdyt?

@dancysoft
Copy link
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.

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
Copy link
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
75 checks passed
@dancysoft
Copy link
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.

None yet

3 participants