Handle initiation of not supported IKE versions properly

diff --git a/src/libcharon/control/controller.c b/src/libcharon/control/controller.c
index 0f24796..4aded8f 100644 (file)
--- a/src/libcharon/control/controller.c
+++ b/src/libcharon/control/controller.c
@@ -217,6 +217,17 @@ METHOD(job_t, initiate_execute, void,
 
        ike_sa = charon->ike_sa_manager->checkout_by_config(charon->ike_sa_manager,
                                                                                                                peer_cfg);
+       if (!ike_sa)
+       {
+               listener->child_cfg->destroy(listener->child_cfg);
+               peer_cfg->destroy(peer_cfg);
+               /* trigger down event to release listener */
+               listener->ike_sa = charon->ike_sa_manager->checkout_new(
+                                                                               charon->ike_sa_manager, IKE_ANY, TRUE);
+               DESTROY_IF(listener->ike_sa);
+               listener->status = FAILED;
+               return;
+       }
        listener->ike_sa = ike_sa;
 
        if (ike_sa->get_peer_cfg(ike_sa) == NULL)

diff --git a/src/libcharon/plugins/android/android_service.c b/src/libcharon/plugins/android/android_service.c
index 96603ab..8aba925 100644 (file)
--- a/src/libcharon/plugins/android/android_service.c
+++ b/src/libcharon/plugins/android/android_service.c
@@ -300,12 +300,17 @@ static job_requeue_t initiate(private_android_service_t *this)
                                                                                         0, "255.255.255.255", 65535);
        child_cfg->add_traffic_selector(child_cfg, FALSE, ts);
        peer_cfg->add_child_cfg(peer_cfg, child_cfg);
-       /* get an additional reference because initiate consumes one */
-       child_cfg->get_ref(child_cfg);
 
        /* get us an IKE_SA */
        ike_sa = charon->ike_sa_manager->checkout_by_config(charon->ike_sa_manager,
                                                                                                                peer_cfg);
+       if (!ike_sa)
+       {
+               peer_cfg->destroy(peer_cfg);
+               send_status(this, VPN_ERROR_CONNECTION_FAILED);
+               return JOB_REQUEUE_NONE;
+       }
+
        if (!ike_sa->get_peer_cfg(ike_sa))
        {
                ike_sa->set_peer_cfg(ike_sa, peer_cfg);
@@ -318,6 +323,8 @@ static job_requeue_t initiate(private_android_service_t *this)
        /* confirm that we received the request */
        send_status(this, i);
 
+       /* get an additional reference because initiate consumes one */
+       child_cfg->get_ref(child_cfg);
        if (ike_sa->initiate(ike_sa, child_cfg, 0, NULL, NULL) != SUCCESS)
        {
                DBG1(DBG_CFG, "failed to initiate tunnel");

diff --git a/src/libcharon/plugins/maemo/maemo_service.c b/src/libcharon/plugins/maemo/maemo_service.c
index 58361a4..69eac08 100644 (file)
--- a/src/libcharon/plugins/maemo/maemo_service.c
+++ b/src/libcharon/plugins/maemo/maemo_service.c
@@ -355,12 +355,16 @@ static gboolean initiate_connection(private_maemo_service_t *this,
                                                                                         0, "255.255.255.255", 65535);
        child_cfg->add_traffic_selector(child_cfg, FALSE, ts);
        peer_cfg->add_child_cfg(peer_cfg, child_cfg);
-       /* get an additional reference because initiate consumes one */
-       child_cfg->get_ref(child_cfg);
 
        /* get us an IKE_SA */
        ike_sa = charon->ike_sa_manager->checkout_by_config(charon->ike_sa_manager,
                                                                                                                peer_cfg);
+       if (!ike_sa)
+       {
+               peer_cfg->destroy(peer_cfg);
+               this->status = VPN_STATUS_CONNECTION_FAILED;
+               return FALSE;
+       }
        if (!ike_sa->get_peer_cfg(ike_sa))
        {
                ike_sa->set_peer_cfg(ike_sa, peer_cfg);
@@ -374,6 +378,8 @@ static gboolean initiate_connection(private_maemo_service_t *this,
        this->public.listener.ike_state_change = _ike_state_change;
        charon->bus->add_listener(charon->bus, &this->public.listener);
 
+       /* get an additional reference because initiate consumes one */
+       child_cfg->get_ref(child_cfg);
        if (ike_sa->initiate(ike_sa, child_cfg, 0, NULL, NULL) != SUCCESS)
        {
                DBG1(DBG_CFG, "failed to initiate tunnel");

diff --git a/src/libcharon/plugins/nm/nm_service.c b/src/libcharon/plugins/nm/nm_service.c
index 8135476..7882915 100644 (file)
--- a/src/libcharon/plugins/nm/nm_service.c
+++ b/src/libcharon/plugins/nm/nm_service.c
@@ -533,6 +533,13 @@ static gboolean connect_(NMVPNPlugin *plugin, NMConnection *connection,
         */
        ike_sa = charon->ike_sa_manager->checkout_by_config(charon->ike_sa_manager,
                                                                                                                peer_cfg);
+       if (!ike_sa)
+       {
+               peer_cfg->destroy(peer_cfg);
+               g_set_error(err, NM_VPN_PLUGIN_ERROR, NM_VPN_PLUGIN_ERROR_LAUNCH_FAILED,
+                                       "IKE version not supported.");
+               return FALSE;
+       }
        if (!ike_sa->get_peer_cfg(ike_sa))
        {
                ike_sa->set_peer_cfg(ike_sa, peer_cfg);
@@ -550,6 +557,7 @@ static gboolean connect_(NMVPNPlugin *plugin, NMConnection *connection,
        /**
         * Initiate
         */
+       child_cfg->get_ref(child_cfg);
        if (ike_sa->initiate(ike_sa, child_cfg, 0, NULL, NULL) != SUCCESS)
        {
                charon->bus->remove_listener(charon->bus, &priv->listener);

diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c
index 78ffbe5..442eb72 100644 (file)
--- a/src/libcharon/sa/ike_sa.c
+++ b/src/libcharon/sa/ike_sa.c
@@ -1412,6 +1412,10 @@ METHOD(ike_sa_t, reestablish, status_t,
 
        new = charon->ike_sa_manager->checkout_new(charon->ike_sa_manager,
                                                                                           this->version, TRUE);
+       if (!new)
+       {
+               return FAILED;
+       }
        new->set_peer_cfg(new, this->peer_cfg);
        host = this->other_host;
        new->set_other_host(new, host->clone(host));
@@ -1802,7 +1806,7 @@ METHOD(ike_sa_t, destroy, void,
        charon->bus->set_sa(charon->bus, &this->public);
 
        set_state(this, IKE_DESTROYING);
-       this->task_manager->destroy(this->task_manager);
+       DESTROY_IF(this->task_manager);
 
        /* remove attributes first, as we pass the IKE_SA to the handler */
        while (this->attributes->remove_last(this->attributes,
@@ -1820,7 +1824,7 @@ METHOD(ike_sa_t, destroy, void,
        /* unset SA after here to avoid usage by the listeners */
        charon->bus->set_sa(charon->bus, NULL);
 
-       this->keymat->destroy(this->keymat);
+       DESTROY_IF(this->keymat);
 
        if (this->my_virtual_ip)
        {
@@ -1881,6 +1885,15 @@ ike_sa_t * ike_sa_create(ike_sa_id_t *ike_sa_id, bool initiator,
        private_ike_sa_t *this;
        static u_int32_t unique_id = 0;
 
+       if (version == IKE_ANY)
+       {       /* prefer IKEv2 if protocol not specified */
+#ifdef USE_IKEV2
+               version = IKEV2;
+#else
+               version = IKEV1;
+#endif
+       }
+
        INIT(this,
                .public = {
                        .get_version = _get_version,
@@ -1991,8 +2004,11 @@ ike_sa_t * ike_sa_create(ike_sa_id_t *ike_sa_id, bool initiator,
        this->task_manager = task_manager_create(&this->public);
        this->my_host->set_port(this->my_host, IKEV2_UDP_PORT);
 
-       /* TODO-IKEv1: check if keymat and task manager created successfully.
-        * Return NULL otherwise? */
-
+       if (!this->task_manager || !this->keymat)
+       {
+               DBG1(DBG_IKE, "IKE version %d not supported", this->version);
+               destroy(this);
+               return NULL;
+       }
        return &this->public;
 }

diff --git a/src/libcharon/sa/ike_sa_manager.c b/src/libcharon/sa/ike_sa_manager.c
index 776b2b7..d992ce1 100644 (file)
--- a/src/libcharon/sa/ike_sa_manager.c
+++ b/src/libcharon/sa/ike_sa_manager.c
@@ -955,9 +955,11 @@ METHOD(ike_sa_manager_t, checkout_new, ike_sa_t*,
        ike_sa = ike_sa_create(ike_sa_id, initiator, version);
        ike_sa_id->destroy(ike_sa_id);
 
-       DBG2(DBG_MGR, "created IKE_SA %s[%u]", ike_sa->get_name(ike_sa),
-                       ike_sa->get_unique_id(ike_sa));
-
+       if (ike_sa)
+       {
+               DBG2(DBG_MGR, "created IKE_SA %s[%u]", ike_sa->get_name(ike_sa),
+                        ike_sa->get_unique_id(ike_sa));
+       }
        return ike_sa;
 }
 
@@ -1033,23 +1035,26 @@ METHOD(ike_sa_manager_t, checkout_by_message, ike_sa_t*,
                {
                        /* no IKE_SA found, create a new one */
                        id->set_responder_spi(id, get_spi(this));
-                       entry = entry_create();
-                       /* a new SA checked out by message is a responder SA */
-                       entry->ike_sa = ike_sa_create(id, FALSE, ike_version);
-                       entry->ike_sa_id = id->clone(id);
+                       ike_sa = ike_sa_create(id, FALSE, ike_version);
+                       if (ike_sa)
+                       {
+                               entry = entry_create();
+                               /* a new SA checked out by message is a responder SA */
+                               entry->ike_sa = ike_sa;
+                               entry->ike_sa_id = id->clone(id);
 
-                       segment = put_entry(this, entry);
-                       entry->checked_out = TRUE;
-                       unlock_single_segment(this, segment);
+                               segment = put_entry(this, entry);
+                               entry->checked_out = TRUE;
+                               unlock_single_segment(this, segment);
 
-                       entry->message_id = message->get_message_id(message);
-                       entry->init_hash = hash;
-                       ike_sa = entry->ike_sa;
+                               entry->message_id = message->get_message_id(message);
+                               entry->init_hash = hash;
 
-                       DBG2(DBG_MGR, "created IKE_SA %s[%u]",
-                                       ike_sa->get_name(ike_sa), ike_sa->get_unique_id(ike_sa));
+                               DBG2(DBG_MGR, "created IKE_SA %s[%u]",
+                                        ike_sa->get_name(ike_sa), ike_sa->get_unique_id(ike_sa));
+                       }
                }
-               else
+               if (ike_sa == NULL)
                {
                        chunk_free(&hash);
                        DBG1(DBG_MGR, "ignoring message, no such IKE_SA");

diff --git a/src/libcharon/sa/ikev2/tasks/ike_reauth.c b/src/libcharon/sa/ikev2/tasks/ike_reauth.c
index d9f3fe8..8371b8b 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/ike_reauth.c
+++ b/src/libcharon/sa/ikev2/tasks/ike_reauth.c
@@ -54,7 +54,6 @@ METHOD(task_t, process_i, status_t,
        ike_sa_t *new;
        host_t *host;
        enumerator_t *enumerator;
-       ike_version_t version;
        child_sa_t *child_sa;
        peer_cfg_t *peer_cfg;
 
@@ -75,9 +74,12 @@ METHOD(task_t, process_i, status_t,
                return FAILED;
        }
 
-       version = this->ike_sa->get_version(this->ike_sa);
-       new = charon->ike_sa_manager->checkout_new(charon->ike_sa_manager, version,
-                                                                                          TRUE);
+       new = charon->ike_sa_manager->checkout_new(charon->ike_sa_manager,
+                                                               this->ike_sa->get_version(this->ike_sa), TRUE);
+       if (!new)
+       {       /* shouldn't happen */
+               return FAILED;
+       }
 
        new->set_peer_cfg(new, peer_cfg);
        host = this->ike_sa->get_other_host(this->ike_sa);

diff --git a/src/libcharon/sa/ikev2/tasks/ike_rekey.c b/src/libcharon/sa/ikev2/tasks/ike_rekey.c
index 2cfcdc1..c3c6cf0 100644 (file)
--- a/src/libcharon/sa/ikev2/tasks/ike_rekey.c
+++ b/src/libcharon/sa/ikev2/tasks/ike_rekey.c
@@ -123,16 +123,20 @@ METHOD(task_t, process_i_delete, status_t,
 METHOD(task_t, build_i, status_t,
        private_ike_rekey_t *this, message_t *message)
 {
+       ike_version_t version;
        peer_cfg_t *peer_cfg;
        host_t *other_host;
 
        /* create new SA only on first try */
        if (this->new_sa == NULL)
        {
-               ike_version_t version = this->ike_sa->get_version(this->ike_sa);
+               version = this->ike_sa->get_version(this->ike_sa);
                this->new_sa = charon->ike_sa_manager->checkout_new(
                                                                                charon->ike_sa_manager, version, TRUE);
-
+               if (!this->new_sa)
+               {       /* shouldn't happen */
+                       return FAILED;
+               }
                peer_cfg = this->ike_sa->get_peer_cfg(this->ike_sa);
                other_host = this->ike_sa->get_other_host(this->ike_sa);
                this->new_sa->set_peer_cfg(this->new_sa, peer_cfg);
@@ -149,7 +153,6 @@ METHOD(task_t, process_r, status_t,
        private_ike_rekey_t *this, message_t *message)
 {
        enumerator_t *enumerator;
-       ike_version_t version;
        peer_cfg_t *peer_cfg;
        child_sa_t *child_sa;
 
@@ -177,9 +180,12 @@ METHOD(task_t, process_r, status_t,
        }
        enumerator->destroy(enumerator);
 
-       version = this->ike_sa->get_version(this->ike_sa);
        this->new_sa = charon->ike_sa_manager->checkout_new(charon->ike_sa_manager,
-                                                                                                               version, FALSE);
+                                                       this->ike_sa->get_version(this->ike_sa), FALSE);
+       if (!this->new_sa)
+       {       /* shouldn't happen */
+               return FAILED;
+       }
 
        peer_cfg = this->ike_sa->get_peer_cfg(this->ike_sa);
        this->new_sa->set_peer_cfg(this->new_sa, peer_cfg);

diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c
index bf9f843..3f434da 100644 (file)
--- a/src/libcharon/sa/trap_manager.c
+++ b/src/libcharon/sa/trap_manager.c
@@ -274,21 +274,24 @@ METHOD(trap_manager_t, acquire, void,
                peer = found->peer_cfg;
                ike_sa = charon->ike_sa_manager->checkout_by_config(
                                                                                                charon->ike_sa_manager, peer);
-               if (ike_sa->get_peer_cfg(ike_sa) == NULL)
+               if (ike_sa)
                {
-                       ike_sa->set_peer_cfg(ike_sa, peer);
-               }
-               child->get_ref(child);
-               reqid = found->child_sa->get_reqid(found->child_sa);
-               if (ike_sa->initiate(ike_sa, child, reqid, src, dst) != DESTROY_ME)
-               {
-                       found->pending = ike_sa;
-                       charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa);
-               }
-               else
-               {
-                       charon->ike_sa_manager->checkin_and_destroy(
+                       if (ike_sa->get_peer_cfg(ike_sa) == NULL)
+                       {
+                               ike_sa->set_peer_cfg(ike_sa, peer);
+                       }
+                       child->get_ref(child);
+                       reqid = found->child_sa->get_reqid(found->child_sa);
+                       if (ike_sa->initiate(ike_sa, child, reqid, src, dst) != DESTROY_ME)
+                       {
+                               found->pending = ike_sa;
+                               charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa);
+                       }
+                       else
+                       {
+                               charon->ike_sa_manager->checkin_and_destroy(
                                                                                                charon->ike_sa_manager, ike_sa);
+                       }
                }
        }
        this->lock->unlock(this->lock);