Code review for a little password generation project

xuanbao · · 585 次点击    
这是一个分享于 的资源,其中的信息可能已经有所发展或是发生改变。
<p>Hi all, </p> <p>Lately I&#39;ve being playing with a project for generating per-domain passwords. It&#39;s something that I wanted to use to generate unique passwords for each site I have a login on, but also not having to remember them. Password managers are an option of course, but I never got used to using one. This and the desire to use Go in a real project led to the creation of <a href="https://github.com/makpoc/gopass/">GoPass</a>. However I&#39;m only in the beginning of understanding both the go language as well as the concept of web programming with it. I didn&#39;t want to use any routers, frameworks, etc just to get to know the standard library better.</p> <p>And after this introduction I wanted to ask you to take a look at the project and provide any feedback you have (especially for ui/gopass-web). I know the password generator function is not very clever (the part where it adds special characters), but when I have more time I&#39;ll most probably redo it.</p> <hr/>**评论:**<br/><br/>drewindo: <pre><p>I&#39;m not seeing any major structural/style issues so far.</p> <p>In the CLI, it wasn&#39;t immediately clear that the special characters flag needed to be written in the form &#34;special-characters=false&#34; rather than &#34;special-characters false&#34;, which would have been similar to every other flag. Some alternative short flags would be nice too, as is the case with many CLI utilities.</p> <p>Running the command naked, the error was almost overwhelmed by the help dump. A short and to the point message might be better, that informs the user they need to either manually set master or create a master file. Provide an additional flag &#34;-help or -h&#34; to dump the full flag options? A default domain makes sense to me, too. &#34;localhost&#34;?</p> <p>For the webui, running the command should probably indicate where the app is being served, along the lines of the gotour. I can&#39;t tell if the webui will pull from the master file, if it exists.</p></pre>Makpoc: <pre><p>That&#39;s awesome :) I&#39;ll work with the CLI output to make it simpler and more understandable. I don&#39;t see a point in setting default value for domain, because it&#39;s useless in most cases (that I can think of at least). For the short/full flags - I might move to using <a href="https://github.com/codegangsta/cli" rel="nofollow">codegangsta&#39;s cli</a> package. </p> <p>For the webui - I&#39;ll make sure to add a start message. It does not read the masterfile because if at some point it&#39;s hosted somerwhere it would not make sence for everyone to use the same secret. The web UI has no state, nothing is stored and/or loaded from the system - the password is generated purely based on the provided parameters.</p> <p>What about the templates. To me it feels strange to have a separate variable for each page. The templating system is unnatural or (the better explanation) I don&#39;t understand how to use it. Right now I generate a template, containing everything except the actual content of the page (headers, navbar, ...) and then I add the concrete content and store the resulting page in a variable, which is known by the handlers. I don&#39;t like it :)</p></pre>jerf: <pre><p>If you&#39;re having fun, OK, but bear in mind that everybody who takes this approach ultimately ends up having to abandon it in terms of real-world use. You end up unable to deal with all the stupid password requirements in the world, and you end up with no choice but to allow users to store arbitrary passwords, which requires storage.</p> <p>While this has since been fixed, I had a site that my work outsourced some HR stuff to that said it required &#34;at least one number&#34; in its password, but a lot of us were having trouble satisfying it. Fortunately the client-side JS had a copy of the validation code, so we could see that what they meant was that we had to have precisely 1 character that was either a 0 or a 1, and no other digit. (I say &#34;copy&#34; because we did try the obvious &#34;just submit a good password anyhow via direct HTTP&#34;, but the server rejected it too.)</p> <p>This is not all the problems I&#39;ve encountered. This is merely an <em>example</em>.</p> <p>You just don&#39;t want to be in the position of being yanked around by every developer who thinks they know something about password security and their password requirements are <em>really secure</em>, yo.</p></pre>Makpoc: <pre><p>Hey. I understand your point. I already encountered several sites with &#34;strange requirements&#34; (including paypal). Apperantly &#34;security&#34; means a lot of things for a lot of people (including me by the way). </p> <p>As you said at the beginning - I&#39;m using this project to better understand some points of go programming as well as to practice what I&#39;ve already learned from previous such attempts.</p></pre>opennota: <pre><blockquote> <p>(the part where it adds special characters), but when I have more time I&#39;ll most probably redo it.</p> </blockquote> <p>You can write a simple PRNG for that, <a href="https://github.com/opennota/rand/blob/master/rand.go">like this</a>.</p></pre>Makpoc: <pre><p>Thanks, I&#39;ll definitely look into it!</p></pre>lwe: <pre><p>If you want to look at a good password generator have a look at pwgen: <a href="http://sourceforge.net/p/pwgen/code/ci/master/tree/" rel="nofollow">http://sourceforge.net/p/pwgen/code/ci/master/tree/</a> . By default it generates passwords which can be easily remembered by humans by using phonetics, without losing entropy.</p></pre>drewindo: <pre><p>In reply to both approaches, the classic comic on exactly this subject: <a href="https://xkcd.com/936/" rel="nofollow">https://xkcd.com/936/</a></p></pre>Makpoc: <pre><p>Thanks to both of you. I know there are different ways to generate a password. The project I was working on is mostly for educational purposes. As I mention im both the description above and the README.md - I am aware that the generator logic is somehow flawed, but I still need to know where my other bad decisions are :)</p> <p>@Iwe - I&#39;ll take a look a this project - thanks @drewindo - xkcd is alread in my news feed ;)</p></pre>drewindo: <pre><p>Heh, I wasn&#39;t trying to be snarky, really. I&#39;m looking over your code for more substantive comments, although I&#39;m by no means an expert.</p> <p>It&#39;s an interesting approach, if I understand it, allowing any password to be instantly regenerated, simply given a few config options and a baseline master password. It makes the master all important.</p> <p>I don&#39;t love your project layout, with no package in the root. For a project that&#39;s primarily providing a binary, I&#39;d prefer if the main executable package was in the root of your repo, that way go getting installs the binary to a user&#39;s GOPATH/bin.</p> <p>&#39;Course I realize there are a number of theories on project layout within the community, but it might flow better with gopass-cmd right in the repo&#39;s root, generator and the web ui still in their respective own packages.</p></pre>Makpoc: <pre><p>@drewindo - yeah, that&#39;s the main idea - to be able to regenerate (thus not remember) the per-domain password at any time. You are right however, that the master is important and if you decide to change it - all site-passwords will have to be regenerated. The additional-info field is there just to provide a way for users to change the site passwords without changing the master (e.g. in case it&#39;s compromised or the password, generated with the default settings does not meet the website policy for password complexity).</p> <p>I got the layout idea from SO exactly because I didn&#39;t know how to structure the different components. I don&#39;t like the idea of empty root as well, but if I move the cmd to / it feels inconsistent with the web UI. I might go with it and move it to the root at the end.</p></pre>drewindo: <pre><p>At the least if keeping the gopass-cmd out of the root of the repo, make sure there&#39;s a note in your Readme that designates what the user should install:</p> <p>go get github.com/makpoc/gopass/ui/gopass-cmd</p></pre>xkcd_transcriber: <pre><p><a href="http://imgs.xkcd.com/comics/password_strength.png" rel="nofollow">Image</a></p> <p><strong>Title:</strong> Password Strength</p> <p><strong>Title-text:</strong> To anyone who understands information theory and security and is in an infuriating argument with someone who does not (possibly involving mixed case), I sincerely apologize.</p> <p><a href="http://www.explainxkcd.com/wiki/index.php/936#Explanation" rel="nofollow">Comic Explanation</a></p> <p><strong>Stats:</strong> This comic has been referenced 1659 times, representing 1.9960% of referenced xkcds.</p> <hr/> <p><sup><a href="http://www.xkcd.com" rel="nofollow">xkcd.com</a></sup> <sup>|</sup> <sup><a href="http://www.reddit.com/r/xkcd/" rel="nofollow">xkcd sub</a></sup> <sup>|</sup> <sup><a href="http://www.reddit.com/r/xkcd_transcriber/" rel="nofollow">Problems/Bugs?</a></sup> <sup>|</sup> <sup><a href="http://xkcdref.info/statistics/" rel="nofollow">Statistics</a></sup> <sup>|</sup> <sup><a href="http://reddit.com/message/compose/?to=xkcd_transcriber&amp;subject=ignore%20me&amp;message=ignore%20me" rel="nofollow">Stop Replying</a></sup> <sup>|</sup> <sup><a href="http://reddit.com/message/compose/?to=xkcd_transcriber&amp;subject=delete&amp;message=delete%20t1_cvdl2uc" rel="nofollow">Delete</a></sup></p></pre>barsonme: <pre><p>I already made a little something sort of like this on a whim because coming up with passwords sucks. Take a look and see if there&#39;s anything that you like.</p> <p><a href="https://github.com/EricLagergren/pwpls" rel="nofollow">https://github.com/EricLagergren/pwpls</a></p></pre>opennota: <pre><p><a href="https://github.com/EricLagergren/go-prng/blob/master/xorshift/xorshift_test.go" rel="nofollow">Your dieharder test</a> probably doesn&#39;t test what you think it tests.</p> <pre><code>#===============# rng_name | mt19937 | #===============# </code></pre> <p>To properly test your PRNGs specify <code>-g 200</code> on the dieharder command line and feed it raw binary 32-bit integers, as described on the dieharder man page under &#34;BEST PRACTICE.&#34;</p> <p><a href="https://github.com/opennota/rand/blob/master/rand_dieharder_test.go" rel="nofollow">Example</a></p></pre>

入群交流(和以上内容无关):加入Go大咖交流群,或添加微信:liuxiaoyan-s 备注:入群;或加QQ群:692541889

585 次点击  
加入收藏 微博
暂无回复
添加一条新回复 (您需要 登录 后才能回复 没有账号 ?)
  • 请尽量让自己的回复能够对别人有帮助
  • 支持 Markdown 格式, **粗体**、~~删除线~~、`单行代码`
  • 支持 @ 本站用户;支持表情(输入 : 提示),见 Emoji cheat sheet
  • 图片支持拖拽、截图粘贴等方式上传