Closed Bug 1311379 Opened 3 years ago Closed 3 years ago

Stop using Scoped.h NSS types in WebCryptoTask.(cpp|h)

Categories

(Core :: Security: PSM, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: Cykesiopka, Assigned: Cykesiopka)

Details

(Whiteboard: [psm-assigned])

Attachments

(1 file)

Scoped.h is deprecated.
Priority: -- → P1
Whiteboard: [psm-assigned]
I guess WebCrypto should be under PSM now.
Component: Security → Security: PSM
Tim, would you be able to review this?
Flags: needinfo?(ttaubert)
Comment on attachment 8802539 [details]
Bug 1311379 - Stop using Scoped.h NSS types in WebCryptoTask.(cpp|h).

https://reviewboard.mozilla.org/r/86926/#review93436

LGTM. Thanks!
Attachment #8802539 - Flags: review+
Flags: needinfo?(ttaubert)
Comment on attachment 8802539 [details]
Bug 1311379 - Stop using Scoped.h NSS types in WebCryptoTask.(cpp|h).

https://reviewboard.mozilla.org/r/86926/#review93738

::: dom/crypto/WebCryptoTask.cpp:1805
(Diff revision 1)
>          (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK) &&
>           mJwk.mD.WasPassed())) {
>        // Private key import
>        if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_PKCS8)) {
> -        privKey = CryptoKey::PrivateKeyFromPkcs8(mKeyData, locker);
> +        privKey = UniqueSECKEYPrivateKey(
> +          CryptoKey::PrivateKeyFromPkcs8(mKeyData, locker));

This wrapping is pretty unfortunate.  Could you avoid by just having PrivateKeyFromPkcs8 return UniqueSECKEYPrivateKey?

::: dom/crypto/WebCryptoTask.cpp:1950
(Diff revision 1)
>                 (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_JWK) &&
>                  !mJwk.mD.WasPassed())) {
>        // Public key import
>        if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_RAW)) {
> -        pubKey = CryptoKey::PublicECKeyFromRaw(mKeyData, mNamedCurve, locker);
> +        pubKey = UniqueSECKEYPublicKey(
> +          CryptoKey::PublicECKeyFromRaw(mKeyData, mNamedCurve, locker));

Same comment about wrapping here.

::: dom/crypto/WebCryptoTask.cpp:2091
(Diff revision 1)
>      if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_RAW) ||
>          mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_SPKI)) {
>        // Public key import
>        if (mFormat.EqualsLiteral(WEBCRYPTO_KEY_FORMAT_RAW)) {
> -        pubKey = CryptoKey::PublicDhKeyFromRaw(mKeyData, mPrime, mGenerator, locker);
> +        pubKey = UniqueSECKEYPublicKey(
> +          CryptoKey::PublicDhKeyFromRaw(mKeyData, mPrime, mGenerator, locker));

Ditto
Comment on attachment 8802539 [details]
Bug 1311379 - Stop using Scoped.h NSS types in WebCryptoTask.(cpp|h).

https://reviewboard.mozilla.org/r/86926/#review93744
Attachment #8802539 - Flags: review?(rlb) → review+
Comment on attachment 8802539 [details]
Bug 1311379 - Stop using Scoped.h NSS types in WebCryptoTask.(cpp|h).

https://reviewboard.mozilla.org/r/86926/#review93738

> This wrapping is pretty unfortunate.  Could you avoid by just having PrivateKeyFromPkcs8 return UniqueSECKEYPrivateKey?

Yes, but I'm going to defer doing this since it's easier to get rid of these wrappings when I get rid of Scoped.h usage in CryptoKey.(cpp|h).
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/01aafb2359c9
Stop using Scoped.h NSS types in WebCryptoTask.(cpp|h). r=rbarnes,ttaubert
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/01aafb2359c9
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.