From 2025afee717394964aefbca604344b7ba109ebbd Mon Sep 17 00:00:00 2001
From: Zach Wasserman <zach@fleetdm.com>
Date: Fri, 5 Mar 2021 08:25:46 -0800
Subject: [PATCH] Better handling of config_refresh values from clients (#388)

Since the original logic was implemented, there have been some changes
in the way that config refreshes are configured. This commit reflects
those changes and should be backwards compatible.

Closes #357
---
 .../hosts/HostDetailsPage/HostDetailsPage.jsx |  2 +-
 server/service/service_osquery.go             | 22 ++++++++++++++-----
 server/service/service_osquery_test.go        | 19 ++++++++++++++++
 3 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/frontend/pages/hosts/HostDetailsPage/HostDetailsPage.jsx b/frontend/pages/hosts/HostDetailsPage/HostDetailsPage.jsx
index 360080a350..e98ae7bc35 100644
--- a/frontend/pages/hosts/HostDetailsPage/HostDetailsPage.jsx
+++ b/frontend/pages/hosts/HostDetailsPage/HostDetailsPage.jsx
@@ -296,7 +296,7 @@ export class HostDetailsPage extends Component {
           <p className="section__header">Osquery configuration</p>
           <div className="info">
             <div className="info__item info__item--title">
-              <span className="info__header">Config TLS refresh</span>
+              <span className="info__header">Config refresh</span>
               <span className="info__data">{osqueryData.config_tls_refresh}</span>
             </div>
             <div className="info__item info__item--title">
diff --git a/server/service/service_osquery.go b/server/service/service_osquery.go
index b9f93a7895..e1e895068e 100644
--- a/server/service/service_osquery.go
+++ b/server/service/service_osquery.go
@@ -188,11 +188,8 @@ func (svc service) GetClientConfig(ctx context.Context) (map[string]interface{},
 		config["packs"] = json.RawMessage(packJSON)
 	}
 
-	// Save interval values if they have been updated. Note
-	// config_tls_refresh can only be set in the osquery flags so is
-	// ignored here.
+	// Save interval values if they have been updated.
 	saveHost := false
-
 	if options, ok := config["options"].(map[string]interface{}); ok {
 		distributedIntervalVal, ok := options["distributed_interval"]
 		distributedInterval, err := cast.ToUintE(distributedIntervalVal)
@@ -207,6 +204,16 @@ func (svc service) GetClientConfig(ctx context.Context) (map[string]interface{},
 			host.LoggerTLSPeriod = loggerTLSPeriod
 			saveHost = true
 		}
+
+		// Note config_tls_refresh can only be set in the osquery flags (and has
+		// also been deprecated in osquery for quite some time) so is ignored
+		// here.
+		configRefreshVal, ok := options["config_refresh"]
+		configRefresh, err := cast.ToUintE(configRefreshVal)
+		if ok && err == nil && host.ConfigTLSRefresh != configRefresh {
+			host.ConfigTLSRefresh = configRefresh
+			saveHost = true
+		}
 	}
 
 	if saveHost {
@@ -338,6 +345,7 @@ var detailQueries = map[string]struct {
 		Query: `select name, value from osquery_flags where name in ("distributed_interval", "config_tls_refresh", "config_refresh", "logger_tls_period")`,
 		IngestFunc: func(logger log.Logger, host *kolide.Host, rows []map[string]string) error {
 			var configTLSRefresh, configRefresh uint
+			var configRefreshSeen, configTLSRefreshSeen bool
 			for _, row := range rows {
 				switch row["name"] {
 
@@ -356,6 +364,7 @@ var detailQueries = map[string]struct {
 						return errors.Wrap(err, "parsing config_tls_refresh")
 					}
 					configTLSRefresh = uint(interval)
+					configTLSRefreshSeen = true
 
 				case "config_refresh":
 					// After 2.4.6 `config_tls_refresh` was
@@ -365,6 +374,7 @@ var detailQueries = map[string]struct {
 						return errors.Wrap(err, "parsing config_refresh")
 					}
 					configRefresh = uint(interval)
+					configRefreshSeen = true
 
 				case "logger_tls_period":
 					interval, err := strconv.Atoi(emptyToZero(row["value"]))
@@ -379,9 +389,9 @@ var detailQueries = map[string]struct {
 			// 2.4.6 and had a different meaning, we prefer
 			// `config_tls_refresh` if it was set, and use
 			// `config_refresh` as a fallback.
-			if configTLSRefresh != 0 {
+			if configTLSRefreshSeen {
 				host.ConfigTLSRefresh = configTLSRefresh
-			} else {
+			} else if configRefreshSeen {
 				host.ConfigTLSRefresh = configRefresh
 			}
 
diff --git a/server/service/service_osquery_test.go b/server/service/service_osquery_test.go
index 4817c0ca69..4c0de26830 100644
--- a/server/service/service_osquery_test.go
+++ b/server/service/service_osquery_test.go
@@ -1405,6 +1405,25 @@ func TestUpdateHostIntervals(t *testing.T) {
 			}}`),
 			true,
 		},
+		// config_refresh should also cause an update
+		{
+			kolide.Host{
+				DistributedInterval: 11,
+				LoggerTLSPeriod:     33,
+				ConfigTLSRefresh:    60,
+			},
+			kolide.Host{
+				DistributedInterval: 11,
+				LoggerTLSPeriod:     33,
+				ConfigTLSRefresh:    42,
+			},
+			json.RawMessage(`{"options":{
+				"distributed_interval": 11,
+				"logger_tls_period":    33,
+				"config_refresh":    42
+			}}`),
+			true,
+		},
 		// SaveHost should not be called with no changes
 		{
 			kolide.Host{