asmcrypto / asmcrypto.js

JavaScript Cryptographic Library with performance in mind.
MIT License
659 stars 182 forks source link

Use of location.protocol stops inclusion in non-browser commonjs #100

Closed ndastur closed 7 years ago

ndastur commented 8 years ago

I am including the file in an appcelerator project but if fails with: Message: Uncaught TypeError: Cannot read property 'protocol' of undefined [ERROR] TiExceptionHandler: (main) [0,190] - Source: = b.Float64Array || b.Float32Array, Vb = b.console, Wb = !b.location.protocol.

I tried defining var document.location.protocol but it didn't seem to help

vibornoff commented 8 years ago

That comes from src/origin.js:3. I'm not familiar with appcelerator runtime in order to implement a way to detect it. Could you please contribute a bit?

ndastur commented 8 years ago

Thanks for prompt response. The Appc runtime doesn't have a DOM. I.e. not browser based. I think is mainly a "not found" problem so could this pattern help?

!(global || {}).(location || {}).(protocol || {}).search( /https:|file:|chrome:|chrome-extension:/ );

BTW where is the global object defined?

vibornoff commented 8 years ago

Nope, this pattern will cause "asmCrypto seems to be load from an insecure origin" spam.

Appc loads scripts from local filesystem and hence it is secure (no MiTM attack may be performed) and we need a way to reliably detect Appc and set _secure_origin variable to true.

ndastur commented 8 years ago

Okay, I see that. There is a Titanium global object that is always defined. Would checking for it's presence be enough?

vibornoff commented 8 years ago

Does it have any meaningful attribues? Maybe global.navigator?

ndastur commented 8 years ago

Yes, but none that are constant between apps. http://docs.appcelerator.com/platform/latest/#!/api/Titanium.Platform-property-runtime

Wonder if could just check for the Titanium object

vibornoff commented 8 years ago

Titanium.version is readonly and look suitable. Could you please verify the patch?

diff --git a/src/origin.js b/src/origin.js
index 798570d..4003ff3 100644
--- a/src/origin.js
+++ b/src/origin.js
@@ -1,6 +1,13 @@
 var _global_console = global.console;

-var _secure_origin = !global.location.protocol.search( /https:|file:|chrome:|chrome-extension:/ );
+var _secure_origin = false;
+
+if ( global.Titanium !== undefined && global.Titanium.version ) { // inside Appcelerator runtime?
+    _secure_origin = true;
+}
+else {
+    _secure_origin = !global.location.protocol.search( /https:|file:|chrome:|chrome-extension:/ );
+}

 if ( !_secure_origin && _global_console !== undefined ) {
     _global_console.warn("asmCrypto seems to be load from an insecure origin; this may cause to MitM-attack vulnerability. Consider using secure transport protocol.");
ndastur commented 8 years ago

The patch does fix that error but now in random/random.js reference is made to location.href which throws an error. I can't obviously see why it is used in the function generate on line 65

vibornoff commented 8 years ago

Seems Appc lacks of crypto.getRandomValues API. So there is no high-quality source of random bits for generating keys.

Such a situation causes asmCrypto to get random bits from its own PRNG seeded with low-quality material combined from Math.rangom(), Date.now(), performance.now(), location.href and so on...

Fortunately you may ask your user to "shake" his device in order to get some random bits, estimate the amount of Shannon Entropy of collected "shake"-samples, and then seed the PRNG with that data.

Unfortunately you have to implement entropy collector from the ground as my another project entropy-collector.js lacks support of mobile devices.

Also you have to guarantee that there will be no keys generated before PRNG seeding with reasonable amount of entropy is complete to turn off "opportunistic weak seeding" at src/random/random.js:149. The last is absolutely needed on mobile devices as "weak seeding" is very-very CPU-hungry and may "freeze" your device for a long period wasting battery charge.