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

rpk: redact secret values when importing from file #8339

Merged
merged 1 commit into from Jan 24, 2023

Conversation

r-vasquez
Copy link
Contributor

@r-vasquez r-vasquez commented Jan 20, 2023

Now if a value is a secret we will use [redacted] instead of printing the raw value.

# Running import when the file have a secret field:
$ rpk cluster config import -f /tmp/config.yaml    
PROPERTY                  PRIOR     NEW
cloud_storage_secret_key  [secret]  [redacted]

Successfully updated configuration. New configuration version is 54.

Cluster needs to be restarted. See more details with 'rpk cluster config status'.

# No edits in rpk cluster config edit:
$ rpk cluster config edit
No changes were made.

# Edit in rpk cluster config edit
$ rpk cluster config edit
PROPERTY                  PRIOR     NEW
cloud_storage_secret_key  [secret]  [redacted]

Successfully updated configuration. New configuration version is 54.

Cluster needs to be restarted. See more details with 'rpk cluster config status'.

Backports Required

  • v22.3.x
  • v22.2.x
  • v22.1.x

Release Notes

Bug Fixes

  • [CVE-2023-24619] rpk won't print the values of the cluster properties [cloud_storage_secret_key, cloud_storage_azure_shared_key] when they are being imported from a file when using rpk cluster config import/edit

Note: already saved properties are not shown.

RafalKorepta
RafalKorepta previously approved these changes Jan 20, 2023
Copy link
Contributor

@RafalKorepta RafalKorepta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM:

Could we have some unit tests for the output?
Should this be backported?
I found v22.1.x, v22.2.x and v22.3.x are affected

@r-vasquez
Copy link
Contributor Author

Force Push:

  • Added ducktape tests
  • Added a validation for nil values, so we don't end up changing <nil> to [secret] when there is no need and can lead to a confusion.

RafalKorepta
RafalKorepta previously approved these changes Jan 23, 2023
Copy link
Contributor

@twmb twmb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment - I think this is bugged so I'm putting a block on it for now to prevent any accidental merge

Now if a value is a secret we will use [redacted]
instead of printing the raw value.
@r-vasquez
Copy link
Contributor Author

/backport v22.3.x

@r-vasquez
Copy link
Contributor Author

/backport v22.2.x

@vbotbuildovich
Copy link
Collaborator

Failed to run cherry-pick command. I executed the below command:

git cherry-pick -x f3a40542107e1c0710f91232a29d76137a762b6e

Workflow run logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants