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

webhook tls min version to tls_v1.2 #1225

Closed
kangsheng89 opened this issue Nov 4, 2022 · 3 comments
Closed

webhook tls min version to tls_v1.2 #1225

kangsheng89 opened this issue Nov 4, 2022 · 3 comments
Labels
help wanted Extra attention is needed

Comments

@kangsheng89
Copy link
Contributor

Currently the webhook on controller-manager is operate on port 9443. The webhook server of the https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/webhook/server.go#L77 is configurable

Is there any plan to support setting to use min tls v1.2 as open telemetry collector of v0.59.0? and
TLS 1.0 and 1.1 are deprecated due to known vulnerabilities and should be avoided.

@kangsheng89
Copy link
Contributor Author

kangsheng89 commented Nov 7, 2022

Before add this, the controller-runtime has to be updated to v0.13.1
kubernetes-sigs/controller-runtime@v0.13.0...v0.13.1

depends on #1221

@pavolloffay pavolloffay added the help wanted Extra attention is needed label Nov 7, 2022
@pavolloffay
Copy link
Member

Now we rely on the manager to create the default webhook server. The #1221 has been merged. Do you know if the default server will set TLS to 1.2?

mgrOptions := ctrl.Options{

	// WebhookServer is an externally configured webhook.Server. By default,
	// a Manager will create a default server using Port, Host, and CertDir;
	// if this is set, the Manager will use this server instead.
	WebhookServer *webhook.Server

@kangsheng89
Copy link
Contributor Author

kangsheng89 commented Nov 7, 2022

i think we can update TLS config through this https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/manager/manager.go#L253

first using crypto/tls lib

    import (
         "crypto/tls"
    ...
    )

       #set const of minTLS
        const defaultMinTLSVersion = tls.VersionTLS12

        ...
        # array of function for config setting
	optionsTlSOptsFuncs := []func(*tls.Config){
		func(config *tls.Config) { minTlsDefault(config) },
	}
	mgrOptions := ctrl.Options{
		Scheme:                 scheme,
		MetricsBindAddress:     metricsAddr,
		Port:                   webhookPort,
		TLSOpts:                optionsTlSOptsFuncs,
        ....}
# function to set minTls Default
func minTlsDefault(cfg *tls.Config) {
	cfg.MinVersion = tls.VersionTLS12
}

This is my initial idea, need to be test and verify. And i need some help to understand the build and debug of opentelemetry-operator on local.

kangsheng89 added a commit to kangsheng89/opentelemetry-operator that referenced this issue Nov 8, 2022
pavolloffay pushed a commit that referenced this issue Nov 8, 2022
* fix min tls setting for webhook server (#1225)

* using constant as setting

* fix godot issue on comment
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this issue May 1, 2024
…lemetry#1230)

* fix min tls setting for webhook server (open-telemetry#1225)

* using constant as setting

* fix godot issue on comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants