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

ZOOKEEPER-3989 GenerateLoad needs to use log for protecting sensitive… #1530

Conversation

mino181295
Copy link

Sensitive data about args[0], zkHostPort, and cmdNumber are directly printed and may leak.
For security, log should be used to record these data, as well as log in other classes such as org.apache.zookeeper.server.ZooKeeperServer:
LOG = LoggerFactory.getLogger(GenerateLoad.class);

LOG.error("Could not connect to " + args[0]);
......
LOG.info("Connecting exclusively to " + zkHostPort.toString());
......
LOG.error("Not a valid number: " + e.getMessage());

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines 716 to 714
System.err.println("USAGE: " + GenerateLoad.class.getName()
LOG.info("USAGE: " + GenerateLoad.class.getName()
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say that the usage synopsis should still go to System.err. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not have a strong opinion here
@mino181295 what's your opinion ?

Copy link
Author

Choose a reason for hiding this comment

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

I agree with @ztzg do you think it should be System.err.println also line 673-676?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say so. (Sorry for missing it on the first round.)

Copy link
Contributor

Choose a reason for hiding this comment

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

ah dang, i hit approve too quickly. i agree with this requested change as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh and i see that it has been addressed :)

Copy link
Contributor

@breed breed left a comment

Choose a reason for hiding this comment

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

this looks great. there is one requested change on USAGE, and then i think it's ready to go.

Copy link
Contributor

@breed breed left a comment

Choose a reason for hiding this comment

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

apologies everyone! i see the USAGE request has been addressed. i agree that it is ready to go in. +1

@ztzg ztzg closed this in ea3042d Nov 23, 2020
@ztzg
Copy link
Contributor

ztzg commented Nov 23, 2020

Thank you, @mino181295. This is now in master. (I see that the ticket references 3.4; would you also like this to go to older branches?)

RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
Sensitive data about args[0], zkHostPort, and cmdNumber are directly printed and may leak.
For security, log should be used to record these data, as well as log in other classes such as org.apache.zookeeper.server.ZooKeeperServer:
LOG = LoggerFactory.getLogger(GenerateLoad.class);
```
LOG.error("Could not connect to " + args[0]);
......
LOG.info("Connecting exclusively to " + zkHostPort.toString());
......
LOG.error("Not a valid number: " + e.getMessage());
```

Author: Matteo Minardi <matteo.minardi@diennea.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Benjamin Reed <breed@apache.org>, Damien Diederen <ddiederen@apache.org>

Closes apache#1530 from mino181295/fix/ZOOKEEPER-3989/generate-load-uselog
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
Sensitive data about args[0], zkHostPort, and cmdNumber are directly printed and may leak.
For security, log should be used to record these data, as well as log in other classes such as org.apache.zookeeper.server.ZooKeeperServer:
LOG = LoggerFactory.getLogger(GenerateLoad.class);
```
LOG.error("Could not connect to " + args[0]);
......
LOG.info("Connecting exclusively to " + zkHostPort.toString());
......
LOG.error("Not a valid number: " + e.getMessage());
```

Author: Matteo Minardi <matteo.minardi@diennea.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Benjamin Reed <breed@apache.org>, Damien Diederen <ddiederen@apache.org>

Closes apache#1530 from mino181295/fix/ZOOKEEPER-3989/generate-load-uselog
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
Sensitive data about args[0], zkHostPort, and cmdNumber are directly printed and may leak.
For security, log should be used to record these data, as well as log in other classes such as org.apache.zookeeper.server.ZooKeeperServer:
LOG = LoggerFactory.getLogger(GenerateLoad.class);
```
LOG.error("Could not connect to " + args[0]);
......
LOG.info("Connecting exclusively to " + zkHostPort.toString());
......
LOG.error("Not a valid number: " + e.getMessage());
```

Author: Matteo Minardi <matteo.minardi@diennea.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Benjamin Reed <breed@apache.org>, Damien Diederen <ddiederen@apache.org>

Closes apache#1530 from mino181295/fix/ZOOKEEPER-3989/generate-load-uselog
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
Sensitive data about args[0], zkHostPort, and cmdNumber are directly printed and may leak.
For security, log should be used to record these data, as well as log in other classes such as org.apache.zookeeper.server.ZooKeeperServer:
LOG = LoggerFactory.getLogger(GenerateLoad.class);
```
LOG.error("Could not connect to " + args[0]);
......
LOG.info("Connecting exclusively to " + zkHostPort.toString());
......
LOG.error("Not a valid number: " + e.getMessage());
```

Author: Matteo Minardi <matteo.minardi@diennea.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>, Benjamin Reed <breed@apache.org>, Damien Diederen <ddiederen@apache.org>

Closes apache#1530 from mino181295/fix/ZOOKEEPER-3989/generate-load-uselog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants