http and insecure are mutually exclusive, when used on a registry with a non-standard port

[alexcb]

The insecure option only applies when connecting to a https server. If both http and insecure options are set, the resolver will ignore the isHTTP option and will create an additional https host entry (which will ignore certs)

For example, consider a self-hosted registry which incorrectly had both http and insecure set:

[registry."172.17.0.2:5000"]
  http = true
  insecure = true

When interacting with this registry, one gets the error:

Head "https://172.17.0.2:5000/v2/test1/manifests/latest": http: server gave HTTP response to HTTPS client

Which is unexpected since the http=true setting was applied.

Now we get:

Solve returned error: rpc error: code = Unknown desc = the http and insecure options are mutually exclusive

Signed-off-by: Alex Couture-Beil alex@mofo.ca

[alexcb]

I'm not entirely sure if raising this error via a solve is appropriate; perhaps buildkit should just fail to start in such a case where invalid configs are given?

[jedevc]

Hey @alexcb thanks for the PR 🎉

I'm not actually sure if this is the right route - to me, it looks like this commit 6704487 seems to imply that it should be possible to supply both insecure and http? It seems like an issue if we're not able to supply both of those: I'm not quite sure when/if that stopped working, will try and take a look if I have a spare moment this week 👀

[alexcb]

perhaps raising an error should only occur when both insecure and http are set and a specific port is configured.

If no port is configured, then we should end up with two entries: ["http://host:80", "https://host:443"]; however when a port override is given (and these two flags are set), then we'll end up with ["http://host:5000", "https://host:5000"] -- which seems like a undesirable configuration (unless you have a tcp-based load balancer that round-robins between both http and https workers -- and if that's the case, run away!).

Thanks for taking a look.

[lazywhite]

perhaps raising an error should only occur when both insecure and http are set and a specific port is configured.

If no port is configured, then we should end up with two entries: ["http://host:80", "https://host:443"]; however when a port override is given (and these two flags are set), then we'll end up with ["http://host:5000", "https://host:5000"] -- which seems like a undesirable configuration (unless you have a tcp-based load balancer that round-robins between both http and https workers -- and if that's the case, run away!).

Thanks for taking a look.

@alexcb you mean we should not configure http and insecure as true at same time?
but I tried this buildkitd.toml, only configure http = true, buildx still gives me x509 error

# /etc/docker/buildkitd.toml
debug = true
insecure-entitlements = [ "network.host", "security.insecure" ]
[registry."harbor.demohub.com"]
  http = true

docker buildx build \
	--platform linux/amd64,linux/arm64 \
	--output type=image \
	--tag harbor.demohub.com/library/alpine:3.16 \
	--network host \
	--push \
	.
[+] Building 48.7s (8/8) FINISHED                                                             
 => [internal] load build definition from Dockerfile                                     0.1s
 => => transferring dockerfile: 54B                                                      0.0s
 => [internal] load .dockerignore                                                        0.0s
 => => transferring context: 2B                                                          0.0s
 => [linux/arm64 internal] load metadata for docker.io/library/alpine:3.16              48.3s
 => [linux/amd64 internal] load metadata for docker.io/library/alpine:3.16              19.3s
 => [auth] library/alpine:pull token for registry-1.docker.io                            0.0s
 => [linux/arm64 1/1] FROM docker.io/library/alpine:3.16@sha256:b95359c2505145f16c6aa38  0.1s
 => => resolve docker.io/library/alpine:3.16@sha256:b95359c2505145f16c6aa384f9cc74eeff7  0.0s
 => [linux/amd64 1/1] FROM docker.io/library/alpine:3.16@sha256:b95359c2505145f16c6aa38  0.1s
 => => resolve docker.io/library/alpine:3.16@sha256:b95359c2505145f16c6aa384f9cc74eeff7  0.0s
 => ERROR exporting to image                                                             0.1s
 => => exporting layers                                                                  0.0s
 => => exporting manifest sha256:3349e20df077de5eba3cf212edf5d028e1658c8ab1222cbe617cf9  0.0s
 => => exporting config sha256:3de62af236353810c436f3eed6684e67a6270251dbcaf886a5bdb570  0.0s
 => => exporting manifest sha256:fe04bcb3e8fc9f8976c29b900662f7e3671d3a7f4b8e7bbbe7358c  0.0s
 => => exporting config sha256:8aa37678b2f06847f906c397082aada09154408018687e20f87c27ee  0.0s
 => => exporting manifest list sha256:04c38a1345227d30da8875c809d5d6a7f625b4f214f8323a5  0.0s
 => => pushing layers                                                                    0.0s
------
 > exporting to image:
------
ERROR: failed to solve: failed to do request: Head "https://harbor.demohub.com:443/v2/library/alpine/blobs/sha256:3de62af236353810c436f3eed6684e67a6270251dbcaf886a5bdb5700ece9b94": x509: certificate signed by unknown authority
make: *** [Makefile:7: images] Error 1

I finally figure out that the x509 error is caused by harbor.
because I enabled both http and https, harbor will redirect http to https, this is wrote in harbor.yml

# Configuration file of Harbor

# The IP address or hostname to access admin UI and registry service.
# DO NOT use localhost or 127.0.0.1, because Harbor needs to be accessed by external clients.
hostname: harbor.demohub.com

# http related config
http:
  # port for http, default is 80. If https enabled, this port will redirect to https port
  port: 80

now with only http = true, buildx can push successfully

[alexcb]

I backed out my initial change to return an error, and wrote a few unit-tests to better illustrate what I was confused by.

In particular the case where a non-standard port is used and both http and insecure are set.

Please take a look at TestFillInsecureOptsPlainHTTPAndInsecureWithTheSamePort:

https://github.com/moby/buildkit/pull/3382/files#diff-ec1173f1a055187e6e8af19ddbec7c12093aefeb1a9cf575e623c66919b9c9a8R121-R126

My suggestion is such a case should raise an error instead of attempting both a http and https connection -- but perhaps there's a use-case for supporting such a fallback mechanism?

[jedevc]

@alexcb massive apologies for not getting to look at this sooner - I was reminded of this by docker/buildx#1642, which seems similar.

So I've discovered where http and insecure are both set - while users shouldn't set it, it's the same behavior as when insecure-registries is set in /etc/docker/daemon.json (see docker/buildx#1642). So to be able to express the same type of config, we need to support both.

I do think there is a possible underlying issue though here. fillInsecureOpts creates both an HTTP connection and an HTTPS connection - the errors propagated back from containerd's docker resolver are not great. Even if we hit a 404 error from the first registry, we delay and try and return the TLS failure error which is kinda confusing.

The discrepancy seen in docker/buildx#1642 looks to be due to differences in how the legacy builder vs buildkit pull images: while buildkit uses containerd, the legacy builder doesn't (yet?)

• https://github.com/moby/moby/blob/2fa8f54d0869511f88efd318f67fe1d06915e78a/distribution/pull.go#L37-L40

• buildkit/vendor/github.com/containerd/containerd/remotes/docker/resolver.go

  Lines 285 to 295 in 3187d2d

  	if err != nil {
  	if errors.Is(err, ErrInvalidAuthorization) {
  	err = fmt.Errorf("pull access denied, repository does not exist or may require authorization: %w", err)
  	}
  	// Store the error for referencing later
  	if firstErr == nil {
  	firstErr = err
  	}
  	log.G(ctx).WithError(err).Info("trying next host")
  	continue // try another host
  	}

Moby does some clever magic to avoid requesting HTTPS on endpoints that are known to be HTTP - we might want to upstream something similar to containerd? cc @thaJeztah you might have some better background context here.

[thaJeztah]

Would have to do some digging, as I don't have all the logic in mind immediately.
I know some of the interactions between options have been somewhat "confusing" (and not all combinations, in hindsight, may have been "good", but sometimes due to backward-compat), such as "tls-verify=false" meaning enable TLS (if it's not enabled), but skip validation. Need to double check "insecure", which may still try TLS, but allow falling back to non-TLS?.

[alexcb]

massive apologies for not getting to look at this sooner

No worries at all -- this PR isn't really in a mergable state, it devolved into a combinatorics-question 😆

it's the same behavior as when insecure-registries is set in /etc/docker/daemon.json

this is exactly how I hit this -- while debugging why buildkit was attempting to hit a http server with a https client.

Feel free to close this PR since it's being tracked in docker/buildx#1642 now.