From: Alan T. DeKok Date: Wed, 17 May 2023 12:11:51 +0000 (-0400) Subject: make MS-CHAP use flat or nested attributes. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=27edebd9cb03aeba832009575e00edccd4f197dc;p=thirdparty%2Ffreeradius-server.git make MS-CHAP use flat or nested attributes. As a minor optimization, search for responses in the same group as the challenge. Having them in another location doesn't make sense. We also likely need to update the FreeRADIUS "MS-CHAP-Foo" attributes which control the behavior of the MS-CHAP module. Perhaps we could auto-define module-specific attributes? --- diff --git a/src/modules/rlm_mschap/rlm_mschap.c b/src/modules/rlm_mschap/rlm_mschap.c index 65c2e8d4ea9..ef2807cf07f 100644 --- a/src/modules/rlm_mschap/rlm_mschap.c +++ b/src/modules/rlm_mschap/rlm_mschap.c @@ -324,7 +324,7 @@ static xlat_action_t mschap_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, * hash of MS-CHAPv2 challenge, and peer challenge. */ if (strncasecmp(arg->vb_strvalue, "Challenge", 9) == 0) { - chap_challenge = fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap_challenge); + chap_challenge = fr_pair_find_by_da_nested(&request->request_pairs, NULL, attr_ms_chap_challenge); if (!chap_challenge) { REDEBUG("No MS-CHAP-Challenge in the request"); return XLAT_ACTION_FAIL; @@ -350,7 +350,7 @@ static xlat_action_t mschap_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, char const *username_str; size_t username_len; - response = fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap2_response); + response = fr_pair_find_by_da_nested(&request->request_pairs, NULL, attr_ms_chap2_response); if (!response) { REDEBUG("Vendor-Specific.Microsoft.CHAP2-Response is required to calculate MS-CHAPv1 challenge"); return XLAT_ACTION_FAIL; @@ -428,8 +428,8 @@ static xlat_action_t mschap_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, * response. */ } else if (strncasecmp(arg->vb_strvalue, "NT-Response", 11) == 0) { - response = fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap_response); - if (!response) response = fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap2_response); + response = fr_pair_find_by_da_nested(&request->request_pairs, NULL, attr_ms_chap_response); + if (!response) response = fr_pair_find_by_da_nested(&request->request_pairs, NULL, attr_ms_chap2_response); if (!response) { REDEBUG("No MS-CHAP-Response or MS-CHAP2-Response was found in the request"); return XLAT_ACTION_FAIL; @@ -463,7 +463,7 @@ static xlat_action_t mschap_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, * in MS-CHAPv1, and not often there. */ } else if (strncasecmp(arg->vb_strvalue, "LM-Response", 11) == 0) { - response = fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap_response); + response = fr_pair_find_by_da_nested(&request->request_pairs, NULL, attr_ms_chap_response); if (!response) { REDEBUG("No MS-CHAP-Response was found in the request"); return XLAT_ACTION_FAIL; @@ -1456,13 +1456,20 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize(rlm_rcode_t *p_result, mod { rlm_mschap_t const *inst = talloc_get_type_abort_const(mctx->inst->data, rlm_mschap_t); fr_pair_t *challenge = NULL; + fr_pair_t *parent; - challenge = fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap_challenge); + challenge = fr_pair_find_by_da_nested(&request->request_pairs, NULL, attr_ms_chap_challenge); if (!challenge) RETURN_MODULE_NOOP; - if (!fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap_response) && - !fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap2_response) && - !fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap2_cpw)) { + /* + * The responses MUST be in the same group as the challenge. + */ + parent = fr_pair_parent(challenge); + fr_assert(parent != NULL); + + if (!fr_pair_find_by_da(&parent->vp_group, NULL, attr_ms_chap_response) && + !fr_pair_find_by_da(&parent->vp_group, NULL, attr_ms_chap2_response) && + !fr_pair_find_by_da(&parent->vp_group, NULL, attr_ms_chap2_cpw)) { RDEBUG2("Found MS-CHAP-Challenge, but no MS-CHAP response or Change-Password"); RETURN_MODULE_NOOP; } @@ -2016,6 +2023,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authenticate(rlm_rcode_t *p_result, fr_pair_t *challenge = NULL; fr_pair_t *response = NULL; fr_pair_t *cpw = NULL; + fr_pair_t *parent; fr_pair_t *nt_password = NULL, *smb_ctrl; uint8_t nthashhash[NT_DIGEST_LENGTH]; int mschap_version = 0; @@ -2081,7 +2089,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authenticate(rlm_rcode_t *p_result, * Check to see if this is a change password request, and process * it accordingly if so. */ - cpw = fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap2_cpw); + cpw = fr_pair_find_by_da_nested(&request->request_pairs, NULL, attr_ms_chap2_cpw); if (cpw) { uint8_t *p; @@ -2113,7 +2121,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authenticate(rlm_rcode_t *p_result, memcpy(p + 2, cpw->vp_octets + 18, 48); } - challenge = fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap_challenge); + challenge = fr_pair_find_by_da_nested(&request->request_pairs, NULL, attr_ms_chap_challenge); if (!challenge) { REDEBUG("&control.Auth-Type = %s set for a request that does not contain &%s", mctx->inst->name, attr_ms_chap_challenge->name); @@ -2121,10 +2129,16 @@ static unlang_action_t CC_HINT(nonnull) mod_authenticate(rlm_rcode_t *p_result, goto finish; } + /* + * The responses MUST be in the same group as the challenge. + */ + parent = fr_pair_parent(challenge); + fr_assert(parent != NULL); + /* * We also require an MS-CHAP-Response. */ - if ((response = fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap_response))) { + if ((response = fr_pair_find_by_da(&parent->vp_group, NULL, attr_ms_chap_response))) { mschap_process_response(&rcode, &mschap_version, nthashhash, inst, request, @@ -2132,7 +2146,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authenticate(rlm_rcode_t *p_result, challenge, response, method); if (rcode != RLM_MODULE_OK) goto finish; - } else if ((response = fr_pair_find_by_da(&request->request_pairs, NULL, attr_ms_chap2_response))) { + } else if ((response = fr_pair_find_by_da_nested(&parent->vp_group, NULL, attr_ms_chap2_response))) { mschap_process_v2_response(&rcode, &mschap_version, nthashhash, inst, request, diff --git a/src/tests/keywords/all.mk b/src/tests/keywords/all.mk index b82a4df6fdb..ce73f0b9ac5 100644 --- a/src/tests/keywords/all.mk +++ b/src/tests/keywords/all.mk @@ -79,6 +79,10 @@ else ifneq "$(findstring ${1}, $(KEYWORD_UPDATE_REWRITE_TESTS))" "" $(OUTPUT)/${1}: NEW_COND=-S use_new_conditions=yes -S rewrite_update=yes else $(OUTPUT)/${1}: NEW_COND=-S use_new_conditions=yes -S forbid_update=yes + +ifeq "${1}" "mschap" +$(OUTPUT)/${1}: rlm_mschap.la +endif endif endef