Handle initiation of not supported IKE versions properly

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)

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");

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");

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);

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;
 }

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");

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);

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);

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);