To check if a received IKE_SA_INIT request is a new request or a
retransmit, charon maintains hashes of the pending IKE_SA_INIT
exchanges.

However, the hash calculation is not reentrant because a single hasher
is used for the whole IKE SA manager. It leads to bogus calculations
under high load and hence dropped messages on responder
(IkeInInvalidSpi incremented).

Don't share a single hasher in the IKE SA manager, create a transient
one whenever a message must be hashed.

Signed-off-by: Christophe Gouault <[email protected]>
---
 src/libcharon/sa/ike_sa_manager.c |   42 +++++++++++++++----------------------
 1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/src/libcharon/sa/ike_sa_manager.c 
b/src/libcharon/sa/ike_sa_manager.c
index f38cc41..a6880c4 100644
--- a/src/libcharon/sa/ike_sa_manager.c
+++ b/src/libcharon/sa/ike_sa_manager.c
@@ -384,11 +384,6 @@ struct private_ike_sa_manager_t {
        rng_t *rng;
 
        /**
-        * SHA1 hasher for IKE_SA_INIT retransmit detection
-        */
-       hasher_t *hasher;
-
-       /**
         * reuse existing IKE_SAs in checkout_by_config
         */
        bool reuse_ikesa;
@@ -966,9 +961,12 @@ static bool get_init_hash(private_ike_sa_manager_t *this, 
message_t *message,
                                                  chunk_t *hash)
 {
        host_t *src;
+       hasher_t *hasher;
+       bool result = FALSE;
 
-       if (!this->hasher)
-       {       /* this might be the case when flush() has been called */
+       hasher = lib->crypto->create_hasher(lib->crypto, HASH_SHA1);
+       if (hasher == NULL)
+       {
                return FALSE;
        }
        if (message->get_first_payload_type(message) == PLV1_FRAGMENT)
@@ -977,34 +975,38 @@ static bool get_init_hash(private_ike_sa_manager_t *this, 
message_t *message,
                u_int64_t spi;
 
                src = message->get_source(message);
-               if (!this->hasher->allocate_hash(this->hasher,
+               if (!hasher->allocate_hash(hasher,
                                                                                
 src->get_address(src), NULL))
                {
-                       return FALSE;
+                       goto end;
                }
                port = src->get_port(src);
-               if (!this->hasher->allocate_hash(this->hasher,
+               if (!hasher->allocate_hash(hasher,
                                                                                
 chunk_from_thing(port), NULL))
                {
-                       return FALSE;
+                       goto end;
                }
                spi = message->get_initiator_spi(message);
-               return this->hasher->allocate_hash(this->hasher,
+               result = hasher->allocate_hash(hasher,
                                                                                
   chunk_from_thing(spi), hash);
+               goto end;
        }
        if (message->get_exchange_type(message) == ID_PROT)
        {       /* include the source for Main Mode as the hash will be the 
same if
                 * SPIs are reused by two initiators that use the same proposal 
*/
                src = message->get_source(message);
 
-               if (!this->hasher->allocate_hash(this->hasher,
+               if (!hasher->allocate_hash(hasher,
                                                                                
 src->get_address(src), NULL))
                {
-                       return FALSE;
+                       goto end;
                }
        }
-       return this->hasher->allocate_hash(this->hasher,
+       result = hasher->allocate_hash(hasher,
                                                                           
message->get_packet_data(message), hash);
+end:
+       hasher->destroy(hasher);
+       return result;
 }
 
 /**
@@ -2072,8 +2074,6 @@ METHOD(ike_sa_manager_t, flush, void,
 
        this->rng->destroy(this->rng);
        this->rng = NULL;
-       this->hasher->destroy(this->hasher);
-       this->hasher = NULL;
 }
 
 METHOD(ike_sa_manager_t, destroy, void,
@@ -2148,18 +2148,10 @@ ike_sa_manager_t *ike_sa_manager_create()
                },
        );
 
-       this->hasher = lib->crypto->create_hasher(lib->crypto, HASH_SHA1);
-       if (this->hasher == NULL)
-       {
-               DBG1(DBG_MGR, "manager initialization failed, no hasher 
supported");
-               free(this);
-               return NULL;
-       }
        this->rng = lib->crypto->create_rng(lib->crypto, RNG_WEAK);
        if (this->rng == NULL)
        {
                DBG1(DBG_MGR, "manager initialization failed, no RNG 
supported");
-               this->hasher->destroy(this->hasher);
                free(this);
                return NULL;
        }
-- 
1.7.10.4

_______________________________________________
Dev mailing list
[email protected]
https://lists.strongswan.org/mailman/listinfo/dev

Reply via email to