From 14fa2d45d86b102e6b6b6993b4c2b7c18ff18b68 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Tue, 10 May 2022 14:37:40 +0200 Subject: [PATCH] proposal: Add constructor that does no checks or fixups This allows an easier construction of invalid proposals (e.g. IKE proposals without DH groups) than parsing tokens and using add_algorithm() manually. --- src/libstrongswan/crypto/proposal/proposal.c | 22 ++++++++-- src/libstrongswan/crypto/proposal/proposal.h | 18 ++++++++ .../tests/suites/test_proposal.c | 44 +++++++++++++++++++ 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/src/libstrongswan/crypto/proposal/proposal.c b/src/libstrongswan/crypto/proposal/proposal.c index f88f58ff1..00b72d5a1 100644 --- a/src/libstrongswan/crypto/proposal/proposal.c +++ b/src/libstrongswan/crypto/proposal/proposal.c @@ -1395,7 +1395,8 @@ proposal_t *proposal_create_default_aead(protocol_id_t protocol) /* * Described in header */ -proposal_t *proposal_create_from_string(protocol_id_t protocol, const char *algs) +proposal_t *proposal_create_from_string_unchecked(protocol_id_t protocol, + const char *algs) { private_proposal_t *this; enumerator_t *enumerator; @@ -1417,15 +1418,30 @@ proposal_t *proposal_create_from_string(protocol_id_t protocol, const char *algs } enumerator->destroy(enumerator); - if (failed || !check_proposal(this)) + if (failed) { destroy(this); return NULL; } - return &this->public; } +/* + * Described in header + */ +proposal_t *proposal_create_from_string(protocol_id_t protocol, const char *algs) +{ + proposal_t *this; + + this = proposal_create_from_string_unchecked(protocol, algs); + if (!this || !check_proposal((private_proposal_t*)this)) + { + DESTROY_IF(this); + return NULL; + } + return this; +} + /* * Described in header */ diff --git a/src/libstrongswan/crypto/proposal/proposal.h b/src/libstrongswan/crypto/proposal/proposal.h index cbb45309f..098a704cb 100644 --- a/src/libstrongswan/crypto/proposal/proposal.h +++ b/src/libstrongswan/crypto/proposal/proposal.h @@ -281,6 +281,24 @@ proposal_t *proposal_create_default_aead(protocol_id_t protocol); proposal_t *proposal_create_from_string(protocol_id_t protocol, const char *algs); +/** + * Create a proposal from a string identifying the algorithms without enforcing + * any checks or doing automatic fixups. + * + * The syntax is the same as for proposal_create_from_string(), but the + * proposal is constructed verbatim, like manually doing so via + * proposal_t::add_algorithm(). + * + * @warning This constructor can lead to invalid proposals (e.g. missing + * required or disallowed transforms). Use proposal_create_from_string() instead. + * + * @param protocol protocol, such as PROTO_ESP + * @param algs algorithms as string + * @return proposal_t object, NULL if invalid + */ +proposal_t *proposal_create_from_string_unchecked(protocol_id_t protocol, + const char *algs); + /** * Select a common proposal from the given lists of proposals. * diff --git a/src/libstrongswan/tests/suites/test_proposal.c b/src/libstrongswan/tests/suites/test_proposal.c index 78724bb6f..239441e95 100644 --- a/src/libstrongswan/tests/suites/test_proposal.c +++ b/src/libstrongswan/tests/suites/test_proposal.c @@ -110,6 +110,48 @@ START_TEST(test_create_from_string) } END_TEST +static struct { + protocol_id_t proto; + char *proposal; + char *expected; +} create_unchecked_data[] = { + { PROTO_IKE, "", NULL }, + { PROTO_IKE, "aesxxx", NULL }, + { PROTO_IKE, "aes128-sha256-prfsha384-modp3072", "IKE:AES_CBC_128/HMAC_SHA2_256_128/PRF_HMAC_SHA2_384/MODP_3072" }, + { PROTO_IKE, "aes128gcm16-prfsha256-modp3072", "IKE:AES_GCM_16_128/PRF_HMAC_SHA2_256/MODP_3072" }, + /* these are invalid because required transforms are missing */ + { PROTO_IKE, "aes128", "IKE:AES_CBC_128" }, + { PROTO_IKE, "sha256", "IKE:HMAC_SHA2_256_128" }, + { PROTO_IKE, "aes128-sha256-modp3072", "IKE:AES_CBC_128/HMAC_SHA2_256_128/MODP_3072" }, + { PROTO_IKE, "aes128gcm16", "IKE:AES_GCM_16_128" }, + /* mixing combined and classic algorithms isn't valid either */ + { PROTO_IKE, "aes128gcm16-aes128-modp3072", "IKE:AES_GCM_16_128/AES_CBC_128/MODP_3072" }, + { PROTO_IKE, "aes128gcm16-aes128-sha256-modp3072", "IKE:AES_GCM_16_128/AES_CBC_128/HMAC_SHA2_256_128/MODP_3072" }, + { PROTO_IKE, "aes128gcm16-sha256-modp3072", "IKE:AES_GCM_16_128/HMAC_SHA2_256_128/MODP_3072" }, + { PROTO_ESP, "", NULL }, + { 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" }, + /* these are invalid because required transforms are missing */ + { PROTO_ESP, "sha256", "ESP:HMAC_SHA2_256_128" }, + { PROTO_ESP, "aes128-sha256", "ESP:AES_CBC_128/HMAC_SHA2_256_128" }, + /* mixing combined and classic algorithms isn't valid either */ + { PROTO_ESP, "aes128gcm16-aes128-sha256", "ESP:AES_GCM_16_128/AES_CBC_128/HMAC_SHA2_256_128" }, + /* prfs are not valid in ESP proposals */ + { PROTO_ESP, "aes128-prfsha256", "ESP:AES_CBC_128/PRF_HMAC_SHA2_256" }, +}; + +START_TEST(test_create_from_string_unchecked) +{ + proposal_t *proposal; + + proposal = proposal_create_from_string_unchecked(create_unchecked_data[_i].proto, + create_unchecked_data[_i].proposal); + assert_proposal_eq(proposal, create_unchecked_data[_i].expected); + DESTROY_IF(proposal); +} +END_TEST + static struct { protocol_id_t proto; char *self; @@ -576,6 +618,8 @@ Suite *proposal_suite_create() tc = tcase_create("create_from_string"); tcase_add_loop_test(tc, test_create_from_string, 0, countof(create_data)); + tcase_add_loop_test(tc, test_create_from_string_unchecked, 0, + countof(create_unchecked_data)); suite_add_tcase(s, tc); tc = tcase_create("select");