Cleaned up memory management and return values for encryption payload
authorMartin Willi <martin@revosec.ch>
Tue, 10 Jul 2012 12:24:46 +0000 (14:24 +0200)
committerMartin Willi <martin@revosec.ch>
Mon, 16 Jul 2012 12:55:07 +0000 (14:55 +0200)
src/libcharon/encoding/message.c
src/libcharon/encoding/payloads/encryption_payload.c
src/libcharon/encoding/payloads/encryption_payload.h
src/libcharon/plugins/ha/ha_ike.c
src/libcharon/sa/ikev1/keymat_v1.h

index 7e4c6e0..75a54b4 100644 (file)
@@ -1581,19 +1581,11 @@ METHOD(message_t, generate, status_t,
                        htoun32(lenpos, chunk.len + encryption->get_length(encryption));
                }
                this->payloads->insert_last(this->payloads, encryption);
-               if (!encryption->encrypt(encryption, chunk))
+               if (encryption->encrypt(encryption, chunk) != SUCCESS)
                {
-                       if (this->is_encrypted)
-                       {
-                               free(chunk.ptr);
-                       }
                        generator->destroy(generator);
                        return INVALID_STATE;
                }
-               if (this->is_encrypted)
-               {
-                       free(chunk.ptr);
-               }
                generator->generate_payload(generator, &encryption->payload_interface);
        }
        chunk = generator->get_chunk(generator, &lenpos);
@@ -1862,19 +1854,24 @@ static status_t decrypt_payloads(private_message_t *this, keymat_t *keymat)
                        {       /* instead of associated data we provide the IV, we also update
                                 * the IV with the last encrypted block */
                                keymat_v1_t *keymat_v1 = (keymat_v1_t*)keymat;
-                               chunk_t iv = chunk_empty;
+                               chunk_t iv;
 
-                               if (keymat_v1->get_iv(keymat_v1, this->message_id, &iv) &&
-                                       keymat_v1->update_iv(keymat_v1, this->message_id,
-                                                       chunk_create(chunk.ptr + chunk.len - bs, bs)))
+                               if (keymat_v1->get_iv(keymat_v1, this->message_id, &iv))
                                {
                                        status = encryption->decrypt(encryption, iv);
+                                       if (status == SUCCESS)
+                                       {
+                                               if (!keymat_v1->update_iv(keymat_v1, this->message_id,
+                                                               chunk_create(chunk.ptr + chunk.len - bs, bs)))
+                                               {
+                                                       status = FAILED;
+                                               }
+                                       }
                                }
                                else
                                {
                                        status = FAILED;
                                }
-                               free(chunk.ptr);
                        }
                        else
                        {
index c40bd2a..c4d8d10 100644 (file)
@@ -308,7 +308,7 @@ static chunk_t append_header(private_encryption_payload_t *this, chunk_t assoc)
        return chunk_cat("cc", assoc, chunk_from_thing(header));
 }
 
-METHOD(encryption_payload_t, encrypt, bool,
+METHOD(encryption_payload_t, encrypt, status_t,
        private_encryption_payload_t *this, chunk_t assoc)
 {
        chunk_t iv, plain, padding, icv, crypt;
@@ -319,14 +319,14 @@ METHOD(encryption_payload_t, encrypt, bool,
        if (this->aead == NULL)
        {
                DBG1(DBG_ENC, "encrypting encryption payload failed, transform missing");
-               return FALSE;
+               return INVALID_STATE;
        }
 
        rng = lib->crypto->create_rng(lib->crypto, RNG_WEAK);
        if (!rng)
        {
                DBG1(DBG_ENC, "encrypting encryption payload failed, no RNG found");
-               return FALSE;
+               return NOT_SUPPORTED;
        }
 
        assoc = append_header(this, assoc);
@@ -362,7 +362,7 @@ METHOD(encryption_payload_t, encrypt, bool,
                DBG1(DBG_ENC, "encrypting encryption payload failed, no IV or padding");
                rng->destroy(rng);
                free(assoc.ptr);
-               return FALSE;
+               return FAILED;
        }
        padding.ptr[padding.len - 1] = padding.len - 1;
        rng->destroy(rng);
@@ -376,7 +376,7 @@ METHOD(encryption_payload_t, encrypt, bool,
        if (!this->aead->encrypt(this->aead, crypt, assoc, iv, NULL))
        {
                free(assoc.ptr);
-               return FALSE;
+               return FAILED;
        }
 
        DBG3(DBG_ENC, "encrypted %B", &crypt);
@@ -384,10 +384,10 @@ METHOD(encryption_payload_t, encrypt, bool,
 
        free(assoc.ptr);
 
-       return TRUE;
+       return SUCCESS;
 }
 
-METHOD(encryption_payload_t, encrypt_v1, bool,
+METHOD(encryption_payload_t, encrypt_v1, status_t,
        private_encryption_payload_t *this, chunk_t iv)
 {
        generator_t *generator;
@@ -397,8 +397,7 @@ METHOD(encryption_payload_t, encrypt_v1, bool,
        if (this->aead == NULL)
        {
                DBG1(DBG_ENC, "encryption failed, transform missing");
-               chunk_free(&iv);
-               return FALSE;
+               return INVALID_STATE;
        }
 
        generator = generator_create();
@@ -422,14 +421,12 @@ METHOD(encryption_payload_t, encrypt_v1, bool,
 
        if (!this->aead->encrypt(this->aead, this->encrypted, chunk_empty, iv, NULL))
        {
-               chunk_free(&iv);
-               return FALSE;
+               return FAILED;
        }
-       chunk_free(&iv);
 
        DBG3(DBG_ENC, "encrypted %B", &this->encrypted);
 
-       return TRUE;
+       return SUCCESS;
 }
 
 /**
@@ -548,7 +545,6 @@ METHOD(encryption_payload_t, decrypt_v1, status_t,
        if (this->aead == NULL)
        {
                DBG1(DBG_ENC, "decryption failed, transform missing");
-               chunk_free(&iv);
                return INVALID_STATE;
        }
 
@@ -557,15 +553,16 @@ METHOD(encryption_payload_t, decrypt_v1, status_t,
                this->encrypted.len < iv.len || this->encrypted.len % iv.len)
        {
                DBG1(DBG_ENC, "decryption failed, invalid length");
-               chunk_free(&iv);
                return FAILED;
        }
 
        DBG3(DBG_ENC, "decrypting payloads:");
        DBG3(DBG_ENC, "encrypted %B", &this->encrypted);
 
-       this->aead->decrypt(this->aead, this->encrypted, chunk_empty, iv, NULL);
-       chunk_free(&iv);
+       if (!this->aead->decrypt(this->aead, this->encrypted, chunk_empty, iv, NULL))
+       {
+               return FAILED;
+       }
 
        DBG3(DBG_ENC, "plain %B", &this->encrypted);
 
index 60774bd..5c60693 100644 (file)
@@ -72,14 +72,18 @@ struct encryption_payload_t {
         * Generate, encrypt and sign contained payloads.
         *
         * @param assoc                 associated data
-        * @return                              TRUE if encrypted
+        * @return
+        *                                              - SUCCESS if encryption successful
+        *                                              - FAILED if encryption failed
+        *                                              - INVALID_STATE if aead not supplied, but needed
         */
-       bool (*encrypt) (encryption_payload_t *this, chunk_t assoc);
+       status_t (*encrypt) (encryption_payload_t *this, chunk_t assoc);
 
        /**
         * Decrypt, verify and parse contained payloads.
         *
         * @param assoc                 associated data
+        * @return
         *                                              - SUCCESS if parsing successful
         *                                              - PARSE_ERROR if sub-payload parsing failed
         *                                              - VERIFY_ERROR if sub-payload verification failed
index 2d02917..d523e89 100644 (file)
@@ -307,7 +307,6 @@ METHOD(listener_t, message_hook, bool,
                                m = ha_message_create(HA_IKE_IV);
                                m->add_attribute(m, HA_IKE_ID, ike_sa->get_id(ike_sa));
                                m->add_attribute(m, HA_IV, iv);
-                               free(iv.ptr);
                                this->socket->push(this->socket, m);
                                this->cache->cache(this->cache, ike_sa, m);
                        }
index a25bc98..cc9f3b3 100644 (file)
@@ -120,8 +120,11 @@ struct keymat_v1_t {
        /**
         * Returns the IV for a message with the given message ID.
         *
+        * The return chunk contains internal data and is valid until the next
+        * get_iv/udpate_iv/confirm_iv call.
+        *
         * @param mid                   message ID
-        * @param iv                    chunk receiving allocated IV
+        * @param iv                    chunk receiving IV, internal data
         * @return                              TRUE if IV allocated successfully
         */
        bool (*get_iv)(keymat_v1_t *this, u_int32_t mid, chunk_t *iv);