Created attachment 408585 [details] [diff] [review] Possible fix See bug 520386, comment 15. Kai, what's the best way to test this patch? I see /security/nss/cmd/crmf-cgi/crmfcgi.html -- but it isn't entirely clear how to use it.
Hi, I can help test on dogtag certificate system (http://pki.fedoraproject.org) If someone can provide a test build of firefox.
Builds can be found at http://email@example.com .
kashyap chamarthy, the test builds have a limited lifetime, I guess you didn't test them. I apologize that testing hasn't happened yet. Blake, maybe you can use http://kuix.de/misc/test3/crmf-rsa.php for testing. If you click the button, you should get content inserted into the page. Paste that content to a file named bla. Then, Linux, use the following command to pretty print the blob: cat bla | sed 's# #\n#g' | openssl base64 -d | openssl asn1parse -inform DER On my system, this produced a result of 0:d=0 hl=4 l= 415 cons: SEQUENCE 4:d=1 hl=4 l= 411 cons: SEQUENCE 8:d=2 hl=4 l= 257 cons: SEQUENCE 12:d=3 hl=2 l= 4 prim: INTEGER :3BA0B5EB 18:d=3 hl=3 l= 208 cons: SEQUENCE 21:d=4 hl=2 l= 1 prim: cont [ 0 ] 24:d=4 hl=2 l= 23 cons: cont [ 5 ] 26:d=5 hl=2 l= 21 cons: SEQUENCE 28:d=6 hl=2 l= 19 cons: SET 30:d=7 hl=2 l= 17 cons: SEQUENCE 32:d=8 hl=2 l= 3 prim: OBJECT :commonName 37:d=8 hl=2 l= 10 prim: PRINTABLESTRING :Kai Engert 49:d=4 hl=3 l= 159 cons: cont [ 6 ] 52:d=5 hl=2 l= 13 cons: SEQUENCE 54:d=6 hl=2 l= 9 prim: OBJECT :rsaEncryption 65:d=6 hl=2 l= 0 prim: NULL 67:d=5 hl=3 l= 141 prim: BIT STRING 211:d=4 hl=2 l= 16 cons: cont [ 9 ] 213:d=5 hl=2 l= 14 cons: SEQUENCE 215:d=6 hl=2 l= 3 prim: OBJECT :X509v3 Key Usage 220:d=6 hl=2 l= 1 prim: BOOLEAN :255 223:d=6 hl=2 l= 4 prim: OCTET STRING [HEX DUMP]:030205E0 229:d=3 hl=2 l= 38 cons: SEQUENCE 231:d=4 hl=2 l= 17 cons: SEQUENCE 233:d=5 hl=2 l= 9 prim: OBJECT :id-regCtrl-regToken 244:d=5 hl=2 l= 4 prim: UTF8STRING :bla1 250:d=4 hl=2 l= 17 cons: SEQUENCE 252:d=5 hl=2 l= 9 prim: OBJECT :id-regCtrl-authenticator 263:d=5 hl=2 l= 4 prim: UTF8STRING :bla2 269:d=2 hl=3 l= 147 cons: cont [ 1 ] 272:d=3 hl=2 l= 13 cons: SEQUENCE 274:d=4 hl=2 l= 9 prim: OBJECT :sha1WithRSAEncryption 285:d=4 hl=2 l= 0 prim: NULL 287:d=3 hl=3 l= 129 prim: BIT STRING If, after your change, you still get content that produce similar output, your patch is fine.
Kai, Apologies. I couldn't get to this at the time the builds arrived, as I was dragged into a more higher priority task and haven't had spare cycles. And, after that, this issue slipped my mind. If it's possible to provide another test build, I'd be happy to test it out on Dogtag certificate system. PS: FWIW, I tried out the instructions from comment #3 and I got a similar output on my firefox version 3.5.9 (on Fedora12)
Brian, I think I mentioned this bug to you a while back. It would be nice to get it in (and tested, which was the reason I never managed to check this in) to help reduce the number of places that we manually execute JS code from Gecko. Would you be willing to drive it?
This bug isn't blocking any other bug and it isn't clear if/why it is a high priority on its own. The summary proposes a solution without stating the problem, and the problem is likely non-obvious to all PSM people. What is the actual problem?
This used to be higher priority. Now, I think it's more "nice to have." At the time that I filed this, it was possible to cause us to navigate while generateCRMFRequest was running, which could have caused us to crash (in particular, we would call JS_ClearScope when navigating a window, causing references to global variables to become reads and writes past the end of an array). At the time, bug 520386 fixed the problem by adding state to nsIScriptContext::EvaluateString that caused us to not JS_ClearScope until we were truly ready for the JS_ClearScope (after the script had stopped running). Therefore, having code that didn't go through the script context could have still crashed. We've since removed the JS_ClearScope, so the only real gain from this patch is simplifying the code and reducing the number of places in Gecko where we touch the JS engine directly (which, IMO, is always a good thing).
Bug 1030963 removed window.crypto functions/properties, which includes nsCrypto::GenerateCRMFRequest(): https://hg.mozilla.org/mozilla-central/diff/68499003df5e/security/manager/ssl/src/nsCrypto.cpp So this is now "invalid", or whatever status makes most sense.
When I bug is made invalid because another bug made it irrelevant, it is good to add a dependency. That way, if that other bug fix gets undone, we know to reopen the dependent bugs.