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.

Sign up for GitHub

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

Jump to bottom

Deprecate TLS v1.0, TLS v1.1 and SSLv3 #8633

Closed
rikatz opened this issue May 24, 2022 · 22 comments · Fixed by #11343
Closed

Deprecate TLS v1.0, TLS v1.1 and SSLv3 #8633

rikatz opened this issue May 24, 2022 · 22 comments · Fixed by #11343
Assignees
@longwuyuan
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@rikatz
Copy link
Contributor

rikatz commented May 24, 2022 •
edited

TLS v1.0 and TLS v1.1 is being deprecated in a lot of software:

Go v1.18 already removed it as a default: golang/go#45428

Some SSL Libraries (OpenSSL, as an example) and some browsers already don't support it anymore.

We need to establish a plan to deprecate those and keep only TLS v1.2 to v1.3
@rikatz rikatz added the kind/bug Categorizes issue or PR as related to a bug. label May 24, 2022
@k8s-ci-robot
Copy link
Contributor

@rikatz: This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels May 24, 2022
@rikatz rikatz added kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels May 24, 2022
@rikatz rikatz changed the title Deprecate TLS v1.0 and SSLv3 Deprecate TLS v1.0, TLS v1.1 and SSLv3 May 24, 2022
@rikatz rikatz mentioned this issue May 24, 2022
Merged
@tao12345666333
Copy link
Member

tao12345666333 commented May 25, 2022 •
edited

I think it's valuable and to be more secure

But considering that this project is a Proxy, we need to have a clear deprecation plan to avoid sudden impact on users

@rikatz
Copy link
Contributor Author

rikatz commented May 25, 2022

agreed, not sure what approach we can take. Maybe for the next release a feature flag with tls disabled but being able to enable it, then removing on the other one

@tao12345666333 tao12345666333 mentioned this issue May 26, 2022
Closed
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 24, 2022
@rikatz
Copy link
Contributor Author

rikatz commented Aug 24, 2022

/lifecycle active

@k8s-ci-robot k8s-ci-robot added lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 24, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. labels Nov 22, 2022
@rikatz
Copy link
Contributor Author

rikatz commented Nov 22, 2022

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 22, 2022
@tao12345666333
Copy link
Member

I think we have achieved the goal of this issue and the documentation has also been updated. #10473

@rikatz
Copy link
Contributor Author

rikatz commented Oct 5, 2023

We still support people setting tls 1.0, not?

@tao12345666333
Copy link
Member

In fact, as described in the PR I quoted above, it can no longer be set after 1.6. I will check again later with the latest version

@rikatz
Copy link
Contributor Author

rikatz commented Oct 11, 2023

https://github.com/kubernetes/ingress-nginx/blob/main/internal/ingress/annotations/proxyssl/main.go#L43 here's an example where we should be removing it

@longwuyuan
Copy link
Contributor

/assign
/triage accepted
/priority backlog

@k8s-ci-robot k8s-ci-robot added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label May 1, 2024
@k8s-ci-robot k8s-ci-robot added the priority/backlog Higher priority than priority/awaiting-more-evidence. label May 1, 2024
@longwuyuan
Copy link
Contributor

image

@longwuyuan
Copy link
Contributor

Minimum requirement now is TLSv1.2

% curl httpbun.dev.enjoydevops.com -LI --tls-max 1.0
HTTP/1.1 308 Permanent Redirect
Date: Wed, 01 May 2024 12:30:18 GMT
Content-Type: text/html
Content-Length: 164
Connection: keep-alive
Location: https://httpbun.dev.enjoydevops.com

curl: (35) OpenSSL/3.2.1: error:0A0000BF:SSL routines::no protocols available
[~] 
% curl httpbun.dev.enjoydevops.com -LI --tls-max 1.1
HTTP/1.1 308 Permanent Redirect
Date: Wed, 01 May 2024 12:30:23 GMT
Content-Type: text/html
Content-Length: 164
Connection: keep-alive
Location: https://httpbun.dev.enjoydevops.com

curl: (35) OpenSSL/3.2.1: error:0A0000BF:SSL routines::no protocols available
[~] 
% curl httpbun.dev.enjoydevops.com -LI --tls-max 1.2
HTTP/1.1 308 Permanent Redirect
Date: Wed, 01 May 2024 12:30:38 GMT
Content-Type: text/html
Content-Length: 164
Connection: keep-alive
Location: https://httpbun.dev.enjoydevops.com

HTTP/2 200 
date: Wed, 01 May 2024 12:30:38 GMT
content-type: text/html
x-powered-by: httpbun/af040d24038613575a85f74c2283ae79f8169927
strict-transport-security: max-age=31536000; includeSubDomains

The logs are supporting



172.19.0.1 - - [01/May/2024:12:30:18 +0000] "HEAD / HTTP/1.1" 308 0 "-" "curl/8.6.0" 91 0.000 [httpbun-httpbun-80] [] - - - - dee123caed56fa2794a205ad379927f5
172.19.0.1 - - [01/May/2024:12:30:18 +0000] "\x15\x03\x03\x00\x02\x02F" 400 150 "-" "-" 0 0.015 [] [] - - - - c40f8da89c7285ef5e5ce21075fac172
172.19.0.1 - - [01/May/2024:12:30:23 +0000] "HEAD / HTTP/1.1" 308 0 "-" "curl/8.6.0" 91 0.000 [httpbun-httpbun-80] [] - - - - c78476580e57d853e5ca7e1e08fd6601
172.19.0.1 - - [01/May/2024:12:30:23 +0000] "\x15\x03\x03\x00\x02\x02F" 400 150 "-" "-" 0 0.009 [] [] - - - - de489d962a6a1886c3c57a5e8c859c47
172.19.0.1 - - [01/May/2024:12:30:38 +0000] "HEAD / HTTP/1.1" 308 0 "-" "curl/8.6.0" 91 0.000 [httpbun-httpbun-80] [] - - - - 7ae702c0ab3c8467d61298fb449b5133
172.19.0.1 - - [01/May/2024:12:30:38 +0000] "HEAD / HTTP/2.0" 200 0 "-" "curl/8.6.0" 45 0.002 [httpbun-httpbun-80] [] 10.244.0.3:80 0 0.002 200 85f5f302dd8f8f12089f539b0debee88

So I will close the issue for now. If we find the deprecation is not complete, then we can re-open this.

cc @rikatz @tao12345666333

/close

@k8s-ci-robot
Copy link
Contributor

@longwuyuan: Closing this issue.

In response to this:

Minimum requirement now is TLSv1.2

% curl httpbun.dev.enjoydevops.com -LI --tls-max 1.0
HTTP/1.1 308 Permanent Redirect
Date: Wed, 01 May 2024 12:30:18 GMT
Content-Type: text/html
Content-Length: 164
Connection: keep-alive
Location: https://httpbun.dev.enjoydevops.com

curl: (35) OpenSSL/3.2.1: error:0A0000BF:SSL routines::no protocols available
[~] 
% curl httpbun.dev.enjoydevops.com -LI --tls-max 1.1
HTTP/1.1 308 Permanent Redirect
Date: Wed, 01 May 2024 12:30:23 GMT
Content-Type: text/html
Content-Length: 164
Connection: keep-alive
Location: https://httpbun.dev.enjoydevops.com

curl: (35) OpenSSL/3.2.1: error:0A0000BF:SSL routines::no protocols available
[~] 
% curl httpbun.dev.enjoydevops.com -LI --tls-max 1.2
HTTP/1.1 308 Permanent Redirect
Date: Wed, 01 May 2024 12:30:38 GMT
Content-Type: text/html
Content-Length: 164
Connection: keep-alive
Location: https://httpbun.dev.enjoydevops.com

HTTP/2 200 
date: Wed, 01 May 2024 12:30:38 GMT
content-type: text/html
x-powered-by: httpbun/af040d24038613575a85f74c2283ae79f8169927
strict-transport-security: max-age=31536000; includeSubDomains

The logs are supporting



172.19.0.1 - - [01/May/2024:12:30:18 +0000] "HEAD / HTTP/1.1" 308 0 "-" "curl/8.6.0" 91 0.000 [httpbun-httpbun-80] [] - - - - dee123caed56fa2794a205ad379927f5
172.19.0.1 - - [01/May/2024:12:30:18 +0000] "\x15\x03\x03\x00\x02\x02F" 400 150 "-" "-" 0 0.015 [] [] - - - - c40f8da89c7285ef5e5ce21075fac172
172.19.0.1 - - [01/May/2024:12:30:23 +0000] "HEAD / HTTP/1.1" 308 0 "-" "curl/8.6.0" 91 0.000 [httpbun-httpbun-80] [] - - - - c78476580e57d853e5ca7e1e08fd6601
172.19.0.1 - - [01/May/2024:12:30:23 +0000] "\x15\x03\x03\x00\x02\x02F" 400 150 "-" "-" 0 0.009 [] [] - - - - de489d962a6a1886c3c57a5e8c859c47
172.19.0.1 - - [01/May/2024:12:30:38 +0000] "HEAD / HTTP/1.1" 308 0 "-" "curl/8.6.0" 91 0.000 [httpbun-httpbun-80] [] - - - - 7ae702c0ab3c8467d61298fb449b5133
172.19.0.1 - - [01/May/2024:12:30:38 +0000] "HEAD / HTTP/2.0" 200 0 "-" "curl/8.6.0" 45 0.002 [httpbun-httpbun-80] [] 10.244.0.3:80 0 0.002 200 85f5f302dd8f8f12089f539b0debee88

So I will close the issue for now. If we find the deprecation is not complete, then we can re-open this.

cc @rikatz @tao12345666333

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@tao12345666333
Copy link
Member

I'll add some more information for others to refer to.

I use the following configuration and directly use it through the openresty openresty/openresty:1.25.3.1-buster-fat image.

server {                                                                       
  server_name ingress.moelove-test.xyz ;                                      
  http2 on;                                                                                                                                                     
                                                                                                                                                          
  listen 80  ;                                                                                                                                                  
  listen [::]:80  ;                                                      
  listen 443  ssl;                                                       
  listen [::]:443  ssl;                                                  

  ssl_certificate /etc/nginx/conf.d/ingress.cert;
  ssl_certificate_key /etc/nginx/conf.d/ingress.key;
                                                                   
  ssl_protocols TLSv1 TLSv1.1 TLSv1.2 TLSv1.3;
  ssl_ciphers 'DHE-PSK-AES128-CBC-SHA256:DHE-PSK-AES256-CBC-SHA384:ECDHE-ECDSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA:ECDHE-PSK-AES128-CBC-SHA:ECDHE-PSK-AES128-CBC-SHA256:ECDHE-PSK-AES256-CBC-SHA:ECDHE-PSK-AES256-CBC-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-SHA:PSK-AES128-CBC-SHA256:PSK-AES256-CBC-SHA384:RSA-PSK-AES128-CBC-SHA256:RSA-PSK-AES256-CBC-SHA384:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA256:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:@SECLEVEL=0';



  location / {
        proxy_pass http://httpbin.org;
        proxy_set_header Host $host;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header X-Forwarded-Proto $scheme;
  }
}

It can only work with TLS v1.2+ for requests.

In addition, as mentioned by @longwuyuan in the above comment, I also configured it through configmap and annotations. Similarly, it can only work with TLS v1.2+.

@longwuyuan
Copy link
Contributor

/reopen

I will put in a PR to remove references to TLSv1 & TLSv1.1 in the const and the var in the proxyprotocol.go

@k8s-ci-robot
Copy link
Contributor

@longwuyuan: Reopened this issue.

In response to this:

/reopen

I will put in a PR to remove references to TLSv1 & TLSv1.1 in the const and the var in the proxyprotocol.go

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot reopened this May 4, 2024
@tao12345666333
Copy link
Member

tao12345666333 commented May 4, 2024 •
edited

In addition, the following annotation seems to not be working properly. I will investigate why it did not take effect when I have time.

nginx.ingress.kubernetes.io/proxy-ssl-protocols
nginx.ingress.kubernetes.io/proxy-ssl-ciphers

I can only use configmap to change the configurations related ssl_protocols and ssl_ciphers.

@longwuyuan
Copy link
Contributor

great, there are open issues related to that annotation, if I am not wrong

@longwuyuan
Copy link
Contributor

@tao12345666333 @rikatz , I understand tons of software ship with SSLv2 and SSLv3 support but since we are in this issue, what is the general opinion you both have in keeping SSLv2 & SSLv3 showing up in the code. Since I am already trying to submit a cosmetic PR to remove the strings TLSv1 & TLSv1.1, I can also remove the strings SSLv2 & SSLv3, if you think its an improvement.

@tao12345666333
Copy link
Member

Since TLSv1.0 and v1.1 are no longer applicable, SSL v2 and v3 can also be removed.

@longwuyuan longwuyuan mentioned this issue May 4, 2024
Merged
10 tasks
@Tommyf Tommyf mentioned this issue May 30, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@longwuyuan longwuyuan

Labels
kind/bug Categorizes issue or PR as related to a bug. kind/deprecation Categorizes issue or PR as related to a feature/enhancement marked for deprecation. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Annotations: Drop SSLv2, SSLv3, TLSv1 & TLSv1.1 support. longwuyuan/ingress-nginx
5 participants
@tao12345666333 @longwuyuan @rikatz @k8s-ci-robot @k8s-triage-robot