4 ms·
I agree there are more jerks in the crypto community than elsewhere, but in this instance it is justified, crypto is serious and if you are gonna claim to be se
by antocv 13y ago
I agree there are more jerks in the crypto community than elsewhere, but in this instance it is justified, crypto is serious and if you are gonna claim to be secure and all that - peoples lives can hang on it - then make sure you get the basics right.
I mean come on, Im not a crypto-programmer, but this, Cryptocat.random()*123456789 !?
- anonymous 13y agoActually it's 1234567890. Which is even worse. Cryptocat is written by people who don't fully understand how their program works. They don't fully understand how the platform on which their program is built works. They may be smart, but they need to sit down and learn things.
- adekok 13y agoThis is the most important take-away. They're not just incompetent at crypto, they're incompetent at programming. And they don't understand that they don't understand programming.
- oinksoft 13y agoFascinating. For those of us not versed in cryptography, can anyone elaborate on why foo.random() * 123456789 is insecure, and why it is made moreso with the added zero? Is this what the author describes, using floating point to generate random numbers (where he cites this code)? I am wondering about the theory behind this.
- IanCal 13y agoI'm not a crypto guy, so this is just my interpretation of part of the issue. Anything that should be random that introduces easily predictable could be a problem. With that in mind, what we've got is a sixteen digit floating point number (their random number generator works in decimal, not binary) multiplied by a very specific number. This is then turned back into a string, so I'm going to assume it's ascii (I'm sure a similar problem would be if this was in utf-8). Now, ascii representations of numbers means that each digit is converted to hex 30 to 39 So, what we're passing into the MD5 sum is 3-3-3-3-3-3-3-3... with one of them 2E because of the decimal place. The first digit cannot be a 0 or the decimal place, so it's going to be 31-39 to start with. This first digit is also significantly more likely to be a 1 than anything else, here's a random sample of the first digit counts: Counter(str(get_uniform()*1234567890)[0] for i in range(100000)) Counter({'1': 28042, '2': 9208, '5': 9068, '3': 9053, '7': 9045, '9': 8987, '8': 8972, '4': 8829, '6': 8796}) How about the second digit? Still biased: Counter({'0': 16368, '1': 16227, '2': 10942, '6': 8141, '5': 8124, '3': 8118, '4': 8067, '9': 8012, '7': 8006, '8': 7995}) Sixth digit? More normalised but the decimal pace is not really that common here. Counter({'8': 10235, '2': 10166, '4': 10114, '9': 10032, '6': 10007, '3': 9969, '7': 9911, '0': 9883, '1': 9872, '5': 9804, '.': 7}) Ninth digit? Now we're getting flatter Counter({'2': 9418, '1': 9406, '9': 9279, '3': 9269, '0': 9258, '4': 9248, '7': 9228, '5': 9227, '8': 9215, '6': 9208, '.': 7244}) Where do we see the decimal place? Counter(str(get_uniform()*1234567890).find(".") for i in range(100000)) Counter({9: 72887, 10: 18823, 8: 7460, 7: 740, 6: 81, 5: 7, 4: 2}) Fundamentally, we've taken a random series of bytes, turned them into a very regular pattern (3-3-3...) and added a significant bias towards particular digits in particular places. How about the length? Counter({13: 90052, 12: 9150, 11: 792, 10: 6}) We've got something with a very predictable structure, in short. Multiplying by 10 I don't think makes it any less secure, it's just silly. All you do is move the decimal place over by one, although to be fair at least it doesn't add any more bias.
- deleted 13y ago[deleted]
- magikarp 13y agoThat line isn't used in anything sensitive.
- oinksoft 13y agoThe JsDoc for the function where var cnonce = MD5.hexdigest("" + (Cryptocat.random() * 1234567890)); occurs is /** PrivateFunction: _sasl_challenge1_cb * _Private_ handler for DIGEST-MD5 SASL authentication. * * Parameters: * (XMLElement) elem - The challenge stanza. * * Returns: * false to remove the handler. */ which is only called by /** PrivateFunction: _connect_cb * _Private_ handler for initial connection request. * * This handler is used to process the initial connection request * response from the BOSH server. It is used to set up authentication * handlers and start the authentication process. * * SASL authentication will be attempted if available, otherwise * the code will fall back to legacy authentication. * * Parameters: * (Strophe.Request) req - The current request. */ Now, I might be nuts, but unless the documentation is incorrect, this appears to be a "sensitive" application. To make sure I'm not just going on innuendo, I read into SASL authentication a bit ... seems `cnonce' is pretty important: A unique, encoded value that is generated by the client for each challenge response, and that is used to avoid chosen plaintext attacks, provides some message integrity protection, and provides mutual authentication. This authentication is provided mutually in that the server proves it knows the user’s secret information and not in that the server proves its identity. The nonce must be specified if a QOP directive is sent. Am I dead wrong, or are you hand-waving to defend your product?
- amouat 13y agoI notice they've continued their defense in an update to the blog post: "One more small note: Much has been said about a line of code in our XMPP library that supposedly is a sign of bad practice — this line is not used for anything security-sensitive. It is not a security weakness. It came as part of the third-party XMPP library that Cryptocat uses."