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 65c2e8d4ea9d6c53e517eaf3366fe4c4adffddaa..ef2807cf07f47b1b4f78f8a35a7dfc95d717dd08 100644 (file)
--- 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 b82a4df6fdb21cfe8c6fa232b88cadb2a93c16b6..ce73f0b9ac5765c790e6572b06c8ca1b79115192 100644 (file)
--- 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