479341 - make WeaveCrypto::GenerateKeypair() call PK11_GenerateKeyPairWithOpFlags()

Status: RESOLVED FIXED

(Cloud Services :: General, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

According to dolske in bug 476539, comment 9: "You might be able work around [a test failure on PK11_GenerateKeyPair() with the XULRunner-based test harness in bug 476539] by having WeaveCrypto::GenerateKeypair()
call PK11_GenerateKeyPairWithOpFlags() instead of PK11_GenerateKeyPair()... The
latter generates a key with CKA_PRIVATE set, which requires the token to be
logged into. Weave doesn't need that, it's only using this API (instead of the
WithOpFlags flavor) because it's a bit simpler simpler to use."

Assigning this bug per bug 476539, comment 10.

Comment 1

[Dan Mills [:thunder]]

mmmmm bacon :-)

Comment 2

[Justin Dolske [:Dolske]]

Yeah, sorry, forgot about this.

Comment 3

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

Well, I feel a little silly... I remembered this being as a tedious problem that required building up an object template by hand, but it's actually a fairly trivial fix.

This fixes test_crypto_keypair, but not test_records_crypto...

That fails with:

*** CHECK FAILED: TypeError: crypto.generateRandomKey is not a function

Which is a little odd, because it certainly is a function. Looks like the problem is that something's redefining |crypto| after it's initially assigned. If I dump it right before the error it's "[object Crypto]". But if I assign it from getService() again, it's the expected "[xpconnect wrapped IWeaveCrypto]". [But then the test fails a little further down with a different problem.]

Not sure exactly what's causing that, but I suspect it's a test problem.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Yup, it's a test problem.  The XULRunner harness runs tests in a window, which has a window.crypto property that cannot be set, so the test throws "TypeError: setting a property that has only a getter" when trying to set it to the IWeaveCrypto service, and |crypto| is not what the test code needs it to be.

I fixed that in the latest version of the harness, and I also fixed one other issue in the Weave code that the new harness reveals.  With the patch in this bug, the latest harness patch in bug 476539 works as expected.  Thanks Justin!

Comment 5

[Justin Dolske [:Dolske]]

Pushed: http://hg.mozilla.org/labs/weave/rev/8fac87daad0f