-
-
Notifications
You must be signed in to change notification settings - Fork 126
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
Stop instructing people to write plaintext passwords directly to env #10
Comments
@tiangolo do you approve of this proposed change? |
I'm glad you like it! Requiring people to type their passwords inside a command, in the middle of it, makes it a bit harder to use directly, to copy and paste it, updating just the pieces custom to each user. Also, the I think it's easier for more advanced users to modify the command live, like you did, to customize it to some specific requirements, than to force everyone to use some specific more complex instructions. |
It's up to you, @tiangolo. One last attempt to get you to agree with my suggestion: god forbid anyone feels like it would make sense to run all this in a |
While understanding security concerns raised by @rayrrr I have to admit, the documentation is doing good job in explaining complex concepts in very straightforward way. To me the existing (not perfectly secure, but easy to understand and reproduce) way of dealing with passwords in this documentation seems to be good choice. One has to admit, that perfect solutions (totally secure, efficient etc.) are great concepts but not really reachable. If one attempts to write documentation for such perfect solution, it starts growing and probably never completes. Even if it completes, than it is likely to suffer from being too extensive and difficult to understand and use, not talking about growing difficulty to maintain it. Personally, I think, that adopting these new technologies is very likely to contribute to overall solution quality. As soon as developers get used to that, they will have enough capacity to improve on more details such as not storing credentials in env. variables. |
One more reason I brought this up in the first place: I'm going to use a boxing metaphor here. I was training once and a trainer came to me and said I was holding my guard up wrong; he showed me the right way and then said "you fight how you train." In other words, the practices you learn as a beginner are likely to stick around. This is among the reasons for my concern, since the tutorial just might be aimed at command-line-beginners. @vlcinsky I like your explanation of how to just store the hashed password in the env in #21, thanks! |
Welll, you could ask people to write out their password directly before hashing, but that would be useless, the password still appear and persist in ~/.bash_history or ~./zsh_history with both methods. |
@QuentinFAIDIDE not true; there are ways to do this that totally hide the input from stdin and thus any *history files as well. In fact, @vlcinsky wrote it up in such a way in #21. My vote is for that approach! |
@rayrrr Sure, there are ways to hide stdin, you are right, and we should do just like #21 , but you wrote in OP as your alternative method:
So, using this line would definitely leak pwds to *history files. No need therefore to state that my statement is 'not true', since it's as true as 1 + 1 = 2. |
Whoops, we meant stdout. And yes @QuentinFAIDIDE you are technically correct, all I was saying was that in #21 the |
Thank you @tiangolo and everyone! Closing this out. |
This tutorial ROCKS! The one issue I've noticed is that for no good reason you are instructing people to put their plaintext passwords into their production environment variables. This is not a good security practice.
I skipped that step, and just typed in my plaintext password directly while creating the hashed version. Please update accordingly: remove the
ADMIN_PASSWORD
stuff from all the tutorials, and update the hashed password instructions to something like:etc.
FWIW, I did it this way and got Traefik and Consul up and running just fine.
Also, I'd be happy to make these changes myself in a PR, just want to see if there are any objections first.
The text was updated successfully, but these errors were encountered: