Skip to content

Commit 6e17971

Browse files
jschmidbjschmidb
committed
Fixes for protecting buffers containing sensitive data
For OpenSSL BIGNUMs containing sensitive private key material, use BN_secure_new() to allocate the BIGNUM on OpenSSLs secure heap. Also clean any buffers that might contain sensitive data before freeing them. Signed-off-by: Joerg Schmidbauer <jschmidb@de.ibm.com>
1 parent 435d2ed commit 6e17971

4 files changed

Lines changed: 112 additions & 64 deletions

File tree

‎src/ica_api.c‎

Lines changed: 60 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,62 +1182,76 @@ unsigned int ica_rsa_mod_expo(ica_adapter_handle_t adapter_handle,
11821182
unsigned int ica_rsa_crt_key_check(ica_rsa_key_crt_t *rsa_key)
11831183
{
11841184
int pq_comp;
1185-
int keyfmt = 1;
11861185
BIGNUM *bn_p;
11871186
BIGNUM *bn_q;
11881187
BIGNUM *bn_invq;
11891188
BN_CTX *ctx;
11901189
unsigned char *tmp_buf = NULL;
1190+
unsigned int rc;
11911191

11921192
#ifdef ICA_FIPS
11931193
if (fips >> 1)
11941194
return EACCES;
11951195
#endif /* ICA_FIPS */
11961196

11971197
/* check if p > q */
1198-
pq_comp = memcmp( (rsa_key->p + 8), (rsa_key->q), rsa_key->key_length/2);
1199-
if (pq_comp < 0) /* unprivileged key format */
1200-
keyfmt = 0;
1201-
1202-
if (!keyfmt) {
1203-
/* swap p and q */
1204-
tmp_buf = calloc(1, rsa_key->key_length/2);
1205-
if (!tmp_buf)
1206-
return ENOMEM;
1207-
memcpy(tmp_buf, rsa_key->p + 8, rsa_key->key_length/2);
1208-
memcpy(rsa_key->p + 8, rsa_key->q, rsa_key->key_length/2);
1209-
memcpy(rsa_key->q, tmp_buf, rsa_key->key_length/2);
1210-
1211-
/* swap dp and dq */
1212-
memcpy(tmp_buf, rsa_key->dp + 8, rsa_key->key_length/2);
1213-
memcpy(rsa_key->dp + 8, rsa_key->dq, rsa_key->key_length/2);
1214-
memcpy(rsa_key->dq, tmp_buf, rsa_key->key_length/2);
1215-
1216-
/* calculate new qInv */
1217-
bn_p = BN_new();
1218-
bn_q = BN_new();
1219-
bn_invq = BN_new();
1220-
ctx = BN_CTX_new();
1221-
1222-
BN_bin2bn(rsa_key->p, rsa_key->key_length/2+8, bn_p);
1223-
BN_bin2bn(rsa_key->q, rsa_key->key_length/2, bn_q);
1224-
1225-
/* qInv = (1/q) mod p */
1226-
BN_mod_inverse(bn_invq, bn_q, bn_p, ctx);
1227-
memset(tmp_buf, 0, rsa_key->key_length/2);
1228-
BN_bn2binpad(bn_invq, tmp_buf, rsa_key->key_length/2);
1229-
memcpy(rsa_key->qInverse + 8, tmp_buf, rsa_key->key_length/2);
1230-
1231-
free(tmp_buf);
1232-
1233-
BN_CTX_free(ctx);
1234-
BN_clear_free(bn_p);
1235-
BN_clear_free(bn_q);
1236-
BN_clear_free(bn_invq);
1237-
1238-
return 1;
1198+
pq_comp = memcmp( (rsa_key->p + 8), (rsa_key->q), rsa_key->key_length / 2);
1199+
if (pq_comp >= 0) /* privileged key format, p and q ok */
1200+
return 0;
1201+
1202+
/* unprivileged key format: swap p and q */
1203+
tmp_buf = calloc(1, rsa_key->key_length / 2);
1204+
if (!tmp_buf)
1205+
return ENOMEM;
1206+
1207+
bn_p = BN_secure_new();
1208+
bn_q = BN_secure_new();
1209+
bn_invq = BN_secure_new();
1210+
ctx = BN_CTX_new();
1211+
if (!bn_p || !bn_q || !bn_invq || !ctx) {
1212+
rc = ENOMEM;
1213+
goto done;
12391214
}
1240-
return 0;
1215+
1216+
/* swap p and q */
1217+
memcpy(tmp_buf, rsa_key->p + 8, rsa_key->key_length / 2);
1218+
memcpy(rsa_key->p + 8, rsa_key->q, rsa_key->key_length / 2);
1219+
memcpy(rsa_key->q, tmp_buf, rsa_key->key_length / 2);
1220+
1221+
/* swap dp and dq */
1222+
memcpy(tmp_buf, rsa_key->dp + 8, rsa_key->key_length / 2);
1223+
memcpy(rsa_key->dp + 8, rsa_key->dq, rsa_key->key_length / 2);
1224+
memcpy(rsa_key->dq, tmp_buf, rsa_key->key_length / 2);
1225+
1226+
if (BN_bin2bn(rsa_key->p, rsa_key->key_length / 2 + 8, bn_p) == NULL ||
1227+
BN_bin2bn(rsa_key->q, rsa_key->key_length / 2, bn_q) == NULL) {
1228+
rc = EFAULT;
1229+
goto done;
1230+
}
1231+
1232+
/* qInv = (1/q) mod p */
1233+
if (BN_mod_inverse(bn_invq, bn_q, bn_p, ctx) == NULL) {
1234+
rc = EFAULT;
1235+
goto done;
1236+
}
1237+
memset(tmp_buf, 0, rsa_key->key_length / 2);
1238+
if (BN_bn2binpad(bn_invq, tmp_buf, rsa_key->key_length / 2) <= 0) {
1239+
rc = EFAULT;
1240+
goto done;
1241+
}
1242+
memcpy(rsa_key->qInverse + 8, tmp_buf, rsa_key->key_length / 2);
1243+
1244+
rc = 1;
1245+
1246+
done:
1247+
OPENSSL_cleanse(tmp_buf, rsa_key->key_length / 2);
1248+
free(tmp_buf);
1249+
BN_CTX_free(ctx);
1250+
BN_clear_free(bn_p);
1251+
BN_clear_free(bn_q);
1252+
BN_clear_free(bn_invq);
1253+
1254+
return rc;
12411255
}
12421256

12431257
unsigned int ica_rsa_crt(ica_adapter_handle_t adapter_handle,
@@ -1273,7 +1287,9 @@ unsigned int ica_rsa_crt(ica_adapter_handle_t adapter_handle,
12731287
rb.outputdata = output_data;
12741288
rb.outputdatalength = rsa_key->key_length;
12751289

1276-
ica_rsa_crt_key_check(rsa_key);
1290+
rc = ica_rsa_crt_key_check(rsa_key);
1291+
if (rc > 1)
1292+
return rc;
12771293

12781294
rb.np_prime = rsa_key->p;
12791295
rb.nq_prime = rsa_key->q;

‎src/s390_ecc.c‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,11 @@ static EVP_PKEY *make_pkey(int nid, const unsigned char *p, size_t plen)
169169
goto err;
170170
}
171171

172-
bn_priv = BN_bin2bn(p, plen, NULL);
173-
if (bn_priv == NULL) {
172+
bn_priv = BN_secure_new();
173+
if (bn_priv == NULL)
174+
goto err;
175+
if (BN_bin2bn(p, plen, bn_priv) == NULL)
174176
goto err;
175-
}
176177

177178
if (!EC_POINT_mul(group, point, bn_priv, NULL, NULL, NULL)) {
178179
goto err;
@@ -235,7 +236,7 @@ static EVP_PKEY *make_pkey(int nid, const unsigned char *p, size_t plen)
235236
err:
236237
EC_POINT_free(point);
237238
EC_GROUP_free(group);
238-
BN_free(bn_priv);
239+
BN_clear_free(bn_priv);
239240

240241
#if !OPENSSL_VERSION_PREREQ(3, 0)
241242
// because we use EVP_PKEY_set1_EC_KEY above, free the ec_key here.
@@ -1268,7 +1269,11 @@ static unsigned int provide_pubkey(const ICA_EC_KEY *privkey, unsigned char *X,
12681269
}
12691270

12701271
/* Get (D) as BIGNUM */
1271-
if ((bn_d = BN_bin2bn(privkey->D, privlen, NULL)) == NULL) {
1272+
bn_d = BN_secure_new();
1273+
if (bn_d == NULL)
1274+
return ENOMEM;
1275+
if (BN_bin2bn(privkey->D, privlen, bn_d) == NULL) {
1276+
BN_clear_free(bn_d);
12721277
return EFAULT;
12731278
}
12741279

@@ -1716,7 +1721,7 @@ struct { \
17161721
#undef DEF_PARAM
17171722

17181723
unsigned long fc;
1719-
size_t off, counter = 0;
1724+
size_t off;
17201725
int rc;
17211726

17221727
memset(&param, 0, sizeof(param));
@@ -1743,6 +1748,7 @@ struct { \
17431748
if (k == NULL) {
17441749
#ifdef ICA_FIPS
17451750
if (fips & ICA_FIPS_MODE) {
1751+
size_t counter = 0;
17461752
fc |= 0x80; /* deterministic signature */
17471753
do {
17481754
/* If the random number is not invertible, s390_kdsa will
@@ -1802,6 +1808,7 @@ struct { \
18021808
if (k == NULL) {
18031809
#ifdef ICA_FIPS
18041810
if (fips & ICA_FIPS_MODE) {
1811+
size_t counter = 0;
18051812
fc |= 0x80; /* deterministic signature */
18061813
do {
18071814
if (RAND_bytes(param.P384.rand + off, sizeof(param.P384.rand) - off) != 1) {
@@ -1855,6 +1862,7 @@ struct { \
18551862
if (k == NULL) {
18561863
#ifdef ICA_FIPS
18571864
if (fips & ICA_FIPS_MODE) {
1865+
size_t counter = 0;
18581866
fc |= 0x80; /* deterministic signature */
18591867
do {
18601868
if (RAND_bytes(param.P521.rand + off, sizeof(param.P521.rand) - off) != 1) {
@@ -2268,7 +2276,7 @@ static int eckeygen_cpacf(ICA_EC_KEY *key)
22682276
int rv, numbytes;
22692277

22702278
ctx = BN_CTX_new();
2271-
priv = BN_new();
2279+
priv = BN_secure_new();
22722280
ord = BN_new();
22732281

22742282
if (ctx == NULL || priv == NULL || ord == NULL) {
@@ -2321,7 +2329,7 @@ static int eckeygen_cpacf(ICA_EC_KEY *key)
23212329
if (ctx != NULL)
23222330
BN_CTX_free(ctx);
23232331
if (priv != NULL)
2324-
BN_free(priv);
2332+
BN_clear_free(priv);
23252333
if (ord != NULL)
23262334
BN_free(ord);
23272335

@@ -2541,15 +2549,21 @@ unsigned int eckeygen_sw(ICA_EC_KEY *key)
25412549
EVP_PKEY *icakey2pkey(const ICA_EC_KEY *icakey, int *is_public_key)
25422550
{
25432551
EVP_PKEY *pkey = NULL;
2544-
BIGNUM *bn_D, *bn_X, *bn_Y;
2552+
BIGNUM *bn_D, *bn_X = NULL, *bn_Y = NULL;
25452553

25462554
if (icakey == NULL)
25472555
return NULL;
25482556

25492557
/* Check if any key parts available */
2550-
bn_D = BN_bin2bn(icakey->D, privlen_from_nid(icakey->nid), NULL);
2558+
bn_D = BN_secure_new();
2559+
if (bn_D == NULL)
2560+
return NULL;
2561+
if (BN_bin2bn(icakey->D, privlen_from_nid(icakey->nid), bn_D) == NULL)
2562+
goto done;
25512563
bn_X = BN_bin2bn(icakey->X, privlen_from_nid(icakey->nid), NULL);
25522564
bn_Y = BN_bin2bn(icakey->Y, privlen_from_nid(icakey->nid), NULL);
2565+
if (!bn_X || !bn_Y)
2566+
goto done;
25532567
if (BN_is_zero(bn_D) && BN_is_zero(bn_X) && BN_is_zero(bn_Y))
25542568
goto done;
25552569

@@ -2567,7 +2581,7 @@ EVP_PKEY *icakey2pkey(const ICA_EC_KEY *icakey, int *is_public_key)
25672581
done:
25682582
BN_free(bn_X);
25692583
BN_free(bn_Y);
2570-
BN_free(bn_D);
2584+
BN_clear_free(bn_D);
25712585
return pkey;
25722586
}
25732587

‎src/s390_rsa.c‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,13 @@ unsigned int rsa_key_generate_mod_expo(ica_adapter_handle_t deviceHandle,
232232
goto err;
233233
}
234234

235+
n = BN_secure_new();
236+
d = BN_secure_new();
237+
if (!n || !d) {
238+
rc = ENOMEM;
239+
goto err;
240+
}
241+
235242
if (!EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_N, &n) ||
236243
!EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_D, &d)) {
237244
rc = EFAULT;
@@ -270,8 +277,8 @@ unsigned int rsa_key_generate_mod_expo(ica_adapter_handle_t deviceHandle,
270277
#if !OPENSSL_VERSION_PREREQ(3, 0)
271278
RSA_free(rsa);
272279
#else
273-
BN_free(n);
274-
BN_free(d);
280+
BN_clear_free(n);
281+
BN_clear_free(d);
275282
EVP_PKEY_free(pkey);
276283
#endif
277284

@@ -337,6 +344,17 @@ unsigned int rsa_key_generate_crt(ica_adapter_handle_t deviceHandle,
337344
goto err;
338345
}
339346

347+
n = BN_secure_new();
348+
p = BN_secure_new();
349+
q = BN_secure_new();
350+
dmp1 = BN_secure_new();
351+
dmq1 = BN_secure_new();
352+
iqmp = BN_secure_new();
353+
if (!n || !p || !q || !dmp1 || !dmq1 || !iqmp) {
354+
rc = ENOMEM;
355+
goto err;
356+
}
357+
340358
if (!EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_N, &n) ||
341359
!EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_FACTOR1, &p) ||
342360
!EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_FACTOR2, &q) ||
@@ -425,12 +443,12 @@ unsigned int rsa_key_generate_crt(ica_adapter_handle_t deviceHandle,
425443
#if !OPENSSL_VERSION_PREREQ(3, 0)
426444
RSA_free(rsa);
427445
#else
428-
BN_free(n);
429-
BN_free(p);
430-
BN_free(q);
431-
BN_free(dmp1);
432-
BN_free(dmq1);
433-
BN_free(iqmp);
446+
BN_clear_free(n);
447+
BN_clear_free(p);
448+
BN_clear_free(q);
449+
BN_clear_free(dmp1);
450+
BN_clear_free(dmq1);
451+
BN_clear_free(iqmp);
434452
EVP_PKEY_free(pkey);
435453
#endif
436454

‎test/rsa_key_check_test.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ int main(int argc, char **argv)
4141

4242
gettimeofday(&start, NULL);
4343
rc = ica_rsa_crt_key_check(&crt_key);
44-
if(rc){
44+
if (rc > 1) {
4545
V_(printf("ica_rsa_crt_key_check failed!\n"));
4646
}
4747

0 commit comments

Comments
 (0)