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

RESOLVED FIXED in Firefox 53

Status

()

Core
Security: PSM
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Cykesiopka, Assigned: Cykesiopka)

Tracking

unspecified
mozilla53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: [psm-assigned])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Scoped.h is deprecated.
Comment hidden (mozreview-request)
Priority: -- → P1
Whiteboard: [psm-assigned]
(Assignee)

Comment 2

2 years ago
I guess WebCrypto should be under PSM now.
Component: Security → Security: PSM
Tim, would you be able to review this?
Flags: needinfo?(ttaubert)

Comment 4

2 years ago
mozreview-review
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 5

2 years ago
mozreview-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

::: 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 6

2 years ago
mozreview-review
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+
(Assignee)

Comment 7

2 years ago
mozreview-review-reply
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).
Comment hidden (mozreview-request)
(Assignee)

Comment 9

2 years ago
Thanks for the reviews!

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1eb48fefe2dc638a02d1d4004adfc942145e2c05
Keywords: checkin-needed

Comment 10

2 years ago
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

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/01aafb2359c9
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.