Skip to content

Commit b3996e4

Browse files
jcjonesjcjones
committed
Bug 1384623 - WebAuthn [SameObject] attributes must cache those objects r=qdot
Peter points out [1] that I made assumptions that [SameObject] would handle caching at the JS-layer, but it does not. This bug is to cache those objects [2] on the heap, and add tests that they are indeed the same. [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1382888#c6 [2] https://hg.mozilla.org/mozilla-central/rev/811510fdb51a MozReview-Commit-ID: KQySNAOnyeE --HG-- extra : rebase_source : 8422e9e8eafacc1071191a00d49bc85797571ebe
1 parent 4684293 commit b3996e4

9 files changed

+140
-29
lines changed

‎dom/webauthn/AuthenticatorAssertionResponse.cpp‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,24 @@
1010
namespace mozilla {
1111
namespace dom {
1212

13+
NS_IMPL_CYCLE_COLLECTION_CLASS(AuthenticatorAssertionResponse)
14+
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(AuthenticatorAssertionResponse,
15+
AuthenticatorResponse)
16+
tmp->mAuthenticatorDataCachedObj = nullptr;
17+
tmp->mSignatureCachedObj = nullptr;
18+
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
19+
20+
NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(AuthenticatorAssertionResponse,
21+
AuthenticatorResponse)
22+
NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
23+
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mAuthenticatorDataCachedObj)
24+
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mSignatureCachedObj)
25+
NS_IMPL_CYCLE_COLLECTION_TRACE_END
26+
27+
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(AuthenticatorAssertionResponse,
28+
AuthenticatorResponse)
29+
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
30+
1331
NS_IMPL_ADDREF_INHERITED(AuthenticatorAssertionResponse, AuthenticatorResponse)
1432
NS_IMPL_RELEASE_INHERITED(AuthenticatorAssertionResponse, AuthenticatorResponse)
1533

@@ -18,10 +36,16 @@ NS_INTERFACE_MAP_END_INHERITING(AuthenticatorResponse)
1836

1937
AuthenticatorAssertionResponse::AuthenticatorAssertionResponse(nsPIDOMWindowInner* aParent)
2038
: AuthenticatorResponse(aParent)
21-
{}
39+
, mAuthenticatorDataCachedObj(nullptr)
40+
, mSignatureCachedObj(nullptr)
41+
{
42+
mozilla::HoldJSObjects(this);
43+
}
2244

2345
AuthenticatorAssertionResponse::~AuthenticatorAssertionResponse()
24-
{}
46+
{
47+
mozilla::DropJSObjects(this);
48+
}
2549

2650
JSObject*
2751
AuthenticatorAssertionResponse::WrapObject(JSContext* aCx,
@@ -32,9 +56,12 @@ AuthenticatorAssertionResponse::WrapObject(JSContext* aCx,
3256

3357
void
3458
AuthenticatorAssertionResponse::GetAuthenticatorData(JSContext* aCx,
35-
JS::MutableHandle<JSObject*> aRetVal) const
59+
JS::MutableHandle<JSObject*> aRetVal)
3660
{
37-
aRetVal.set(mAuthenticatorData.ToUint8Array(aCx));
61+
if (!mAuthenticatorDataCachedObj) {
62+
mAuthenticatorDataCachedObj = mAuthenticatorData.ToUint8Array(aCx);
63+
}
64+
aRetVal.set(mAuthenticatorDataCachedObj);
3865
}
3966

4067
nsresult
@@ -48,9 +75,12 @@ AuthenticatorAssertionResponse::SetAuthenticatorData(CryptoBuffer& aBuffer)
4875

4976
void
5077
AuthenticatorAssertionResponse::GetSignature(JSContext* aCx,
51-
JS::MutableHandle<JSObject*> aRetVal) const
78+
JS::MutableHandle<JSObject*> aRetVal)
5279
{
53-
aRetVal.set(mSignature.ToUint8Array(aCx));
80+
if (!mSignatureCachedObj) {
81+
mSignatureCachedObj = mSignature.ToUint8Array(aCx);
82+
}
83+
aRetVal.set(mSignatureCachedObj);
5484
}
5585

5686
nsresult

‎dom/webauthn/AuthenticatorAssertionResponse.h‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ class AuthenticatorAssertionResponse final : public AuthenticatorResponse
2323
{
2424
public:
2525
NS_DECL_ISUPPORTS_INHERITED
26+
NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED(AuthenticatorAssertionResponse,
27+
AuthenticatorResponse)
2628

2729
explicit AuthenticatorAssertionResponse(nsPIDOMWindowInner* aParent);
2830

@@ -34,20 +36,22 @@ class AuthenticatorAssertionResponse final : public AuthenticatorResponse
3436
WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
3537

3638
void
37-
GetAuthenticatorData(JSContext* aCx, JS::MutableHandle<JSObject*> aRetVal) const;
39+
GetAuthenticatorData(JSContext* aCx, JS::MutableHandle<JSObject*> aRetVal);
3840

3941
nsresult
4042
SetAuthenticatorData(CryptoBuffer& aBuffer);
4143

4244
void
43-
GetSignature(JSContext* aCx, JS::MutableHandle<JSObject*> aRetVal) const;
45+
GetSignature(JSContext* aCx, JS::MutableHandle<JSObject*> aRetVal);
4446

4547
nsresult
4648
SetSignature(CryptoBuffer& aBuffer);
4749

4850
private:
4951
CryptoBuffer mAuthenticatorData;
52+
JS::Heap<JSObject*> mAuthenticatorDataCachedObj;
5053
CryptoBuffer mSignature;
54+
JS::Heap<JSObject*> mSignatureCachedObj;
5155
};
5256

5357
} // namespace dom

‎dom/webauthn/AuthenticatorAttestationResponse.cpp‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,22 @@
1010
namespace mozilla {
1111
namespace dom {
1212

13+
NS_IMPL_CYCLE_COLLECTION_CLASS(AuthenticatorAttestationResponse)
14+
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(AuthenticatorAttestationResponse,
15+
AuthenticatorResponse)
16+
tmp->mAttestationObjectCachedObj = nullptr;
17+
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
18+
19+
NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(AuthenticatorAttestationResponse,
20+
AuthenticatorResponse)
21+
NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
22+
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mAttestationObjectCachedObj)
23+
NS_IMPL_CYCLE_COLLECTION_TRACE_END
24+
25+
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(AuthenticatorAttestationResponse,
26+
AuthenticatorResponse)
27+
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
28+
1329
NS_IMPL_ADDREF_INHERITED(AuthenticatorAttestationResponse, AuthenticatorResponse)
1430
NS_IMPL_RELEASE_INHERITED(AuthenticatorAttestationResponse, AuthenticatorResponse)
1531

@@ -18,10 +34,15 @@ NS_INTERFACE_MAP_END_INHERITING(AuthenticatorResponse)
1834

1935
AuthenticatorAttestationResponse::AuthenticatorAttestationResponse(nsPIDOMWindowInner* aParent)
2036
: AuthenticatorResponse(aParent)
21-
{}
37+
, mAttestationObjectCachedObj(nullptr)
38+
{
39+
mozilla::HoldJSObjects(this);
40+
}
2241

2342
AuthenticatorAttestationResponse::~AuthenticatorAttestationResponse()
24-
{}
43+
{
44+
mozilla::DropJSObjects(this);
45+
}
2546

2647
JSObject*
2748
AuthenticatorAttestationResponse::WrapObject(JSContext* aCx,
@@ -32,9 +53,12 @@ AuthenticatorAttestationResponse::WrapObject(JSContext* aCx,
3253

3354
void
3455
AuthenticatorAttestationResponse::GetAttestationObject(JSContext* aCx,
35-
JS::MutableHandle<JSObject*> aRetVal) const
56+
JS::MutableHandle<JSObject*> aRetVal)
3657
{
37-
aRetVal.set(mAttestationObject.ToUint8Array(aCx));
58+
if (!mAttestationObjectCachedObj) {
59+
mAttestationObjectCachedObj = mAttestationObject.ToUint8Array(aCx);
60+
}
61+
aRetVal.set(mAttestationObjectCachedObj);
3862
}
3963

4064
nsresult

‎dom/webauthn/AuthenticatorAttestationResponse.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ class AuthenticatorAttestationResponse final : public AuthenticatorResponse
2323
{
2424
public:
2525
NS_DECL_ISUPPORTS_INHERITED
26+
NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_INHERITED(AuthenticatorAttestationResponse,
27+
AuthenticatorResponse)
2628

2729
explicit AuthenticatorAttestationResponse(nsPIDOMWindowInner* aParent);
2830

@@ -34,13 +36,14 @@ class AuthenticatorAttestationResponse final : public AuthenticatorResponse
3436
WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
3537

3638
void
37-
GetAttestationObject(JSContext* aCx, JS::MutableHandle<JSObject*> aRetVal) const;
39+
GetAttestationObject(JSContext* aCx, JS::MutableHandle<JSObject*> aRetVal);
3840

3941
nsresult
4042
SetAttestationObject(CryptoBuffer& aBuffer);
4143

4244
private:
4345
CryptoBuffer mAttestationObject;
46+
JS::Heap<JSObject*> mAttestationObjectCachedObj;
4447
};
4548

4649
} // namespace dom

‎dom/webauthn/AuthenticatorResponse.cpp‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,41 @@
1010
namespace mozilla {
1111
namespace dom {
1212

13-
// Only needed for refcounted objects.
14-
NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(AuthenticatorResponse, mParent)
13+
NS_IMPL_CYCLE_COLLECTION_CLASS(AuthenticatorResponse)
14+
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(AuthenticatorResponse)
15+
NS_IMPL_CYCLE_COLLECTION_UNLINK(mParent)
16+
NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
17+
tmp->mClientDataJSONCachedObj = nullptr;
18+
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
19+
20+
NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(AuthenticatorResponse)
21+
NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
22+
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mClientDataJSONCachedObj)
23+
NS_IMPL_CYCLE_COLLECTION_TRACE_END
24+
25+
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(AuthenticatorResponse)
26+
NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mParent)
27+
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
28+
1529
NS_IMPL_CYCLE_COLLECTING_ADDREF(AuthenticatorResponse)
1630
NS_IMPL_CYCLE_COLLECTING_RELEASE(AuthenticatorResponse)
31+
1732
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(AuthenticatorResponse)
1833
NS_WRAPPERCACHE_INTERFACE_MAP_ENTRY
1934
NS_INTERFACE_MAP_ENTRY(nsISupports)
2035
NS_INTERFACE_MAP_END
2136

2237
AuthenticatorResponse::AuthenticatorResponse(nsPIDOMWindowInner* aParent)
2338
: mParent(aParent)
24-
{}
39+
, mClientDataJSONCachedObj(nullptr)
40+
{
41+
mozilla::HoldJSObjects(this);
42+
}
2543

2644
AuthenticatorResponse::~AuthenticatorResponse()
27-
{}
45+
{
46+
mozilla::DropJSObjects(this);
47+
}
2848

2949
JSObject*
3050
AuthenticatorResponse::WrapObject(JSContext* aCx,
@@ -35,9 +55,12 @@ AuthenticatorResponse::WrapObject(JSContext* aCx,
3555

3656
void
3757
AuthenticatorResponse::GetClientDataJSON(JSContext* aCx,
38-
JS::MutableHandle<JSObject*> aRetVal) const
58+
JS::MutableHandle<JSObject*> aRetVal)
3959
{
40-
aRetVal.set(mClientDataJSON.ToUint8Array(aCx));
60+
if (!mClientDataJSONCachedObj) {
61+
mClientDataJSONCachedObj = mClientDataJSON.ToUint8Array(aCx);
62+
}
63+
aRetVal.set(mClientDataJSONCachedObj);
4164
}
4265

4366
nsresult

‎dom/webauthn/AuthenticatorResponse.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,15 @@ class AuthenticatorResponse : public nsISupports
4444
GetFormat(nsString& aRetVal) const;
4545

4646
void
47-
GetClientDataJSON(JSContext* aCx, JS::MutableHandle<JSObject*> aRetVal) const;
47+
GetClientDataJSON(JSContext* aCx, JS::MutableHandle<JSObject*> aRetVal);
4848

4949
nsresult
5050
SetClientDataJSON(CryptoBuffer& aBuffer);
5151

5252
private:
5353
nsCOMPtr<nsPIDOMWindowInner> mParent;
5454
CryptoBuffer mClientDataJSON;
55+
JS::Heap<JSObject*> mClientDataJSONCachedObj;
5556
};
5657

5758
} // namespace dom

‎dom/webauthn/PublicKeyCredential.cpp‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,37 @@
1111
namespace mozilla {
1212
namespace dom {
1313

14-
NS_IMPL_CYCLE_COLLECTION_INHERITED(PublicKeyCredential, Credential, mResponse)
14+
NS_IMPL_CYCLE_COLLECTION_CLASS(PublicKeyCredential)
15+
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(PublicKeyCredential,
16+
Credential)
17+
tmp->mRawIdCachedObj = nullptr;
18+
NS_IMPL_CYCLE_COLLECTION_UNLINK_END
19+
20+
NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(PublicKeyCredential, Credential)
21+
NS_IMPL_CYCLE_COLLECTION_TRACE_PRESERVED_WRAPPER
22+
NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mRawIdCachedObj)
23+
NS_IMPL_CYCLE_COLLECTION_TRACE_END
24+
25+
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(PublicKeyCredential, Credential)
26+
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
1527

1628
NS_IMPL_ADDREF_INHERITED(PublicKeyCredential, Credential)
1729
NS_IMPL_RELEASE_INHERITED(PublicKeyCredential, Credential)
1830

1931
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(PublicKeyCredential)
2032
NS_INTERFACE_MAP_END_INHERITING(Credential)
2133

22-
NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN_INHERITED(PublicKeyCredential, Credential)
23-
NS_IMPL_CYCLE_COLLECTION_TRACE_END
24-
2534
PublicKeyCredential::PublicKeyCredential(nsPIDOMWindowInner* aParent)
2635
: Credential(aParent)
27-
{}
36+
, mRawIdCachedObj(nullptr)
37+
{
38+
mozilla::HoldJSObjects(this);
39+
}
2840

2941
PublicKeyCredential::~PublicKeyCredential()
30-
{}
42+
{
43+
mozilla::DropJSObjects(this);
44+
}
3145

3246
JSObject*
3347
PublicKeyCredential::WrapObject(JSContext* aCx,
@@ -38,9 +52,12 @@ PublicKeyCredential::WrapObject(JSContext* aCx,
3852

3953
void
4054
PublicKeyCredential::GetRawId(JSContext* aCx,
41-
JS::MutableHandle<JSObject*> aRetVal) const
55+
JS::MutableHandle<JSObject*> aRetVal)
4256
{
43-
aRetVal.set(mRawId.ToUint8Array(aCx));
57+
if (!mRawIdCachedObj) {
58+
mRawIdCachedObj = mRawId.ToUint8Array(aCx);
59+
}
60+
aRetVal.set(mRawIdCachedObj);
4461
}
4562

4663
already_AddRefed<AuthenticatorResponse>

‎dom/webauthn/PublicKeyCredential.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class PublicKeyCredential final : public Credential
3535
WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
3636

3737
void
38-
GetRawId(JSContext* cx, JS::MutableHandle<JSObject*> aRetVal) const;
38+
GetRawId(JSContext* cx, JS::MutableHandle<JSObject*> aRetVal);
3939

4040
already_AddRefed<AuthenticatorResponse>
4141
Response() const;
@@ -48,6 +48,7 @@ class PublicKeyCredential final : public Credential
4848

4949
private:
5050
CryptoBuffer mRawId;
51+
JS::Heap<JSObject*> mRawIdCachedObj;
5152
RefPtr<AuthenticatorResponse> mResponse;
5253
// Extensions are not supported yet.
5354
// <some type> mClientExtensionResults;

‎dom/webauthn/tests/test_webauthn_loopback.html‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ <h1>Full-run test for MakeCredential/GetAssertion for W3C Web Authentication</h1
5656
ok(aCredInfo.rawId.length > 0, "Key ID exists");
5757
is(aCredInfo.id, bytesToBase64UrlSafe(aCredInfo.rawId), "Encoded Key ID and Raw Key ID match");
5858

59+
ok(aCredInfo.rawId === aCredInfo.rawId, "PublicKeyCredential.RawID is SameObject");
60+
ok(aCredInfo.response === aCredInfo.response, "PublicKeyCredential.Response is SameObject");
61+
ok(aCredInfo.response.clientDataJSON === aCredInfo.response.clientDataJSON, "PublicKeyCredential.Response.ClientDataJSON is SameObject");
62+
ok(aCredInfo.response.attestationObject === aCredInfo.response.attestationObject, "PublicKeyCredential.Response.AttestationObject is SameObject");
63+
5964
let clientData = JSON.parse(buffer2string(aCredInfo.response.clientDataJSON));
6065
is(clientData.challenge, bytesToBase64UrlSafe(gCredentialChallenge), "Challenge is correct");
6166
// WD-05 vs. WD-06: In WD-06, the second parameter should be "window.location.origin". Fix
@@ -90,6 +95,9 @@ <h1>Full-run test for MakeCredential/GetAssertion for W3C Web Authentication</h1
9095
ok(aAssertion.rawId.length > 0, "Key ID exists");
9196
is(aAssertion.id, bytesToBase64UrlSafe(aAssertion.rawId), "Encoded Key ID and Raw Key ID match");
9297

98+
ok(aAssertion.response.authenticatorData === aAssertion.response.authenticatorData, "AuthenticatorAssertionResponse.AuthenticatorData is SameObject");
99+
ok(aAssertion.response.signature === aAssertion.response.signature, "AuthenticatorAssertionResponse.Signature is SameObject");
100+
93101
ok(aAssertion.response.authenticatorData.length > 0, "Authenticator data exists");
94102
let clientData = JSON.parse(buffer2string(aAssertion.response.clientDataJSON));
95103
is(clientData.challenge, bytesToBase64UrlSafe(gAssertionChallenge), "Challenge is correct");

0 commit comments

Comments
 (0)