Skip to content

Commit edeb94b

Browse files
karmada-botkarmada-bot
authored
Merge pull request #4183 from jwcesign/fix-insecure
feat: Add ca to check the validation of the member clusters' server certificate
2 parents e03f5d5 + 473b2be commit edeb94b

File tree

4 files changed

+84
-23
lines changed

4 files changed

+84
-23
lines changed

‎pkg/registry/cluster/storage/proxy_test.go‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,9 @@ func TestProxyREST_Connect(t *testing.T) {
6262
return &clusterapis.Cluster{
6363
ObjectMeta: metav1.ObjectMeta{Name: name},
6464
Spec: clusterapis.ClusterSpec{
65-
APIEndpoint: s.URL,
66-
ImpersonatorSecretRef: &clusterapis.LocalSecretReference{Namespace: "ns", Name: "secret"},
65+
APIEndpoint: s.URL,
66+
ImpersonatorSecretRef: &clusterapis.LocalSecretReference{Namespace: "ns", Name: "secret"},
67+
InsecureSkipTLSVerification: true,
6768
},
6869
}, nil
6970
},
@@ -109,8 +110,9 @@ func TestProxyREST_Connect(t *testing.T) {
109110
return &clusterapis.Cluster{
110111
ObjectMeta: metav1.ObjectMeta{Name: name},
111112
Spec: clusterapis.ClusterSpec{
112-
APIEndpoint: s.URL,
113-
ImpersonatorSecretRef: &clusterapis.LocalSecretReference{Namespace: "ns", Name: "secret"},
113+
APIEndpoint: s.URL,
114+
ImpersonatorSecretRef: &clusterapis.LocalSecretReference{Namespace: "ns", Name: "secret"},
115+
InsecureSkipTLSVerification: true,
114116
},
115117
}, nil
116118
},
@@ -134,8 +136,9 @@ func TestProxyREST_Connect(t *testing.T) {
134136
return &clusterapis.Cluster{
135137
ObjectMeta: metav1.ObjectMeta{Name: name},
136138
Spec: clusterapis.ClusterSpec{
137-
APIEndpoint: s.URL,
138-
ImpersonatorSecretRef: &clusterapis.LocalSecretReference{Namespace: "ns", Name: "secret"},
139+
APIEndpoint: s.URL,
140+
ImpersonatorSecretRef: &clusterapis.LocalSecretReference{Namespace: "ns", Name: "secret"},
141+
InsecureSkipTLSVerification: true,
139142
},
140143
}, nil
141144
},

‎pkg/registry/cluster/storage/storage.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net/http"
77
"net/url"
88

9+
corev1 "k8s.io/api/core/v1"
910
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1011
"k8s.io/apimachinery/pkg/runtime"
1112
"k8s.io/apiserver/pkg/registry/generic"
@@ -58,7 +59,7 @@ func NewStorage(scheme *runtime.Scheme, kubeClient kubernetes.Interface, secretL
5859
statusStore.UpdateStrategy = statusStrategy
5960
statusStore.ResetFieldsStrategy = statusStrategy
6061

61-
clusterRest := &REST{store}
62+
clusterRest := &REST{secretLister, store}
6263
return &ClusterStorage{
6364
Cluster: clusterRest,
6465
Status: &StatusREST{&statusStore},
@@ -72,6 +73,7 @@ func NewStorage(scheme *runtime.Scheme, kubeClient kubernetes.Interface, secretL
7273

7374
// REST implements a RESTStorage for Cluster.
7475
type REST struct {
76+
secretLister listcorev1.SecretLister
7577
*genericregistry.Store
7678
}
7779

@@ -85,7 +87,15 @@ func (r *REST) ResourceLocation(ctx context.Context, name string) (*url.URL, htt
8587
return nil, nil, err
8688
}
8789

88-
return proxy.Location(cluster)
90+
secretGetter := func(ctx context.Context, namespace, name string) (*corev1.Secret, error) {
91+
return r.secretLister.Secrets(namespace).Get(name)
92+
}
93+
tlsConfig, err := proxy.GetTlsConfigForCluster(ctx, cluster, secretGetter)
94+
if err != nil {
95+
return nil, nil, err
96+
}
97+
98+
return proxy.Location(cluster, tlsConfig)
8999
}
90100

91101
func (r *REST) getCluster(ctx context.Context, name string) (*clusterapis.Cluster, error) {

‎pkg/util/proxy/proxy.go‎

Lines changed: 60 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package proxy
33
import (
44
"context"
55
"crypto/tls"
6+
"crypto/x509"
67
"errors"
78
"fmt"
89
"net/http"
@@ -25,9 +26,16 @@ import (
2526
clusterapis "github.com/karmada-io/karmada/pkg/apis/cluster"
2627
)
2728

29+
type SecretGetterFunc func(context.Context, string, string) (*corev1.Secret, error)
30+
2831
// ConnectCluster returns a handler for proxy cluster.
29-
func ConnectCluster(ctx context.Context, cluster *clusterapis.Cluster, proxyPath string, secretGetter func(context.Context, string, string) (*corev1.Secret, error), responder registryrest.Responder) (http.Handler, error) {
30-
location, proxyTransport, err := Location(cluster)
32+
func ConnectCluster(ctx context.Context, cluster *clusterapis.Cluster, proxyPath string, secretGetter SecretGetterFunc, responder registryrest.Responder) (http.Handler, error) {
33+
tlsConfig, err := GetTlsConfigForCluster(ctx, cluster, secretGetter)
34+
if err != nil {
35+
return nil, err
36+
}
37+
38+
location, proxyTransport, err := Location(cluster, tlsConfig)
3139
if err != nil {
3240
return nil, err
3341
}
@@ -37,20 +45,20 @@ func ConnectCluster(ctx context.Context, cluster *clusterapis.Cluster, proxyPath
3745
return nil, fmt.Errorf("the impersonatorSecretRef of cluster %s is nil", cluster.Name)
3846
}
3947

40-
secret, err := secretGetter(ctx, cluster.Spec.ImpersonatorSecretRef.Namespace, cluster.Spec.ImpersonatorSecretRef.Name)
48+
impersonateTokenSecret, err := secretGetter(ctx, cluster.Spec.ImpersonatorSecretRef.Namespace, cluster.Spec.ImpersonatorSecretRef.Name)
4149
if err != nil {
4250
return nil, err
4351
}
44-
45-
impersonateToken, err := getImpersonateToken(cluster.Name, secret)
52+
impersonateToken, err := getImpersonateToken(cluster.Name, impersonateTokenSecret)
4653
if err != nil {
4754
return nil, fmt.Errorf("failed to get impresonateToken for cluster %s: %v", cluster.Name, err)
4855
}
4956

50-
return newProxyHandler(location, proxyTransport, cluster, impersonateToken, responder)
57+
return newProxyHandler(location, proxyTransport, cluster, impersonateToken, responder, tlsConfig)
5158
}
5259

53-
func newProxyHandler(location *url.URL, proxyTransport http.RoundTripper, cluster *clusterapis.Cluster, impersonateToken string, responder registryrest.Responder) (http.Handler, error) {
60+
func newProxyHandler(location *url.URL, proxyTransport http.RoundTripper, cluster *clusterapis.Cluster, impersonateToken string,
61+
responder registryrest.Responder, tlsConfig *tls.Config) (http.Handler, error) {
5462
return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
5563
requester, exist := request.UserFrom(req.Context())
5664
if !exist {
@@ -76,7 +84,7 @@ func newProxyHandler(location *url.URL, proxyTransport http.RoundTripper, cluste
7684
location.RawQuery = req.URL.RawQuery
7785

7886
upgradeDialer := NewUpgradeDialerWithConfig(UpgradeDialerWithConfig{
79-
TLS: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec
87+
TLS: tlsConfig,
8088
Proxier: http.ProxyURL(proxyURL),
8189
PingPeriod: time.Second * 5,
8290
Header: ParseProxyHeaders(cluster.Spec.ProxyHeader),
@@ -94,14 +102,46 @@ func NewThrottledUpgradeAwareProxyHandler(location *url.URL, transport http.Roun
94102
return proxy.NewUpgradeAwareHandler(location, transport, wrapTransport, upgradeRequired, proxy.NewErrorResponder(responder))
95103
}
96104

105+
func GetTlsConfigForCluster(ctx context.Context, cluster *clusterapis.Cluster, secretGetter SecretGetterFunc) (*tls.Config, error) {
106+
// The secret is optional for a pull-mode cluster, if no secret just returns a config with root CA unset.
107+
if cluster.Spec.SecretRef == nil {
108+
return &tls.Config{
109+
MinVersion: tls.VersionTLS13,
110+
// Ignore false positive warning: "TLS InsecureSkipVerify may be true. (gosec)"
111+
// Whether to skip server certificate verification depends on the
112+
// configuration(.spec.insecureSkipTLSVerification, defaults to false) in a Cluster object.
113+
InsecureSkipVerify: cluster.Spec.InsecureSkipTLSVerification, //nolint:gosec
114+
}, nil
115+
}
116+
caSecret, err := secretGetter(ctx, cluster.Spec.SecretRef.Namespace, cluster.Spec.SecretRef.Name)
117+
if err != nil {
118+
return nil, err
119+
}
120+
caBundle, err := getClusterCABundle(cluster.Name, caSecret)
121+
if err != nil {
122+
return nil, fmt.Errorf("failed to get CA bundle for cluster %s: %v", cluster.Name, err)
123+
}
124+
125+
caCertPool := x509.NewCertPool()
126+
caCertPool.AppendCertsFromPEM([]byte(caBundle))
127+
return &tls.Config{
128+
RootCAs: caCertPool,
129+
MinVersion: tls.VersionTLS13,
130+
// Ignore false positive warning: "TLS InsecureSkipVerify may be true. (gosec)"
131+
// Whether to skip server certificate verification depends on the
132+
// configuration(.spec.insecureSkipTLSVerification, defaults to false) in a Cluster object.
133+
InsecureSkipVerify: cluster.Spec.InsecureSkipTLSVerification, //nolint:gosec
134+
}, nil
135+
}
136+
97137
// Location returns a URL to which one can send traffic for the specified cluster.
98-
func Location(cluster *clusterapis.Cluster) (*url.URL, http.RoundTripper, error) {
138+
func Location(cluster *clusterapis.Cluster, tlsConfig *tls.Config) (*url.URL, http.RoundTripper, error) {
99139
location, err := constructLocation(cluster)
100140
if err != nil {
101141
return nil, nil, err
102142
}
103143

104-
proxyTransport, err := createProxyTransport(cluster)
144+
proxyTransport, err := createProxyTransport(cluster, tlsConfig)
105145
if err != nil {
106146
return nil, nil, err
107147
}
@@ -122,12 +162,11 @@ func constructLocation(cluster *clusterapis.Cluster) (*url.URL, error) {
122162
return uri, nil
123163
}
124164

125-
func createProxyTransport(cluster *clusterapis.Cluster) (*http.Transport, error) {
165+
func createProxyTransport(cluster *clusterapis.Cluster, tlsConfig *tls.Config) (*http.Transport, error) {
126166
var proxyDialerFn utilnet.DialFunc
127-
proxyTLSClientConfig := &tls.Config{InsecureSkipVerify: true} // #nosec
128167
trans := utilnet.SetTransportDefaults(&http.Transport{
129168
DialContext: proxyDialerFn,
130-
TLSClientConfig: proxyTLSClientConfig,
169+
TLSClientConfig: tlsConfig,
131170
})
132171

133172
if proxyURL := cluster.Spec.ProxyURL; proxyURL != "" {
@@ -162,6 +201,14 @@ func getImpersonateToken(clusterName string, secret *corev1.Secret) (string, err
162201
return string(token), nil
163202
}
164203

204+
func getClusterCABundle(clusterName string, secret *corev1.Secret) (string, error) {
205+
caBundle, found := secret.Data[clusterapis.SecretCADataKey]
206+
if !found {
207+
return "", fmt.Errorf("the CA bundle of cluster %s is empty", clusterName)
208+
}
209+
return string(caBundle), nil
210+
}
211+
165212
func skipGroup(group string) bool {
166213
switch group {
167214
case user.AllAuthenticated, user.AllUnauthenticated:

‎pkg/util/proxy/proxy_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,9 @@ func TestConnectCluster(t *testing.T) {
167167
cluster: &clusterapis.Cluster{
168168
ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "cluster"},
169169
Spec: clusterapis.ClusterSpec{
170-
APIEndpoint: s.URL,
171-
ImpersonatorSecretRef: &clusterapis.LocalSecretReference{Namespace: "ns", Name: "secret"},
170+
APIEndpoint: s.URL,
171+
ImpersonatorSecretRef: &clusterapis.LocalSecretReference{Namespace: "ns", Name: "secret"},
172+
InsecureSkipTLSVerification: true,
172173
},
173174
},
174175
secretGetter: func(_ context.Context, ns string, name string) (*corev1.Secret, error) {

0 commit comments

Comments
 (0)