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

Add Validation for Client Body Buffer Size #1236

Merged
merged 1 commit into from
Aug 24, 2017

Conversation

diazjf
Copy link

@diazjf diazjf commented Aug 24, 2017

Adds validation so that if a bad value is input into the client body buffer size annotation then client_body_buffer_size is not set. That way a log error is thrown and it fails gracefully rather than killing the ingress controller.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 24, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@diazjf
Copy link
Author

diazjf commented Aug 24, 2017

Ingress Resource:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: ok-ingress
  annotations:
    ingress.kubernetes.io/client-body-buffer-size: BAD
spec:
  tls:
  - hosts:
    - ok.com
  rules:
  - host: ok.com
    http:
      paths:
      - path: /tea
        backend:
          serviceName: tea-svc
          servicePort: 80

Provides Location:

location /tea {
        set $proxy_upstream_name "default-tea-svc-80";

        # enforce ssl on server side
        if ($pass_access_scheme = http) {
        return 301 https://$best_http_host$request_uri;
        }
        port_in_redirect off;

        client_max_body_size                    "1m";

        proxy_set_header Host                   $best_http_host;

        # Pass the extracted client certificate to the backend

        # Allow websocket connections
        proxy_set_header                        Upgrade           $http_upgrade;
        proxy_set_header                        Connection        $connection_upgrade;

        proxy_set_header X-Real-IP              $the_real_ip;
        proxy_set_header X-Forwarded-For        $the_real_ip;
        proxy_set_header X-Forwarded-Host       $best_http_host;
        proxy_set_header X-Forwarded-Port       $pass_port;
        proxy_set_header X-Forwarded-Proto      $pass_access_scheme;
        proxy_set_header X-Original-URI         $request_uri;
        proxy_set_header X-Scheme               $pass_access_scheme;

        # mitigate HTTPoxy Vulnerability
        # https://www.nginx.com/blog/mitigating-the-httpoxy-vulnerability-with-nginx/
        proxy_set_header Proxy                  "";

        # Custom headers to proxied server

        proxy_connect_timeout                   5s;
        proxy_send_timeout                      60s;
        proxy_read_timeout                      60s;

        proxy_redirect                          off;
        proxy_buffering                         off;
        proxy_buffer_size                       "4k";
        proxy_buffers                           4 "4k";

        proxy_http_version                      1.1;

        proxy_cookie_domain                     off;
        proxy_cookie_path                       off;

        # In case of errors try the next upstream server before returning an error
        proxy_next_upstream                     error timeout invalid_header http_502 http_503 http_504;

        proxy_pass http://default-tea-svc-80;
        }

with log:

client-body-buffer-size 'BAD' was provided in an incorrect format, hence it will not be set.

Adds validation so that if a bad value is input into the client
body buffer size annotation then client_body_buffer_size is not set.
That way a log error is thrown and it fails gracefully rather than
killing the ingress controller.
@diazjf diazjf force-pushed the client-buffer-body-validation branch from 4e925ed to 8635733 Compare August 24, 2017 01:04
@diazjf
Copy link
Author

diazjf commented Aug 24, 2017

@aledbf ready for review!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 44.499% when pulling 8635733 on diazjf:client-buffer-body-validation into 338df02 on kubernetes:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 44.499% when pulling 8635733 on diazjf:client-buffer-body-validation into 338df02 on kubernetes:master.

@aledbf aledbf self-assigned this Aug 24, 2017
@aledbf
Copy link
Member

aledbf commented Aug 24, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 24, 2017
@aledbf
Copy link
Member

aledbf commented Aug 24, 2017

@diazjf thanks!

@aledbf aledbf merged commit 02e20eb into kubernetes:master Aug 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants