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

Fixes to remove traced password #776

Merged
merged 1 commit into from
Oct 13, 2018

Conversation

wallyqs
Copy link
Member

@wallyqs wallyqs commented Oct 12, 2018

Alternate approach to fix #762 and #766 continuing to use regex based approach to filter password entries from the log instead of using JSON.

Signed-off-by: Waldemar Quevedo wally@synadia.com

  • Resolves Password obfuscation in trace does not work properly #762
  • Documentation added (if applicable)
  • Tests added
  • Branch rebased on top of current master (git pull --rebase origin master)
  • Changes squashed to a single commit (described here)
  • Build is green in Travis CI
  • You have certified that the contribution is your original work and that you license the work to the project under the Apache 2 license

Resolves #762

/cc @nats-io/core

Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

Minor comment.

server/client.go Outdated
return arg
}
m := passPat.FindAllSubmatch(arg, -1)
// Take a copy of the connect proto just for the trace message.
a := make([]byte, len(arg))
Copy link
Member

Choose a reason for hiding this comment

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

Maybe place on the stack since it probably does not escape? Compiler may do this for you these days.

I used to do

var _raw [4096]byte
buf := _raw[:0]
buf = append(buf, ...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, updated to take this approach.

@wallyqs wallyqs force-pushed the connect-pass-regex-fixes branch from fbe9710 to ee367ec Compare October 12, 2018 23:45
server/client.go Outdated Show resolved Hide resolved
Signed-off-by: Waldemar Quevedo <wally@synadia.com>
@wallyqs wallyqs force-pushed the connect-pass-regex-fixes branch from ee367ec to a0fe8fd Compare October 13, 2018 00:00
@derekcollison
Copy link
Member

LGTM

@coveralls
Copy link

coveralls commented Oct 13, 2018

Coverage Status

Coverage decreased (-0.1%) to 92.181% when pulling fbe9710 on wallyqs:connect-pass-regex-fixes into 71eb6d8 on nats-io:master.

@wallyqs wallyqs merged commit 3b0b139 into nats-io:master Oct 13, 2018
@wallyqs wallyqs deleted the connect-pass-regex-fixes branch October 13, 2018 01:28
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.

Password obfuscation in trace does not work properly
3 participants