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

[CORE-2426] tls: Added new functions to support OpenSSL #116

Conversation

michael-redpanda
Copy link

@michael-redpanda michael-redpanda commented May 24, 2024

GnuTLS allows for setting priority and server precedence all through the GnuTLS priority system. OpenSSL on the other hand, has different APIs to handle this.

  • To set server precedence in GnuTLS, simply provide the string %SERVER_PRECEDENCE in the priority string. For OpenSSL, the flag SSL_OP_CIPHER_SERVER_PREFERENCE must be passed to SSL_CTX_set_options

  • GnuTLS's priority string can be used to set the priority for all versions of TLS. For OpenSSL, to set the ciphers available in TLSv1.2 and below, the user must call SSL_CTX_set_cipher_list. For TLSv1.3, the call is SSL_CTX_set_ciphersuites

As the format of the cipher/priority strings are different between OpenSSL and GnuTLS, the user of Seastar would have to change their code in order to change the format to match the requirements of the underlying TLS provider. Therefor, this change adds three new functions to credentials_builder and server_credentials that plumb through these strings/settings to OpenSSL and are no-ops in GnuTLS. Conversely, the set_priority_string function is a no-op when using OpenSSL.

@michael-redpanda michael-redpanda self-assigned this May 24, 2024
@michael-redpanda michael-redpanda requested review from a team, BenPope and graphcareful and removed request for a team May 24, 2024 19:31
@michael-redpanda
Copy link
Author

To see how it's used, here's a commit on redpanda: michael-redpanda/redpanda@adbc4e6

include/seastar/net/tls.hh Outdated Show resolved Hide resolved
}
// This call is required to lower SSL's security level to permit TLSv1.0 and TLSv1.1
// See https://www.openssl.org/docs/man3.0/man3/SSL_CTX_set_security_level.html
SSL_CTX_set_security_level(ssl_ctx.get(), 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be configurable, or is an empty ciphersuite sufficient to disable TLS v1.2 and below?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, leaving either ciphersuites or cipher_string empty just results in a default.

There is another project for us to work on to allow the user to control which TLS versions are enabled. Would probably cover that there. This PR was just to get to feature parity with GnuTLS

include/seastar/net/tls.hh Outdated Show resolved Hide resolved
include/seastar/net/tls.hh Outdated Show resolved Hide resolved
tests/unit/tls_test.cc Outdated Show resolved Hide resolved
tests/unit/tls_test.cc Outdated Show resolved Hide resolved
@michael-redpanda michael-redpanda force-pushed the CORE-2426-Support-OSSL-Cipher-Strings branch from 5da1c16 to af36250 Compare May 28, 2024 13:24
graphcareful
graphcareful previously approved these changes May 28, 2024
Copy link

@graphcareful graphcareful left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@michael-redpanda michael-redpanda force-pushed the CORE-2426-Support-OSSL-Cipher-Strings branch from af36250 to 432b2cc Compare May 28, 2024 18:39
src/net/ossl.cc Show resolved Hide resolved
tests/unit/tls_test.cc Outdated Show resolved Hide resolved
tests/unit/tls_test.cc Outdated Show resolved Hide resolved
GnuTLS allows for setting priority and server precedence all
through the GnuTLS priority system.  OpenSSL on the other hand,
has different APIs to handle this.

- To set server precedence in GnuTLS, simply provide the string
  %SERVER_PRECEDENCE in the priority string.  For OpenSSL, the
  flag SSL_OP_CIPHER_SERVER_PREFERENCE must be passed to
  SSL_CTX_set_options

- GnuTLS's priority string can be used to set the priority for
  all versions of TLS.  For OpenSSL, to set the ciphers available
  in TLSv1.2 and below, the user must call
  SSL_CTX_set_cipher_list.
  For TLSv1.3, the call is SSL_CTX_set_ciphersuites

As the format of the cipher/priority strings are different between
OpenSSL and GnuTLS, the user of Seastar would have to change their
code in order to change the format to match the requirements of the
underlying TLS provider.  Therefor, this change adds three new
functions to credentials_builder and server_credentials that plumb
through these strings/settings to OpenSSL and are no-ops in GnuTLS.
Conversely, the set_priority_string function is a no-op when using
OpenSSL.

Signed-off-by: Michael Boquard <michael@redpanda.com>ggV
@michael-redpanda michael-redpanda force-pushed the CORE-2426-Support-OSSL-Cipher-Strings branch from 432b2cc to 2bc4309 Compare May 29, 2024 12:27
@michael-redpanda
Copy link
Author

Force push 2bc4309:

  • Addressed g++ no return warning
  • Addressed no TLSv1.3 ciphersuite tests

@michael-redpanda michael-redpanda merged commit 2f2f06e into redpanda-data:rebase-feature-ossl May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants