diff --git a/changes/16775-device-policies-device-safe b/changes/16775-device-policies-device-safe new file mode 100644 index 0000000000..481470ef56 --- /dev/null +++ b/changes/16775-device-policies-device-safe @@ -0,0 +1 @@ +- Fixed device-authenticated ("My device") endpoints so that host policies are returned in a device-safe representation that no longer exposes the policy author's name and email or the policy's raw SQL query. diff --git a/client/device_client.go b/client/device_client.go index 23930b3f7b..3e8b71cda5 100644 --- a/client/device_client.go +++ b/client/device_client.go @@ -188,13 +188,13 @@ func (dc *DeviceClient) Ping() error { // listDevicePoliciesResponse is a local response type for deserializing the device policies response. // Definition duplicated for now (orbit should not depend server/service). type listDevicePoliciesResponse struct { - Err error `json:"error,omitempty"` - Policies []*fleet.HostPolicy `json:"policies"` + Err error `json:"error,omitempty"` + Policies []*fleet.DevicePolicy `json:"policies"` } func (r listDevicePoliciesResponse) Error() error { return r.Err } -func (dc *DeviceClient) getListDevicePolicies(token string) ([]*fleet.HostPolicy, error) { +func (dc *DeviceClient) getListDevicePolicies(token string) ([]*fleet.DevicePolicy, error) { verb, path := "GET", "/api/latest/fleet/device/%s/policies" var responseBody listDevicePoliciesResponse err := dc.request(verb, path, token, "", nil, &responseBody) diff --git a/ee/server/service/devices.go b/ee/server/service/devices.go index ef0a761b5e..6e8526d7da 100644 --- a/ee/server/service/devices.go +++ b/ee/server/service/devices.go @@ -15,8 +15,14 @@ import ( "github.com/fleetdm/fleet/v4/server/ptr" ) -func (svc *Service) ListDevicePolicies(ctx context.Context, host *fleet.Host) ([]*fleet.HostPolicy, error) { - return svc.ds.ListPoliciesForHost(ctx, host) +func (svc *Service) ListDevicePolicies(ctx context.Context, host *fleet.Host) ([]*fleet.DevicePolicy, error) { + policies, err := svc.ds.ListPoliciesForHost(ctx, host) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "list policies for host") + } + // return the device-safe representation of the policies, which excludes + // the policy author's identity and the raw SQL query. + return fleet.HostPoliciesToDevicePolicies(policies), nil } // TriggerMigrateMDMDevice triggers the webhook associated with the MDM diff --git a/orbit/changes/16775-device-policies-device-safe b/orbit/changes/16775-device-policies-device-safe new file mode 100644 index 0000000000..e348e48c78 --- /dev/null +++ b/orbit/changes/16775-device-policies-device-safe @@ -0,0 +1 @@ +* Updated client response type for the `GET /api/latest/fleet/device/{token}/policies` endpoint for consistency with the server response type (`fleet.DevicePolicy`). diff --git a/server/fleet/policies.go b/server/fleet/policies.go index 93d421692c..f273fa4734 100644 --- a/server/fleet/policies.go +++ b/server/fleet/policies.go @@ -554,6 +554,65 @@ type HostPolicy struct { Response string `json:"response" db:"response"` } +// DevicePolicy is a device-safe representation of a policy in the context of +// a host, for device-authenticated ("My device") endpoints. It intentionally +// omits fields that must not be exposed to end users holding only a device +// token, such as the policy author's name and email and the raw SQL query. +type DevicePolicy struct { + // ID is the unique ID of the policy. + ID uint `json:"id"` + // Name is the name of the policy. + Name string `json:"name"` + // Description describes the policy. + Description string `json:"description"` + // Resolution describes how to solve a failing policy. + Resolution *string `json:"resolution,omitempty"` + // Platform is a comma-separated string to indicate the target platforms. + // + // Empty string targets all platforms. + Platform string `json:"platform"` + // Critical marks the policy as high impact. + Critical bool `json:"critical"` + // ConditionalAccessEnabled indicates whether this is a policy used for + // conditional access. + ConditionalAccessEnabled bool `json:"conditional_access_enabled"` + // Response can be one of the following values: + // - "pass": if the policy was executed and passed. + // - "fail": if the policy was executed and did not pass. + // - "": if the policy did not run yet. + Response string `json:"response"` +} + +// ToDevicePolicy returns the device-safe representation of the host policy. +func (p *HostPolicy) ToDevicePolicy() *DevicePolicy { + return &DevicePolicy{ + ID: p.ID, + Name: p.Name, + Description: p.Description, + Resolution: p.Resolution, + Platform: p.Platform, + Critical: p.Critical, + ConditionalAccessEnabled: p.ConditionalAccessEnabled, + Response: p.Response, + } +} + +// HostPoliciesToDevicePolicies converts host policies to their device-safe +// representation for device-authenticated endpoints. +func HostPoliciesToDevicePolicies(policies []*HostPolicy) []*DevicePolicy { + if policies == nil { + return nil + } + devicePolicies := make([]*DevicePolicy, 0, len(policies)) + for _, p := range policies { + if p == nil { + continue + } + devicePolicies = append(devicePolicies, p.ToDevicePolicy()) + } + return devicePolicies +} + // PolicySpec is used to hold policy data to apply policy specs. // // Policies are currently identified by name (unique). diff --git a/server/fleet/service.go b/server/fleet/service.go index 01101435f2..8fc5719f0a 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -461,8 +461,10 @@ type Service interface { // HostLiteByIdentifier returns a host and a subset of its fields from its id. HostLiteByID(ctx context.Context, id uint) (*HostLite, error) - // ListDevicePolicies lists all policies for the given host, including passing / failing summaries - ListDevicePolicies(ctx context.Context, host *Host) ([]*HostPolicy, error) + // ListDevicePolicies lists all policies for the given host in their + // device-safe representation (which excludes the policy author's identity + // and the raw SQL query), including passing / failing responses. + ListDevicePolicies(ctx context.Context, host *Host) ([]*DevicePolicy, error) // BypassConditionalAccess lets a host skip conditional access checks for one check BypassConditionalAccess(ctx context.Context, host *Host) error diff --git a/server/mock/service/service_mock.go b/server/mock/service/service_mock.go index f1756597b5..edc9999619 100644 --- a/server/mock/service/service_mock.go +++ b/server/mock/service/service_mock.go @@ -264,7 +264,7 @@ type HostLiteByIdentifierFunc func(ctx context.Context, identifier string) (*fle type HostLiteByIDFunc func(ctx context.Context, id uint) (*fleet.HostLite, error) -type ListDevicePoliciesFunc func(ctx context.Context, host *fleet.Host) ([]*fleet.HostPolicy, error) +type ListDevicePoliciesFunc func(ctx context.Context, host *fleet.Host) ([]*fleet.DevicePolicy, error) type BypassConditionalAccessFunc func(ctx context.Context, host *fleet.Host) error @@ -3288,7 +3288,7 @@ func (s *Service) HostLiteByID(ctx context.Context, id uint) (*fleet.HostLite, e return s.HostLiteByIDFunc(ctx, id) } -func (s *Service) ListDevicePolicies(ctx context.Context, host *fleet.Host) ([]*fleet.HostPolicy, error) { +func (s *Service) ListDevicePolicies(ctx context.Context, host *fleet.Host) ([]*fleet.DevicePolicy, error) { s.mu.Lock() s.ListDevicePoliciesFuncInvoked = true s.mu.Unlock() diff --git a/server/service/devices.go b/server/service/devices.go index b2c0dfce6a..990564257e 100644 --- a/server/service/devices.go +++ b/server/service/devices.go @@ -107,8 +107,17 @@ func (r *getDeviceHostRequest) deviceAuthToken() string { return r.Token } +// deviceHostDetailResponse wraps the host detail response to shadow the +// host's policies with their device-safe representation, which excludes the +// policy author's identity and the raw SQL query (this is a device-authenticated +// endpoint, so it must not expose admin-only data). +type deviceHostDetailResponse struct { + *fleet.HostDetailResponse + Policies *[]*fleet.DevicePolicy `json:"policies,omitempty"` +} + type getDeviceHostResponse struct { - Host *fleet.HostDetailResponse `json:"host"` + Host *deviceHostDetailResponse `json:"host"` // Deprecated: use OrgLogoURLDarkMode. OrgLogoURL string `json:"org_logo_url"` // Deprecated: use OrgLogoURLLightMode. @@ -237,8 +246,19 @@ func getDeviceHostEndpoint(ctx context.Context, request interface{}, svc fleet.S }, } + deviceHost := &deviceHostDetailResponse{HostDetailResponse: resp} + if resp.Policies != nil { + devicePolicies := fleet.HostPoliciesToDevicePolicies(*resp.Policies) + deviceHost.Policies = &devicePolicies + // defense-in-depth: the shadow field above already wins over the + // embedded policies when marshaling, but clear the admin-facing + // policies anyway so they cannot leak if the wrapped response is ever + // marshaled directly. + resp.Policies = nil + } + return getDeviceHostResponse{ - Host: resp, + Host: deviceHost, OrgLogoURL: ac.OrgInfo.OrgLogoURL, OrgLogoURLLightBackground: ac.OrgInfo.OrgLogoURLLightBackground, OrgLogoURLDarkMode: ac.OrgInfo.OrgLogoURLDarkMode, @@ -463,8 +483,8 @@ func (r *listDevicePoliciesRequest) deviceAuthToken() string { } type listDevicePoliciesResponse struct { - Err error `json:"error,omitempty"` - Policies []*fleet.HostPolicy `json:"policies"` + Err error `json:"error,omitempty"` + Policies []*fleet.DevicePolicy `json:"policies"` } func (r listDevicePoliciesResponse) Error() error { return r.Err } @@ -484,7 +504,7 @@ func listDevicePoliciesEndpoint(ctx context.Context, request interface{}, svc fl return listDevicePoliciesResponse{Policies: data}, nil } -func (svc *Service) ListDevicePolicies(ctx context.Context, host *fleet.Host) ([]*fleet.HostPolicy, error) { +func (svc *Service) ListDevicePolicies(ctx context.Context, host *fleet.Host) ([]*fleet.DevicePolicy, error) { // skipauth: No authorization check needed due to implementation returning // only license error. svc.authz.SkipAuthorization(ctx) diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index b86eba9afc..4103aa530e 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -4439,23 +4439,49 @@ func (s *integrationEnterpriseTestSuite) TestListDevicePolicies() { err = res.Body.Close() require.NoError(t, err) + // asserts that a JSON-decoded policy from a device-authenticated endpoint + // only contains device-safe fields, i.e. it never exposes the policy + // author's identity nor the raw SQL query. + assertDeviceSafePolicy := func(policy map[string]any) { + require.NotContains(t, policy, "query") + require.NotContains(t, policy, "author_id") + require.NotContains(t, policy, "author_name") + require.NotContains(t, policy, "author_email") + require.Contains(t, policy, "name") + require.Contains(t, policy, "response") + } + // GET `/api/_version_/fleet/device/{token}/policies` listDevicePoliciesResp := listDevicePoliciesResponse{} res = s.DoRawNoAuth("GET", "/api/latest/fleet/device/"+token+"/policies", nil, http.StatusOK) - err = json.NewDecoder(res.Body).Decode(&listDevicePoliciesResp) + rawBody, err := io.ReadAll(res.Body) require.NoError(t, err) err = res.Body.Close() require.NoError(t, err) + err = json.Unmarshal(rawBody, &listDevicePoliciesResp) + require.NoError(t, err) require.Len(t, listDevicePoliciesResp.Policies, 2) require.NoError(t, listDevicePoliciesResp.Err) + // the response must not leak the policy author's identity nor the raw SQL query + var rawPoliciesResp struct { + Policies []map[string]any `json:"policies"` + } + err = json.Unmarshal(rawBody, &rawPoliciesResp) + require.NoError(t, err) + require.Len(t, rawPoliciesResp.Policies, 2) + for _, policy := range rawPoliciesResp.Policies { + assertDeviceSafePolicy(policy) + } // GET `/api/_version_/fleet/device/{token}` getDeviceHostResp := getDeviceHostResponse{} res = s.DoRawNoAuth("GET", "/api/latest/fleet/device/"+token, nil, http.StatusOK) - err = json.NewDecoder(res.Body).Decode(&getDeviceHostResp) + rawBody, err = io.ReadAll(res.Body) require.NoError(t, err) err = res.Body.Close() require.NoError(t, err) + err = json.Unmarshal(rawBody, &getDeviceHostResp) + require.NoError(t, err) require.NoError(t, getDeviceHostResp.Err) require.Equal(t, host.ID, getDeviceHostResp.Host.ID) require.False(t, getDeviceHostResp.Host.RefetchRequested) @@ -4463,6 +4489,19 @@ func (s *integrationEnterpriseTestSuite) TestListDevicePolicies() { require.Equal(t, "http://example.com/contact", getDeviceHostResp.OrgContactURL) require.Len(t, *getDeviceHostResp.Host.Policies, 2) require.False(t, getDeviceHostResp.GlobalConfig.Features.EnableSoftwareInventory) + // the host's policies must not leak the policy author's identity nor the + // raw SQL query + var rawHostResp struct { + Host struct { + Policies []map[string]any `json:"policies"` + } `json:"host"` + } + err = json.Unmarshal(rawBody, &rawHostResp) + require.NoError(t, err) + require.Len(t, rawHostResp.Host.Policies, 2) + for _, policy := range rawHostResp.Host.Policies { + assertDeviceSafePolicy(policy) + } // GET `/api/_version_/fleet/device/{token}/desktop` getDesktopResp := fleetDesktopResponse{}