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? > > - 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? > + 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? > + st_err_log(112, __FILE__, __LINE__); > return CKR_ENCRYPTED_DATA_LEN_RANGE; > } > + > return rc; > } > Thanks for the patch! -- Klaus Heinrich Kiwi | [email protected] | http://blog.klauskiwi.com Open Source Security blog : http://www.ratliff.net/blog IBM Linux Technology Center : http://www.ibm.com/linux/ltc ------------------------------------------------------------------------------ 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
