lanrat / czds

simple golang API and tools to interact with czds.icann.org
https://pkg.go.dev/github.com/lanrat/czds
GNU General Public License v3.0
72 stars 14 forks source link

Use 'getpass' to handle the password. #17

Closed jschauma closed 1 year ago

jschauma commented 1 year ago

This allows the user to specify the password retrieval method and avoid leaking their password into the process table and shell history.

If '-password' is not specified, the user is prompted for the password on the tty; if the user wants to provide the password as before, they would pass '-password pass:password'.

See https://github.com/jschauma/getpass for all supported options.

See also: https://www.netmeister.org/blog/passing-passwords.html

lanrat commented 1 year ago

Thanks for the PR @jschauma!

I agree with your security concerns that having the password in the flags is not ideal for security. That design choice was leftover from a prior version if this tool.

However, I have 2 concerns with making this change:

  1. Currently one of the good things about this library are that is has no external dependencies. This would change that. It's not a deal breaker, but not something I'm not eager to change. Would it be possible to implement this using just the go standard library?
  2. This would be a breaking change. Currently everyone passes their passwords in using -password PASSWORD, your change would break that requiring everyone to change it to -password env:PASSWORD or one of the other new methods. Again while not a deal breaker, this is something I would like to avoid if possible. Perhaps it could be changed to use a different flag to get the password via other means if -password is not present?

What do you think?

jschauma commented 1 year ago

Ian Foster @.***> wrote:

  1. Currently one of the good things about this library are that is has no external dependencies. This would change that. It's not a deal breaker, but not something I'm eager to change. Would it be possible to implement this using just the go standard library?

You could slurp in the 'getpass' module code itself instead of using it as a dependency. It's only 280 lines of code, and you might be able to reduce that to only those methods you are about.

Probably would bring this down to maybe 100 - 150 lines of code?

Of course that means you lose some of the flexibility of the module, but I'm sympathetic to the desire to avoid dependencies.

  1. This would be a breaking change.

Understood. Might be preferable to use a different flag, perhaps "-passin" or something like that.

That way, you could keep old functionality by letting the user insecurely provide the password on the command-line, or use another option.

-Jan

jschauma commented 1 year ago

Come to think of it, since the other commands also should use a better mechanism, you could just grab https://github.com/jschauma/getpass/blob/main/getpass.go and import it verbatim into util.go and then use 'czds.Getpass' wherever you want.

jschauma commented 1 year ago

Here, updated the PR to pull in the module completely. See if you prefer that, even if it feels like the go ecosystem would favor importing modules. Either's fine with me.

lanrat commented 1 year ago

This looks good.

One last thing: Can you update README.md with the new flag information?

Using the output of the command help is enough.

jschauma commented 1 year ago

Done.