diff --git a/src/libcharon/config/proposal.c b/src/libcharon/config/proposal.c index ddbc155c3..011c0b8b0 100644 --- a/src/libcharon/config/proposal.c +++ b/src/libcharon/config/proposal.c @@ -419,25 +419,41 @@ static const struct { {AUTH_AES_CMAC_96, PRF_AES128_CMAC }, }; +/** + * Remove all entries of the given transform type + */ +static void remove_transform(private_proposal_t *this, transform_type_t type) +{ + enumerator_t *e; + entry_t *entry; + + e = array_create_enumerator(this->transforms); + while (e->enumerate(e, &entry)) + { + if (entry->type == type) + { + array_remove_at(this->transforms, e); + } + } + e->destroy(e); +} + /** * Checks the proposal read from a string. */ -static void check_proposal(private_proposal_t *this) +static bool check_proposal(private_proposal_t *this) { enumerator_t *e; entry_t *entry; uint16_t alg, ks; - bool all_aead = TRUE; + bool all_aead = TRUE, any_aead = FALSE, any_enc = FALSE; int i; if (this->protocol == PROTO_IKE) { - e = create_enumerator(this, PSEUDO_RANDOM_FUNCTION); - if (!e->enumerate(e, &alg, &ks)) - { - /* No explicit PRF found. We assume the same algorithm as used - * for integrity checking */ - e->destroy(e); + if (!get_algorithm(this, PSEUDO_RANDOM_FUNCTION, NULL, NULL)) + { /* No explicit PRF found. We assume the same algorithm as used + * for integrity checking. */ e = create_enumerator(this, INTEGRITY_ALGORITHM); while (e->enumerate(e, &alg, &ks)) { @@ -451,8 +467,13 @@ static void check_proposal(private_proposal_t *this) } } } + e->destroy(e); + } + if (!get_algorithm(this, PSEUDO_RANDOM_FUNCTION, NULL, NULL)) + { + DBG1(DBG_CFG, "a PRF algorithm is mandatory in IKE proposals"); + return FALSE; } - e->destroy(e); /* remove MODP_NONE from IKE proposal */ e = array_create_enumerator(this->transforms); while (e->enumerate(e, &entry)) @@ -463,48 +484,103 @@ static void check_proposal(private_proposal_t *this) } } e->destroy(e); + if (!get_algorithm(this, DIFFIE_HELLMAN_GROUP, NULL, NULL)) + { + DBG1(DBG_CFG, "a DH group is mandatory in IKE proposals"); + return FALSE; + } + } + else + { /* remove PRFs from ESP/AH proposals */ + remove_transform(this, PSEUDO_RANDOM_FUNCTION); } - if (this->protocol == PROTO_ESP) + if (this->protocol == PROTO_IKE || this->protocol == PROTO_ESP) { e = create_enumerator(this, ENCRYPTION_ALGORITHM); while (e->enumerate(e, &alg, &ks)) { - if (!encryption_algorithm_is_aead(alg)) + any_enc = TRUE; + if (encryption_algorithm_is_aead(alg)) { - all_aead = FALSE; - break; + any_aead = TRUE; + continue; + } + all_aead = FALSE; + } + e->destroy(e); + + if (!any_enc) + { + DBG1(DBG_CFG, "an encryption algorithm is mandatory in %N proposals", + protocol_id_names, this->protocol); + return FALSE; + } + else if (any_aead && !all_aead) + { + DBG1(DBG_CFG, "classic and combined-mode (AEAD) encryption " + "algorithms can't be contained in the same %N proposal", + protocol_id_names, this->protocol); + return FALSE; + } + else if (all_aead) + { /* if all encryption algorithms in the proposal are AEADs, + * we MUST NOT propose any integrity algorithms */ + remove_transform(this, INTEGRITY_ALGORITHM); + } + } + else + { /* AES-GMAC is parsed as encryption algorithm, so we map that to the + * proper integrity algorithm */ + e = array_create_enumerator(this->transforms); + while (e->enumerate(e, &entry)) + { + if (entry->type == ENCRYPTION_ALGORITHM) + { + if (entry->alg == ENCR_NULL_AUTH_AES_GMAC) + { + entry->type = INTEGRITY_ALGORITHM; + ks = entry->key_size; + entry->key_size = 0; + switch (ks) + { + case 128: + entry->alg = AUTH_AES_128_GMAC; + continue; + case 192: + entry->alg = AUTH_AES_192_GMAC; + continue; + case 256: + entry->alg = AUTH_AES_256_GMAC; + continue; + default: + break; + } + } + /* remove all other encryption algorithms */ + array_remove_at(this->transforms, e); } } e->destroy(e); - if (all_aead) + if (!get_algorithm(this, INTEGRITY_ALGORITHM, NULL, NULL)) { - /* if all encryption algorithms in the proposal are AEADs, - * we MUST NOT propose any integrity algorithms */ - e = array_create_enumerator(this->transforms); - while (e->enumerate(e, &entry)) - { - if (entry->type == INTEGRITY_ALGORITHM) - { - array_remove_at(this->transforms, e); - } - } - e->destroy(e); + DBG1(DBG_CFG, "an integrity algorithm is mandatory in AH " + "proposals"); + return FALSE; } } if (this->protocol == PROTO_AH || this->protocol == PROTO_ESP) { - e = create_enumerator(this, EXTENDED_SEQUENCE_NUMBERS); - if (!e->enumerate(e, NULL, NULL)) + if (!get_algorithm(this, EXTENDED_SEQUENCE_NUMBERS, NULL, NULL)) { /* ESN not specified, assume not supported */ add_algorithm(this, EXTENDED_SEQUENCE_NUMBERS, NO_EXT_SEQ_NUMBERS, 0); } - e->destroy(e); } array_compress(this->transforms); + return TRUE; } /** @@ -1000,13 +1076,11 @@ proposal_t *proposal_create_from_string(protocol_id_t protocol, const char *algs } enumerator->destroy(enumerator); - if (failed) + if (failed || !check_proposal(this)) { destroy(this); return NULL; } - check_proposal(this); - return &this->public; } diff --git a/src/libcharon/sa/ikev2/tasks/ike_init.c b/src/libcharon/sa/ikev2/tasks/ike_init.c index 92909cff8..d82e206b8 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_init.c +++ b/src/libcharon/sa/ikev2/tasks/ike_init.c @@ -655,6 +655,7 @@ METHOD(task_t, build_r, status_t, else { DBG1(DBG_IKE, "no acceptable proposal found"); + message->add_notify(message, TRUE, NO_PROPOSAL_CHOSEN, chunk_empty); } return FAILED; } diff --git a/src/libcharon/tests/libcharon_tests.c b/src/libcharon/tests/libcharon_tests.c index e25e5434f..1ef13e978 100644 --- a/src/libcharon/tests/libcharon_tests.c +++ b/src/libcharon/tests/libcharon_tests.c @@ -32,6 +32,22 @@ static test_configuration_t tests[] = { { .suite = NULL, } }; +static void initialize_logging() +{ + int level = LEVEL_SILENT; + char *verbosity; + + verbosity = getenv("TESTS_VERBOSITY"); + if (verbosity) + { + level = atoi(verbosity); + } + lib->settings->set_int(lib->settings, "%s.filelog.stderr.default", + lib->settings->get_int(lib->settings, "%s.filelog.stderr.default", + level, lib->ns), lib->ns); + charon->load_loggers(charon, NULL, TRUE); +} + static bool test_runner_init(bool init) { if (init) @@ -39,6 +55,7 @@ static bool test_runner_init(bool init) char *plugins, *plugindir; libcharon_init(); + initialize_logging(); plugins = getenv("TESTS_PLUGINS") ?: lib->settings->get_str(lib->settings, diff --git a/src/libcharon/tests/suites/test_proposal.c b/src/libcharon/tests/suites/test_proposal.c index a6226f68f..19f4cd1e1 100644 --- a/src/libcharon/tests/suites/test_proposal.c +++ b/src/libcharon/tests/suites/test_proposal.c @@ -18,38 +18,100 @@ #include static struct { + protocol_id_t proto; + char *proposal; + char *expected; +} create_data[] = { + { PROTO_IKE, "", NULL }, + { PROTO_IKE, "sha256", NULL }, + { PROTO_IKE, "sha256-modp3072", NULL }, + { PROTO_IKE, "null-sha256-modp3072", "IKE:NULL/HMAC_SHA2_256_128/PRF_HMAC_SHA2_256/MODP_3072" }, + { PROTO_IKE, "aes128", NULL }, + { PROTO_IKE, "aes128-sha256", NULL }, + { PROTO_IKE, "aes128-sha256-modpnone", NULL }, + { PROTO_IKE, "aes128-sha256-modp3072", "IKE:AES_CBC_128/HMAC_SHA2_256_128/PRF_HMAC_SHA2_256/MODP_3072" }, + { PROTO_IKE, "aes128-sha256-prfsha384-modp3072", "IKE:AES_CBC_128/HMAC_SHA2_256_128/PRF_HMAC_SHA2_384/MODP_3072" }, + { PROTO_IKE, "aes128gcm16-modp3072", NULL }, + { PROTO_IKE, "aes128gcm16-prfsha256-modp3072", "IKE:AES_GCM_16_128/PRF_HMAC_SHA2_256/MODP_3072" }, + { PROTO_IKE, "aes128gcm16-sha256-modp3072", "IKE:AES_GCM_16_128/PRF_HMAC_SHA2_256/MODP_3072" }, + { PROTO_IKE, "aes128gcm16-aes128-modp3072", NULL }, + { PROTO_IKE, "aes128gcm16-aes128-sha256-modp3072", NULL }, + { PROTO_ESP, "", NULL }, + { PROTO_ESP, "sha256", NULL }, + { PROTO_ESP, "aes128-sha256", "ESP:AES_CBC_128/HMAC_SHA2_256_128/NO_EXT_SEQ" }, + { PROTO_ESP, "aes128-sha256-esn", "ESP:AES_CBC_128/HMAC_SHA2_256_128/EXT_SEQ" }, + { PROTO_ESP, "aes128-sha256-noesn", "ESP:AES_CBC_128/HMAC_SHA2_256_128/NO_EXT_SEQ" }, + { PROTO_ESP, "aes128-sha256-esn-noesn", "ESP:AES_CBC_128/HMAC_SHA2_256_128/EXT_SEQ/NO_EXT_SEQ" }, + { PROTO_ESP, "aes128-sha256-prfsha256-modp3072", "ESP:AES_CBC_128/HMAC_SHA2_256_128/MODP_3072/NO_EXT_SEQ" }, + { PROTO_ESP, "aes128gcm16-aes128-sha256-modp3072", NULL }, + { PROTO_ESP, "aes128gmac", "ESP:NULL_AES_GMAC_128/NO_EXT_SEQ" }, + { PROTO_AH, "", NULL }, + { PROTO_AH, "aes128", NULL }, + { PROTO_AH, "aes128-sha256", "AH:HMAC_SHA2_256_128/NO_EXT_SEQ" }, + { PROTO_AH, "sha256-sha1", "AH:HMAC_SHA2_256_128/HMAC_SHA1_96/NO_EXT_SEQ" }, + { PROTO_AH, "aes128gmac-sha256", "AH:AES_128_GMAC/HMAC_SHA2_256_128/NO_EXT_SEQ" }, + { PROTO_AH, "aes128gmac-sha256-prfsha256", "AH:AES_128_GMAC/HMAC_SHA2_256_128/NO_EXT_SEQ" }, + { PROTO_AH, "aes128gmac-aes256gmac-aes128-sha256", "AH:AES_128_GMAC/AES_256_GMAC/HMAC_SHA2_256_128/NO_EXT_SEQ" }, + { PROTO_AH, "sha256-esn", "AH:HMAC_SHA2_256_128/EXT_SEQ" }, + { PROTO_AH, "sha256-noesn", "AH:HMAC_SHA2_256_128/NO_EXT_SEQ" }, + { PROTO_AH, "sha256-esn-noesn", "AH:HMAC_SHA2_256_128/EXT_SEQ/NO_EXT_SEQ" }, +}; + +START_TEST(test_create_from_string) +{ + proposal_t *proposal; + char str[BUF_LEN]; + + proposal = proposal_create_from_string(create_data[_i].proto, + create_data[_i].proposal); + if (!create_data[_i].expected) + { + ck_assert(!proposal); + return; + } + snprintf(str, sizeof(str), "%P", proposal); + ck_assert_str_eq(create_data[_i].expected, str); + proposal->destroy(proposal); +} +END_TEST + +static struct { + protocol_id_t proto; char *self; char *other; char *expected; } select_data[] = { - { "aes128", "aes128", "aes128" }, - { "aes128", "aes256", NULL }, - { "aes128-aes256", "aes256-aes128", "aes128" }, - { "aes256-aes128", "aes128-aes256", "aes256" }, - { "aes128-aes256-sha1-sha256", "aes256-aes128-sha256-sha1", "aes128-sha1" }, - { "aes256-aes128-sha256-sha1", "aes128-aes256-sha1-sha256", "aes256-sha256" }, - { "aes128-sha256-modp3072", "aes128-sha256", NULL }, - { "aes128-sha256", "aes128-sha256-modp3072", NULL }, - { "aes128-sha256-modp3072", "aes128-sha256-modpnone", NULL }, - { "aes128-sha256-modpnone", "aes128-sha256-modp3072", NULL }, - { "aes128-sha256-modp3072-modpnone", "aes128-sha256", "aes128-sha256" }, - { "aes128-sha256", "aes128-sha256-modp3072-modpnone", "aes128-sha256" }, - { "aes128-sha256-modp3072-modpnone", "aes128-sha256-modpnone-modp3072", "aes128-sha256-modp3072" }, - { "aes128-sha256-modpnone-modp3072", "aes128-sha256-modp3072-modpnone", "aes128-sha256-modpnone" }, + { PROTO_ESP, "aes128", "aes128", "aes128" }, + { PROTO_ESP, "aes128", "aes256", NULL }, + { PROTO_ESP, "aes128-aes256", "aes256-aes128", "aes128" }, + { PROTO_ESP, "aes256-aes128", "aes128-aes256", "aes256" }, + { PROTO_ESP, "aes128-aes256-sha1-sha256", "aes256-aes128-sha256-sha1", "aes128-sha1" }, + { PROTO_ESP, "aes256-aes128-sha256-sha1", "aes128-aes256-sha1-sha256", "aes256-sha256" }, + { PROTO_ESP, "aes128-sha256-modp3072", "aes128-sha256", NULL }, + { PROTO_ESP, "aes128-sha256", "aes128-sha256-modp3072", NULL }, + { PROTO_ESP, "aes128-sha256-modp3072", "aes128-sha256-modpnone", NULL }, + { PROTO_ESP, "aes128-sha256-modpnone", "aes128-sha256-modp3072", NULL }, + { PROTO_ESP, "aes128-sha256-modp3072-modpnone", "aes128-sha256", "aes128-sha256" }, + { PROTO_ESP, "aes128-sha256", "aes128-sha256-modp3072-modpnone", "aes128-sha256" }, + { PROTO_ESP, "aes128-sha256-modp3072-modpnone", "aes128-sha256-modpnone-modp3072", "aes128-sha256-modp3072" }, + { PROTO_ESP, "aes128-sha256-modpnone-modp3072", "aes128-sha256-modp3072-modpnone", "aes128-sha256-modpnone" }, + { PROTO_IKE, "aes128-sha256-modp3072", "aes128-sha256-modp3072", "aes128-sha256-modp3072" }, + { PROTO_IKE, "aes128-sha256-modp3072", "aes128-sha256-modp3072-modpnone", "aes128-sha256-modp3072" }, + { PROTO_IKE, "aes128-sha256-modp3072-modpnone", "aes128-sha256-modp3072", "aes128-sha256-modp3072" }, }; START_TEST(test_select) { proposal_t *self, *other, *selected, *expected; - self = proposal_create_from_string(PROTO_ESP, + self = proposal_create_from_string(select_data[_i].proto, select_data[_i].self); - other = proposal_create_from_string(PROTO_ESP, + other = proposal_create_from_string(select_data[_i].proto, select_data[_i].other); selected = self->select(self, other, FALSE); if (select_data[_i].expected) { - expected = proposal_create_from_string(PROTO_ESP, + expected = proposal_create_from_string(select_data[_i].proto, select_data[_i].expected); ck_assert(selected); ck_assert_msg(expected->equals(expected, selected), "proposal %P does " @@ -73,6 +135,10 @@ Suite *proposal_suite_create() s = suite_create("proposal"); + tc = tcase_create("create_from_string"); + tcase_add_loop_test(tc, test_create_from_string, 0, countof(create_data)); + suite_add_tcase(s, tc); + tc = tcase_create("select"); tcase_add_loop_test(tc, test_select, 0, countof(select_data)); suite_add_tcase(s, tc);