[Bug]: tlsVerify does not work for sync

[Jauchi]

zot version

v2.1.0 (docker/helm)

Describe the bug

Hello!
It seems like the sync plugin does not respect the tlsVerify setting.

To reproduce

1. Configuration

{
  "storage": {
    "rootDirectory": "/var/lib/registry",
    "gc": true
  },
  "http": {
    "address": "0.0.0.0",
    "port": "5000"
  },
  "log": {
    "level": "debug"
  },
  "extensions": {
    "search": {
      "enable": true
    },
    "ui": {
      "enable": true
    },
    "sync": {
      "enable": true,
      "credentialsFile": "/etc/zot/auth.json",
      "registries": [
        {
          "urls": [
            "https://registry.p1ng.link/"
          ],
          "content": [
            {
              "prefix": "**",
              "destination": "/p1nglink"
            }
          ],
          "onDemand": true,
          "tlsVerify": false,
          "certDir": "/etc/zot/ca/",
          "onlySigned": false
        }
      ]
    }
  }
}

2. docker pull from the zot registry (goes to the p1nglink registry)
3. Log reads: {"level":"error","error":"Get \"https://registry.p1ng.link/v2/\": tls: failed to verify certificate: x509: certificate signed by unknown authority","url":"https://registry.p1ng.link/v2/","component":"sync","errorType":"*url.Error","goroutine":1,"caller":"zotregistry.dev/zot/pkg/extensions/sync/httpclient/client.go:272","time":"2024-07-21T14:47:22.930557924Z","message":"failed to make request"}

Expected behavior

Pull should succeed.

Screenshots

No response

Additional context

I also haven't been able to import the certificate into the container, any help in that direction would also be greatly appreciated (what file goes where)?

[eusebiu-constantin-petu-dbk]

Hello @Jauchi. Thanks for trying zot!

I'm not sure what is the problem but I took a guess and pushed a patch for this: #2558
Can you try it please?

Also, can you post the logs please?

Thank you!

[Jauchi]

Hi there!
Sorry for the late response - I actually tried running your commit and didn't realize that your forked was a lot older than expected, so I was hitting errors I couldn't explain before (all good now and I learned a couple of things about docker as well ;).

Right, I ran 1.2.0 with your patch applied:

• tlsVerify still has no effect
• certDir also does nothing

Not quite sure what you mean by log, is this what you're looking for?
log.txt
helm_values.txt

[Jauchi]

Right, I ran 1.2.0 with your patch applied

Sorry, meant 2.1.0 - everything else still applies.

[rchincha]

{"level":"error","error":"Get "https://registry.p1ng.link/v2/\": tls: failed to verify certificate: x509: certificate signed by unknown

@Jauchi the host above has an invalid certificate - the issuer is unknown and hence unsafe. Is this really what you want?
If so, would just download the CA cert and launch zot from a container.

[Jauchi]

Hi there!
Correct, that's exactly what I tried to do using certDir. When that failed, I set tlsVerify to false, which also did not work - hence the issue.

According to my understanding, the helm_values.txt should be set correctly, that's why I'm assuming it's a bug with zot.

[rchincha]

#2558
^ does this fix your issue?

[Jauchi]

#2558 ^ does this fix your issue?

No, I don't think it does, doesn't seem to have any effect. Maybe I messed something up. Could you check whether or not you get an error with an invalid SSL certificate? https://untrusted-root.badssl.com/ as URL should work when skipping CA checks (but will fail because it's not a registry)

#2557 (comment)

[mariusbertram]

I think the problem is. the http.client config for sync sets tls to disable, if tlsVerify is set to false.

zot/pkg/extensions/sync/httpclient/client.go

Lines 118 to 123 in f735680

	clientOpts := common.HTTPClientOptions{
	// we want TLS enabled when verifyTLS is true.
	TLSEnabled: config.TLSVerify,
	VerifyTLS: config.TLSVerify,
	Host: clientURL.Host,
	}

[evanebb]

@mariusbertram is right, I noticed the same thing. TLSEnabled is disabled if tlsVerify is set to false, which means that further downstream it'll return a HTTP client without SSL verification disabled:

zot/pkg/common/http_client.go

Lines 87 to 102 in f735680

	// If TLS is not enabled, return the client without any further TLS config.
	if !clientOptions.TLSEnabled {
	return &http.Client{
	Timeout: httpTimeout,
	Transport: htr,
	}, nil
	}
	if !clientOptions.VerifyTLS {
	htr.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} //nolint: gosec
	return &http.Client{
	Timeout: httpTimeout,
	Transport: htr,
	}, nil
	}

I have proposed a fix in #2747, which will determine whether TLS should be enabled based on whether the upstream registry is a HTTPS URL.

[mariusbertram]

The option with the custom ca doesnt work because it tries to add clientCertandclientKey. Which should only be set on mTLS` (authentication via cert) connections.

zot/pkg/common/http_client.go

Lines 11 to 33 in 51e779f

	func GetTLSConfig(certsPath string, caCertPool *x509.CertPool) (*tls.Config, error) {
	clientCert := filepath.Join(certsPath, ClientCertFilename)
	clientKey := filepath.Join(certsPath, ClientKeyFilename)
	caCertFile := filepath.Join(certsPath, CaCertFilename)
	cert, err := tls.LoadX509KeyPair(clientCert, clientKey)
	if err != nil {
	return nil, err
	}
	caCert, err := os.ReadFile(caCertFile)
	if err != nil {
	return nil, err
	}
	caCertPool.AppendCertsFromPEM(caCert)
	return &tls.Config{
	Certificates: []tls.Certificate{cert},
	RootCAs: caCertPool,
	MinVersion: tls.VersionTLS12,
	}, nil
	}

A better aproach would be to set a path to a ca-bundle and add this bundle to the caCertPool.

I will try my best on an PR but go is not my best "friend"

[mariusbertram]

Should be fixed with #2628

[andaaron]

#2747 is merged. @Jauchi, can you please try it out, can we close this issue?

[Jauchi]

#2747 is merged. @Jauchi, can you please try it out, can we close this issue?

Thanks for addressing the issue, however I'm quite busy atm, I'll check it out on the weekend.

[Jauchi]

#2747 is merged. @Jauchi, can you please try it out, can we close this issue?

Hi there!
Sorry for the late reply, I was trying to get the latest version built using docker, and for some reason, it always failed.
After about an hour, I noticed that there already a ready to use release candidate for me to try.
So I gave that a spin and now I don't get TLS errors anymore - just authentication issues.
But that seems to be a seperate problem.

Thanks for fixing, have a great week!