<p>Original tweet:</p>
<p><a href="https://twitter.com/elithrar/status/837492981582057472">https://twitter.com/elithrar/status/837492981582057472</a></p>
<p>The code in question (beego, utils.RandomCreateBytes) can be found here:</p>
<p><a href="https://github.com/astaxie/beego/blob/master/utils/rand.go">https://github.com/astaxie/beego/blob/master/utils/rand.go</a></p>
<p>The commit that introduced this "workaround" is here:</p>
<p><a href="https://github.com/astaxie/beego/commit/c3a23b28ee030fa8a43a683023fb09f7ab79c74e">https://github.com/astaxie/beego/commit/c3a23b28ee030fa8a43a683023fb09f7ab79c74e</a></p>
<p>Relevant issue here:</p>
<p><a href="https://github.com/astaxie/beego/issues/620">https://github.com/astaxie/beego/issues/620</a></p>
<p>golang-nuts comment from agl here:</p>
<p><a href="https://groups.google.com/forum/#!msg/golang-nuts/HbxuDsYOAuU/v54fcAzerJ8J">https://groups.google.com/forum/#!msg/golang-nuts/HbxuDsYOAuU/v54fcAzerJ8J</a></p>
<p>Please, carefully consider whether or not you truly need a "framework" for your next application. You never know what kind of bad decisions that framework might make on your behalf.</p>
<hr/>**评论：**<br/><br/>MatthiasLuft: <pre><p>The persistent ignorance is scary.</p></pre>mwholt: <pre><p>Also note that <a href="https://github.com/henrylee2cn/faygo">faygo</a> is affected by this as well, since it borrows the vulnerable code from beego.</p></pre>epiris: <pre><p>It's good to raise awareness of crypto usage, happy to see this! Just want to point out this isn't a security issue by itself, it's just alarming for the reader. This becomes a security issue based on how and where it is used. Since neither of those are mentioned and I'm curious if this usage opened any attack vectors I searched the repo real fast and there is three places it's used.</p>
<p>Captchas are fine with a PRNG in my opinion, no need to waste high quality entropy for defeating bots.</p>
<p>Csrf token generation is a vulnerability with the severity depending on the threat actors position, the more tokens (consecutively greatly increases the ability to synchronize here) they observe the more likely they are to predict the seed. Once the seed is known the constraints of common csrf mitigation techniques including this one are easily circumvented, game over.</p>
<p>The sessions utils file is super misleading, it calls the iv generation key generation. Using math rand for key gen is a security flaw. Using math.rand for for <strong>aes iv generation in ctr mode</strong> is OK. I am not digging through other files to see how encode / decide is called but if the key generation is done using math rand it's insecure, otherwise it's just terribly misleading while being "secure" as it pertains to discussion here, there could be more deep issues that a careful audit would uncover.</p>
<p>If you want a CPRNG, use one. If you want a PRNG, use one. Having random byte generation mixing the two in a utility file like this just creates inevitable security issues. It is unfortunate this existed for three entire years, but common these days it seems.</p></pre>FiloSottile: <pre><blockquote>
<p>waste high quality entropy for defeating bots</p>
</blockquote>
<p>Entropy does not run out. It's a myth.</p>
<blockquote>
<p>Using math.rand for for aes iv generation in ctr mode is OK.</p>
</blockquote>
<p>This is dangerous advice. I know what you mean, CTR IVs only have to be unique, not unpredictable. But <code>math/rand</code>is not seeded by default, so that would explode very easily. And even if seeded with the time, time rolls back. And other modes than CTR are less forgiving and users can't be expected to know.</p>
<p>And in general, <strong>it's bad advice to suggest to use anything else than <code>crypto/rand</code> for anything crypto or security related</strong>.</p></pre>epiris: <pre><blockquote>
<p>Entropy does not run out. It's a myth.</p>
</blockquote>
<p>I'm not sure how wasting high quality entropy implies I think it "runs out", perhaps just looking for things to disagree with. Regardless such a sweeping statement that entropy does not "run out" is incorrect without qualifying your source of entropy. When your source is a entropy pool that derives from many origins of both finite noise as well as other hardware entropy sources than your statement is fair.</p>
<blockquote>
<p>This is dangerous advice.</p>
</blockquote>
<p>I'm not giving advice, I am evaluating this discovery in a security context. I'm sorry if this didn't clearly set my intent, maybe I could have worded it better: <em>I'm curious if this usage opened any attack vectors I searched the repo real fast and there is three places it's used.</em></p>
<blockquote>
<p>I know what you mean, CTR IVs only have to be unique, not unpredictable. But math/randis not seeded by default, so that would explode very easily.</p>
</blockquote>
<p>Wrong, the default rand.Read() method uses a source seeded with the numeric constant 1 if memory serves me right.</p>
<blockquote>
<p>And even if seeded with the time, time rolls back. And other modes than CTR are less forgiving and users can't be expected to know.</p>
</blockquote>
<p>I'm not sure what your point is here, it doesn't make sense to me. You would need to elaborate for me to comment further.</p>
<blockquote>
<p>And in general, it's bad advice to suggest to use anything else than crypto/rand for anything crypto or security related.</p>
</blockquote>
<p>Again this was not advice, it was a quick cursory evaluation of the potential attack vectors of this discovery. Regardless this regurgitates what I finished my post with: <em>If you want a CPRNG, use one. If you want a PRNG, use one.</em> lol, you could have elaborated on what I said .. adding value instead of taking an adversarial approach that made it seem like our information was conflicting. Have a good one.</p></pre>dchapes: <pre><blockquote>
<blockquote>
<p>waste high quality entropy for defeating bots</p>
</blockquote>
<p>Entropy does not run out. It's a myth.</p>
</blockquote>
<p>Although perhaps uncommon now, there was a time that system crypto random devices (e.g. <code>/dev/random</code>) were a limited system resource. E.g. before FreeBSD switched to Yarrow (and more recently to Fortuna) their <code>/dev/random</code> would block based on how much was read from it; <code>/dev/urandom</code> was an unlimited/non-blocking device (which for FreeBSD since Yarrow is just a symlink to <code>/dev/random</code>). The Go documentation mentions <code>/dev/urandom</code> but the code has both <code>/dev/random</code> and <code>/dev/urandom</code> so it's unclear if there are platforms on which <code>crypto/rand</code> will block.</p></pre>nerr: <pre><p>Thank you for your in-depth and informative explanation of the matter. I do not have the security background to determine if this poses an exploitable threat in the wild, but I wanted to raise attention to the matter after it was brought to my attention earlier today.</p>
<p>Thanks again. I'm saving this comment for future reference.</p></pre>icholy: <pre><p>At the very very least, the call to <code>r.Seed(time.Now().UnixNano())</code> shouldn't be called while generating it. It's constantly being seeded with a known number!</p></pre>yuhatemepvp: <pre><p>Maybe someone could explain this. I wrote a little uuid4 generator which intially used crypt/Rand. Fairly fast; generating about 80 MB/s of uuid4's. Unfortunately I was seeing a really high number of collisions. On the order of about 1 per 10000.</p>
<p>Ended up switching to math/Rand with time.Now() as a seed which seemed to fix the collision issue, though is a magnitude slower.</p></pre>dchapes: <pre><p>First, the <code>math/rand</code> functions are go-routine safe which means they do locking. If you care about throughput (you seem to since you mention a throughput rate) you should avoid this, often by using a <code>*rand.Rand</code> provided by the caller. The caller can arrange to make a non-locking PRNG per go-routine (e.g. something like <code>rand.New(rand.NewSource(time.Now().UnixNano()))</code>; and it's per go-routine not per call, just as seeding a PRNG on each call is bad/wrong creating a new <code>rand.Rand</code> for each call would be bad). A side benefit of using a caller provided PRNG is that they can use whatever source/seed/etc they want.</p></pre>dchapes: <pre><p>E.g. <a href="https://play.golang.org/p/CxoINwV6qu.go" rel="nofollow">https://play.golang.org/p/CxoINwV6qu.go</a></p>
<p>For me, calling <code>rand.Int63()</code> is ~23 ns whereas calling r.Int63() ~7.3 ns.
With <a href="https://golang.org/pkg/testing/#B.RunParallel" rel="nofollow">B.RunParallel</a> and 8 go-routines (on a 4-core Xeon with 8 processors) calling <code>rand.Int63()</code> goes to ~155 ns whereas <code>r.Int63()</code> is ~2 ns.</p>
<pre><code>BenchmarkRandG 100000000 23.0 ns/op
BenchmarkRandL 200000000 7.31 ns/op
BenchmarkRandGParallel-2 30000000 52.1 ns/op
BenchmarkRandGParallel-4 20000000 114 ns/op
BenchmarkRandGParallel-8 10000000 155 ns/op
BenchmarkRandLParallel-2 300000000 4.17 ns/op
BenchmarkRandLParallel-4 500000000 2.67 ns/op
BenchmarkRandLParallel-8 1000000000 2.02 ns/op
</code></pre></pre>yuhatemepvp: <pre><p>Interesting, all I am using the uuid4's for is to be able to trace a request thru my stack. So randomness isn't as important as uniqueness. Found it strange that crypt/rand had such a high collision rate for something that was "cryptographically ready"</p></pre>: <pre><p>[deleted]</p></pre>nerr: <pre><blockquote>
<p>Oh, now I remember why I ditched go and It wasn't because of the language...</p>
</blockquote>
<p>Out of curiosity, what brings you back to <a href="/r/golang">/r/golang</a> then?</p>
<p>The same advice applies to any language, and any external dependency. By choosing to use a third party's code in your project, you are accepting the fact that there may be decisions made by that dependency, on your behalf, that put your users in jeopardy.</p>
<p>A quote from the GitHub issue:</p>
<blockquote>
<p>I should simply use the
rand.Reader. while not to catch the error and use another way to solve
the problem. leave this to user. But user just want the sessionID, it's
really unfriendly for users.</p>
</blockquote>
<p>The author of beego has chosen to proritize "user friendliness" over "user safety", and in my opinion, this is completely unacceptable, hence the motivation for submitting this post. </p>
<p>If you disagree, you are more than welcome to state your case, though I suspect you're just trying to stir up trouble.</p></pre>dmikalova: <pre><p>Please elaborate on why :)</p></pre>Sphax: <pre><p>Because of good advice ?</p></pre>
