-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi Klaus,
On 06/17/2010 04:31 PM, Klaus Heinrich Kiwi wrote: > On Wed, 16 Jun 2010 11:34:52 -0300 > Ramon de Carvalho Valle <[email protected]> wrote: > >> Add rsa_parse_block function to RSA mechanisms. The rsa_parse_block >> function parses an encryption block according to PKCS #1: RSA >> Encryption, Version 1.5. >> >> Signed-off-by: Ramon de Carvalho Valle <[email protected]> >> --- >> usr/lib/pkcs11/common/mech_rsa.c | 201 >> ++++++++++++++++++++++++++++++++------ 1 files changed, 172 >> insertions(+), 29 deletions(-) > > > Ramon, thanks for the patch. I'll accept and push upstream, but please > see comments inlined below: > >> >> } >> >> - rc = ckm_rsa_decrypt( in_data, modulus_bytes, out, key_obj ); >> - if (rc == CKR_OK) { >> - CK_ULONG len; >> - >> - // strip off the PKCS block formatting data >> - // >> - // 00 | BT | PADDING | 00 | DATA >> - // >> - for (i=2; i < in_data_len; i++) { >> - if (out[i] == 0x0) { >> - i++; // point i at the first data byte >> - break; >> - } >> - } >> - >> - if (i == in_data_len){ >> - st_err_log(14, __FILE__, __LINE__); >> - return CKR_ENCRYPTED_DATA_INVALID; >> - } >> - len = in_data_len - i; >> + if (*out_data_len < (modulus_bytes - 11)) { >> + *out_data_len = modulus_bytes - 11; >> + st_err_log(68, __FILE__, __LINE__); >> + return CKR_BUFFER_TOO_SMALL; >> + } >> - if (len > *out_data_len) { >> - *out_data_len = len; >> - st_err_log(111, __FILE__, __LINE__); >> - return CKR_BUFFER_TOO_SMALL; >> + rc = ckm_rsa_decrypt( in_data, modulus_bytes, out, key_obj ); >> + if (rc != CKR_OK) { >> + if (rc == CKR_DATA_LEN_RANGE) { >> + st_err_log(112, __FILE__, __LINE__); >> + return CKR_ENCRYPTED_DATA_LEN_RANGE; >> } > > I saw that you moved this over from down there, but looking for the > CKR_DATA_LEN_RANGE symbol in the code, I'm not finding any function > down in ckm_rsa_decrypt() path that returns this code. I think this > needs fixing somewhere. Could you investigate if possible? At least for software token I don't see it happen either. It would be possible only if RSA_private_decrypt() returns CKR_DATA_LEN_RANGE through ckm_rsa_decrypt, which is not the case. However, for ICA the same function is used for encryption/decryption operations and I am not sure of its return values. My guess is that it does not distinguish from encryption/decryption operations and returns CKR_DATA_LEN_RANGE for both operations, and openCryptoki adjusts the error code accordingly. This is the original code block from openCryptoki: // ckm_rsa_operation is used for all RSA operations so we need to adjust // the return code accordingly // if (rc == CKR_DATA_LEN_RANGE){ st_err_log(112, __FILE__, __LINE__); return CKR_ENCRYPTED_DATA_LEN_RANGE; } return rc; Let me know what you think. > > > >> >> - memcpy( out_data, &out[i], len ); >> - *out_data_len = len; >> + st_err_log(133, __FILE__, __LINE__); >> + return rc; >> } >> - else >> + >> + rc = rsa_parse_block(out, modulus_bytes, out_data, out_data_len, >> PKCS_BT_2); >> + if (rc != CKR_OK) { >> + /* >> + * FIXME: rsa_parse_block() should have it's own error message. >> + */ >> st_err_log(133, __FILE__, __LINE__); > > Could you please add this error and provide a patch when you are able > to? Sure, I will do. > >> + return rc; >> + } >> >> - if (rc == CKR_DATA_LEN_RANGE){ >> - st_err_log(109, __FILE__, __LINE__); >> + /* >> + * For PKCS #1 v1.5 padding, out_data_len must be less than >> + * modulus_bytes - 11. >> + */ >> + if (*out_data_len > (modulus_bytes - 11)) { > > I thought this exact same check was being done above, before calling > ckm_rsa_decrypt(), and with a different error code in case the check > fails (CKR_BUFFER_TOO_SMALL). Am I missing something or has something > changed? This is because previous to calling rsa_parse_block out_data_len contains the out_data buffer size. At the end of rsa_parse_block, the destination buffer size is checked through out_data_len value, then the size of the parsed block is attributed to out_data_len. Afterwards, out_data_len, which contains the size of the parsed block, is checked to be not less than or equal modulus_bytes - 11. The check at the beginning of rsa_pkcs_decrypt verifies the out_data buffer length, and the check at the end of rsa_pkcs_decrypt verifies the length of the parsed block, that is why it returns different error codes. > >> + st_err_log(112, __FILE__, __LINE__); >> return CKR_ENCRYPTED_DATA_LEN_RANGE; >> } >> + >> return rc; >> } >> > > > Thanks for the patch! > Thank you Klaus! - -- Ramon de Carvalho Valle Software Engineer IBM Linux Technology Center E-Mail: [email protected] -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAkwa03kACgkQGIS0iEuhp4NMJACfSjiSKJi5vAyLgJ7EblV52BoI 6YgAn37FUWkqA/L+AX3U4KYG3/ekRshxiEYEARECAAYFAkwa03kACgkQkcIYeh81 wLlMJACcDKhDlUdke2reJz1vtXhUcbEvpfwAn3SnbiutAA7DBck1SYIwuawI3pVv =Fy/X -----END PGP SIGNATURE----- ------------------------------------------------------------------------------ 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
