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

import,backup: show redacted secrets in job UI #44737

Merged
merged 2 commits into from
Feb 5, 2020
Merged

Conversation

dt
Copy link
Member

@dt dt commented Feb 5, 2020

Previously the IMPORT and BACKUP jobs would hide all query parameters.
This gave some users / engineers the false sense that those secrets were
gone or irrecoverable from the job record, but any super-user with read
access to the system tables can extract the raw URLs, including the params
that include things like AWS_SECRET_ACCESS_KEY, from the raw job data.

This change replaces the blanket hiding of all params with targetted
redacting of just the values for sensitive params, replacing their value
with the word 'redacted'. This is hoped to make is clearer what is going
on -- that the params are still there and we're just not showing the value.

Release note (general change): Jobs show the parameters used for connecting to external storage in SHOW JOBS and the Jobs UI page, with only the values of parameters classified as secrets redacted.

While I'm here: remove duplicate filename prefix in some IMPORT error messages (see first commit for details).

Previously the IMPORT code prefixed individual errors in makeRowErr with the filename,
but then also wrapped any error encountered while processing a given file with the
filename as well, resulting in the same filename prefixed to the error twice in some
cases.

Release note: none.
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@miretskiy miretskiy left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @aaron-crl and @miretskiy)

Copy link

@aaron-crl aaron-crl left a comment

Choose a reason for hiding this comment

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

LGTM

@dt
Copy link
Member Author

dt commented Feb 5, 2020

Added changefeeds, which also include some secret params in their URIs that are not cloud URIs (and this the cloud package doesn't know about them) so added an extra-params-to-scrub argument to the helper as well. RFAL (and cc'ed @ajwerner for the changefeed side)

Copy link
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

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

CDC stuff :lgtm:

Reviewed 3 of 9 files at r2.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @aaron-crl, @ajwerner, and @miretskiy)

Previously the IMPORT and BACKUP jobs would hide all query parameters.
This gave some users / engineers the false sense that those secrets were
gone or irrecoverable from the job record, but any super-user with read
access to the system tables can extract the raw URLs, including the params
that include things like AWS_SECRET_ACCESS_KEY, from the raw job data.

This change replaces the blanket hiding of all params with targetted
redacting of just the values for sensitive params, replacing their value
with the word 'redacted'. This is hoped to make is clearer what is going
on -- that the params are still there and we're just not showing the value.

Release note (general change): Jobs show the parameters used for connecting to external storage in SHOW JOBS and the Jobs UI page, with only the values of parameters classified as secrets redacted.
@dt
Copy link
Member Author

dt commented Feb 5, 2020

bors r+

craig bot pushed a commit that referenced this pull request Feb 5, 2020
44737: import,backup: show redacted secrets in job UI r=dt a=dt

Previously the IMPORT and BACKUP jobs would hide all query parameters.
This gave some users / engineers the false sense that those secrets were
gone or irrecoverable from the job record, but any super-user with read
access to the system tables can extract the raw URLs, including the params
that include things like AWS_SECRET_ACCESS_KEY, from the raw job data.

This change replaces the blanket hiding of all params with targetted
redacting of just the values for sensitive params, replacing their value
with the word 'redacted'. This is hoped to make is clearer what is going
on -- that the params are still there and we're just not showing the value.

Release note (general change): Jobs show the parameters used for connecting to external storage in SHOW JOBS and the Jobs UI page, with only the values of parameters classified as secrets redacted.

While I'm here: remove duplicate filename prefix in some IMPORT error messages (see first commit for details).


Co-authored-by: David Taylor <tinystatemachine@gmail.com>
@craig
Copy link
Contributor

craig bot commented Feb 5, 2020

Build succeeded

@craig craig bot merged commit 08e68ec into cockroachdb:master Feb 5, 2020
@dt dt deleted the job-keys branch February 5, 2020 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants