Skip to content

Commit

Permalink
Revert "Implement support for bidNonce and seller nonces"
Browse files Browse the repository at this point in the history 
This reverts commit 803a93609b702479399a8376bd90b1d7780d2794.

Reason for revert: Deterministic failures of browser tests linked to the seller nonce on the bots.

E.g.
https://ci.chromium.org/ui/p/chromium/builders/ci/fuchsia-arm64-cast-receiver-rel/18747/overview
https://ci.chromium.org/ui/p/chrome/builders/ci/linux-chromeos-chrome

Original change's description:
> Implement support for bidNonce and seller nonces
>
> Introduce a mechanism that allows sellers to avoid giving the same nonce
> to all buyers, since this can be used to determine that these buyers are
> in the same auction. Instead, a new nonce is given to each buyer based
> on the SHA-256 combination of both the auctionNonce and a new seller
> nonce, which is given to the browser, but not to buyers.
>
> The browser can verify the bidNonce from bids by computing the expected
> bidNonce for a given auctionNonce and seller nonce, thus preserving the
> existing replay protections of the auctionNonce.
>
> Bug: 40275797
> Fuchsia-Binary-Size: Size increase is unavoidable.
> Change-Id: I9f2a6117891b30c33cc6831c63f6622faf6f55f1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5905479
> Reviewed-by: Orr Bernstein <[email protected]>
> Reviewed-by: Mike Taylor <[email protected]>
> Reviewed-by: Brendon Tiszka <[email protected]>
> Commit-Queue: Caleb Raitto <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1376464}

Bug: 40275797
Change-Id: I61e65f1f7fc7ecb732f49d8860e3976a0fd5cd2a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5979680
Reviewed-by: Orr Bernstein <[email protected]>
Commit-Queue: Liviu Tinta <[email protected]>
Reviewed-by: Mike Taylor <[email protected]>
Auto-Submit: Liviu Tinta <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>
Owners-Override: Liviu Tinta <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1376577}
  • Loading branch information
@liviutinta @pull
liviutinta authored and pull[bot] committed Nov 14, 2024
1 parent 5b3adea commit 1408436
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 328 deletions.
256 changes: 2 additions & 254 deletions fledge/tentative/additional-bids.https.window.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
// which the `adAuctionHeaders=true` attribute is not specified.
// - test that additional bids are not fetched using a Fetch request for which
// `adAuctionHeaders: true` is not specified.
// - test that an additional bid with an incorrect seller and / or top-level
// seller is not used included in an auction.
// - test that an additional bid with an incorrect auction nonce is not used
// included in an auction. Same for seller and top-level seller.
// - lots of tests for different types of malformed additional bids, e.g.
// missing fields, malformed signature, invalid currency code,
// missing joining origin for multiple negative interest groups, etc.
Expand Down Expand Up @@ -149,258 +149,6 @@ subsetTest(promise_test, async test => {
/*winningAdditionalBidId=*/'planes');
}, 'two valid additional bids from two distinct Fetch requests');

// Single-seller auction with a single buyer who places a single additional
// bid with the wrong auctionNonce in the bid, causing the bid to fail.
subsetTest(promise_test, async test => {
const uuid = generateUuid(test);
const auctionNonce = await navigator.createAuctionNonce();
const auctionNonceInBid = crypto.randomUUID();
const seller = SINGLE_SELLER_AUCTION_SELLER;

const buyer = OTHER_ORIGIN1;
const additionalBid = additionalBidHelper.createAdditionalBid(
uuid, auctionNonce, seller, buyer, 'horses', 1.99);
additionalBidHelper.setBidAuctionNonceOverride(
additionalBid, auctionNonceInBid);

await runAdditionalBidTestNoWinner(
test, uuid, [buyer], auctionNonce,
additionalBidHelper.fetchAdditionalBids(seller, [additionalBid]),
/*highestScoringOtherBid=*/0);
}, 'single valid additional bid with wrong auctionNonce in bid');

// Single-seller auction with a single buyer who places a single additional
// bid with no auctionNonce in the bid, causing the bid to fail.
subsetTest(promise_test, async test => {
const uuid = generateUuid(test);
const auctionNonce = await navigator.createAuctionNonce();
const seller = SINGLE_SELLER_AUCTION_SELLER;

const buyer = OTHER_ORIGIN1;
const additionalBid = additionalBidHelper.createAdditionalBid(
uuid, auctionNonce, seller, buyer, 'horses', 1.99);
additionalBidHelper.removeAuctionNonceFromBid(additionalBid);

await runAdditionalBidTestNoWinner(
test, uuid, [buyer], auctionNonce,
additionalBidHelper.fetchAdditionalBids(seller, [additionalBid]),
/*highestScoringOtherBid=*/0);
}, 'single valid additional bid with no auctionNonce in bid');

// Single-seller auction with a single buyer who places a single additional
// bid with auctionNonce and bidNonce in the bid (but no seller nonce), causing
// the bid to fail.
subsetTest(promise_test, async test => {
const uuid = generateUuid(test);
const auctionNonce = await navigator.createAuctionNonce();
const sellerNonce = crypto.randomUUID();
const bidNonce =
await additionalBidHelper.computeBidNonce(auctionNonce, sellerNonce);
const seller = SINGLE_SELLER_AUCTION_SELLER;
const additionalBidOverrides = {
bidNonce: bidNonce,
};

const buyer = OTHER_ORIGIN1;
const additionalBid = additionalBidHelper.createAdditionalBid(
uuid, auctionNonce, seller, buyer, 'horses', 1.99,
/*additionalBidOverrides=*/ additionalBidOverrides);

await runAdditionalBidTestNoWinner(
test, uuid, [buyer], auctionNonce,
additionalBidHelper.fetchAdditionalBids(seller, [additionalBid]),
/*highestScoringOtherBid=*/0);
}, 'single valid additional bid with auctionNonce and bidNonce in bid (but no seller nonce)');

// Single-seller auction with a single buyer who places a single additional
// bid that uses seller nonce / bidNonce. As the only bid, this wins.
subsetTest(promise_test, async test => {
const uuid = generateUuid(test);
const auctionNonce = await navigator.createAuctionNonce();
const sellerNonce = crypto.randomUUID();
const bidNonce =
await additionalBidHelper.computeBidNonce(auctionNonce, sellerNonce);
const seller = SINGLE_SELLER_AUCTION_SELLER;
const additionalBidOverrides = {
bidNonce: bidNonce,
};

const buyer = OTHER_ORIGIN1;
const additionalBid = additionalBidHelper.createAdditionalBid(
uuid, auctionNonce, seller, buyer, 'horses', 1.99,
/*additionalBidOverrides=*/ additionalBidOverrides);
additionalBidHelper.setSellerNonce(additionalBid, sellerNonce);
additionalBidHelper.removeAuctionNonceFromBid(additionalBid);

await runAdditionalBidTest(
test, uuid, [buyer], auctionNonce,
additionalBidHelper.fetchAdditionalBids(seller, [additionalBid]),
/*highestScoringOtherBid=*/0,
/*winningAdditionalBidId=*/'horses');
}, 'single valid additional bid with seller nonce');

// Single-seller auction with a single buyer who places a single additional bid
// that uses seller nonce, but no bidNonce. Since the bidNonce is missing, there
// is no winner.
subsetTest(promise_test, async test => {
const uuid = generateUuid(test);
const auctionNonce = await navigator.createAuctionNonce();
const sellerNonce = crypto.randomUUID();
const seller = SINGLE_SELLER_AUCTION_SELLER;

const buyer = OTHER_ORIGIN1;
const additionalBid = additionalBidHelper.createAdditionalBid(
uuid, auctionNonce, seller, buyer, 'horses', 1.99);
additionalBidHelper.setSellerNonce(additionalBid, sellerNonce);

await runAdditionalBidTestNoWinner(
test, uuid, [buyer], auctionNonce,
additionalBidHelper.fetchAdditionalBids(seller, [additionalBid]),
/*highestScoringOtherBid=*/0);
}, 'single additional bid with seller nonce with no bidNonce');

// Single-seller auction with a single buyer who places a single additional bid
// that uses seller nonce, but no bidNonce or auctionNonce. Since the bidNonce
// is missing, there is no winner.
subsetTest(promise_test, async test => {
const uuid = generateUuid(test);
const auctionNonce = await navigator.createAuctionNonce();
const sellerNonce = crypto.randomUUID();
const seller = SINGLE_SELLER_AUCTION_SELLER;

const buyer = OTHER_ORIGIN1;
const additionalBid = additionalBidHelper.createAdditionalBid(
uuid, auctionNonce, seller, buyer, 'horses', 1.99);
additionalBidHelper.setSellerNonce(additionalBid, sellerNonce);
additionalBidHelper.removeAuctionNonceFromBid(additionalBid);

await runAdditionalBidTestNoWinner(
test, uuid, [buyer], auctionNonce,
additionalBidHelper.fetchAdditionalBids(seller, [additionalBid]),
/*highestScoringOtherBid=*/0);
}, 'single additional bid with seller nonce with no bidNonce or auctionNonce');

// Single-seller auction with a single buyer who places a single additional
// bid that uses seller nonce / bidNonce. Since the bidNonce is invalid there is
// no winner.
subsetTest(promise_test, async test => {
const uuid = generateUuid(test);
const auctionNonce = await navigator.createAuctionNonce();
const sellerNonce = crypto.randomUUID();
// Intentionally use the wrong bidNonce, base64(sha256("incorrect")).
const bidNonce = 'ID01Nr1irTOscLfqPU9eELbVLr0Mt1goQaBTrrtxhqM=';
const seller = SINGLE_SELLER_AUCTION_SELLER;
const additionalBidOverrides = {
bidNonce: bidNonce,
};

const buyer = OTHER_ORIGIN1;
const additionalBid = additionalBidHelper.createAdditionalBid(
uuid, auctionNonce, seller, buyer, 'horses', 1.99,
/*additionalBidOverrides=*/ additionalBidOverrides);
additionalBidHelper.setSellerNonce(additionalBid, sellerNonce);
additionalBidHelper.removeAuctionNonceFromBid(additionalBid);

await runAdditionalBidTestNoWinner(
test, uuid, [buyer], auctionNonce,
additionalBidHelper.fetchAdditionalBids(seller, [additionalBid]),
/*highestScoringOtherBid=*/0);
}, 'single invalid additional bid with seller nonce');

// Single-seller auction with a two buyers competing with additional bids, using
// seller nonce.
subsetTest(promise_test, async test => {
const uuid = generateUuid(test);
const auctionNonce = await navigator.createAuctionNonce();
const sellerNonce1 = crypto.randomUUID();
const sellerNonce2 = crypto.randomUUID();
const bidNonce1 =
await additionalBidHelper.computeBidNonce(auctionNonce, sellerNonce1);
const bidNonce2 =
await additionalBidHelper.computeBidNonce(auctionNonce, sellerNonce2);
const seller = SINGLE_SELLER_AUCTION_SELLER;
const additionalBidOverrides1 = {
bidNonce: bidNonce1,
};
const additionalBidOverrides2 = {
bidNonce: bidNonce2,
};

const buyer1 = OTHER_ORIGIN1;
const additionalBid1 = additionalBidHelper.createAdditionalBid(
uuid, auctionNonce, seller, buyer1, 'horses', 1.99,
/*additionalBidOverrides=*/ additionalBidOverrides1);
additionalBidHelper.setSellerNonce(additionalBid1, sellerNonce1);
additionalBidHelper.removeAuctionNonceFromBid(additionalBid1);

const buyer2 = OTHER_ORIGIN2;
const additionalBid2 = additionalBidHelper.createAdditionalBid(
uuid, auctionNonce, seller, buyer2, 'planes', 2.99,
/*additionalBidOverrides=*/ additionalBidOverrides2);
additionalBidHelper.setSellerNonce(additionalBid2, sellerNonce2);
additionalBidHelper.removeAuctionNonceFromBid(additionalBid2);

await runAdditionalBidTest(
test, uuid, [buyer1, buyer2], auctionNonce,
additionalBidHelper.fetchAdditionalBids(
seller, [additionalBid1, additionalBid2]),
/*highestScoringOtherBid=*/1.99,
/*winningAdditionalBidId=*/'planes');
}, 'two valid additional bids using seller nonce');

// Single-seller auction with a single buyer who places a single additional
// bid that uses seller nonce / bidNonce, but also auctionNonce in the bid. As
// exactly one of bidNonce / auctionNonce is allowed in the bid, this fails.
subsetTest(promise_test, async test => {
const uuid = generateUuid(test);
const auctionNonce = await navigator.createAuctionNonce();
const sellerNonce = crypto.randomUUID();
const bidNonce =
await additionalBidHelper.computeBidNonce(auctionNonce, sellerNonce);
const seller = SINGLE_SELLER_AUCTION_SELLER;
const additionalBidOverrides = {
bidNonce: bidNonce,
};

const buyer = OTHER_ORIGIN1;
const additionalBid = additionalBidHelper.createAdditionalBid(
uuid, auctionNonce, seller, buyer, 'horses', 1.99,
/*additionalBidOverrides=*/ additionalBidOverrides);
additionalBidHelper.setSellerNonce(additionalBid, sellerNonce);
// Don't remove the auctionNonce from the bid.

await runAdditionalBidTestNoWinner(
test, uuid, [buyer], auctionNonce,
additionalBidHelper.fetchAdditionalBids(seller, [additionalBid]),
/*highestScoringOtherBid=*/0);
}, 'single additional bid with seller nonce, but also with auctionNonce in bid');

// Single-seller auction with a single buyer who places a single additional
// bid with auctionNonce and bidNonce in the bid (with seller nonce), causing
// the bid to fail.
subsetTest(promise_test, async test => {
const uuid = generateUuid(test);
const auctionNonce = await navigator.createAuctionNonce();
const sellerNonce = crypto.randomUUID();
const bidNonce =
await additionalBidHelper.computeBidNonce(auctionNonce, sellerNonce);
const seller = SINGLE_SELLER_AUCTION_SELLER;
const additionalBidOverrides = {
bidNonce: bidNonce,
};

const buyer = OTHER_ORIGIN1;
const additionalBid = additionalBidHelper.createAdditionalBid(
uuid, auctionNonce, seller, buyer, 'horses', 1.99,
/*additionalBidOverrides=*/ additionalBidOverrides);
additionalBidHelper.setSellerNonce(additionalBid, sellerNonce);

await runAdditionalBidTestNoWinner(
test, uuid, [buyer], auctionNonce,
additionalBidHelper.fetchAdditionalBids(seller, [additionalBid]),
/*highestScoringOtherBid=*/0);
}, 'single valid additional bid with auctionNonce and bidNonce in bid (with seller nonce)');

// Single-seller auction with a single additional bid. Because this additional
// bid is filtered by negative targeting, this auction has no winner.
subsetTest(promise_test, async test => {
Expand Down
19 changes: 3 additions & 16 deletions fledge/tentative/resources/additional-bids.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,28 +89,15 @@ def main(request, response):
# Each additional bid may have associated testMetadata. Remove this from
# the additional bid and use it to adjust the behavior of this handler.
test_metadata = additional_bid.pop("testMetadata", {})
seller_nonce = test_metadata.get("sellerNonce", None)
remove_auction_nonce_from_bid = test_metadata.get("removeAuctionNonceFromBid", False)
bid_auction_nonce_override = test_metadata.get("bidAuctionNonceOverride", None)
if remove_auction_nonce_from_bid:
auction_nonce = additional_bid.pop("auctionNonce", None)
else:
auction_nonce = additional_bid.get("auctionNonce", None)
if bid_auction_nonce_override:
additional_bid["auctionNonce"] = bid_auction_nonce_override
auction_nonce = additional_bid.get("auctionNonce", None)
if not auction_nonce:
raise BadRequestError("Additional bid missing required 'auctionNonce' field")
signed_additional_bid = _sign_additional_bid(
json.dumps(additional_bid),
test_metadata.get("secretKeysForValidSignatures", []),
test_metadata.get("secretKeysForInvalidSignatures", []))
if seller_nonce:
additional_bid_header_value = (auction_nonce.encode("utf-8") + b":" +
seller_nonce.encode("utf-8") + b":" +
base64.b64encode(signed_additional_bid.encode("utf-8")))
else:
additional_bid_header_value = (auction_nonce.encode("utf-8") + b":" +
base64.b64encode(signed_additional_bid.encode("utf-8")))
additional_bid_header_value = (auction_nonce.encode("utf-8") + b":" +
base64.b64encode(signed_additional_bid.encode("utf-8")))
response.headers.append(b"Ad-Auction-Additional-Bid", additional_bid_header_value)

response.status = (200, b"OK")
Expand Down
59 changes: 1 addition & 58 deletions fledge/tentative/resources/fledge-util.sub.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -544,10 +544,8 @@ async function runReportTest(test, uuid, codeToInsert, expectedReportURLs,
// - test/uuid: the test object and uuid from the test case (see generateUuid)
// - buyers: array of strings, each a domain for a buyer participating in this
// auction
// - auctionNonce: string, the auction nonce for this auction, typically
// - actionNonce: string, the auction nonce for this auction, typically
// retrieved from a prior call to navigator.createAuctionNonce
// - additionalBidsPromise: promise resolving to undefined, to be resolved when
// the additional bids have been retrieved with fetch().
// - highestScoringOtherBid: the amount of the second-highest bid,
// or zero if there's no second-highest bid
// - winningAdditionalBidId: the label of the winning bid
Expand All @@ -569,25 +567,6 @@ async function runAdditionalBidTest(test, uuid, buyers, auctionNonce,
createBidderReportURL(uuid, winningAdditionalBidId)]);
}

// Similar to runAdditionalBidTest(), but expects no winner. It takes the
// following arguments:
// - test/uuid: the test object and uuid from the test case (see generateUuid)
// - buyers: array of strings, each a domain for a buyer participating in this
// auction
// - auctionNonce: string, the auction nonce for this auction, typically
// retrieved from a prior call to navigator.createAuctionNonce
// - additionalBidsPromise: promise resolving to undefined, to be resolved when
// the additional bids have been retrieved with fetch().
async function runAdditionalBidTestNoWinner(
test, uuid, buyers, auctionNonce, additionalBidsPromise) {
await runBasicFledgeTestExpectingNoWinner(test, uuid, {
interestGroupBuyers: buyers,
auctionNonce: auctionNonce,
additionalBids: additionalBidsPromise,
decisionLogicURL: createDecisionScriptURL(uuid)
});
}

// Runs "script" in "child_window" via an eval call. The "child_window" must
// have been created by calling "createFrame()" below. "param" is passed to the
// context "script" is run in, so can be used to pass objects that
Expand Down Expand Up @@ -833,38 +812,6 @@ let additionalBidHelper = function() {
secretKeysForInvalidSignatures = secretKeys;
}

// Sets the seller nonce that will be used in the server response.
function setSellerNonce(additionalBid, sellerNonce) {
getAndMaybeInitializeTestMetadata(additionalBid).
sellerNonce = sellerNonce;
}

// Instructs the server to remove the auctionNonce from the bid, and only
// include it in the header.
function removeAuctionNonceFromBid(additionalBid) {
getAndMaybeInitializeTestMetadata(additionalBid).
removeAuctionNonceFromBid = true;
}

// Instructs the server to use `bidAuctionNonceOverride` as the `auctionNonce`
// in the bid, even it doesn't match the auctionNonce in the header. Overrides
// the behavior of removeAuctionNonceFromBid().
function setBidAuctionNonceOverride(additionalBid, bidAuctionNonceOverride) {
getAndMaybeInitializeTestMetadata(additionalBid).
bidAuctionNonceOverride = bidAuctionNonceOverride;
}

// Takes the auctionNonce and sellerNonce as strings, and combines them with
// SHA256, returning the result as a base64 string.
async function computeBidNonce(auctionNonce, sellerNonce) {
// Compute the bidNonce as hashed bytes.
const combined_utf8 = new TextEncoder().encode(auctionNonce + sellerNonce);
const hashed = await crypto.subtle.digest('SHA-256',combined_utf8);

// Convert the hashed bytes to base64.
return btoa(String.fromCharCode(...new Uint8Array(hashed)));
}

// Adds a single negative interest group to an additional bid, as described at:
// https://github.com/WICG/turtledove/blob/main/FLEDGE.md#622-how-additional-bids-specify-their-negative-interest-groups
function addNegativeInterestGroup(additionalBid, negativeInterestGroup) {
Expand Down Expand Up @@ -899,10 +846,6 @@ let additionalBidHelper = function() {
createAdditionalBid: createAdditionalBid,
signWithSecretKeys: signWithSecretKeys,
incorrectlySignWithSecretKeys: incorrectlySignWithSecretKeys,
setSellerNonce: setSellerNonce,
removeAuctionNonceFromBid: removeAuctionNonceFromBid,
setBidAuctionNonceOverride: setBidAuctionNonceOverride,
computeBidNonce: computeBidNonce,
addNegativeInterestGroup: addNegativeInterestGroup,
addNegativeInterestGroups: addNegativeInterestGroups,
fetchAdditionalBids: fetchAdditionalBids
Expand Down

0 comments on commit 1408436

Please sign in to comment.