github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-28 14:07:14 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #55534 from lavalamp/wh-api-fixes

Browse Source 
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Tighten webhook client config validation

ref https://github.com/kubernetes/features/issues/492

Fix up some nits left from #54889.

```release-note
NONE
```
This commit is contained in:
Kubernetes Submit Queue 2017-11-12 06:04:50 -08:00 committed by GitHub
commit 2db28383e1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 77 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
api/openapi-spec/swagger.json generated
View File

@ -68056,7 +68056,7 @@
"$ref": "#/definitions/io.k8s.api.admissionregistration.v1alpha1.ServiceReference"
},
"url": {
"description": "`url` gives the location of the webhook, in standard URL form (`[scheme://]host:port/path`). Exactly one of `url` or `service` must be specified.\n\nThe `host` should not refer to a service running in the cluster; use the `service` field instead. The host might be resolved via external DNS in some apiservers (e.g., `kube-apiserver` cannot resolve in-cluster DNS as that would be a layering violation). `host` may also be an IP address.\n\nPlease note that using `localhost` or `127.0.0.1` as a `host` is risky unless you take great care to run this webhook on all hosts which run an apiserver which might need to make calls to this webhook. Such installs are likely to be non-portable, i.e., not easy to turn up in a new cluster.\n\nIf the scheme is present, it must be \"https://\".\n\nA 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.",
"description": "`url` gives the location of the webhook, in standard URL form (`[scheme://]host:port/path`). Exactly one of `url` or `service` must be specified.\n\nThe `host` should not refer to a service running in the cluster; use the `service` field instead. The host might be resolved via external DNS in some apiservers (e.g., `kube-apiserver` cannot resolve in-cluster DNS as that would be a layering violation). `host` may also be an IP address.\n\nPlease note that using `localhost` or `127.0.0.1` as a `host` is risky unless you take great care to run this webhook on all hosts which run an apiserver which might need to make calls to this webhook. Such installs are likely to be non-portable, i.e., not easy to turn up in a new cluster.\n\nThe scheme must be \"https\"; the URL must begin with \"https://\".\n\nA 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.\n\nAttempting to use a user or basic auth e.g. \"user:password@\" is not allowed. Fragments (\"#...\") and query parameters (\"?...\") are not allowed, either.",
"type": "string"
}
}

2
api/swagger-spec/admissionregistration.k8s.io_v1alpha1.json generated
View File

@ -2636,7 +2636,7 @@
"properties": {
"url": {
"type": "string",
"description": "`url` gives the location of the webhook, in standard URL form (`[scheme://]host:port/path`). Exactly one of `url` or `service` must be specified.\n\nThe `host` should not refer to a service running in the cluster; use the `service` field instead. The host might be resolved via external DNS in some apiservers (e.g., `kube-apiserver` cannot resolve in-cluster DNS as that would be a layering violation). `host` may also be an IP address.\n\nPlease note that using `localhost` or `127.0.0.1` as a `host` is risky unless you take great care to run this webhook on all hosts which run an apiserver which might need to make calls to this webhook. Such installs are likely to be non-portable, i.e., not easy to turn up in a new cluster.\n\nIf the scheme is present, it must be \"https://\".\n\nA 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."
"description": "`url` gives the location of the webhook, in standard URL form (`[scheme://]host:port/path`). Exactly one of `url` or `service` must be specified.\n\nThe `host` should not refer to a service running in the cluster; use the `service` field instead. The host might be resolved via external DNS in some apiservers (e.g., `kube-apiserver` cannot resolve in-cluster DNS as that would be a layering violation). `host` may also be an IP address.\n\nPlease note that using `localhost` or `127.0.0.1` as a `host` is risky unless you take great care to run this webhook on all hosts which run an apiserver which might need to make calls to this webhook. Such installs are likely to be non-portable, i.e., not easy to turn up in a new cluster.\n\nThe scheme must be \"https\"; the URL must begin with \"https://\".\n\nA 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.\n\nAttempting to use a user or basic auth e.g. \"user:password@\" is not allowed. Fragments (\"#...\") and query parameters (\"?...\") are not allowed, either."
},
"service": {
"$ref": "v1alpha1.ServiceReference",

6
docs/api-reference/admissionregistration.k8s.io/v1alpha1/definitions.html generated
View File

@ -634,9 +634,11 @@ The <code>host</code> should not refer to a service running in the cluster; use
<br>
Please note that using <code>localhost</code> or <code>127.0.0.1</code> as a <code>host</code> is risky unless you take great care to run this webhook on all hosts which run an apiserver which might need to make calls to this webhook. Such installs are likely to be non-portable, i.e., not easy to turn up in a new cluster.<br>
<br>
If the scheme is present, it must be "https://".<br>
The scheme must be "https"; the URL must begin with "https://".<br>
<br>
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.</p></td>
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.<br>
<br>
Attempting to use a user or basic auth e.g. "user:password@" is not allowed. Fragments ("#&#8230;") and query parameters ("?&#8230;") are not allowed, either.</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
<td class="tableblock halign-left valign-top"><p class="tableblock">string</p></td>
<td class="tableblock halign-left valign-top"></td>

6
pkg/apis/admissionregistration/types.go
View File

@ -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

15
pkg/apis/admissionregistration/validation/validation.go
View File

@ -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"))
}
}
}

41
pkg/apis/admissionregistration/validation/validation_test.go
View File

@ -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(

6
staging/src/k8s.io/api/admissionregistration/v1alpha1/generated.proto
View File

@ -274,12 +274,16 @@ message WebhookClientConfig {
// 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
optional string url = 3;

6
staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go
View File

@ -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"`

2
staging/src/k8s.io/api/admissionregistration/v1alpha1/types_swagger_doc_generated.go generated
View File

@ -143,7 +143,7 @@ func (Webhook) SwaggerDoc() map[string]string {
var map_WebhookClientConfig = map[string]string{
"": "WebhookClientConfig contains the information to make a TLS connection with the webhook",
"url": "`url` gives the location of the webhook, in standard URL form (`[scheme://]host:port/path`). Exactly one of `url` or `service` must be specified.\n\nThe `host` should not refer to a service running in the cluster; use the `service` field instead. The host might be resolved via external DNS in some apiservers (e.g., `kube-apiserver` cannot resolve in-cluster DNS as that would be a layering violation). `host` may also be an IP address.\n\nPlease note that using `localhost` or `127.0.0.1` as a `host` is risky unless you take great care to run this webhook on all hosts which run an apiserver which might need to make calls to this webhook. Such installs are likely to be non-portable, i.e., not easy to turn up in a new cluster.\n\nIf the scheme is present, it must be \"https://\".\n\nA 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.",
"url": "`url` gives the location of the webhook, in standard URL form (`[scheme://]host:port/path`). Exactly one of `url` or `service` must be specified.\n\nThe `host` should not refer to a service running in the cluster; use the `service` field instead. The host might be resolved via external DNS in some apiservers (e.g., `kube-apiserver` cannot resolve in-cluster DNS as that would be a layering violation). `host` may also be an IP address.\n\nPlease note that using `localhost` or `127.0.0.1` as a `host` is risky unless you take great care to run this webhook on all hosts which run an apiserver which might need to make calls to this webhook. Such installs are likely to be non-portable, i.e., not easy to turn up in a new cluster.\n\nThe scheme must be \"https\"; the URL must begin with \"https://\".\n\nA 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.\n\nAttempting to use a user or basic auth e.g. \"user:password@\" is not allowed. Fragments (\"#...\") and query parameters (\"?...\") are not allowed, either.",
"service": "`service` is a reference to the service for this webhook. Either `service` or `url` must be specified.\n\nIf the webhook is running within the cluster, then you should use `service`.\n\nIf there is only one port open for the service, that port will be used. If there are multiple ports open, port 443 will be used if it is open, otherwise it is an error.",
"caBundle": "`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate. Required.",
}

4
staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/admission.go
View File

@ -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)
}