The last couple of commits refactored some of the RSA mechanisms in common/mech_rsa.c, mainly around PKCS#1 v1.5 padding handling. Those also introduced a couple of issues that are being resolved by this patch:
* Segfault when rsa_format_block() was called with block type '2', caused by un-initialized 'i' * Possible presence of null-padding bytes generated by rng_generate() rsa_format_block(), which would cause an invalid padding. Both this and the above item were fixed by generating the random padding bytes one-by-one, and replacing 0x00 by 0xff when needed. * RSA Verify and VerifyRecover were calling rsa_format_block() with block type '2', when the PKCS#1 specified block type '1'. * rsa_parse_block() may return 'CKR_ENCRYPTED_DATA_INVALID' when it verifies that the signature is invalid. PKCS#1 specifies that the caller shouldn't be able to distinguish between padding errors and invalid signatures. Fixed by adjusting some return codes as well as the calling functions. RSA Verify and VerifyRecover should return 'CKR_SIGNATURE_INVALID' * Remove the '195 - RSA Parse block failed' log message as it may indicate failure in decoding PKCS#1 v.1.5 padding, thus breaking the spec. With the above, all tests in testcases/drivers/rsa_func.c are now passing. Signed-off-by: Klaus Heinrich Kiwi <[email protected]> --- usr/lib/pkcs11/common/log.c | 4 +-- usr/lib/pkcs11/common/mech_rsa.c | 50 +++++++++++++++++++++++++------------- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/usr/lib/pkcs11/common/log.c b/usr/lib/pkcs11/common/log.c index fd28e66..bce00b6 100755 --- a/usr/lib/pkcs11/common/log.c +++ b/usr/lib/pkcs11/common/log.c @@ -673,8 +673,6 @@ struct messages err_msg[]={ {"AES Wrap Get Data Failed"}, {"AES Wrap Format Failed"}, {"Domain Parameter Invalid"}, - {"File \"%s\" could not be opened, errno=%d"}, - //195 - {"RSA Parse Block Failed"} + {"File \"%s\" could not be opened, errno=%d"} }; diff --git a/usr/lib/pkcs11/common/mech_rsa.c b/usr/lib/pkcs11/common/mech_rsa.c index b0b0f34..076317c 100755 --- a/usr/lib/pkcs11/common/mech_rsa.c +++ b/usr/lib/pkcs11/common/mech_rsa.c @@ -417,13 +417,19 @@ rsa_format_block( CK_BYTE * in_data, * Where ?? is nonzero. */ case 2: - rc = rng_generate(&out_data[2], padding_len); - if (rc != CKR_OK){ - st_err_log(130, __FILE__, __LINE__); - return rc; + for (i = 2; i < (padding_len + 2); i++) { + rc = rng_generate(&out_data[i], 1); + if (rc != CKR_OK) { + st_err_log(130, __FILE__, __LINE__); + return rc; + } + if (out_data[i] == (CK_BYTE)0) { + /* avoid zeros by explicitly making them all 0xff - + * won't hurt entropy that bad, and it's better than + * looping over rng_generate */ + out_data[i] = (CK_BYTE)0xff; + } } - i = i + padding_len; - break; default: @@ -564,14 +570,14 @@ rsa_parse_block( CK_BYTE * in_data, * encryption blocks. */ if ((type == 1 || type == 2) && ((i - 3) < 8)) { - st_err_log(112, __FILE__, __LINE__); - rc = CKR_ENCRYPTED_DATA_LEN_RANGE; + st_err_log(14, __FILE__, __LINE__); + rc = CKR_ENCRYPTED_DATA_INVALID; return rc; } if (in_data_len <= i) { - st_err_log(112, __FILE__, __LINE__); - rc = CKR_ENCRYPTED_DATA_LEN_RANGE; + st_err_log(14, __FILE__, __LINE__); + rc = CKR_ENCRYPTED_DATA_INVALID; return rc; } @@ -711,7 +717,7 @@ rsa_pkcs_decrypt( SESSION *sess, rc = rsa_parse_block(out, modulus_bytes, out_data, out_data_len, PKCS_BT_2); if (rc != CKR_OK) { - st_err_log(195, __FILE__, __LINE__); + st_err_log(133, __FILE__, __LINE__); return rc; } @@ -834,7 +840,7 @@ rsa_pkcs_verify( SESSION * sess, if (rc == CKR_OK) { CK_ULONG len; - rc = rsa_parse_block( out, modulus_bytes, out_data, &out_data_len, PKCS_BT_2); + rc = rsa_parse_block( out, modulus_bytes, out_data, &out_data_len, PKCS_BT_1); if (rc == CKR_OK) { if (in_data_len != out_data_len){ st_err_log(47, __FILE__, __LINE__); @@ -846,9 +852,13 @@ rsa_pkcs_verify( SESSION * sess, return CKR_SIGNATURE_INVALID; } } - else - st_err_log(195, __FILE__, __LINE__); - + else if (rc == CKR_ENCRYPTED_DATA_INVALID ) { + st_err_log(47, __FILE__, __LINE__); + return CKR_SIGNATURE_INVALID; + } else { + st_err_log(4, __FILE__, __LINE__, __FUNCTION__); + return CKR_FUNCTION_FAILED; + } } else st_err_log(132, __FILE__, __LINE__); @@ -908,8 +918,14 @@ rsa_pkcs_verify_recover( SESSION * sess, // rc = ckm_rsa_encrypt( signature, modulus_bytes, out, key_obj ); if (rc == CKR_OK) { - rc = rsa_parse_block(out, modulus_bytes, out_data, out_data_len, PKCS_BT_2); - return rc; + rc = rsa_parse_block(out, modulus_bytes, out_data, out_data_len, PKCS_BT_1); + if (rc == CKR_ENCRYPTED_DATA_INVALID ) { + st_err_log(47, __FILE__, __LINE__); + return CKR_SIGNATURE_INVALID; + } else if (rc != CKR_OK) { + st_err_log(4, __FILE__, __LINE__, __FUNCTION__); + return rc; + } } else st_err_log(132, __FILE__, __LINE__); -- 1.6.6.1 ------------------------------------------------------------------------------ ThinkGeek and WIRED's GeekDad team up for the Ultimate GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the lucky parental unit. See the prize list and enter to win: http://p.sf.net/sfu/thinkgeek-promo _______________________________________________ Opencryptoki-tech mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opencryptoki-tech
