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 3d306f7176f79c92d59682448bfab7cabe5cdb14..b37aad9e474debf8d213076a5f6d720806a0b024 100644 (file)
--- 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 e08f8cd7cb502325115473ee3fa9721bca2c18eb..c77fd0b4cd1d535e95730ed936b55147abc5d83c 100644 (file)
--- 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
         *      .<oid>
         */
        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 7e1a38a80432b9b1cc976d4efc7df4b536546389..7dc0c381b4c0afe739b078b48d541dc5ad282209 100644 (file)
--- 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 cfd97ea757bcf5bb9c2b9b72ba743738f9b7ae8c..0708281e25ec835ea0e0616e6bae82ad57c5b020 100644 (file)
--- 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 4706ab7f0a67b88f8b31619a92dc7af45301e043..8c93b4788ac7cb281332ad07f4fbce38cd09d891 100644 (file)
--- 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 3665ee36a1914a6a9417cfb3378803000855d2db..4880870dabc844b2ab8102d129fe2b2d610e35a0 100644 (file)
--- 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 62a86058f7e0b493e01a31ddb652478b3d53358b..2f8267654929beb23361ca2e351268ca93b02c79 100644 (file)
--- 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 afdbba4ab06dde1812c32c94bc0c31eb76d28b87..35c02251d408493a3c8e783010672dc07fe5a5ba 100644 (file)
--- 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