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

No default cipher list for openssl client sockets #13686

Closed

Conversation

carlhoerberg
Copy link
Contributor

It should be up to the OS to set the default ciphers, eg. if a cipher
suddenly is deemed insecure or there's a reason to have another cipher
order, such as ChaCha over any AES cipher because of lack of hardware
support.

Currently Crystal will expose up-to-date OS to security considerations
because of hard coded cipher lists, that might not be so up to date,
unless the application is recompiled with a modern Crystal version.

@carlhoerberg
Copy link
Contributor Author

This is the recommended behavior by RedHat and Fedora: https://docs.fedoraproject.org/en-US/packaging-guidelines/CryptoPolicies/

And rpmlint complaints about all crystal apps that we try to package because it calls SSL_CTX_set_cipher_list by default.

@carlhoerberg carlhoerberg force-pushed the no-default-ciphers branch 2 times, most recently from 678719f to 015ec87 Compare July 22, 2023 23:13
@carlhoerberg carlhoerberg changed the title No default ciphers No default cipher list for openssl client sockets Jul 22, 2023
It should be up to the OS to set the default ciphers, eg. if a cipher
suddenly is deemed insecure or there's a reason to have another cipher
order, such as ChaCha over any AES cipher because of lack of hardware
support.

Currently Crystal will expose up-to-date OS to security considirations
because of hard coded cipher lists, that might not be so up to date,
unless the application is recompiled with a modern Crystal version.
@carlhoerberg
Copy link
Contributor Author

Restored the server behavior, this now only affects client connections.

@bararchy
Copy link
Contributor

@carlhoerberg This isn't a clear case.
Saying to "trust the OS" is very optimistic, some of those systems will not be updated, or will be static-compiled with some OpenSSL version that won't be updated.

Even go has their own default TLS\SSL cipher suite https://go.dev/src/crypto/tls/cipher_suites.go

The best practice is to offer an ever-updated list of default ciphers which are at least based on the Intermediate list, while preferring the modern list if available by the underlying SSL\TLS lib. (https://ssl-config.mozilla.org/)

Also, the OS itself will be using some library, like Linux will use OpenSSL\GNUtls\bearssl\etc.. etc.. and Windows will get updates to Windows SChannel.

I'm not sure saying "remove default ciphers" is the way to go 🤷

@carlhoerberg
Copy link
Contributor Author

Then the list should be queried and updated at run time, not at compile time of the compiler. Applications might run for years, recommended ciphers updated much more often. You might save some outdated OSes, but risks everybody else who run up to date OSes. And running TLS on not up to date OSes is always a major security risk, think heartbleed.

@carlhoerberg
Copy link
Contributor Author

The so called "best practice" is for servers, not clients, and as you say, up-to-date list, applications compiled with Crystal 1.9.2 already have an outdated list (5.6, 5.7 is current).

Go have to have a list because they dont link to openssl or gnutls. But also requires all Go applications to be recomplie once a security issue in their TLS implementation is discovered or a ciphers is broken. Crystal doesnt have that issue. And allows secure OSes to limit the ciphers further.

@bararchy
Copy link
Contributor

@carlhoerberg I actually don't really know what's the best approach here, but I guess for the client side to give the decision to the OS is fine.

@HertzDevil
Copy link
Contributor

Two things I'm wondering:

  • After this PR, if you package any server software you'd still get the same warning, right?
  • It doesn't look like SSL_CTX_set_ciphersuites is checked. On OpenSSL 1.1.0 and above #set_old_ciphers should call that function instead of SSL_CTX_set_cipher_list. Are those two functions interchangeable?

@carlhoerberg
Copy link
Contributor Author

carlhoerberg commented Aug 1, 2023

Two things I'm wondering:

* After this PR, if you package any server software you'd still get the same warning, right?

correct, but it's a step in the right direction at least.

* It doesn't look like `SSL_CTX_set_ciphersuites` is checked. On OpenSSL 1.1.0 and above `#set_old_ciphers` should call that function instead of `SSL_CTX_set_cipher_list`. Are those two functions interchangeable?

They are not interchangeable, SSL_CTX_set_cipher_list controls TLS 1.3 ciphers (which crystal currently doesn't override), SSL_CTX_set_ciphersuites controls TLS v1.0-1.2 ciphers.

@beta-ziliani
Copy link
Member

An idea suggested by @ysbaddaden was to use the OS default but filtering out known bad ciphers

@HertzDevil
Copy link
Contributor

I looked at the rpmlint code responsible for generating the warnings. In short, if the binary refers to the SSL_CTX_set_cipher_list symbol, and running strings on the binary doesn't yield the PROFILE=SYSTEM string, then a warning is emitted.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants