diff --git a/pkg/apis/admissionregistration/types.go b/pkg/apis/admissionregistration/types.go index d2986a4d11b..091782b81a0 100644 --- a/pkg/apis/admissionregistration/types.go +++ b/pkg/apis/admissionregistration/types.go @@ -282,12 +282,16 @@ type WebhookClientConfig struct { // webhook. Such installs are likely to be non-portable, i.e., not easy // to turn up in a new cluster. // - // If the scheme is present, it must be "https://". + // The scheme must be "https"; the URL must begin with "https://". // // A path is optional, and if present may be any string permissible in // a URL. You may use the path to pass an arbitrary string to the // webhook, for example, a cluster identifier. // + // Attempting to use a user or basic auth e.g. "user:password@" is not + // allowed. Fragments ("#...") and query parameters ("?...") are not + // allowed, either. + // // +optional URL *string diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go index f22fa38c650..958ebf44022 100644 --- a/pkg/apis/admissionregistration/validation/validation.go +++ b/pkg/apis/admissionregistration/validation/validation.go @@ -213,11 +213,20 @@ func validateWebhookClientConfig(fldPath *field.Path, cc *admissionregistration. if u, err := url.Parse(*cc.URL); err != nil { allErrors = append(allErrors, field.Required(fldPath.Child("url"), "url must be a valid URL: "+err.Error()+form)) } else { - if u.Scheme != "" && u.Scheme != "https" { - allErrors = append(allErrors, field.Required(fldPath.Child("url"), "'https' is the only allowed URL scheme"+form)) + if u.Scheme != "https" { + allErrors = append(allErrors, field.Invalid(fldPath.Child("url"), u.Scheme, "'https' is the only allowed URL scheme"+form)) } if len(u.Host) == 0 { - allErrors = append(allErrors, field.Required(fldPath.Child("url"), "host must be provided"+form)) + allErrors = append(allErrors, field.Invalid(fldPath.Child("url"), u.Host, "host must be provided"+form)) + } + if u.User != nil { + allErrors = append(allErrors, field.Invalid(fldPath.Child("url"), u.User.String(), "user information is not permitted in the URL")) + } + if len(u.Fragment) != 0 { + allErrors = append(allErrors, field.Invalid(fldPath.Child("url"), u.Fragment, "fragments are not permitted in the URL")) + } + if len(u.RawQuery) != 0 { + allErrors = append(allErrors, field.Invalid(fldPath.Child("url"), u.RawQuery, "query parameters are not permitted in the URL")) } } } diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go index c7d27ab8635..5dd3fc551b7 100644 --- a/pkg/apis/admissionregistration/validation/validation_test.go +++ b/pkg/apis/admissionregistration/validation/validation_test.go @@ -538,7 +538,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) { }, }, }), - expectedError: `[0].clientConfig.url: Required value: host must be provided`, + expectedError: `[0].clientConfig.url: Invalid value: "": host must be provided`, }, { name: "wrong scheme", @@ -566,6 +566,45 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) { }), expectedError: `host must be provided`, }, + { + name: "fragment", + config: newValidatingWebhookConfiguration( + []admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + URL: strPtr("https://example.com/#bookmark"), + }, + }, + }), + expectedError: `"bookmark": fragments are not permitted`, + }, + { + name: "query", + config: newValidatingWebhookConfiguration( + []admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + URL: strPtr("https://example.com?arg=value"), + }, + }, + }), + expectedError: `"arg=value": query parameters are not permitted`, + }, + { + name: "user", + config: newValidatingWebhookConfiguration( + []admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.WebhookClientConfig{ + URL: strPtr("https://harry.potter@example.com/"), + }, + }, + }), + expectedError: `"harry.potter": user information is not permitted`, + }, { name: "just totally wrong", config: newValidatingWebhookConfiguration( diff --git a/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go b/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go index 60f579651bf..43770b30273 100644 --- a/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go +++ b/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go @@ -288,12 +288,16 @@ type WebhookClientConfig struct { // webhook. Such installs are likely to be non-portable, i.e., not easy // to turn up in a new cluster. // - // If the scheme is present, it must be "https://". + // The scheme must be "https"; the URL must begin with "https://". // // A path is optional, and if present may be any string permissible in // a URL. You may use the path to pass an arbitrary string to the // webhook, for example, a cluster identifier. // + // Attempting to use a user or basic auth e.g. "user:password@" is not + // allowed. Fragments ("#...") and query parameters ("?...") are not + // allowed, either. + // // +optional URL *string `json:"url,omitempty" protobuf:"bytes,3,opt,name=url"` diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/admission.go index 1eba62672d8..7e950132e65 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/admission.go @@ -400,6 +400,9 @@ func toStatusErr(name string, result *metav1.Status) *apierrors.StatusError { func (a *GenericAdmissionWebhook) hookClient(h *v1alpha1.Webhook) (*rest.RESTClient, error) { cacheKey, err := json.Marshal(h.ClientConfig) + if err != nil { + return nil, err + } if client, ok := a.cache.Get(string(cacheKey)); ok { return client.(*rest.RESTClient), nil } @@ -464,7 +467,6 @@ func (a *GenericAdmissionWebhook) hookClient(h *v1alpha1.Webhook) (*rest.RESTCli cfg := rest.CopyConfig(restConfig) cfg.Host = u.Host cfg.APIPath = u.Path - // TODO: test if this is needed: cfg.TLSClientConfig.ServerName = u.Host return complete(cfg) }