Refactor TR_FILTER using a GPtrArray of filter lines
authorJennifer Richards <jennifer@painless-security.com>
Sat, 21 Apr 2018 05:04:36 +0000 (01:04 -0400)
committerJennifer Richards <jennifer@painless-security.com>
Sat, 21 Apr 2018 05:04:36 +0000 (01:04 -0400)
common/tr_config_filters.c
common/tr_config_rp_clients.c
common/tr_filter.c
common/tr_filter_encoders.c
include/tr_filter.h

index 4035e68..5be737f 100644 (file)
@@ -98,6 +98,7 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR
 {
   TALLOC_CTX *tmp_ctx = talloc_new(NULL);
   TR_FILTER *filt = NULL;
+  TR_FLINE *fline = NULL;
   json_t *jfaction = NULL;
   json_t *jfline = NULL;
   json_t *jfspecs = NULL;
@@ -125,13 +126,6 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR
   }
   tr_filter_set_type(filt, ftype);
 
-  /* make sure we have space to represent the filter */
-  if (json_array_size(jfilt) > TR_MAX_FILTER_LINES) {
-    tr_err("tr_cfg_parse_one_filter: Filter has too many lines, maximum of %d.", TR_MAX_FILTER_LINES);
-    *rc = TR_CFG_NOPARSE;
-    goto cleanup;
-  }
-
   /* For each entry in the filter... */
   json_array_foreach(jfilt, i, jfline) {
     if ((NULL == (jfaction = json_object_get(jfline, "action"))) ||
@@ -155,16 +149,17 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR
       goto cleanup;
     }
 
-    if (NULL == (filt->lines[i] = tr_fline_new(filt))) {
+    fline = tr_fline_new(tmp_ctx);
+    if (fline == NULL) {
       tr_debug("tr_cfg_parse_one_filter: Out of memory allocating filter line %d.", i + 1);
       *rc = TR_CFG_NOMEM;
       goto cleanup;
     }
 
     if (!strcmp(json_string_value(jfaction), "accept")) {
-      filt->lines[i]->action = TR_FILTER_ACTION_ACCEPT;
+      fline->action = TR_FILTER_ACTION_ACCEPT;
     } else if (!strcmp(json_string_value(jfaction), "reject")) {
-      filt->lines[i]->action = TR_FILTER_ACTION_REJECT;
+      fline->action = TR_FILTER_ACTION_REJECT;
     } else {
       tr_debug("tr_cfg_parse_one_filter: Error parsing filter action, unknown action' %s'.",
                json_string_value(jfaction));
@@ -184,7 +179,7 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR
         goto cleanup;
       } else if (json_array_size(jrc) > 0) {
         /* ok we actually have entries to process */
-        if (NULL == (filt->lines[i]->realm_cons = tr_cfg_parse_one_constraint(filt->lines[i], "realm", jrc, rc))) {
+        if (NULL == (fline->realm_cons = tr_cfg_parse_one_constraint(fline, "realm", jrc, rc))) {
           tr_debug("tr_cfg_parse_one_filter: Error parsing realm constraint");
           *rc = TR_CFG_NOPARSE;
           goto cleanup;
@@ -203,7 +198,7 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR
         *rc = TR_CFG_NOPARSE;
         goto cleanup;
       } else if (json_array_size(jdc) > 0) {
-        if (NULL == (filt->lines[i]->domain_cons = tr_cfg_parse_one_constraint(filt->lines[i], "domain", jdc, rc))) {
+        if (NULL == (fline->domain_cons = tr_cfg_parse_one_constraint(fline, "domain", jdc, rc))) {
           tr_debug("tr_cfg_parse_one_filter: Error parsing domain constraint");
           *rc = TR_CFG_NOPARSE;
           goto cleanup;
@@ -239,14 +234,14 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR
       }
 
       /* allocate the filter spec */
-      if (NULL == (filt->lines[i]->specs[j] = tr_fspec_new(filt->lines[i]))) {
+      if (NULL == (fline->specs[j] = tr_fspec_new(fline))) {
         tr_debug("tr_cfg_parse_one_filter: Out of memory.");
         *rc = TR_CFG_NOMEM;
         goto cleanup;
       }
 
       /* fill in the field */
-      if (NULL == (filt->lines[i]->specs[j]->field = tr_new_name(json_string_value(jfield)))) {
+      if (NULL == (fline->specs[j]->field = tr_new_name(json_string_value(jfield)))) {
         tr_debug("tr_cfg_parse_one_filter: Out of memory.");
         *rc = TR_CFG_NOMEM;
         goto cleanup;
@@ -259,7 +254,7 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR
           *rc = TR_CFG_NOMEM;
           goto cleanup;
         }
-        tr_fspec_add_match(filt->lines[i]->specs[j], name);
+        tr_fspec_add_match(fline->specs[j], name);
       } else {
         /* jmatch is an array (we checked earlier) */
         json_array_foreach(jmatch, k, this_jmatch) {
@@ -268,19 +263,26 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR
             *rc = TR_CFG_NOMEM;
             goto cleanup;
           }
-          tr_fspec_add_match(filt->lines[i]->specs[j], name);
+          tr_fspec_add_match(fline->specs[j], name);
         }
       }
-      if (!tr_filter_validate_spec_field(ftype, filt->lines[i]->specs[j])){
+      if (!tr_filter_validate_spec_field(ftype, fline->specs[j])){
         tr_debug("tr_cfg_parse_one_filter: Invalid filter field \"%.*s\" for %s filter, spec %d, filter %d.",
-                 filt->lines[i]->specs[j]->field->len,
-                 filt->lines[i]->specs[j]->field->buf,
+                 fline->specs[j]->field->len,
+                 fline->specs[j]->field->buf,
                  tr_filter_type_to_string(filt->type),
                  i, j);
         *rc = TR_CFG_ERROR;
         goto cleanup;
       }
     }
+
+    if (NULL == tr_filter_add_line(filt, fline)) {
+      tr_debug("tr_cfg_parse_one_filter: Error adding line %d for %s filter",
+               i+1, tr_filter_type_to_string(filt->type));
+      *rc = TR_CFG_NOMEM;
+      goto cleanup;
+    }
   }
 
   /* check that the filter is valid */
index 960edce..3487cb7 100644 (file)
@@ -113,6 +113,7 @@ static TR_FILTER_SET *tr_cfg_default_filters(TALLOC_CTX *mem_ctx, TR_NAME *realm
 {
   TALLOC_CTX *tmp_ctx=talloc_new(NULL);
   TR_FILTER *filt=NULL;
+  TR_FLINE *fline = NULL;
   TR_FILTER_SET *filt_set=NULL;
   TR_CONSTRAINT *cons=NULL;
   TR_NAME *name=NULL;
@@ -147,16 +148,17 @@ static TR_FILTER_SET *tr_cfg_default_filters(TALLOC_CTX *mem_ctx, TR_NAME *realm
     goto cleanup;
   }
   tr_filter_set_type(filt, TR_FILTER_TYPE_TID_INBOUND);
-  filt->lines[0]=tr_fline_new(filt);
-  if (filt->lines[0]==NULL) {
+
+  fline = tr_fline_new(tmp_ctx);
+  if (fline==NULL) {
     tr_debug("tr_cfg_default_filters: could not allocate filter line.");
     *rc=TR_CFG_NOMEM;
     goto cleanup;
   }
 
-  filt->lines[0]->action=TR_FILTER_ACTION_ACCEPT;
-  filt->lines[0]->specs[0]=tr_fspec_new(filt->lines[0]);
-  filt->lines[0]->specs[0]->field=n_rp_realm_1;
+  fline->action=TR_FILTER_ACTION_ACCEPT;
+  fline->specs[0]=tr_fspec_new(fline);
+  fline->specs[0]->field=n_rp_realm_1;
   n_rp_realm_1=NULL; /* we don't own this name any more */
 
   name=tr_dup_name(realm);
@@ -165,12 +167,12 @@ static TR_FILTER_SET *tr_cfg_default_filters(TALLOC_CTX *mem_ctx, TR_NAME *realm
     *rc=TR_CFG_NOMEM;
     goto cleanup;
   }
-  tr_fspec_add_match(filt->lines[0]->specs[0], name);
+  tr_fspec_add_match(fline->specs[0], name);
   name=NULL; /* we no longer own the name */
 
   /* now do the wildcard name */
-  filt->lines[0]->specs[1]=tr_fspec_new(filt->lines[0]);
-  filt->lines[0]->specs[1]->field=n_rp_realm_2;
+  fline->specs[1]=tr_fspec_new(fline);
+  fline->specs[1]->field=n_rp_realm_2;
   n_rp_realm_2=NULL; /* we don't own this name any more */
 
   if (NULL==(name=tr_name_cat(n_prefix, realm))) {
@@ -179,11 +181,11 @@ static TR_FILTER_SET *tr_cfg_default_filters(TALLOC_CTX *mem_ctx, TR_NAME *realm
     goto cleanup;
   }
 
-  tr_fspec_add_match(filt->lines[0]->specs[1], name);
+  tr_fspec_add_match(fline->specs[1], name);
   name=NULL; /* we no longer own the name */
 
   /* domain constraint */
-  if (NULL==(cons=tr_constraint_new(filt->lines[0]))) {
+  if (NULL==(cons=tr_constraint_new(fline))) {
     tr_debug("tr_cfg_default_filters: could not allocate domain constraint.");
     *rc=TR_CFG_NOMEM;
     goto cleanup;
@@ -206,11 +208,11 @@ static TR_FILTER_SET *tr_cfg_default_filters(TALLOC_CTX *mem_ctx, TR_NAME *realm
   }
   cons->matches[1]=name;
   name=NULL;
-  filt->lines[0]->domain_cons=cons;
+  fline->domain_cons=cons;
 
 
   /* realm constraint */
-  if (NULL==(cons=tr_constraint_new(filt->lines[0]))) {
+  if (NULL==(cons=tr_constraint_new(fline))) {
     tr_debug("tr_cfg_default_filters: could not allocate realm constraint.");
     *rc=TR_CFG_NOMEM;
     goto cleanup;
@@ -233,7 +235,14 @@ static TR_FILTER_SET *tr_cfg_default_filters(TALLOC_CTX *mem_ctx, TR_NAME *realm
   }
   cons->matches[1]=name;
   name=NULL;
-  filt->lines[0]->realm_cons=cons;
+  fline->realm_cons=cons;
+
+  /* put the fline in the filter */
+  if (NULL == tr_filter_add_line(filt, fline)) {
+    tr_debug("tr_cfg_default_filters: could not add line to filter.");
+    *rc = TR_CFG_NOMEM;
+    goto cleanup;
+  }
 
   /* put the filter in a set */
   filt_set=tr_filter_set_new(tmp_ctx);
index 52dffb4..529be85 100644 (file)
@@ -408,34 +408,34 @@ int tr_filter_apply(TR_FILTER_TARGET *target,
                     TR_CONSTRAINT_SET **constraints,
                     TR_FILTER_ACTION *out_action)
 {
-  unsigned int ii=0, jj=0;
+  TALLOC_CTX *tmp_ctx = talloc_new(NULL);
+  TR_FILTER_ITER *filt_iter = tr_filter_iter_new(tmp_ctx);
+  TR_FLINE *this_fline = NULL;
+  unsigned int jj=0;
   int retval=TR_FILTER_NO_MATCH;
 
   /* Default action is reject */
   *out_action = TR_FILTER_ACTION_REJECT;
 
   /* Validate filter */
-  if ((filt==NULL) || (filt->type==TR_FILTER_TYPE_UNKNOWN))
+  if ((filt_iter == NULL) || (filt==NULL) || (filt->type==TR_FILTER_TYPE_UNKNOWN)) {
+    talloc_free(tmp_ctx);
     return TR_FILTER_NO_MATCH;
+  }
 
   /* Step through filter lines looking for a match. If a line matches, retval
    * will be set to TR_FILTER_MATCH, so stop then. */
-  for (ii=0, retval=TR_FILTER_NO_MATCH;
-       ii<TR_MAX_FILTER_LINES;
-       ii++) {
-    /* skip empty lines (these shouldn't really happen) */
-    if (filt->lines[ii]==NULL)
-      continue;
-
+  this_fline = tr_filter_iter_first(filt_iter, filt);
+  while(this_fline) {
     /* Assume we are going to succeed. If any specs fail to match, we'll set
      * this to TR_FILTER_NO_MATCH. */
     retval=TR_FILTER_MATCH;
     for (jj=0; jj<TR_MAX_FILTER_SPECS; jj++) {
       /* skip empty specs (these shouldn't really happen either) */
-      if (filt->lines[ii]->specs[jj]==NULL)
+      if (this_fline->specs[jj]==NULL)
         continue;
 
-      if (!tr_fspec_matches(filt->lines[ii]->specs[jj], filt->type, target)) {
+      if (!tr_fspec_matches(this_fline->specs[jj], filt->type, target)) {
         retval=TR_FILTER_NO_MATCH; /* set this in case this is the last filter line */
         break; /* give up on this filter line */
       }
@@ -447,11 +447,11 @@ int tr_filter_apply(TR_FILTER_TARGET *target,
 
   if (retval==TR_FILTER_MATCH) {
     /* Matched line ii. Grab its action and constraints. */
-    *out_action = filt->lines[ii]->action;
+    *out_action = this_fline->action;
     if (constraints!=NULL) {
       /* if either constraint is missing, these are no-ops */
-      tr_constraint_add_to_set(constraints, filt->lines[ii]->realm_cons);
-      tr_constraint_add_to_set(constraints, filt->lines[ii]->domain_cons);
+      tr_constraint_add_to_set(constraints, this_fline->realm_cons);
+      tr_constraint_add_to_set(constraints, this_fline->domain_cons);
     }
   }
 
@@ -576,12 +576,14 @@ TR_FLINE *tr_fline_new(TALLOC_CTX *mem_ctx)
 TR_FILTER *tr_filter_new(TALLOC_CTX *mem_ctx)
 {
   TR_FILTER *f = talloc(mem_ctx, TR_FILTER);
-  int ii = 0;
 
   if (f != NULL) {
     f->type = TR_FILTER_TYPE_UNKNOWN;
-    for (ii = 0; ii < TR_MAX_FILTER_LINES; ii++)
-      f->lines[ii] = NULL;
+    f->lines = g_ptr_array_new();
+    if (f->lines == NULL) {
+      talloc_free(f);
+      return NULL;
+    }
   }
   return f;
 }
@@ -602,6 +604,65 @@ TR_FILTER_TYPE tr_filter_get_type(TR_FILTER *filt)
 }
 
 /**
+ * Add a TR_FLINE to a filter
+ *
+ * Steals the line into its context on success
+ *
+ * @param filt
+ * @param line
+ * @return line, or null on failure
+ */
+TR_FLINE *tr_filter_add_line(TR_FILTER *filt, TR_FLINE *line)
+{
+  guint old_len = filt->lines->len;
+  g_ptr_array_add(filt->lines, line);
+  talloc_steal(filt, line); /* take this no matter what */
+  if (old_len == filt->lines->len)
+    return NULL; /* failed to add */
+  return line;
+}
+
+/**
+ * Iterator for TR_FLINES in a TR_FILTER
+ *
+ * @param mem_ctx
+ * @return
+ */
+TR_FILTER_ITER *tr_filter_iter_new(TALLOC_CTX *mem_ctx)
+{
+  TR_FILTER_ITER *iter = talloc(mem_ctx, TR_FILTER_ITER);
+  if (iter) {
+    iter->filter = NULL;
+  }
+  return iter;
+}
+
+void tr_filter_iter_free(TR_FILTER_ITER *iter)
+{
+  talloc_free(iter);
+}
+
+TR_FLINE *tr_filter_iter_next(TR_FILTER_ITER *iter)
+{
+  if (!iter)
+    return NULL;
+
+  if (iter->ii < iter->filter->lines->len)
+    return g_ptr_array_index(iter->filter->lines, iter->ii++);
+  return NULL;
+}
+
+TR_FLINE *tr_filter_iter_first(TR_FILTER_ITER *iter, TR_FILTER *filter)
+{
+  if (!iter || !filter)
+    return NULL;
+
+  iter->filter = filter;
+  iter->ii = 0;
+  return tr_filter_iter_next(iter);
+}
+
+/**
  * Check that a filter is valid, i.e., can be processed.
  *
  * @param filt Filter to verify
@@ -609,10 +670,15 @@ TR_FILTER_TYPE tr_filter_get_type(TR_FILTER *filt)
  */
 int tr_filter_validate(TR_FILTER *filt)
 {
-  size_t ii=0, jj=0, kk=0;
-
-  if (!filt)
+  TALLOC_CTX *tmp_ctx = talloc_new(NULL);
+  size_t jj=0, kk=0;
+  TR_FILTER_ITER *filt_iter = tr_filter_iter_new(tmp_ctx);
+  TR_FLINE *this_fline = NULL;
+  
+  if (!filt) {
+    talloc_free(tmp_ctx);
     return 0;
+  }
 
   /* check that we recognize the type */
   switch(filt->type) {
@@ -622,41 +688,48 @@ int tr_filter_validate(TR_FILTER *filt)
       break;
 
     default:
+      talloc_free(tmp_ctx);
       return 0; /* if we get here, either TR_FILTER_TYPE_UNKNOWN or an invalid value was found */
   }
-  for (ii=0; ii<TR_MAX_FILTER_LINES; ii++) {
-    if (filt->lines[ii]==NULL)
-      continue; /* an empty filter line is valid */
-
+  
+  this_fline = tr_filter_iter_first(filt_iter, filt);
+  while(this_fline) {
     /* check that we recognize the action */
-    switch(filt->lines[ii]->action) {
+    switch(this_fline->action) {
       case TR_FILTER_ACTION_ACCEPT:
       case TR_FILTER_ACTION_REJECT:
         break;
 
       default:
         /* if we get here, either TR_FILTER_ACTION_UNKNOWN or an invalid value was found */
+        talloc_free(tmp_ctx);
         return 0;
     }
 
     for (jj=0; jj<TR_MAX_FILTER_SPECS; jj++) {
-      if (filt->lines[ii]->specs[jj]==NULL)
+      if (this_fline->specs[jj]==NULL)
         continue; /* an empty filter spec is valid */
 
-      if (!tr_filter_validate_spec_field(filt->type, filt->lines[ii]->specs[jj]))
+      if (!tr_filter_validate_spec_field(filt->type, this_fline->specs[jj])) {
+        talloc_free(tmp_ctx);
         return 0;
+      }
 
       /* check that at least one match is non-null */
       for (kk=0; kk<TR_MAX_FILTER_SPEC_MATCHES; kk++) {
-        if (filt->lines[ii]->specs[jj]->match[kk]!=NULL)
+        if (this_fline->specs[jj]->match[kk]!=NULL)
           break;
       }
-      if (kk==TR_MAX_FILTER_SPEC_MATCHES)
+      if (kk==TR_MAX_FILTER_SPEC_MATCHES) {
+        talloc_free(tmp_ctx);
         return 0;
+      }
     }
+    this_fline = tr_filter_iter_next(filt_iter);
   }
 
   /* We ran the gauntlet. Success! */
+  talloc_free(tmp_ctx);
   return 1;
 }
 
index cc22b02..68da1b1 100644 (file)
@@ -145,6 +145,33 @@ cleanup:
   return retval;
 }
 
+static json_t *tr_flines_to_json_array(TR_FILTER *filt)
+{
+  json_t *jarray = json_array();
+  json_t *retval = NULL;
+  TR_FILTER_ITER *iter = tr_filter_iter_new(NULL);
+  TR_FLINE *this_fline = NULL;
+
+  if ((jarray == NULL) || (iter == NULL))
+    goto cleanup;
+
+  this_fline = tr_filter_iter_first(iter, filt);
+  while(this_fline) {
+    ARRAY_APPEND_OR_FAIL(jarray, tr_fline_to_json(this_fline));
+    this_fline = tr_filter_iter_next(iter);
+  }
+  /* success */
+  retval = jarray;
+  json_incref(retval);
+
+cleanup:
+  if (jarray)
+    json_decref(jarray);
+  if (iter)
+    tr_filter_iter_free(iter);
+
+  return retval;
+}
 json_t *tr_filter_set_to_json(TR_FILTER_SET *filter_set)
 {
   json_t *fset_json = NULL;
@@ -166,9 +193,7 @@ json_t *tr_filter_set_to_json(TR_FILTER_SET *filter_set)
     filt = tr_filter_set_get(filter_set, *filt_type);
     if (filt) {
       OBJECT_SET_OR_FAIL(fset_json, tr_filter_type_to_string(*filt_type),
-                         items_to_json_array((void **)filt->lines,
-                                             (ITEM_ENCODER_FUNC *) tr_fline_to_json,
-                                             TR_MAX_FILTER_LINES));
+                         tr_flines_to_json_array(filt));
     }
   }
 
index ece3650..caabe5d 100644 (file)
 
 #include <talloc.h>
 #include <jansson.h>
+#include <glib.h>
 
 #include <tr_name_internal.h>
 #include <trust_router/tr_constraint.h>
 #include <trust_router/tid.h>
 #include <trust_router/trp.h>
 
-#define TR_MAX_FILTERS  5
-#define TR_MAX_FILTER_LINES 8
 #define TR_MAX_FILTER_SPECS 8
 #define TR_MAX_FILTER_SPEC_MATCHES 64
 
@@ -81,9 +80,13 @@ typedef struct tr_fline {
 
 typedef struct tr_filter {
   TR_FILTER_TYPE type;
-  TR_FLINE *lines[TR_MAX_FILTER_LINES];
+  GPtrArray *lines;
 } TR_FILTER;
 
+typedef struct tr_filter_iter {
+  TR_FILTER *filter;
+  guint ii;
+} TR_FILTER_ITER;
 
 typedef struct tr_filter_set TR_FILTER_SET;
 struct tr_filter_set {
@@ -109,12 +112,11 @@ int tr_filter_set_add(TR_FILTER_SET *set, TR_FILTER *new);
 TR_FILTER *tr_filter_set_get(TR_FILTER_SET *set, TR_FILTER_TYPE type);
 
 TR_FILTER *tr_filter_new(TALLOC_CTX *mem_ctx);
-
 void tr_filter_free(TR_FILTER *filt);
 
 void tr_filter_set_type(TR_FILTER *filt, TR_FILTER_TYPE type);
-
 TR_FILTER_TYPE tr_filter_get_type(TR_FILTER *filt);
+TR_FLINE *tr_filter_add_line(TR_FILTER *filt, TR_FLINE *line);
 
 TR_FLINE *tr_fline_new(TALLOC_CTX *mem_ctx);
 
@@ -128,6 +130,10 @@ void tr_fspec_add_match(TR_FSPEC *fspec, TR_NAME *match);
 
 int tr_fspec_matches(TR_FSPEC *fspec, TR_FILTER_TYPE ftype, TR_FILTER_TARGET *target);
 
+TR_FILTER_ITER *tr_filter_iter_new(TALLOC_CTX *mem_ctx);
+void tr_filter_iter_free(TR_FILTER_ITER *iter);
+TR_FLINE *tr_filter_iter_first(TR_FILTER_ITER *iter, TR_FILTER *filter);
+TR_FLINE *tr_filter_iter_next(TR_FILTER_ITER *iter);
 
 /*In tr_constraint.c and exported, but not really a public symbol; needed by tr_filter.c and by tr_constraint.c*/
 int TR_EXPORT tr_prefix_wildcard_match(const char *str, const char *wc_str);