From 3ba1f548c8bb8ce88ba11b356b8e32124a648ea0 Mon Sep 17 00:00:00 2001 From: Donat Zenichev Date: Fri, 9 Dec 2022 17:21:41 +0100 Subject: [PATCH] MT#56125 Add a flag to only accept/add allowed SDES crypto suits Add a new flag to only accept these individual crypto suites and none of the others. For example, `SDES-only-NULL_HMAC_SHA1_32` would only accept the crypto suite `NULL_HMAC_SHA1_32` for the offer being generated. This also takes precedence over the `SDES-no-` flag(s), if used together, so the `SDES-no` will be not taken into account. This has two effects: - if a given crypto suite was present in a received offer, it will be kept, so will be present in the outgoing offer; and - if a given crypto suite was not present in the received offer, it will be added to it. The rest, which is not mentioned, will be dropped/not added. Flag name: 'SDES-only-' Additionally: add another new flag 'SDES-nonew'. It will not add any new crypto suites into the offer. It takes precedence over the `SDES-no` and `SDES-only` flags, if used in combination. Change-Id: Ic4fa03957ee3d4d24b0c4f3fd003eada05f49b0b --- README.md | 17 +++ daemon/call.c | 192 ++++++++++++++++++------- daemon/call_interfaces.c | 12 ++ daemon/crypto.c | 1 + include/call_interfaces.h | 6 +- t/auto-daemon-tests.pl | 293 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 467 insertions(+), 54 deletions(-) diff --git a/README.md b/README.md index 3e3db3612..5592f897b 100644 --- a/README.md +++ b/README.md @@ -1237,6 +1237,23 @@ Optionally included keys are: offer, it will be removed and will be missing in the outgoing offer; and if a given crypto suite was not present in the received offer, it will not be added to it. + - `only-`*SUITE* + + Add only these individual crypto suites and none of the others. For example, + `only-NULL_HMAC_SHA1_32` would only accept the crypto suite `NULL_HMAC_SHA1_32` for + the offer being generated. This takes precedence over the `SDES-no-` flag(s), + if used together, so the `SDES-no` will be not taken into account. + This has two effects: if a given crypto suite was present in a received offer, it will + be kept, so will be present in the outgoing offer; and if a given crypto suite was not + present in the received offer, it will be added to it. The rest, which is not mentioned, + will be dropped/not added. + + - `nonew` + + Don't add any new crypto suites into the offer. This means, offered SDES crypto suites + will accepted, meanwhile no new is going to be generated by RTPEngine. + It takes precedence over the `SDES-no` and `SDES-only` flags, if used in combination. + - `pad` RFC 4568 (section 6.1) is somewhat ambiguous regarding the base64 encoding format of diff --git a/daemon/call.c b/daemon/call.c index 49a35d26a..3fdce02d5 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -1675,12 +1675,16 @@ static void __sdes_flags(struct crypto_params_sdes *cps, const struct sdp_ng_fla cps->params.session_params.unauthenticated_srtp = 0; } -/* generates SDES parameters for outgoing SDP, which is our media "out" direction */ -// `other` can be NULL +/** + * Only generates SDES parameters for outgoing SDP, which is our media "out" direction. + * `other` can be NULL. + */ static void __generate_crypto(const struct sdp_ng_flags *flags, struct call_media *this, struct call_media *other) { + /* SDES options, which will be present in the outgoing offer */ GQueue *cpq = &this->sdes_out; + /* SDES options coming to us for processing */ GQueue *cpq_in = &this->sdes_in; const GQueue *offered_cpq = other ? &other->sdes_in : NULL; @@ -1753,38 +1757,66 @@ static void __generate_crypto(const struct sdp_ng_flags *flags, struct call_medi /* SDES parameters below */ + /* OP_OFFER */ if (is_offer) { - // generate full set of params - // re-create the entire list - steal for later flushing + + /* generate full set of params + * re-create the entire list - steal for later flushing */ GQueue cpq_orig = *cpq; + + /* re-initialize it, in order to fill it out later, taking into account + * all the provided SDES flags and parameters */ g_queue_init(cpq); - // if we were offered some crypto suites, copy those first into our offer - unsigned int c_tag = 1; // tag for next crypto suite generated by us + /* if we were offered some crypto suites, copy those first into our offer */ + + unsigned int c_tag = 1; /* tag for next crypto suite generated by us */ unsigned long types_offered = 0; - // make sure our bit field is large enough + /* make sure our bit field is large enough */ assert(num_crypto_suites <= sizeof(types_offered) * 8); + /* add offered crypto parameters */ for (GList *l = offered_cpq ? offered_cpq->head : NULL; l; l = l->next) { struct crypto_params_sdes *offered_cps = l->data; + /* if we accept only certain SDES suites, then add only them, + * this takes precedence above the 'SDES-no-' flag(s). + * We mustn't check the 'flags->sdes_no' at all, if 'flags->sdes_only' is set. + */ + if (!flags->sdes_nonew && flags->sdes_only) { + if (!g_hash_table_lookup(flags->sdes_only, &offered_cps->params.crypto_suite->name_str)) { + ilogs(crypto, LOG_DEBUG, "'%s' crypto suite not added, because not one of 'SDES-only-'", + offered_cps->params.crypto_suite->name); + continue; + } + } + + /* SDES suites to be excluded */ + else if (!flags->sdes_nonew && flags->sdes_no && + g_hash_table_lookup(flags->sdes_no, &offered_cps->params.crypto_suite->name_str)) + { + ilogs(crypto, LOG_DEBUG, "Not offering crypto suite '%s' due to 'SDES-no' option", + offered_cps->params.crypto_suite->name); + continue; + } + struct crypto_params_sdes *cps = g_slice_alloc0(sizeof(*cps)); g_queue_push_tail(cpq, cps); cps->tag = offered_cps->tag; - // our own offered tags will be higher than the ones we received + /* our own offered tags will be higher than the ones we received */ if (cps->tag >= c_tag) c_tag = cps->tag + 1; crypto_params_copy(&cps->params, &offered_cps->params, 1); - // we use a bit field to keep track of which types we've seen here + /* we use a bit field to keep track of which types we've seen here */ types_offered |= 1 << cps->params.crypto_suite->idx; __sdes_flags(cps, flags); } - // if we had any suites added before, re-add those that aren't there yet + /* if we had any suites added before, re-add those that aren't there yet */ struct crypto_params_sdes *cps_orig; while ((cps_orig = g_queue_pop_head(&cpq_orig))) { if ((types_offered & (1 << cps_orig->params.crypto_suite->idx))) { @@ -1792,7 +1824,7 @@ static void __generate_crypto(const struct sdp_ng_flags *flags, struct call_medi continue; } - // make sure our tag is higher than what we've seen + /* make sure our tag is higher than what we've seen */ if (cps_orig->tag < c_tag) cps_orig->tag = c_tag; if (cps_orig->tag >= c_tag) @@ -1803,38 +1835,58 @@ static void __generate_crypto(const struct sdp_ng_flags *flags, struct call_medi types_offered |= 1 << cps_orig->params.crypto_suite->idx; } - // generate crypto suite offers for any types that we haven't seen above - // XXX for re-invites, this always creates new crypto keys for suites - // that weren't accepted before, instead of re-using the same keys (and - // suites) that were previously offered but not accepted - for (unsigned int i = 0; i < num_crypto_suites; i++) { - if ((types_offered & (1 << i))) - continue; + /* don't add any new crypto suites into the outgoing offer, if `SDES-nonew` is set */ + if (!flags->sdes_nonew) { - if (flags->sdes_no && g_hash_table_lookup(flags->sdes_no, - &crypto_suites[i].name_str)) - { - ilogs(crypto, LOG_DEBUG, "Not offering crypto suite '%s' " - "due to 'SDES-no' option", - crypto_suites[i].name); - continue; - } + /* generate crypto suite offers for any types that we haven't seen above. + * IMPORTANT: for re-invites, this always creates new crypto keys for suites + * that weren't accepted before, instead of re-using the same keys (and + * suites) that were previously offered but not accepted */ + for (unsigned int i = 0; i < num_crypto_suites; i++) { + if ((types_offered & (1 << i))) + continue; - struct crypto_params_sdes *cps = g_slice_alloc0(sizeof(*cps)); - g_queue_push_tail(cpq, cps); + /* if we accept only certain SDES suites, then add only them, + * this takes precedence above the 'SDES-no-' flag(s). + * We mustn't check the 'flags->sdes_no' at all, if 'flags->sdes_only' is set. + */ + if (flags->sdes_only) + { + if (!g_hash_table_lookup(flags->sdes_only, &crypto_suites[i].name_str)) { + ilogs(crypto, LOG_DEBUG, "'%s' crypto suite not added, because not one of 'SDES-only-'", + crypto_suites[i].name); + continue; + } + } + + /* SDES suites to be excluded */ + else if (flags->sdes_no && + g_hash_table_lookup(flags->sdes_no, &crypto_suites[i].name_str)) + { + ilogs(crypto, LOG_DEBUG, "Not offering crypto suite '%s' due to 'SDES-no' option", + crypto_suites[i].name); + continue; + } - cps->tag = c_tag++; - cps->params.crypto_suite = &crypto_suites[i]; - random_string((unsigned char *) cps->params.master_key, - cps->params.crypto_suite->master_key_len); - random_string((unsigned char *) cps->params.master_salt, - cps->params.crypto_suite->master_salt_len); - /* mki = mki_len = 0 */ + struct crypto_params_sdes *cps = g_slice_alloc0(sizeof(*cps)); + g_queue_push_tail(cpq, cps); - __sdes_flags(cps, flags); + cps->tag = c_tag++; + cps->params.crypto_suite = &crypto_suites[i]; + random_string((unsigned char *) cps->params.master_key, + cps->params.crypto_suite->master_key_len); + random_string((unsigned char *) cps->params.master_salt, + cps->params.crypto_suite->master_salt_len); + /* mki = mki_len = 0 */ + + __sdes_flags(cps, flags); + } } } - else { // OP_ANSWER + + /* OP_ANSWER */ + else + { // we pick the first supported crypto suite struct crypto_params_sdes *cps = cpq->head ? cpq->head->data : NULL; struct crypto_params_sdes *cps_in = cpq_in->head ? cpq_in->head->data : NULL; @@ -1919,42 +1971,78 @@ skip_sdes: this->fp_hash_func = dtls_find_hash_func(&flags->dtls_fingerprint); } } -// for an answer, uses the incoming received list of SDES crypto suites to prune -// the list of (generated) outgoing crypto suites to contain only the one that was -// accepted +/** + * Only accepts or declines proposed crypto suites. Does not generate. + * + * For an answer, uses the incoming received list of SDES crypto suites to prune + * the list of (generated) outgoing crypto suites to contain only the one that was + * accepted. + */ static void __sdes_accept(struct call_media *media, const struct sdp_ng_flags *flags) { if (!media->sdes_in.length) return; - if (flags && flags->sdes_no) { - // first remove SDES-no suites from offered ones - GList *l = media->sdes_in.head; + /* if 'flags->sdes_nonew' is set, don't prune anything, just pass all coming. + * 'flags->sdes_nonew' takes precedence over 'sdes_only' and 'sdes_no'. */ + if (flags && (flags->sdes_only || flags->sdes_no) && !flags->sdes_nonew) { + GList *l = media->sdes_in.tail; while (l) { struct crypto_params_sdes *offered_cps = l->data; - if (!g_hash_table_lookup(flags->sdes_no, + /* if 'SDES-only-' flag(s) present, then + * accept only those SDES suites mentioned in the 'SDES-only-', + * all the rest will be dropped / not added. + * This takes precedence over 'SDES-no-'. + * + * We mustn't check the 'flags->sdes_no' at all, if 'flags->sdes_only' is set. */ + if (flags->sdes_only) + { + if (g_hash_table_lookup(flags->sdes_only, &offered_cps->params.crypto_suite->name_str)) + { + l = l->prev; + continue; + } + } + + /* if 'SDES-no-' flag(s) present, then + * remove SDES-no suites from offered ones */ + else if (flags->sdes_no && !g_hash_table_lookup(flags->sdes_no, + &offered_cps->params.crypto_suite->name_str)) { - l = l->next; + l = l->prev; continue; } - ilogs(crypto, LOG_DEBUG, "Dropping offered crypto suite '%s' from offer " - "due to 'SDES-no' option", - offered_cps->params.crypto_suite->name); + /* stop the iteration intentionally, if only one suite is left + * this helps with a case, when the offerer left with no suites, + * which can be allowed, but we need to still have at least something */ + if (g_list_length(l) == 1) { + l = l->prev; + break; + } - GList *next = l->next; + ilogs(crypto, LOG_DEBUG, "Dropping offered crypto suite '%s' from offer due to %s", + offered_cps->params.crypto_suite->name, + flags->sdes_only ? "not being in SDES-only" : "SDES-no"); + + GList *prev = l->prev; g_queue_delete_link(&media->sdes_in, l); crypto_params_sdes_free(offered_cps); - l = next; + l = prev; } } if (media->sdes_in.head == NULL) return; + /* now prune those suites, which weren't accepted */ + + /* currently incoming suites */ struct crypto_params_sdes *cps_in = media->sdes_in.head->data; + /* outgoing suites */ GList *l = media->sdes_out.head; + while (l) { struct crypto_params_sdes *cps_out = l->data; if (cps_out->params.crypto_suite != cps_in->params.crypto_suite) @@ -1962,11 +2050,11 @@ static void __sdes_accept(struct call_media *media, const struct sdp_ng_flags *f if (cps_out->tag != cps_in->tag) goto del_next; - // this one's good + /* this one's good */ l = l->next; continue; del_next: - // mismatch, prune this one out + /* mismatch, prune this one out */ crypto_params_sdes_free(cps_out); GList *next = l->next; g_queue_delete_link(&media->sdes_out, l); diff --git a/daemon/call_interfaces.c b/daemon/call_interfaces.c index 366a8c88d..9dd343542 100644 --- a/daemon/call_interfaces.c +++ b/daemon/call_interfaces.c @@ -507,6 +507,11 @@ INLINE char *bencode_get_alt(bencode_item_t *i, const char *one, const char *two INLINE void ng_sdes_option(struct sdp_ng_flags *out, str *s, void *dummy) { str_hyphenate(s); + /* Accept only certain individual crypto suites */ + if (call_ng_flags_prefix(out, s, "only-", call_ng_flags_str_ht, &out->sdes_only)) + return; + + /* Exclude individual crypto suites */ if (call_ng_flags_prefix(out, s, "no-", call_ng_flags_str_ht, &out->sdes_no)) return; @@ -550,6 +555,9 @@ INLINE void ng_sdes_option(struct sdp_ng_flags *out, str *s, void *dummy) { case CSH_LOOKUP("static"): out->sdes_static = 1; break; + case CSH_LOOKUP("nonew"): + out->sdes_nonew = 1; + break; default: ilog(LOG_WARN, "Unknown 'SDES' flag encountered: '"STR_FORMAT"'", STR_FMT(s)); @@ -922,6 +930,8 @@ static void call_ng_flags_flags(struct sdp_ng_flags *out, str *s, void *dummy) { if (call_ng_flags_prefix(out, s, "from-tags-", call_ng_flags_esc_str_list, &out->from_tags)) return; + if (call_ng_flags_prefix(out, s, "SDES-only-", call_ng_flags_str_ht, &out->sdes_only)) + return; if (call_ng_flags_prefix(out, s, "SDES-no-", call_ng_flags_str_ht, &out->sdes_no)) return; if (call_ng_flags_prefix(out, s, "SDES-", ng_sdes_option, NULL)) @@ -1547,6 +1557,8 @@ void call_ng_free_flags(struct sdp_ng_flags *flags) { g_hash_table_destroy(flags->codec_set); if (flags->sdes_no) g_hash_table_destroy(flags->sdes_no); + if (flags->sdes_only) + g_hash_table_destroy(flags->sdes_only); if (flags->frequencies) g_array_free(flags->frequencies, true); g_queue_clear_full(&flags->from_tags, free); diff --git a/daemon/crypto.c b/daemon/crypto.c index 62122d72d..9611f229f 100644 --- a/daemon/crypto.c +++ b/daemon/crypto.c @@ -329,6 +329,7 @@ struct crypto_suite __crypto_suites[] = { }, }; +/* those crypto suites we can */ const struct crypto_suite *crypto_suites = __crypto_suites; const unsigned int num_crypto_suites = G_N_ELEMENTS(__crypto_suites); diff --git a/include/call_interfaces.h b/include/call_interfaces.h index c8e9de543..4968bbf16 100644 --- a/include/call_interfaces.h +++ b/include/call_interfaces.h @@ -53,7 +53,8 @@ struct sdp_ng_flags { GHashTable *codec_set; int ptime, rev_ptime; - GHashTable *sdes_no; + GHashTable *sdes_no; /* individual crypto suites which are excluded */ + GHashTable *sdes_only; /* individual crypto suites which are only accepted */ str dtls_fingerprint; enum { ICE_DEFAULT = 0, @@ -172,7 +173,8 @@ struct sdp_ng_flags { sdes_authenticated_srtp:1, sdes_lifetime:1, sdes_pad:1, - sdes_static:1, + sdes_static:1, + sdes_nonew:1, drop_traffic_start:1, drop_traffic_stop:1, passthrough_on:1, diff --git a/t/auto-daemon-tests.pl b/t/auto-daemon-tests.pl index 6f6e2b70f..e077a6213 100755 --- a/t/auto-daemon-tests.pl +++ b/t/auto-daemon-tests.pl @@ -13964,9 +13964,302 @@ a=sendrecv a=rtcp:PORT SDP +new_call; +offer('SDES no new crypto suites', { ICE => 'remove', DTLS => 'off', SDES => [ 'nonew' ] }, < 'remove' }, < 'remove', DTLS => 'off', SDES => [ 'only-AES_CM_128_HMAC_SHA1_80' ] }, < 'remove' }, < 'remove', DTLS => 'off', SDES => [ 'only-AES_CM_128_HMAC_SHA1_80', 'no-AES_CM_128_HMAC_SHA1_80' ] }, < 'remove' }, < 'remove', DTLS => 'off', SDES => [ 'only-AES_CM_128_HMAC_SHA1_80', 'no-AES_CM_128_HMAC_SHA1_32' ] }, < 'remove' }, < 'remove', DTLS => 'off', SDES => [ 'nonew', 'only-AES_CM_128_HMAC_SHA1_80', 'no-AES_CM_128_HMAC_SHA1_32' ] }, < 'remove' }, < 'remove', DTLS => 'off', SDES => [ 'only-AES_CM_128_HMAC_SHA1_80' ] }, < 'remove' }, <