10 ms·
I'm not big on bans. What I am big on is forcing developers to make deliberate choices. That's why I like React's policy of naming functionality "dangerouslySe
by ThePadawan 6y ago
I'm not big on bans.
What I am big on is forcing developers to make deliberate choices. That's why I like React's policy of naming functionality "dangerouslySetInnerHTML" or "__SECRET_DOM_DO_NOT_USE_OR_YOU_WILL_BE_FIRED".
If you add usages for these in a PR I'm reviewing without justification, it's not getting merged.
So why not make cryptographically unsafe random unsafeRandom() or shittyRandom() or iCopyPastedThisFromStackOverflowRandom()?
- cpeterso 6y agoFirefox has a secret setting used in test automation called “turn_off_all_security_so_that_viruses_can_take_over_this_computer”. https://searchfox.org/mozilla-central/rev/3ff133d19f87da2ba01ade55d6e7fdf0fc28b08c/js/xpconnect/src/nsXPConnect.cpp#1179 https://searchfox.org/mozilla-central/rev/3ff133d19f87da2ba0...
- wnevets 6y agohmm I wonder what would happen if I enable this setting...
- throwaway744678 6y ago*** wnevets quit (Connection reset by peer)
- makomk 6y agoYeah, and if I remember correctly one neat technique for exploiting security vulnerabilities in Firefox was to use them in order to set turn_off_all_security_so_that_viruses_can_take_over_this_computer to true, with obvious results.
- zrm 6y agoIt's also a good idea to give safer things shorter names. So make random() a CSPRNG (and an alias for SecureRandom() for people who want to be explicit) while InsecureFastRandom() is just what it says and has no other name. Then if you really need performance over unpredictability, it's there, but nobody is confused about what they're getting. And lazy people who don't like to type or pay close attention get the safe one.
- cogman10 6y agoThat's be my preference. random() should be the most universally applicable random which includes making it as secure as possible. Non-universally applicable randoms should be named accordingly.
- njharman 6y agoMost simulations, games, everything that isn't generating cryptography does not need security in it's random. For most domains secure random is a niche, not universally applicable.
- epanchin 6y agoAnd one would hope secure devs know which random to use, whereas gaming devs have no reason to know.
- josephg 6y agoLook up “password generator” or similar terms on npm and take a look at how the packages you find generate random numbers. I did this ~5 years ago and it took until the second page of results before I found any packages that used a crypto-secure rng.
- esrauch 6y agoEven that seems unlikely to be problematic for anything short of literally constant seeds AND a generator becoming extremely popular. The vast majority of people reuse low-entropy passwords, figuring out what password generator someone used would be a much higher bar than figuring out passwords, and just knowing the insecure generator wouldnt reduce the entropy by that much. Actually, a password generator on GitHub that generates the password that is literally just seconds-since-1970 would still be a good generator for almost all use cases.
- 6y ago
- Spivak 6y agoSo you’re not big on bans but if you use dangerouslySetInnerHTML then it’s definitely not getting merged? Is that not a ban? Do you just not like when tooling enforces it?
- deleted 6y ago[deleted]
- dariusj18 6y agoThey said "without justification"
- ThePadawan 6y agoNo, as I said, it would raise a red flag. That flag can be lowered by justification, e. g. if you add types or constraints to only allow safe-enough parameters etc.
- deleted 6y ago[deleted]
- sp332 6y agoThe "ban" can be evaded by telling semgrep to ignore it for one line. https://semgrep.dev/docs/ignoring-findings/ https://semgrep.dev/docs/ignoring-findings/ This doesn't really scale though - if someone bans it with a different tool, you'd have to tell each tool to ignore this line.
- iratewizard 6y agoBeginner friendliness is something to remember, too. There are half a dozen words you could use to describe pseudoRandom(). Random() is easy for a first year or non-professional to remember.
- dariusj18 6y agoMost of the time the people who write and name the functions don't know it's not secure or safe. So you would still need to ban random when the new name is implemented.
- mannerheim 6y agoaccursedUnutterablePerformIO https://hackage.haskell.org/package/bytestring-0.11.0.0/docs/Data-ByteString-Internal.html https://hackage.haskell.org/package/bytestring-0.11.0.0/docs...
- deleted 6y ago[deleted]
- shadowgovt 6y agoI agree, with the caveat that use of random for cryptography is actually a domain specific use case. It's probably okay to leave the function as it is and just drill into people that if you're doing cryptography, you either need to know exactly what you're doing all the way down to the hardware or you need to leave it a task for somebody else more specialized than you. I, for one, never assume random() is cryptographically secure, but it might be because I grew up programming during the era where random was computed off of clock cycles since CPU startup because there wasn't much other cheap entropy to lay a hand on ("battery-backed onboard date clock?! Oh, look who has AKERS money!").
- nicoburns 6y agoReact's dangerouslySetInnerHTML is indeed a good way of handling this kind of thing. Rust's `unsafe` is another example of the same approach.
- setr 6y agoI don't think rust's unsafe says anything about cryptographic security, beyond pointer-safety implications.
- nicoburns 6y agoNeither does React's dangerouslySetInnerHTML. What both of these do is mark a potentially dangerous operation with an in-your-face warning message that you have to go out of your way to ignore. Which in practice is very useful, as often the biggest problem with security issues is not know what you don't know. It's impractical to review the entire codebase on a regular basis, and it can be hard to know which bits to focus on.
- andoriyu 6y agouhm no. Rust's "unsafe" is a pretty bad name and completely different reasoning behind using it. It doesn't mark something as "dangerously unsafe, don't use", to a consumer it indicates "exercise caution" and to a compiler it just allows 5 things: Dereference a raw pointer Call an unsafe function or method Access or modify a mutable static variable Implement an unsafe trait Access fields of unions The point of "unsafe" in rust is to highlight which area requires more human attention... not to discourage its usage. `dangerouslySetInnerHTML` is literally dangerous and allows XSS if used with outside input. It also is faster than the other variant. The same is true for `random()`. Both can be used when you know what are you doing to gain some performance. Meanwhile, `unsafe` rust by itself is not different from safe rust in terms of speed. You have no choice, but to use it places it supposed to be used.
- hyperman1 6y agoIsn't this just part of the world view of Rust? The word 'safe' in Rust has a very specific, technical meaning. 'unsafe' is simply code that is not automatically 'safe' in the Rust sense. A non-Rust developer sees safe/unsafe and gets worked up, but that just means that he should put his rust-colored glasses [*] on. This is not unusual in ICT, known for its colorful language. A non-ICTer hears 'black hat' and thinks about how cool and stylish the hat is. An ICTer hears 'hacker' and thinks about how cool and stylish the hack is. [*] Thanks, Raymond!
- suzzer99 6y agoI have required parameter to push our app to production called: YES_I_HAVE_ALREADY_MERGED_THE_LIB_REPOS_AND_WAITED_FOR_THEM_TO_COMPLETE_BEFORE_MERGING_THE_APP_REPOS Gets the point across and will still work when I'm long gone.
- Macha 6y agoThis assumes writing crypto code is the most common use case for random numbers. How often do you write crypto code? vs How often do people use random numbers + threshold for A/B tests? How often do game developers use random numbers for gameplay variety? How often is random used for animation variety? Do these use cases need the overhead of a cryptography RNG? A former employer had the same issue as in the article - the security team implemented an automated vulnerability scanner in our github enterprise instance, and it spammed comments and marked a review as requiring changes if it edited any merge request which touched a file which used java.util.Random. It lasted a day before the security team was made turn it off as on our team (and many others), literally 0 uses of random numbers were those requiring a secure random.
- jcelerier 6y ago> It lasted a day before the security team was made turn it off as on our team (and many others), literally 0 uses of random numbers were those requiring a secure random. can concur, currently approaching 400k SLOC of C++ in the repo. A few dozens different places crop up where random is needed (with a quick and dirty grepping). Literally 0% is for secure stuff. Most of it has to be as fast as possible (and very low quality, as it just needs to be random / noisy enough to look random for human perception)
- 1MoreThing 6y agoThe creation of a trueRandom function certainly seems to solve this problem more than taking away a useful tool for cases where pseudo-random is good enough.
- extropy 6y agoThe problem is that true random is quite expensive without dedicated hardware and you can still easily bias true random if you are not careful. IMO there is no use of true random unless you really know what you are doing. Also there is true random - pure entropy bits and there is cryptographically secure pseudorandom, seeded with true random bits.
- 6y ago
- ddlsmurf 6y agoIndeed this is a stupid debate. Knives help us in the kitchen but also sometimes stab people - should we ban knives ?