To verify downloaded app content (bug 1197720) we would like to use SHA-256. Using Java's crypto framework has caused noticeable startup penalties (see bug bug 959652 and linked bugs). Therefore we would like to extend NativeCrypto to support SHA-256. This will be used to calculate hashes of files, so we would like to avoid reading the whole file into memory. Instead the native code should read the file itself or accept byte chunks (see ByteBuffer to avoid overhead). Additionally we should extend testNativeCrypto to cover SHA-256 too.

I'm not experienced enough in C++ to just add this. I'm open to learn but I'll need some hints. @Michael: I saw you added SHA-1 support to NativeCrypto. Do you know what's needed to add sha256 support?

(In reply to Sebastian Kaspari (:sebastian) from comment #1) > @Michael: I saw you added SHA-1 support to NativeCrypto. Do you know what's > needed to add sha256 support? I am actually not that familiar with C++ or the build goo – that was a bug much like this one, I guess! :P I originally implemented it in bug 915312, and the comments might document some of my journey (because I unfortunately can't remember most of it!). However, this bug doesn't entirely look carbon-copy. (I'll reference patch parts from bug 915312 in parens) It looks like you may be able to import sha256 from ./ipc/ [1] or NSS (see [2]). If not, see if you can find where the imports in the existing C++ code is coming from [3]. Once you have the libraries, you'll need to make the JNI calls through NativeCrypto.cpp (part 2) and add the appropriate bindings to the Java NativeCrypto (part 3). Best way to test is a good old-fashioned unit test (part 4)! :) If you have questions, feel free to ask – I might remember a bit more. [1]: http://mxr.mozilla.org/mozilla-central/source/ipc/app/sha256.h [2]: http://mxr.mozilla.org/mozilla-central/find?text=&string=sha256 [3]: https://mxr.mozilla.org/mozilla-central/search?string=sha256&find=\.cpp%24&findi=\.xul%24&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

Hey platform people! I wonder if this is easy to add for you. I'd rather like to have it sooner than learning writing it myself by letting someone review a series of crappy patches. :)

I see that we do appear to have an implementation[0] of SHA256 in the tree, so this would be fairly straightforward to implement. Dylan, do you want to take a crack at it? [0] https://dxr.mozilla.org/mozilla-central/source/ipc/app/sha256.h

Proposed patch. Jim, I flagged you for review because I think there's some JNI code generation weirdness happening here: the changes I made to NativeCrypto.java should result in a new NativeCrypto.h being generated with a prototype for the sha256 function, right? That's not happening, but this is somehow still building fine and passing tests that use NativeCrypto.sha256.

Now with manually generated NativeCrypto.h

Hey Dylan! Thanks for the patches. :) I saw that the method takes a byte[] array. We want to use it to hash files. Do you think it would be possible to pass byte[] chunks (Or maybe better: ByteBuffer) to it (like in the CPP version)? Alternatively we could pass the file path to CPP and do the file handling there? Whatever is easier/better. I would like to avoid keeping the whole file as byte array in the memory and copying to the native layer.

Sebastian, I think I can more directly expose the SHA256_Init/Update/Final functions to Java, then you could just pass byte[] chunks or possibly a ByteBuffer to SHA256_Update. I'll look into that and post an updated patch.

(In reply to Dylan Roeh (:droeh) from comment #9) > I think I can more directly expose the SHA256_Init/Update/Final functions to > Java, then you could just pass byte[] chunks or possibly a ByteBuffer to > SHA256_Update. I'll look into that and post an updated patch. Awesome! Thank you. :)

Updated to more directly expose SHA-256 init/update/finalize functions.

Comment on attachment 8689024 [details] [diff] [review] bug-1221040-fix-2.patch Review of attachment 8689024 [details] [diff] [review]: ----------------------------------------------------------------- The byte-array-as-SHA256_CTX thing seems a little crazy, but I guess I'm ok with it. The alternative would be to allocate the ctx on the C++ heap and pass a 'long long' pointer around. Looks good if you avoid the copy in initialization ::: mozglue/android/NativeCrypto.cpp @@ +83,5 @@ > + SHA256_Init(&shaContext); > + > + jbyteArray out = env->NewByteArray(sizeof(SHA256_CTX)); > + if (nullptr != out) { > + env->SetByteArrayRegion(out, 0, sizeof(SHA256_CTX), (jbyte*)&shaContext); You can use GetDirectAddress to avoid a copy here, and init the shaContext with that memory instead.

Updated to avoid a copy in sha256init; carrying over snorp's r+.