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

tls: Introduce OpenSSL #1

Closed
wants to merge 5 commits into from
Closed

Conversation

michael-redpanda
Copy link
Owner

@michael-redpanda michael-redpanda commented Sep 13, 2024

Introduces OpenSSL as an alternative TLS implementation to GnuTLS. This is a build-time configuration controlled by the CMake variable Seastar_USE_OPENSSL. The configure.py script has been updated to now have a --crypto-provider option. Valid arguments to that are OpenSSL and GnuTLS.

This implementation was released in Redpanda v24.2 on July 31st, and has been running on production clusters since.

Redpanda implemented these changes in order to provide a FIPS-compliant build to customers that require it (such as those wishing to undergo FedRAMP evaluation). OpenSSL was selected as it allows implementors to maintain the validation of the cryptographic module even when it's built from source.

No changes have been introduced to enable the FIPS provider for Seastar. It is up to the implementor to enable and use the FIPS cryptographic module if desired.

Notes for Redpanda developers:

This implementation is nearly identical to the one we are currently running with the following exceptions:

@michael-redpanda michael-redpanda force-pushed the add-openssl-implementation branch 15 times, most recently from 01a3d05 to bda87fc Compare September 20, 2024 18:31
@michael-redpanda michael-redpanda force-pushed the add-openssl-implementation branch 4 times, most recently from 0ad0b66 to 2f16ad8 Compare October 1, 2024 18:13
@oleiman
Copy link

oleiman commented Oct 1, 2024

@michael-redpanda - made a subtask for this https://redpandadata.atlassian.net/browse/CORE-7791

Seems that the ossl backend is not raising a verification_error on the issuer/owner of other.crt, which is some dummy name that definitely doesn't appear in the trust store...not sure why that would be.

@oleiman
Copy link

oleiman commented Oct 1, 2024

Seems to be failing on https://github.com/redpanda-data/seastar/tree/v24.2.x as well

@oleiman
Copy link

oleiman commented Oct 2, 2024

This is from our fork v24.2.x branch:

 $ git bisect bad
6c0ac039bd60f79335ede465cc6a958479e065a6 is the first bad commit
commit 6c0ac039bd60f79335ede465cc6a958479e065a6
Author: Michael Boquard <[email protected]>
Date:   Thu May 30 15:39:52 2024 -0400

    build: Updated configure scripts

    - Updated configure helper scripts to be able to select OpenSSL
    vs GnuTLS.  Updated the SeastarDependencies file to select the
    correct minimum verison of OpenSSL.

    - Any code that exist in tls-impl.cc is meant to be shared across
    implementations, therefore any ones specific GnuTLS must have an
    additional condition to check that we are not building with OpenSSL

    Signed-off-by: Michael Boquard <[email protected]>
    Co-authored-by: Michael Boquard <[email protected]>
    Co-authored-by: Rob Blafford <[email protected]>

 CMakeLists.txt                  | 17 ++++++++++++++---
 cmake/SeastarDependencies.cmake |  3 +++
 configure.py                    |  7 +++++++
 seastar_cmake.py                |  2 ++
 src/CMakeLists.txt              |  7 +++++--
 src/net/tls-impl.cc             |  2 +-
 src/seastar.cc                  |  4 ++++
 7 files changed, 36 insertions(+), 6 deletions(-)

@oleiman
Copy link

oleiman commented Oct 2, 2024

oh, duh, that's just where we turned OpenSSL on I guess? prior to that would the openssl have been totally inert, or is there another way to run the tests w/ openssl backend or what?

@michael-redpanda michael-redpanda force-pushed the add-openssl-implementation branch 5 times, most recently from 939311d to 5638717 Compare December 5, 2024 20:12
@michael-redpanda michael-redpanda self-assigned this Dec 5, 2024
@michael-redpanda michael-redpanda changed the title got gnutls changes building tls: Introduce OpenSSL Dec 6, 2024
@michael-redpanda michael-redpanda force-pushed the add-openssl-implementation branch 3 times, most recently from 31f5632 to 637a9a3 Compare December 6, 2024 18:49
Created tls-impl.cc and tls-impl.h which contains common structures and
definitions that are not dependent on the underlying TLS mechanism.

These changes set the stage for implementing other TLS providers.

Signed-off-by: Michael Boquard <[email protected]>
This commit adds support for using OpenSSL, instead of GnuTLS, as the
TLS provider within Seastar.  To support this change, the configure
script has been updated to allow users to select which cryptographic
provider should be used by supply `--crypto-provider` and specificying
either `OpenSSL` or `GnuTLS`.

The OpenSSL implementation mirrors the GnuTLS implementation.  Instead
of using callbacks, a custom BIO was created to handle moving data
on/off of the OpenSSL SSL session into the Seastar TLS session data
sinks.

When compiled for OpenSSL, the
`certificate_credentials::set_priority_string` method is compiled out and
replaced with the following:

* `set_cipher_string`
* `set_ciphersuites`
* `enable_server_precedence`
* `set_minimum_tls_version`
* `set_maximum_tls_version`

These methods are specific to OpenSSL.

The github actions have been updated to run the full suite of tests
against both cryptographic providers.

`src/net/tcp.hh` and `src/websocket/server.cc` have been updated to use
OpenSSL instead of GnuTLS, depending upon the build configuration.

Signed-off-by: Michael Boquard <[email protected]>
A class that wraps around a typical logger that appends useful
information about the connection when logging.

Added use to the OpenSSL implementation.

Signed-off-by: Michael Boquard <[email protected]>
More recent versions of OpenSSL requrire CA certificates to have CA:true

Signed-off-by: Michael Boquard <[email protected]>
Now handling situations where the get() call doesn't throw but does
return an empty buffer indicating EOF.

Signed-off-by: Michael Boquard <[email protected]>
@michael-redpanda michael-redpanda force-pushed the add-openssl-implementation branch from 637a9a3 to 5c709c2 Compare December 6, 2024 19:02
@michael-redpanda
Copy link
Owner Author

Started upstream PR: scylladb#2569

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.

2 participants