Merge pull request #92 from painless-security/jennifer/reduce_logging

Reduce logging during connection accept and validate internal configuration

diff --git a/common/tr_config.c b/common/tr_config.c
index 2ddda12..e0becec 100644 (file)
--- a/common/tr_config.c
+++ b/common/tr_config.c
@@ -195,11 +195,10 @@ TR_CFG_RC tr_cfg_validate(TR_CFG *trc)
   if (!trc)
     return TR_CFG_BAD_PARAMS;
 
-  if ((NULL == trc->internal)||
-      (NULL == trc->internal->hostname)) {
-    tr_debug("tr_cfg_validate: Error: No internal configuration, or no hostname.");
+  /* validate the internal config - error messages will be generated there, so don't genreate
+   * our own */
+  if (tr_cfg_validate_internal(trc->internal) != TR_CFG_SUCCESS)
     rc = TR_CFG_ERROR;
-  }
 
   if (NULL == trc->rp_clients) {
     tr_debug("tr_cfg_validate: Error: No RP Clients configured");

diff --git a/common/tr_config_internal.c b/common/tr_config_internal.c
index b6a16ad..f49c02f 100644 (file)
--- a/common/tr_config_internal.c
+++ b/common/tr_config_internal.c
@@ -123,11 +123,14 @@ static TR_CFG_RC tr_cfg_parse_unsigned(json_t *src, const char *key, unsigned in
   /* See if we have a value for this key; do nothing if not */
   jtmp = json_object_get(src, key);
   if (jtmp) {
-    if (json_is_number(jtmp)) {
-      *dest = (unsigned int) json_integer_value(jtmp);
-    } else {
+    if (! json_is_number(jtmp)) {
       tr_debug("tr_cfg_parse_unsigned: Parsing error, %s is not a number.", key);
       return TR_CFG_NOPARSE;
+    } else if (json_integer_value(jtmp) < 0) {
+      tr_debug("tr_cfg_parse_unsigned: Value %d < 0.", json_integer_value(jtmp));
+      return TR_CFG_NOPARSE;
+    } else {
+      *dest = (unsigned int) json_integer_value(jtmp);
     }
   }
 
@@ -279,3 +282,108 @@ TR_CFG_RC tr_cfg_parse_internal(TR_CFG *trc, json_t *jint)
   tr_debug("tr_cfg_parse_internal: Internal config parsed.");
   return TR_CFG_SUCCESS;
 }
+
+static int invalid_port(int port)
+{
+  return ((port <= 0) || (port > 65536));
+}
+
+/**
+ * Validate the internal configuration of the trust router
+ *
+ * Validates fields, emitting errors if there are any. Safe to call with
+ * a null int_cfg, but this results in an error being returned.
+ *
+ * @param int_cfg pointer to an internal configuration (NULL is safe)
+ * @return success or error
+ */
+TR_CFG_RC tr_cfg_validate_internal(TR_CFG_INTERNAL *int_cfg)
+{
+  TR_CFG_RC rc;
+
+  /* ensure we have an internal configuration and exit if not  */
+  if (NULL == int_cfg) {
+    tr_debug("tr_cfg_validate_internal: No internal configuration present.");
+    return TR_CFG_BAD_PARAMS;
+  }
+
+  /* Assume we are going to succeed. If any errors are encountered, emit a message
+   * and set the return code to an error. Don't exit early, emit all the errors
+   * at once if we can. */
+  rc = TR_CFG_SUCCESS;
+
+  /*** Validate hostname ***/
+  if (NULL == int_cfg->hostname) {
+    tr_debug("tr_cfg_validate_internal: No hostname specified.");
+    rc = TR_CFG_ERROR;
+  }
+
+  /*** Validate various intervals ***/
+  if (TR_MIN_TRP_CONNECT_INTERVAL > int_cfg->trp_connect_interval) {
+    tr_debug(
+        "tr_cfg_validate_internal: Error: trp_connect_interval must be at least %d (currently %d).",
+        TR_MIN_TRP_CONNECT_INTERVAL, int_cfg->trp_connect_interval);
+    rc = TR_CFG_ERROR;
+  }
+
+  if (TR_MIN_TRP_SWEEP_INTERVAL > int_cfg->trp_sweep_interval) {
+    tr_debug(
+        "tr_cfg_validate_internal: Error: trp_sweep_interval must be at least %d (currently %d).",
+        TR_MIN_TRP_SWEEP_INTERVAL, int_cfg->trp_sweep_interval);
+    rc = TR_CFG_ERROR;
+  }
+
+  if (TR_MIN_TRP_UPDATE_INTERVAL > int_cfg->trp_update_interval) {
+    tr_debug(
+        "tr_cfg_validate_internal: Error: trp_update_interval must be at least %d (currently %d).",
+        TR_MIN_TRP_UPDATE_INTERVAL, int_cfg->trp_update_interval);
+    rc = TR_CFG_ERROR;
+  }
+
+  if (TR_MIN_CFG_POLL_INTERVAL > int_cfg->cfg_poll_interval) {
+    tr_debug(
+        "tr_cfg_validate_internal: Error: cfg_poll_interval must be at least %d (currently %d).",
+        TR_MIN_CFG_POLL_INTERVAL, int_cfg->cfg_poll_interval);
+    rc = TR_CFG_ERROR;
+  }
+
+  if (TR_MIN_CFG_SETTLING_TIME > int_cfg->cfg_settling_time) {
+    tr_debug(
+        "tr_cfg_validate_internal: Error: cfg_settling_time must be at least %d (currently %d).",
+        TR_MIN_CFG_SETTLING_TIME, int_cfg->cfg_settling_time);
+    rc = TR_CFG_ERROR;
+  }
+
+  /*** Validate ports ***/
+  if (invalid_port(int_cfg->tids_port)) {
+    tr_debug("tr_cfg_validate_internal: Error: invalid tids_port (%d).", int_cfg->tids_port);
+    rc = TR_CFG_ERROR;
+  }
+
+  if (invalid_port(int_cfg->trps_port)) {
+    tr_debug("tr_cfg_validate_internal: Error: invalid trps_port (%d).", int_cfg->trps_port);
+    rc = TR_CFG_ERROR;
+  }
+
+  if (invalid_port(int_cfg->monitoring_port)) {
+    tr_debug("tr_cfg_validate_internal: Error: invalid monitoring port (%d).", int_cfg->monitoring_port);
+    rc = TR_CFG_ERROR;
+  }
+
+  /*** Validate tid request timeout ***/
+  if (TR_MIN_TID_REQ_TIMEOUT > int_cfg->tid_req_timeout) {
+    tr_debug("tr_cfg_validate_internal: Error: tid_request_timeout must be at least %d (currently %d).",
+             TR_MIN_TID_REQ_TIMEOUT, int_cfg->tid_req_timeout);
+    rc = TR_CFG_ERROR;
+  }
+
+  /*** Validate tid response parameters ***/
+  if ((int_cfg->tid_resp_numer <= 0)
+      || (int_cfg->tid_resp_denom <= 0)
+      || (int_cfg->tid_resp_numer > int_cfg->tid_resp_denom)) {
+    tr_debug("tr_cfg_validate_internal: Error: invalid tid_response_numerator / tid_response_denominator. Both must be positive and the numerator/denominator ratio must be <= 1 (currently %d/%d).",
+             int_cfg->tid_resp_numer, int_cfg->tid_resp_denom);
+    rc = TR_CFG_ERROR;
+  }
+  return rc;
+}
\ No newline at end of file

diff --git a/common/tr_socket.c b/common/tr_socket.c
index a45ff31..2a4d34e 100644 (file)
--- a/common/tr_socket.c
+++ b/common/tr_socket.c
@@ -188,9 +188,9 @@ int tr_sock_accept(int sock)
   if (0 > (conn = accept(sock, (struct sockaddr *)&(peeraddr), &addr_len))) {
     if (strerror_r(errno, err, sizeof(err)))
       snprintf(err, sizeof(err), "errno = %d", errno);
-    tr_err("tr_sock_accept: Unable to accept connection: %s", err);
+    tr_debug("tr_sock_accept: Unable to accept connection: %s", err);
   } else {
-    tr_notice("tr_sock_accept: Incoming connection on fd %d from %s",
+    tr_info("tr_sock_accept: Incoming connection on fd %d from %s",
               conn,
               tr_sock_ip_address((struct sockaddr *)&peeraddr,
                                  peeraddr_string,

diff --git a/include/tr_config.h b/include/tr_config.h
index 47be960..51b98ef 100644 (file)
--- a/include/tr_config.h
+++ b/include/tr_config.h
@@ -64,6 +64,14 @@
 #define TR_DEFAULT_TID_RESP_NUMER 2
 #define TR_DEFAULT_TID_RESP_DENOM 3
 
+/* limits on values for validations */
+#define TR_MIN_TRP_CONNECT_INTERVAL 5
+#define TR_MIN_TRP_SWEEP_INTERVAL 5
+#define TR_MIN_TRP_UPDATE_INTERVAL 5
+#define TR_MIN_CFG_POLL_INTERVAL 1
+#define TR_MIN_CFG_SETTLING_TIME 0
+#define TR_MIN_TID_REQ_TIMEOUT 1
+
 #define TR_CFG_INVALID_SERIAL -1
 
 typedef enum tr_cfg_rc {
@@ -131,6 +139,7 @@ void tr_print_comm_rps(TR_COMM_TABLE *ctab, TR_COMM *comm);
 
 /* tr_config_internal.c */
 TR_CFG_RC tr_cfg_parse_internal(TR_CFG *trc, json_t *jint);
+TR_CFG_RC tr_cfg_validate_internal(TR_CFG_INTERNAL *int_cfg);
 
 /* tr_config_comms.c */
 TR_IDP_REALM *tr_cfg_find_idp (TR_CFG *tr_cfg, TR_NAME *idp_id, TR_CFG_RC *rc);

diff --git a/mon/mons.c b/mon/mons.c
index 72070cf..4aff0d1 100644 (file)
--- a/mon/mons.c
+++ b/mon/mons.c
@@ -275,7 +275,7 @@ int mons_accept(MONS_INSTANCE *mons, int listen)
   int pid=-1;
 
   if (0 > (conn = tr_sock_accept(listen))) {
-    tr_err("mons_accept: Error accepting connection");
+    tr_debug("mons_accept: Error accepting connection");
     return 1;
   }
 

diff --git a/tid/tids.c b/tid/tids.c
index f600d66..a984b8d 100644 (file)
--- a/tid/tids.c
+++ b/tid/tids.c
@@ -464,7 +464,7 @@ int tids_accept(TIDS_INSTANCE *tids, int listen)
   struct tid_process tp = {0};
 
   if (0 > (conn = tr_sock_accept(listen))) {
-    tr_err("tids_accept: Error accepting connection");
+    tr_debug("tids_accept: Error accepting connection");
     return 1;
   }
 
@@ -623,7 +623,7 @@ int tids_start(TIDS_INSTANCE *tids,
 
       if (poll_fd[ii].revents & POLLIN) {
         if (tids_accept(tids, poll_fd[ii].fd))
-          tr_err("tids_start: error in tids_accept().");
+          tr_debug("tids_start: error in tids_accept().");
       }
     }
   }