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

Disable RC4 when built with Openssl-3.0 #3982

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 1 addition & 39 deletions crypto/s2n_libcrypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,13 @@

#include <openssl/crypto.h>
#include <openssl/opensslv.h>
#include <string.h>

#include "crypto/s2n_crypto.h"
#include "crypto/s2n_fips.h"
#include "crypto/s2n_openssl.h"
#include "utils/s2n_safety.h"
#include "utils/s2n_safety_macros.h"
#if S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0)
#include <openssl/provider.h>
#endif

#include <string.h>

/* Note: OpenSSL 1.0.2 -> 1.1.0 implemented a new API to get the version number
* and version name. We have to handle that by using old functions
Expand Down Expand Up @@ -149,40 +145,6 @@ bool s2n_libcrypto_is_libressl()
#endif
}

S2N_RESULT s2n_libcrypto_init(void)
{
#if S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0)
RESULT_ENSURE(OSSL_PROVIDER_load(NULL, "default") != NULL, S2N_ERR_OSSL_PROVIDER);
#ifdef S2N_LIBCRYPTO_SUPPORTS_EVP_RC4
/* needed to support RC4 algorithm
* https://www.openssl.org/docs/man3.0/man7/OSSL_PROVIDER-legacy.html
*/
RESULT_ENSURE(OSSL_PROVIDER_load(NULL, "legacy") != NULL, S2N_ERR_OSSL_PROVIDER);
#endif
#endif

return S2N_RESULT_OK;
}

#if S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0)
int s2n_libcrypto_cleanup_cb(OSSL_PROVIDER *provider, void *cbdata)
{
return OSSL_PROVIDER_unload(provider);
}

S2N_RESULT s2n_libcrypto_cleanup(void)
{
RESULT_GUARD_OSSL(OSSL_PROVIDER_do_all(NULL, *s2n_libcrypto_cleanup_cb, NULL), S2N_ERR_ATEXIT);

return S2N_RESULT_OK;
}
#else
S2N_RESULT s2n_libcrypto_cleanup(void)
{
return S2N_RESULT_OK;
}
#endif

/* Performs various checks to validate that the libcrypto used at compile-time
* is the same libcrypto being used at run-time.
*/
Expand Down
2 changes: 0 additions & 2 deletions crypto/s2n_libcrypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,4 @@

#include "utils/s2n_result.h"

S2N_RESULT s2n_libcrypto_init(void);
S2N_RESULT s2n_libcrypto_validate_runtime(void);
S2N_RESULT s2n_libcrypto_cleanup(void);
61 changes: 19 additions & 42 deletions crypto/s2n_stream_cipher_rc4.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,30 @@
#include "utils/s2n_blob.h"
#include "utils/s2n_safety.h"

static uint8_t s2n_stream_cipher_rc4_available()
static const EVP_CIPHER *s2n_evp_rc4()
Copy link
Contributor

Choose a reason for hiding this comment

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

It's hard to tell from the docs but does this EVP_CIPHER need to be free'd with EVP_CIPHER_free? Or is it a global?

{
#ifdef S2N_LIBCRYPTO_SUPPORTS_EVP_RC4
return EVP_rc4();
Comment on lines 26 to +27
Copy link
Contributor

Choose a reason for hiding this comment

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

should this also check for Openssl-3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. It's not testing for support, just limiting the blast radius of the #if we need to check for the existence of the EVP_rc4() symbol.

#else
return NULL;
#endif
}

static uint8_t s2n_stream_cipher_rc4_available()
{
if (s2n_is_in_fips_mode()) {
return 0;
} else {
return (EVP_rc4() ? 1 : 0);
}
#else
return 0;
#endif /* S2N_LIBCRYPTO_SUPPORTS_EVP_RC4 */
/* RC4 MIGHT be available in Openssl-3.0, depending on whether or not the
* "legacy" provider is loaded. However, for simplicity, assume that RC4
* is unavailable.
*/
if (S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0)) {
return 0;
}
return (s2n_evp_rc4() ? 1 : 0);
lrstewart marked this conversation as resolved.
Show resolved Hide resolved
}

#ifdef S2N_LIBCRYPTO_SUPPORTS_EVP_RC4
static int s2n_stream_cipher_rc4_encrypt(struct s2n_session_key *key, struct s2n_blob *in, struct s2n_blob *out)
{
POSIX_ENSURE_GTE(out->size, in->size);
Expand Down Expand Up @@ -64,15 +74,15 @@ static int s2n_stream_cipher_rc4_decrypt(struct s2n_session_key *key, struct s2n
static int s2n_stream_cipher_rc4_set_encryption_key(struct s2n_session_key *key, struct s2n_blob *in)
{
POSIX_ENSURE_EQ(in->size, 16);
POSIX_GUARD_OSSL(EVP_EncryptInit_ex(key->evp_cipher_ctx, EVP_rc4(), NULL, in->data, NULL), S2N_ERR_KEY_INIT);
POSIX_GUARD_OSSL(EVP_EncryptInit_ex(key->evp_cipher_ctx, s2n_evp_rc4(), NULL, in->data, NULL), S2N_ERR_KEY_INIT);

return 0;
}

static int s2n_stream_cipher_rc4_set_decryption_key(struct s2n_session_key *key, struct s2n_blob *in)
{
POSIX_ENSURE_EQ(in->size, 16);
POSIX_GUARD_OSSL(EVP_DecryptInit_ex(key->evp_cipher_ctx, EVP_rc4(), NULL, in->data, NULL), S2N_ERR_KEY_INIT);
POSIX_GUARD_OSSL(EVP_DecryptInit_ex(key->evp_cipher_ctx, s2n_evp_rc4(), NULL, in->data, NULL), S2N_ERR_KEY_INIT);

return 0;
}
Expand All @@ -90,39 +100,6 @@ static int s2n_stream_cipher_rc4_destroy_key(struct s2n_session_key *key)

return 0;
}
#else

static int s2n_stream_cipher_rc4_encrypt(struct s2n_session_key *key, struct s2n_blob *in, struct s2n_blob *out)
{
POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
}

static int s2n_stream_cipher_rc4_decrypt(struct s2n_session_key *key, struct s2n_blob *in, struct s2n_blob *out)
{
POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
}

static int s2n_stream_cipher_rc4_set_encryption_key(struct s2n_session_key *key, struct s2n_blob *in)
{
POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
}

static int s2n_stream_cipher_rc4_set_decryption_key(struct s2n_session_key *key, struct s2n_blob *in)
{
POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
}

static int s2n_stream_cipher_rc4_init(struct s2n_session_key *key)
{
POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
}

static int s2n_stream_cipher_rc4_destroy_key(struct s2n_session_key *key)
{
POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
}

#endif /* S2N_LIBCRYPTO_SUPPORTS_EVP_RC4 */

const struct s2n_cipher s2n_rc4 = {
.type = S2N_STREAM,
Expand Down
6 changes: 6 additions & 0 deletions docs/USAGE-GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,12 @@ cd ../../ # root of project
make
```

### Available algorithms
Not all algorithms are available from all versions of Openssl:
* ChaChaPoly is not supported before Openssl-1.1.1
* RSA-PSS is not supported before Openssl-1.1.1
* RC4 is not supported with Openssl-3.0 or later.

## Building s2n-tls with LibreSSL

To build s2n-tls with LibreSSL, do the following:
Expand Down
21 changes: 15 additions & 6 deletions tests/unit/s2n_rc4_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "crypto/s2n_cipher.h"
#include "crypto/s2n_fips.h"
#include "crypto/s2n_hmac.h"
#include "crypto/s2n_openssl.h"
#include "s2n_test.h"
#include "stuffer/s2n_stuffer.h"
#include "testlib/s2n_testlib.h"
Expand All @@ -32,6 +33,16 @@ int main(int argc, char **argv)
{
BEGIN_TEST();

/* Test Openssl-3.0 does not support RC4 */
if (S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0)) {
EXPECT_FALSE(s2n_rc4.is_available());
}

/* Test FIPS does not support RC4 */
if (s2n_is_in_fips_mode()) {
EXPECT_FALSE(s2n_rc4.is_available());
}

struct s2n_connection *conn;
uint8_t mac_key[] = "sample mac key";
uint8_t rc4_key[] = "123456789012345";
Expand All @@ -57,9 +68,9 @@ int main(int argc, char **argv)

/* test the RC4 cipher with a SHA1 hash */
conn->secure->cipher_suite->record_alg = &s2n_record_alg_rc4_sha;
EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->init(&conn->secure->server_key));
EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->init(&conn->secure->client_key));
if (conn->secure->cipher_suite->record_alg->cipher->is_available()) {
EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->init(&conn->secure->server_key));
EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->init(&conn->secure->client_key));
EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->set_decryption_key(&conn->secure->client_key, &key_iv));
EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->set_encryption_key(&conn->secure->server_key, &key_iv));
EXPECT_SUCCESS(s2n_hmac_init(&conn->secure->client_record_mac, S2N_HMAC_SHA1, mac_key, sizeof(mac_key)));
Expand Down Expand Up @@ -119,10 +130,8 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->destroy_key(&conn->secure->client_key));
EXPECT_SUCCESS(s2n_connection_free(conn));
} else {
EXPECT_FAILURE_WITH_ERRNO(conn->secure->cipher_suite->record_alg->cipher->init(&conn->secure->server_key), S2N_ERR_UNIMPLEMENTED);
EXPECT_FAILURE_WITH_ERRNO(conn->secure->cipher_suite->record_alg->cipher->init(&conn->secure->client_key), S2N_ERR_UNIMPLEMENTED);
EXPECT_FAILURE_WITH_ERRNO(conn->secure->cipher_suite->record_alg->cipher->set_decryption_key(&conn->secure->client_key, &key_iv), S2N_ERR_UNIMPLEMENTED);
EXPECT_FAILURE_WITH_ERRNO(conn->secure->cipher_suite->record_alg->cipher->set_encryption_key(&conn->secure->server_key, &key_iv), S2N_ERR_UNIMPLEMENTED);
EXPECT_FAILURE(conn->secure->cipher_suite->record_alg->cipher->set_decryption_key(&conn->secure->client_key, &key_iv));
EXPECT_FAILURE(conn->secure->cipher_suite->record_alg->cipher->set_encryption_key(&conn->secure->server_key, &key_iv));
}

END_TEST();
Expand Down
2 changes: 0 additions & 2 deletions utils/s2n_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ int s2n_init(void)
POSIX_GUARD(s2n_mem_init());
/* Must run before any init method that calls libcrypto methods. */
POSIX_GUARD_RESULT(s2n_locking_init());
POSIX_GUARD_RESULT(s2n_libcrypto_init());
POSIX_GUARD(s2n_fips_init());
POSIX_GUARD_RESULT(s2n_rand_init());
POSIX_GUARD(s2n_cipher_suites_init());
Expand Down Expand Up @@ -100,7 +99,6 @@ static bool s2n_cleanup_atexit_impl(void)
bool cleaned_up = s2n_result_is_ok(s2n_cipher_suites_cleanup())
&& s2n_result_is_ok(s2n_rand_cleanup_thread())
&& s2n_result_is_ok(s2n_rand_cleanup())
&& s2n_result_is_ok(s2n_libcrypto_cleanup())
&& s2n_result_is_ok(s2n_locking_cleanup())
&& (s2n_mem_cleanup() == S2N_SUCCESS);

Expand Down