From 4b183f77b21f209ccca19c829c760fe150498fc8 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 25 Jun 2019 06:48:41 +0000 Subject: [PATCH] Bug 1350254 part 8. Switch CryptoKey to [Serializable]. r=baku The spec doesn't say to do this, but I think we should. See https://github.com/w3c/webcrypto/issues/222 Differential Revision: https://phabricator.services.mozilla.com/D35722 --HG-- extra : moz-landing-system : lando --- dom/base/StructuredCloneHolder.cpp | 25 +++------------------ dom/base/StructuredCloneTags.h | 2 +- dom/crypto/CryptoKey.cpp | 36 +++++++++++++++++------------- dom/crypto/CryptoKey.h | 7 ++++-- dom/webidl/SubtleCrypto.webidl | 1 + 5 files changed, 31 insertions(+), 40 deletions(-) diff --git a/dom/base/StructuredCloneHolder.cpp b/dom/base/StructuredCloneHolder.cpp index 40ed5a117c32..b0b201557921 100644 --- a/dom/base/StructuredCloneHolder.cpp +++ b/dom/base/StructuredCloneHolder.cpp @@ -10,7 +10,6 @@ #include "mozilla/AutoRestore.h" #include "mozilla/dom/BindingUtils.h" #include "mozilla/dom/BlobBinding.h" -#include "mozilla/dom/CryptoKey.h" #include "mozilla/dom/StructuredCloneBlob.h" #include "mozilla/dom/Directory.h" #include "mozilla/dom/DirectoryBinding.h" @@ -28,11 +27,9 @@ #include "mozilla/dom/StructuredCloneTags.h" #include "mozilla/dom/StructuredCloneTester.h" #include "mozilla/dom/StructuredCloneTesterBinding.h" -#include "mozilla/dom/SubtleCryptoBinding.h" #include "mozilla/dom/ToJSValue.h" #include "mozilla/dom/URLSearchParams.h" #include "mozilla/dom/URLSearchParamsBinding.h" -#include "mozilla/dom/WebCryptoCommon.h" #include "mozilla/dom/WebIDLSerializable.h" #include "mozilla/gfx/2D.h" #include "mozilla/ipc/BackgroundChild.h" @@ -123,7 +120,7 @@ void AssertTagValues() { static_assert(SCTAG_DOM_IMAGEDATA == 0xffff8007 && SCTAG_DOM_DOMPOINT == 0xffff8008 && SCTAG_DOM_DOMPOINTREADONLY == 0xffff8009 && - SCTAG_DOM_WEBCRYPTO_KEY == 0xffff800a && + SCTAG_DOM_CRYPTOKEY == 0xffff800a && SCTAG_DOM_NULL_PRINCIPAL == 0xffff800b && SCTAG_DOM_SYSTEM_PRINCIPAL == 0xffff800c && SCTAG_DOM_CONTENT_PRINCIPAL == 0xffff800d && @@ -351,18 +348,11 @@ JSObject* StructuredCloneHolder::ReadFullySerializableObjects( return deserializer(aCx, global, aReader); } - if (aTag == SCTAG_DOM_WEBCRYPTO_KEY || aTag == SCTAG_DOM_URLSEARCHPARAMS) { + if (aTag == SCTAG_DOM_URLSEARCHPARAMS) { // Prevent the return value from being trashed by a GC during ~nsRefPtr. JS::Rooted result(aCx); { - if (aTag == SCTAG_DOM_WEBCRYPTO_KEY) { - RefPtr key = new CryptoKey(global); - if (!key->ReadStructuredClone(aReader)) { - result = nullptr; - } else { - result = key->WrapObject(aCx, nullptr); - } - } else if (aTag == SCTAG_DOM_URLSEARCHPARAMS) { + if (aTag == SCTAG_DOM_URLSEARCHPARAMS) { RefPtr usp = new URLSearchParams(global); if (!usp->ReadStructuredClone(aReader)) { result = nullptr; @@ -459,15 +449,6 @@ bool StructuredCloneHolder::WriteFullySerializableObjects( } } - // Handle Key cloning - { - CryptoKey* key = nullptr; - if (NS_SUCCEEDED(UNWRAP_OBJECT(CryptoKey, &obj, key))) { - return JS_WriteUint32Pair(aWriter, SCTAG_DOM_WEBCRYPTO_KEY, 0) && - key->WriteStructuredClone(aWriter); - } - } - #ifdef MOZ_WEBRTC { // Handle WebRTC Certificate cloning diff --git a/dom/base/StructuredCloneTags.h b/dom/base/StructuredCloneTags.h index a560cda8f8c6..703142399ae9 100644 --- a/dom/base/StructuredCloneTags.h +++ b/dom/base/StructuredCloneTags.h @@ -51,7 +51,7 @@ enum StructuredCloneTags { // IMPORTANT: Don't change the order of these enum values. You could break // IDB. // This tag is for WebCrypto keys - SCTAG_DOM_WEBCRYPTO_KEY, + SCTAG_DOM_CRYPTOKEY, // IMPORTANT: Don't change the order of these enum values. You could break // IDB. diff --git a/dom/crypto/CryptoKey.cpp b/dom/crypto/CryptoKey.cpp index e2bc6e1eddb7..6ef5730cf4e5 100644 --- a/dom/crypto/CryptoKey.cpp +++ b/dom/crypto/CryptoKey.cpp @@ -1138,7 +1138,8 @@ bool CryptoKey::PublicKeyValid(SECKEYPublicKey* aPubKey) { return (rv == SECSuccess); } -bool CryptoKey::WriteStructuredClone(JSStructuredCloneWriter* aWriter) const { +bool CryptoKey::WriteStructuredClone(JSContext* aCX, + JSStructuredCloneWriter* aWriter) const { // Write in five pieces // 1. Attributes // 2. Symmetric key as raw (if present) @@ -1164,42 +1165,47 @@ bool CryptoKey::WriteStructuredClone(JSStructuredCloneWriter* aWriter) const { WriteBuffer(aWriter, pub) && mAlgorithm.WriteStructuredClone(aWriter); } -bool CryptoKey::ReadStructuredClone(JSStructuredCloneReader* aReader) { +// static +already_AddRefed CryptoKey::ReadStructuredClone( + JSContext* aCx, nsIGlobalObject* aGlobal, + JSStructuredCloneReader* aReader) { // Ensure that NSS is initialized. if (!EnsureNSSInitializedChromeOrContent()) { - return false; + return nullptr; } + RefPtr key = new CryptoKey(aGlobal); + uint32_t version; CryptoBuffer sym, priv, pub; - bool read = JS_ReadUint32Pair(aReader, &mAttributes, &version) && + bool read = JS_ReadUint32Pair(aReader, &key->mAttributes, &version) && (version == CRYPTOKEY_SC_VERSION) && ReadBuffer(aReader, sym) && ReadBuffer(aReader, priv) && ReadBuffer(aReader, pub) && - mAlgorithm.ReadStructuredClone(aReader); + key->mAlgorithm.ReadStructuredClone(aReader); if (!read) { - return false; + return nullptr; } - if (sym.Length() > 0 && !mSymKey.Assign(sym)) { - return false; + if (sym.Length() > 0 && !key->mSymKey.Assign(sym)) { + return nullptr; } if (priv.Length() > 0) { - mPrivateKey = CryptoKey::PrivateKeyFromPkcs8(priv); + key->mPrivateKey = CryptoKey::PrivateKeyFromPkcs8(priv); } if (pub.Length() > 0) { - mPublicKey = CryptoKey::PublicKeyFromSpki(pub); + key->mPublicKey = CryptoKey::PublicKeyFromSpki(pub); } // Ensure that what we've read is consistent // If the attributes indicate a key type, should have a key of that type - if (!((GetKeyType() == SECRET && mSymKey.Length() > 0) || - (GetKeyType() == PRIVATE && mPrivateKey) || - (GetKeyType() == PUBLIC && mPublicKey))) { - return false; + if (!((key->GetKeyType() == SECRET && key->mSymKey.Length() > 0) || + (key->GetKeyType() == PRIVATE && key->mPrivateKey) || + (key->GetKeyType() == PUBLIC && key->mPublicKey))) { + return nullptr; } - return true; + return key.forget(); } } // namespace dom diff --git a/dom/crypto/CryptoKey.h b/dom/crypto/CryptoKey.h index de3722f9d6bf..bd1cb2e8117b 100644 --- a/dom/crypto/CryptoKey.h +++ b/dom/crypto/CryptoKey.h @@ -164,8 +164,11 @@ class CryptoKey final : public nsISupports, public nsWrapperCache { static bool PublicKeyValid(SECKEYPublicKey* aPubKey); // Structured clone methods use these to clone keys - bool WriteStructuredClone(JSStructuredCloneWriter* aWriter) const; - bool ReadStructuredClone(JSStructuredCloneReader* aReader); + bool WriteStructuredClone(JSContext* aCx, + JSStructuredCloneWriter* aWriter) const; + static already_AddRefed ReadStructuredClone( + JSContext* aCx, nsIGlobalObject* aGlobal, + JSStructuredCloneReader* aReader); private: ~CryptoKey() {} diff --git a/dom/webidl/SubtleCrypto.webidl b/dom/webidl/SubtleCrypto.webidl index 98e6bf090b33..e0eae2115996 100644 --- a/dom/webidl/SubtleCrypto.webidl +++ b/dom/webidl/SubtleCrypto.webidl @@ -152,6 +152,7 @@ dictionary JsonWebKey { /***** The Main API *****/ +[Serializable] interface CryptoKey { readonly attribute KeyType type; readonly attribute boolean extractable;