From: Alan T. DeKok Date: Mon, 8 Dec 2025 16:49:17 +0000 (-0500) Subject: allow bare oids only if a new flag is set X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4c4f78007ae10e92955beb266fff7c7bc6fc3b02;p=thirdparty%2Ffreeradius-server.git allow bare oids only if a new flag is set the code also sets that flag when the "raw" prefix is used. The tests are updated for this new syntax. Adding a flag is useful not just for "raw", but also for protocols wuch as DER or SNMP which might want to always allow numerical OID strings. --- diff --git a/src/lib/server/tmpl.h b/src/lib/server/tmpl.h index 3d306f7176f..b37aad9e474 100644 --- a/src/lib/server/tmpl.h +++ b/src/lib/server/tmpl.h @@ -313,6 +313,8 @@ struct tmpl_attr_rules_s { uint8_t allow_foreign:1; //!< Allow arguments not found in dict_def. + uint8_t allow_oid:1; //!< allow numerical OIDs. + uint8_t disallow_filters:1; //!< disallow filters. uint8_t xlat:1 ; //!< for %{User-Name} @@ -1026,7 +1028,8 @@ typedef enum { TMPL_ATTR_ERROR_INVALID_FILTER, //!< Invalid filter TMPL_ATTR_ERROR_NESTING_TOO_DEEP, //!< Too many levels of nesting. TMPL_ATTR_ERROR_MISSING_TERMINATOR, //!< Unexpected text found after attribute reference - TMPL_ATTR_ERROR_BAD_CAST //!< Specified cast was invalid. + TMPL_ATTR_ERROR_BAD_CAST, //!< Specified cast was invalid. + TMPL_ATTR_ERROR_INVALID_OID //!< OIDs are not allowed } tmpl_attr_error_t; /** Map ptr type to a boxed type diff --git a/src/lib/server/tmpl_tokenize.c b/src/lib/server/tmpl_tokenize.c index e08f8cd7cb5..c77fd0b4cd1 100644 --- a/src/lib/server/tmpl_tokenize.c +++ b/src/lib/server/tmpl_tokenize.c @@ -1747,12 +1747,12 @@ static void tmpl_attr_ref_fixup(TALLOC_CTX *ctx, tmpl_t *vpt, fr_dict_attr_t con * - <0 on error. * - 0 on success. */ -static inline int tmpl_attr_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err, - tmpl_t *vpt, - fr_dict_attr_t const *parent, fr_dict_attr_t const *namespace, - fr_sbuff_t *name, - fr_sbuff_parse_rules_t const *p_rules, tmpl_attr_rules_t const *at_rules, - unsigned int depth) +static int tmpl_attr_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err, + tmpl_t *vpt, + fr_dict_attr_t const *parent, fr_dict_attr_t const *namespace, + fr_sbuff_t *name, + fr_sbuff_parse_rules_t const *p_rules, tmpl_attr_rules_t const *at_rules, + unsigned int depth) { uint32_t oid = 0; tmpl_attr_t *ar = NULL; @@ -1928,6 +1928,27 @@ static inline int tmpl_attr_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t * . */ if (fr_sbuff_out(NULL, &oid, name) > 0) { + if (!at_rules->allow_oid) { + uint8_t c = fr_sbuff_char(name, '\0'); + + /* + * This extra test is to give the user better errors. The string "3G" is parsed + * as "3", and then an error of "what the heck do you mean by G?" + * + * In contrast, the string "3." is parsed as "3", and then "nope, that's not an attribute reference". + */ + if (c != '.') { + fr_strerror_const("Unexpected text after attribute reference"); + if (err) *err = TMPL_ATTR_ERROR_MISSING_TERMINATOR; + } else { + fr_strerror_const("Numerical attribute references are not allowed here"); + if (err) *err = TMPL_ATTR_ERROR_INVALID_OID; + + fr_sbuff_set(name, &m_s); + } + goto error; + } + our_parent = namespace = fr_dict_unlocal(namespace); fr_assert(ar == NULL); @@ -2110,10 +2131,7 @@ do_suffix: break; default: - fr_strerror_printf("Parent type of nested attribute %s must be of type " - "\"struct\", \"tlv\", \"vendor\", \"vsa\" or \"group\", got \"%s\"", - da->name, - fr_type_to_str(da->type)); + fr_strerror_printf("Attribute %s of data type '%s' cannot have child attributes", da->name, fr_type_to_str(da->type)); fr_sbuff_set(name, &m_s); goto error; } @@ -2203,6 +2221,7 @@ ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err, fr_sbuff_t our_name = FR_SBUFF(name); /* Take a local copy in case we need to back track */ bool is_raw = false; tmpl_attr_rules_t const *at_rules; + tmpl_attr_rules_t my_attr_rules; fr_sbuff_marker_t m_l; fr_dict_attr_t const *namespace; DEFAULT_RULES; @@ -2237,7 +2256,13 @@ ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err, * users to stick whatever they want in there as * a value. */ - if (fr_sbuff_adv_past_strcase_literal(&our_name, "raw.")) is_raw = true; + if (fr_sbuff_adv_past_strcase_literal(&our_name, "raw.")) { + my_attr_rules = *at_rules; + my_attr_rules.allow_oid = true; + at_rules = &my_attr_rules; + + is_raw = true; + } /* * Parse one or more request references diff --git a/src/tests/eapol_test/config/servers.conf b/src/tests/eapol_test/config/servers.conf index 7e1a38a8043..7dc0c381b4c 100644 --- a/src/tests/eapol_test/config/servers.conf +++ b/src/tests/eapol_test/config/servers.conf @@ -142,9 +142,9 @@ server test { NAS-Port = 12345 Reply-Message = "Powered by FreeRADIUS" - &26.1234.56 = 0xdeadbeef - Vendor-Specific.20.30 = 0xcafecafe - Vendor-Specific.20.30 = 0xcadecade + raw.26.1234.56 = 0xdeadbeef + raw.Vendor-Specific.20.30 = 0xcafecafe + raw.Vendor-Specific.20.30 = 0xcadecade Vendor-Specific.Alcatel.FR-Direct-Profile = "Alcatel Profile" Vendor-Specific.Alcatel.Home-Agent-UDP-Port = 4130 diff --git a/src/tests/keywords/unknown b/src/tests/keywords/unknown index cfd97ea757b..0708281e25e 100644 --- a/src/tests/keywords/unknown +++ b/src/tests/keywords/unknown @@ -6,18 +6,18 @@ Vendor-Specific.FreeRADIUS.Proxied-To := 127.0.0.2 # # Check that a known but malformed attr is malformed # -26.24757.84.9.5.7 := 0xab +raw.26.24757.84.9.5.7 := 0xab -if (!(26.24757.84.9.5.7 == 0xab)) { +if (!(raw.26.24757.84.9.5.7 == 0xab)) { test_fail } # # Check that an unknown attr is OK # -26.24757.84.9.5.15 := 0xabcdef +raw.26.24757.84.9.5.15 := 0xabcdef -if (!(26.24757.84.9.5.15 == 0xabcdef)) { +if (!(raw.26.24757.84.9.5.15 == 0xabcdef)) { test_fail } @@ -25,15 +25,15 @@ if (!(26.24757.84.9.5.15 == 0xabcdef)) { # Check that unknown attributes which are defined # get automatically resolved to the real attribute. # -if (26.11344.1 == 127.0.0.1) { +if (raw.26.11344.1 == 127.0.0.1) { test_fail } -if (!(26.11344.1 == 127.0.0.2)) { +if (!(raw.26.11344.1 == 127.0.0.2)) { test_fail } -26.11344.1 := 0x7f000001 +raw.26.11344.1 := 0x7f000001 if (Vendor-Specific.FreeRADIUS.Proxied-To == 127.0.0.2) { test_fail @@ -43,17 +43,17 @@ if (!(Vendor-Specific.FreeRADIUS.Proxied-To == 127.0.0.1)) { test_fail } -if (26.11344.1 == 127.0.0.2) { +if (raw.26.11344.1 == 127.0.0.2) { test_fail } -if (!(26.11344.1 == 127.0.0.1)) { +if (!(raw.26.11344.1 == 127.0.0.1)) { test_fail } -26.66.1 = 0x01020304 +raw.26.66.1 = 0x01020304 -if (!(26.66.1 == 0x01020304)) { +if (!(raw.26.66.1 == 0x01020304)) { test_fail } diff --git a/src/tests/keywords/wimax b/src/tests/keywords/wimax index 4706ab7f0a6..8c93b4788ac 100644 --- a/src/tests/keywords/wimax +++ b/src/tests/keywords/wimax @@ -4,8 +4,15 @@ if (!(Vendor-Specific.WiMAX.Packet-Flow-Descriptor-v2.Classifier.Src-Spec.Port = test_fail } -26.24757.84.9.5.7 := 0x01 -if (!(Vendor-Specific.WiMAX.Packet-Flow-Descriptor-v2.Classifier.Src-Spec.Assigned == 1)) { +# +# Raw attributes get resolved to real ones if they exist. +# +raw.26.24757.84.9.5.7 := 0x01 +if (!Vendor-Specific.WiMAX.Packet-Flow-Descriptor-v2.Classifier.Src-Spec.Assigned) { + test_fail +} + +if (Vendor-Specific.WiMAX.Packet-Flow-Descriptor-v2.Classifier.Src-Spec.Assigned != 1) { test_fail } diff --git a/src/tests/unit/condition/base.txt b/src/tests/unit/condition/base.txt index 3665ee36a19..4880870dabc 100644 --- a/src/tests/unit/condition/base.txt +++ b/src/tests/unit/condition/base.txt @@ -258,6 +258,9 @@ match true condition (ether) 00:11:22:33:44:55 == "%hash.md4('00:11:22:33:44:55')" match (00:11:22:33:44:55 == "%hash.md4(0x30303a31313a32323a33333a34343a3535)") +# +# @todo - we have a hint on the cast. Perhaps we want to return a different error? +# condition (ether) 00:XX:22:33:44:55 == 00:11:22:33:44:55 match ERROR offset 10: Unexpected text after attribute reference @@ -339,8 +342,11 @@ match true condition (ipaddr)127.0.0.1/32 == 127.0.0.1 match true +# +# @todo - we have a hint on the cast. Perhaps we want to return a different error? +# condition (ipaddr)127.0.0.1/327 == 127.0.0.1 -match ERROR offset 13: Parent type of nested attribute 0 must be of type "struct", "tlv", "vendor", "vsa" or "group", got "octets" +match ERROR offset 8: Numerical attribute references are not allowed here condition (ipaddr)127.0.0.1/32 == 127.0.0.1 match true @@ -495,8 +501,8 @@ match (Acct-Input-Octets64 > "%{Session-Timeout}") # @todo - peephole - Perhaps this isn't done because the xlat name is taken from the # input, and not from the canonicalized names. # -condition 26.24757.84.9.5.4 == 0x1a99 -match (26.24757.84.9.5.4 == 0x1a99) +condition raw.26.24757.84.9.5.4 == 0x1a99 +match (raw.26.24757.84.9.5.4 == 0x1a99) #match Vendor-Specific.WiMAX.Packet-Flow-Descriptor-v2.Classifier.Src-Spec.Port == 6809 # @@ -509,8 +515,8 @@ match (raw.26.24757.84.9.5.7 == 0x1a99) #match raw.Vendor-Specific.WiMAX.Packet-Flow-Descriptor-v2.Classifier.Src-Spec.Assigned == 0x1a99 # This one is really unknown -condition 26.24757.84.9.5.15 == 0x1a99 -match (26.24757.84.9.5.15 == 0x1a99) +condition raw.26.24757.84.9.5.15 == 0x1a99 +match (raw.26.24757.84.9.5.15 == 0x1a99) #match Vendor-Specific.WiMAX.Packet-Flow-Descriptor-v2.Classifier.Src-Spec.15 == 0x1a99 # diff --git a/src/tests/unit/condition/nested.txt b/src/tests/unit/condition/nested.txt index 62a86058f7e..2f826765492 100644 --- a/src/tests/unit/condition/nested.txt +++ b/src/tests/unit/condition/nested.txt @@ -44,7 +44,7 @@ match ERROR offset 45: Zero length attribute name: Unresolved attributes are not # Trailing dots, point to the dot that's an error condition Vendor-Specific.WiMAX.Packet-Flow-Descriptor.Uplink-QOS-Id[0]. -match ERROR offset 61: Parent type of nested attribute Uplink-QOS-Id must be of type "struct", "tlv", "vendor", "vsa" or "group", got "uint8" +match ERROR offset 61: Attribute Uplink-QOS-Id of data type 'uint8' cannot have child attributes count match 19 diff --git a/src/tests/unit/xlat/cond_base.txt b/src/tests/unit/xlat/cond_base.txt index afdbba4ab06..35c02251d40 100644 --- a/src/tests/unit/xlat/cond_base.txt +++ b/src/tests/unit/xlat/cond_base.txt @@ -275,7 +275,7 @@ xlat_purify_cond (ether)00:11:22:33:44:55 == "%hash.md4('00:11:22:33:44:55')" match ERROR purifying node - Missing separator, expected ':' xlat_purify_cond (ether) 00:XX:22:33:44:55 == 00:11:22:33:44:55 -match ERROR offset 8: Unknown attributes not allowed here +match ERROR offset 10: Unexpected text after attribute reference # # Tests for boolean data types. @@ -353,7 +353,7 @@ xlat_purify_cond (ipaddr)127.0.0.1/32 == 127.0.0.1 match true xlat_purify_cond (ipaddr)127.0.0.1/327 == 127.0.0.1 -match ERROR offset 12: Unknown attributes not allowed here +match ERROR offset 8: Numerical attribute references are not allowed here xlat_purify_cond (ipaddr)127.0.0.1/32 == 127.0.0.1 match true @@ -509,8 +509,8 @@ match (Acct-Input-Octets64 > Session-Timeout) # Parse OIDs into known attributes, where possible. # # @todo - peephole - resolve the unknown attribute to something real! -xlat_purify_cond 26.24757.84.9.5.4 == 0x1a99 -match (26.24757.84.9.5.4 == 0x1a99) +xlat_purify_cond raw.26.24757.84.9.5.4 == 0x1a99 +match (raw.26.24757.84.9.5.4 == 0x1a99) #match Vendor-Specific.WiMAX.Packet-Flow-Descriptor-v2.Classifier.Src-Spec.Port == 6809 # @@ -524,8 +524,11 @@ match (raw.26.24757.84.9.5.7 == 0x1a99) # This one is really unknown xlat_purify_cond 26.24757.84.9.5.15 == 0x1a99 -match ERROR offset 16: Unknown attributes not allowed here -#match Vendor-Specific.WiMAX.Packet-Flow-Descriptor-v2.Classifier.Src-Spec.15 == 0x1a99 +match ERROR offset 0: Numerical attribute references are not allowed here + +xlat_purify_cond raw.26.24757.84.9.5.15 == 0x1a99 +match ERROR offset 20: Unknown attributes not allowed here +#match raw.Vendor-Specific.WiMAX.Packet-Flow-Descriptor-v2.Classifier.Src-Spec.15 == 0x1a99 # # Invalid array references. @@ -767,4 +770,4 @@ xlat_purify_cond (192.168.0.1 !== 192.168.0.2) match true count -match 328 +match 330