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.

Sign up for GitHub

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

refactor: remove unused prf hmac impls #5148

Merged
merged 3 commits into from
Feb 28, 2025
Merged

refactor: remove unused prf hmac impls #5148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
30d383c
refactor: remove unused prf hmac impls
lrstewart Feb 26, 2025
20a6819
Also clean up header
lrstewart Feb 27, 2025
7f24d45
also clean up memory
lrstewart Feb 28, 2025
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
8 changes: 0 additions & 8 deletions crypto/s2n_evp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,6 @@ struct s2n_evp_digest {
EVP_MD_CTX *ctx;
};

struct s2n_evp_hmac_state {
struct s2n_evp_digest evp_digest;
union {
HMAC_CTX *hmac_ctx;
EVP_PKEY *evp_pkey;
} ctx;
};

/* Define API's that change based on the OpenSSL Major Version. */
#if S2N_OPENSSL_VERSION_AT_LEAST(1, 1, 0) && !defined(LIBRESSL_VERSION_NUMBER)
#define S2N_EVP_MD_CTX_NEW() (EVP_MD_CTX_new())
Expand Down
10 changes: 0 additions & 10 deletions tests/cbmc/include/cbmc_proof/make_common_datastructures.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,16 +197,6 @@ void cbmc_populate_s2n_evp_digest(struct s2n_evp_digest *evp_digest);
*/
struct s2n_evp_digest* cbmc_allocate_s2n_evp_digest();

/*
* Populates the fields of a pre-allocated s2n_evp_digest for CBMC proofs.
*/
void cbmc_populate_s2n_evp_hmac_state(struct s2n_evp_hmac_state *evp_hmac_state);

/*
* Properly allocates s2n_evp_hmac_state for CBMC proofs.
*/
struct s2n_evp_hmac_state *cbmc_allocate_s2n_evp_hmac_state();

/*
* Populates the fields of a pre-allocated s2n_hash_state for CBMC proofs.
*/
Expand Down
19 changes: 0 additions & 19 deletions tests/cbmc/sources/make_common_datastructures.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,24 +244,6 @@ struct s2n_evp_digest* cbmc_allocate_s2n_evp_digest()
return evp_digest;
}

void cbmc_populate_s2n_evp_hmac_state(struct s2n_evp_hmac_state *evp_hmac_state)
{
CBMC_ENSURE_REF(evp_hmac_state);
cbmc_populate_s2n_evp_digest(&(evp_hmac_state->evp_digest));
if (s2n_libcrypto_is_awslc() || s2n_libcrypto_is_boringssl()) {
evp_hmac_state->ctx.hmac_ctx = malloc(sizeof(*(evp_hmac_state->ctx.hmac_ctx)));
} else {
evp_hmac_state->ctx.evp_pkey = malloc(sizeof(*(evp_hmac_state->ctx.evp_pkey)));
}
}

struct s2n_evp_hmac_state *cbmc_allocate_s2n_evp_hmac_state()
{
struct s2n_evp_hmac_state *evp_hmac_state = malloc(sizeof(*evp_hmac_state));
cbmc_populate_s2n_evp_hmac_state(evp_hmac_state);
return evp_hmac_state;
}

void cbmc_populate_s2n_hash_state(struct s2n_hash_state* state)
{
CBMC_ENSURE_REF(state);
Expand Down Expand Up @@ -716,7 +698,6 @@ void cbmc_populate_s2n_prf_working_space(struct s2n_prf_working_space *s2n_prf_w
* If required, this initialization should be done in the validation function.
*/
cbmc_populate_s2n_hmac_state(&(s2n_prf_working_space->p_hash.s2n_hmac));
cbmc_populate_s2n_evp_hmac_state(&(s2n_prf_working_space->p_hash.evp_hmac));
}

struct s2n_prf_working_space* cbmc_allocate_s2n_prf_working_space()
Expand Down
192 changes: 19 additions & 173 deletions tls/s2n_prf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@
#define S2N_LIBCRYPTO_SUPPORTS_TLS_PRF 0
#endif

/* The s2n p_hash implementation is abstracted to allow for separate implementations, using
* either s2n's formally verified HMAC or OpenSSL's EVP HMAC, for use by the TLS PRF. */
/* The s2n p_hash implementation is abstracted to allow for separate implementations.
* Currently the only implementation uses s2n-tls's custom HMAC implementation.
*/
struct s2n_p_hash_hmac {
int (*alloc)(struct s2n_prf_working_space *ws);
int (*init)(struct s2n_prf_working_space *ws, s2n_hmac_algorithm alg, struct s2n_blob *secret);
Expand Down Expand Up @@ -194,175 +195,9 @@ static int s2n_prf_sslv3(struct s2n_connection *conn, struct s2n_blob *secret, s
return 0;
}

#if !defined(OPENSSL_IS_BORINGSSL) && !defined(OPENSSL_IS_AWSLC)
static int s2n_evp_pkey_p_hash_alloc(struct s2n_prf_working_space *ws)
{
POSIX_ENSURE_REF(ws->p_hash.evp_hmac.evp_digest.ctx = S2N_EVP_MD_CTX_NEW());
return 0;
}

static int s2n_evp_pkey_p_hash_digest_init(struct s2n_prf_working_space *ws)
{
POSIX_ENSURE_REF(ws->p_hash.evp_hmac.evp_digest.md);
POSIX_ENSURE_REF(ws->p_hash.evp_hmac.evp_digest.ctx);
POSIX_ENSURE_REF(ws->p_hash.evp_hmac.ctx.evp_pkey);

POSIX_GUARD_OSSL(EVP_DigestSignInit(ws->p_hash.evp_hmac.evp_digest.ctx, NULL, ws->p_hash.evp_hmac.evp_digest.md, NULL, ws->p_hash.evp_hmac.ctx.evp_pkey),
S2N_ERR_P_HASH_INIT_FAILED);

return 0;
}

static int s2n_evp_pkey_p_hash_init(struct s2n_prf_working_space *ws, s2n_hmac_algorithm alg, struct s2n_blob *secret)
{
/* Initialize the message digest */
POSIX_GUARD_RESULT(s2n_hmac_md_from_alg(alg, &ws->p_hash.evp_hmac.evp_digest.md));

/* Initialize the mac key using the provided secret */
POSIX_ENSURE_REF(ws->p_hash.evp_hmac.ctx.evp_pkey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, secret->data, secret->size));

/* Initialize the message digest context with the above message digest and mac key */
return s2n_evp_pkey_p_hash_digest_init(ws);
}

static int s2n_evp_pkey_p_hash_update(struct s2n_prf_working_space *ws, const void *data, uint32_t size)
{
POSIX_GUARD_OSSL(EVP_DigestSignUpdate(ws->p_hash.evp_hmac.evp_digest.ctx, data, (size_t) size), S2N_ERR_P_HASH_UPDATE_FAILED);

return 0;
}

static int s2n_evp_pkey_p_hash_final(struct s2n_prf_working_space *ws, void *digest, uint32_t size)
{
/* EVP_DigestSign API's require size_t data structures */
size_t digest_size = size;

POSIX_GUARD_OSSL(EVP_DigestSignFinal(ws->p_hash.evp_hmac.evp_digest.ctx, (unsigned char *) digest, &digest_size), S2N_ERR_P_HASH_FINAL_FAILED);

return 0;
}

static int s2n_evp_pkey_p_hash_wipe(struct s2n_prf_working_space *ws)
{
POSIX_GUARD_OSSL(S2N_EVP_MD_CTX_RESET(ws->p_hash.evp_hmac.evp_digest.ctx), S2N_ERR_P_HASH_WIPE_FAILED);

return 0;
}

static int s2n_evp_pkey_p_hash_reset(struct s2n_prf_working_space *ws)
{
POSIX_GUARD(s2n_evp_pkey_p_hash_wipe(ws));

/*
* On some cleanup paths s2n_evp_pkey_p_hash_reset can be called before s2n_evp_pkey_p_hash_init so there is nothing
* to reset.
*/
if (ws->p_hash.evp_hmac.ctx.evp_pkey == NULL) {
return S2N_SUCCESS;
}
return s2n_evp_pkey_p_hash_digest_init(ws);
}

static int s2n_evp_pkey_p_hash_cleanup(struct s2n_prf_working_space *ws)
{
/* Prepare the workspace md_ctx for the next p_hash */
POSIX_GUARD(s2n_evp_pkey_p_hash_wipe(ws));

/* Free mac key - PKEYs cannot be reused */
POSIX_ENSURE_REF(ws->p_hash.evp_hmac.ctx.evp_pkey);
EVP_PKEY_free(ws->p_hash.evp_hmac.ctx.evp_pkey);
ws->p_hash.evp_hmac.ctx.evp_pkey = NULL;

return 0;
}

static int s2n_evp_pkey_p_hash_free(struct s2n_prf_working_space *ws)
{
POSIX_ENSURE_REF(ws->p_hash.evp_hmac.evp_digest.ctx);
S2N_EVP_MD_CTX_FREE(ws->p_hash.evp_hmac.evp_digest.ctx);
ws->p_hash.evp_hmac.evp_digest.ctx = NULL;

return 0;
}

static const struct s2n_p_hash_hmac s2n_evp_pkey_p_hash_hmac = {
.alloc = &s2n_evp_pkey_p_hash_alloc,
.init = &s2n_evp_pkey_p_hash_init,
.update = &s2n_evp_pkey_p_hash_update,
.final = &s2n_evp_pkey_p_hash_final,
.reset = &s2n_evp_pkey_p_hash_reset,
.cleanup = &s2n_evp_pkey_p_hash_cleanup,
.free = &s2n_evp_pkey_p_hash_free,
};
#else
static int s2n_evp_hmac_p_hash_alloc(struct s2n_prf_working_space *ws)
{
POSIX_ENSURE_REF(ws->p_hash.evp_hmac.ctx.hmac_ctx = HMAC_CTX_new());
return S2N_SUCCESS;
}

static int s2n_evp_hmac_p_hash_init(struct s2n_prf_working_space *ws, s2n_hmac_algorithm alg, struct s2n_blob *secret)
{
/* Figure out the correct EVP_MD from s2n_hmac_algorithm */
POSIX_GUARD_RESULT(s2n_hmac_md_from_alg(alg, &ws->p_hash.evp_hmac.evp_digest.md));

/* Initialize the mac and digest */
POSIX_GUARD_OSSL(HMAC_Init_ex(ws->p_hash.evp_hmac.ctx.hmac_ctx, secret->data, secret->size, ws->p_hash.evp_hmac.evp_digest.md, NULL), S2N_ERR_P_HASH_INIT_FAILED);
return S2N_SUCCESS;
}

static int s2n_evp_hmac_p_hash_update(struct s2n_prf_working_space *ws, const void *data, uint32_t size)
{
POSIX_GUARD_OSSL(HMAC_Update(ws->p_hash.evp_hmac.ctx.hmac_ctx, data, (size_t) size), S2N_ERR_P_HASH_UPDATE_FAILED);
return S2N_SUCCESS;
}

static int s2n_evp_hmac_p_hash_final(struct s2n_prf_working_space *ws, void *digest, uint32_t size)
{
/* HMAC_Final API's require size_t data structures */
unsigned int digest_size = size;
POSIX_GUARD_OSSL(HMAC_Final(ws->p_hash.evp_hmac.ctx.hmac_ctx, (unsigned char *) digest, &digest_size), S2N_ERR_P_HASH_FINAL_FAILED);
return S2N_SUCCESS;
}

static int s2n_evp_hmac_p_hash_reset(struct s2n_prf_working_space *ws)
{
POSIX_ENSURE_REF(ws);
if (ws->p_hash.evp_hmac.evp_digest.md == NULL) {
return S2N_SUCCESS;
}
POSIX_GUARD_OSSL(HMAC_Init_ex(ws->p_hash.evp_hmac.ctx.hmac_ctx, NULL, 0, ws->p_hash.evp_hmac.evp_digest.md, NULL), S2N_ERR_P_HASH_INIT_FAILED);
return S2N_SUCCESS;
}

static int s2n_evp_hmac_p_hash_cleanup(struct s2n_prf_working_space *ws)
{
/* Prepare the workspace md_ctx for the next p_hash */
HMAC_CTX_reset(ws->p_hash.evp_hmac.ctx.hmac_ctx);
return S2N_SUCCESS;
}

static int s2n_evp_hmac_p_hash_free(struct s2n_prf_working_space *ws)
{
HMAC_CTX_free(ws->p_hash.evp_hmac.ctx.hmac_ctx);
return S2N_SUCCESS;
}

static const struct s2n_p_hash_hmac s2n_evp_hmac_p_hash_hmac = {
.alloc = &s2n_evp_hmac_p_hash_alloc,
.init = &s2n_evp_hmac_p_hash_init,
.update = &s2n_evp_hmac_p_hash_update,
.final = &s2n_evp_hmac_p_hash_final,
.reset = &s2n_evp_hmac_p_hash_reset,
.cleanup = &s2n_evp_hmac_p_hash_cleanup,
.free = &s2n_evp_hmac_p_hash_free,
};
#endif /* !defined(OPENSSL_IS_BORINGSSL) && !defined(OPENSSL_IS_AWSLC) */

static int s2n_hmac_p_hash_new(struct s2n_prf_working_space *ws)
{
POSIX_GUARD(s2n_hmac_new(&ws->p_hash.s2n_hmac));

return s2n_hmac_init(&ws->p_hash.s2n_hmac, S2N_HMAC_NONE, NULL, 0);
}

Expand Down Expand Up @@ -412,13 +247,20 @@ static const struct s2n_p_hash_hmac s2n_internal_p_hash_hmac = {
.free = &s2n_hmac_p_hash_free,
};

/*
* For now, use the internal s2n-tls hmac abstraction.
* However, that is a custom implementation of hmac built on hashes.
* Ideally we should stop using our custom implementation here and switch
* to using a libcrypto implementation. Unfortunately, what each libcrypto
* can support varies a lot for HMACs.
*
* For historical reference, there used to be two other hmac implementations:
* https://github.com/aws/s2n-tls/blob/711ee0df658cd7c44088cf7a1b20a9f3cf5296d6/tls/s2n_prf.c#L174-L337
* Both implementations have compatibility issues with one or more libcryptos.
*/
const struct s2n_p_hash_hmac *s2n_get_hmac_implementation()
{
#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC)
return s2n_is_in_fips_mode() ? &s2n_evp_hmac_p_hash_hmac : &s2n_internal_p_hash_hmac;
#else
return s2n_is_in_fips_mode() ? &s2n_evp_pkey_p_hash_hmac : &s2n_internal_p_hash_hmac;
#endif
return &s2n_internal_p_hash_hmac;
}

static int s2n_p_hash(struct s2n_prf_working_space *ws, s2n_hmac_algorithm alg, struct s2n_blob *secret, struct s2n_blob *label,
Expand All @@ -428,6 +270,7 @@ static int s2n_p_hash(struct s2n_prf_working_space *ws, s2n_hmac_algorithm alg,
POSIX_GUARD(s2n_hmac_digest_size(alg, &digest_size));

const struct s2n_p_hash_hmac *hmac = s2n_get_hmac_implementation();
POSIX_ENSURE_REF(hmac);

/* First compute hmac(secret + A(0)) */
POSIX_GUARD(hmac->init(ws, alg, secret));
Expand Down Expand Up @@ -494,6 +337,7 @@ S2N_RESULT s2n_prf_new(struct s2n_connection *conn)

/* Allocate the hmac state */
const struct s2n_p_hash_hmac *hmac_impl = s2n_get_hmac_implementation();
RESULT_ENSURE_REF(hmac_impl);
RESULT_GUARD_POSIX(hmac_impl->alloc(conn->prf_space));
return S2N_RESULT_OK;
}
Expand All @@ -504,6 +348,7 @@ S2N_RESULT s2n_prf_wipe(struct s2n_connection *conn)
RESULT_ENSURE_REF(conn->prf_space);

const struct s2n_p_hash_hmac *hmac_impl = s2n_get_hmac_implementation();
RESULT_ENSURE_REF(hmac_impl);
RESULT_GUARD_POSIX(hmac_impl->reset(conn->prf_space));

return S2N_RESULT_OK;
Expand All @@ -517,6 +362,7 @@ S2N_RESULT s2n_prf_free(struct s2n_connection *conn)
}

const struct s2n_p_hash_hmac *hmac_impl = s2n_get_hmac_implementation();
RESULT_ENSURE_REF(hmac_impl);
RESULT_GUARD_POSIX(hmac_impl->free(conn->prf_space));

RESULT_GUARD_POSIX(s2n_free_object((uint8_t **) &conn->prf_space, sizeof(struct s2n_prf_working_space)));
Expand Down
1 change: 0 additions & 1 deletion tls/s2n_prf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

union p_hash_state {
struct s2n_hmac_state s2n_hmac;
struct s2n_evp_hmac_state evp_hmac;
};

struct s2n_prf_working_space {
Expand Down
Loading