[webkit-reviews] review granted: [Bug 222019] PCM: Generate secret token and corresponding unlinkable token : [Attachment 420615] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Feb 17 11:28:26 PST 2021
John Wilander <wilander at apple.com> has granted Jiewen Tan
<jiewen_tan at apple.com>'s request for review:
Bug 222019: PCM: Generate secret token and corresponding unlinkable token
https://bugs.webkit.org/show_bug.cgi?id=222019
Attachment 420615: Patch
https://bugs.webkit.org/attachment.cgi?id=420615&action=review
--- Comment #9 from John Wilander <wilander at apple.com> ---
Comment on attachment 420615
--> https://bugs.webkit.org/attachment.cgi?id=420615
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=420615&action=review
r=me with comments.
> Source/WebCore/loader/PrivateClickMeasurement.cpp:150
> +#if HAVE(RSA_BSSA)
These don't have to be behind RSA_BSSA, right? That will only make testing
harder since EWS bots will be behind.
> Source/WebCore/loader/PrivateClickMeasurement.h:274
> +#if HAVE(RSA_BSSA)
Again, EphemeralSourceNonce doesn't have to be behind RSA_BSSA. Or am I missing
something?
> Source/WebCore/loader/PrivateClickMeasurement.h:319
> + BlindedToken m_blindedToken;
This should be optional too and it needs to have "source" be part of its name
since there will eventually be a token for the attribute-on site too.
> Source/WebCore/loader/cocoa/PrivateClickMeasurementCocoa.mm:58
> + blindedSecret->setString("click_nonce"_s,
m_ephemeralSourceNonce->nonce);
Let's call it source_nonce.
This is how I've set it up in my patch:
reportDetails->setString("source_engagement_type"_s, "click"_s);
reportDetails->setString("source_nonce"_s, m_ephemeralSourceNonce->nonce);
reportDetails->setString("unlinkable_token"_s, "TODO"_s);
reportDetails->setInteger("version"_s, 2);
> Source/WebCore/loader/cocoa/PrivateClickMeasurementCocoa.mm:59
> + blindedSecret->setString("blinded_secret"_s,
WTF::base64URLEncode([m_blindedToken.waitingToken blindedMessage].bytes,
[m_blindedToken.waitingToken blindedMessage].length));
We should try to use unlinkable_token in any developer facing parts so that we
are not tied to blinded signatures as the crypto underpinning. Could even be
unlinkable_token_to_sign.
> Tools/TestWebKitAPI/Tests/WebCore/PrivateClickMeasurement.cpp:150
> + EXPECT_STREQ(blindedSecret->getString("click_nonce"_s).utf8().data(),
"ABCDEFabcdef0123456789");
source_nonce
> Tools/TestWebKitAPI/Tests/WebCore/PrivateClickMeasurement.cpp:151
> + EXPECT_FALSE(blindedSecret->getString("blinded_secret"_s).isEmpty());
unlinkable_token or unlinkable_token_to_sign.
More information about the webkit-reviews
mailing list