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

BR: --crypter.key & --full-backup-storage are not redacted from the log #57585

Closed
kennytm opened this issue Nov 21, 2024 · 1 comment · Fixed by #57593
Closed

BR: --crypter.key & --full-backup-storage are not redacted from the log #57585

kennytm opened this issue Nov 21, 2024 · 1 comment · Fixed by #57593
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.5 This bug affects the 8.5.x(LTS) versions. component/br This issue is related to BR of TiDB. severity/major type/bug The issue is confirmed as a bug.

Comments

@kennytm
Copy link
Contributor

kennytm commented Nov 21, 2024

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

br restore point -s 's3://foo/bar/?access-key=SecretValue&secret-access-key=SuperSecretValue' \
    --full-backup-storage 's3://foo/bar2/?access-key=SecretValue&secret-access-key=SuperSecretValue' \
    --s3.endpoint http://127.0.0.1:9999 \
    --crypter.method aes128-ctr \
    --crypter.key 537570657253656372657456616C7565

then read the first few lines of the BR log

2. What did you expect to see? (Required)

The secret keys "SecretValue", "SuperSecretValue" and "537570657253656372657456616C7565" do not appear in the log.

3. What did you see instead (Required)

They are written in plaintext.

[2024/11/21 16:03:36.806 +08:00] [INFO] [common.go:765] [arguments] [__command="br restore point"] [crypter.key=537570657253656372657456616C7565] [crypter.method=aes128-ctr] [full-backup-storage="s3://foo/bar2/?access-key=SecretValue&secret-access-key=SuperSecretValue"] [s3.endpoint=http://127.0.0.1:9999] [storage=s3://foo/bar/]

4. What is your TiDB version? (Required)

BR v8.4.0, v8.2.0, v7.5.4, v7.1.5 (probably every version)

@kennytm kennytm added type/bug The issue is confirmed as a bug. component/br This issue is related to BR of TiDB. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. labels Nov 21, 2024
@kennytm
Copy link
Contributor Author

kennytm commented Nov 21, 2024

Note that the --storage field is correctly redacted. This is handled by

tidb/br/pkg/task/common.go

Lines 897 to 908 in c091dba

func flagToZapField(f *pflag.Flag) zap.Field {
if f.Name == flagStorage {
hiddenQuery, err := url.Parse(f.Value.String())
if err != nil {
return zap.String(f.Name, "<invalid URI>")
}
// hide all query here.
hiddenQuery.RawQuery = ""
return zap.Stringer(f.Name, hiddenQuery)
}
return zap.Stringer(f.Name, f.Value)
}

We should apply it for --crypter.key and --full-backup-storage too.

@kennytm kennytm added affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. labels Nov 21, 2024
@kennytm kennytm added the affects-8.5 This bug affects the 8.5.x(LTS) versions. label Nov 21, 2024
@ti-chi-bot ti-chi-bot bot added the may-affects-5.4 This bug maybe affects 5.4.x versions. label Nov 21, 2024
@BornChanger BornChanger removed the may-affects-5.4 This bug maybe affects 5.4.x versions. label Nov 21, 2024
@ti-chi-bot ti-chi-bot bot closed this as completed in fe1b9ed Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. affects-8.5 This bug affects the 8.5.x(LTS) versions. component/br This issue is related to BR of TiDB. severity/major type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants