Changes to come for select “Update Firefox” buttons

25 03 2014

Do you have this lovely yellow Affiliate button hosted on your website or blog? If so, this post is for you.
FFx_36Upgrade_160x600_Preview_lrg_EN.png

As we are updating the look and functionality of the new site (check out some of the features coming on the Affiliates wiki), we’ve found a better to way to make smart banners work. Smart banners detect the version of Firefox that a visitor to a website or blog is on. If they’re on an older version, they get an update message. Up to date? They get a Firefox brand message. It’s a pretty neat feature and we’re hoping to have more banners that work like this in the future.

However, the Affiliates dev team has decided to change how we make the update banner work. Previously, your banner embed code would point to a special image on the Affiliates site that changes depending on the browser version of whoever is viewing it. In the new system, the banner embed code detects to user’s browser and changes the image to point to the upgrade banner if necessary.

We’re making this change because the old system was difficult to maintain and difficult to extend to other banners. With the new system, it will be easier for us to release new banners that do more than just switch images based on the viewer’s browser, meaning a larger selection of banners for you to choose from.

The downside is that, after the change, existing smart banners will not longer switch to the upgrade message. Users will always see the “Different by Design” brand image. There are only a select number of Affiliates using these banners and I will be sending notifications to all of these users directly to warm them of the change when the new site launches later in the spring. At that point, they can replace the embed code on their sites with the new banner.

Have any concerns or questions about this change? Drop us a note at affiliates@mozilla.org or drop by the #affiliates irc channel.

About these ads

Actions

Information

7 responses

25 03 2014
Josh Triplett

Switching from a mechanism that works with a static img tag to one that requires embedding JavaScript seems like an unfortunate downgrade.

25 03 2014
Andrew Sutherland

I agree with Josh. There are a lot of downsides to the operators of sites embedding the image and the users of the sites by this change as described. If an iframe is used, there is memory overhead to the additional origin. If an iframe is not used, there is the potential to stall load time unless the “async” flag is used. If an iframe is not used, there are also security concerns about the compromise of the server hosting the code (at least until sub-resource integrity gets standardized and adopted, assuming its constraints are acceptable: http://www.w3.org/TR/2014/WD-SRI-20140318/). If the script/resources are not served via https (implying the original page is not https), this may make it easier for an attacker to attack the site/multiple sites by compromising common network paths to the Mozilla servers.

In general, it seems like Mozilla is striking the wrong balance for its own convenience for a comparatively superficial feature. If it’s exceedingly difficult to operate the server infrastructure, it seems like the feature should be dropped instead. I realize there may be complicated issues at play; it helps when posts like this link to a bug where discussion occurred or a public mailing list archive to the relevant threats. The affiliates link is useful but not quite topical, and if anything it makes me think that the motivation for this is to be able to extract more metrics/user-data by being able to further integrate google-analytics into the banner.

I do want to finish up by saying that I understand how important it is to help people learn about Firefox and the benefits of the software itself and its development process and goals. And how marketing-type activities frequently desire, if not depend on, information that is at odds with the overall goals of the mission that do require trade-offs/trust.

25 03 2014
Michael Kelly

Hi! So I’m the main dev on Affiliates and the one who made this call. I think it’s the right call, but I’m also open to being convinced otherwise (which would screw up our timeline for release but oh well we’ll deal with it). Sorry in advance if this comment’s a bit long, but this issue has a bit of history.

So my current implementation can be seen at https://github.com/Osmose/affiliates/commit/f4658c32457c2821b4e2b0219bae401eee653368 (it hasn’t gone under code review or been merged yet so take it with a grain of salt). Specifically, the banner code that users will embed is at https://github.com/Osmose/affiliates/blob/f4658c32457c2821b4e2b0219bae401eee653368/affiliates/banners/templates/banners/banner_code/firefox_upgrade_banner.html and the included JavaScript that does the magic is at https://github.com/Osmose/affiliates/blob/f4658c32457c2821b4e2b0219bae401eee653368/affiliates/banners/static/js/firefox_upgrade_banner.js.

We’re not using any iframes. Instead, we just have the user embed the no-upgrade image, and have the JS change the image to the upgrade image if the user’s browser needs to be upgraded.

The reason why this is easier than the old system is because the old system involved manually commiting banner image filenames (which are hashed because Adblock hates images with 500×1000 in their filenames) that are uploaded by an admin to a settings file, and auto-generating the htaccess file that swapped the images out. This file had to be re-generated on a regular basis in case there was an update out. The better alternatives mostly involve hardcoding the upgrade banners into the codebase, which makes it difficult when we want to add things that relate to banners stored in the database vs hardcoded banners.

With the JS and banner code above, the banner code contains both filenames to the images uploaded by an admin, and the JavaScript chooses between them. This also can be reused as a pattern for other future dynamic banners that switch between images, whereas the old system was heavily biased towards banners that switch based on stuff Apache can know (for example, a banner that switched if you visited another Mozilla property? Difficult!).

Now for your specific points:

> If an iframe is not used, there is the potential to stall load time unless the “async” flag is used.

Good point! I forget if tags spit out by the template are async or not, but they should be.

> If an iframe is not used, there are also security concerns about the compromise of the server hosting the code…

This concern exists currently anyway, although in a possibly-reduced-risk form; a compromised Affiliates could serve a malicious image currently, but in the new world a compromised Affiliates could serve malicious JavaScript. First, this assumes that we can make the choice to never distribute a banner with JavaScript, which I don’t think is realistic (eventually someone will want us to add a banner so fancy that it needs JavaScript). We could distribute the JavaScript as inline JS in the banner, but then any errors in the JS will never be corrected for users who already created banners.

Of course, if you are very concerned about the security of this banner, you can self-host the JavaScript. In fact, for advanced users, you can just grab the link from the banner code and construct whatever banner you want. Using the default banner code is balancing convenience with security, and I don’t think this is an unreasonable balance, especially given, say, the number of people relying on things like Facebook and G+ embed buttons or the jQuery CDN for some of their site.

> If the script/resources are not served via https (implying the original page is not https), this may make it easier for an attacker to attack the site/multiple sites by compromising common network paths to the Mozilla servers.

I think our current banner code uses protocol-relative paths, but off the top of my head I don’t see a reason why it can’t be HTTPS-always. I feel like there was a good reason for this once though.

> it helps when posts like this link to a bug where discussion occurred or a public mailing list archive to the relevant threads.

There are none! I’m the only dev actively coding for Affiliates right now (contributors don’t stick with Affiliates long because it doesn’t always have interesting work available), and after coming up with the idea, I flew it by a few smart people in IRC and decided to roll with it. I could’ve, in theory, looked for people who would care about this kind’ve decision and asked their opinion before coding it and moving forward, but IMO that would’ve resulted in not a whole lot of a feedback but a lot of waiting as the site’s launch date looms closer. Instead, we opted for making the decision and being open to feedback. It’s hard balancing development speed with responsiveness to the rest of the community, and this time we opted for speed.

> The affiliates link is useful but not quite topical, and if anything it makes me think that the motivation for this is to be able to extract more metrics/user-data by being able to further integrate google-analytics into the banner.

Never. Google Analytics or anything close to it in banner code would violate our Privacy Policy and our principles. Banner code that Affiliates embed on their sites will NEVER be used to track users.

(Now we do some tracking on users that click the banner, which visits affiliates.mozilla.org, but we pretty much have to do that to determine if a banner click resulted in a Firefox download, and to get the data we need to improve banner performance. Our privacy policy covers how this data is used and how long it’s retained.)

Sorry for the long-winded comment. I hope that gives you a bit more info. Let me know if you disagree with anything, and thanks for the feedback! :D

25 03 2014
Andrew Sutherland

Thank you for the fantastic reply, Michael. It’s fine for this to be the request for feedback as this is an exceedingly public form. In a lot of other cases the discussion phase has largely been concluded and it’s an oversight that links were not provided which is why I call that out. (Also, it help makes it clear that I am willing to do the legwork to understand the context but that the resources were not available to me.)

> a compromised Affiliates could serve a malicious image currently, but in the new world a compromised Affiliates could serve malicious JavaScript

These are very different risk profiles. A malicious image is limited to exploiting vulnerabilities in the legal MIME types for the img tag. This includes the well-tested image libraries and the SVG sub-system.

All JS executed in a page context operates with the same authority as the rest of the JS on the page. The only way to avoid this is through use of an iframe which inherently has a different origin. Replacement of the Mozilla-provided JS by an attacker allows the attacker full access to the cookies accessible to the page, the localStorage, the IndexedDB, et cetera.

> Using the default banner code is balancing convenience with security, and I don’t think this is an unreasonable balance, especially given, say, the number of people relying on things like Facebook and G+ embed buttons or the jQuery CDN for some of their site.

The G+ and Facebook like buttons are for use in social networks with an explicit “share” interaction paradigm that can provide some convincing justifications related to good UX. There is also some mututal benefit to that relationship with the site owner; by smoothing out the UX flow for sharing their content, they increase the chance of getting more people to view their site. Also, Google and Facebook are in the business of selling ads and inferring behaviour about website users, so they have bountiful ulterior motives. They aren’t really what you would hold up as paragons of putting their users ahead of themselves; Mozilla claims to put users first, so adopting that behaviour then lets Google/Facebook make the same argument with a stronger benefit derived.

I’d also make the argument that all 3 of the examples probably benefit from a significantly larger ops staff and monitoring than this use-case and are significantly likely to detect such an attack as perceived from multiple geographic regions.

As to the risky thing only being the default, I don’t realistically expect most of the people putting this type of banner on their webpage to be sufficiently skilled to assess the risk profile for themselves of following Mozilla’s suggestions. As someone who is technically able to assess the risks, if I found the default suggestion was a bad idea, I’d bail right there.

> This also can be reused as a pattern for other future dynamic banners that switch between images, whereas the old system was heavily biased towards banners that switch based on stuff Apache can know (for example, a banner that switched if you visited another Mozilla property? Difficult!).

A banner that changes based on what you do at other websites does not seem like a good thing. People already find it creepy when things they search for follow them around the web on ads. As a potentially absurd straw-man, the evangelism benefit of the positive message of “hey, you! Thanks for filing your first bug the other day, you rock!” if offset by the creepiness.

> https://github.com/Osmose/affiliates/blob/f4658c32457c2821b4e2b0219bae401eee653368/affiliates/banners/static/js/firefox_upgrade_banner.js

This accesses localStorage (synchronously). While it’s good that it avoids the additional network traffic that this change is introducing, localStorage is something Mozilla is explicitly trying to stamp out. It will also perform worse on desktop Firefox than competing multi-process browsers which is somewhat ironic. (Noting that localStorage performance has gotten less pathological on Firefox since around Firefox 20 or so.) For example, LocalStorage is categorically forbidden in Firefox OS Gaia apps on pain of being yelled at by :gal. So it’s nice to avoid localStorage use in Mozilla properties.

It’s also notable that because of the origin thing, it’s a bit jerky to use localStorage since you’re playing in the website’s origin/sandbox. If the site uses localStorage as a top-level object dictionary that gets enumerated, you will break their logic.

> The reason why this is easier than the old system is because the old system involved manually commiting banner image filenames (which are hashed because Adblock hates images with 500×1000 in their filenames) that are uploaded by an admin to a settings file, and auto-generating the htaccess file that swapped the images out. This file had to be re-generated on a regular basis in case there was an update out. The better alternatives mostly involve hardcoding the upgrade banners into the codebase, which makes it difficult when we want to add things that relate to banners stored in the database vs hardcoded banners.

This doesn’t actually sound that bad. If I grok, there are 2 reasons things change:
1) Intentional desire to change something about the banners involving human interaction and the Django management UI.
2) A new firefox release came out and an update is required.

It sounds like situation 1 is largely already automated. It seems like situation 2 could be automated by a cron job that runs the same logic from situation 1. Assuming idempotent logic that fetches the JSON blob characterizing, re-computer things, and generates the current expected state of the .htaccess file, the cron job then just does an hg diff/git diff/whatever tells us if there’s a change we need to commit. If so, existing tooling like bzexport (http://hg.mozilla.org/users/tmielczarek_mozilla.com/bzexport/) can be used to attach a diff to bugzilla, or a github pull request, or any other number of fairly minimal-effort options up through automatically committing the change and sending it after the fact.

I think such a semi-automated procedure could reduce whatever the existing hassle-burden is without radically altering the security and performance profile of the banner. As is probably clear, I do feel strongly about the right course of action here, so do feel free to cc/needinfo me on any bug if you would like any input or some patch proposals. I’m in the (mozillians.org) book and did a fair amount of django devwork back in the day.

26 03 2014
Michael Kelly

(There’s no reply button on your reply, so I’m just replying to myself)

> The only way to avoid this is through use of an iframe which inherently has a different origin.

Getting the styling right might be a bit difficult (I’ve never really styled an iframe before so I dunno if it’s finnicky or not) but I don’t otherwise see a major issue with this.

> Also, Google and Facebook are in the business of selling ads and inferring behaviour about website users, … They aren’t really what you would hold up as paragons of putting their users ahead of themselves; …

I wasn’t using them as an example of companies putting users ahead of themselves, but instead as an example of what users are already doing. My point was that no one is going to change how they view including third-party code in their pages because of Affiliate banners, and we have options for people who do want security-conscious options, so there’s little benefit in avoiding doing certain techniques if we don’t abuse them ourselves.

> A banner that changes based on what you do at other websites does not seem like a good thing.

I had to come up with an example, and I suspect there’s a balance for that kind’ve banner that works, but you’re right, it’s a strawman anyway. There’s better examples.

> This accesses localStorage (synchronously).

Namespace-wise, you have a point. Using an iframe as you suggested would help here. Performance-wise, the intersection of “people who need such tight website performance as to avoid a few localStorage operations” and “people who are using Affiliate banner code without being willing to customize it to their needs” is, I strongly suspect, empty, or at least very small. Using localStorage over the alternatives and making it work cross-browser is a simpler option with a rather small performance impact, especially considering the audience.

> It sounds like situation 1 is largely already automated.

So after thinking harder about it and your comments, I’ve realized the problem with situation 1 isn’t that it’s not automated (and once you add the image filename to the static list it’s mostly automated) but that it requires syncing two sources of information; the images uploaded by an admin and the static list of image filenames. It’s a fragile setup that I really want to avoid, because it’s easy for an admin to change one of the images and break the whole thing, plus changing the other, automatically subbed-in image requires a deploy of the whole site since they aren’t uploaded via the admin.

A possible solution is to store the upgrade banners as a seperate table in the database, and generate the htaccess file whenever those models are saved, or on a cron job in order to change when the Firefox version updates.

There’s a few potential downsides here due to technical details (banner image filenames include the image size, thus a slight change in image size would break existing banners unless we log all filenames ever used for these banners ever somewhere, which is annoying), but it’s another viable option overall. I’ll have to think about it some more.

26 03 2014
Andrew Sutherland

Yes, apparently they took a short-cut to avoid really deeply nested threads! :)

Thanks for thinking about this more; two last thoughts:

I’d like to clarify that the downside of localStorage is not just a website performance thing, it’s a Firefox performance thing. As you can see at http://dxr.mozilla.org/mozilla-central/source/dom/src/storage/DOMStorageCache.cpp#443 the main-thread of the Firefox process will synchronously block until the database read completes. In a worst-case scenario, the use of localStorage can cause Firefox to become unresponsive for seconds.

We do have numbers on these thanks to telemetry, and the good news is high numbers are rare but do happen: http://telemetry.mozilla.org/#nightly/31/LOCALDOMSTORAGE_GETVALUE_BLOCKING_MS

But the bad news is still that if we block for 16.6 ms, Firefox will inherently be unable to maintain anything at 60fps that involves the main thread.

> there’s little benefit in avoiding doing certain techniques if we don’t abuse them ourselves

In general I find it’s better architecture to do things in such a way that ongoing trust/competence doesn’t come into the picture. Firefox Sync was amazing because it wasn’t a question of “we’re Mozilla, we’re the good guys, we promise we won’t read your bookmarks and we are amazing at IT-stuff so we will never get hacked and let bad people read them.” Sync’s thing is/was “we’re Mozilla and we’ve designed the system so that we can’t read your bookmarks even if we tried.” Because of UX decisions made for a flow for user benefit based on user research into expectations of being able to restore data, the risk profile has slightly changed to “we would have to brute-force your password to read your bookmarks and we engineered it to be very very expensive to brute force.”

In this case I still feel like the provisional decision is being made to optimize for the benefit of Mozilla because of limitations in the site admin console and site deployment process at the expense of the site-owner (who is doing us a solid by including the banner) and the potential banner-clicker. If the effort/cost to maintain the changing banner images is that bad, I would argue it’s a more “Mozilla-y” call to only have static banner images that never change rather than the provisional strategy.

25 03 2014
Andrew Sutherland

Uh, and replace “threats” with “threads” in the second paragraph. While analysis of threats to the sites/users using the code is important, the discussion threads are the useful thing to link to :)

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s




Follow

Get every new post delivered to your Inbox.

Join 1,512 other followers

%d bloggers like this: