From 0fc4dd429deafbf734ad687be73d5aaff7277c72 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 24 Apr 2014 14:06:05 +0200 Subject: [PATCH 1/7] proposal: Don't include AEAD algorithms in the default proposal According to RFC 5996 3.3 we should use a separate proposal for AEAD algorithms. This was not clear in RFC 5282, hence we previously included both AEAD and non-AEAD algorithms in a single proposal. --- src/libcharon/config/proposal.c | 133 +++++++++++++++++--------------- 1 file changed, 69 insertions(+), 64 deletions(-) diff --git a/src/libcharon/config/proposal.c b/src/libcharon/config/proposal.c index 2ecdb4f2e..fcfaebeb4 100644 --- a/src/libcharon/config/proposal.c +++ b/src/libcharon/config/proposal.c @@ -627,7 +627,7 @@ proposal_t *proposal_create(protocol_id_t protocol, u_int number) /** * Add supported IKE algorithms to proposal */ -static void proposal_add_supported_ike(private_proposal_t *this) +static void proposal_add_supported_ike(private_proposal_t *this, bool aead) { enumerator_t *enumerator; encryption_algorithm_t encryption; @@ -636,76 +636,81 @@ static void proposal_add_supported_ike(private_proposal_t *this) diffie_hellman_group_t group; const char *plugin_name; - enumerator = lib->crypto->create_crypter_enumerator(lib->crypto); - while (enumerator->enumerate(enumerator, &encryption, &plugin_name)) + if (aead) { - switch (encryption) + enumerator = lib->crypto->create_aead_enumerator(lib->crypto); + while (enumerator->enumerate(enumerator, &encryption, &plugin_name)) { - case ENCR_AES_CBC: - case ENCR_AES_CTR: - case ENCR_CAMELLIA_CBC: - case ENCR_CAMELLIA_CTR: - /* we assume that we support all AES/Camellia sizes */ - add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 128); - add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 192); - add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 256); - break; - case ENCR_3DES: - add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 0); - break; - case ENCR_DES: - /* no, thanks */ - break; - default: - break; + switch (encryption) + { + case ENCR_AES_CCM_ICV8: + case ENCR_AES_CCM_ICV12: + case ENCR_AES_CCM_ICV16: + case ENCR_AES_GCM_ICV8: + case ENCR_AES_GCM_ICV12: + case ENCR_AES_GCM_ICV16: + case ENCR_CAMELLIA_CCM_ICV8: + case ENCR_CAMELLIA_CCM_ICV12: + case ENCR_CAMELLIA_CCM_ICV16: + /* we assume that we support all AES/Camellia sizes */ + add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 128); + add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 192); + add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 256); + break; + default: + break; + } } + enumerator->destroy(enumerator); } - enumerator->destroy(enumerator); + else + { + enumerator = lib->crypto->create_crypter_enumerator(lib->crypto); + while (enumerator->enumerate(enumerator, &encryption, &plugin_name)) + { + switch (encryption) + { + case ENCR_AES_CBC: + case ENCR_AES_CTR: + case ENCR_CAMELLIA_CBC: + case ENCR_CAMELLIA_CTR: + /* we assume that we support all AES/Camellia sizes */ + add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 128); + add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 192); + add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 256); + break; + case ENCR_3DES: + add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 0); + break; + case ENCR_DES: + /* no, thanks */ + break; + default: + break; + } + } + enumerator->destroy(enumerator); - enumerator = lib->crypto->create_aead_enumerator(lib->crypto); - while (enumerator->enumerate(enumerator, &encryption, &plugin_name)) - { - switch (encryption) + enumerator = lib->crypto->create_signer_enumerator(lib->crypto); + while (enumerator->enumerate(enumerator, &integrity, &plugin_name)) { - case ENCR_AES_CCM_ICV8: - case ENCR_AES_CCM_ICV12: - case ENCR_AES_CCM_ICV16: - case ENCR_AES_GCM_ICV8: - case ENCR_AES_GCM_ICV12: - case ENCR_AES_GCM_ICV16: - case ENCR_CAMELLIA_CCM_ICV8: - case ENCR_CAMELLIA_CCM_ICV12: - case ENCR_CAMELLIA_CCM_ICV16: - /* we assume that we support all AES/Camellia sizes */ - add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 128); - add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 192); - add_algorithm(this, ENCRYPTION_ALGORITHM, encryption, 256); - break; - default: - break; + switch (integrity) + { + case AUTH_HMAC_SHA1_96: + case AUTH_HMAC_SHA2_256_128: + case AUTH_HMAC_SHA2_384_192: + case AUTH_HMAC_SHA2_512_256: + case AUTH_HMAC_MD5_96: + case AUTH_AES_XCBC_96: + case AUTH_AES_CMAC_96: + add_algorithm(this, INTEGRITY_ALGORITHM, integrity, 0); + break; + default: + break; + } } + enumerator->destroy(enumerator); } - enumerator->destroy(enumerator); - - enumerator = lib->crypto->create_signer_enumerator(lib->crypto); - while (enumerator->enumerate(enumerator, &integrity, &plugin_name)) - { - switch (integrity) - { - case AUTH_HMAC_SHA1_96: - case AUTH_HMAC_SHA2_256_128: - case AUTH_HMAC_SHA2_384_192: - case AUTH_HMAC_SHA2_512_256: - case AUTH_HMAC_MD5_96: - case AUTH_AES_XCBC_96: - case AUTH_AES_CMAC_96: - add_algorithm(this, INTEGRITY_ALGORITHM, integrity, 0); - break; - default: - break; - } - } - enumerator->destroy(enumerator); enumerator = lib->crypto->create_prf_enumerator(lib->crypto); while (enumerator->enumerate(enumerator, &prf, &plugin_name)) @@ -779,7 +784,7 @@ proposal_t *proposal_create_default(protocol_id_t protocol) switch (protocol) { case PROTO_IKE: - proposal_add_supported_ike(this); + proposal_add_supported_ike(this, FALSE); break; case PROTO_ESP: add_algorithm(this, ENCRYPTION_ALGORITHM, ENCR_AES_CBC, 128); From 8642f8bdb782e35be84183351a20a3df33e55586 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 24 Apr 2014 14:15:49 +0200 Subject: [PATCH 2/7] proposal: Use an additional "default" constructor specific to AEAD algorithms This allows a caller to create a separated proposal for supported AEAD algorithms, as required by RFC 5996. --- src/libcharon/config/proposal.c | 23 +++++++++++++++++++++++ src/libcharon/config/proposal.h | 8 ++++++++ 2 files changed, 31 insertions(+) diff --git a/src/libcharon/config/proposal.c b/src/libcharon/config/proposal.c index fcfaebeb4..48313b753 100644 --- a/src/libcharon/config/proposal.c +++ b/src/libcharon/config/proposal.c @@ -809,6 +809,29 @@ proposal_t *proposal_create_default(protocol_id_t protocol) return &this->public; } +/* + * Describtion in header-file + */ +proposal_t *proposal_create_default_aead(protocol_id_t protocol) +{ + private_proposal_t *this; + + switch (protocol) + { + case PROTO_IKE: + this = (private_proposal_t*)proposal_create(protocol, 0); + proposal_add_supported_ike(this, TRUE); + return &this->public; + case PROTO_ESP: + /* we currently don't include any AEAD proposal for ESP, as we + * don't know if our kernel backend actually supports it. */ + return NULL; + case PROTO_AH: + default: + return NULL; + } +} + /* * Describtion in header-file */ diff --git a/src/libcharon/config/proposal.h b/src/libcharon/config/proposal.h index 7733143a8..78b868868 100644 --- a/src/libcharon/config/proposal.h +++ b/src/libcharon/config/proposal.h @@ -195,6 +195,14 @@ proposal_t *proposal_create(protocol_id_t protocol, u_int number); */ proposal_t *proposal_create_default(protocol_id_t protocol); +/** + * Create a default proposal for supported AEAD algorithms + * + * @param protocol protocol, such as PROTO_ESP + * @return proposal_t object, NULL if none supported + */ +proposal_t *proposal_create_default_aead(protocol_id_t protocol); + /** * Create a proposal from a string identifying the algorithms. * From 3312c447ef099db8a95a553d7203ac28a58e062a Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 24 Apr 2014 14:19:12 +0200 Subject: [PATCH 3/7] ike-cfg: Allow passing NULL to add_proposal() This simplifies adding default proposals with constructors potentially returning NULL. --- src/libcharon/config/ike_cfg.c | 5 ++++- src/libcharon/config/ike_cfg.h | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libcharon/config/ike_cfg.c b/src/libcharon/config/ike_cfg.c index e08bb3f67..42a3e9057 100644 --- a/src/libcharon/config/ike_cfg.c +++ b/src/libcharon/config/ike_cfg.c @@ -281,7 +281,10 @@ METHOD(ike_cfg_t, get_dscp, u_int8_t, METHOD(ike_cfg_t, add_proposal, void, private_ike_cfg_t *this, proposal_t *proposal) { - this->proposals->insert_last(this->proposals, proposal); + if (proposal) + { + this->proposals->insert_last(this->proposals, proposal); + } } METHOD(ike_cfg_t, get_proposals, linked_list_t*, diff --git a/src/libcharon/config/ike_cfg.h b/src/libcharon/config/ike_cfg.h index f9e4fbebc..adfcabf70 100644 --- a/src/libcharon/config/ike_cfg.h +++ b/src/libcharon/config/ike_cfg.h @@ -148,9 +148,10 @@ struct ike_cfg_t { * Adds a proposal to the list. * * The first added proposal has the highest priority, the last - * added the lowest. + * added the lowest. It is safe to add NULL as proposal, which has no + * effect. * - * @param proposal proposal to add + * @param proposal proposal to add, or NULL */ void (*add_proposal) (ike_cfg_t *this, proposal_t *proposal); From 356846db5dfe57f813d2ec1cdbced75214fa878a Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 24 Apr 2014 14:20:21 +0200 Subject: [PATCH 4/7] child-cfg: Allow passing NULL as proposal to add_proposal() Making the API consistent to the one of ike_cfg. --- src/libcharon/config/child_cfg.c | 5 ++++- src/libcharon/config/child_cfg.h | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/libcharon/config/child_cfg.c b/src/libcharon/config/child_cfg.c index 8d8214ebe..3f07b5860 100644 --- a/src/libcharon/config/child_cfg.c +++ b/src/libcharon/config/child_cfg.c @@ -149,7 +149,10 @@ METHOD(child_cfg_t, get_name, char*, METHOD(child_cfg_t, add_proposal, void, private_child_cfg_t *this, proposal_t *proposal) { - this->proposals->insert_last(this->proposals, proposal); + if (proposal) + { + this->proposals->insert_last(this->proposals, proposal); + } } METHOD(child_cfg_t, get_proposals, linked_list_t*, diff --git a/src/libcharon/config/child_cfg.h b/src/libcharon/config/child_cfg.h index 20d1fa811..43ad1c5c8 100644 --- a/src/libcharon/config/child_cfg.h +++ b/src/libcharon/config/child_cfg.h @@ -73,10 +73,10 @@ struct child_cfg_t { * Add a proposal to the list. * * The proposals are stored by priority, first added - * is the most preferred. - * After add, proposal is owned by child_cfg. + * is the most preferred. It is safe to add NULL as proposal, which has no + * effect. After add, proposal is owned by child_cfg. * - * @param proposal proposal to add + * @param proposal proposal to add, or NULL */ void (*add_proposal) (child_cfg_t *this, proposal_t *proposal); From 879e3d12ca5e1496a8f34f82bd73ef8853e5d5e9 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 24 Apr 2014 14:24:43 +0200 Subject: [PATCH 5/7] ike: Add an additional but separate AEAD proposal to IKE config, if supported --- src/charon-cmd/cmd/cmd_connection.c | 1 + src/charon-nm/nm/nm_service.c | 1 + src/conftest/config.c | 1 + .../backend/android_service.c | 1 + src/frontends/osx/charon-xpc/xpc_dispatch.c | 1 + src/libcharon/plugins/ha/ha_tunnel.c | 1 + src/libcharon/plugins/maemo/maemo_service.c | 1 + src/libcharon/plugins/medcli/medcli_config.c | 2 ++ src/libcharon/plugins/medsrv/medsrv_config.c | 1 + src/libcharon/plugins/sql/sql_config.c | 1 + src/libcharon/plugins/stroke/stroke_config.c | 1 + src/libcharon/plugins/vici/vici_config.c | 35 +++++++++++++------ 12 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/charon-cmd/cmd/cmd_connection.c b/src/charon-cmd/cmd/cmd_connection.c index c4c8a2a96..79df8037b 100644 --- a/src/charon-cmd/cmd/cmd_connection.c +++ b/src/charon-cmd/cmd/cmd_connection.c @@ -187,6 +187,7 @@ static peer_cfg_t* create_peer_cfg(private_cmd_connection_t *this) else { ike_cfg->add_proposal(ike_cfg, proposal_create_default(PROTO_IKE)); + ike_cfg->add_proposal(ike_cfg, proposal_create_default_aead(PROTO_IKE)); } peer_cfg = peer_cfg_create("cmd", ike_cfg, CERT_SEND_IF_ASKED, UNIQUE_REPLACE, 1, /* keyingtries */ diff --git a/src/charon-nm/nm/nm_service.c b/src/charon-nm/nm/nm_service.c index 67366a067..82d212d20 100644 --- a/src/charon-nm/nm/nm_service.c +++ b/src/charon-nm/nm/nm_service.c @@ -532,6 +532,7 @@ static gboolean connect_(NMVPNPlugin *plugin, NMConnection *connection, (char*)address, IKEV2_UDP_PORT, FRAGMENTATION_NO, 0); ike_cfg->add_proposal(ike_cfg, proposal_create_default(PROTO_IKE)); + ike_cfg->add_proposal(ike_cfg, proposal_create_default_aead(PROTO_IKE)); peer_cfg = peer_cfg_create(priv->name, ike_cfg, CERT_SEND_IF_ASKED, UNIQUE_REPLACE, 1, /* keyingtries */ 36000, 0, /* rekey 10h, reauth none */ diff --git a/src/conftest/config.c b/src/conftest/config.c index 5aa742d79..bd63df02a 100644 --- a/src/conftest/config.c +++ b/src/conftest/config.c @@ -129,6 +129,7 @@ static ike_cfg_t *load_ike_config(private_config_t *this, else { ike_cfg->add_proposal(ike_cfg, proposal_create_default(PROTO_IKE)); + ike_cfg->add_proposal(ike_cfg, proposal_create_default_aead(PROTO_IKE)); } return ike_cfg; } diff --git a/src/frontends/android/jni/libandroidbridge/backend/android_service.c b/src/frontends/android/jni/libandroidbridge/backend/android_service.c index ccf5ce8e7..db9bebcc5 100644 --- a/src/frontends/android/jni/libandroidbridge/backend/android_service.c +++ b/src/frontends/android/jni/libandroidbridge/backend/android_service.c @@ -530,6 +530,7 @@ static job_requeue_t initiate(private_android_service_t *this) this->gateway, IKEV2_UDP_PORT, FRAGMENTATION_NO, 0); ike_cfg->add_proposal(ike_cfg, proposal_create_default(PROTO_IKE)); + ike_cfg->add_proposal(ike_cfg, proposal_create_default_aead(PROTO_IKE)); peer_cfg = peer_cfg_create("android", ike_cfg, CERT_SEND_IF_ASKED, UNIQUE_REPLACE, 0, /* keyingtries */ diff --git a/src/frontends/osx/charon-xpc/xpc_dispatch.c b/src/frontends/osx/charon-xpc/xpc_dispatch.c index 564fd6e7d..f20c54bce 100644 --- a/src/frontends/osx/charon-xpc/xpc_dispatch.c +++ b/src/frontends/osx/charon-xpc/xpc_dispatch.c @@ -87,6 +87,7 @@ static peer_cfg_t* create_peer_cfg(char *name, char *host) ike_cfg = ike_cfg_create(IKEV2, FALSE, FALSE, "0.0.0.0", local_port, host, remote_port, FRAGMENTATION_NO, 0); ike_cfg->add_proposal(ike_cfg, proposal_create_default(PROTO_IKE)); + ike_cfg->add_proposal(ike_cfg, proposal_create_default_aead(PROTO_IKE)); peer_cfg = peer_cfg_create(name, ike_cfg, CERT_SEND_IF_ASKED, UNIQUE_REPLACE, 1, /* keyingtries */ 36000, 0, /* rekey 10h, reauth none */ diff --git a/src/libcharon/plugins/ha/ha_tunnel.c b/src/libcharon/plugins/ha/ha_tunnel.c index 74147e553..53369008b 100644 --- a/src/libcharon/plugins/ha/ha_tunnel.c +++ b/src/libcharon/plugins/ha/ha_tunnel.c @@ -207,6 +207,7 @@ static void setup_tunnel(private_ha_tunnel_t *this, charon->socket->get_port(charon->socket, FALSE), remote, IKEV2_UDP_PORT, FRAGMENTATION_NO, 0); ike_cfg->add_proposal(ike_cfg, proposal_create_default(PROTO_IKE)); + ike_cfg->add_proposal(ike_cfg, proposal_create_default_aead(PROTO_IKE)); peer_cfg = peer_cfg_create("ha", ike_cfg, CERT_NEVER_SEND, UNIQUE_KEEP, 0, 86400, 0, 7200, 3600, FALSE, FALSE, TRUE, 30, 0, FALSE, NULL, NULL); diff --git a/src/libcharon/plugins/maemo/maemo_service.c b/src/libcharon/plugins/maemo/maemo_service.c index f0f3105c4..82e90694b 100644 --- a/src/libcharon/plugins/maemo/maemo_service.c +++ b/src/libcharon/plugins/maemo/maemo_service.c @@ -327,6 +327,7 @@ static gboolean initiate_connection(private_maemo_service_t *this, charon->socket->get_port(charon->socket, FALSE), hostname, IKEV2_UDP_PORT, FRAGMENTATION_NO, 0); ike_cfg->add_proposal(ike_cfg, proposal_create_default(PROTO_IKE)); + ike_cfg->add_proposal(ike_cfg, proposal_create_default_aead(PROTO_IKE)); peer_cfg = peer_cfg_create(this->current, ike_cfg, CERT_SEND_IF_ASKED, diff --git a/src/libcharon/plugins/medcli/medcli_config.c b/src/libcharon/plugins/medcli/medcli_config.c index d048b003b..c0b39e415 100644 --- a/src/libcharon/plugins/medcli/medcli_config.c +++ b/src/libcharon/plugins/medcli/medcli_config.c @@ -106,6 +106,7 @@ METHOD(backend_t, get_peer_cfg_by_name, peer_cfg_t*, charon->socket->get_port(charon->socket, FALSE), address, IKEV2_UDP_PORT, FRAGMENTATION_NO, 0); ike_cfg->add_proposal(ike_cfg, proposal_create_default(PROTO_IKE)); + ike_cfg->add_proposal(ike_cfg, proposal_create_default_aead(PROTO_IKE)); med_cfg = peer_cfg_create( "mediation", ike_cfg, CERT_NEVER_SEND, UNIQUE_REPLACE, @@ -382,6 +383,7 @@ medcli_config_t *medcli_config_create(database_t *db) FRAGMENTATION_NO, 0), ); this->ike->add_proposal(this->ike, proposal_create_default(PROTO_IKE)); + this->ike->add_proposal(this->ike, proposal_create_default_aead(PROTO_IKE)); schedule_autoinit(this); diff --git a/src/libcharon/plugins/medsrv/medsrv_config.c b/src/libcharon/plugins/medsrv/medsrv_config.c index ac6076ae8..02d805e06 100644 --- a/src/libcharon/plugins/medsrv/medsrv_config.c +++ b/src/libcharon/plugins/medsrv/medsrv_config.c @@ -145,6 +145,7 @@ medsrv_config_t *medsrv_config_create(database_t *db) FRAGMENTATION_NO, 0), ); this->ike->add_proposal(this->ike, proposal_create_default(PROTO_IKE)); + this->ike->add_proposal(this->ike, proposal_create_default_aead(PROTO_IKE)); return &this->public; } diff --git a/src/libcharon/plugins/sql/sql_config.c b/src/libcharon/plugins/sql/sql_config.c index a8d34f2d4..152c4ec52 100644 --- a/src/libcharon/plugins/sql/sql_config.c +++ b/src/libcharon/plugins/sql/sql_config.c @@ -242,6 +242,7 @@ static void add_ike_proposals(private_sql_config_t *this, if (use_default) { ike_cfg->add_proposal(ike_cfg, proposal_create_default(PROTO_IKE)); + ike_cfg->add_proposal(ike_cfg, proposal_create_default_aead(PROTO_IKE)); } } diff --git a/src/libcharon/plugins/stroke/stroke_config.c b/src/libcharon/plugins/stroke/stroke_config.c index e5e6d9246..aa6138bcf 100644 --- a/src/libcharon/plugins/stroke/stroke_config.c +++ b/src/libcharon/plugins/stroke/stroke_config.c @@ -174,6 +174,7 @@ static void add_proposals(private_stroke_config_t *this, char *string, if (ike_cfg) { ike_cfg->add_proposal(ike_cfg, proposal_create_default(proto)); + ike_cfg->add_proposal(ike_cfg, proposal_create_default_aead(proto)); } else { diff --git a/src/libcharon/plugins/vici/vici_config.c b/src/libcharon/plugins/vici/vici_config.c index 52a3dba16..83d221653 100644 --- a/src/libcharon/plugins/vici/vici_config.c +++ b/src/libcharon/plugins/vici/vici_config.c @@ -442,17 +442,24 @@ static bool parse_proposal(linked_list_t *list, protocol_id_t proto, chunk_t v) if (strcaseeq("default", buf)) { proposal = proposal_create_default(proto); + if (proposal) + { + list->insert_last(list, proposal); + } + proposal = proposal_create_default_aead(proto); + if (proposal) + { + list->insert_last(list, proposal); + } + return TRUE; } - else + proposal = proposal_create_from_string(proto, buf); + if (proposal) { - proposal = proposal_create_from_string(proto, buf); + list->insert_last(list, proposal); + return TRUE; } - if (!proposal) - { - return FALSE; - } - list->insert_last(list, proposal); - return TRUE; + return FALSE; } /** @@ -1755,8 +1762,16 @@ CALLBACK(config_sn, bool, } if (peer.proposals->get_count(peer.proposals) == 0) { - peer.proposals->insert_last(peer.proposals, - proposal_create_default(PROTO_IKE)); + proposal = proposal_create_default(PROTO_IKE); + if (proposal) + { + peer.proposals->insert_last(peer.proposals, proposal); + } + proposal = proposal_create_default_aead(PROTO_IKE); + if (proposal) + { + peer.proposals->insert_last(peer.proposals, proposal); + } } if (!peer.local_addrs) { From 8d74ec9e803805f259a2388d8f8e0d636a7d72f3 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 24 Apr 2014 14:28:57 +0200 Subject: [PATCH 6/7] ike: Add an additional but separate AEAD proposal to CHILD config This currently has no effect: We don't include AEAD algorithms in the default ESP proposal, as we don't know if it is supported by the backend. But as we hopefully get an algorithm query mechanism on kernel interfaces some day, we add the appropriate functionality nonetheless. --- src/charon-cmd/cmd/cmd_connection.c | 2 ++ src/charon-nm/nm/nm_service.c | 1 + src/conftest/config.c | 2 ++ src/frontends/osx/charon-xpc/xpc_dispatch.c | 1 + src/libcharon/plugins/ha/ha_tunnel.c | 1 + src/libcharon/plugins/maemo/maemo_service.c | 1 + src/libcharon/plugins/medcli/medcli_config.c | 2 ++ src/libcharon/plugins/sql/sql_config.c | 1 + src/libcharon/plugins/stroke/stroke_config.c | 1 + src/libcharon/plugins/vici/vici_config.c | 12 ++++++++++-- 10 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/charon-cmd/cmd/cmd_connection.c b/src/charon-cmd/cmd/cmd_connection.c index 79df8037b..2c0b7b9d5 100644 --- a/src/charon-cmd/cmd/cmd_connection.c +++ b/src/charon-cmd/cmd/cmd_connection.c @@ -358,6 +358,8 @@ static child_cfg_t* create_child_cfg(private_cmd_connection_t *this, else { child_cfg->add_proposal(child_cfg, proposal_create_default(PROTO_ESP)); + child_cfg->add_proposal(child_cfg, + proposal_create_default_aead(PROTO_ESP)); } while (this->local_ts->remove_first(this->local_ts, (void**)&ts) == SUCCESS) { diff --git a/src/charon-nm/nm/nm_service.c b/src/charon-nm/nm/nm_service.c index 82d212d20..fc7e89958 100644 --- a/src/charon-nm/nm/nm_service.c +++ b/src/charon-nm/nm/nm_service.c @@ -566,6 +566,7 @@ static gboolean connect_(NMVPNPlugin *plugin, NMConnection *connection, ACTION_NONE, ACTION_NONE, ACTION_NONE, ipcomp, 0, 0, NULL, NULL, 0); child_cfg->add_proposal(child_cfg, proposal_create_default(PROTO_ESP)); + child_cfg->add_proposal(child_cfg, proposal_create_default_aead(PROTO_ESP)); ts = traffic_selector_create_dynamic(0, 0, 65535); child_cfg->add_traffic_selector(child_cfg, TRUE, ts); ts = traffic_selector_create_from_string(0, TS_IPV4_ADDR_RANGE, diff --git a/src/conftest/config.c b/src/conftest/config.c index bd63df02a..c83db7ecd 100644 --- a/src/conftest/config.c +++ b/src/conftest/config.c @@ -181,6 +181,8 @@ static child_cfg_t *load_child_config(private_config_t *this, else { child_cfg->add_proposal(child_cfg, proposal_create_default(PROTO_ESP)); + child_cfg->add_proposal(child_cfg, + proposal_create_default_aead(PROTO_ESP)); } token = settings->get_str(settings, "configs.%s.%s.lts", NULL, config, child); diff --git a/src/frontends/osx/charon-xpc/xpc_dispatch.c b/src/frontends/osx/charon-xpc/xpc_dispatch.c index f20c54bce..04aad8735 100644 --- a/src/frontends/osx/charon-xpc/xpc_dispatch.c +++ b/src/frontends/osx/charon-xpc/xpc_dispatch.c @@ -141,6 +141,7 @@ static child_cfg_t* create_child_cfg(char *name) "aes128gcm8-aes128gcm12-aes128gcm16-" "aes256gcm8-aes256gcm12-aes256gcm16")); child_cfg->add_proposal(child_cfg, proposal_create_default(PROTO_ESP)); + child_cfg->add_proposal(child_cfg, proposal_create_default_aead(PROTO_ESP)); ts = traffic_selector_create_dynamic(0, 0, 65535); child_cfg->add_traffic_selector(child_cfg, TRUE, ts); ts = traffic_selector_create_from_string(0, TS_IPV4_ADDR_RANGE, diff --git a/src/libcharon/plugins/ha/ha_tunnel.c b/src/libcharon/plugins/ha/ha_tunnel.c index 53369008b..dd2399366 100644 --- a/src/libcharon/plugins/ha/ha_tunnel.c +++ b/src/libcharon/plugins/ha/ha_tunnel.c @@ -236,6 +236,7 @@ static void setup_tunnel(private_ha_tunnel_t *this, ts = traffic_selector_create_dynamic(IPPROTO_ICMP, 0, 65535); child_cfg->add_traffic_selector(child_cfg, FALSE, ts); child_cfg->add_proposal(child_cfg, proposal_create_default(PROTO_ESP)); + child_cfg->add_proposal(child_cfg, proposal_create_default_aead(PROTO_ESP)); peer_cfg->add_child_cfg(peer_cfg, child_cfg); this->backend.cfg = peer_cfg; diff --git a/src/libcharon/plugins/maemo/maemo_service.c b/src/libcharon/plugins/maemo/maemo_service.c index 82e90694b..2e96f8fb4 100644 --- a/src/libcharon/plugins/maemo/maemo_service.c +++ b/src/libcharon/plugins/maemo/maemo_service.c @@ -352,6 +352,7 @@ static gboolean initiate_connection(private_maemo_service_t *this, TRUE, MODE_TUNNEL, ACTION_NONE, ACTION_NONE, ACTION_NONE, FALSE, 0, 0, NULL, NULL, 0); child_cfg->add_proposal(child_cfg, proposal_create_default(PROTO_ESP)); + child_cfg->add_proposal(child_cfg, proposal_create_default_aead(PROTO_ESP)); ts = traffic_selector_create_dynamic(0, 0, 65535); child_cfg->add_traffic_selector(child_cfg, TRUE, ts); ts = traffic_selector_create_from_string(0, TS_IPV4_ADDR_RANGE, "0.0.0.0", diff --git a/src/libcharon/plugins/medcli/medcli_config.c b/src/libcharon/plugins/medcli/medcli_config.c index c0b39e415..1fb57b928 100644 --- a/src/libcharon/plugins/medcli/medcli_config.c +++ b/src/libcharon/plugins/medcli/medcli_config.c @@ -169,6 +169,7 @@ METHOD(backend_t, get_peer_cfg_by_name, peer_cfg_t*, ACTION_NONE, ACTION_NONE, ACTION_NONE, FALSE, 0, 0, NULL, NULL, 0); child_cfg->add_proposal(child_cfg, proposal_create_default(PROTO_ESP)); + child_cfg->add_proposal(child_cfg, proposal_create_default_aead(PROTO_ESP)); child_cfg->add_traffic_selector(child_cfg, TRUE, ts_from_string(local_net)); child_cfg->add_traffic_selector(child_cfg, FALSE, ts_from_string(remote_net)); peer_cfg->add_child_cfg(peer_cfg, child_cfg); @@ -243,6 +244,7 @@ METHOD(enumerator_t, peer_enumerator_enumerate, bool, ACTION_NONE, ACTION_NONE, ACTION_NONE, FALSE, 0, 0, NULL, NULL, 0); child_cfg->add_proposal(child_cfg, proposal_create_default(PROTO_ESP)); + child_cfg->add_proposal(child_cfg, proposal_create_default_aead(PROTO_ESP)); child_cfg->add_traffic_selector(child_cfg, TRUE, ts_from_string(local_net)); child_cfg->add_traffic_selector(child_cfg, FALSE, ts_from_string(remote_net)); this->current->add_child_cfg(this->current, child_cfg); diff --git a/src/libcharon/plugins/sql/sql_config.c b/src/libcharon/plugins/sql/sql_config.c index 152c4ec52..c47c7c0f8 100644 --- a/src/libcharon/plugins/sql/sql_config.c +++ b/src/libcharon/plugins/sql/sql_config.c @@ -153,6 +153,7 @@ static void add_esp_proposals(private_sql_config_t *this, if (use_default) { child->add_proposal(child, proposal_create_default(PROTO_ESP)); + child->add_proposal(child, proposal_create_default_aead(PROTO_ESP)); } } diff --git a/src/libcharon/plugins/stroke/stroke_config.c b/src/libcharon/plugins/stroke/stroke_config.c index aa6138bcf..df15a1608 100644 --- a/src/libcharon/plugins/stroke/stroke_config.c +++ b/src/libcharon/plugins/stroke/stroke_config.c @@ -179,6 +179,7 @@ static void add_proposals(private_stroke_config_t *this, char *string, else { child_cfg->add_proposal(child_cfg, proposal_create_default(proto)); + child_cfg->add_proposal(child_cfg, proposal_create_default_aead(proto)); } } diff --git a/src/libcharon/plugins/vici/vici_config.c b/src/libcharon/plugins/vici/vici_config.c index 83d221653..3f2fec444 100644 --- a/src/libcharon/plugins/vici/vici_config.c +++ b/src/libcharon/plugins/vici/vici_config.c @@ -1350,8 +1350,16 @@ CALLBACK(children_sn, bool, } if (child.proposals->get_count(child.proposals) == 0) { - child.proposals->insert_last(child.proposals, - proposal_create_default(PROTO_ESP)); + proposal = proposal_create_default(PROTO_ESP); + if (proposal) + { + child.proposals->insert_last(child.proposals, proposal); + } + proposal = proposal_create_default_aead(PROTO_ESP); + if (proposal) + { + child.proposals->insert_last(child.proposals, proposal); + } } /* if no hard lifetime specified, add one at soft lifetime + 10% */ From 2f893f278d5fe2bf43081bf5c341f9feaf3aedcb Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Fri, 16 May 2014 16:18:16 +0200 Subject: [PATCH 7/7] proposal: Don't return a default IKE proposal without encryption/AEAD algs --- src/libcharon/config/proposal.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/libcharon/config/proposal.c b/src/libcharon/config/proposal.c index 48313b753..4d881cd2f 100644 --- a/src/libcharon/config/proposal.c +++ b/src/libcharon/config/proposal.c @@ -627,7 +627,7 @@ proposal_t *proposal_create(protocol_id_t protocol, u_int number) /** * Add supported IKE algorithms to proposal */ -static void proposal_add_supported_ike(private_proposal_t *this, bool aead) +static bool proposal_add_supported_ike(private_proposal_t *this, bool aead) { enumerator_t *enumerator; encryption_algorithm_t encryption; @@ -662,6 +662,11 @@ static void proposal_add_supported_ike(private_proposal_t *this, bool aead) } } enumerator->destroy(enumerator); + + if (!array_count(this->transforms)) + { + return FALSE; + } } else { @@ -691,6 +696,11 @@ static void proposal_add_supported_ike(private_proposal_t *this, bool aead) } enumerator->destroy(enumerator); + if (!array_count(this->transforms)) + { + return FALSE; + } + enumerator = lib->crypto->create_signer_enumerator(lib->crypto); while (enumerator->enumerate(enumerator, &integrity, &plugin_name)) { @@ -772,6 +782,8 @@ static void proposal_add_supported_ike(private_proposal_t *this, bool aead) } } enumerator->destroy(enumerator); + + return TRUE; } /* @@ -784,7 +796,11 @@ proposal_t *proposal_create_default(protocol_id_t protocol) switch (protocol) { case PROTO_IKE: - proposal_add_supported_ike(this, FALSE); + if (!proposal_add_supported_ike(this, FALSE)) + { + destroy(this); + return NULL; + } break; case PROTO_ESP: add_algorithm(this, ENCRYPTION_ALGORITHM, ENCR_AES_CBC, 128); @@ -820,7 +836,11 @@ proposal_t *proposal_create_default_aead(protocol_id_t protocol) { case PROTO_IKE: this = (private_proposal_t*)proposal_create(protocol, 0); - proposal_add_supported_ike(this, TRUE); + if (!proposal_add_supported_ike(this, TRUE)) + { + destroy(this); + return NULL; + } return &this->public; case PROTO_ESP: /* we currently don't include any AEAD proposal for ESP, as we