]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
Remove fr_pair_find_by_num
authorArran Cudbard-Bell <a.cudbardb@freeradius.org>
Sun, 18 Apr 2021 15:54:28 +0000 (10:54 -0500)
committerArran Cudbard-Bell <a.cudbardb@freeradius.org>
Sun, 18 Apr 2021 15:59:20 +0000 (10:59 -0500)
src/lib/server/auth.c
src/lib/server/paircmp.c
src/lib/util/pair.c
src/lib/util/pair.h
src/lib/util/pair_tests.c

index 9f9e66dd77871bf17593b6fb4762d94d360638f2..019f6d344e8e642bd268db4b25276279bc80ff01 100644 (file)
@@ -47,111 +47,116 @@ RCSID("$Id$")
  */
 unlang_action_t rad_virtual_server(rlm_rcode_t *p_result, request_t *request)
 {
-       fr_pair_t *vp, *username, *parent_username = NULL;
        rlm_rcode_t final;
 
        RDEBUG("Virtual server %s received request", cf_section_name2(unlang_call_current(request)));
        log_request_pair_list(L_DBG_LVL_1, request, NULL, &request->request_pairs, NULL);
 
-       username = fr_pair_find_by_num(&request->request_pairs, 0, FR_STRIPPED_USER_NAME);
-       if (!username) username = fr_pair_find_by_num(&request->request_pairs, 0, FR_USER_NAME);
+#if 0
+       {
+               fr_pair_t *username, *parent_username = NULL;
+               fr_pair_t *vp;
 
-       if (request->parent) {
-               parent_username = fr_pair_find_by_num(&request->parent->request_pairs, 0, FR_STRIPPED_USER_NAME);
-               if (!parent_username) parent_username = fr_pair_find_by_num(&request->parent->request_pairs, 0, FR_USER_NAME);
-       }
+               username = fr_pair_find_by_num(&request->request_pairs, 0, FR_STRIPPED_USER_NAME);
+               if (!username) username = fr_pair_find_by_num(&request->request_pairs, 0, FR_USER_NAME);
 
-       /*
-        *      Complain about possible issues related to tunnels.
-        */
-       if (username && parent_username) {
-               /*
-                *      Look at the full User-Name with realm.
-                */
-               if (parent_username->da->attr == FR_STRIPPED_USER_NAME) {
-                       vp = fr_pair_find_by_num(&request->parent->request_pairs, 0, FR_USER_NAME);
-                       if (!vp) goto runit;
-               } else {
-                       vp = parent_username;
+               if (request->parent) {
+                       parent_username = fr_pair_find_by_num(&request->parent->request_pairs, 0, FR_STRIPPED_USER_NAME);
+                       if (!parent_username) parent_username = fr_pair_find_by_num(&request->parent->request_pairs, 0, FR_USER_NAME);
                }
 
                /*
-                *      If the names aren't identical, we do some detailed checks.
+                *      Complain about possible issues related to tunnels.
                 */
-               if (strcmp(vp->vp_strvalue, username->vp_strvalue) != 0) {
-                       char const *outer, *inner;
-
-                       outer = strchr(vp->vp_strvalue, '@');
+               if (username && parent_username) {
+                       /*
+                        *      Look at the full User-Name with realm.
+                        */
+                       if (parent_username->da->attr == FR_STRIPPED_USER_NAME) {
+                               vp = fr_pair_find_by_num(&request->parent->request_pairs, 0, FR_USER_NAME);
+                               if (!vp) goto runit;
+                       } else {
+                               vp = parent_username;
+                       }
 
                        /*
-                        *      If there's no realm, or there's a user identifier before
-                        *      the realm name, check the user identifier.
-                        *
-                        *      It SHOULD be "anonymous", or "anonymous@realm"
+                        *      If the names aren't identical, we do some detailed checks.
                         */
-                       if (outer) {
-                               if ((outer != vp->vp_strvalue) &&
-                                   ((vp->vp_length < 10) || (memcmp(vp->vp_strvalue, "anonymous@", 10) != 0))) {
-                                       RWDEBUG("Outer User-Name is not anonymized.  User privacy is compromised.");
-                               } /* else it is anonymized */
+                       if (strcmp(vp->vp_strvalue, username->vp_strvalue) != 0) {
+                               char const *outer, *inner;
+
+                               outer = strchr(vp->vp_strvalue, '@');
 
                                /*
-                                *      Check when there's no realm, and without the trailing '@'
+                                *      If there's no realm, or there's a user identifier before
+                                *      the realm name, check the user identifier.
+                                *
+                                *      It SHOULD be "anonymous", or "anonymous@realm"
                                 */
-                       } else if ((vp->vp_length < 9) || (memcmp(vp->vp_strvalue, "anonymous", 9) != 0)) {
-                                       RWDEBUG("Outer User-Name is not anonymized.  User privacy is compromised.");
+                               if (outer) {
+                                       if ((outer != vp->vp_strvalue) &&
+                                           ((vp->vp_length < 10) || (memcmp(vp->vp_strvalue, "anonymous@", 10) != 0))) {
+                                               RWDEBUG("Outer User-Name is not anonymized.  User privacy is compromised.");
+                                       } /* else it is anonymized */
 
-                       } /* else the user identifier is anonymized */
+                                       /*
+                                        *      Check when there's no realm, and without the trailing '@'
+                                        */
+                               } else if ((vp->vp_length < 9) || (memcmp(vp->vp_strvalue, "anonymous", 9) != 0)) {
+                                               RWDEBUG("Outer User-Name is not anonymized.  User privacy is compromised.");
 
-                       /*
-                        *      Look for an inner realm, which may or may not exist.
-                        */
-                       inner = strchr(username->vp_strvalue, '@');
-                       if (outer && inner) {
-                               outer++;
-                               inner++;
+                               } /* else the user identifier is anonymized */
 
                                /*
-                                *      The realms are different, do
-                                *      more detailed checks.
+                                *      Look for an inner realm, which may or may not exist.
                                 */
-                               if (strcmp(outer, inner) != 0) {
-                                       size_t outer_len, inner_len;
-
-                                       outer_len = vp->vp_length;
-                                       outer_len -= (outer - vp->vp_strvalue);
-
-                                       inner_len = username->vp_length;
-                                       inner_len -= (inner - username->vp_strvalue);
+                               inner = strchr(username->vp_strvalue, '@');
+                               if (outer && inner) {
+                                       outer++;
+                                       inner++;
 
                                        /*
-                                        *      Inner: secure.example.org
-                                        *      Outer: example.org
+                                        *      The realms are different, do
+                                        *      more detailed checks.
                                         */
-                                       if (inner_len > outer_len) {
-                                               char const *suffix;
+                                       if (strcmp(outer, inner) != 0) {
+                                               size_t outer_len, inner_len;
 
-                                               suffix = inner + (inner_len - outer_len) - 1;
+                                               outer_len = vp->vp_length;
+                                               outer_len -= (outer - vp->vp_strvalue);
 
-                                               if ((*suffix != '.') ||
-                                                   (strcmp(suffix + 1, outer) != 0)) {
-                                                       RWDEBUG("Possible spoofing: Inner realm '%s' is not a "
-                                                               "subdomain of the outer realm '%s'", inner, outer);
-                                               }
+                                               inner_len = username->vp_length;
+                                               inner_len -= (inner - username->vp_strvalue);
+
+                                               /*
+                                                *      Inner: secure.example.org
+                                                *      Outer: example.org
+                                                */
+                                               if (inner_len > outer_len) {
+                                                       char const *suffix;
+
+                                                       suffix = inner + (inner_len - outer_len) - 1;
 
-                                       } else {
-                                               RWDEBUG("Possible spoofing: Inner realm and "
-                                                       "outer realms are different");
+                                                       if ((*suffix != '.') ||
+                                                           (strcmp(suffix + 1, outer) != 0)) {
+                                                               RWDEBUG("Possible spoofing: Inner realm '%s' is not a "
+                                                                       "subdomain of the outer realm '%s'", inner, outer);
+                                                       }
+
+                                               } else {
+                                                       RWDEBUG("Possible spoofing: Inner realm and "
+                                                               "outer realms are different");
+                                               }
                                        }
                                }
-                       }
 
-               } else {
-                       RWDEBUG("Outer and inner identities are the same.  User privacy is compromised.");
+                       } else {
+                               RWDEBUG("Outer and inner identities are the same.  User privacy is compromised.");
+                       }
                }
        }
+#endif
 
-runit:
        if (!request->async) {
 #ifdef __clang_analyzer__
                if (!request->parent) RETURN_MODULE_FAIL;
index c6b27caf4691620efca68a675a94e042e8125088..bf152ba05b03d54bad3d3dc141205a7ce9fa4f5b 100644 (file)
@@ -548,7 +548,7 @@ int paircmp(request_t *request,
                                WARN("Are you sure you don't mean Password.Cleartext?");
                                WARN("See \"man rlm_pap\" for more information");
                        }
-                       if (fr_pair_find_by_num(request_list, 0, FR_USER_PASSWORD) == NULL) continue;
+                       if (fr_pair_find_by_da(request_list, attr_user_password) == NULL) continue;
                }
 
                /*
index 1a219b76a5139013d410fa66e37e79c91ec21117..d060e2627a0b0e53686c8d3077e53f9fcbe5df50 100644 (file)
@@ -546,43 +546,6 @@ fr_pair_t *fr_pair_find_by_ancestor(fr_pair_list_t const *list, fr_dict_attr_t c
        return NULL;
 }
 
-/** Find the pair with the matching attribute by vendor id and attribute number.
- *
- * @todo should take DAs and do a pointer comparison.
- *
- * @param[in] list     to search in.
- * @param[in] vendor   id to match in the list (0 for non vendor-specific).
- * @param[in] attr     number to match in the list.
- * @return
- *     - first matching fr_pair_t
- *     - NULL if no fr_pair_ts match
- */
-fr_pair_t *fr_pair_find_by_num(fr_pair_list_t *list, unsigned int vendor, unsigned int attr)
-{
-       fr_pair_t       *vp;
-
-       if (fr_dlist_empty(&list->head)) return NULL;
-
-       LIST_VERIFY(list);
-
-       for (vp = fr_pair_list_head(list); vp != NULL; vp = fr_pair_list_next(list, vp)) {
-               if (!fr_dict_attr_is_top_level(vp->da)) continue;
-
-               if (vendor > 0) {
-                       fr_dict_vendor_t const *dv;
-
-                       dv = fr_dict_vendor_by_da(vp->da);
-                       if (!dv) continue;
-
-                       if (dv->pen != vendor) continue;
-               }
-
-               if (attr == vp->da->attr) return vp;
-       }
-
-       return NULL;
-}
-
 /** Find the pair with the matching child attribute
  *
  * @param[in] list     in which to search.
index 23c3517537b51110bb70aa178676c150cacab585..c6cfdce2dbb40508552266a5c73dd42b79707703 100644 (file)
@@ -233,8 +233,6 @@ fr_pair_t   *fr_pair_find_by_da(fr_pair_list_t const *list, fr_dict_attr_t const *
 
 fr_pair_t      *fr_pair_find_by_ancestor(fr_pair_list_t const *list, fr_dict_attr_t const *ancestor);
 
-fr_pair_t      *fr_pair_find_by_num(fr_pair_list_t *list, unsigned int vendor, unsigned int attr);
-
 fr_pair_t      *fr_pair_find_by_child_num(fr_pair_list_t *list, fr_dict_attr_t const *parent, unsigned int attr);
 
 int            fr_pair_append(fr_pair_list_t *list, fr_pair_t *vp);
index 98e3458e4389088e9abb2916accd5835487a7f93..0b570db2def8873e9f9faf0bded1d79196f0cbce 100644 (file)
@@ -361,20 +361,6 @@ static void test_fr_pair_find_by_da(void)
        TEST_CHECK(vp && vp->da == attr_test_tlv_string);
 }
 
-static void test_fr_pair_find_by_num(void)
-{
-       fr_pair_t *vp;
-
-       TEST_CASE("Search for FR_TEST_STRING using fr_pair_find_by_num()");
-       TEST_CHECK((vp = fr_pair_find_by_num(&sample_pairs, 0, FR_TEST_STRING)) != NULL);
-
-       TEST_CASE("Validating VP_VERIFY()");
-       VP_VERIFY(vp);
-
-       TEST_CASE("Expected (vp->da == attr_test_string)");
-       TEST_CHECK(vp && vp->da == attr_test_string);
-}
-
 static void test_fr_pair_find_by_child_num(void)
 {
        fr_pair_t *vp;
@@ -421,7 +407,7 @@ static void test_fr_pair_delete_by_child_num(void)
        fr_pair_delete_by_child_num(&sample_pairs, fr_dict_root(dict_test), FR_TEST_STRING);
 
        TEST_CASE("The attr_test_string shouldn't exist in 'sample_pairs'");
-       TEST_CHECK(fr_pair_find_by_num(&sample_pairs, 0, FR_TEST_STRING) == NULL);
+       TEST_CHECK(fr_pair_find_by_child_num(&sample_pairs, fr_dict_root(dict_test), FR_TEST_STRING) == NULL);
 
        TEST_CASE("Add attr_test_string back into 'sample_pairs'");
        TEST_CHECK(fr_pair_prepend_by_da(autofree, NULL, &sample_pairs, attr_test_string) == 0);
@@ -485,11 +471,11 @@ static void test_fr_pair_delete(void)
        fr_pair_t *vp;
 
        TEST_CASE("Delete attr_test_string using fr_pair_delete()");
-       TEST_CHECK((vp = fr_pair_find_by_num(&sample_pairs, 0, FR_TEST_STRING)) != NULL);
+       TEST_CHECK((vp = fr_pair_find_by_child_num(&sample_pairs, fr_dict_root(dict_test), FR_TEST_STRING)) != NULL);
        fr_pair_delete(&sample_pairs, vp);
 
        TEST_CASE("The attr_test_string shouldn't exist in 'sample_pairs'");
-       TEST_CHECK((vp = fr_pair_find_by_num(&sample_pairs, 0, FR_TEST_STRING)) == NULL);
+       TEST_CHECK((vp = fr_pair_find_by_child_num(&sample_pairs, fr_dict_root(dict_test), FR_TEST_STRING)) == NULL);
 
        TEST_CASE("Add attr_test_string back into 'sample_pairs'");
        TEST_CHECK(fr_pair_prepend_by_da(autofree, NULL, &sample_pairs, attr_test_string) == 0);
@@ -1256,10 +1242,9 @@ TEST_LIST = {
        { "fr_dcursor_iter_by_ancestor_init",     test_fr_dcursor_iter_by_ancestor_init },
        { "fr_pair_to_unknown",                   test_fr_pair_to_unknown },
        { "fr_pair_find_by_da",                   test_fr_pair_find_by_da },
-       { "fr_pair_find_by_num",                  test_fr_pair_find_by_num },
        { "fr_pair_find_by_child_num",            test_fr_pair_find_by_child_num },
        { "fr_pair_append",                       test_fr_pair_append },
-       { "fr_pair_prepend_by_da",                 test_fr_pair_prepend_by_da },
+       { "fr_pair_prepend_by_da",                test_fr_pair_prepend_by_da },
        { "fr_pair_delete_by_child_num",          test_fr_pair_delete_by_child_num },
        { "fr_pair_update_by_da",                 test_fr_pair_update_by_da },
        { "fr_pair_delete",                       test_fr_pair_delete },