On 06/22/2011 05:51 PM, Crypto User wrote:
> On Jun 22, 11:56 am, Robert Relyea <rrel...@redhat.com> wrote:
>>
>>> Can anybody pl. provide any pointer.
>>> Thanks
Below I made some guesses based on the description I had from you, but
now that I've reviewed the code, I think the problem is just a basic
programming issue. Look at my inline comments on your code first.
Also, what you are trying to on the face of it, is not good security
hygene. If you are trying to match some existing protocol, so be it
(though I would like to know what protocol you are trying to match). If
you are building your own, however, then I would strongly encourage you
to use the more standard practive of encrypting a symmetric key (called
wrapping) with the public key and then using the symmetric key to
encrypt your data. I've made the same comment below, but I don't want
you to miss the point, so I've included it here.
> The PubEncryptPKCS1 works but the PrivDecryptPKCS1 does not work. It
> still gives error -8023.
Ah, the likelihood then is the decrypted data was somehow mangled. It
may mean your keypair doesn't really match up (that is you are trying to
decrypt with a private key that does not match the public key that
encrypted the data). It could also be that your private key is corrupted
in some way. Finally, the encrypted data itself could be mangled. *One
way to check the former is to print out the modulus and the public
exponent of both your public key and your private key and make sure they
match. This won't detect a corrupted private key, however.
*This, in fact is the real problem. It's clear you are dropping bytes
from your encrypted data, which will cause your decryption to fail.
> The same keys work for pubencrypy/decryptRaw with the changes in the
> data length. (128 for 1024 RSA) for PKCS1 I have 111 bytes of data.
> I encrypt the data and write out to a file and then read it from the
> file for decryption again.
decryptRaw does not care about the format of the decrypted data, so it's
unlikey to throw any errors. You can use decrypt raw on the data
encrypted with pubEncryptPKCS1. The result should be a block that's 128
bytes long of the form:
00 02 R R R R .... R 0 D D D D D D.
where R is a random number not equal to 0 and D is the encrypted data.
If you get data vastly different than this, then you are looking at
either some sort of key corruption or some sort of input data
corruption. If the format looks close to this, but not exact, then you
have run into a bug in pubEncryptPKCS1.
> I follow the same technique for symmetric encrypt/decrypt and it
> works.So I know my writting out routines do not add any extra stuff.
> My code is part of a bigger scheme of code but here are the snippets -
> Let me know if it is illegible.
OK, looking at the code, I see one thing that I was worried about. It's
usually not considered good security practice to encrypt blocks of data
directly with RSA. What is usually done instead is to generate a key,
encrypt (wrap) that key with RSA and encrypt your data with the
symmetric algorithm. The latter is faster and more reliable.
> signed int AsymmetricDecrypt(CCS_Context *ccsContext,
>
> CCS_CryptParameters *decryptionParams,
>
> CCS_Stream_Input *cipherText,
>
> unsigned int cipherTextLength,
>
> CCS_Stream_Output *plainText,
>
> unsigned int *plainTextLength) {
>
>
>
> signed int err =
> CCS_Success;
>
> SECKEYPrivateKey *privateKey = NULL;
>
> int modulus_length = 0;
>
> int offset = 0;/* seek position in the
> stream */
>
> int numBytesToRead = 0; /* numBytes to
> read from the stream at a time */
>
> unsigned char *dataBuffer = 0;
>
> size_t numBytesRead = 0;/* num bytes read
> from the stream */
>
> unsigned char *decryptedText = NULL;
>
> unsigned int decryptedTextLen = 0;
>
> unsigned int isLastBlock = 0;
>
> int blockNum = 0;
>
>
>
> if ((err = getUnWrappedPrivateKey(decryptionParams->key,
>
> &(decryptionParams-
>> keyWrapParams),
> &privateKey)) != CCS_Success)
> {
>
> CCS_DebugOut(CCS_DEBUG_LEVEL_ERROR,
>
> "import of SymmetricKey failed with error code %d %s %d
> \n",
>
> err, __FILE__, __LINE__);
>
> goto cleanup;
>
> }
>
> modulus_length = PK11_GetPrivateModulusLen(privateKey);
>
>
>
> if ((dataBuffer = (unsigned char*)malloc(cipherTextLength *
>
> sizeof(unsigned char))) ==
> NULL) {
Even though you allocate enough space for the whole cipherText, you only
use the first modulus_length bytes. This isn't a bug (well as long as
cipherTextLength > modulus_length, which do to other bugs, it may not
be). but is is inefficient.
> err = CCS_OutOfMemoryError;
>
> goto cleanup;
>
> }
>
>
>
> if ((decryptedText = (unsigned char*)malloc(modulus_length *
>
> sizeof(unsigned
> char))) == NULL) {
>
> err = CCS_OutOfMemoryError;
>
> goto cleanup;
>
> }
>
>
>
>
>
> /* Position the stream offset at 0 to start.*/
>
> if ((err = CCS_Stream_Input_seek(cipherText,
> offset,CCS_Stream_SeekBeg) )
>
> != CCS_Success) {
>
>
>
> CCS_DebugOut(CCS_DEBUG_LEVEL_ERROR,
>
> " CCS_Stream_Input_seek() call failed with error
> code %d %s %d\n",
>
> err, __FILE__, __LINE__);
>
> goto cleanup;
>
> }
>
>
>
> while (!isLastBlock) {
>
> /* Read data in blocks of modulus_length */
>
> numBytesToRead = (cipherTextLength > modulus_length) ?
>
> modulus_length :
> cipherTextLength;
>
>
>
> if ((err = CCS_Stream_Input_readBytes(cipherText, (unsigned
> int) numBytesToRead,
>
> &numBytesRead,
> dataBuffer)) != CCS_Success) {
>
> CCS_DebugOut(CCS_DEBUG_LEVEL_ERROR,
>
> " CCS_Stream_Input_readBytes() call failed
> with error code %d %s %d\n",
>
> err, __FILE__, __LINE__);
>
> goto cleanup;
>
> }
>
>
>
> cipherTextLength -= numBytesRead;
>
>
>
> if ((numBytesRead < modulus_length) || (cipherTextLength ==
> 0)) {
>
> isLastBlock = 1;
>
> }
numBytesRead < modulus_length should be a hard error. numBytesRead
should equal modulus_length (always).
> //initialize it all to zero so that if the text encrypted is
> less that the max, there is no leftover
>
> decryptedText[0] = '\0' ;
This isn't doing what the comment says, though it's not likely to be our
problem.
>
>
> if ((err = PK11_PrivDecryptPKCS1(privateKey, decryptedText,
> plainTextLength,
>
>
> modulus_length, dataBuffer + (modulus_length * blockNum),
dataBuffer+(modulus_length*blockNum) is incorrect here. Your read above
always reads the next block directly into dataBuffer (overwriting the
last encrypted block, which is fine because you no longer need it).
>
> modulus_length)) != SECSuccess ) {
>
> err = PR_GetError();
>
> CCS_DebugOut(CCS_DEBUG_LEVEL_ERROR,
>
> "PK11_PubEncryptRaw() call failed with error
> code %d %s %d\n",
>
> err, __FILE__, __LINE__);
Obviously your debug output is incorrect here...
One question, is this failing on the last block (you may want to output
block num here).
> goto cleanup;
>
> }
>
> blockNum ++;
>
> if ((err =
> CCS_Stream_Output_writeBytes(plainText,decryptedText, 0,
>
> numBytesRead,
>
> plainTextLength)) !=
> SECSuccess ) {
This is also wrong, you want to write *plainTextLength, not numBytesRead
(or you'll write garbage).
> CCS_DebugOut(CCS_DEBUG_LEVEL_ERROR,
>
> "CCS_Stream_Output_writeBytes() call failed
> with error code %d %s %d\n",
>
> err, __FILE__, __LINE__);
>
> goto cleanup;
>
> }
>
>
>
> }
>
>
>
>
>
> cleanup:
>
>
>
> if (dataBuffer)
>
> free(dataBuffer);
>
>
>
> if (decryptedText)
>
> //free(decryptedText);
>
>
>
> if (privateKey)
>
> //SECKEY_DestroyPrivateKey(privateKey);
>
>
>
> return err;
>
>
>
> }
>
>
>
> signed int AsymmetricEncrypt(CCS_Context *ccsContext,
>
> CCS_CryptParameters
> *encryptionParams,
>
> CCS_Stream_Input *plainText,
>
> unsigned int plainTextLength,
>
> CCS_Stream_Output *cipherText,
>
> unsigned int *cipherTextLength) {
>
>
>
>
>
> signed int err =
> CCS_Success;
>
> SECKEYPublicKey *pubKey = NULL;
>
> int modulus_length = 0;
>
> int offset = 0;/* seek position in the
> stream */
>
> int numBytesToRead = 0; /* numBytes to
> read from the stream at a time */
>
> unsigned char *dataBuffer = 0;
>
> size_t numBytesRead = 0;/* num bytes read
> from the stream */
>
> unsigned char *encryptedText = NULL;
>
> unsigned int encryptedTextLen = 0;
>
> unsigned int isLastBlock = 0;
>
> int blockNum = 0;
>
>
>
> if ((err = importPublicKey(encryptionParams->key, &pubKey)) !=
> CCS_Success) {
>
> CCS_DebugOut(CCS_DEBUG_LEVEL_ERROR,
>
> "import of SymmetricKey failed with error code %d
> %s %d\n",
>
> err, __FILE__, __LINE__);
>
> goto cleanup;
>
> }
>
> modulus_length = SECKEY_PublicKeyStrength(pubKey);
>
> modulus_length -= 11;
I would make this another variable, like 'max_plaintext_block'.
> if ((dataBuffer = (unsigned char*)malloc(plainTextLength *
>
> sizeof(unsigned char))) ==
> NULL) {
NOTE: this is more space than you actually need for dataBuffer. You
aren't reading in the full dataBuffer, only one block at a time.
max_plaintext_block should be sufficient.
> err = CCS_OutOfMemoryError;
>
> goto cleanup;
>
> }
>
>
>
> if ((encryptedText = (unsigned char*)malloc(modulus_length *
>
> sizeof(unsigned
> char))) == NULL) {
>
> err = CCS_OutOfMemoryError;
>
> goto cleanup;
>
> }
Here's a case your your modulus_length -= 11 is causing problems. This
should be the full modulus_length.
>
>
>
>
> /* Position the stream offset at 0 to start.*/
>
> if ((err = CCS_Stream_Input_seek(plainText,
> offset,CCS_Stream_SeekBeg) )
>
> != CCS_Success) {
>
>
>
> CCS_DebugOut(CCS_DEBUG_LEVEL_ERROR,
>
> " CCS_Stream_Input_seek() call failed with error
> code %d %s %d\n",
>
> err, __FILE__, __LINE__);
>
> goto cleanup;
>
> }
>
>
>
> while (!isLastBlock) {
>
> /* Read data in blocks of modulus_length */
>
> numBytesToRead = (plainTextLength > modulus_length) ?
>
> modulus_length :
> plainTextLength;
This should be your new max_plaintext_length.
>
>
> if ((err = CCS_Stream_Input_readBytes(plainText, (unsigned
> int) numBytesToRead,
>
> &numBytesRead,
> dataBuffer)) != CCS_Success) {
>
> CCS_DebugOut(CCS_DEBUG_LEVEL_ERROR,
>
> " CCS_Stream_Input_readBytes() call failed
> with error code %d %s %d\n",
>
> err, __FILE__, __LINE__);
>
> goto cleanup;
>
> }
>
>
>
> plainTextLength -= numBytesRead;
>
>
>
> if ((numBytesRead < modulus_length) || (plainTextLength == 0))
> {
>
> isLastBlock = 1;
>
> }
This should be max_plaintext_length again. Unlike decrypt, numBytesRead
can be less then max_plaintext_length. This should be a bit of a
warning. Encrypt and Decrypt operations are paired, but they don't
always have the same form or semantic. Be careful of copy and paste;).
> //initialize it all to zero so that if the text encrypted is
> less that the max, there is no leftover
>
> encryptedText[0] = '\0' ;
>
>
>
> if ((err = PK11_PubEncryptPKCS1(pubKey, encryptedText,
>
> dataBuffer,
>
> modulus_length, NULL)) !=
> SECSuccess ) {
PK11_PubEncryptPKCS1 is going to assume encryptedText is modulus_length
long, but you've cut it off by 11 bytes (which is likely to trash
something else or get trashed itself. The modulus_length specified there
is really the length of the databuffer, which should be what I'm calling
max_plaintext_length.
> err = PR_GetError();
>
> CCS_DebugOut(CCS_DEBUG_LEVEL_ERROR,
>
> "PK11_PubEncryptRaw() call failed with error
> code %d %s %d\n",
>
> err, __FILE__, __LINE__);
>
> goto cleanup;
>
> }
>
> blockNum ++;
>
> if ((err =
> CCS_Stream_Output_writeBytes(cipherText,encryptedText, 0,
>
> modulus_length,
>
> cipherTextLength)) !=
> SECSuccess ) {
OK, where you should be writing out the full modulus length. You are
only writing modulus_length -11. There is no way to rebuild the original
data if now because you are 11 bytes short. RSA needs all 128 bytes.
> CCS_DebugOut(CCS_DEBUG_LEVEL_ERROR,
>
> "CCS_Stream_Output_writeBytes() call failed
> with error code %d %s %d\n",
>
> err, __FILE__, __LINE__);
>
> goto cleanup;
>
> }
>
>
>
> }
>
>
>
>
>
> cleanup:
>
>
>
> if (dataBuffer)
>
> free(dataBuffer);
>
>
>
> if (encryptedText)
>
> //free(encryptedText);
yes, this is the result of having encryptedText too short, this should
have been a clue something else was up;).
>
>
> if (pubKey)
>
> // SECKEY_DestroyPublicKey(pubKey);
>
>
>
> return err;
>
>
>
> }
--
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-crypto
