From 5ad1cd7383fc13285d36dbabd1bc82e04e2f3004 Mon Sep 17 00:00:00 2001 From: "Alan T. DeKok" Date: Sat, 10 Apr 2021 09:24:34 -0400 Subject: [PATCH] Allow relative attributes in SQL. And add test cases for it. --- src/modules/rlm_sql/sql.c | 98 ++++++++++++++++++---- src/tests/modules/sql_sqlite/nested.attrs | 14 ++++ src/tests/modules/sql_sqlite/nested.unlang | 45 ++++++++++ 3 files changed, 142 insertions(+), 15 deletions(-) create mode 100644 src/tests/modules/sql_sqlite/nested.attrs create mode 100644 src/tests/modules/sql_sqlite/nested.unlang diff --git a/src/modules/rlm_sql/sql.c b/src/modules/rlm_sql/sql.c index 14b45445e1..4d34bbc58a 100644 --- a/src/modules/rlm_sql/sql.c +++ b/src/modules/rlm_sql/sql.c @@ -108,12 +108,12 @@ void *sql_mod_conn_create(TALLOC_CTX *ctx, void *instance, fr_time_delta_t timeo /************************************************************************* * - * Function: sql_pair_list_afrom_str + * Function: sql_pair_afrom_row * - * Purpose: Read entries from the database and fill fr_pair_t structures + * Purpose: Convert one rlm_sql_row_t to a fr_pair_t, and add it to "out" * *************************************************************************/ -static int sql_pair_list_afrom_str(TALLOC_CTX *ctx, request_t *request, fr_pair_list_t *out, rlm_sql_row_t row) +static int sql_pair_afrom_row(TALLOC_CTX *ctx, request_t *request, fr_pair_list_t *out, rlm_sql_row_t row, fr_pair_t **relative_vp) { fr_pair_t *vp; char const *ptr, *value; @@ -121,6 +121,8 @@ static int sql_pair_list_afrom_str(TALLOC_CTX *ctx, request_t *request, fr_pair_ char do_xlat = 0; fr_dict_attr_t const *da; fr_token_t token, op = T_EOL; + fr_pair_list_t *my_list; + TALLOC_CTX *my_ctx; /* * Verify the 'Attribute' field @@ -158,11 +160,13 @@ static int sql_pair_list_afrom_str(TALLOC_CTX *ctx, request_t *request, fr_pair_ return -1; } + RDEBUG3("Found row: %s %s %s", row[0], fr_table_str_by_value(fr_tokens_table, op, "???"), row[3]); + value = row[3]; /* - * If we have a new-style quoted string, where the - * *entire* string is quoted, do xlat's. + * If we have a string, where the *entire* string is + * quoted, do xlat's. */ if (row[3] != NULL && ((row[3][0] == '\'') || (row[3][0] == '`') || (row[3][0] == '"')) && @@ -195,19 +199,49 @@ static int sql_pair_list_afrom_str(TALLOC_CTX *ctx, request_t *request, fr_pair_ } /* - * Search in our local dictionary - * falling back to internal. + * Check for relative attributes + * + * @todo - allow "..foo" to mean "grandparent of + * relative_vp", and it should also update relative_vp + * with the new parent. However, doing this means + * walking the list of the current relative_vp, finding + * the dlist head, and then converting that into a + * fr_pair_t pointer. That's complex, so we don't do it + * right now. */ - da = fr_dict_attr_by_oid(NULL, fr_dict_root(request->dict), row[2]); - if (!da) { - da = fr_dict_attr_by_oid(NULL, fr_dict_root(fr_dict_internal()), row[2]); - if (!da) { - RPEDEBUG("Failed creating pair from SQL data"); + if (row[2][0] == '.') { + char const *p = row[2]; + + if (!*relative_vp) { + REDEBUG("Relative attribute '%s' can only be used immediately after an attribute of type 'group'", row[2]); return -1; } + + da = fr_dict_attr_by_oid(NULL, (*relative_vp)->da, p + 1); + if (!da) goto unknown; + + my_list = &(*relative_vp)->vp_group; + my_ctx = *relative_vp; + } else { + /* + * Search in our local dictionary + * falling back to internal. + */ + da = fr_dict_attr_by_oid(NULL, fr_dict_root(request->dict), row[2]); + if (!da) { + da = fr_dict_attr_by_oid(NULL, fr_dict_root(fr_dict_internal()), row[2]); + if (!da) { + unknown: + RPEDEBUG("Unknown attribute '%s'", row[2]); + return -1; + } + } + + my_list = out; + my_ctx = ctx; } - MEM(vp = fr_pair_afrom_da(ctx, da)); + MEM(vp = fr_pair_afrom_da(my_ctx, da)); vp->op = op; if (do_xlat) { @@ -217,6 +251,16 @@ static int sql_pair_list_afrom_str(TALLOC_CTX *ctx, request_t *request, fr_pair_ talloc_free(vp); return -1; } + + } else if ((vp->da->type == FR_TYPE_TLV) && !*value) { + /* + * Allow empty values for TLVs: we just create the value. + * + * fr_pair_value_from_str() is not yet updated to + * handle TLVs. Until such time as we know what + * to do there, we will just do a hack here, + * specific to the SQL module. + */ } else { if (fr_pair_value_from_str(vp, value, -1, '\0', true) < 0) { RPEDEBUG("Error parsing value"); @@ -229,7 +273,30 @@ static int sql_pair_list_afrom_str(TALLOC_CTX *ctx, request_t *request, fr_pair_ /* * Add the pair into the packet */ - fr_pair_append(out, vp); + fr_pair_append(my_list, vp); + + /* + * Update the relative vp. + */ + if (my_list == out) switch (da->type) { + case FR_TYPE_STRUCTURAL: + *relative_vp = vp; + break; + + default: + break; + } + + /* + * If there's a relative VP, and it's not the one + * we just added above, and we're not adding this + * VP to the relative one, then nuke the relative + * VP. + */ + if (*relative_vp && (vp != *relative_vp) && (my_ctx != *relative_vp)) { + *relative_vp = NULL; + } + return 0; } @@ -514,6 +581,7 @@ int sql_getvpdata(TALLOC_CTX *ctx, rlm_sql_t const *inst, request_t *request, rl rlm_sql_row_t row; int rows = 0; sql_rcode_t rcode; + fr_pair_t *relative_vp = NULL; fr_assert(request); @@ -521,7 +589,7 @@ int sql_getvpdata(TALLOC_CTX *ctx, rlm_sql_t const *inst, request_t *request, rl if (rcode != RLM_SQL_OK) return -1; /* error handled by rlm_sql_select_query */ while (rlm_sql_fetch_row(&row, inst, request, handle) == RLM_SQL_OK) { - if (sql_pair_list_afrom_str(ctx, request, out, row) != 0) { + if (sql_pair_afrom_row(ctx, request, out, row, &relative_vp) != 0) { REDEBUG("Error parsing user data from database result"); (inst->driver->sql_finish_select_query)(*handle, inst->config); diff --git a/src/tests/modules/sql_sqlite/nested.attrs b/src/tests/modules/sql_sqlite/nested.attrs new file mode 100644 index 0000000000..40131c0937 --- /dev/null +++ b/src/tests/modules/sql_sqlite/nested.attrs @@ -0,0 +1,14 @@ +# +# Input packet +# +Packet-Type = Access-Request +User-Name = "user_auth_nested" +User-Password = "password" +NAS-IP-Address = "1.2.3.4" + +# +# Expected answer +# +Packet-Type == Access-Accept +Idle-Timeout == 3600 +Digest-Attributes == { Nonce == "dcd98b7102dd2f0e8b11d0f600bfb0c093", Method == "Invite", URI == "sip:bob@biloxi.com" } diff --git a/src/tests/modules/sql_sqlite/nested.unlang b/src/tests/modules/sql_sqlite/nested.unlang new file mode 100644 index 0000000000..f5c95219b5 --- /dev/null +++ b/src/tests/modules/sql_sqlite/nested.unlang @@ -0,0 +1,45 @@ +# +# Clear out old data. We don't care if the deletion deletes any rows. +# +"%{sql:DELETE FROM radcheck WHERE username = 'user_auth_nested'}" +"%{sql:DELETE FROM radreply WHERE username = 'user_auth_nested'}" + +# +# Add in the check items. +# +if ("%{sql:INSERT INTO radcheck (username, attribute, op, value) VALUES ('user_auth_nested', 'NAS-IP-Address', '==', '1.2.3.4')}" != "1") { + test_fail +} + +if ("%{sql:INSERT INTO radcheck (username, attribute, op, value) VALUES ('user_auth_nested', 'Password.Cleartext', ':=', 'password')}" != "1") { + test_fail +} + +# +# Add in the reply items. +# +if ("%{sql:INSERT INTO radreply (username, attribute, op, value) VALUES ('user_auth_nested', 'Idle-Timeout', ':=', '3600')}" != "1") { + test_fail +} + +if ("%{sql:INSERT INTO radreply (username, attribute, op, value) VALUES ('user_auth_nested', 'Digest-Attributes', ':=', '')}" != "1") { + test_fail +} + + +if ("%{sql:INSERT INTO radreply (username, attribute, op, value) VALUES ('user_auth_nested', '.Nonce', ':=', 'dcd98b7102dd2f0e8b11d0f600bfb0c093')}" != "1") { + test_fail +} + +if ("%{sql:INSERT INTO radreply (username, attribute, op, value) VALUES ('user_auth_nested', '.Method', ':=', 'Invite')}" != "1") { + test_fail +} + +if ("%{sql:INSERT INTO radreply (username, attribute, op, value) VALUES ('user_auth_nested', '.URI', ':=', 'sip:bob@biloxi.com')}" != "1") { + test_fail +} + +# +# Run the SQL module to get the replies. +# +sql -- 2.47.2