From 4909612c3bcfb951a3f8ecccf433756b450f4e8e Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 23 Mar 2015 10:44:55 +0100 Subject: [PATCH 1/9] load-tester: Migrate NULL DH implementation to INIT/METHOD macros --- .../load_tester/load_tester_diffie_hellman.c | 47 ++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c b/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c index d5ec3599b..94e1acc99 100644 --- a/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c +++ b/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c @@ -15,33 +15,36 @@ #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, void, + load_tester_diffie_hellman_t *this, chunk_t *value) { *value = chunk_empty; } -/** - * 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, void, + load_tester_diffie_hellman_t *this, chunk_t value) +{ +} + +METHOD(diffie_hellman_t, get_shared_secret, status_t, + load_tester_diffie_hellman_t *this, chunk_t *secret) { *secret = chunk_empty; return SUCCESS; } -/** - * 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 +58,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; } From bace1d647971e8300c5abdb1e950adf3756ec328 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 23 Mar 2015 10:54:24 +0100 Subject: [PATCH 2/9] diffie-hellman: Use bool instead of status_t as get_shared_secret() return value While such a change is not unproblematic, keeping status_t makes the API inconsistent once we introduce return values for the public value operations. --- src/charon-tkm/src/tkm/tkm_diffie_hellman.c | 4 ++-- src/libcharon/plugins/ha/ha_child.c | 2 +- src/libcharon/plugins/ha/ha_dispatcher.c | 4 ++-- src/libcharon/plugins/ha/ha_ike.c | 2 +- .../plugins/load_tester/load_tester_diffie_hellman.c | 4 ++-- src/libcharon/sa/ikev1/keymat_v1.c | 4 ++-- src/libcharon/sa/ikev2/keymat_v2.c | 4 ++-- src/libimcv/pts/pts.c | 2 +- src/libstrongswan/crypto/diffie_hellman.h | 5 +++-- src/libstrongswan/plugins/gcrypt/gcrypt_dh.c | 6 +++--- src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c | 8 ++++---- src/libstrongswan/plugins/ntru/ntru_ke.c | 6 +++--- .../plugins/openssl/openssl_diffie_hellman.c | 6 +++--- .../plugins/openssl/openssl_ec_diffie_hellman.c | 6 +++--- src/libstrongswan/plugins/pkcs11/pkcs11_dh.c | 6 +++--- src/libstrongswan/tests/suites/test_ntru.c | 11 ++++------- src/libtls/tls_peer.c | 2 +- src/libtls/tls_server.c | 2 +- 18 files changed, 41 insertions(+), 43 deletions(-) diff --git a/src/charon-tkm/src/tkm/tkm_diffie_hellman.c b/src/charon-tkm/src/tkm/tkm_diffie_hellman.c index 836e0b7f0..02ae67f73 100644 --- a/src/charon-tkm/src/tkm/tkm_diffie_hellman.c +++ b/src/charon-tkm/src/tkm/tkm_diffie_hellman.c @@ -61,11 +61,11 @@ METHOD(diffie_hellman_t, get_my_public_value, void, sequence_to_chunk(this->pubvalue.data, this->pubvalue.size, value); } -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; } diff --git a/src/libcharon/plugins/ha/ha_child.c b/src/libcharon/plugins/ha/ha_child.c index ed6ca7196..17f2d50d1 100644 --- a/src/libcharon/plugins/ha/ha_child.c +++ b/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); diff --git a/src/libcharon/plugins/ha/ha_dispatcher.c b/src/libcharon/plugins/ha/ha_dispatcher.c index 88160fe4f..abd08e2fe 100644 --- a/src/libcharon/plugins/ha/ha_dispatcher.c +++ b/src/libcharon/plugins/ha/ha_dispatcher.c @@ -81,11 +81,11 @@ 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, diff --git a/src/libcharon/plugins/ha/ha_ike.c b/src/libcharon/plugins/ha/ha_ike.c index 442a3a23d..815cb5389 100644 --- a/src/libcharon/plugins/ha/ha_ike.c +++ b/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; } diff --git a/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c b/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c index 94e1acc99..b248e78c5 100644 --- a/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c +++ b/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c @@ -26,11 +26,11 @@ METHOD(diffie_hellman_t, set_other_public_value, void, { } -METHOD(diffie_hellman_t, get_shared_secret, status_t, +METHOD(diffie_hellman_t, get_shared_secret, bool, load_tester_diffie_hellman_t *this, chunk_t *secret) { *secret = chunk_empty; - return SUCCESS; + return TRUE; } METHOD(diffie_hellman_t, get_dh_group, diffie_hellman_group_t, diff --git a/src/libcharon/sa/ikev1/keymat_v1.c b/src/libcharon/sa/ikev1/keymat_v1.c index 619d197bd..b171adc1e 100644 --- a/src/libcharon/sa/ikev1/keymat_v1.c +++ b/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; } @@ -661,7 +661,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; } diff --git a/src/libcharon/sa/ikev2/keymat_v2.c b/src/libcharon/sa/ikev2/keymat_v2.c index f237f7059..f70f5cfed 100644 --- a/src/libcharon/sa/ikev2/keymat_v2.c +++ b/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; } diff --git a/src/libimcv/pts/pts.c b/src/libimcv/pts/pts.c index 2fff4c901..a7def9b7a 100644 --- a/src/libimcv/pts/pts.c +++ b/src/libimcv/pts/pts.c @@ -264,7 +264,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; diff --git a/src/libstrongswan/crypto/diffie_hellman.h b/src/libstrongswan/crypto/diffie_hellman.h index d5161d077..79977d7da 100644 --- a/src/libstrongswan/crypto/diffie_hellman.h +++ b/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. diff --git a/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c b/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c index 299865da2..44f33c9a6 100644 --- a/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c +++ b/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c @@ -138,15 +138,15 @@ METHOD(diffie_hellman_t, get_my_public_value, void, *value = export_mpi(this->ya, this->p_len); } -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, diff --git a/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c b/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c index 9936f7e45..d07999dfb 100644 --- a/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c +++ b/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c @@ -155,20 +155,20 @@ METHOD(diffie_hellman_t, get_my_public_value, void, } } -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, diff --git a/src/libstrongswan/plugins/ntru/ntru_ke.c b/src/libstrongswan/plugins/ntru/ntru_ke.c index e64f32b91..0aafd4caf 100644 --- a/src/libstrongswan/plugins/ntru/ntru_ke.c +++ b/src/libstrongswan/plugins/ntru/ntru_ke.c @@ -139,17 +139,17 @@ METHOD(diffie_hellman_t, get_my_public_value, void, } } -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; } diff --git a/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c index 1e68ac59b..603580277 100644 --- a/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c +++ b/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c @@ -70,19 +70,19 @@ METHOD(diffie_hellman_t, get_my_public_value, void, value->ptr + value->len - BN_num_bytes(this->dh->pub_key)); } -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; } diff --git a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c index 50853d6f0..625990b0f 100644 --- a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c +++ b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c @@ -241,15 +241,15 @@ METHOD(diffie_hellman_t, get_my_public_value, void, ecp2chunk(this->ec_group, EC_KEY_get0_public_key(this->key), value, FALSE); } -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, diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c b/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c index 23b63d238..99702f9c5 100644 --- a/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c +++ b/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c @@ -154,15 +154,15 @@ METHOD(diffie_hellman_t, get_my_public_value, void, *value = chunk_clone(this->pub_key); } -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, diff --git a/src/libstrongswan/tests/suites/test_ntru.c b/src/libstrongswan/tests/suites/test_ntru.c index a28b4bc58..5d5448fcc 100644 --- a/src/libstrongswan/tests/suites/test_ntru.c +++ b/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); @@ -1088,13 +1087,11 @@ START_TEST(test_ntru_ke) 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->get_shared_secret(i_ntru, &i_shared_secret)); ck_assert(chunk_equals(i_shared_secret, r_shared_secret)); chunk_clear(&i_shared_secret); @@ -1195,7 +1192,7 @@ START_TEST(test_ntru_ciphertext) 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_shared_secret(i_ntru, &shared_secret)); ck_assert(shared_secret.len == 0); chunk_free(&pub_key); @@ -1218,7 +1215,7 @@ START_TEST(test_ntru_wrong_ciphertext) 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_shared_secret(i_ntru, &shared_secret)); ck_assert(shared_secret.len == 0); chunk_free(&pub_key_i); diff --git a/src/libtls/tls_peer.c b/src/libtls/tls_peer.c index 99bc92ac0..82ec262e4 100644 --- a/src/libtls/tls_peer.c +++ b/src/libtls/tls_peer.c @@ -973,7 +973,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); diff --git a/src/libtls/tls_server.c b/src/libtls/tls_server.c index b6e706d23..df5d00ab5 100644 --- a/src/libtls/tls_server.c +++ b/src/libtls/tls_server.c @@ -495,7 +495,7 @@ 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->get_shared_secret(this->dh, &premaster)) { DBG1(DBG_TLS, "calculating premaster from DH failed"); this->alert->add(this->alert, TLS_FATAL, TLS_INTERNAL_ERROR); From 520d58e0109ecd792c83eab72a3f3357ffc41ba0 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 23 Mar 2015 11:10:40 +0100 Subject: [PATCH 3/9] encoding: Allow ke_payload_create_from_diffie_hellman() to fail --- src/libcharon/encoding/payloads/ke_payload.h | 2 +- src/libcharon/sa/ikev1/phase1.c | 8 +++++++- src/libcharon/sa/ikev1/tasks/quick_mode.c | 21 ++++++++++++++++---- src/libcharon/sa/ikev2/tasks/child_create.c | 20 ++++++++++++++++--- src/libcharon/sa/ikev2/tasks/ike_init.c | 21 ++++++++++++++++---- 5 files changed, 59 insertions(+), 13 deletions(-) diff --git a/src/libcharon/encoding/payloads/ke_payload.h b/src/libcharon/encoding/payloads/ke_payload.h index dfc6308b4..96c5096a5 100644 --- a/src/libcharon/encoding/payloads/ke_payload.h +++ b/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); diff --git a/src/libcharon/sa/ikev1/phase1.c b/src/libcharon/sa/ikev1/phase1.c index d01a831f8..a81fa5886 100644 --- a/src/libcharon/sa/ikev1/phase1.c +++ b/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); diff --git a/src/libcharon/sa/ikev1/tasks/quick_mode.c b/src/libcharon/sa/ikev1/tasks/quick_mode.c index 4b5b0160a..ac82452c8 100644 --- a/src/libcharon/sa/ikev1/tasks/quick_mode.c +++ b/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; } /** @@ -880,7 +887,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 +1228,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); diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index 1bb934002..7b1c44e7e 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/src/libcharon/sa/ikev2/tasks/child_create.c @@ -716,7 +716,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 +748,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 +781,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; } /** @@ -1035,7 +1041,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)); @@ -1288,7 +1297,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 */ diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.c b/src/libcharon/sa/ikev2/tasks/ike_init.c index ab3d57af6..1f59296d9 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_init.c +++ b/src/libcharon/sa/ikev2/tasks/ike_init.c @@ -210,7 +210,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 +254,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 +295,7 @@ static void build_payloads(private_ike_init_t *this, message_t *message) send_supported_hash_algorithms(this, message); } } + return TRUE; } /** @@ -438,7 +445,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 { @@ -572,7 +582,10 @@ METHOD(task_t, build_r, status_t, message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty); return FAILED; } - build_payloads(this, message); + if (!build_payloads(this, message)) + { + return FAILED; + } return SUCCESS; } From 83cda57e2d0639eca19bcd956f4736d4cdcf7849 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 23 Mar 2015 11:25:37 +0100 Subject: [PATCH 4/9] libimcv: Allow pts_t.get_my_public_value() to fail --- .../plugins/imc_attestation/imc_attestation_process.c | 6 +++++- src/libimcv/plugins/imv_attestation/imv_attestation_build.c | 6 +++++- src/libimcv/pts/pts.c | 3 ++- src/libimcv/pts/pts.h | 3 ++- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/libimcv/plugins/imc_attestation/imc_attestation_process.c b/src/libimcv/plugins/imc_attestation/imc_attestation_process.c index 2fc2998e1..6f8e4ea5a 100644 --- a/src/libimcv/plugins/imc_attestation/imc_attestation_process.c +++ b/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, diff --git a/src/libimcv/plugins/imv_attestation/imv_attestation_build.c b/src/libimcv/plugins/imv_attestation/imv_attestation_build.c index c39fe8d47..db93ac45f 100644 --- a/src/libimcv/plugins/imv_attestation/imv_attestation_build.c +++ b/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); diff --git a/src/libimcv/pts/pts.c b/src/libimcv/pts/pts.c index a7def9b7a..6deccc82a 100644 --- a/src/libimcv/pts/pts.c +++ b/src/libimcv/pts/pts.c @@ -224,11 +224,12 @@ 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); *nonce = this->is_imc ? this->responder_nonce : this->initiator_nonce; + return TRUE; } METHOD(pts_t, set_peer_public_value, void, diff --git a/src/libimcv/pts/pts.h b/src/libimcv/pts/pts.h index be32a3464..4d3284e36 100644 --- a/src/libimcv/pts/pts.h +++ b/src/libimcv/pts/pts.h @@ -143,8 +143,9 @@ 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 From 8a7dbf3c2a0a4e8a4a503b630dbad0d46ae25756 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 23 Mar 2015 11:28:57 +0100 Subject: [PATCH 5/9] libimcv: Allow pts_t.set_peer_public_value() to fail --- .../plugins/imc_attestation/imc_attestation_process.c | 6 ++++-- .../plugins/imv_attestation/imv_attestation_process.c | 6 +++--- src/libimcv/pts/pts.c | 3 ++- src/libimcv/pts/pts.h | 3 ++- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/libimcv/plugins/imc_attestation/imc_attestation_process.c b/src/libimcv/plugins/imc_attestation/imc_attestation_process.c index 6f8e4ea5a..f24aec881 100644 --- a/src/libimcv/plugins/imc_attestation/imc_attestation_process.c +++ b/src/libimcv/plugins/imc_attestation/imc_attestation_process.c @@ -178,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; } diff --git a/src/libimcv/plugins/imv_attestation/imv_attestation_process.c b/src/libimcv/plugins/imv_attestation/imv_attestation_process.c index bad536abe..fbeb6618e 100644 --- a/src/libimcv/plugins/imv_attestation/imv_attestation_process.c +++ b/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; } diff --git a/src/libimcv/pts/pts.c b/src/libimcv/pts/pts.c index 6deccc82a..8d13bfca4 100644 --- a/src/libimcv/pts/pts.c +++ b/src/libimcv/pts/pts.c @@ -232,7 +232,7 @@ METHOD(pts_t, get_my_public_value, bool, 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); @@ -246,6 +246,7 @@ METHOD(pts_t, set_peer_public_value, void, { this->responder_nonce = nonce; } + return TRUE; } METHOD(pts_t, calculate_secret, bool, diff --git a/src/libimcv/pts/pts.h b/src/libimcv/pts/pts.h index 4d3284e36..d525306dd 100644 --- a/src/libimcv/pts/pts.h +++ b/src/libimcv/pts/pts.h @@ -152,8 +152,9 @@ struct pts_t { * * @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 From 42431690e04a8614e759e0a3a3bcd76e30e6207e Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 23 Mar 2015 11:37:27 +0100 Subject: [PATCH 6/9] diffie-hellman: Add a bool return value to get_my_public_value() --- scripts/dh_speed.c | 5 +++-- src/charon-tkm/src/tkm/tkm_diffie_hellman.c | 3 ++- src/charon-tkm/tests/diffie_hellman_tests.c | 2 +- src/charon-tkm/tests/keymat_tests.c | 2 +- src/libcharon/encoding/payloads/ke_payload.c | 10 +++++++-- src/libcharon/plugins/ha/ha_dispatcher.c | 3 ++- src/libcharon/plugins/ha/ha_ike.c | 8 ++++--- .../load_tester/load_tester_diffie_hellman.c | 3 ++- .../authenticators/psk_v1_authenticator.c | 10 +++++++-- .../authenticators/pubkey_v1_authenticator.c | 11 ++++++++-- src/libcharon/sa/ikev1/keymat_v1.c | 5 ++++- src/libcharon/sa/ikev2/tasks/ike_init.c | 1 + src/libimcv/pts/pts.c | 5 ++++- src/libstrongswan/crypto/diffie_hellman.h | 4 +++- src/libstrongswan/plugins/gcrypt/gcrypt_dh.c | 3 ++- .../plugins/gmp/gmp_diffie_hellman.c | 3 ++- src/libstrongswan/plugins/ntru/ntru_ke.c | 5 +++-- .../plugins/openssl/openssl_diffie_hellman.c | 3 ++- .../openssl/openssl_ec_diffie_hellman.c | 3 ++- src/libstrongswan/plugins/pkcs11/pkcs11_dh.c | 3 ++- src/libstrongswan/tests/suites/test_ntru.c | 22 +++++++++---------- src/libtls/tls_peer.c | 6 ++++- src/libtls/tls_server.c | 6 ++++- 23 files changed, 87 insertions(+), 39 deletions(-) diff --git a/scripts/dh_speed.c b/scripts/dh_speed.c index 8a782d80b..21652e787 100644 --- a/scripts/dh_speed.c +++ b/scripts/dh_speed.c @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -88,12 +89,12 @@ 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); + assert(l[round]->get_my_public_value(l[round], &chunk)); 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++) { diff --git a/src/charon-tkm/src/tkm/tkm_diffie_hellman.c b/src/charon-tkm/src/tkm/tkm_diffie_hellman.c index 02ae67f73..d4691fa63 100644 --- a/src/charon-tkm/src/tkm/tkm_diffie_hellman.c +++ b/src/charon-tkm/src/tkm/tkm_diffie_hellman.c @@ -55,10 +55,11 @@ 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, bool, diff --git a/src/charon-tkm/tests/diffie_hellman_tests.c b/src/charon-tkm/tests/diffie_hellman_tests.c index 89658a770..5ef6f41ab 100644 --- a/src/charon-tkm/tests/diffie_hellman_tests.c +++ b/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"); diff --git a/src/charon-tkm/tests/keymat_tests.c b/src/charon-tkm/tests/keymat_tests.c index 1982671d3..c1f3872f7 100644 --- a/src/charon-tkm/tests/keymat_tests.c +++ b/src/charon-tkm/tests/keymat_tests.c @@ -53,7 +53,7 @@ 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); + ck_assert(dh->dh.get_my_public_value(&dh->dh, &pubvalue)); dh->dh.set_other_public_value(&dh->dh, pubvalue); fail_unless(keymat->keymat_v2.derive_ike_keys(&keymat->keymat_v2, proposal, diff --git a/src/libcharon/encoding/payloads/ke_payload.c b/src/libcharon/encoding/payloads/ke_payload.c index 7f3c4e400..c2599a682 100644 --- a/src/libcharon/encoding/payloads/ke_payload.c +++ b/src/libcharon/encoding/payloads/ke_payload.c @@ -320,9 +320,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; diff --git a/src/libcharon/plugins/ha/ha_dispatcher.c b/src/libcharon/plugins/ha/ha_dispatcher.c index abd08e2fe..31eeb934e 100644 --- a/src/libcharon/plugins/ha/ha_dispatcher.c +++ b/src/libcharon/plugins/ha/ha_dispatcher.c @@ -88,10 +88,11 @@ METHOD(diffie_hellman_t, dh_get_shared_secret, bool, 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, diff --git a/src/libcharon/plugins/ha/ha_ike.c b/src/libcharon/plugins/ha/ha_ike.c index 815cb5389..6b4b53c9c 100644 --- a/src/libcharon/plugins/ha/ha_ike.c +++ b/src/libcharon/plugins/ha/ha_ike.c @@ -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) { diff --git a/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c b/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c index b248e78c5..faa586d17 100644 --- a/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c +++ b/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c @@ -15,10 +15,11 @@ #include "load_tester_diffie_hellman.h" -METHOD(diffie_hellman_t, get_my_public_value, void, +METHOD(diffie_hellman_t, get_my_public_value, bool, load_tester_diffie_hellman_t *this, chunk_t *value) { *value = chunk_empty; + return TRUE; } METHOD(diffie_hellman_t, set_other_public_value, void, diff --git a/src/libcharon/sa/ikev1/authenticators/psk_v1_authenticator.c b/src/libcharon/sa/ikev1/authenticators/psk_v1_authenticator.c index aa966cd5f..bb187f07c 100644 --- a/src/libcharon/sa/ikev1/authenticators/psk_v1_authenticator.c +++ b/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, diff --git a/src/libcharon/sa/ikev1/authenticators/pubkey_v1_authenticator.c b/src/libcharon/sa/ikev1/authenticators/pubkey_v1_authenticator.c index bfe5ff449..52228ef2e 100644 --- a/src/libcharon/sa/ikev1/authenticators/pubkey_v1_authenticator.c +++ b/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, diff --git a/src/libcharon/sa/ikev1/keymat_v1.c b/src/libcharon/sa/ikev1/keymat_v1.c index b171adc1e..f5a91dbeb 100644 --- a/src/libcharon/sa/ikev1/keymat_v1.c +++ b/src/libcharon/sa/ikev1/keymat_v1.c @@ -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; diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.c b/src/libcharon/sa/ikev2/tasks/ike_init.c index 1f59296d9..09860c93a 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_init.c +++ b/src/libcharon/sa/ikev2/tasks/ike_init.c @@ -584,6 +584,7 @@ METHOD(task_t, build_r, status_t, } if (!build_payloads(this, message)) { + message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty); return FAILED; } return SUCCESS; diff --git a/src/libimcv/pts/pts.c b/src/libimcv/pts/pts.c index 8d13bfca4..e8e7600ea 100644 --- a/src/libimcv/pts/pts.c +++ b/src/libimcv/pts/pts.c @@ -227,7 +227,10 @@ METHOD(pts_t, create_dh_nonce, bool, 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; } diff --git a/src/libstrongswan/crypto/diffie_hellman.h b/src/libstrongswan/crypto/diffie_hellman.h index 79977d7da..f253f18c7 100644 --- a/src/libstrongswan/crypto/diffie_hellman.h +++ b/src/libstrongswan/crypto/diffie_hellman.h @@ -109,8 +109,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. diff --git a/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c b/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c index 44f33c9a6..9714dd68f 100644 --- a/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c +++ b/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c @@ -132,10 +132,11 @@ 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, bool, diff --git a/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c b/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c index d07999dfb..89740a0ec 100644 --- a/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c +++ b/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c @@ -144,7 +144,7 @@ METHOD(diffie_hellman_t, set_other_public_value, void, mpz_clear(p_min_1); } -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,6 +153,7 @@ METHOD(diffie_hellman_t, get_my_public_value, void, { value->len = 0; } + return TRUE; } METHOD(diffie_hellman_t, get_shared_secret, bool, diff --git a/src/libstrongswan/plugins/ntru/ntru_ke.c b/src/libstrongswan/plugins/ntru/ntru_ke.c index 0aafd4caf..5c7547577 100644 --- a/src/libstrongswan/plugins/ntru/ntru_ke.c +++ b/src/libstrongswan/plugins/ntru/ntru_ke.c @@ -109,7 +109,7 @@ struct private_ntru_ke_t { 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,13 +130,14 @@ 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, bool, diff --git a/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c index 603580277..64b650b4a 100644 --- a/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c +++ b/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c @@ -61,13 +61,14 @@ 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, bool, diff --git a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c index 625990b0f..54dfbd01b 100644 --- a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c +++ b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c @@ -235,10 +235,11 @@ METHOD(diffie_hellman_t, set_other_public_value, void, this->computed = 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, bool, diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c b/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c index 99702f9c5..89ddf65e3 100644 --- a/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c +++ b/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c @@ -148,10 +148,11 @@ METHOD(diffie_hellman_t, set_other_public_value, void, 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, bool, diff --git a/src/libstrongswan/tests/suites/test_ntru.c b/src/libstrongswan/tests/suites/test_ntru.c index 5d5448fcc..3264379d1 100644 --- a/src/libstrongswan/tests/suites/test_ntru.c +++ b/src/libstrongswan/tests/suites/test_ntru.c @@ -1077,14 +1077,14 @@ 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->get_my_public_value(r_ntru, &cipher_text)); ck_assert(cipher_text.len > 0); ck_assert(r_ntru->get_shared_secret(r_ntru, &r_shared_secret)); @@ -1109,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); @@ -1137,7 +1137,7 @@ START_TEST(test_ntru_pubkey_oid) 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->get_my_public_value(r_ntru, &cipher_text)); ck_assert(cipher_text.len == 0); r_ntru->destroy(r_ntru); } @@ -1152,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->get_my_public_value(r_ntru, &cipher_text)); ck_assert(cipher_text.len == 0); chunk_free(&pub_key); @@ -1190,7 +1190,7 @@ 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); + ck_assert(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)); ck_assert(shared_secret.len == 0); @@ -1210,10 +1210,10 @@ 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); + 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)); r_ntru->set_other_public_value(r_ntru, pub_key_m); - r_ntru->get_my_public_value(r_ntru, &cipher_text); + ck_assert(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)); ck_assert(shared_secret.len == 0); diff --git a/src/libtls/tls_peer.c b/src/libtls/tls_peer.c index 82ec262e4..68a2c9421 100644 --- a/src/libtls/tls_peer.c +++ b/src/libtls/tls_peer.c @@ -990,7 +990,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); diff --git a/src/libtls/tls_server.c b/src/libtls/tls_server.c index df5d00ab5..30ff0ad9e 100644 --- a/src/libtls/tls_server.c +++ b/src/libtls/tls_server.c @@ -915,7 +915,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); From a777155ffed7fc6382a2e344ebd748f70b1c61c2 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 23 Mar 2015 13:09:32 +0100 Subject: [PATCH 7/9] diffie-hellman: Add a bool return value to set_other_public_value() --- scripts/dh_speed.c | 5 ++-- src/charon-tkm/src/tkm/tkm_diffie_hellman.c | 6 ++--- src/charon-tkm/tests/keymat_tests.c | 2 +- .../load_tester/load_tester_diffie_hellman.c | 3 ++- src/libcharon/sa/ikev1/phase1.c | 6 ++++- src/libcharon/sa/ikev1/tasks/quick_mode.c | 8 ++++-- src/libcharon/sa/ikev2/tasks/child_create.c | 24 ++++++++++++++++-- src/libcharon/sa/ikev2/tasks/ike_init.c | 21 +++++++++++++++- src/libimcv/pts/pts.c | 5 +++- src/libstrongswan/crypto/diffie_hellman.h | 4 ++- src/libstrongswan/plugins/gcrypt/gcrypt_dh.c | 5 ++-- .../plugins/gmp/gmp_diffie_hellman.c | 3 ++- src/libstrongswan/plugins/ntru/ntru_ke.c | 25 +++++++++---------- .../plugins/openssl/openssl_diffie_hellman.c | 5 ++-- .../openssl/openssl_ec_diffie_hellman.c | 7 +++--- src/libstrongswan/plugins/pkcs11/pkcs11_dh.c | 14 +++++------ src/libstrongswan/tests/suites/test_ntru.c | 14 +++++------ src/libtls/tls_peer.c | 14 +++++++++-- src/libtls/tls_server.c | 7 +++++- 19 files changed, 123 insertions(+), 55 deletions(-) diff --git a/scripts/dh_speed.c b/scripts/dh_speed.c index 21652e787..0643ea92a 100644 --- a/scripts/dh_speed.c +++ b/scripts/dh_speed.c @@ -90,7 +90,7 @@ static void run_test(diffie_hellman_group_t group, int rounds) for (round = 0; round < rounds; round++) { assert(l[round]->get_my_public_value(l[round], &chunk)); - r->set_other_public_value(r, chunk); + assert(r->set_other_public_value(r, chunk)); chunk_free(&chunk); } @@ -98,7 +98,7 @@ static void run_test(diffie_hellman_group_t group, int rounds) 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); @@ -144,4 +144,3 @@ int main(int argc, char *argv[]) } return 0; } - diff --git a/src/charon-tkm/src/tkm/tkm_diffie_hellman.c b/src/charon-tkm/src/tkm/tkm_diffie_hellman.c index d4691fa63..c4953b6aa 100644 --- a/src/charon-tkm/src/tkm/tkm_diffie_hellman.c +++ b/src/charon-tkm/src/tkm/tkm_diffie_hellman.c @@ -70,16 +70,14 @@ METHOD(diffie_hellman_t, get_shared_secret, bool, } -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, diff --git a/src/charon-tkm/tests/keymat_tests.c b/src/charon-tkm/tests/keymat_tests.c index c1f3872f7..889965a78 100644 --- a/src/charon-tkm/tests/keymat_tests.c +++ b/src/charon-tkm/tests/keymat_tests.c @@ -54,7 +54,7 @@ START_TEST(test_derive_ike_keys) /* Use the same pubvalue for both sides */ chunk_t pubvalue; ck_assert(dh->dh.get_my_public_value(&dh->dh, &pubvalue)); - dh->dh.set_other_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), diff --git a/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c b/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c index faa586d17..e1c7c0e0b 100644 --- a/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c +++ b/src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c @@ -22,9 +22,10 @@ METHOD(diffie_hellman_t, get_my_public_value, bool, return TRUE; } -METHOD(diffie_hellman_t, set_other_public_value, void, +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, diff --git a/src/libcharon/sa/ikev1/phase1.c b/src/libcharon/sa/ikev1/phase1.c index a81fa5886..c968b2a9c 100644 --- a/src/libcharon/sa/ikev1/phase1.c +++ b/src/libcharon/sa/ikev1/phase1.c @@ -745,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) diff --git a/src/libcharon/sa/ikev1/tasks/quick_mode.c b/src/libcharon/sa/ikev1/tasks/quick_mode.c index ac82452c8..b48ace4ca 100644 --- a/src/libcharon/sa/ikev1/tasks/quick_mode.c +++ b/src/libcharon/sa/ikev1/tasks/quick_mode.c @@ -493,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; } diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index 7b1c44e7e..6d9132a68 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/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 */ @@ -893,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; @@ -1185,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: @@ -1480,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) @@ -1557,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)); diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.c b/src/libcharon/sa/ikev2/tasks/ike_init.c index 09860c93a..0d5700ef2 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_init.c +++ b/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) */ @@ -384,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)); } } @@ -576,6 +581,13 @@ 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"); @@ -701,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"); @@ -724,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); diff --git a/src/libimcv/pts/pts.c b/src/libimcv/pts/pts.c index e8e7600ea..1ca72098e 100644 --- a/src/libimcv/pts/pts.c +++ b/src/libimcv/pts/pts.c @@ -238,7 +238,10 @@ METHOD(pts_t, get_my_public_value, bool, 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) diff --git a/src/libstrongswan/crypto/diffie_hellman.h b/src/libstrongswan/crypto/diffie_hellman.h index f253f18c7..99df3cb6b 100644 --- a/src/libstrongswan/crypto/diffie_hellman.h +++ b/src/libstrongswan/crypto/diffie_hellman.h @@ -100,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. diff --git a/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c b/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c index 9714dd68f..80bd85d87 100644 --- a/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c +++ b/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c @@ -73,7 +73,7 @@ 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; @@ -88,7 +88,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 +112,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; } /** diff --git a/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c b/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c index 89740a0ec..0fbfc24aa 100644 --- a/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c +++ b/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c @@ -85,7 +85,7 @@ 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; @@ -142,6 +142,7 @@ 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, bool, diff --git a/src/libstrongswan/plugins/ntru/ntru_ke.c b/src/libstrongswan/plugins/ntru/ntru_ke.c index 5c7547577..3b5df81d9 100644 --- a/src/libstrongswan/plugins/ntru/ntru_ke.c +++ b/src/libstrongswan/plugins/ntru/ntru_ke.c @@ -106,7 +106,7 @@ 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, bool, @@ -153,8 +153,7 @@ METHOD(diffie_hellman_t, get_shared_secret, bool, 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) @@ -163,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; } @@ -186,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; @@ -205,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; @@ -213,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, @@ -302,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, @@ -327,4 +327,3 @@ ntru_ke_t *ntru_ke_create(diffie_hellman_group_t group, chunk_t g, chunk_t p) return &this->public; } - diff --git a/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c index 64b650b4a..7a0aa1a6a 100644 --- a/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c +++ b/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c @@ -87,7 +87,7 @@ METHOD(diffie_hellman_t, get_shared_secret, bool, } -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; @@ -100,10 +100,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, diff --git a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c index 54dfbd01b..9ef15b41e 100644 --- a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c +++ b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c @@ -216,23 +216,24 @@ 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 (!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, bool, diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c b/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c index 89ddf65e3..56102c5a4 100644 --- a/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c +++ b/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,17 +102,18 @@ 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) { switch (this->group) @@ -137,7 +138,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,7 +146,7 @@ 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, bool, @@ -444,4 +445,3 @@ pkcs11_dh_t *pkcs11_dh_create(diffie_hellman_group_t group, } return NULL; } - diff --git a/src/libstrongswan/tests/suites/test_ntru.c b/src/libstrongswan/tests/suites/test_ntru.c index 3264379d1..d209fa2bc 100644 --- a/src/libstrongswan/tests/suites/test_ntru.c +++ b/src/libstrongswan/tests/suites/test_ntru.c @@ -1083,14 +1083,14 @@ START_TEST(test_ntru_ke) 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); + 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); 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); + 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)); @@ -1136,7 +1136,7 @@ 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]); + 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); @@ -1158,7 +1158,7 @@ START_TEST(test_ntru_wrong_set) "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); + 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); @@ -1191,7 +1191,7 @@ START_TEST(test_ntru_ciphertext) { i_ntru = lib->crypto->create_dh(lib->crypto, NTRU_128_BIT); ck_assert(i_ntru->get_my_public_value(i_ntru, &pub_key)); - i_ntru->set_other_public_value(i_ntru, test[i]); + 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); @@ -1212,9 +1212,9 @@ START_TEST(test_ntru_wrong_ciphertext) 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)); - r_ntru->set_other_public_value(r_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)); - i_ntru->set_other_public_value(i_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); diff --git a/src/libtls/tls_peer.c b/src/libtls/tls_peer.c index 68a2c9421..e6be36b7b 100644 --- a/src/libtls/tls_peer.c +++ b/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; diff --git a/src/libtls/tls_server.c b/src/libtls/tls_server.c index 30ff0ad9e..b1a214f7f 100644 --- a/src/libtls/tls_server.c +++ b/src/libtls/tls_server.c @@ -494,7 +494,12 @@ 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->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"); From 0356089d0f94ab86dd82fd686703560988833e3c Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 23 Mar 2015 14:32:11 +0100 Subject: [PATCH 8/9] diffie-hellman: Verify public DH values in backends --- src/libstrongswan/crypto/diffie_hellman.c | 72 +++++++++++++++++++ src/libstrongswan/crypto/diffie_hellman.h | 11 ++- src/libstrongswan/plugins/gcrypt/gcrypt_dh.c | 5 ++ .../plugins/gmp/gmp_diffie_hellman.c | 5 ++ .../plugins/openssl/openssl_diffie_hellman.c | 5 ++ .../openssl/openssl_ec_diffie_hellman.c | 5 ++ src/libstrongswan/plugins/pkcs11/pkcs11_dh.c | 5 ++ 7 files changed, 107 insertions(+), 1 deletion(-) diff --git a/src/libstrongswan/crypto/diffie_hellman.c b/src/libstrongswan/crypto/diffie_hellman.c index ac106e9c4..0d4cd9109 100644 --- a/src/libstrongswan/crypto/diffie_hellman.c +++ b/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; +} diff --git a/src/libstrongswan/crypto/diffie_hellman.h b/src/libstrongswan/crypto/diffie_hellman.h index 99df3cb6b..4704cd0da 100644 --- a/src/libstrongswan/crypto/diffie_hellman.h +++ b/src/libstrongswan/crypto/diffie_hellman.h @@ -175,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_ @}*/ diff --git a/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c b/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c index 80bd85d87..744ec0bbf 100644 --- a/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c +++ b/src/libstrongswan/plugins/gcrypt/gcrypt_dh.c @@ -79,6 +79,11 @@ METHOD(diffie_hellman_t, set_other_public_value, bool, 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); diff --git a/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c b/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c index 0fbfc24aa..0ca24d76a 100644 --- a/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c +++ b/src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c @@ -90,6 +90,11 @@ METHOD(diffie_hellman_t, set_other_public_value, bool, { 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); diff --git a/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c index 7a0aa1a6a..2615d60a2 100644 --- a/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c +++ b/src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c @@ -92,6 +92,11 @@ METHOD(diffie_hellman_t, set_other_public_value, bool, { 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)); diff --git a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c index 9ef15b41e..550a5432f 100644 --- a/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c +++ b/src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c @@ -219,6 +219,11 @@ error: 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"); diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c b/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c index 56102c5a4..c0033bd8e 100644 --- a/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c +++ b/src/libstrongswan/plugins/pkcs11/pkcs11_dh.c @@ -116,6 +116,11 @@ static bool derive_secret(private_pkcs11_dh_t *this, chunk_t other) 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: From 41fc94c92454e965ca10cf8eabc8f9485a8c4576 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 23 Mar 2015 14:34:11 +0100 Subject: [PATCH 9/9] encoding: Remove DH public value verification from KE payload This commit reverts 84738b1a and 2ed5f569. As we have no DH group available in the KE payload for IKEv1, the verification can't work in that stage. Instead, we now verify DH groups in the DH backends, which works for any IKE version or any other purpose. --- src/libcharon/encoding/payloads/ke_payload.c | 73 -------------------- 1 file changed, 73 deletions(-) diff --git a/src/libcharon/encoding/payloads/ke_payload.c b/src/libcharon/encoding/payloads/ke_payload.c index c2599a682..50fd73f90 100644 --- a/src/libcharon/encoding/payloads/ke_payload.c +++ b/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; }