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 support for enabling ssl_ciphers per host #2006

Merged
merged 2 commits into from
Jan 31, 2018

Conversation

aramase
Copy link
Member

@aramase aramase commented Jan 31, 2018

This PR adds annotation option to configure ssl_ciphers. Using the annotation will set the ssl_ciphers at the server level and will apply to all paths on the host.
(http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_ciphers)

fixes #1956

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

This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 31, 2018
@aramase
Copy link
Member Author

aramase commented Jan 31, 2018

Example ingress resource:

→ cat /tmp/test.yaml
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: test-ing
  annotations:
    nginx.ingress.kubernetes.io/proxy-buffering: "on"
#    nginx.ingress.kubernetes.io/ssl-ciphers: "ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP"
#    nginx.ingress.kubernetes.io/client-body-buffer-size: 1k
spec:
  tls:
  - hosts:
    - test.net
  rules:
  - host: test.net
    http:
      paths:
      - path: /tea
        backend:
          serviceName: tea-svc
          servicePort: 80
  - host: test2.net
    http:
      paths:
      - path: /tea
        backend:
           serviceName: tea-svc
           servicePort: 80
---
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: test-ing2
  annotations:
    nginx.ingress.kubernetes.io/ssl-ciphers: "ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP"
#    nginx.ingress.kubernetes.io/client-body-buffer-size: 1k
spec:
  tls:
  - hosts:
    - test.net
  rules:
  - host: test3.net
    http:
      paths:
      - path: /tea-test
        backend:
          serviceName: tea-svc
          servicePort: 80

Server sections from nginx config generated:

    ## start server test.net
    server {
        server_name test.net ;

        listen 80;

        listen [::]:80;

        set $proxy_upstream_name "-";

        listen 443  ssl http2;

        listen [::]:443  ssl http2;

        # PEM sha: 907d86930e081d4086fa8bea5f25191205c504b1
        ssl_certificate                         /ingress-controller/ssl/default-fake-certificate.pem;
        ssl_certificate_key                     /ingress-controller/ssl/default-fake-certificate.pem;

        more_set_headers                        "Strict-Transport-Security: max-age=15724800; includeSubDomains;";
...

    ## start server test2.net
    server {
        server_name test2.net ;

        listen 80;

        listen [::]:80;

        set $proxy_upstream_name "-";
...

    ## start server test3.net
    server {
        server_name test3.net ;

        listen 80;

        listen [::]:80;

        set $proxy_upstream_name "-";

        ssl_ciphers                             ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP;
...

@aramase
Copy link
Member Author

aramase commented Jan 31, 2018

Another sample ingress resource used for testing:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: test-ing
  annotations:
    nginx.ingress.kubernetes.io/ssl-ciphers: "ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP"
#    nginx.ingress.kubernetes.io/client-body-buffer-size: 1k
spec:
  tls:
  - hosts:
    - test.net
  rules:
  - host: test.net
    http:
      paths:
      - path: /tea
        backend:
          serviceName: tea-svc
          servicePort: 80
  - host: test2.net
    http:
      paths:
      - path: /tea
        backend:
           serviceName: tea-svc
           servicePort: 80
    server {
        server_name test.net ;

        listen 80;

        listen [::]:80;

        set $proxy_upstream_name "-";

        listen 443  ssl http2;

        listen [::]:443  ssl http2;

        # PEM sha: 907d86930e081d4086fa8bea5f25191205c504b1
        ssl_certificate                         /ingress-controller/ssl/default-fake-certificate.pem;
        ssl_certificate_key                     /ingress-controller/ssl/default-fake-certificate.pem;

        more_set_headers                        "Strict-Transport-Security: max-age=15724800; includeSubDomains;";

        ssl_ciphers                             ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP;
...

    ## start server test2.net
    server {
        server_name test2.net ;

        listen 80;

        listen [::]:80;

        set $proxy_upstream_name "-";

        ssl_ciphers                             ALL:!aNULL:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP;
...

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 39.742% when pulling 38cad21 on aramase:ssl-cipher into 2f700a9 on kubernetes:master.

@aledbf
Copy link
Member

aledbf commented Jan 31, 2018

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, aramase

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 31, 2018
@aledbf
Copy link
Member

aledbf commented Jan 31, 2018

@aramase thanks!

@aledbf aledbf merged commit d7ef6b3 into kubernetes:master Jan 31, 2018
@f0o
Copy link

f0o commented May 15, 2018

This doesnt seem to work for the parameter nginx.ingress.kubernetes.io/ssl-protocols or am I mistaken somewhere?

I specified:

  annotations:
    nginx.ingress.kubernetes.io/ssl-ciphers: "ALL"
    nginx.ingress.kubernetes.io/ssl-protocols: "TLSv1 TLSv1.1 TLSv1.2"

But the ingress is still only doing TLSv1.2 and none of the rest, any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST] allow enabling / disabling tls ciphers per service
6 participants