From 7151e05c6df596fd3678b04f4ab7ccd3e3609a3b Mon Sep 17 00:00:00 2001 From: Martin Angers Date: Mon, 3 Oct 2022 16:43:47 -0400 Subject: [PATCH] Remove value validation of agent_options.{packs,schedule} (#8066) --- server/fleet/agent_options.go | 65 +++--------------------------- server/fleet/agent_options_test.go | 13 ++++-- 2 files changed, 15 insertions(+), 63 deletions(-) diff --git a/server/fleet/agent_options.go b/server/fleet/agent_options.go index 9d3dd9afbd..4f14c791c9 100644 --- a/server/fleet/agent_options.go +++ b/server/fleet/agent_options.go @@ -73,21 +73,13 @@ func ValidateJSONAgentOptions(rawJSON json.RawMessage) error { type osqueryAgentOptions struct { Options osqueryOptions `json:"options"` - Schedule map[string]struct { - Query string `json:"query"` - Interval int `json:"interval"` - Removed bool `json:"removed"` - Snapshot bool `json:"snapshot"` - Platform string `json:"platform"` - Version string `json:"version"` - Shard int `json:"shard"` - Denylist bool `json:"denylist"` - } `json:"schedule"` + // Schedule is allowed as top-level key but we don't validate its value. + // See https://github.com/fleetdm/fleet/issues/7871#issuecomment-1265531018 + Schedule json.RawMessage `json:"schedule"` - // Packs may have a string or struct value, both are supported, so a raw value - // is used to unmarshal and the type-check is done after. When it is a struct, - // it must be compatible with the osqueryPack struct below. - Packs map[string]json.RawMessage `json:"packs"` + // Packs is allowed as top-level key but we don't validate its value. + // See https://github.com/fleetdm/fleet/issues/7871#issuecomment-1265531018 + Packs json.RawMessage `json:"packs"` FilePaths map[string][]string `json:"file_paths"` FileAccesses []string `json:"file_accesses"` @@ -127,24 +119,6 @@ type osqueryAgentOptions struct { } `json:"events"` } -// When the osqueryAgentOptions.Packs field maps to a struct, this is the -// definition that the struct must have. -type osqueryPack struct { - Queries map[string]struct { - Query string `json:"query"` - Interval int `json:"interval"` - Description string `json:"description"` - // TODO(mna): unclear if the following fields can be present in a pack's query? - Removed bool `json:"removed"` - Snapshot bool `json:"snapshot"` - Denylist bool `json:"denylist"` - } `json:"schedule"` - Platform string `json:"platform"` - Version string `json:"version"` - Shard int `json:"shard"` - Discovery []string `json:"discovery"` -} - // NOTE: generate automatically with `go run ./tools/osquery-agent-options/main.go` type osqueryOptions struct { AuditAllowAcceptSocketEvents bool `json:"audit_allow_accept_socket_events"` @@ -518,33 +492,6 @@ func validateJSONAgentOptionsSet(rawJSON json.RawMessage) error { return fmt.Errorf("options.logger_tls_endpoint must be a path starting with '/': %q", opts.Options.LoggerTlsEndpoint) } } - - // Packs may have a string or struct value, both are supported - for packKey, pack := range opts.Packs { - if len(pack) == 0 { - // should never happen, just to make sure we avoid a panic reading the first byte - continue - } - switch pack[0] { - case '"': - // a string, this is fine - case '{': - // an object, must match the pack struct - var packStruct osqueryPack - if err := jsonStrictDecode(pack, &packStruct); err != nil { - return fmt.Errorf("pack %q: %w", packKey, err) - } - case 't', 'f': - return fmt.Errorf("pack %q: invalid bool value, expected string or object", packKey) - case 'n': - return fmt.Errorf("pack %q: invalid null value, expected string or object", packKey) - case '[': - return fmt.Errorf("pack %q: invalid array value, expected string or object", packKey) - default: - return fmt.Errorf("pack %q: invalid number value, expected string or object", packKey) - } - } - return nil } diff --git a/server/fleet/agent_options_test.go b/server/fleet/agent_options_test.go index d903672f15..fdbae8a6e1 100644 --- a/server/fleet/agent_options_test.go +++ b/server/fleet/agent_options_test.go @@ -78,7 +78,7 @@ func TestValidateAgentOptions(t *testing.T) { } } }}`, ""}, - {"invalid packs object key", `{"config":{ + {"invalid packs object key is accepted as we do not validate packs", `{"config":{ "packs": { "pack1": { "schedule": { @@ -90,12 +90,17 @@ func TestValidateAgentOptions(t *testing.T) { "platform": "darwin" } } - }}`, `unknown field "foo"`}, - {"invalid packs type", `{"config":{ + }}`, ``}, + {"invalid packs type is accepted as we do not validate packs", `{"config":{ "packs": { "pack1": 1 } - }}`, `invalid number value`}, + }}`, ``}, + {"invalid schedule type is accepted as we do not validate schedule", `{"config":{ + "schedule": { + "foo": 1 + } + }}`, ``}, {"option added in osquery 5.5.1", `{"config":{ "options": { "malloc_trim_threshold": 100