Skip to content

Commit 3f7f9a5

Browse files
prasdenjustsmth
andauthored
Add OPENSSL_cleanse to zero stack secrets before return (#3227)
### Issues: Addresses: P409219367 ### Description of changes: Several functions store sensitive key material in stack buffers but do not zero them before returning. This pull request adds `OPENSSL_cleanse` calls to ensure these buffers are zeroed before function exits or returns in HPKE key derivation, ECDH shared secret computation, EC key derivation, PEM decoding, and PBKDF2 iteration to avoid residual secrets in memory. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license. Co-authored-by: Justin W Smith <103147162+justsmth@users.noreply.github.com>
1 parent 443d7f7 commit 3f7f9a5

5 files changed

Lines changed: 24 additions & 9 deletions

File tree

crypto/ecdh_extra/ecdh_extra.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,18 @@ int ECDH_compute_key(void *out, size_t out_len, const EC_POINT *pub_key,
2727
const EC_KEY *priv_key,
2828
void *(*kdf)(const void *in, size_t inlen, void *out,
2929
size_t *out_len)) {
30-
30+
int ret = -1;
3131
uint8_t buf[EC_MAX_BYTES];
3232
size_t buf_len = sizeof(buf);
3333

3434
if (!ECDH_compute_shared_secret(buf, &buf_len, pub_key, priv_key)) {
35-
return -1;
35+
goto end;
3636
}
3737

3838
if (kdf != NULL) {
3939
if (kdf(buf, buf_len, out, &out_len) == NULL) {
4040
OPENSSL_PUT_ERROR(ECDH, ECDH_R_KDF_FAILED);
41-
return -1;
41+
goto end;
4242
}
4343
} else {
4444
// no KDF, just copy as much as we can
@@ -50,8 +50,11 @@ int ECDH_compute_key(void *out, size_t out_len, const EC_POINT *pub_key,
5050

5151
if (out_len > INT_MAX) {
5252
OPENSSL_PUT_ERROR(ECDH, ERR_R_OVERFLOW);
53-
return -1;
53+
goto end;
5454
}
5555

56-
return (int)out_len;
56+
ret = (int)out_len;
57+
end:
58+
OPENSSL_cleanse(buf, sizeof(buf));
59+
return ret;
5760
}

crypto/fipsmodule/evp/p_ec.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ static int pkey_ec_verify(EVP_PKEY_CTX *ctx, const uint8_t *sig, size_t siglen,
8888

8989
static int pkey_ec_derive(EVP_PKEY_CTX *ctx, uint8_t *key,
9090
size_t *keylen) {
91+
int ret = 0;
9192
const EC_POINT *pubkey = NULL;
9293
EC_KEY *eckey;
9394
uint8_t buf[EC_MAX_BYTES];
@@ -115,7 +116,7 @@ static int pkey_ec_derive(EVP_PKEY_CTX *ctx, uint8_t *key,
115116
// Note: This is an internal function which will not update
116117
// the service indicator.
117118
if (!ECDH_compute_shared_secret(buf, &buflen, pubkey, eckey)) {
118-
return 0;
119+
goto end;
119120
}
120121

121122
if (buflen < *keylen) {
@@ -127,7 +128,11 @@ static int pkey_ec_derive(EVP_PKEY_CTX *ctx, uint8_t *key,
127128
// referenced from the higher level function |EVP_PKEY_derive|. |EC_KEY| is
128129
// is the only possible key that can do derivations.
129130
ECDH_verify_service_indicator(eckey);
130-
return 1;
131+
ret = 1;
132+
133+
end:
134+
OPENSSL_cleanse(buf, sizeof(buf));
135+
return ret;
131136
}
132137

133138
static int pkey_ec_ctrl(EVP_PKEY_CTX *ctx, int type, int p1, void *p2) {

crypto/fipsmodule/pbkdf/pbkdf.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <string.h>
88

99
#include <openssl/hmac.h>
10+
#include <openssl/mem.h>
1011

1112
#include "../../internal.h"
1213
#include "../service_indicator/internal.h"
@@ -21,6 +22,7 @@ int PKCS5_PBKDF2_HMAC(const char *password, size_t password_len,
2122
uint32_t i = 1;
2223
HMAC_CTX hctx;
2324
HMAC_CTX_init(&hctx);
25+
uint8_t digest_tmp[EVP_MAX_MD_SIZE];
2426

2527
// We have to avoid the underlying SHA services updating the indicator
2628
// state, so we lock the state here.
@@ -43,7 +45,6 @@ int PKCS5_PBKDF2_HMAC(const char *password, size_t password_len,
4345
i_buf[3] = (uint8_t)(i & 0xff);
4446

4547
// Compute U_1.
46-
uint8_t digest_tmp[EVP_MAX_MD_SIZE];
4748
if (!HMAC_Init_ex(&hctx, NULL, 0, NULL, NULL) ||
4849
!HMAC_Update(&hctx, salt, salt_len) ||
4950
!HMAC_Update(&hctx, i_buf, 4) ||
@@ -87,6 +88,7 @@ int PKCS5_PBKDF2_HMAC(const char *password, size_t password_len,
8788
ret = 1;
8889

8990
err:
91+
OPENSSL_cleanse(digest_tmp, sizeof(digest_tmp));
9092
FIPS_service_indicator_unlock_state();
9193
HMAC_CTX_cleanup(&hctx);
9294
if (ret) {

crypto/hpke/hpke.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <openssl/err.h>
1414
#include <openssl/evp_errors.h>
1515
#include <openssl/hkdf.h>
16+
#include <openssl/mem.h>
1617
#include <openssl/rand.h>
1718
#include <openssl/sha.h>
1819

@@ -126,11 +127,13 @@ static int dhkem_extract_and_expand(uint16_t kem_id, const EVP_MD *hkdf_md,
126127
uint8_t suite_id[5] = {'K', 'E', 'M', kem_id >> 8, kem_id & 0xff};
127128
uint8_t prk[EVP_MAX_MD_SIZE];
128129
size_t prk_len;
129-
return hpke_labeled_extract(hkdf_md, prk, &prk_len, NULL, 0, suite_id,
130+
int ret = hpke_labeled_extract(hkdf_md, prk, &prk_len, NULL, 0, suite_id,
130131
sizeof(suite_id), "eae_prk", dh, dh_len) &&
131132
hpke_labeled_expand(hkdf_md, out_key, out_len, prk, prk_len, suite_id,
132133
sizeof(suite_id), "shared_secret", kem_context,
133134
kem_context_len);
135+
OPENSSL_cleanse(prk, sizeof(prk));
136+
return ret;
134137
}
135138

136139
static int x25519_init_key(EVP_HPKE_KEY *key, const uint8_t *priv_key,

crypto/pem/pem_lib.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -722,11 +722,13 @@ int PEM_read_bio(BIO *bp, char **name, char **header, unsigned char **data,
722722
*header = headerB->data;
723723
*data = (unsigned char *)dataB->data;
724724
*len = bl;
725+
OPENSSL_cleanse(buf, sizeof(buf));
725726
OPENSSL_free(nameB);
726727
OPENSSL_free(headerB);
727728
OPENSSL_free(dataB);
728729
return 1;
729730
err:
731+
OPENSSL_cleanse(buf, sizeof(buf));
730732
BUF_MEM_free(nameB);
731733
BUF_MEM_free(headerB);
732734
BUF_MEM_free(dataB);

0 commit comments

Comments
 (0)