Project

General

Profile

Actions

Bug #44237

closed

Feature #47765: mgr/dashboard: security improvements

mgr/dashboard: security: some system roles allow accessing sensitive information

Added by Ernesto Puerta about 4 years ago. Updated about 3 years ago.

Status:
Resolved
Priority:
High
Category:
Component - Users & Roles
Target version:
% Done:

100%

Source:
Q/A
Tags:
Backport:
nautilus
Regression:
No
Severity:
2 - major
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

Some system roles (pool-manager, cephfs-manager, ganesha-manager etc) have the configOpt read permissions enabled, which allows to read all cluster config options and manager module config options. The latter includes RGW keys or Grafana user/admin, plus any sensitive information used by existing or new modules. As dashboard cannot control what new information is exposed by these modules, the suggestion is to remove that read permission from all system roles except the specific management ones (adminstrator and cluster-manager).

The reason why configOpts was added to those roles is that at some point they require access to some cluster configuration settings:
  • pool-manager: checks /api/cluster_conf/osd_pool_default_pg_autoscale_mode. This parameter could/should also be exposed via /api/pools/_info, which already returns other ceph config params (e.g.: bluestore_compression_algorithm).
  • ganesha-manager, cephfs-manager, rgw-manager: I couldn't find any direct dependency with cluster config options.

different case is the read-only role. While it initially makes sense to allow configOpt read permission, dashboard administrator might guess that read-only perfectly fits for a guest/low-privileged user. On the contrary, a read-only user has access to the same sensitive data as mentioned above.

Suggested next steps:
  • Discuss and agree on read-only user with/without access to configOpts. This could improve by splitting it into 2: administrator-read-only and guest (without the read permission on sensitive data). As I'm against adding more roles, I'd simply leave the low-privilege guest one.
  • Make pool-form get osd_pool_default_pg_autoscale_mode from /pool/_info.
  • Remove configOpt read perm (and test) in all other roles.

Related issues 2 (0 open2 closed)

Related to Dashboard - Feature #24672: mgr/dashboard: Prevent user from accessing unallowed pagesClosed

Actions
Copied to Dashboard - Backport #44435: nautilus: mgr/dashboard: security: some system roles allow accessing sensitive informationResolvedAlfonso MartínezActions
Actions #1

Updated by Ernesto Puerta about 4 years ago

  • Description updated (diff)
Actions #2

Updated by Ernesto Puerta about 4 years ago

  • Description updated (diff)
Actions #3

Updated by Volker Theile about 4 years ago

I had a similar problem implementing https://github.com/ceph/ceph/pull/32680. There was a need to get some settings exposed to the UI but for users without configOpts privileges. I enhanced the already existing /ui-api/standard_settings endpoint (see https://github.com/ceph/ceph/pull/32680/files#diff-c3a875d99d1cd079a9e1f9b86dbe8195R92), but this would not fit in the future when there is the need to expose more settings in such a case. Currently i do not have any idea how to improve the privilege model, but i have the strong feeling that it requires some refactoring.

Actions #4

Updated by Lenz Grimmer about 4 years ago

Good observation; thanks for bringing this up!

About your first suggestion: I think the term guest is too generic and may leave users confused about what this could be used for.

The original intention of the readonly role was being able to have these user to log into the dashboard and to view that cluster's health state and current configuration (e.g. Pools and services), which includes it's config settings, but does not allow making any changes to any object or entity of the cluster. But there's indeed a potential risk that this gives these users a way to obtain credentials that would allow them to elevate their privileges for certain services outside of using the dashboard.

Renaming a role during an upgrade may be more complicated than simply updating it's privileges. I'm fine with revoking access to configOpts in general for the readonly role, otherwise we'd end up with maintaining black/whitelists of which config options should be accessible (but maybe such a black/whitelist filter for config options would be desirable?)

I also agree to your other two suggested next steps:

  • Make pool-form get osd_pool_default_pg_autoscale_mode from /pool/_info.
  • Remove configOpt read perm (and test) in all other roles.
Actions #5

Updated by Lenz Grimmer about 4 years ago

  • Target version set to v15.0.0
Actions #6

Updated by Lenz Grimmer about 4 years ago

  • Related to Feature #24672: mgr/dashboard: Prevent user from accessing unallowed pages added
Actions #7

Updated by Lenz Grimmer about 4 years ago

  • Translation missing: en.field_tag_list set to administration, security
Actions #8

Updated by Tatjana Dehler about 4 years ago

Lenz Grimmer wrote:

..., otherwise we'd end up with maintaining black/whitelists of which config options should be accessible (but maybe such a black/whitelist filter for config options would be desirable?)

I'm not sure if there is a way around it. As Volker already wrote, we do have some config options that are required by any user to be able to log into the dashboard (for example the password expiration date). We currently have a "workaround" by using the /ui-api/standard_settings API endpoint. I fully agree with Volker that's not the right way going forward.

Actions #9

Updated by Ernesto Puerta about 4 years ago

Thanks @Lenz Grimmer, @Volker and @Tatjana!

Summarizing the proposals here:
  • Embedding all required config setting info in ui-api/ helper endpoints:
    • Pros: existing approach (proven), easier to implement (already there) and keeps complexity/hides Ceph specifics in the back-end.
    • Cons: these endpoints are usually pre-fetched on component loading and never refreshed. It makes harder to react to live changes.
  • Providing fine-grained permissions at config setting-level:
    • Pros: more extensible, and forms might react dynamically to changes.
    • Cons: requires changes to config controller, it might be harder to extend/maintain and track security issues, and brings complexity to front-end (routing single front-end component to multiple back-end endpoints).

I personally prefer the latter approach, as it'd be really easy to implement if all Ceph config options would have their corresponding service tags, as we could reuse that info for automatically tagging scopes per cluster config option:

//src/common/options.h
struct Option { 
...
  // Items like mon, osd, rgw, rbd, ceph-fuse.  This is advisory metadata                               
  // for presentation layers (like web dashboards, or generated docs), so that                          
  // they know which options to display where.                                                          
  // Additionally: "common" for settings that exist in any Ceph code.  Do                                                                                                                                                                                                                   
  // not use common for settings that are just shared some places: for those                            
  // places, list them.                                                                                 
  std::vector<const char*> services; 

However, for this specific case, we may see that there's no match between our pool-manager role and any Ceph service (rbd, rgw, ...).

    Option("osd_pool_default_pg_autoscale_mode", Option::TYPE_STR, Option::LEVEL_ADVANCED)                                                                                                                                                                                                  
    .set_default("on")                                                                               
    .set_flag(Option::FLAG_RUNTIME)                                                                  
    .set_enum_allowed({"off", "warn", "on"})                                                         
    .set_description("Default PG autoscaling behavior for new pools"),
+   .add_service("pool")  // weird. Instead: rbd + cephfs + rgw

IMHO this is an indication that the pool scope and pool-manager role are not very meaningful per se. Instead we should probably remove pool-manager and pool scope and only allow <service>-manager to create/edit/delete pools with application tag <service> (let <service> be rgw, cephfs or rbd).

Actions #10

Updated by Alfonso Martínez about 4 years ago

  • Status changed from New to In Progress
  • Assignee set to Alfonso Martínez
Actions #11

Updated by Alfonso Martínez about 4 years ago

  • Status changed from In Progress to Fix Under Review
  • Pull request ID set to 33690
Actions #12

Updated by Kefu Chai about 4 years ago

  • Status changed from Fix Under Review to Pending Backport
Actions #13

Updated by Alfonso Martínez about 4 years ago

  • Copied to Backport #44435: nautilus: mgr/dashboard: security: some system roles allow accessing sensitive information added
Actions #14

Updated by Nathan Cutler about 4 years ago

  • Status changed from Pending Backport to Resolved

While running with --resolve-parent, the script "backport-create-issue" noticed that all backports of this issue are in status "Resolved" or "Rejected".

Actions #15

Updated by Ernesto Puerta over 3 years ago

  • Parent task set to #47765
Actions #16

Updated by Alfonso Martínez over 3 years ago

  • % Done changed from 0 to 100
Actions #17

Updated by Ernesto Puerta about 3 years ago

  • Project changed from mgr to Dashboard
  • Category changed from 150 to Component - Users & Roles
Actions

Also available in: Atom PDF