Skip to content

Commit

Permalink
pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
goatgoose committed May 26, 2023
1 parent 446f817 commit 2fbecdb
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 30 deletions.
10 changes: 10 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,16 @@ if(PLATFORM_SUPPORTS_KTLS)
target_compile_options(${PROJECT_NAME} PUBLIC -DS2N_PLATFORM_SUPPORTS_KTLS)
endif()

try_compile(
LIBCRYPTO_SUPPORTS_TLS_PRF
${CMAKE_BINARY_DIR}
SOURCES "${CMAKE_CURRENT_LIST_DIR}/tests/features/tls_prf.c"
LINK_LIBRARIES ${LINK_LIB} ${OS_LIBS}
)
if(LIBCRYPTO_SUPPORTS_TLS_PRF)
target_compile_options(${PROJECT_NAME} PUBLIC -DS2N_LIBCRYPTO_SUPPORTS_TLS_PRF)
endif()

if (NOT DEFINED CMAKE_AR)
message(STATUS "CMAKE_AR undefined, setting to `ar` by default")
SET(CMAKE_AR ar)
Expand Down
6 changes: 6 additions & 0 deletions s2n.mk
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,12 @@ ifeq ($(TRY_COMPILE_KTLS), 0)
DEFAULT_CFLAGS += -DS2N_PLATFORM_SUPPORTS_KTLS
endif

# Determine if libcrypto PRF implementation is available
TRY_COMPILE_TLS_PRF := $(call try_compile,$(S2N_ROOT)/tests/features/tls_prf.c)
ifeq ($(TRY_COMPILE_TLS_PRF), 0)
DEFAULT_CFLAGS += -DS2N_LIBCRYPTO_SUPPORTS_TLS_PRF
endif

CFLAGS_LLVM = ${DEFAULT_CFLAGS} -emit-llvm -c -g -O1

$(BITCODE_DIR)%.bc: %.c
Expand Down
33 changes: 33 additions & 0 deletions tests/features/tls_prf.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@

#include <openssl/base.h>
#include <openssl/digest.h>

#define TEST_BUFFER_SIZE 64

/* BoringSSL and AWSLC define the CRYPTO_tls1_prf API in a private header. This function is
* forward-declared to make it accessible.
*/
int CRYPTO_tls1_prf(const EVP_MD *digest,
uint8_t *out, size_t out_len,
const uint8_t *secret, size_t secret_len,
const char *label, size_t label_len,
const uint8_t *seed1, size_t seed1_len,
const uint8_t *seed2, size_t seed2_len);

int main()
{
const EVP_MD *digest = EVP_md5_sha1();

uint8_t out[TEST_BUFFER_SIZE] = { 0 };
uint8_t secret[TEST_BUFFER_SIZE] = { 0 };
const char label[TEST_BUFFER_SIZE] = { 0 };
uint8_t seed1[TEST_BUFFER_SIZE] = { 0 };
uint8_t seed2[TEST_BUFFER_SIZE] = { 0 };

if (!CRYPTO_tls1_prf(digest, out, TEST_BUFFER_SIZE, secret, TEST_BUFFER_SIZE, label,
TEST_BUFFER_SIZE, seed1, TEST_BUFFER_SIZE, seed2, TEST_BUFFER_SIZE)) {
return 1;
}

return 0;
}
18 changes: 12 additions & 6 deletions tests/unit/s2n_tls_prf_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -325,19 +325,25 @@ int main(int argc, char **argv)
{
DEFER_CLEANUP(struct s2n_connection *connection = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free);

EXPECT_FAILURE_WITH_ERRNO(s2n_prf(NULL, &secret, &label, &seed_a, &seed_b, &seed_c, &out), S2N_ERR_NULL);
EXPECT_FAILURE_WITH_ERRNO(s2n_prf(connection, NULL, &label, &seed_a, &seed_b, &seed_c, &out), S2N_ERR_NULL);
EXPECT_FAILURE_WITH_ERRNO(s2n_prf(connection, &secret, NULL, &seed_a, &seed_b, &seed_c, &out), S2N_ERR_NULL);
EXPECT_FAILURE_WITH_ERRNO(s2n_prf(connection, &secret, &label, &seed_a, &seed_b, &seed_c, NULL), S2N_ERR_NULL);
EXPECT_FAILURE_WITH_ERRNO(s2n_prf(NULL, &secret, &label, &seed_a, &seed_b, &seed_c, &out),
S2N_ERR_NULL);
EXPECT_FAILURE_WITH_ERRNO(s2n_prf(connection, NULL, &label, &seed_a, &seed_b, &seed_c, &out),
S2N_ERR_NULL);
EXPECT_FAILURE_WITH_ERRNO(s2n_prf(connection, &secret, NULL, &seed_a, &seed_b, &seed_c, &out),
S2N_ERR_NULL);
EXPECT_FAILURE_WITH_ERRNO(s2n_prf(connection, &secret, &label, &seed_a, &seed_b, &seed_c, NULL),
S2N_ERR_NULL);

/* seed_a is required */
EXPECT_FAILURE_WITH_ERRNO(s2n_prf(connection, &secret, &label, NULL, &seed_b, &seed_c, &out), S2N_ERR_PRF_INVALID_SEED);
EXPECT_FAILURE_WITH_ERRNO(s2n_prf(connection, &secret, &label, NULL, &seed_b, &seed_c, &out),
S2N_ERR_PRF_INVALID_SEED);

/* seed_b and seed_c are optional */
EXPECT_SUCCESS(s2n_prf(connection, &secret, &label, &seed_a, NULL, NULL, &out));

/* seed_b is required if seed_c is provided */
EXPECT_FAILURE_WITH_ERRNO(s2n_prf(connection, &secret, &label, &seed_a, NULL, &seed_c, &out), S2N_ERR_PRF_INVALID_SEED);
EXPECT_FAILURE_WITH_ERRNO(s2n_prf(connection, &secret, &label, &seed_a, NULL, &seed_c, &out),
S2N_ERR_PRF_INVALID_SEED);

/* seed_c is optional */
EXPECT_SUCCESS(s2n_prf(connection, &secret, &label, &seed_a, &seed_b, NULL, &out));
Expand Down
47 changes: 29 additions & 18 deletions tls/s2n_prf.c
Original file line number Diff line number Diff line change
Expand Up @@ -463,11 +463,15 @@ S2N_RESULT s2n_prf_free(struct s2n_connection *conn)

static bool s2n_libcrypto_supports_tls_prf()
{
return S2N_LIBCRYPTO_SUPPORTS_TLS_PRF;
#ifdef S2N_LIBCRYPTO_SUPPORTS_TLS_PRF
return true;
#else
return false;
#endif
}

static S2N_RESULT s2n_custom_prf(struct s2n_connection *conn, struct s2n_blob *secret, struct s2n_blob *label, struct s2n_blob *seed_a,
struct s2n_blob *seed_b, struct s2n_blob *seed_c, struct s2n_blob *out)
static S2N_RESULT s2n_custom_prf(struct s2n_connection *conn, struct s2n_blob *secret, struct s2n_blob *label,
struct s2n_blob *seed_a, struct s2n_blob *seed_b, struct s2n_blob *seed_c, struct s2n_blob *out)
{
/* We zero the out blob because p_hash works by XOR'ing with the existing
* buffer. This is a little convoluted but means we can avoid dynamic memory
Expand All @@ -478,7 +482,8 @@ static S2N_RESULT s2n_custom_prf(struct s2n_connection *conn, struct s2n_blob *s
RESULT_GUARD_POSIX(s2n_blob_zero(out));

if (conn->actual_protocol_version == S2N_TLS12) {
RESULT_GUARD_POSIX(s2n_p_hash(conn->prf_space, conn->secure->cipher_suite->prf_alg, secret, label, seed_a, seed_b, seed_c, out));
RESULT_GUARD_POSIX(s2n_p_hash(conn->prf_space, conn->secure->cipher_suite->prf_alg, secret, label, seed_a,
seed_b, seed_c, out));
return S2N_RESULT_OK;
}

Expand All @@ -492,7 +497,7 @@ static S2N_RESULT s2n_custom_prf(struct s2n_connection *conn, struct s2n_blob *s
return S2N_RESULT_OK;
}

#if S2N_LIBCRYPTO_SUPPORTS_TLS_PRF
#ifdef S2N_LIBCRYPTO_SUPPORTS_TLS_PRF
/* BoringSSL and AWSLC define the CRYPTO_tls1_prf API in a private header. This function is
* forward-declared to make it accessible.
*/
Expand All @@ -503,17 +508,19 @@ int CRYPTO_tls1_prf(const EVP_MD *digest,
const uint8_t *seed1, size_t seed1_len,
const uint8_t *seed2, size_t seed2_len);

static S2N_RESULT s2n_libcrypto_prf(struct s2n_connection *conn, struct s2n_blob *secret, struct s2n_blob *label, struct s2n_blob *seed_a,
struct s2n_blob *seed_b, struct s2n_blob *seed_c, struct s2n_blob *out)
static S2N_RESULT s2n_libcrypto_prf(struct s2n_connection *conn, struct s2n_blob *secret, struct s2n_blob *label,
struct s2n_blob *seed_a, struct s2n_blob *seed_b, struct s2n_blob *seed_c, struct s2n_blob *out)
{
/* md5_sha1 is a digest that indicates both MD5 and SHA1 should be used in the PRF calculation.
* This is needed for pre-TLS12 PRFs.
*/
const EVP_MD *digest = EVP_md5_sha1();

if (conn->actual_protocol_version == S2N_TLS12) {
const EVP_MD *digest = NULL;
if (conn->actual_protocol_version < S2N_TLS12) {
/* md5_sha1 is a digest that indicates both MD5 and SHA1 should be used in the PRF calculation.
* This is needed for pre-TLS12 PRFs.
*/
digest = EVP_md5_sha1();
} else {
RESULT_GUARD(s2n_md_from_hmac_alg(conn->secure->cipher_suite->prf_alg, &digest));
}
RESULT_ENSURE_REF(digest);

DEFER_CLEANUP(struct s2n_stuffer seed_b_stuffer = { 0 }, s2n_stuffer_free);
size_t seed_b_len = 0;
Expand All @@ -528,7 +535,7 @@ static S2N_RESULT s2n_libcrypto_prf(struct s2n_connection *conn, struct s2n_blob
/* The AWSLC/BoringSSL TLS PRF implementation only provides two seed arguments. If three seeds
* were provided, pass in the third seed by concatenating it with the second seed.
*/
RESULT_GUARD_POSIX(s2n_stuffer_growable_alloc(&seed_b_stuffer, seed_b->size + seed_c->size));
RESULT_GUARD_POSIX(s2n_stuffer_alloc(&seed_b_stuffer, seed_b->size + seed_c->size));
RESULT_GUARD_POSIX(s2n_stuffer_write_bytes(&seed_b_stuffer, seed_b->data, seed_b->size));
RESULT_GUARD_POSIX(s2n_stuffer_write_bytes(&seed_b_stuffer, seed_c->data, seed_c->size));
}
Expand All @@ -538,15 +545,19 @@ static S2N_RESULT s2n_libcrypto_prf(struct s2n_connection *conn, struct s2n_blob
RESULT_ENSURE_REF(seed_b_data);
}

RESULT_GUARD_OSSL(CRYPTO_tls1_prf(digest, out->data, out->size, secret->data, secret->size, (const char *) label->data,
label->size, seed_a->data, seed_a->size, seed_b_data, seed_b_len),
RESULT_GUARD_OSSL(CRYPTO_tls1_prf(digest,
out->data, out->size,
secret->data, secret->size,
(const char *) label->data, label->size,
seed_a->data, seed_a->size,
seed_b_data, seed_b_len),
S2N_ERR_PRF_DERIVE);

return S2N_RESULT_OK;
}
#else
static S2N_RESULT s2n_libcrypto_prf(struct s2n_connection *conn, struct s2n_blob *secret, struct s2n_blob *label, struct s2n_blob *seed_a,
struct s2n_blob *seed_b, struct s2n_blob *seed_c, struct s2n_blob *out)
static S2N_RESULT s2n_libcrypto_prf(struct s2n_connection *conn, struct s2n_blob *secret, struct s2n_blob *label,
struct s2n_blob *seed_a, struct s2n_blob *seed_b, struct s2n_blob *seed_c, struct s2n_blob *out)
{
RESULT_BAIL(S2N_ERR_UNIMPLEMENTED);
}
Expand Down
6 changes: 0 additions & 6 deletions tls/s2n_prf.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@
/* Enough to support TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384, 2*SHA384_DIGEST_LEN + 2*AES256_KEY_SIZE */
#define S2N_MAX_KEY_BLOCK_LEN 160

#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC)
#define S2N_LIBCRYPTO_SUPPORTS_TLS_PRF 1
#else
#define S2N_LIBCRYPTO_SUPPORTS_TLS_PRF 0
#endif

union p_hash_state {
struct s2n_hmac_state s2n_hmac;
struct s2n_evp_hmac_state evp_hmac;
Expand Down

0 comments on commit 2fbecdb

Please sign in to comment.