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

Configurable SSL cipher suite #9691

Closed
2 tasks done
tgurr opened this issue Jan 10, 2020 · 38 comments · Fixed by #17440
Closed
2 tasks done

Configurable SSL cipher suite #9691

tgurr opened this issue Jan 10, 2020 · 38 comments · Fixed by #17440
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@tgurr
Copy link
Contributor

tgurr commented Jan 10, 2020

  • Gitea version: 1.10.2
  • Git version: 2.24.0
  • Operating system: Linux
  • Database:
    • MySQL
  • Can you reproduce the bug at https://try.gitea.io:
    • Not tested / may run behind a reverse proxy so the results could vary

Description

'Vulnerable' cipher suites accepted by this service via the TLSv1.0 protocol:

TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA (SWEET32)
TLS_RSA_WITH_3DES_EDE_CBC_SHA (SWEET32)

'Vulnerable' cipher suites accepted by this service via the TLSv1.1 protocol:

TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA (SWEET32)
TLS_RSA_WITH_3DES_EDE_CBC_SHA (SWEET32)

'Vulnerable' cipher suites accepted by this service via the TLSv1.2 protocol:

TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA (SWEET32)
TLS_RSA_WITH_3DES_EDE_CBC_SHA (SWEET32)

#913 added configurable SSH cipher suite, it would be nice to have the same to configure the SSL cipher suite since currently TLSv1.0 and TLSv1.1 are still accepted which shouldn't be used in 2020 anymore: https://blog.qualys.com/ssllabs/2018/11/19/grade-change-for-tls-1-0-and-tls-1-1-protocols
and it would also be nice to configure excludes for TLSv1.2 to get rid of the weak ones there as well / provide a way to configure them in general.

@tgurr tgurr changed the title Configurable SSL cipher suites Configurable SSL cipher suite Jan 10, 2020
@bagasme
Copy link
Contributor

bagasme commented Jan 10, 2020

@tgurr and perhaps enforcing TLS v1.2?

@tgurr
Copy link
Contributor Author

tgurr commented Jan 10, 2020

@bagasme no objections from my side, maybe it should just be the default setting then. Just a note and directly related to Gitea, we encountered some problems on Windows 7 / Server 2008 when disabling TLSv1.0 and TLSv1.1 in other software, they're EOL anyways, but there may still be use cases for it.

@clarfonthey
Copy link
Contributor

It may be worthwhile to simplify the configuration options to allow picking between some of Mozilla's recommendations (i.e. Modern, Intermediate, or Old), as shown here: https://wiki.mozilla.org/Security/Server_Side_TLS

Also, adding onto this: this is TLS, not SSL. It seems pedantic but it is important to mention.

@lunny lunny added type/proposal The new feature has not been accepted yet but needs to be discussed first. topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! labels Jan 11, 2020
@CRCinAU
Copy link

CRCinAU commented May 13, 2020

Oh good - I managed to find this when looking for the exact same... Currently, I see:

 SSLv2      not offered (OK)
 SSLv3      not offered (OK)
 TLS 1      offered (deprecated)
 TLS 1.1    offered (deprecated)
 TLS 1.2    offered (OK)
 TLS 1.3    offered (OK): final
 NPN/SPDY   h2, http/1.1, acme-tls/1 (advertised)
 ALPN/HTTP2 h2, http/1.1 (offered)
...
 Has server cipher order?     no (NOT ok)
...
 Strict Transport Security    not offered
...
 SWEET32 (CVE-2016-2183, CVE-2016-6329)    VULNERABLE, uses 64 bit block ciphers
...
BEAST (CVE-2011-3389)                     TLS1: ECDHE-ECDSA-AES256-SHA
                                                 ECDHE-ECDSA-AES128-SHA
                                                 ECDHE-RSA-AES256-SHA
                                                 AES256-SHA
                                                 ECDHE-RSA-AES128-SHA
                                                 AES128-SHA
                                                 ECDHE-RSA-DES-CBC3-SHA
                                                 DES-CBC3-SHA 
                                           VULNERABLE -- but also supports higher protocols  TLSv1.1 TLSv1.2 (likely mitigated)
 LUCKY13 (CVE-2013-0169), experimental     potentially VULNERABLE, uses cipher block chaining (CBC) ciphers with TLS. Check patches

It would certainly be good to get this part of things configurable...

@zeripath
Copy link
Contributor

The place to adjust this would likely be in modules/graceful/server.go:97:ListenAndServeTLS

you'd need to add the appropriate settings to modules/settings/setting.go probably add some settings at line 91 and line 580.

@rmbleeker
Copy link

Is this actively being worked on? I got back a network scan showing the same results and was asked if I could disable the potentially vulnerable cipher suites. If this is not on the roadmap for a near future Gitea release I might just install a reverse proxy as a work-around.

@eduardok
Copy link

eduardok commented Oct 8, 2021

Is this actively being worked on? I got back a network scan showing the same results and was asked if I could disable the potentially vulnerable cipher suites. If this is not on the roadmap for a near future Gitea release I might just install a reverse proxy as a work-around.

Edited my comment, you are right, I used the ones for SSH (all good), but it seems HTTPS is still not covered, I see I have a reverse proxy set up to get that cleared.

@CRCinAU
Copy link

CRCinAU commented Oct 9, 2021

Running 1.15.x, I see a much more sane config without having to alter the app.ini anywhere:

SSLv2
 - 
SSLv3
 - 
TLSv1
 - 
TLSv1.1
 - 
TLSv1.2 (server order)
 xc02c   ECDHE-ECDSA-AES256-GCM-SHA384     ECDH 256   AESGCM      256      TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384            
 xc02b   ECDHE-ECDSA-AES128-GCM-SHA256     ECDH 256   AESGCM      128      TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256            
 xcca9   ECDHE-ECDSA-CHACHA20-POLY1305     ECDH 256   ChaCha20    256      TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256      
TLSv1.3 (no server order, thus listed by strength)
 x1302   TLS_AES_256_GCM_SHA384            ECDH 253   AESGCM      256      TLS_AES_256_GCM_SHA384                             
 x1303   TLS_CHACHA20_POLY1305_SHA256      ECDH 253   ChaCha20    256      TLS_CHACHA20_POLY1305_SHA256                       
 x1301   TLS_AES_128_GCM_SHA256            ECDH 253   AESGCM      128      TLS_AES_128_GCM_SHA256    

Looks like at some point it was tightened up a bit...

@zeripath
Copy link
Contributor

zeripath commented Oct 9, 2021

Describe the settings, the values it would take and what you expect them to do and it would be relatively easy to implement.

@eduardok
Copy link

M_SHA256

I will see if I have time to upgrade my Gitea to latest this Monday/tomorrow, then will make it visible to the scanner and see if it still complains, otherwise I'll let you know what ciphers should be removed.

@rmbleeker
Copy link

rmbleeker commented Oct 11, 2021

Running 1.15.x, I see a much more sane config without having to alter the app.ini anywhere:

SSLv2
 - 
SSLv3
 - 
TLSv1
 - 
TLSv1.1
 - 
TLSv1.2 (server order)
 xc02c   ECDHE-ECDSA-AES256-GCM-SHA384     ECDH 256   AESGCM      256      TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384            
 xc02b   ECDHE-ECDSA-AES128-GCM-SHA256     ECDH 256   AESGCM      128      TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256            
 xcca9   ECDHE-ECDSA-CHACHA20-POLY1305     ECDH 256   ChaCha20    256      TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256      
TLSv1.3 (no server order, thus listed by strength)
 x1302   TLS_AES_256_GCM_SHA384            ECDH 253   AESGCM      256      TLS_AES_256_GCM_SHA384                             
 x1303   TLS_CHACHA20_POLY1305_SHA256      ECDH 253   ChaCha20    256      TLS_CHACHA20_POLY1305_SHA256                       
 x1301   TLS_AES_128_GCM_SHA256            ECDH 253   AESGCM      128      TLS_AES_128_GCM_SHA256    

Looks like at some point it was tightened up a bit...

It's curious that I don't see any 3DES ciphers show up in your overview, while our network scan reports that they are offered by Gitea 1.15.3. Here is the relevant part of the scan report: https://gist.github.com/rmbleeker/13168441a2297e0f10faa39242c072a4

@rmbleeker
Copy link

rmbleeker commented Oct 11, 2021

Describe the settings, the values it would take and what you expect them to do and it would be relatively easy to implement.

It would probably be a good idea to implement this in the way Nginx does it for example: http://nginx.org/en/docs/http/ngx_http_ssl_module.html
So the options would then be:

  • SSL_PROTOCOLS = TLSv1.2, TLSv1.3
  • SSL_CIPHERS = some string that defines the ciphers that are enabled, or disabled if preceded by an exclamation mark

edit: since Gitea works with comma separated lists rather than space separated, modified my comment slightly for consistency with other Gitea options that take a list of values.

@CRCinAU
Copy link

CRCinAU commented Oct 11, 2021

they are offered by Gitea 1.5.3

Note that I'm running 1.15.x not 1.5.x... Earlier versions did indeed offer everything and the kitchen sink...

@rmbleeker
Copy link

they are offered by Gitea 1.5.3

Note that I'm running 1.15.x not 1.5.x... Earlier versions did indeed offer everything and the kitchen sink...

Oops, typo. We're also running 1.15.3. I corrected my earlier post.

@eduardok
Copy link

I've upgraded to 1.15.4 and made it listen on all interfaces again. My Tenable scanner runs late on Wednesday, so I'll post results on Thursday.

@eduardok
Copy link

Ok, Tenable shows these two as issues:

  Medium Strength Ciphers (> 64-bit and < 112-bit key, or 3DES)

    Name                          Code             KEX           Auth     Encryption             MAC
    ----------------------        ----------       ---           ----     ---------------------  ---
    ECDHE-RSA-DES-CBC3-SHA        0xC0, 0x12       ECDH          RSA      3DES-CBC(168)          SHA1
    DES-CBC3-SHA                  0x00, 0x0A       RSA           RSA      3DES-CBC(168)          SHA1

@zeripath
Copy link
Contributor

Describe the settings, the values it would take and what you expect them to do and it would be relatively easy to implement.

@CRCinAU
Copy link

CRCinAU commented Oct 19, 2021

I don't see the SSL ciphers you see in your scan. I'm wondering if it has more to do with the systems openssl version?

@rmbleeker
Copy link

Describe the settings, the values it would take and what you expect them to do and it would be relatively easy to implement.

I haven't been able to find any guidelines for a naming convention for the options in the app.ini file, but here's a suggestion for the protocols and ciphers:

  • SSL_PROTOCOLS = a comma separated list of SSL/TLS protocol versions (e.g.: SSL3.0, TLSv1.0, TLSv1.1, TLSv1.2, TLSv1.3)
  • SSL_CIPHERS = a list of ciphers that's allowed to be negotiated for the secure connection. Could be a comma separated list explicitly listing the allowed ciphers, but I'd prefer if this was done in a similar way as to how OpenSSL does it. See https://www.openssl.org/docs/man3.0/man1/openssl-ciphers.html for details.

Here's my suggestion for options to configure the HSTS header:

  • SSL_HSTS_ENABLED = true/false
  • SSL_HSTS_MAX_AGE = 31536000 (the minimum requirement for hsts preloading according to https://hstspreload.org/)
  • SSL_HSTS_INCLUDE_SUBDOMAINS = true/false
  • SSL_HSTS_PRELOAD = true/false

@rmbleeker
Copy link

I don't see the SSL ciphers you see in your scan. I'm wondering if it has more to do with the systems openssl version?

That might be the case if Gitea (or Go) is using the system's OpenSSL for TLS connections. I have Gitea on a RHEL7 system which has OpenSSL 1.0.2k-fips as it's OpenSSL library and seeing the same cipher suites in our scans as @eduardok.

@CRCinAU
Copy link

CRCinAU commented Oct 19, 2021

I'm using OpenSSL 1.1.1l - so I'm guessing that explains the cipher selections.

Technically, OpenSSL 1.0.2 is no longer in support.

@rmbleeker
Copy link

Technically, OpenSSL 1.0.2 is no longer in support.

The developers of OpenSSL might not support it any longer, but as long as RHEL7 is supported, Red Hat supports any of the software that's found in their official repositories. Either way, once we're able to configure the list of ciphers Gitea is allowed to use for TLS connections I'll disable all ciphers using 3DES.

@CRCinAU
Copy link

CRCinAU commented Oct 19, 2021

You shouldn't need to. Modify the system policy:
https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/security_hardening/using-the-system-wide-cryptographic-policies_security-hardening

(EDIT: Yes, I know this is for RHEL8, but the procedures are pretty much the same)

@eduardok
Copy link

You shouldn't need to. Modify the system policy: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/security_hardening/using-the-system-wide-cryptographic-policies_security-hardening

(EDIT: Yes, I know this is for RHEL8, but the procedures are pretty much the same)

Very good point, I've changed it to FUTURE now, and I should know after tomorrow's scan.

@rmbleeker
Copy link

You shouldn't need to. Modify the system policy: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/security_hardening/using-the-system-wide-cryptographic-policies_security-hardening
(EDIT: Yes, I know this is for RHEL8, but the procedures are pretty much the same)

Very good point, I've changed it to FUTURE now, and I should know after tomorrow's scan.

I assume you're on RHEL8 then? Because if these commands to conveniently change the crypto policy system-wide exist on RHEL7, I haven't been able to find them for years. All the documentation I have for RHEL7 suggests that this needs to be configured on a per-application basis, which is why I'm interested in this functionality for Gitea. I would love to be proven wrong though.

@eduardok
Copy link

You shouldn't need to. Modify the system policy: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/security_hardening/using-the-system-wide-cryptographic-policies_security-hardening
(EDIT: Yes, I know this is for RHEL8, but the procedures are pretty much the same)

Very good point, I've changed it to FUTURE now, and I should know after tomorrow's scan.

I assume you're on RHEL8 then? Because if these commands to conveniently change the crypto policy system-wide exist on RHEL7, I haven't been able to find them for years. All the documentation I have for RHEL7 suggests that this needs to be configured on a per-application basis, which is why I'm interested in this functionality for Gitea. I would love to be proven wrong though.

Gitea is running alone on a Fedora 33, and update-crypto-policies is available. It won't be running there for much longer, but it would be nice to clear this doubt before I move it.

@lunny
Copy link
Member

lunny commented Oct 20, 2021

Gitea (or Go) itself doesn't depend on openssl. If you want that, you could use nginx as a reverse proxy of Gitea. I mean for a workround before there is any PR to send and merged.

@eduardok
Copy link

You shouldn't need to. Modify the system policy: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/security_hardening/using-the-system-wide-cryptographic-policies_security-hardening

(EDIT: Yes, I know this is for RHEL8, but the procedures are pretty much the same)

As indicated by @lunny , update-crypto-policies had no effect, the scan still shows the same weak ciphers.

@CRCinAU
Copy link

CRCinAU commented Oct 24, 2021

I wonder why my scan is so different to yours then.... Same gitea version - just I'm using Fedora 34 instead of RHEL7...

As a reminder, I get:

Hexcode  Cipher Suite Name (OpenSSL)       KeyExch.   Encryption  Bits     Cipher Suite Name (IANA/RFC)
-----------------------------------------------------------------------------------------------------------------------------
SSLv2
 - 
SSLv3
 - 
TLSv1
 - 
TLSv1.1
 - 
TLSv1.2 (server order)
 xc02c   ECDHE-ECDSA-AES256-GCM-SHA384     ECDH 256   AESGCM      256      TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384            
 xc02b   ECDHE-ECDSA-AES128-GCM-SHA256     ECDH 256   AESGCM      128      TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256            
 xcca9   ECDHE-ECDSA-CHACHA20-POLY1305     ECDH 256   ChaCha20    256      TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256      
TLSv1.3 (no server order, thus listed by strength)
 x1302   TLS_AES_256_GCM_SHA384            ECDH 253   AESGCM      256      TLS_AES_256_GCM_SHA384                             
 x1303   TLS_CHACHA20_POLY1305_SHA256      ECDH 253   ChaCha20    256      TLS_CHACHA20_POLY1305_SHA256                       
 x1301   TLS_AES_128_GCM_SHA256            ECDH 253   AESGCM      128      TLS_AES_128_GCM_SHA256 

zeripath added a commit to zeripath/gitea that referenced this issue Oct 26, 2021
Add options for configuring SSL cipher suite.

Fix go-gitea#9691
@zeripath
Copy link
Contributor

I think I have an answer as to why

I suspect @CRCinAU is using letsencrypt and the @eduardok is not.

@CRCinAU
Copy link

CRCinAU commented Oct 27, 2021

Likely, I do use letsencrypt...

@andypost
Copy link

having issue to build 1.15.4 for Alpinelinux because of RSA keys in tests. Basically we found no way to point tests to use pre-configured /.gitconfig and ~/.ssh even trying to override HOME for tests

Ref https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/26273

@zeripath
Copy link
Contributor

having issue to build 1.15.4 for Alpinelinux because of RSA keys in tests. Basically we found no way to point tests to use pre-configured /.gitconfig and ~/.ssh even trying to override HOME for tests

Ref https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/26273

This doesn't seem completely related to this issue and is more related to #17175.

I don't understand though as #17376 should have fixed this. So I'm confused as to why this is still happening.

It would be helpful to see the contents of /etc/ssh/ssh_config

You could patch integrations/git_helper_for_declarative_test.go:withKeyFile() to add another -o option to specifically allow an rsa cipher algorithms.

@andypost
Copy link

@zeripath thank you, following

@zeripath
Copy link
Contributor

zeripath commented Nov 1, 2021

Aha! #17376 only fixed the host key algorithm not the pub key algorithm part of the problem.

So you don't need to add ssh-rsa to your host keys but you would need to add it to the publickeys (!)

Damn. Need to do a bit more work here.


for others wanting to replicate the bug but who have an ssh that has ssh-rsa in by default

ssh git@localhost -p 2222 -o HostKeyAlgorithms=-ssh-rsa -o pubkeyacceptedkeytypes=-ssh-rsa -i ~/.ssh/id_rsa

would disable the ssh-rsa algorithm. (Note that the HostKeyAlgorithms=-ssh-rsa alone works fine)

@zeripath
Copy link
Contributor

zeripath commented Nov 6, 2021

OK looking at how openssh responds and the gossh server responds - it looks like the gossh server is missing a SSH2_MSG_EXT_INFO response. This looks a lot like we're either going to have to rip out the x/crypto/ssh stuff or get patches in upstream. 😢 (This is being proposed at golang/go#49269)

@rmbleeker
Copy link

OK looking at how openssh responds and the gossh server responds - it looks like the gossh server is missing a SSH2_MSG_EXT_INFO response. This looks a lot like we're either going to have to rip out the x/crypto/ssh stuff or get patches in upstream. cry (This is being proposed at golang/go#49269)

I assume this response was meant for #17175 since that issue deals with SSH, whereas this issue deals with SSL/TLS?

@zeripath
Copy link
Contributor

zeripath commented Nov 8, 2021

@rmbleeker yeah unforunately we have a cross-issue conversation going on. I'll open another issue for @andypost's reported issue.

If you're interested in configurable SSL options please look at my PR #17440

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants