[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