ulif / diceware

Passphrases to remember
GNU General Public License v3.0
354 stars 45 forks source link

Add `--inline` flag for realdice random source #73

Closed adinklotz closed 4 years ago

adinklotz commented 4 years ago

I took a crack at implementing #58 When using the 'realdice' method, you can now pass the additional --inline flag, causing it to expect a list of whitespace-separated numbers all in one line, rather than one at a time like normal:

❯ diceware -r realdice --inline -n 3 -s 1
Please roll 5 dice (or a single dice 5 times).
Enter your 5 dice results, separated by spaces: 1 1 1 1 1
Please roll 5 dice (or a single dice 5 times).
Enter your 5 dice results, separated by spaces: 2 2 2 2 2
Please roll 5 dice (or a single dice 5 times).
Enter your 5 dice results, separated by spaces: 3 3 3 3 3
Please roll 2 dice (or a single dice 2 times).
Enter your 2 dice results, separated by spaces: 4 4
Warning: entropy is reduced! Using only first 6 of 20 words/items of your wordlist.
Please roll 1 dice (or a single dice 1 times).
Enter your 1 dice results, separated by spaces: 5
Abac'sDatingHandgrip

Unlike #25, this takes rolls from standard input just like the existing methods, so hopefully doesn't have any extra security implications. I'm not very familiar with pytest, but I made some simple modified versions of the existing tests, updated to use inline numbers. They're nothing particularly exciting, but enough to make sure that the inline mode works the same as the standard realdice mode

ulif commented 4 years ago

Hi @adinklotz,

thank you very much for you patch!

I roughly reviewed everything and overall I would like to merge your propsal. It looks properly coded and I cannot see any security flaws arising from it :) Thank you very much.

I would, however, prefer if we could make this the default or even only method to enter real dice values.

There was one time in development when I considered to request an unforeseeable number of rolls to keep the maximum entropy in cases where the number of wordlist terms does not equal to any power of dice sides.

But this never happend and I guess having two different kinds of input is not justified for some ordinary numbers. As most people (including myself) seem to prefer entering all rolls in a row, we might make it the one and only method.

This would mean, that your patch could be reduced somewhat.

Do you want to do these changes to your PR yourself? That would be great!

adinklotz commented 4 years ago

Sure, I can make that change. Having it as the one and only method does make the most sense to me, but I wanted to err on the side of not breaking backwards compatibility (not that anyone's probably using the real dice mode in a script) until I got the go ahead - since you agree I'll go back and remove the old one-at-a-time method.

adinklotz commented 4 years ago

@ulif I have updated to remove the original one-at-a-time realdice method and only use the inline one. Tests have been updated to reflect this, and everything seems to still be running smoothly Since it's now the default, I updated the example in the readme to show this method, however I don't know what other docs will want to be updated so I'll leave that to you (or I can track them down if you want, but you can probably do it quicker since you know the documentation)

ulif commented 4 years ago

Thank you! :D