Merge branch 'dh-checks'

Extend the diffie-hellman interface by success return values, and do some
basic length checks for DH public values.

scripts/dh_speed.c

@@ -15,6 +15,7 @@
 
 #include <stdio.h>
 #include <time.h>
+#include <assert.h>
 #include <library.h>
 #include <utils/debug.h>
 #include <crypto/diffie_hellman.h>
@@ -88,16 +89,16 @@ static void run_test(diffie_hellman_group_t group, int rounds)
 
 	for (round = 0; round < rounds; round++)
 	{
-		l[round]->get_my_public_value(l[round], &chunk);
-		r->set_other_public_value(r, chunk);
+		assert(l[round]->get_my_public_value(l[round], &chunk));
+		assert(r->set_other_public_value(r, chunk));
 		chunk_free(&chunk);
 	}
 
-	r->get_my_public_value(r, &chunk);
+	assert(r->get_my_public_value(r, &chunk));
 	start_timing(&timing);
 	for (round = 0; round < rounds; round++)
 	{
-		l[round]->set_other_public_value(l[round], chunk);
+		assert(l[round]->set_other_public_value(l[round], chunk));
 	}
 	printf(" | S = B^a/s: %8.1f\n", rounds / end_timing(&timing));
 	chunk_free(&chunk);
@@ -143,4 +144,3 @@ int main(int argc, char *argv[])
 	}
 	return 0;
 }
-

src/charon-tkm/src/tkm/tkm_diffie_hellman.c

@@ -55,30 +55,29 @@ struct private_tkm_diffie_hellman_t {
 
 };
 
-METHOD(diffie_hellman_t, get_my_public_value, void,
+METHOD(diffie_hellman_t, get_my_public_value, bool,
 	private_tkm_diffie_hellman_t *this, chunk_t *value)
 {
 	sequence_to_chunk(this->pubvalue.data, this->pubvalue.size, value);
+	return TRUE;
 }
 
-METHOD(diffie_hellman_t, get_shared_secret, status_t,
+METHOD(diffie_hellman_t, get_shared_secret, bool,
 	private_tkm_diffie_hellman_t *this, chunk_t *secret)
 {
 	*secret = chunk_empty;
-	return SUCCESS;
+	return TRUE;
 }
 
 
-METHOD(diffie_hellman_t, set_other_public_value, void,
+METHOD(diffie_hellman_t, set_other_public_value, bool,
 	private_tkm_diffie_hellman_t *this, chunk_t value)
 {
-	// TODO: unvoid this function
-
 	dh_pubvalue_type othervalue;
 	othervalue.size = value.len;
 	memcpy(&othervalue.data, value.ptr, value.len);
 
-	ike_dh_generate_key(this->context_id, othervalue);
+	return ike_dh_generate_key(this->context_id, othervalue) == TKM_OK;
 }
 
 METHOD(diffie_hellman_t, get_dh_group, diffie_hellman_group_t,

src/charon-tkm/tests/diffie_hellman_tests.c

@@ -40,7 +40,7 @@ START_TEST(test_dh_get_my_pubvalue)
 	fail_if(!dh, "Unable to create DH");
 
 	chunk_t value;
-	dh->dh.get_my_public_value(&dh->dh, &value);
+	ck_assert(dh->dh.get_my_public_value(&dh->dh, &value));
 	dh->dh.destroy(&dh->dh);
 
 	fail_if(value.ptr == NULL, "Pubvalue is NULL");

src/charon-tkm/tests/keymat_tests.c

@@ -53,8 +53,8 @@ START_TEST(test_derive_ike_keys)
 
 	/* Use the same pubvalue for both sides */
 	chunk_t pubvalue;
-	dh->dh.get_my_public_value(&dh->dh, &pubvalue);
-	dh->dh.set_other_public_value(&dh->dh, pubvalue);
+	ck_assert(dh->dh.get_my_public_value(&dh->dh, &pubvalue));
+	ck_assert(dh->dh.set_other_public_value(&dh->dh, pubvalue));
 
 	fail_unless(keymat->keymat_v2.derive_ike_keys(&keymat->keymat_v2, proposal,
 				&dh->dh, nonce, nonce, ike_sa_id, PRF_UNDEFINED, chunk_empty),

src/libcharon/encoding/payloads/ke_payload.c

@@ -142,79 +142,6 @@ static encoding_rule_t encodings_v1[] = {
 METHOD(payload_t, verify, status_t,
 	private_ke_payload_t *this)
 {
-	diffie_hellman_params_t *params;
-	diffie_hellman_group_t g = this->dh_group_number;
-	bool valid = TRUE;
-
-	if (this->type == PLV1_KEY_EXCHANGE)
-	{
-		/* IKEv1 does not transmit the group */
-		return SUCCESS;
-	}
-
-	switch (g)
-	{
-		case MODP_NONE:
-			valid = FALSE;
-			break;
-		case MODP_768_BIT:
-		case MODP_1024_BIT:
-		case MODP_1536_BIT:
-		case MODP_2048_BIT:
-		case MODP_3072_BIT:
-		case MODP_4096_BIT:
-		case MODP_6144_BIT:
-		case MODP_8192_BIT:
-		case MODP_1024_160:
-		case MODP_2048_224:
-		case MODP_2048_256:
-			params = diffie_hellman_get_params(g);
-			if (params)
-			{
-				valid = this->key_exchange_data.len == params->prime.len;
-			}
-			break;
-		case ECP_192_BIT:
-			valid = this->key_exchange_data.len == 48;
-			break;
-		case ECP_224_BIT:
-		case ECP_224_BP:
-			valid = this->key_exchange_data.len == 56;
-			break;
-		case ECP_256_BIT:
-		case ECP_256_BP:
-			valid = this->key_exchange_data.len == 64;
-			break;
-		case ECP_384_BIT:
-		case ECP_384_BP:
-			valid = this->key_exchange_data.len == 96;
-			break;
-		case ECP_512_BP:
-			valid = this->key_exchange_data.len == 128;
-			break;
-		case ECP_521_BIT:
-			valid = this->key_exchange_data.len == 132;
-			break;
-		case NTRU_112_BIT:
-		case NTRU_128_BIT:
-		case NTRU_192_BIT:
-		case NTRU_256_BIT:
-			/* NTRU public key size depends on the parameter set, but is
-			 * at least 512 bytes */
-			valid = this->key_exchange_data.len > 512;
-			break;
-		case MODP_NULL:
-		case MODP_CUSTOM:
-			break;
-		/* compile-warn unhandled groups, but accept them so we can negotiate
-		 * a different group that we support. */
-	}
-	if (!valid)
-	{
-		DBG1(DBG_ENC, "invalid KE data size (%zu bytes) for %N",
-			 this->key_exchange_data.len, diffie_hellman_group_names, g);
-		return FAILED;
-	}
 	return SUCCESS;
 }
 
@@ -320,9 +247,15 @@ ke_payload_t *ke_payload_create(payload_type_t type)
 ke_payload_t *ke_payload_create_from_diffie_hellman(payload_type_t type,
 													diffie_hellman_t *dh)
 {
-	private_ke_payload_t *this = (private_ke_payload_t*)ke_payload_create(type);
+	private_ke_payload_t *this;
+	chunk_t value;
 
-	dh->get_my_public_value(dh, &this->key_exchange_data);
+	if (!dh->get_my_public_value(dh, &value))
+	{
+		return NULL;
+	}
+	this = (private_ke_payload_t*)ke_payload_create(type);
+	this->key_exchange_data = value;
 	this->dh_group_number = dh->get_dh_group(dh);
 	this->payload_length += this->key_exchange_data.len;
 

src/libcharon/encoding/payloads/ke_payload.h

@@ -73,7 +73,7 @@ ke_payload_t *ke_payload_create(payload_type_t type);
  *
  * @param type		PLV2_KEY_EXCHANGE or PLV1_KEY_EXCHANGE
  * @param dh		diffie hellman object containing group and key
- * @return 			ke_payload_t object
+ * @return 			ke_payload_t object, NULL on error
  */
 ke_payload_t *ke_payload_create_from_diffie_hellman(payload_type_t type,
 													diffie_hellman_t *dh);

src/libcharon/plugins/ha/ha_child.c

@@ -97,7 +97,7 @@ METHOD(listener_t, child_keys, bool,
 	}
 	m->add_attribute(m, HA_NONCE_I, nonce_i);
 	m->add_attribute(m, HA_NONCE_R, nonce_r);
-	if (dh && dh->get_shared_secret(dh, &secret) == SUCCESS)
+	if (dh && dh->get_shared_secret(dh, &secret))
 	{
 		m->add_attribute(m, HA_SECRET, secret);
 		chunk_clear(&secret);

src/libcharon/plugins/ha/ha_dispatcher.c

@@ -81,17 +81,18 @@ struct ha_diffie_hellman_t {
 	chunk_t pub;
 };
 
-METHOD(diffie_hellman_t, dh_get_shared_secret, status_t,
+METHOD(diffie_hellman_t, dh_get_shared_secret, bool,
 	ha_diffie_hellman_t *this, chunk_t *secret)
 {
 	*secret = chunk_clone(this->secret);
-	return SUCCESS;
+	return TRUE;
 }
 
-METHOD(diffie_hellman_t, dh_get_my_public_value, void,
+METHOD(diffie_hellman_t, dh_get_my_public_value, bool,
 	ha_diffie_hellman_t *this, chunk_t *value)
 {
 	*value = chunk_clone(this->pub);
+	return TRUE;
 }
 
 METHOD(diffie_hellman_t, dh_destroy, void,

src/libcharon/plugins/ha/ha_ike.c

@@ -84,7 +84,7 @@ METHOD(listener_t, ike_keys, bool,
 	{	/* do not sync SA between nodes */
 		return TRUE;
 	}
-	if (dh->get_shared_secret(dh, &secret) != SUCCESS)
+	if (!dh->get_shared_secret(dh, &secret))
 	{
 		return TRUE;
 	}
@@ -127,9 +127,11 @@ METHOD(listener_t, ike_keys, bool,
 	chunk_clear(&secret);
 	if (ike_sa->get_version(ike_sa) == IKEV1)
 	{
-		dh->get_my_public_value(dh, &secret);
-		m->add_attribute(m, HA_LOCAL_DH, secret);
-		chunk_free(&secret);
+		if (dh->get_my_public_value(dh, &secret))
+		{
+			m->add_attribute(m, HA_LOCAL_DH, secret);
+			chunk_free(&secret);
+		}
 		m->add_attribute(m, HA_REMOTE_DH, dh_other);
 		if (shared)
 		{

src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c

@@ -15,33 +15,38 @@
 
 #include "load_tester_diffie_hellman.h"
 
-/**
- * Implementation of gmp_diffie_hellman_t.get_my_public_value.
- */
-static void get_my_public_value(load_tester_diffie_hellman_t *this,
-								chunk_t *value)
+METHOD(diffie_hellman_t, get_my_public_value, bool,
+	load_tester_diffie_hellman_t *this, chunk_t *value)
 {
 	*value = chunk_empty;
+	return TRUE;
 }
 
-/**
- * Implementation of gmp_diffie_hellman_t.get_shared_secret.
- */
-static status_t get_shared_secret(load_tester_diffie_hellman_t *this,
-								  chunk_t *secret)
+METHOD(diffie_hellman_t, set_other_public_value, bool,
+	load_tester_diffie_hellman_t *this, chunk_t value)
+{
+	return TRUE;
+}
+
+METHOD(diffie_hellman_t, get_shared_secret, bool,
+	load_tester_diffie_hellman_t *this, chunk_t *secret)
 {
 	*secret = chunk_empty;
-	return SUCCESS;
+	return TRUE;
 }
 
-/**
- * Implementation of gmp_diffie_hellman_t.get_dh_group.
- */
-static diffie_hellman_group_t get_dh_group(load_tester_diffie_hellman_t *this)
+METHOD(diffie_hellman_t, get_dh_group, diffie_hellman_group_t,
+	load_tester_diffie_hellman_t *this)
 {
 	return MODP_NULL;
 }
 
+METHOD(diffie_hellman_t, destroy, void,
+	load_tester_diffie_hellman_t *this)
+{
+	free(this);
+}
+
 /**
  * See header
  */
@@ -55,13 +60,15 @@ load_tester_diffie_hellman_t *load_tester_diffie_hellman_create(
 		return NULL;
 	}
 
-	this = malloc_thing(load_tester_diffie_hellman_t);
-
-	this->dh.get_shared_secret = (status_t (*)(diffie_hellman_t *, chunk_t *))get_shared_secret;
-	this->dh.set_other_public_value = (void (*)(diffie_hellman_t *, chunk_t ))nop;
-	this->dh.get_my_public_value = (void (*)(diffie_hellman_t *, chunk_t *))get_my_public_value;
-	this->dh.get_dh_group = (diffie_hellman_group_t (*)(diffie_hellman_t *))get_dh_group;
-	this->dh.destroy = (void (*)(diffie_hellman_t *))free;
+	INIT(this,
+		.dh = {
+			.get_shared_secret = _get_shared_secret,
+			.set_other_public_value = _set_other_public_value,
+			.get_my_public_value = _get_my_public_value,
+			.get_dh_group = _get_dh_group,
+			.destroy = _destroy,
+		}
+	);
 
 	return this;
 }

src/libcharon/sa/ikev1/authenticators/psk_v1_authenticator.c

@@ -74,7 +74,10 @@ METHOD(authenticator_t, build, status_t,
 	keymat_v1_t *keymat;
 	chunk_t hash, dh;
 
-	this->dh->get_my_public_value(this->dh, &dh);
+	if (!this->dh->get_my_public_value(this->dh, &dh))
+	{
+		return FAILED;
+	}
 	keymat = (keymat_v1_t*)this->ike_sa->get_keymat(this->ike_sa);
 	if (!keymat->get_hash(keymat, this->initiator, dh, this->dh_value,
 					this->ike_sa->get_id(this->ike_sa), this->sa_payload,
@@ -108,7 +111,10 @@ METHOD(authenticator_t, process, status_t,
 		return FAILED;
 	}
 
-	this->dh->get_my_public_value(this->dh, &dh);
+	if (!this->dh->get_my_public_value(this->dh, &dh))
+	{
+		return FAILED;
+	}
 	keymat = (keymat_v1_t*)this->ike_sa->get_keymat(this->ike_sa);
 	if (!keymat->get_hash(keymat, !this->initiator, this->dh_value, dh,
 					this->ike_sa->get_id(this->ike_sa), this->sa_payload,

src/libcharon/sa/ikev1/authenticators/pubkey_v1_authenticator.c

@@ -94,7 +94,11 @@ METHOD(authenticator_t, build, status_t,
 		return NOT_FOUND;
 	}
 
-	this->dh->get_my_public_value(this->dh, &dh);
+	if (!this->dh->get_my_public_value(this->dh, &dh))
+	{
+		private->destroy(private);
+		return FAILED;
+	}
 	keymat = (keymat_v1_t*)this->ike_sa->get_keymat(this->ike_sa);
 	if (!keymat->get_hash(keymat, this->initiator, dh, this->dh_value,
 					this->ike_sa->get_id(this->ike_sa), this->sa_payload,
@@ -152,7 +156,10 @@ METHOD(authenticator_t, process, status_t,
 	}
 
 	id = this->ike_sa->get_other_id(this->ike_sa);
-	this->dh->get_my_public_value(this->dh, &dh);
+	if (!this->dh->get_my_public_value(this->dh, &dh))
+	{
+		return FAILED;
+	}
 	keymat = (keymat_v1_t*)this->ike_sa->get_keymat(this->ike_sa);
 	if (!keymat->get_hash(keymat, !this->initiator, this->dh_value, dh,
 					this->ike_sa->get_id(this->ike_sa), this->sa_payload,

src/libcharon/sa/ikev1/keymat_v1.c

@@ -425,7 +425,7 @@ METHOD(keymat_v1_t, derive_ike_keys, bool,
 		return FALSE;
 	}
 
-	if (dh->get_shared_secret(dh, &g_xy) != SUCCESS)
+	if (!dh->get_shared_secret(dh, &g_xy))
 	{
 		return FALSE;
 	}
@@ -560,7 +560,10 @@ METHOD(keymat_v1_t, derive_ike_keys, bool,
 		return FALSE;
 	}
 
-	dh->get_my_public_value(dh, &dh_me);
+	if (!dh->get_my_public_value(dh, &dh_me))
+	{
+		return FALSE;
+	}
 	g_xi = this->initiator ? dh_me : dh_other;
 	g_xr = this->initiator ? dh_other : dh_me;
 
@@ -661,7 +664,7 @@ METHOD(keymat_v1_t, derive_child_keys, bool,
 	protocol = proposal->get_protocol(proposal);
 	if (dh)
 	{
-		if (dh->get_shared_secret(dh, &secret) != SUCCESS)
+		if (!dh->get_shared_secret(dh, &secret))
 		{
 			return FALSE;
 		}

src/libcharon/sa/ikev1/phase1.c

@@ -694,7 +694,13 @@ METHOD(phase1_t, add_nonce_ke, bool,
 	nonce_gen_t *nonceg;
 	chunk_t nonce;
 
-	ke_payload = ke_payload_create_from_diffie_hellman(PLV1_KEY_EXCHANGE, this->dh);
+	ke_payload = ke_payload_create_from_diffie_hellman(PLV1_KEY_EXCHANGE,
+													   this->dh);
+	if (!ke_payload)
+	{
+		DBG1(DBG_IKE, "creating KE payload failed");
+		return FALSE;
+	}
 	message->add_payload(message, &ke_payload->payload_interface);
 
 	nonceg = this->keymat->keymat.create_nonce_gen(&this->keymat->keymat);
@@ -739,7 +745,11 @@ METHOD(phase1_t, get_nonce_ke, bool,
 		return FALSE;
 	}
 	this->dh_value = chunk_clone(ke_payload->get_key_exchange_data(ke_payload));
-	this->dh->set_other_public_value(this->dh, this->dh_value);
+	if (!this->dh->set_other_public_value(this->dh, this->dh_value))
+	{
+		DBG1(DBG_IKE, "unable to apply received KE value");
+		return FALSE;
+	}
 
 	nonce_payload = (nonce_payload_t*)message->get_payload(message, PLV1_NONCE);
 	if (!nonce_payload)

src/libcharon/sa/ikev1/tasks/quick_mode.c

@@ -465,12 +465,19 @@ static bool get_nonce(private_quick_mode_t *this, chunk_t *nonce,
 /**
  * Add KE payload to message
  */
-static void add_ke(private_quick_mode_t *this, message_t *message)
+static bool add_ke(private_quick_mode_t *this, message_t *message)
 {
 	ke_payload_t *ke_payload;
 
-	ke_payload = ke_payload_create_from_diffie_hellman(PLV1_KEY_EXCHANGE, this->dh);
+	ke_payload = ke_payload_create_from_diffie_hellman(PLV1_KEY_EXCHANGE,
+													   this->dh);
+	if (!ke_payload)
+	{
+		DBG1(DBG_IKE, "creating KE payload failed");
+		return FALSE;
+	}
 	message->add_payload(message, &ke_payload->payload_interface);
+	return TRUE;
 }
 
 /**
@@ -486,8 +493,12 @@ static bool get_ke(private_quick_mode_t *this, message_t *message)
 		DBG1(DBG_IKE, "KE payload missing");
 		return FALSE;
 	}
-	this->dh->set_other_public_value(this->dh,
-								ke_payload->get_key_exchange_data(ke_payload));
+	if (this->dh->set_other_public_value(this->dh,
+								ke_payload->get_key_exchange_data(ke_payload)))
+	{
+		DBG1(DBG_IKE, "unable to apply received KE value");
+		return FALSE;
+	}
 	return TRUE;
 }
 
@@ -880,7 +891,10 @@ METHOD(task_t, build_i, status_t,
 			}
 			if (group != MODP_NONE)
 			{
-				add_ke(this, message);
+				if (!add_ke(this, message))
+				{
+					return FAILED;
+				}
 			}
 			if (!this->tsi)
 			{
@@ -1218,7 +1232,10 @@ METHOD(task_t, build_r, status_t,
 			}
 			if (this->dh)
 			{
-				add_ke(this, message);
+				if (!add_ke(this, message))
+				{
+					return FAILED;
+				}
 			}
 
 			add_ts(this, message);

src/libcharon/sa/ikev2/keymat_v2.c

@@ -300,7 +300,7 @@ METHOD(keymat_v2_t, derive_ike_keys, bool,
 	spi_i = chunk_alloca(sizeof(u_int64_t));
 	spi_r = chunk_alloca(sizeof(u_int64_t));
 
-	if (dh->get_shared_secret(dh, &secret) != SUCCESS)
+	if (!dh->get_shared_secret(dh, &secret))
 	{
 		return FALSE;
 	}
@@ -554,7 +554,7 @@ METHOD(keymat_v2_t, derive_child_keys, bool,
 
 	if (dh)
 	{
-		if (dh->get_shared_secret(dh, &secret) != SUCCESS)
+		if (!dh->get_shared_secret(dh, &secret))
 		{
 			return FALSE;
 		}

src/libcharon/sa/ikev2/tasks/child_create.c

@@ -104,6 +104,11 @@ struct private_child_create_t {
 	 */
 	diffie_hellman_t *dh;
 
+	/**
+	 * Applying DH public value failed?
+	 */
+	bool dh_failed;
+
 	/**
 	 * group used for DH exchange
 	 */
@@ -716,7 +721,7 @@ static status_t select_and_install(private_child_create_t *this,
 /**
  * build the payloads for the message
  */
-static void build_payloads(private_child_create_t *this, message_t *message)
+static bool build_payloads(private_child_create_t *this, message_t *message)
 {
 	sa_payload_t *sa_payload;
 	nonce_payload_t *nonce_payload;
@@ -748,6 +753,11 @@ static void build_payloads(private_child_create_t *this, message_t *message)
 	{
 		ke_payload = ke_payload_create_from_diffie_hellman(PLV2_KEY_EXCHANGE,
 														   this->dh);
+		if (!ke_payload)
+		{
+			DBG1(DBG_IKE, "creating KE payload failed");
+			return FALSE;
+		}
 		message->add_payload(message, (payload_t*)ke_payload);
 	}
 
@@ -776,6 +786,7 @@ static void build_payloads(private_child_create_t *this, message_t *message)
 		message->add_notify(message, FALSE, ESP_TFC_PADDING_NOT_SUPPORTED,
 							chunk_empty);
 	}
+	return TRUE;
 }
 
 /**
@@ -887,7 +898,7 @@ static void process_payloads(private_child_create_t *this, message_t *message)
 				}
 				if (this->dh)
 				{
-					this->dh->set_other_public_value(this->dh,
+					this->dh_failed = !this->dh->set_other_public_value(this->dh,
 								ke_payload->get_key_exchange_data(ke_payload));
 				}
 				break;
@@ -1035,7 +1046,10 @@ METHOD(task_t, build_i, status_t,
 							NARROW_INITIATOR_PRE_AUTH, this->tsi, this->tsr);
 	}
 
-	build_payloads(this, message);
+	if (!build_payloads(this, message))
+	{
+		return FAILED;
+	}
 
 	this->tsi->destroy_offset(this->tsi, offsetof(traffic_selector_t, destroy));
 	this->tsr->destroy_offset(this->tsr, offsetof(traffic_selector_t, destroy));
@@ -1176,12 +1190,19 @@ METHOD(task_t, build_r, status_t,
 		case IKE_SA_INIT:
 			return get_nonce(message, &this->my_nonce);
 		case CREATE_CHILD_SA:
-			if (generate_nonce(this) != SUCCESS)
+			if (generate_nonce(this) != SUCCESS )
 			{
 				message->add_notify(message, FALSE, NO_PROPOSAL_CHOSEN,
 									chunk_empty);
 				return SUCCESS;
 			}
+			if (this->dh_failed)
+			{
+				DBG1(DBG_IKE, "applying DH public value failed");
+				message->add_notify(message, FALSE, NO_PROPOSAL_CHOSEN,
+									chunk_empty);
+				return SUCCESS;
+			}
 			no_dh = FALSE;
 			break;
 		case IKE_AUTH:
@@ -1288,7 +1309,12 @@ METHOD(task_t, build_r, status_t,
 			return SUCCESS;
 	}
 
-	build_payloads(this, message);
+	if (!build_payloads(this, message))
+	{
+		message->add_notify(message, FALSE, NO_PROPOSAL_CHOSEN, chunk_empty);
+		handle_child_sa_failure(this, message);
+		return SUCCESS;
+	}
 
 	if (!this->rekey)
 	{	/* invoke the child_up() hook if we are not rekeying */
@@ -1466,6 +1492,13 @@ METHOD(task_t, process_i, status_t,
 		return delete_failed_sa(this);
 	}
 
+	if (this->dh_failed)
+	{
+		DBG1(DBG_IKE, "applying DH public value failed");
+		handle_child_sa_failure(this, message);
+		return delete_failed_sa(this);
+	}
+
 	if (select_and_install(this, no_dh, ike_auth) == SUCCESS)
 	{
 		if (!this->rekey)
@@ -1543,6 +1576,7 @@ METHOD(task_t, migrate, void,
 	DESTROY_IF(this->child_sa);
 	DESTROY_IF(this->proposal);
 	DESTROY_IF(this->dh);
+	this->dh_failed = FALSE;
 	if (this->proposals)
 	{
 		this->proposals->destroy_offset(this->proposals, offsetof(proposal_t, destroy));

src/libcharon/sa/ikev2/tasks/ike_init.c

@@ -69,6 +69,11 @@ struct private_ike_init_t {
 	 */
 	diffie_hellman_t *dh;
 
+	/**
+	 * Applying DH public value failed?
+	 */
+	bool dh_failed;
+
 	/**
 	 * Keymat derivation (from IKE_SA)
 	 */
@@ -210,7 +215,7 @@ static void handle_supported_hash_algorithms(private_ike_init_t *this,
 /**
  * build the payloads for the message
  */
-static void build_payloads(private_ike_init_t *this, message_t *message)
+static bool build_payloads(private_ike_init_t *this, message_t *message)
 {
 	sa_payload_t *sa_payload;
 	ke_payload_t *ke_payload;
@@ -254,7 +259,13 @@ static void build_payloads(private_ike_init_t *this, message_t *message)
 
 	nonce_payload = nonce_payload_create(PLV2_NONCE);
 	nonce_payload->set_nonce(nonce_payload, this->my_nonce);
-	ke_payload = ke_payload_create_from_diffie_hellman(PLV2_KEY_EXCHANGE, this->dh);
+	ke_payload = ke_payload_create_from_diffie_hellman(PLV2_KEY_EXCHANGE,
+													   this->dh);
+	if (!ke_payload)
+	{
+		DBG1(DBG_IKE, "creating KE payload failed");
+		return FALSE;
+	}
 
 	if (this->old_sa)
 	{	/* payload order differs if we are rekeying */
@@ -289,6 +300,7 @@ static void build_payloads(private_ike_init_t *this, message_t *message)
 			send_supported_hash_algorithms(this, message);
 		}
 	}
+	return TRUE;
 }
 
 /**
@@ -377,7 +389,7 @@ static void process_payloads(private_ike_init_t *this, message_t *message)
 		}
 		if (this->dh)
 		{
-			this->dh->set_other_public_value(this->dh,
+			this->dh_failed = !this->dh->set_other_public_value(this->dh,
 								ke_payload->get_key_exchange_data(ke_payload));
 		}
 	}
@@ -438,7 +450,10 @@ METHOD(task_t, build_i, status_t,
 		message->add_notify(message, FALSE, COOKIE, this->cookie);
 	}
 
-	build_payloads(this, message);
+	if (!build_payloads(this, message))
+	{
+		return FAILED;
+	}
 
 #ifdef ME
 	{
@@ -566,13 +581,24 @@ METHOD(task_t, build_r, status_t,
 		return FAILED;
 	}
 
+	if (this->dh_failed)
+	{
+		DBG1(DBG_IKE, "applying DH public value failed");
+		message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty);
+		return FAILED;
+	}
+
 	if (!derive_keys(this, this->other_nonce, this->my_nonce))
 	{
 		DBG1(DBG_IKE, "key derivation failed");
 		message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty);
 		return FAILED;
 	}
-	build_payloads(this, message);
+	if (!build_payloads(this, message))
+	{
+		message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty);
+		return FAILED;
+	}
 	return SUCCESS;
 }
 
@@ -687,6 +713,12 @@ METHOD(task_t, process_i, status_t,
 		return FAILED;
 	}
 
+	if (this->dh_failed)
+	{
+		DBG1(DBG_IKE, "applying DH public value failed");
+		return FAILED;
+	}
+
 	if (!derive_keys(this, this->my_nonce, this->other_nonce))
 	{
 		DBG1(DBG_IKE, "key derivation failed");
@@ -710,6 +742,7 @@ METHOD(task_t, migrate, void,
 	this->ike_sa = ike_sa;
 	this->keymat = (keymat_v2_t*)ike_sa->get_keymat(ike_sa);
 	this->proposal = NULL;
+	this->dh_failed = FALSE;
 	if (this->dh && this->dh->get_dh_group(this->dh) != this->dh_group)
 	{	/* reset DH value only if group changed (INVALID_KE_PAYLOAD) */
 		this->dh->destroy(this->dh);

src/libimcv/plugins/imc_attestation/imc_attestation_process.c

@@ -137,7 +137,11 @@ bool imc_attestation_process(pa_tnc_attr_t *attr, imc_msg_t *msg,
 			{
 				return FALSE;
 			}
-			pts->get_my_public_value(pts, &responder_value, &responder_nonce);
+			if (!pts->get_my_public_value(pts, &responder_value,
+										  &responder_nonce))
+			{
+				return FALSE;
+			}
 
 			/* Send DH Nonce Parameters Response attribute */
 			attr = tcg_pts_attr_dh_nonce_params_resp_create(selected_dh_group,
@@ -174,8 +178,10 @@ bool imc_attestation_process(pa_tnc_attr_t *attr, imc_msg_t *msg,
 				return FALSE;
 			}
 
-			pts->set_peer_public_value(pts, initiator_value, initiator_nonce);
-			if (!pts->calculate_secret(pts))
+
+			if (!pts->set_peer_public_value(pts, initiator_value,
+											initiator_nonce) ||
+				!pts->calculate_secret(pts))
 			{
 				return FALSE;
 			}

src/libimcv/plugins/imv_attestation/imv_attestation_build.c

@@ -69,7 +69,11 @@ bool imv_attestation_build(imv_msg_t *out_msg, imv_state_t *state,
 
 			/* Send DH nonce finish attribute */
 			selected_algorithm = pts->get_meas_algorithm(pts);
-			pts->get_my_public_value(pts, &initiator_value, &initiator_nonce);
+			if (!pts->get_my_public_value(pts, &initiator_value,
+										  &initiator_nonce))
+			{
+				return FALSE;
+			}
 			attr = tcg_pts_attr_dh_nonce_finish_create(selected_algorithm,
 											initiator_value, initiator_nonce);
 			attr->set_noskip_flag(attr, TRUE);

src/libimcv/plugins/imv_attestation/imv_attestation_process.c

@@ -134,11 +134,11 @@ bool imv_attestation_process(pa_tnc_attr_t *attr, imv_msg_t *out_msg,
 			}
 
 			responder_value = attr_cast->get_responder_value(attr_cast);
-			pts->set_peer_public_value(pts, responder_value,
-											responder_nonce);
 
 			/* Calculate secret assessment value */
-			if (!pts->calculate_secret(pts))
+			if (!pts->set_peer_public_value(pts, responder_value,
+											responder_nonce) ||
+				!pts->calculate_secret(pts))
 			{
 				return FALSE;
 			}

src/libimcv/pts/pts.c

@@ -224,17 +224,24 @@ METHOD(pts_t, create_dh_nonce, bool,
 	return TRUE;
 }
 
-METHOD(pts_t, get_my_public_value, void,
+METHOD(pts_t, get_my_public_value, bool,
 	private_pts_t *this, chunk_t *value, chunk_t *nonce)
 {
-	this->dh->get_my_public_value(this->dh, value);
+	if (!this->dh->get_my_public_value(this->dh, value))
+	{
+		return FALSE;
+	}
 	*nonce = this->is_imc ? this->responder_nonce : this->initiator_nonce;
+	return TRUE;
 }
 
-METHOD(pts_t, set_peer_public_value, void,
+METHOD(pts_t, set_peer_public_value, bool,
 	private_pts_t *this, chunk_t value, chunk_t nonce)
 {
-	this->dh->set_other_public_value(this->dh, value);
+	if (!this->dh->set_other_public_value(this->dh, value))
+	{
+		return FALSE;
+	}
 
 	nonce = chunk_clone(nonce);
 	if (this->is_imc)
@@ -245,6 +252,7 @@ METHOD(pts_t, set_peer_public_value, void,
 	{
 		this->responder_nonce = nonce;
 	}
+	return TRUE;
 }
 
 METHOD(pts_t, calculate_secret, bool,
@@ -264,7 +272,7 @@ METHOD(pts_t, calculate_secret, bool,
 	DBG3(DBG_PTS, "responder nonce: %B", &this->responder_nonce);
 
 	/* Calculate the DH secret */
-	if (this->dh->get_shared_secret(this->dh, &shared_secret) != SUCCESS)
+	if (!this->dh->get_shared_secret(this->dh, &shared_secret))
 	{
 		DBG1(DBG_PTS, "shared DH secret computation failed");
 		return FALSE;

src/libimcv/pts/pts.h

@@ -143,16 +143,18 @@ struct pts_t {
 	 *
 	 * @param value				My public DH value
 	 * @param nonce				My DH nonce
+	 * @return					TRUE if public value retrieved successfully
 	 */
-	void (*get_my_public_value)(pts_t *this, chunk_t *value, chunk_t *nonce);
+	bool (*get_my_public_value)(pts_t *this, chunk_t *value, chunk_t *nonce);
 
 	/**
 	 * Set peer Diffie.Hellman public value
 	 *
 	 * @param value				Peer public DH value
 	 * @param nonce				Peer DH nonce
+	 * @return					TRUE if public value set successfully
 	 */
-	void (*set_peer_public_value) (pts_t *this, chunk_t value, chunk_t nonce);
+	bool (*set_peer_public_value) (pts_t *this, chunk_t value, chunk_t nonce);
 
 	/**
 	 * Calculates assessment secret to be used for TPM Quote as ExternalData

src/libstrongswan/crypto/diffie_hellman.c

@@ -501,3 +501,75 @@ bool diffie_hellman_group_is_ec(diffie_hellman_group_t group)
 			return FALSE;
 	}
 }
+
+/**
+ * See header.
+ */
+bool diffie_hellman_verify_value(diffie_hellman_group_t group, chunk_t value)
+{
+	diffie_hellman_params_t *params;
+	bool valid = FALSE;
+
+	switch (group)
+	{
+		case MODP_768_BIT:
+		case MODP_1024_BIT:
+		case MODP_1536_BIT:
+		case MODP_2048_BIT:
+		case MODP_3072_BIT:
+		case MODP_4096_BIT:
+		case MODP_6144_BIT:
+		case MODP_8192_BIT:
+		case MODP_1024_160:
+		case MODP_2048_224:
+		case MODP_2048_256:
+			params = diffie_hellman_get_params(group);
+			if (params)
+			{
+				valid = value.len == params->prime.len;
+			}
+			break;
+		case ECP_192_BIT:
+			valid = value.len == 48;
+			break;
+		case ECP_224_BIT:
+		case ECP_224_BP:
+			valid = value.len == 56;
+			break;
+		case ECP_256_BIT:
+		case ECP_256_BP:
+			valid = value.len == 64;
+			break;
+		case ECP_384_BIT:
+		case ECP_384_BP:
+			valid = value.len == 96;
+			break;
+		case ECP_512_BP:
+			valid = value.len == 128;
+			break;
+		case ECP_521_BIT:
+			valid = value.len == 132;
+			break;
+		case NTRU_112_BIT:
+		case NTRU_128_BIT:
+		case NTRU_192_BIT:
+		case NTRU_256_BIT:
+			/* verification currently not supported, do in plugin */
+			valid = FALSE;
+			break;
+		case MODP_NULL:
+		case MODP_CUSTOM:
+			valid = TRUE;
+			break;
+		case MODP_NONE:
+			/* fail */
+			break;
+		/* compile-warn unhandled groups, fail verification */
+	}
+	if (!valid)
+	{
+		DBG1(DBG_ENC, "invalid DH public value size (%zu bytes) for %N",
+			 value.len, diffie_hellman_group_names, group);
+	}
+	return valid;
+}

src/libstrongswan/crypto/diffie_hellman.h

@@ -89,9 +89,10 @@ struct diffie_hellman_t {
 	 * Space for returned secret is allocated and must be freed by the caller.
 	 *
 	 * @param secret	shared secret will be written into this chunk
-	 * @return			SUCCESS, FAILED if not both DH values are set
+	 * @return			TRUE if shared secret computed successfully
 	 */
-	status_t (*get_shared_secret) (diffie_hellman_t *this, chunk_t *secret);
+	bool (*get_shared_secret)(diffie_hellman_t *this, chunk_t *secret)
+		__attribute__((warn_unused_result));
 
 	/**
 	 * Sets the public value of partner.
@@ -99,8 +100,10 @@ struct diffie_hellman_t {
 	 * Chunk gets cloned and can be destroyed afterwards.
 	 *
 	 * @param value		public value of partner
+	 * @return			TRUE if other public value verified and set
 	 */
-	void (*set_other_public_value) (diffie_hellman_t *this, chunk_t value);
+	bool (*set_other_public_value)(diffie_hellman_t *this, chunk_t value)
+		__attribute__((warn_unused_result));
 
 	/**
 	 * Gets the own public value to transmit.
@@ -108,8 +111,10 @@ struct diffie_hellman_t {
 	 * Space for returned chunk is allocated and must be freed by the caller.
 	 *
 	 * @param value		public value of caller is stored at this location
+	 * @return			TRUE if public value retrieved
 	 */
-	void (*get_my_public_value) (diffie_hellman_t *this, chunk_t *value);
+	bool (*get_my_public_value) (diffie_hellman_t *this, chunk_t *value)
+		__attribute__((warn_unused_result));
 
 	/**
 	 * Get the DH group used.
@@ -170,8 +175,17 @@ diffie_hellman_params_t *diffie_hellman_get_params(diffie_hellman_group_t group)
  * Check if a given DH group is an ECDH group
  *
  * @param group			group to check
- * @return				TUE if group is an ECP group
+ * @return				TRUE if group is an ECP group
  */
 bool diffie_hellman_group_is_ec(diffie_hellman_group_t group);
 
+/**
+ * Check if a diffie hellman public value is valid for given group.
+ *
+ * @param group			group the value is used in
+ * @param value			public DH value to check
+ * @return				TRUE if value looks valid for group
+ */
+bool diffie_hellman_verify_value(diffie_hellman_group_t group, chunk_t value);
+
 #endif /** DIFFIE_HELLMAN_H_ @}*/

src/libstrongswan/plugins/gcrypt/gcrypt_dh.c

@@ -73,12 +73,17 @@ struct private_gcrypt_dh_t {
 	size_t p_len;
 };
 
-METHOD(diffie_hellman_t, set_other_public_value, void,
+METHOD(diffie_hellman_t, set_other_public_value, bool,
 	private_gcrypt_dh_t *this, chunk_t value)
 {
 	gcry_mpi_t p_min_1;
 	gcry_error_t err;
 
+	if (!diffie_hellman_verify_value(this->group, value))
+	{
+		return FALSE;
+	}
+
 	if (this->yb)
 	{
 		gcry_mpi_release(this->yb);
@@ -88,7 +93,7 @@ METHOD(diffie_hellman_t, set_other_public_value, void,
 	if (err)
 	{
 		DBG1(DBG_LIB, "importing mpi yb failed: %s", gpg_strerror(err));
-		return;
+		return FALSE;
 	}
 
 	p_min_1 = gcry_mpi_new(this->p_len * 8);
@@ -112,6 +117,7 @@ METHOD(diffie_hellman_t, set_other_public_value, void,
 			 " y < 2 || y > p - 1 ");
 	}
 	gcry_mpi_release(p_min_1);
+	return this->zz != NULL;
 }
 
 /**
@@ -132,21 +138,22 @@ static chunk_t export_mpi(gcry_mpi_t value, size_t len)
 	return chunk;
 }
 
-METHOD(diffie_hellman_t, get_my_public_value, void,
+METHOD(diffie_hellman_t, get_my_public_value, bool,
 	private_gcrypt_dh_t *this, chunk_t *value)
 {
 	*value = export_mpi(this->ya, this->p_len);
+	return TRUE;
 }
 
-METHOD(diffie_hellman_t, get_shared_secret, status_t,
+METHOD(diffie_hellman_t, get_shared_secret, bool,
 	private_gcrypt_dh_t *this, chunk_t *secret)
 {
 	if (!this->zz)
 	{
-		return FAILED;
+		return FALSE;
 	}
 	*secret = export_mpi(this->zz, this->p_len);
-	return SUCCESS;
+	return TRUE;
 }
 
 METHOD(diffie_hellman_t, get_dh_group, diffie_hellman_group_t,

src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c

@@ -85,11 +85,16 @@ struct private_gmp_diffie_hellman_t {
 	bool computed;
 };
 
-METHOD(diffie_hellman_t, set_other_public_value, void,
+METHOD(diffie_hellman_t, set_other_public_value, bool,
 	private_gmp_diffie_hellman_t *this, chunk_t value)
 {
 	mpz_t p_min_1;
 
+	if (!diffie_hellman_verify_value(this->group, value))
+	{
+		return FALSE;
+	}
+
 	mpz_init(p_min_1);
 	mpz_sub_ui(p_min_1, this->p, 1);
 
@@ -142,9 +147,10 @@ METHOD(diffie_hellman_t, set_other_public_value, void,
 			 " y < 2 || y > p - 1 ");
 	}
 	mpz_clear(p_min_1);
+	return this->computed;
 }
 
-METHOD(diffie_hellman_t, get_my_public_value, void,
+METHOD(diffie_hellman_t, get_my_public_value, bool,
 	private_gmp_diffie_hellman_t *this,chunk_t *value)
 {
 	value->len = this->p_len;
@@ -153,22 +159,23 @@ METHOD(diffie_hellman_t, get_my_public_value, void,
 	{
 		value->len = 0;
 	}
+	return TRUE;
 }
 
-METHOD(diffie_hellman_t, get_shared_secret, status_t,
+METHOD(diffie_hellman_t, get_shared_secret, bool,
 	private_gmp_diffie_hellman_t *this, chunk_t *secret)
 {
 	if (!this->computed)
 	{
-		return FAILED;
+		return FALSE;
 	}
 	secret->len = this->p_len;
 	secret->ptr = mpz_export(NULL, NULL, 1, secret->len, 1, 0, this->zz);
 	if (secret->ptr == NULL)
 	{
-		return FAILED;
+		return FALSE;
 	}
-	return SUCCESS;
+	return TRUE;
 }
 
 METHOD(diffie_hellman_t, get_dh_group, diffie_hellman_group_t,

src/libstrongswan/plugins/ntru/ntru_ke.c

@@ -106,10 +106,10 @@ struct private_ntru_ke_t {
 	/**
 	 * Deterministic Random Bit Generator
 	 */
-    ntru_drbg_t *drbg;
+	ntru_drbg_t *drbg;
 };
 
-METHOD(diffie_hellman_t, get_my_public_value, void,
+METHOD(diffie_hellman_t, get_my_public_value, bool,
 	private_ntru_ke_t *this, chunk_t *value)
 {
 	*value = chunk_empty;
@@ -130,30 +130,30 @@ METHOD(diffie_hellman_t, get_my_public_value, void,
 			if (!this->privkey)
 			{
 				DBG1(DBG_LIB, "NTRU keypair generation failed");
-				return;
+				return FALSE;
 			}
 			this->pubkey = this->privkey->get_public_key(this->privkey);
 		}
 		*value = chunk_clone(this->pubkey->get_encoding(this->pubkey));
 		DBG3(DBG_LIB, "NTRU public key: %B", value);
 	}
+	return TRUE;
 }
 
-METHOD(diffie_hellman_t, get_shared_secret, status_t,
+METHOD(diffie_hellman_t, get_shared_secret, bool,
 	private_ntru_ke_t *this, chunk_t *secret)
 {
 	if (!this->computed || !this->shared_secret.len)
 	{
 		*secret = chunk_empty;
-		return FAILED;
+		return FALSE;
 	}
 	*secret = chunk_clone(this->shared_secret);
 
-	return SUCCESS;
+	return TRUE;
 }
 
-
-METHOD(diffie_hellman_t, set_other_public_value, void,
+METHOD(diffie_hellman_t, set_other_public_value, bool,
 	private_ntru_ke_t *this, chunk_t value)
 {
 	if (this->privkey)
@@ -162,15 +162,15 @@ METHOD(diffie_hellman_t, set_other_public_value, void,
 		if (value.len == 0)
 		{
 			DBG1(DBG_LIB, "empty NTRU ciphertext");
-			return;
+			return FALSE;
 		}
 		DBG3(DBG_LIB, "NTRU ciphertext: %B", &value);
 
 		/* decrypt the shared secret */
- 		if (!this->privkey->decrypt(this->privkey, value, &this->shared_secret))
+		if (!this->privkey->decrypt(this->privkey, value, &this->shared_secret))
 		{
 			DBG1(DBG_LIB, "NTRU decryption of shared secret failed");
-			return;
+			return FALSE;
 		}
 		this->computed = TRUE;
 	}
@@ -185,13 +185,13 @@ METHOD(diffie_hellman_t, set_other_public_value, void,
 		pubkey = ntru_public_key_create_from_data(this->drbg, value);
 		if (!pubkey)
 		{
-			return;
+			return FALSE;
 		}
 		if (pubkey->get_id(pubkey) != this->param_set->id)
 		{
 			DBG1(DBG_LIB, "received NTRU public key with wrong OUI");
 			pubkey->destroy(pubkey);
-			return;
+			return FALSE;
 		}
 		this->pubkey = pubkey;
 
@@ -204,7 +204,7 @@ METHOD(diffie_hellman_t, set_other_public_value, void,
 		{
 			DBG1(DBG_LIB, "generation of shared secret failed");
 			chunk_free(&this->shared_secret);
-			return;
+			return FALSE;
 		}
 		this->computed = TRUE;
 
@@ -212,10 +212,11 @@ METHOD(diffie_hellman_t, set_other_public_value, void,
 		if (!pubkey->encrypt(pubkey, this->shared_secret, &this->ciphertext))
 		{
 			DBG1(DBG_LIB, "NTRU encryption of shared secret failed");
-			return;
+			return FALSE;
 		}
 		DBG3(DBG_LIB, "NTRU ciphertext: %B", &this->ciphertext);
 	}
+	return this->computed;
 }
 
 METHOD(diffie_hellman_t, get_dh_group, diffie_hellman_group_t,
@@ -301,10 +302,10 @@ ntru_ke_t *ntru_ke_create(diffie_hellman_group_t group, chunk_t g, chunk_t p)
 
 	drbg = ntru_drbg_create(strength, chunk_from_str("IKE NTRU-KE"), entropy);
 	if (!drbg)
- 	{
+	{
 		DBG1(DBG_LIB, "could not instantiate DRBG at %u bit security", strength);
 		entropy->destroy(entropy);
-        return NULL;
+		return NULL;
 	}
 
 	INIT(this,
@@ -326,4 +327,3 @@ ntru_ke_t *ntru_ke_create(diffie_hellman_group_t group, chunk_t g, chunk_t p)
 
 	return &this->public;
 }
-

src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c

@@ -61,36 +61,42 @@ struct private_openssl_diffie_hellman_t {
 	bool computed;
 };
 
-METHOD(diffie_hellman_t, get_my_public_value, void,
+METHOD(diffie_hellman_t, get_my_public_value, bool,
 	private_openssl_diffie_hellman_t *this, chunk_t *value)
 {
 	*value = chunk_alloc(DH_size(this->dh));
 	memset(value->ptr, 0, value->len);
 	BN_bn2bin(this->dh->pub_key,
 			  value->ptr + value->len - BN_num_bytes(this->dh->pub_key));
+	return TRUE;
 }
 
-METHOD(diffie_hellman_t, get_shared_secret, status_t,
+METHOD(diffie_hellman_t, get_shared_secret, bool,
 	private_openssl_diffie_hellman_t *this, chunk_t *secret)
 {
 	if (!this->computed)
 	{
-		return FAILED;
+		return FALSE;
 	}
 	/* shared secret should requires a len according the DH group */
 	*secret = chunk_alloc(DH_size(this->dh));
 	memset(secret->ptr, 0, secret->len);
 	memcpy(secret->ptr + secret->len - this->shared_secret.len,
 		   this->shared_secret.ptr, this->shared_secret.len);
-	return SUCCESS;
+	return TRUE;
 }
 
 
-METHOD(diffie_hellman_t, set_other_public_value, void,
+METHOD(diffie_hellman_t, set_other_public_value, bool,
 	private_openssl_diffie_hellman_t *this, chunk_t value)
 {
 	int len;
 
+	if (!diffie_hellman_verify_value(this->group, value))
+	{
+		return FALSE;
+	}
+
 	BN_bin2bn(value.ptr, value.len, this->pub_key);
 	chunk_clear(&this->shared_secret);
 	this->shared_secret.ptr = malloc(DH_size(this->dh));
@@ -99,10 +105,11 @@ METHOD(diffie_hellman_t, set_other_public_value, void,
 	if (len < 0)
 	{
 		DBG1(DBG_LIB, "DH shared secret computation failed");
-		return;
+		return FALSE;
 	}
 	this->shared_secret.len = len;
 	this->computed = TRUE;
+	return TRUE;
 }
 
 METHOD(diffie_hellman_t, get_dh_group, diffie_hellman_group_t,

src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c

@@ -216,40 +216,47 @@ error:
 	return ret;
 }
 
-METHOD(diffie_hellman_t, set_other_public_value, void,
+METHOD(diffie_hellman_t, set_other_public_value, bool,
 	private_openssl_ec_diffie_hellman_t *this, chunk_t value)
 {
+	if (!diffie_hellman_verify_value(this->group, value))
+	{
+		return FALSE;
+	}
+
 	if (!chunk2ecp(this->ec_group, value, this->pub_key))
 	{
 		DBG1(DBG_LIB, "ECDH public value is malformed");
-		return;
+		return FALSE;
 	}
 
 	chunk_clear(&this->shared_secret);
 
 	if (!compute_shared_key(this, &this->shared_secret)) {
 		DBG1(DBG_LIB, "ECDH shared secret computation failed");
-		return;
+		return FALSE;
 	}
 
 	this->computed = TRUE;
+	return TRUE;
 }
 
-METHOD(diffie_hellman_t, get_my_public_value, void,
+METHOD(diffie_hellman_t, get_my_public_value, bool,
 	private_openssl_ec_diffie_hellman_t *this,chunk_t *value)
 {
 	ecp2chunk(this->ec_group, EC_KEY_get0_public_key(this->key), value, FALSE);
+	return TRUE;
 }
 
-METHOD(diffie_hellman_t, get_shared_secret, status_t,
+METHOD(diffie_hellman_t, get_shared_secret, bool,
 	private_openssl_ec_diffie_hellman_t *this, chunk_t *secret)
 {
 	if (!this->computed)
 	{
-		return FAILED;
+		return FALSE;
 	}
 	*secret = chunk_clone(this->shared_secret);
-	return SUCCESS;
+	return TRUE;
 }
 
 METHOD(diffie_hellman_t, get_dh_group, diffie_hellman_group_t,

src/libstrongswan/plugins/pkcs11/pkcs11_dh.c

@@ -81,7 +81,7 @@ struct private_pkcs11_dh_t {
  *
  * If this succeeds the shared secret is stored in this->secret.
  */
-static void derive_secret(private_pkcs11_dh_t *this, chunk_t other)
+static bool derive_secret(private_pkcs11_dh_t *this, chunk_t other)
 {
 	CK_OBJECT_CLASS klass = CKO_SECRET_KEY;
 	CK_KEY_TYPE type = CKK_GENERIC_SECRET;
@@ -102,19 +102,25 @@ static void derive_secret(private_pkcs11_dh_t *this, chunk_t other)
 	if (rv != CKR_OK)
 	{
 		DBG1(DBG_CFG, "C_DeriveKey() error: %N", ck_rv_names, rv);
-		return;
+		return FALSE;
 	}
 	if (!this->lib->get_ck_attribute(this->lib, this->session, secret,
 									 CKA_VALUE, &this->secret))
 	{
 		chunk_free(&this->secret);
-		return;
+		return FALSE;
 	}
+	return TRUE;
 }
 
-METHOD(diffie_hellman_t, set_other_public_value, void,
+METHOD(diffie_hellman_t, set_other_public_value, bool,
 	private_pkcs11_dh_t *this, chunk_t value)
 {
+	if (!diffie_hellman_verify_value(this->group, value))
+	{
+		return FALSE;
+	}
+
 	switch (this->group)
 	{
 		case ECP_192_BIT:
@@ -137,7 +143,7 @@ METHOD(diffie_hellman_t, set_other_public_value, void,
 			if (!lib->settings->get_bool(lib->settings,
 									"%s.ecp_x_coordinate_only", TRUE, lib->ns))
 			{	/* we only get the x coordinate back */
-				return;
+				return FALSE;
 			}
 			value = chunk_from_thing(params);
 			break;
@@ -145,24 +151,25 @@ METHOD(diffie_hellman_t, set_other_public_value, void,
 		default:
 			break;
 	}
-	derive_secret(this, value);
+	return derive_secret(this, value);
 }
 
-METHOD(diffie_hellman_t, get_my_public_value, void,
+METHOD(diffie_hellman_t, get_my_public_value, bool,
 	private_pkcs11_dh_t *this, chunk_t *value)
 {
 	*value = chunk_clone(this->pub_key);
+	return TRUE;
 }
 
-METHOD(diffie_hellman_t, get_shared_secret, status_t,
+METHOD(diffie_hellman_t, get_shared_secret, bool,
 	private_pkcs11_dh_t *this, chunk_t *secret)
 {
 	if (!this->secret.ptr)
 	{
-		return FAILED;
+		return FALSE;
 	}
 	*secret = chunk_clone(this->secret);
-	return SUCCESS;
+	return TRUE;
 }
 
 METHOD(diffie_hellman_t, get_dh_group, diffie_hellman_group_t,
@@ -443,4 +450,3 @@ pkcs11_dh_t *pkcs11_dh_create(diffie_hellman_group_t group,
 	}
 	return NULL;
 }
-

src/libstrongswan/tests/suites/test_ntru.c

@@ -1061,7 +1061,6 @@ START_TEST(test_ntru_ke)
 	diffie_hellman_t *i_ntru, *r_ntru;
 	char buf[10];
 	int k, n, len;
-	status_t status;
 
 	k = (_i) / countof(parameter_sets);
 	n = (_i) % countof(parameter_sets);
@@ -1078,23 +1077,21 @@ START_TEST(test_ntru_ke)
 	ck_assert(i_ntru != NULL);
 	ck_assert(i_ntru->get_dh_group(i_ntru) == params[k].group);
 
-	i_ntru->get_my_public_value(i_ntru, &pub_key);
+	ck_assert(i_ntru->get_my_public_value(i_ntru, &pub_key));
 	ck_assert(pub_key.len > 0);
 
 	r_ntru = lib->crypto->create_dh(lib->crypto, params[k].group);
 	ck_assert(r_ntru != NULL);
 
-	r_ntru->set_other_public_value(r_ntru, pub_key);
-	r_ntru->get_my_public_value(r_ntru, &cipher_text);
+	ck_assert(r_ntru->set_other_public_value(r_ntru, pub_key));
+	ck_assert(r_ntru->get_my_public_value(r_ntru, &cipher_text));
 	ck_assert(cipher_text.len > 0);
 
-	status = r_ntru->get_shared_secret(r_ntru, &r_shared_secret);
-	ck_assert(status == SUCCESS);
+	ck_assert(r_ntru->get_shared_secret(r_ntru, &r_shared_secret));
 	ck_assert(r_shared_secret.len > 0);
 
-	i_ntru->set_other_public_value(i_ntru, cipher_text);
-	status = i_ntru->get_shared_secret(i_ntru, &i_shared_secret);
-	ck_assert(status == SUCCESS);
+	ck_assert(i_ntru->set_other_public_value(i_ntru, cipher_text));
+	ck_assert(i_ntru->get_shared_secret(i_ntru, &i_shared_secret));
 	ck_assert(chunk_equals(i_shared_secret, r_shared_secret));
 
 	chunk_clear(&i_shared_secret);
@@ -1112,8 +1109,8 @@ START_TEST(test_ntru_retransmission)
 	chunk_t pub_key1, pub_key2;
 
 	i_ntru = lib->crypto->create_dh(lib->crypto, NTRU_256_BIT);
-	i_ntru->get_my_public_value(i_ntru, &pub_key1);
-	i_ntru->get_my_public_value(i_ntru, &pub_key2);
+	ck_assert(i_ntru->get_my_public_value(i_ntru, &pub_key1));
+	ck_assert(i_ntru->get_my_public_value(i_ntru, &pub_key2));
 	ck_assert(chunk_equals(pub_key1, pub_key2));
 
 	chunk_free(&pub_key1);
@@ -1139,8 +1136,8 @@ START_TEST(test_ntru_pubkey_oid)
 	chunk_t cipher_text;
 
 	r_ntru = lib->crypto->create_dh(lib->crypto, NTRU_128_BIT);
-	r_ntru->set_other_public_value(r_ntru, oid_tests[_i]);
-	r_ntru->get_my_public_value(r_ntru, &cipher_text);
+	ck_assert(!r_ntru->set_other_public_value(r_ntru, oid_tests[_i]));
+	ck_assert(r_ntru->get_my_public_value(r_ntru, &cipher_text));
 	ck_assert(cipher_text.len == 0);
 	r_ntru->destroy(r_ntru);
 }
@@ -1155,14 +1152,14 @@ START_TEST(test_ntru_wrong_set)
 						  "libstrongswan.plugins.ntru.parameter_set",
 			 			  "x9_98_bandwidth");
 	i_ntru = lib->crypto->create_dh(lib->crypto, NTRU_112_BIT);
-	i_ntru->get_my_public_value(i_ntru, &pub_key);
+	ck_assert(i_ntru->get_my_public_value(i_ntru, &pub_key));
 
 	lib->settings->set_str(lib->settings,
 						  "libstrongswan.plugins.ntru.parameter_set",
 						  "optimum");
 	r_ntru = lib->crypto->create_dh(lib->crypto, NTRU_112_BIT);
-	r_ntru->set_other_public_value(r_ntru, pub_key);
-	r_ntru->get_my_public_value(r_ntru, &cipher_text);
+	ck_assert(!r_ntru->set_other_public_value(r_ntru, pub_key));
+	ck_assert(r_ntru->get_my_public_value(r_ntru, &cipher_text));
 	ck_assert(cipher_text.len == 0);
 
 	chunk_free(&pub_key);
@@ -1193,9 +1190,9 @@ START_TEST(test_ntru_ciphertext)
 	for (i = 0; i < countof(test); i++)
 	{
 		i_ntru = lib->crypto->create_dh(lib->crypto, NTRU_128_BIT);
-		i_ntru->get_my_public_value(i_ntru, &pub_key);
-		i_ntru->set_other_public_value(i_ntru, test[i]);
-		ck_assert(i_ntru->get_shared_secret(i_ntru, &shared_secret) != SUCCESS);
+		ck_assert(i_ntru->get_my_public_value(i_ntru, &pub_key));
+		ck_assert(!i_ntru->set_other_public_value(i_ntru, test[i]));
+		ck_assert(!i_ntru->get_shared_secret(i_ntru, &shared_secret));
 		ck_assert(shared_secret.len == 0);
 
 		chunk_free(&pub_key);
@@ -1213,12 +1210,12 @@ START_TEST(test_ntru_wrong_ciphertext)
 	r_ntru = lib->crypto->create_dh(lib->crypto, NTRU_128_BIT);
 	m_ntru = lib->crypto->create_dh(lib->crypto, NTRU_128_BIT);
 
-	i_ntru->get_my_public_value(i_ntru, &pub_key_i);
-	m_ntru->get_my_public_value(m_ntru, &pub_key_m);
-	r_ntru->set_other_public_value(r_ntru, pub_key_m);
-	r_ntru->get_my_public_value(r_ntru, &cipher_text);
-	i_ntru->set_other_public_value(i_ntru, cipher_text);
-	ck_assert(i_ntru->get_shared_secret(i_ntru, &shared_secret) != SUCCESS);
+	ck_assert(i_ntru->get_my_public_value(i_ntru, &pub_key_i));
+	ck_assert(m_ntru->get_my_public_value(m_ntru, &pub_key_m));
+	ck_assert(r_ntru->set_other_public_value(r_ntru, pub_key_m));
+	ck_assert(r_ntru->get_my_public_value(r_ntru, &cipher_text));
+	ck_assert(!i_ntru->set_other_public_value(i_ntru, cipher_text));
+	ck_assert(!i_ntru->get_shared_secret(i_ntru, &shared_secret));
 	ck_assert(shared_secret.len == 0);
 
 	chunk_free(&pub_key_i);

src/libtls/tls_peer.c

@@ -384,7 +384,12 @@ static status_t process_modp_key_exchange(private_tls_peer_t *this,
 		this->alert->add(this->alert, TLS_FATAL, TLS_INTERNAL_ERROR);
 		return NEED_MORE;
 	}
-	this->dh->set_other_public_value(this->dh, pub);
+	if (!this->dh->set_other_public_value(this->dh, pub))
+	{
+		DBG1(DBG_TLS, "applying DH public value failed");
+		this->alert->add(this->alert, TLS_FATAL, TLS_INTERNAL_ERROR);
+		return NEED_MORE;
+	}
 
 	this->state = STATE_KEY_EXCHANGE_RECEIVED;
 	return NEED_MORE;
@@ -494,7 +499,12 @@ static status_t process_ec_key_exchange(private_tls_peer_t *this,
 		this->alert->add(this->alert, TLS_FATAL, TLS_INTERNAL_ERROR);
 		return NEED_MORE;
 	}
-	this->dh->set_other_public_value(this->dh, chunk_skip(pub, 1));
+	if (!this->dh->set_other_public_value(this->dh, chunk_skip(pub, 1)))
+	{
+		DBG1(DBG_TLS, "applying DH public value failed");
+		this->alert->add(this->alert, TLS_FATAL, TLS_INTERNAL_ERROR);
+		return NEED_MORE;
+	}
 
 	this->state = STATE_KEY_EXCHANGE_RECEIVED;
 	return NEED_MORE;
@@ -973,7 +983,7 @@ static status_t send_key_exchange_dhe(private_tls_peer_t *this,
 {
 	chunk_t premaster, pub;
 
-	if (this->dh->get_shared_secret(this->dh, &premaster) != SUCCESS)
+	if (!this->dh->get_shared_secret(this->dh, &premaster))
 	{
 		DBG1(DBG_TLS, "calculating premaster from DH failed");
 		this->alert->add(this->alert, TLS_FATAL, TLS_INTERNAL_ERROR);
@@ -990,7 +1000,11 @@ static status_t send_key_exchange_dhe(private_tls_peer_t *this,
 	}
 	chunk_clear(&premaster);
 
-	this->dh->get_my_public_value(this->dh, &pub);
+	if (!this->dh->get_my_public_value(this->dh, &pub))
+	{
+		this->alert->add(this->alert, TLS_FATAL, TLS_INTERNAL_ERROR);
+		return NEED_MORE;
+	}
 	if (this->dh->get_dh_group(this->dh) == MODP_CUSTOM)
 	{
 		writer->write_data16(writer, pub);

src/libtls/tls_server.c

@@ -494,8 +494,13 @@ static status_t process_key_exchange_dhe(private_tls_server_t *this,
 		}
 		pub = chunk_skip(pub, 1);
 	}
-	this->dh->set_other_public_value(this->dh, pub);
-	if (this->dh->get_shared_secret(this->dh, &premaster) != SUCCESS)
+	if (!this->dh->set_other_public_value(this->dh, pub))
+	{
+		DBG1(DBG_TLS, "applying DH public value failed");
+		this->alert->add(this->alert, TLS_FATAL, TLS_INTERNAL_ERROR);
+		return NEED_MORE;
+	}
+	if (!this->dh->get_shared_secret(this->dh, &premaster))
 	{
 		DBG1(DBG_TLS, "calculating premaster from DH failed");
 		this->alert->add(this->alert, TLS_FATAL, TLS_INTERNAL_ERROR);
@@ -915,7 +920,11 @@ static status_t send_server_key_exchange(private_tls_server_t *this,
 		this->alert->add(this->alert, TLS_FATAL, TLS_INTERNAL_ERROR);
 		return NEED_MORE;
 	}
-	this->dh->get_my_public_value(this->dh, &chunk);
+	if (!this->dh->get_my_public_value(this->dh, &chunk))
+	{
+		this->alert->add(this->alert, TLS_FATAL, TLS_INTERNAL_ERROR);
+		return NEED_MORE;
+	}
 	if (params)
 	{
 		writer->write_data16(writer, chunk);