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

[BUG] Using golang 1.20 and use govmomi as libarary will cause issue with certificate validation because Go 1.20 introduces a new tls.CertificateValidationError type #3174

Closed
lubronzhan opened this issue Jul 10, 2023 · 5 comments · Fixed by kubernetes/cloud-provider-vsphere#738

Comments

@lubronzhan
Copy link
Contributor

lubronzhan commented Jul 10, 2023

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

  1. Bumped Cloud-provider-vsphere to use golang1.20
  2. Govmomi version is 0.30.4
  3. Then compile and deploy Cloud-provider-vsphere with correct tlsthumbprint config

Error shows up

E0710 05:47:20.259058       1 connectionmanager.go:147] Cannot connect to vCenter with err: Post "https://10.191.133.186:443/sdk": tls: failed to verify certificate: x509: cannot validate certificate for 10.191.133.186 because it doesn't contain any IP SANs

More Context
How Cloud provider vsphere use tlsthumbprint to initialize a govmomi client
https://github.com/kubernetes/cloud-provider-vsphere/blob/master/pkg/common/vclib/connection.go#L160-L195

If I use insecureFlag: true in CPI config then there is no issue connecting to VC. Also if I change CPI image to old one that use golang 1.19, there is no issue

The code of thumbprint check might be bypassed

When I just edit the gomod to 1.20 and compile govc directly, I don't see error

kubo@nWJRtIDvpA4fk:~/govmomi/govc$ go version
go version go1.20.5 linux/amd64

kubo@nWJRtIDvpA4fk:~/govmomi/govc$ head ../go.mod
module github.com/vmware/govmomi

go 1.20

require (
	github.com/a8m/tree v0.0.0-20210115125333-10a5fd5b637d
	github.com/dougm/pretty v0.0.0-20171025230240-2ee9d7453c02
	github.com/google/go-cmp v0.5.9
	github.com/google/uuid v1.3.0
	github.com/rasky/go-xdr v0.0.0-20170217172119-4930550ba2e2

kubo@nWJRtIDvpA4fk:~/govmomi/govc$ export GOVC_INSECURE=true
kubo@nWJRtIDvpA4fk:~/govmomi/govc$ ./govc datastore.ls
vCLS-0c1a2080-ecd8-4df8-bcf9-f2de443b77e3
Avi-se-ncvnr



I suspect it's because CPI is using golang 1.20 but govmomi is still pointing to 1.19.

Expected behavior
There is no error when using golang 1.20 and govmomi api

Affected version
Currently, the CPI calls govmomi 0.30.4. It doesn't really related to govc version, as long as the downstream repo use golang 1.20

Screenshots/Debug Output
If applicable, add screenshots or debug output to help explain your problem.

Additional context
Add any other context about the problem here.

@lubronzhan
Copy link
Contributor Author

But when I just edit the gomod to 1.20 and compile govc directly, I don't see error

kubo@nWJRtIDvpA4fk:~/govmomi/govc$ go version
go version go1.20.5 linux/amd64

kubo@nWJRtIDvpA4fk:~/govmomi/govc$ head ../go.mod
module github.com/vmware/govmomi

go 1.20

require (
	github.com/a8m/tree v0.0.0-20210115125333-10a5fd5b637d
	github.com/dougm/pretty v0.0.0-20171025230240-2ee9d7453c02
	github.com/google/go-cmp v0.5.9
	github.com/google/uuid v1.3.0
	github.com/rasky/go-xdr v0.0.0-20170217172119-4930550ba2e2

kubo@nWJRtIDvpA4fk:~/govmomi/govc$ export GOVC_INSECURE=true
kubo@nWJRtIDvpA4fk:~/govmomi/govc$ ./govc datastore.ls
vCLS-0c1a2080-ecd8-4df8-bcf9-f2de443b77e3
Avi-se-ncvnr

I suspect it's because CPI is using golang 1.20 but govmomi is still pointing to 1.19.

Let me try bump the govmomi dependency in govmomi from CPI's go mod

@lubronzhan lubronzhan changed the title [BUG] Using golang 1.20 will cause issue with certificate validation [BUG] Using golang 1.20 and use govmomi as libarary will cause issue with certificate validation Jul 10, 2023
@lubronzhan
Copy link
Contributor Author

I tried updated the govmomi with golang 1.20 in dependency of CPI

grep replace go.mod                                                          
replace github.com/vmware/govmomi v0.30.5 => github.com/lubronzhan/govmomi v0.14.1-0.20230710172525-90d423955eb0

But still CPI reports error

E0710 17:37:01.302419       1 connectionmanager.go:147] Cannot connect to vCenter with err: Post "https://10.191.133.186:443/sdk": tls: failed to verify certificate: x509: cannot validate certificate for 10.191.133.186 because it doesn't contain any IP SANs

@lubronzhan
Copy link
Contributor Author

I suspect this code was bypassed

func (c *Client) dialTLS(network string, addr string) (net.Conn, error) {

Like this issue #2494

@lubronzhan
Copy link
Contributor Author

lubronzhan commented Jul 10, 2023

Go 1.20 introduces a new tls.CertificateValidationError that wraps the x509.UnknownAuthorityError.
Maybe that's why the verification is bypassed here

// We first try tls.Dial with the default tls.Config, only falling back to thumbprint verification
// if it fails with an x509.UnknownAuthorityError or x509.HostnameError

Let me try update govmomi again by adding a new type here https://github.com/vmware/govmomi/blob/main/vim25/soap/error.go#L135

@lubronzhan
Copy link
Contributor Author

Ok it's working now after checking the whole error chain :)

	x509Err := &x509.UnknownAuthorityError{}
	ok := errors.As(err, x509Err)
	if ok {
		return true
	}

	x509HostNameErr := &x509.HostnameError{}
	ok = errors.As(err, x509HostNameErr)
	if ok {
		return true
	}

Let me create a PR

@lubronzhan lubronzhan changed the title [BUG] Using golang 1.20 and use govmomi as libarary will cause issue with certificate validation [BUG] Using golang 1.20 and use govmomi as libarary will cause issue with certificate validation because Go 1.20 introduces a new tls.CertificateValidationError type Jul 10, 2023
lubronzhan added a commit to lubronzhan/govmomi that referenced this issue Jul 17, 2023
lubronzhan added a commit to lubronzhan/govmomi that referenced this issue Jul 17, 2023
priyanka19-98 pushed a commit to priyanka19-98/govmomi that referenced this issue Jan 17, 2024
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 a pull request may close this issue.

1 participant