> From: owner-openssl-us...@openssl.org On Behalf Of SpikeSpiegel
> Sent: Monday, 07 September, 2009 15:23
> I'm writing a little program that encrypt/decrypt some text
> but I've found some troubles using the OpenSSL libraries...
> When I use between the encrypt() function and the decrypt()
> function I receive some strange errors or segmentation faults..
> void printbyte(char b) {
> char c;
> c = b;
> c = c >> 4;
> c = c & 15;
> fprintf(stderr,"%X", c);
> c = b;
> c = c & 15;
> fprintf(stderr,"%X:", c);
> }
>
On practically all C implementations char is 8bits and
this is exactly the same as fprintf(stderr,"%02X:",b).
(And on any system where you actually use more than 8bits
this functionality is probably not what you want anyway.)
> strcpy(kab,"30A75E9E5D962371");
>
see below
> int encrypt(char* key, char* plain, char* crypto)
> {
> char *msg = (char*) malloc(strlen(plain));
> strcpy(msg,plain);
That doesn't allocate room for the null terminator,
which strcpy() does copy. You're probably clobbering
the malloc arena, which can cause arbitrary screwiness.
But you don't need to copy anyway; you can just use
the caller-supplied plain[...].
Nits: you don't need the cast in correct C; in C++ you do,
but in C++ you should use operator new instead. You should
always check if malloc returned null (= failed) before trying
to use the memory. In a small program like this malloc won't
fail, but once you start using the same code in larger programs
someday it will. (C++ new throws an exception automatically.)
> char *plaintext, *ciphertext;
> char k[256]; /* chiave di cifratura */
> int nc, /* numero di byte [de]crittati ad ogni passo*/
> nctot, /* numero totale di byte crittati */
> i, /* indice */
> ct_len, /* lunghezza del buffer */
> ct_ptr, msg_ptr, /* puntatore alla prima posizione
> libera del buffer */
> msg_len, /* lunghezza del messaggio */
> n, /* numero di byte che si cifra di volta in volta*/
> ret = 0;
>
> strcpy(crypto, "");
>
This is legal but useless; the data put in crypto is managed
(correctly) with an explicit count, not as a string.
> /* allocazione del contesto */
>
> EVP_CIPHER_CTX *ctx = (EVP_CIPHER_CTX
> *)malloc(sizeof(EVP_CIPHER_CTX));
>
> /* inizializzazione del contesto */
> EVP_CIPHER_CTX_init(ctx);
> /* setup del contesto per la cifratura */
> EVP_EncryptInit(ctx, EVP_des_ecb(), NULL, NULL);
>
I assume you know single-DES is now fairly easily breakable,
and this is just an example or a toy.
> /* selezione random della chiave */
> strcpy(k,key);
>
> /* impostazione della chiave */
> EVP_EncryptInit(ctx, NULL, k, NULL);
>
The key you set using the routine above was 16 _characters_
in ASCII that happened to be hex digits. DES uses only the
first 8 bytes of the supplied key, in this case 8 characters.
If you meant that key to be hex data, you need to convert it
before passing it in to OpenSSL. And if you do, symmetric keys
generally are binary data and shouldn't be treated as C strings.
> /* allocazione del buffer per ciphertext */
> msg_len = strlen(msg)+1;
> ct_len = msg_len + EVP_CIPHER_CTX_block_size(ctx);
>
> ciphertext = (char *)malloc(ct_len);
>
This time you don't need 1 for the null terminator, because
the ciphertext can't be (and isn't) null terminated. Again
you don't really need a separate buffer, you could just write
into the caller's crypto[...].
> /* cifratura di n byte alla volta */
> n = 10;
> nc = 0;
> nctot = 0;
> ct_ptr = 0;
> msg_ptr =0;
>
> for (i = 0; i < msg_len / n; i++) {
> EVP_EncryptUpdate(ctx, &ciphertext[ct_ptr],
> &nc, &msg[msg_ptr], n);
> ct_ptr += nc;
> msg_ptr += n;
> nctot += nc;
> }
>
> if ( msg_len % n ) {
> EVP_EncryptUpdate(ctx, &ciphertext[ct_ptr], &nc,
> &msg[msg_ptr], msg_len % n);
> ct_ptr += nc;
> msg_ptr += msg_len % n;
> nctot += nc;
> }
>
That code looks correct for doing encrypt in chunks of n, unless
I missed something, although n = 10 is an odd choice of chunk.
nctot and ct_ptr are exactly the same; you don't need both.
> //EVP_EncryptUpdate(ctx, &ciphertext[ct_ptr], &nc,
> &msg[msg_ptr], msg_len);
>
> ret = EVP_EncryptFinal(ctx, &ciphertext[ct_ptr], &nc);
> //fprintf(stderr,"\nENC RET: = %d\n", ret);
> nctot += nc;
>
> //strcpy(crypto, ciphertext);
> memcpy(crypto, ciphertext, nctot);
>
>
> //fprintf(stderr,"\nENCRYPT\nP: %s \nC: %s\nK:
> %s",plain, crypto, k);
> //fprintf(stderr,"\nCT_LEN: = %d\n", msg_len +
> EVP_CIPHER_CTX_block_size(ctx));
> int j = 0;
> for (j = 0; j < ct_len - 1; j++)
> printbyte(ciphertext[j]);
> printbyte(ciphertext[ct_len-1]);
> printf("\n");
>
You are correct to use memcpy NOT strcpy, and similarly
not to printf with %s; the ciphertext often will contain
valid zero bytes. However, you should use nctot as the
length of the ciphertext; this will (with PKCS5 padding)
be plain_len + block ROUNDED DOWN to blocksize, not just
plain_len + block + 1 as you computed in ct_len.
You do return it correctly, next:
> return nctot;
>
> }
>
> int decrypt(char* key, char* plain, char* crypto) {
> char *plaintext, *ciphertext;
>
> strcpy(plain, "");
>
> char k[256]; /* chiave di cifratura */
> int nc, /* numero di byte [de]crittati ad ogni passo*/
> nctot, /* numero totale di byte crittati */
> i, /* indice */
> ct_len, /* lunghezza del buffer */
> ct_ptr, msg_ptr, /* puntatore alla prima posizione
> libera del buffer */
> msg_len, /* lunghezza del messaggio */
> n, /* numero di byte che si cifra di volta in volta*/
> ret;
>
>
> ct_len = strlen(crypto);
>
This however is wrong. The ciphertext will often contain
valid zero bytes. You must pass its length explicitly and use that.
> /* allocazione del contesto */
> EVP_CIPHER_CTX *ctxd = (EVP_CIPHER_CTX
> *)malloc(sizeof(EVP_CIPHER_CTX));
>
> /* inizializzazione del contesto */
> EVP_CIPHER_CTX_init(ctxd);
> /* setup del contesto per la cifratura */
> EVP_DecryptInit(ctxd, EVP_des_ecb(), NULL, NULL);
>
> /* selezione random della chiave */
> strcpy(k,key);
>
As above.
> /* impostazione della chiave */
> EVP_DecryptInit(ctxd, NULL, k, NULL);
>
> plaintext = (char *)malloc(ct_len);
> bzero(plaintext, ct_len);
>
ct_len is a little longer than needed, but that's
much better than being too short and overflowing.
Zeroing the whole buffer is inefficient but not harmful.
And again you don't actually need to copy anyway.
> /* Inizializzazione contesto decifratura */
> EVP_CIPHER_CTX_init(ctxd);
> EVP_DecryptInit(ctxd, EVP_des_ecb(), k, NULL);
> /*decifratura */
>
You don't need to init&DecryptInit AGAIN.
> nc = 0;
> ct_ptr = 0;
> ct_len = 256;
This should be passed in.
> fprintf(stderr,"OK");
> EVP_DecryptUpdate(ctxd, &plaintext[ct_ptr], &nc,
> &ciphertext[ct_ptr], ct_len);
You never set ciphertext, presumably intended to be a
local copy, although as above you don't need to copy.
You're probably clobbering some random area of memory
that happens to sometimes cause a crash and sometimes
produces some result -- but maybe not the right one.
Using 256 for ct_len, instead of the actual size,
will almost certainly overflow the plaintext buffer
which was allocated for roughly the correct size.
If you used the multiple-chunk logic like for encrypt,
you would need to use separate indexes (or pointers)
for plain and cipher, because they differ. For a single
chunk as here, it's clearer to just use 0.
> fprintf(stderr,"OK");
> if (ERR_peek_error())
> {
> ERR_load_crypto_strings();
> ERR_print_errors_fp(stderr);
> //ERR_free_strings();
> }
> ret = EVP_DecryptFinal(ctxd, &plaintext[ct_ptr], &nc);
> //fprintf(stderr,"\nDEC RET: = %d\n", ret);
>
> //strcpy(plain, plaintext);
> memcpy(plain, plaintext, strlen(plaintext));
If the above decryption had been correct, that would
copy the plaintext but NOT the null terminator, so ...
> //fprintf(stderr,"\nDECRYPT\nP: %s \nC: %s\nK:
> %s\n",plain, crypto, k);
>
> //fprintf(stderr,"Cipher: ");
>
> int j = 0;
> for (j = 0; j < strlen(plain); j++)
> printf("%c:", plaintext[j]);
> printf("%c\n", plaintext[strlen(plain)]);
> printf("\n");
>
... this might not correctly identify the end of the data,
and neither might the caller(s).
> return nc;
> }
>
> int main()
> {
> char plain[256];
> char crypto[256];
> char kab[256];
>
> strcpy(plain, "Test");
> read_shared_key(kab);
>
> encrypt(kab, plain, crypto);
You need to save the ciphertext-length returned from encrypt,
and provide it to decrypt to use.
> printf("ERROR????"); //
> <<<<-----------------
>
> decrypt(kab, plain, crypto);
> }
>
______________________________________________________________________
OpenSSL Project http://www.openssl.org
User Support Mailing List openssl-users@openssl.org
Automated List Manager majord...@openssl.org
