Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Synthetics] Remove Ability to View Synthetics Global Parameters from Kibana Synthetics App #167781

Closed
renzedj opened this issue Oct 2, 2023 · 10 comments · Fixed by #195866
Closed
Assignees
Labels
Team:obs-ux-management Observability Management User Experience Team

Comments

@renzedj
Copy link

renzedj commented Oct 2, 2023

Use Case

As an Elastic Cloud admin, I want my users who create Synthetics tests to be able to securely use parameters for any secrets in their tests, however at this point they cannot, because any user with ALL access to the Synthetics app in Kibana can retrieve them from the Global Parameters vault.

Details

Currently, any user with ALL access to the Synthetics app in Kibana can go into Synthetics->Settings->Global Parameters, click on the "show value" option in the line containing a parameter, and retrieve the parameter from Kibana. This creates a security risk, as these are secrets such as passwords, keys, or other items which should not be generally available. To prevent this, I need to remove ALL access, which also removes the ability for users to set their own alerts.

Recommendation

As noted in this post, the decision to allow users with write access to view parameter values in the UI was a conscious one. However, this creates a security risk, and in most cases this there is no requirement to be able to retrieve a parameter from within Kibana. Even as an Elastic Admin, I cannot see a requirement to be able to retrieve a secret from Kibana, as according to best practice any secrets should also be vaulted.

The easiest way to resolve this would be to do the following:

  1. Remove the "show value" option from the line items in Synthetics->Settings->Global Parameters.
  2. Mask the value in both the "Create Parameter" and "Edit Parameter" screens.
  3. Update documentation to indicate that parameters are one-way and cannot be retrieved from within the Kibana UI.

This would effectively make the parameters "one-way" (i.e., a user can create/update a parameter, but not view it after from within Kibana).

@botelastic botelastic bot added the needs-team Issues missing a team label label Oct 2, 2023
@renzedj renzedj changed the title [Synthetics] [Synthetics] Remove Ability to View Synthetics Global Parameters from Kibana Synthetics App Oct 2, 2023
@jughosta jughosta added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Oct 4, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Oct 4, 2023
@paulb-elastic
Copy link
Contributor

@renzedj notwithstanding your point about being able to show it in the Kibana UI (with the correct permissions), it's also worth highlighting it will never truly be "one way", as described in the docs, it would still be possible for a script to be written to make use of those parameters (e.g. even just typing them into the Google search bar and getting a screenshot of the value).

@renzedj
Copy link
Author

renzedj commented Oct 5, 2023

@paulb-elastic -

@renzedj notwithstanding your point about being able to show it in the Kibana UI (with the correct permissions), it's also worth highlighting it will never truly be "one way", as described in the docs, it would still be possible for a script to be written to make use of those parameters (e.g. even just typing them into the Google search bar and getting a screenshot of the value).

You are absolutely correct. There is absolutely nothing magical about this to prevent a bad actor from retrieving this anyway. Ultimately, it's my responsibility as the admin to have the proper controls in place to prevent this.

To use an analogy, Elastic gives me as the admin the tools I need to restrict access to our Elastic Cloud instance, including the ability to secure the installation using various methods of user authentication. However, there's nothing that would prevent me from creating a universal username/password combination of guest/guest and handing that out. It's my responsibility as the admin to use sensible security, such as tying it in to my company's SSO. Even then, my company needs to have additional controls in place to ensure that fake users aren't slipped into our SSO and that users who have exited the company have their access removed in a timely fashion.

In this situation, this is providing a tool to help admins like me to protect sensitive data (e.g., secrets). We do still need to have other controls in place, such as not allowing tests to be created from within the UI and requiring that they be written offline using the @elastic/synthetics node.js tools so that they can be in version control, reviewed by an admin, tested, and deployed via a CI/CD pipeline, just like any other application in the environment.

@shahzad31
Copy link
Contributor

@renzedj do you think restricting access to the show button would be probably better?

I guess it can be something like only show with an admin user.

Another is we also allow editing of the param, so wouldn't that be same as showing? I mean if user has access to edit. They can also View it?

I am also thinking it can be maybe perhaps bit more refined access control we recently implemented a flag in uptime/synthetics privileges where now if you want you can disable access to elastic managed locations for a user.
Perhaps we can refine privileges access more for creating alerts, creating locations and Parana.

@renzedj
Copy link
Author

renzedj commented Oct 5, 2023

@shahzad31 -

@renzedj do you think restricting access to the show button would be probably better?

I guess it can be something like only show with an admin user.

Yes, that would be fine, as long as it is also masked on the create/edit screens. However, I'm not certain why there's a need to retrieve it through the Kibana UI. As an admin, if I can give a user the capability to create the parameter without my involvement so that even I don't know what the value is, then that would be a best-possible outcome. But then again, I'm not your only customer.

Another is we also allow editing of the param, so wouldn't that be same as showing? I mean if user has access to edit. They can also View it?

No. Not if the parameter value is masked when it's created/edited, as described in my original post.

I am also thinking it can be maybe perhaps bit more refined access control we recently implemented a flag in uptime/synthetics privileges where now if you want you can disable access to elastic managed locations for a user.
Perhaps we can refine privileges access more for creating alerts, creating locations and Parana.

This makes me very happy to hear, as I also have another feature request #167789 open, which requests exactly this for alerting. Has this feature already been released, and can you point me to the documentation? I was looking through the permissions settings a few days ago on 8.10.2 and didn't see this permission available.

@shahzad31
Copy link
Contributor

It's a recent PR

#164863

It will ship with 8.11.0

@renzedj
Copy link
Author

renzedj commented Oct 5, 2023

Thx. Frankly, giving my users read + alert access to Synthetics and reserving the privileges to view parameters to all access (as per current practice) would likely accomplish the same thing I'm trying to accomplish with this request.

@renzedj
Copy link
Author

renzedj commented Sep 10, 2024

bump

@shahzad31 shahzad31 self-assigned this Oct 17, 2024
@shahzad31
Copy link
Contributor

@renzedj we have picked this up and is being fixed as part of #195866

@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@shahzad31 shahzad31 removed the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Oct 17, 2024
shahzad31 added a commit that referenced this issue Oct 25, 2024
…5866)

## Summary

Fixes #167781

In docs we says that only key/value pairs are required, but in actual
edit, that means rest of the data was being lost on edits

Allow partial updates to params edit API !!

This PR makes sure prev objects is fetched and merged with new data
hence allowing partial updates !!

We are also allowing the ability to view value of the secret once it's
saved via API !!

### Value is hidden
Param value will not be visible unless user is `super_user` or
`kibana_admin`, though user can assign new value.

---------

Co-authored-by: Justin Kambic <[email protected]>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Oct 25, 2024
…stic#195866)

## Summary

Fixes elastic#167781

In docs we says that only key/value pairs are required, but in actual
edit, that means rest of the data was being lost on edits

Allow partial updates to params edit API !!

This PR makes sure prev objects is fetched and merged with new data
hence allowing partial updates !!

We are also allowing the ability to view value of the secret once it's
saved via API !!

### Value is hidden
Param value will not be visible unless user is `super_user` or
`kibana_admin`, though user can assign new value.

---------

Co-authored-by: Justin Kambic <[email protected]>
(cherry picked from commit 0ff9a8a)
shahzad31 added a commit to shahzad31/kibana that referenced this issue Oct 30, 2024
…stic#195866)

## Summary

Fixes elastic#167781

In docs we says that only key/value pairs are required, but in actual
edit, that means rest of the data was being lost on edits

Allow partial updates to params edit API !!

This PR makes sure prev objects is fetched and merged with new data
hence allowing partial updates !!

We are also allowing the ability to view value of the secret once it's
saved via API !!

### Value is hidden
Param value will not be visible unless user is `super_user` or
`kibana_admin`, though user can assign new value.

---------

Co-authored-by: Justin Kambic <[email protected]>
(cherry picked from commit 0ff9a8a)
shahzad31 added a commit that referenced this issue Nov 1, 2024
…ng (#195866) (#198284)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Synthetics] Fixes partial updates for params and params viewing
(#195866)](#195866)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-25T15:06:52Z","message":"[Synthetics]
Fixes partial updates for params and params viewing (#195866)\n\n##
Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/167781\r\n\r\nIn docs we says
that only key/value pairs are required, but in actual\r\nedit, that
means rest of the data was being lost on edits\r\n\r\nAllow partial
updates to params edit API !!\r\n\r\nThis PR makes sure prev objects is
fetched and merged with new data\r\nhence allowing partial updates
!!\r\n\r\nWe are also allowing the ability to view value of the secret
once it's\r\nsaved via API !!\r\n\r\n### Value is hidden\r\nParam value
will not be visible unless user is `super_user` or\r\n`kibana_admin`,
though user can assign new
value.\r\n\r\n---------\r\n\r\nCo-authored-by: Justin Kambic
<[email protected]>","sha":"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-management","v8.16.0","v8.17.0"],"number":195866,"url":"https://github.com/elastic/kibana/pull/195866","mergeCommit":{"message":"[Synthetics]
Fixes partial updates for params and params viewing (#195866)\n\n##
Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/167781\r\n\r\nIn docs we says
that only key/value pairs are required, but in actual\r\nedit, that
means rest of the data was being lost on edits\r\n\r\nAllow partial
updates to params edit API !!\r\n\r\nThis PR makes sure prev objects is
fetched and merged with new data\r\nhence allowing partial updates
!!\r\n\r\nWe are also allowing the ability to view value of the secret
once it's\r\nsaved via API !!\r\n\r\n### Value is hidden\r\nParam value
will not be visible unless user is `super_user` or\r\n`kibana_admin`,
though user can assign new
value.\r\n\r\n---------\r\n\r\nCo-authored-by: Justin Kambic
<[email protected]>","sha":"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385"}},"sourceBranch":"main","suggestedTargetBranches":["8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195866","number":195866,"mergeCommit":{"message":"[Synthetics]
Fixes partial updates for params and params viewing (#195866)\n\n##
Summary\r\n\r\nFixes
https://github.com/elastic/kibana/issues/167781\r\n\r\nIn docs we says
that only key/value pairs are required, but in actual\r\nedit, that
means rest of the data was being lost on edits\r\n\r\nAllow partial
updates to params edit API !!\r\n\r\nThis PR makes sure prev objects is
fetched and merged with new data\r\nhence allowing partial updates
!!\r\n\r\nWe are also allowing the ability to view value of the secret
once it's\r\nsaved via API !!\r\n\r\n### Value is hidden\r\nParam value
will not be visible unless user is `super_user` or\r\n`kibana_admin`,
though user can assign new
value.\r\n\r\n---------\r\n\r\nCo-authored-by: Justin Kambic
<[email protected]>","sha":"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385"}},{"branch":"8.16","label":"v8.16.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","labelRegex":"^v8.17.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/197850","number":197850,"state":"MERGED","mergeCommit":{"sha":"4c22558e576bd78deb97474b4a514f6eca9e598e","message":"[8.x]
[Synthetics] Fixes partial updates for params and params viewing
(#195866) (#197850)\n\n# Backport\n\nThis will backport the following
commits from `main` to `8.x`:\n- [[Synthetics] Fixes partial updates for
params and params
viewing\n(#195866)](https://github.com/elastic/kibana/pull/195866)\n\n<!---
Backport version: 9.4.3 -->\n\n### Questions ?\nPlease refer to the
[Backport
tool\ndocumentation](https://github.com/sqren/backport)\n\n<!--BACKPORT\n[{\"author\":{\"name\":\"Shahzad\",\"email\":\"[email protected]\"},\"sourceCommit\":{\"committedDate\":\"2024-10-25T15:06:52Z\",\"message\":\"[Synthetics]\nFixes
partial updates for params and params viewing
(#195866)\\n\\n##\nSummary\\r\\n\\r\\nFixes\nhttps://github.com//issues/167781\\r\\n\\r\\nIn
docs we says\nthat only key/value pairs are required, but in
actual\\r\\nedit, that\nmeans rest of the data was being lost on
edits\\r\\n\\r\\nAllow partial\nupdates to params edit API
!!\\r\\n\\r\\nThis PR makes sure prev objects is\nfetched and merged
with new data\\r\\nhence allowing partial updates\n!!\\r\\n\\r\\nWe are
also allowing the ability to view value of the secret\nonce
it's\\r\\nsaved via API !!\\r\\n\\r\\n### Value is hidden\\r\\nParam
value\nwill not be visible unless user is `super_user`
or\\r\\n`kibana_admin`,\nthough user can assign
new\nvalue.\\r\\n\\r\\n---------\\r\\n\\r\\nCo-authored-by: Justin
Kambic\n<[email protected]>\",\"sha\":\"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385\",\"branchLabelMapping\":{\"^v9.0.0$\":\"main\",\"^v8.17.0$\":\"8.x\",\"^v(\\\\d+).(\\\\d+).\\\\d+$\":\"$1.$2\"}},\"sourcePullRequest\":{\"labels\":[\"release_note:skip\",\"v9.0.0\",\"backport:prev-minor\",\"ci:project-deploy-observability\",\"Team:obs-ux-management\"],\"title\":\"[Synthetics]\nFixes
partial updates for params and
params\nviewing\",\"number\":195866,\"url\":\"https://github.com/elastic/kibana/pull/195866\",\"mergeCommit\":{\"message\":\"[Synthetics]\nFixes
partial updates for params and params viewing
(#195866)\\n\\n##\nSummary\\r\\n\\r\\nFixes\nhttps://github.com//issues/167781\\r\\n\\r\\nIn
docs we says\nthat only key/value pairs are required, but in
actual\\r\\nedit, that\nmeans rest of the data was being lost on
edits\\r\\n\\r\\nAllow partial\nupdates to params edit API
!!\\r\\n\\r\\nThis PR makes sure prev objects is\nfetched and merged
with new data\\r\\nhence allowing partial updates\n!!\\r\\n\\r\\nWe are
also allowing the ability to view value of the secret\nonce
it's\\r\\nsaved via API !!\\r\\n\\r\\n### Value is hidden\\r\\nParam
value\nwill not be visible unless user is `super_user`
or\\r\\n`kibana_admin`,\nthough user can assign
new\nvalue.\\r\\n\\r\\n---------\\r\\n\\r\\nCo-authored-by: Justin
Kambic\n<[email protected]>\",\"sha\":\"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385\"}},\"sourceBranch\":\"main\",\"suggestedTargetBranches\":[],\"targetPullRequestStates\":[{\"branch\":\"main\",\"label\":\"v9.0.0\",\"branchLabelMappingKey\":\"^v9.0.0$\",\"isSourceBranch\":true,\"state\":\"MERGED\",\"url\":\"https://github.com/elastic/kibana/pull/195866\",\"number\":195866,\"mergeCommit\":{\"message\":\"[Synthetics]\nFixes
partial updates for params and params viewing
(#195866)\\n\\n##\nSummary\\r\\n\\r\\nFixes\nhttps://github.com//issues/167781\\r\\n\\r\\nIn
docs we says\nthat only key/value pairs are required, but in
actual\\r\\nedit, that\nmeans rest of the data was being lost on
edits\\r\\n\\r\\nAllow partial\nupdates to params edit API
!!\\r\\n\\r\\nThis PR makes sure prev objects is\nfetched and merged
with new data\\r\\nhence allowing partial updates\n!!\\r\\n\\r\\nWe are
also allowing the ability to view value of the secret\nonce
it's\\r\\nsaved via API !!\\r\\n\\r\\n### Value is hidden\\r\\nParam
value\nwill not be visible unless user is `super_user`
or\\r\\n`kibana_admin`,\nthough user can assign
new\nvalue.\\r\\n\\r\\n---------\\r\\n\\r\\nCo-authored-by: Justin
Kambic\n<[email protected]>\",\"sha\":\"0ff9a8a9d9ff2bdb99562eeca29152bd0a0c4385\"}}]}]\nBACKPORT-->\n\nCo-authored-by:
Shahzad <[email protected]>"}}]}] BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:obs-ux-management Observability Management User Experience Team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants