[tor-bugs] #25985 [Obfuscation/Snowflake]: Snowflake rendezvous using AMP cache

Mon Jul 2 11:48:41 UTC 2018

#25985: Snowflake rendezvous using AMP cache
-----------------------------------+--------------------------------
 Reporter:  twim                   |          Owner:  (none)
     Type:  project                |         Status:  needs_revision
 Priority:  Medium                 |      Milestone:
Component:  Obfuscation/Snowflake  |        Version:
 Severity:  Normal                 |     Resolution:
 Keywords:                         |  Actual Points:
Parent ID:                         |         Points:
 Reviewer:                         |        Sponsor:
-----------------------------------+--------------------------------

Comment (by twim):

 Replying to [comment:24 dcf]:
 Thanks for the review!

 For now I've fixed amper-related issues you brought up.

 > (On further reflection, what I was trying to do wouldn't have worked
 anyway, because the AMP client code would have sent a URL path of
 `/c/s/.../amp/client/...`, not `/amp/client/...`, but I could have worked
 around that for testing.) It looks like the cause is
 [https://github.com/nogoegst/amper/blob/6dbbc42eb44c1f389591c83a268524d5eceed1d1/client.go#L55
 here in amper/client.go], with a hardcoded `"https"`. Looking at that, I
 also noticed a
 [https://github.com/nogoegst/amper/blob/6dbbc42eb44c1f389591c83a268524d5eceed1d1/client.go#L23
 CDNDomain = "cdn.ampproject.org"]. I have to say, it skeeves me a bit to
 see hardcoded constants like that in a library; it seems like the kind of
 thing that belongs in a configuration file.

 Thanks to your suggestion I made some constants configurable. One can now
 specify Client.Scheme and Client.CDNDomain.
 As you may guess, it was intended to work with the only AMP cache around
 thus the hardcoded values.

 > Here's an example of what the AMP responses look like:
 > ...
 > The overall AMP container encoding looks pretty reasonable. A few tiny
 questions:
 >  * [https://www.ampproject.org/docs/getting_started/create/basic_markup
 basic_markup] says the `<script async>` has to be the second child of
 `<head>`, right after `<meta charset="utf-8">`.
 You're right. Despite it is being accepted in validator, I moved it to the
 right place to avoid a breakage.
 >  * The `<style>body {opacity: 0}</style><noscript><style>body {opacity:
 1}</style></noscript>` doesn't match the documented
 [https://www.ampproject.org/docs/reference/spec/amp-boilerplate.html AMP
 boilerplate code], which is much longer. Is there a reason?
 This is knowm as "old boilerlate"
 (https://github.com/ampproject/amphtml/commit/0a056ca50ac8cb9ba8e5a6489baeecb5ed958556
 #diff-17672f7dfc8b0583c360b45234dbf59a). The reason is that it's much
 shorter than the new one. I've picked this as a tradeoff to save bandwidth
 as the old boilerplate is still accepted by AMP CDN.
 Now this is also configurable: set Server.UseOldAMPBoilterplate to true to
 use shorter boilerplate, or get completely valid 'modern' AMP page by
 default.

 >  * `<link rel="canonical" href="https://ampproject.org" />`: it would be
 better not to refer to any third parties. The AMP guide says this can be a
 relative URL, so I would prefer one of these if they work:
 >    * `<link rel="canonical" href="">`
 >    * `<link rel="canonical" href="#">`
 >    * `<link rel="canonical" href="?">`

 Yep, changed to `href="#"` and it works.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25985#comment:25>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online