✨Random Subdomains for SafeFrame (#27393)

* random subdomain for each safeframe * fixed some tests * experiment * fix * added experiment * fix * fix * fix * linting * linting * fixes * linting * experiment changes * experiment changes * experiment changes * linting * linting * linting * experiment fix * revert adsense changes * stuff * stuff * added tests * added more tests * changed crypto code a bit * linting * linting * linting * linting * more changes * update experiment * use the existing fn * lint * update * update * updatetests * update * update * whitespace commit just to re-run the tests * whitespace commit just to re-run the tests * empty commit to re-run tests * empty commit to re-run tests * empty commit to re-run flakey tests * empty commit to re-run flakey tests * empty commit to re-run flakey tests * empty commit to re-run flakey tests * empty commit to re-run flakey tests 2 * empty commit to re-run flakey tests 3 * empty commit to re-run flakey tests 4 * empty commit to re-run flakey tests 5 * empty commit to re-run flakey tests 6 * empty commit to re-run flakey tests 7 * empty commit to re-run flakey tests 8 * empty commit to re-run flakey tests 9 * empty commit to re-run flakey tests 10 * empty commit to re-run flakey tests 11 * empty commit to re-run flakey tests 12 * empty commit to re-run flakey tests 13
ampproject · Apr 22, 2020 · 701c62c · 701c62c
1 parent 7daf5f9
commit 701c62c
Show file tree

Hide file tree

Showing 7 changed files with 141 additions and 16 deletions.
diff --git a/build-system/global-configs/canary-config.json b/build-system/global-configs/canary-config.json
@@ -30,6 +30,7 @@
   "ios-fixed-no-transfer": 1,
   "layoutbox-invalidate-on-scroll": 1,
   "pump-early-frame": 1,
+  "random-subdomain-for-safeframe": 0.02,
   "swg-gpay-api": 1,
   "swg-gpay-native": 1,
   "version-locking": 1

diff --git a/build-system/global-configs/prod-config.json b/build-system/global-configs/prod-config.json
@@ -30,6 +30,7 @@
   "ios-fixed-no-transfer": 0,
   "layoutbox-invalidate-on-scroll": 1,
   "pump-early-frame": 1,
+  "random-subdomain-for-safeframe": 0.02,
   "swg-gpay-api": 1,
   "swg-gpay-native": 1,
   "version-locking": 1

diff --git a/extensions/amp-a4a/0.1/test/test-amp-a4a.js b/extensions/amp-a4a/0.1/test/test-amp-a4a.js
@@ -250,10 +250,8 @@ describe('amp-a4a', () => {
     expect(element).to.be.visible;
     expect(element.querySelectorAll('iframe')).to.have.lengthOf(1);
     const safeFrameUrl =
-      'https://tpc.googlesyndication.com/safeframe/' +
-      sfVersion +
-      '/html/container.html';
-    const child = element.querySelector(`iframe[src^="${safeFrameUrl}"][name]`);
+      'googlesyndication.com/safeframe/' + sfVersion + '/html/container.html';
+    const child = element.querySelector(`iframe[src*="${safeFrameUrl}"][name]`);
     expect(child).to.be.ok;
     const name = child.getAttribute('name');
     expect(name).to.match(/[^;]+;\d+;[\s\S]+/);
@@ -945,11 +943,11 @@ describe('amp-a4a', () => {
               expect(devStub).to.not.be.called;
             }
             const safeframeUrl =
-              'https://tpc.googlesyndication.com/safeframe/' +
+              '.googlesyndication.com/safeframe/' +
               DEFAULT_SAFEFRAME_VERSION +
               '/html/container.html';
             const safeChild = a4aElement.querySelector(
-              `iframe[src^="${safeframeUrl}"]`
+              `iframe[src*="${safeframeUrl}"]`
             );
             expect(safeChild).to.not.be.ok;
             if (headerVal != 'nameframe') {
@@ -1769,7 +1767,7 @@ describe('amp-a4a', () => {
       expect(
         doc.querySelector(
           'link[rel=preload]' +
-            '[href="https://tpc.googlesyndication.com/safeframe/' +
+            '[href*=".googlesyndication.com/safeframe/' +
             '1-2-3/html/container.html"]'
         )
       ).to.be.ok;

diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js b/extensions/amp-ad-network-doubleclick-impl/0.1/amp-ad-network-doubleclick-impl.js
@@ -87,6 +87,7 @@ import {
   extractUrlExperimentId,
   isInManualExperiment,
 } from '../../../ads/google/a4a/traffic-experiments';
+import {getCryptoRandomBytesArray, utf8Decode} from '../../../src/utils/bytes';
 import {getMode} from '../../../src/mode';
 import {getMultiSizeDimensions} from '../../../ads/google/utils';
 import {getOrCreateAdCid} from '../../../src/ad-cid';
@@ -108,7 +109,6 @@ import {
 import {parseQueryString} from '../../../src/url';
 import {stringHash32} from '../../../src/string';
 import {tryParseJson} from '../../../src/json';
-import {utf8Decode} from '../../../src/utils/bytes';
 
 /** @type {string} */
 const TAG = 'amp-ad-network-doubleclick-impl';
@@ -139,6 +139,19 @@ const ZINDEX_EXP_BRANCHES = {
   HOLDBACK: '21065357',
 };
 
+/** @const {string} */
+const RANDOM_SUBDOMAIN_SAFEFRAME_EXP = 'random-subdomain-for-safeframe';
+
+/**
+ * Branches of the random subdomain for SafeFrame experiment.
+ * @const @enum{string}
+ * @visibleForTesting
+ */
+export const RANDOM_SUBDOMAIN_SAFEFRAME_BRANCHES = {
+  CONTROL: '21065817',
+  EXPERIMENT: '21065818',
+};
+
 /**
  * Required size to be sent with fluid requests.
  * @const {string}
@@ -276,6 +289,14 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
       }
     }
 
+    /**
+     * The random subdomain to load SafeFrame from, if SafeFrame is
+     * being loaded from a random subdomain and if the subdomain
+     * has been generated.
+     * @private {?string}
+     */
+    this.safeFrameRandomSubdomain_ = null;
+
     /** @protected {?CONSENT_POLICY_STATE} */
     this.consentState = null;
 
@@ -402,6 +423,13 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
         isTrafficEligible: () => true,
         branches: Object.values(ZINDEX_EXP_BRANCHES),
       },
+      [RANDOM_SUBDOMAIN_SAFEFRAME_EXP]: {
+        ifTrafficEligible: () => true,
+        branches: [
+          RANDOM_SUBDOMAIN_SAFEFRAME_BRANCHES.CONTROL,
+          RANDOM_SUBDOMAIN_SAFEFRAME_BRANCHES.EXPERIMENT,
+        ],
+      },
       ...AMPDOC_FIE_EXPERIMENT_INFO_MAP,
     });
     const setExps = this.randomlySelectUnsetExperiments_(experimentInfoMap);
@@ -1019,6 +1047,22 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
     return super.unlayoutCallback();
   }
 
+  /** @override */
+  getSafeframePath() {
+    const randomSubdomainExperimentBranch =
+      RANDOM_SUBDOMAIN_SAFEFRAME_BRANCHES.EXPERIMENT;
+    if (!this.experimentIds.includes(randomSubdomainExperimentBranch)) {
+      return super.getSafeframePath();
+    }
+    this.safeFrameRandomSubdomain_ =
+      this.safeFrameRandomSubdomain_ || this.getRandomString_();
+
+    return (
+      `https://${this.safeFrameRandomSubdomain_}.safeframe.googlesyndication.com/safeframe/` +
+      `${this.safeframeVersion}/html/container.html`
+    );
+  }
+
   /** @visibleForTesting */
   cleanupAfterTest() {
     this.destroySafeFrameApi_();
@@ -1574,6 +1618,34 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
     dev().warn(TAG, message, error);
   }
 
+  /**
+   * Generate a 32-byte random string.
+   * Uses the win.crypto when available.
+   * @return {string} The random string
+   * @private
+   */
+  getRandomString_() {
+    // 16 hex characters * 2 bytes per character = 32 bytes
+    const length = 16;
+
+    const randomValues = getCryptoRandomBytesArray(this.win, length);
+
+    let randomSubdomain = '';
+    for (let i = 0; i < length; i++) {
+      // If crypto isn't available, just use Math.random.
+      const randomValue = randomValues
+        ? randomValues[i]
+        : Math.floor(Math.random() * 255);
+      // Ensure each byte is represented with two hexadecimal characters.
+      if (randomValue <= 15) {
+        randomSubdomain += '0';
+      }
+      randomSubdomain += randomValue.toString(16);
+    }
+
+    return randomSubdomain;
+  }
+
   /** @override */
   getPreconnectUrls() {
     return ['https://securepubads.g.doubleclick.net/'];

diff --git a/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js b/extensions/amp-ad-network-doubleclick-impl/0.1/test/test-amp-ad-network-doubleclick-impl.js
@@ -19,6 +19,7 @@
 // always available for them. However, when we test an impl in isolation,
 // AmpAd is not loaded already, so we need to load it separately.
 import '../../../amp-ad/0.1/amp-ad';
+import * as bytesUtils from '../../../../src/utils/bytes';
 import {
   AMP_SIGNATURE_HEADER,
   VerificationStatus,
@@ -32,6 +33,7 @@ import {
 import {AmpAd} from '../../../amp-ad/0.1/amp-ad';
 import {
   AmpAdNetworkDoubleclickImpl,
+  RANDOM_SUBDOMAIN_SAFEFRAME_BRANCHES,
   getNetworkId,
   getPageviewStateTokensForAdRequest,
   resetLocationQueryParametersForTesting,
@@ -46,7 +48,6 @@ import {SafeframeHostApi} from '../safeframe-host';
 import {Services} from '../../../../src/services';
 import {createElementWithAttributes} from '../../../../src/dom';
 import {toggleExperiment} from '../../../../src/experiments';
-import {utf8Decode, utf8Encode} from '../../../../src/utils/bytes';
 
 /**
  * We're allowing external resources because otherwise using realWin causes
@@ -1170,7 +1171,7 @@ describes.realWin('amp-ad-network-doubleclick-impl', realWinConfig, (env) => {
       return {
         arrayBuffer: () =>
           Promise.resolve(
-            utf8Encode('<html><body>Hello, World!</body></html>')
+            bytesUtils.utf8Encode('<html><body>Hello, World!</body></html>')
           ),
         headers: {
           get(prop) {
@@ -1454,6 +1455,48 @@ describes.realWin('amp-ad-network-doubleclick-impl', realWinConfig, (env) => {
       ).to.equal(XORIGIN_MODE.SAFEFRAME);
     });
   });
+
+  describe('#RandomSubdomainSafeFrame', () => {
+    beforeEach(() => {
+      element = doc.createElement('amp-ad');
+      element.setAttribute('type', 'doubleclick');
+      element.setAttribute('data-ad-client', 'doubleclick');
+      element.setAttribute('width', '320');
+      element.setAttribute('height', '50');
+      doc.body.appendChild(element);
+      impl = new AmpAdNetworkDoubleclickImpl(element);
+    });
+
+    it('should use random subdomain when experiment is enabled', () => {
+      impl.experimentIds = [RANDOM_SUBDOMAIN_SAFEFRAME_BRANCHES.EXPERIMENT];
+
+      const expectedPath =
+        '^https:\\/\\/[\\w\\d]{32}.safeframe.googlesyndication.com' +
+        '\\/safeframe\\/\\d+-\\d+-\\d+\\/html\\/container\\.html$';
+
+      expect(impl.getSafeframePath()).to.match(new RegExp(expectedPath));
+    });
+
+    it('uses random subdomain if experiment is on without win.crypto', () => {
+      impl.experimentIds = [RANDOM_SUBDOMAIN_SAFEFRAME_BRANCHES.EXPERIMENT];
+
+      env.sandbox.stub(bytesUtils, 'getCryptoRandomBytesArray').returns(null);
+
+      const expectedPath =
+        '^https:\\/\\/[\\w\\d]{32}.safeframe.googlesyndication.com' +
+        '\\/safeframe\\/\\d+-\\d+-\\d+\\/html\\/container\\.html$';
+
+      expect(impl.getSafeframePath()).to.match(new RegExp(expectedPath));
+    });
+
+    it('should use constant subdomain when experiment is disabled', () => {
+      const expectedPath =
+        '^https://tpc.googlesyndication.com' +
+        '\\/safeframe\\/\\d+-\\d+-\\d+\\/html\\/container\\.html$';
+
+      expect(impl.getSafeframePath()).to.match(new RegExp(expectedPath));
+    });
+  });
 });
 
 describes.realWin(
@@ -1892,7 +1935,7 @@ describes.realWin(
         };
         expect(
           AmpAdNetworkDoubleclickImpl.prototype.maybeValidateAmpCreative(
-            utf8Encode(creative),
+            bytesUtils.utf8Encode(creative),
             mockHeaders
           )
         ).to.eventually.equal('foo');
@@ -1913,10 +1956,13 @@ describes.realWin(
           },
         };
         return AmpAdNetworkDoubleclickImpl.prototype
-          .maybeValidateAmpCreative(utf8Encode(creative), mockHeaders)
+          .maybeValidateAmpCreative(
+            bytesUtils.utf8Encode(creative),
+            mockHeaders
+          )
           .then((result) => {
             expect(result).to.be.ok;
-            expect(utf8Decode(result)).to.equal(creative);
+            expect(bytesUtils.utf8Decode(result)).to.equal(creative);
           });
       });
 
@@ -1936,7 +1982,7 @@ describes.realWin(
         };
         expect(
           AmpAdNetworkDoubleclickImpl.prototype.maybeValidateAmpCreative(
-            utf8Encode(creative),
+            bytesUtils.utf8Encode(creative),
             mockHeaders
           )
         ).to.eventually.not.be.ok;

diff --git a/src/utils/bytes.js b/src/utils/bytes.js
@@ -100,13 +100,16 @@ export function bytesToUInt32(bytes) {
  * @return {?Uint8Array}
  */
 export function getCryptoRandomBytesArray(win, length) {
-  if (!win.crypto || !win.crypto.getRandomValues) {
+  // Support IE 11
+  const cryptoLib = /** @type {!webCrypto.Crypto|undefined} */ (win.crypto ||
+    win.msCrypto);
+  if (!cryptoLib || !cryptoLib.getRandomValues) {
     return null;
   }
 
   // Widely available in browsers we support:
   // http://caniuse.com/#search=getRandomValues
   const uint8array = new Uint8Array(length);
-  win.crypto.getRandomValues(uint8array);
+  cryptoLib.getRandomValues(uint8array);
   return uint8array;
 }
diff --git a/tools/experiments/experiments-config.js b/tools/experiments/experiments-config.js
@@ -311,6 +311,10 @@ export const EXPERIMENTS = [
     spec: 'https://github.com/ampproject/amphtml/issues/26823',
     cleanupIssue: 'https://github.com/ampproject/amphtml/issues/26823',
   },
+  {
+    id: 'random-subdomains-for-safeframe',
+    name: 'Enable Random Subdomains for SafeFrame',
+  },
   {
     id: 'analytics-chunks',
     name: 'AMP Analytics Break long tasks to chunks',