-
Notifications
You must be signed in to change notification settings - Fork 0
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
[FEATURE] Passphrase generation command #1
Open
ncdulo
wants to merge
6
commits into
master
Choose a base branch
from
feature/passphrase
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a safety check we probably should have been using earlier, but didn't need to. Now it's a bit more important. Because we use one `Password` class that can operate in two different modes, called from two different commands in `__main__`.. It is now important to check for the existence of a key we want to grab into our attributes from `**kwargs`. This is because if we enter `Password.__init__` from another `click` command, were all of the arguments are not specified on the command line, we will hit a `KeyError`. At least, if we were to directly access the argument with a `kwargs['argument']` type statement. If a given key does not exist in the `kwargs` dictionary, it will be assigned a default of `None`.
Down the road, this will handle generating a passphrase. I chose to include this into a separate function because while similar to `generate`, the behavior feels different enough to warrant it. Basic flow of the function has been laid out in the method comments. Some initial experimenting in the REPL has given me a good feel on how I should try to implement this.
Pre-emptively including the CC BY 3.0 attribution for the passphrase word lists. The only reason they are not yet commited is because I'm not sure which location they should reside in. I'll get back to that.
Include the Electronic Frontier Foundation's Wordlists provided under the CC BY 3.0 license to assist with passphrase generation through dice rolls. The content contained in these files is copyright 2020 Electronic Frontier Foundation. - Wordlists - https://www.eff.org/deeplinks/2016/07/new-wordlists-random-passphrases - EFF Copyright Notice - https://www.eff.org/copyright - Creative Commons Attribution 3.0 - http://creativecommons.org/licenses/by/3.0/us/
Add relevant lines so that Pip knows about the EFF Wordlist used for passphrase generation. This has been tested, but I am not 100% it is correct until I can do some further testing. Should work. ;)
Add the first iteration of our lovely new passphrase generator. This commit does several things. I'll try to briefly break them down, without as much of an essay as usual. In approximate order of execution, we.. - Pass the `--debug` flag value into `Password`, this allows us to force passphrase generation without having the command made yet. - Add attributes to `Password` for determining the EFF Wordlist path. - `Password.generate_passphrase` - Load the word list (on demand loading) - For each of the total words requested in the passphrase, loop - Roll five dice, six sides each. `_roll_dice` is a generator, so we iterate over the returned roll results. - Concatenate string dice results into a 5-character index. - Iterate over `_word_list` and return the line which matches our previously generated word index. This is the list comprehension. Should only return one result. - Split the line on '\t' to the left, and a '\n' to the right - This can be more readible. Feels hacky right now. - We now have the word from the EFF Wordlist that corresponds to the result of our five dice rolls! - Append that word onto the password. Deduct one from the `word_remaining` counter. Continue looping. That all said, this works based on my testing so far. It is a *very* rough draft. Just enough to get some words off our list. The proper CLI command, argument specification, and improved passphrases will be coming shortly. For now, to force passphrase generation, and test this new feature, pass the `--debug` flag like so: ``` $ pywrdgen --debug gen --count 6 --length 3 $ pywrdgen -d gen -c 6 -l 3 ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What do we want?
Passphrases
Why?
They are easier to remember, and can be more secure than a traditional random password. The inclusion of real words, in combination with random, can make a brute force attack much more difficult and time consuming.
How are we gonna do it?
I've started this lovely topic branch just for that purpose! The gist of it though, is we will use the EFF Wordlists (graciously provided under the CC BY 3.0 license, thank you!) along with their recommendation of using dice rolls to determine a word index. By grabbing the word at that index, and concatenating all of the words together, we get a rudimentary passphrase. Neat!
The reason I say rudimentary -- we've still got work to do. While a password such as
outsourceacrobatjellied
is probably alright for most purposes. We can do better. Need to add support for swapping around characters, including special characters, mixed case, etc. All to be specified by the CLI args. Using the previous passphrase as an example, I would like a potential output to be something likeoutSOUrc3&aCr0B4T2d6je11i3D
.Any other concerns?
This branch started off very simply, and I felt progress was moving along great. However, I have begun to notice the rough edges and feel the current implementation is a little lackluster. We can do better. I am noticing a definite need for some type of design pattern here. Have not decided which fits best. I think the top candidates will be a object factory (create passwords on demand), or a strategy pattern (passwords decide which strategy/algorithm they need for the proper type of password).
Using the right organizational structure will ease the remaining code being written on this feature, and of course will help us in the long run. More readable code, less room for bugs to sneak in, etc, etc. This feature request may turn into a larger overall code refactor, making a note of that now so I don't feel surprised when I find myself at that point 😂
To summarize though: we want passphrases. The current codebase is beginning to show some symptoms of poor design. Pull it all together and give me an
amBiGUouS^4_proBE&h34ter
.