diffie-hellman: Add a bool return value to get_my_public_value()
authorMartin Willi <martin@revosec.ch>
Mon, 23 Mar 2015 10:37:27 +0000 (11:37 +0100)
committerMartin Willi <martin@revosec.ch>
Mon, 23 Mar 2015 16:54:03 +0000 (17:54 +0100)
23 files changed:
scripts/dh_speed.c
src/charon-tkm/src/tkm/tkm_diffie_hellman.c
src/charon-tkm/tests/diffie_hellman_tests.c
src/charon-tkm/tests/keymat_tests.c
src/libcharon/encoding/payloads/ke_payload.c
src/libcharon/plugins/ha/ha_dispatcher.c
src/libcharon/plugins/ha/ha_ike.c
src/libcharon/plugins/load_tester/load_tester_diffie_hellman.c
src/libcharon/sa/ikev1/authenticators/psk_v1_authenticator.c
src/libcharon/sa/ikev1/authenticators/pubkey_v1_authenticator.c
src/libcharon/sa/ikev1/keymat_v1.c
src/libcharon/sa/ikev2/tasks/ike_init.c
src/libimcv/pts/pts.c
src/libstrongswan/crypto/diffie_hellman.h
src/libstrongswan/plugins/gcrypt/gcrypt_dh.c
src/libstrongswan/plugins/gmp/gmp_diffie_hellman.c
src/libstrongswan/plugins/ntru/ntru_ke.c
src/libstrongswan/plugins/openssl/openssl_diffie_hellman.c
src/libstrongswan/plugins/openssl/openssl_ec_diffie_hellman.c
src/libstrongswan/plugins/pkcs11/pkcs11_dh.c
src/libstrongswan/tests/suites/test_ntru.c
src/libtls/tls_peer.c
src/libtls/tls_server.c

index 8a782d8..21652e7 100644 (file)
@@ -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,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++)
        {
index 02ae67f..d4691fa 100644 (file)
@@ -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,
index 89658a7..5ef6f41 100644 (file)
@@ -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");
index 1982671..c1f3872 100644 (file)
@@ -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,
index 7f3c4e4..c2599a6 100644 (file)
@@ -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;
 
index abd08e2..31eeb93 100644 (file)
@@ -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,
index 815cb53..6b4b53c 100644 (file)
@@ -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)
                {
index b248e78..faa586d 100644 (file)
 
 #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,
index aa966cd..bb187f0 100644 (file)
@@ -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,
index bfe5ff4..52228ef 100644 (file)
@@ -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,
index b171adc..f5a91db 100644 (file)
@@ -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;
 
index 1f59296..09860c9 100644 (file)
@@ -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;
index 8d13bfc..e8e7600 100644 (file)
@@ -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;
 }
index 79977d7..f253f18 100644 (file)
@@ -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.
index 44f33c9..9714dd6 100644 (file)
@@ -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,
index d07999d..89740a0 100644 (file)
@@ -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,
index 0aafd4c..5c75475 100644 (file)
@@ -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,
index 6035802..64b650b 100644 (file)
@@ -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,
index 625990b..54dfbd0 100644 (file)
@@ -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,
index 99702f9..89ddf65 100644 (file)
@@ -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,
index 5d5448f..3264379 100644 (file)
@@ -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);
index 82ec262..68a2c94 100644 (file)
@@ -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);
index df5d00a..30ff0ad 100644 (file)
@@ -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);