Device Telemetry Stack on Separate Domain

[ebollens]

When the MWF does it's refresh to pass telemetry from client to server on the first page load, the cookie definition is occurring against the content provider host, not the service provider host.

This bug doesn't manifest for service providers who use the front splash page, because the write goes against the service provider and then stays set throug other pages. However, if you're running this in an environment where the framework is completely separate from even the front page, the cookie is set to the wrong place.

[ghost]

I presume all service provider and content provider will have the same domain (i.e. ucla.edu) and the cookie will be set at the domain level with path = '/'. We're setting that limitation for js and css loading and images compression anyways, right?

[ebollens]

Ike, in terms of our "federation" principle, that assumption isn't necessarily true - or hasn't been documented as a requirement if we want to require that. Given that we're headed towards a SaaS model, I don't think forcing this is a reasonable choice. Therefore, I'm going to look into getting this resolved by our next milestone.

[ebollens]

It appears there are two options for delivering the cookie to the Service Provider domain:

1. Setting a third-party cookie for the service provider using the domain cookie attribute.
2. A double redirect through the service provider and back to the content provider.

Given the implications of redirects, minimizing these is good, so (1) takes priority. However, need to figure out a way to determine if (1) is failing, and in that case try to take advantage of (2) instead. The reason for this is that many browsers allow you to prevent third-party cookies and, given the increasing fears of being "tracked" on the internet, I think that this setting will become more prevalent, not less.

[ebollens]

To resolve this issue, it will be necessary to rework '''mwf.override''' and '''mwf.server'''.

The new proposed process is outlined as follows:
http://erb1.ats.ucla.edu/upl/DTS-v2.pdf

Essentially, it will do the following:

1. Service Provider will define in vars.php if it already has the necessary cookies.
2. If it does not, script running on content provider will determine cookie support.
3. If no cookie support, then script concludes without a redirect.
4. If on the first page load and likely to support third party cookies, will try to write and reload.
5. If not on the first page load or not able to support third party cookies, will redirect to the service provider.
6. On redirect to service provider, cookies will be set in domain and then user redirected back.

In the linked diagram, there's also a path for override.

There are a few caveats here:

• The '''mwf.capability.cookie()''' check needs to be 100% accurate, or at least 100% averse to false positives. If it claims that cookies are supported when in fact they are not, this will cause an infinite redirect. I have yet to determine a way to ensure that this sort of a redirect does not occur consistently. One option, that would at least alleviate the infinite-ness of it, would be to pass a GET parameter back into the content provider that signals its not the first page load. This would cause every page load to require two page loads, so still a bad use case, but better than unlimited.
• It's important we don't try to set a third party cookie when we cannot. This setting requires a redirect, and if it isn't successful, then we still have to do two more redirects. However, we also want to minimize the case where we don't try third party cookies when they are viable, as the non-third-party method incurs an additional HTTP request in comparison to the third-party method.
• This implementation inherently supports the same-domain case, as it's simply writing a first-party cookie instead of a third-party cookie, and in this case, it should simply use a single redirect. This is the case supported by the current implementation of DTS, and keeping the control path as such keeps its behavior consistent with what is already in DTS.

As should be noted, this keeps current behavior the same while supporting the separate domain case, which is what this bug report is about. Please let me know if anything seems inconsistent.

[ghost]

The new flow looks good. Just throwing another idea out thereŠ Any
concerns about using window.top.name?

http://www.thomasfrank.se/sessionvars.html

Any script on the page can read and write the data and the data persists
as long as the window/tab is open. But since we're not storing any
sensitive information -- I'm not too concerned about it. It could be a
good fall back to cookie. And if the data in window.name is not we
expected (meaning other script is using it), we just do the redirect.

On 10/31/11 9:39 AM, "Eric Bollens"
reply@reply.github.com
wrote:

To resolve this issue, it will be necessary to rework '''mwf.override'''
and '''mwf.server'''.

The new proposed process is outlined as follows:
http://erb1.ats.ucla.edu/upl/DTS-v2.pdf

Essentially, it will do the following:

1. Service Provider will define in vars.php if it already has the
   necessary cookies.
2. If it does not, script running on content provider will determine
   cookie support.
3. If no cookie support, then script concludes without a redirect.
4. If on the first page load and likely to support third party cookies,
   will try to write and reload.
5. If not on the first page load or not able to support third party
   cookies, will redirect to the service provider.
6. On redirect to service provider, cookies will be set in domain and
   then user redirected back.

In the linked diagram, there's also a path for override.

There are a few caveats here:

• The '''mwf.capability.cookie()''' check needs to be 100% accurate, or
  at least 100% averse to false positives. If it claims that cookies are
  supported when in fact they are not, this will cause an infinite
  redirect. I have yet to determine a way to ensure that this sort of a
  redirect does not occur consistently. One option, that would at least
  alleviate the infinite-ness of it, would be to pass a GET parameter back
  into the content provider that signals its not the first page load. This
  would cause every page load to require two page loads, so still a bad use
  case, but better than unlimited.
• It's important we don't try to set a third party cookie when we cannot.
  This setting requires a redirect, and if it isn't successful, then we
  still have to do two more redirects. However, we also want to minimize
  the case where we don't try third party cookies when they are viable, as
  the non-third-party method incurs an additional HTTP request in
  comparison to the third-party method.
• This implementation inherently supports the same-domain case, as it's
  simply writing a first-party cookie instead of a third-party cookie, and
  in this case, it should simply use a single redirect. This is the case
  supported by the current implementation of DTS, and keeping the control
  path as such keeps its behavior consistent with what is already in DTS.

As should be noted, this keeps current behavior the same while supporting
the separate domain case, which is what this bug report is about. Please
let me know if anything seems inconsistent.

Reply to this email directly or view it on GitHub:
#6 (comment)

[ebollens]

Ike, this is an incredible find. I'd rather rely on cookies than not, but I think we can incorporate it in two ways:

1. Help us prevent the infinite loop problem if mwf.capability.cookie() is false positive.
2. As you mentioned, a fallback for cookies.

Very cool hack.

[Trott]

The window.name hack is very cool, but I'm not entirely clear on something:

Isn't the point of the cookies to make it so that the server side stuff has access to the telemetry data? How is the server side going to see the window.name values? I guess JavaScript could write them into a bunch of GET params and some custom PHP on the server side could write that to SESSION variables or something like that, all AJAX-style. Except that SESSION probably requires a cookie.

But I'd add that if we're going to go that route, then we might consider doing the same thing (perhaps with HTML5 Local Storage and/or Session Storage for iPhones and Android instead of the window.name hack) no matter what and dispense with cookies altogether. Or maybe not. Perhaps I'm just confusing the issue. But it might be nice to not have to send around all those cookies with every HTTP request. Of course, PHP SESSION stuff probably works via cookies. But it's probably one very small cookie compared to three.

[ebollens]

Rich, the point of this storage is to let us know if we've already loaded the page - to prevent a false positive infinite loop on a browser that doesn't support cookies but is identified as though it does. I'm going to be implementing a "strict" mode for cookie checking that is a lot more stringent, though a bit more costly in terms of performance, in order to minimize the false positive, but because of the outcome of the false positive, any method to make sure we don't infinite loop is important.

[Trott]

Ah, that totally makes sense now, although depending on how the implementation plays out, a URI parameter might (or might not) fit the bill a little more elegantly than the window.name hack.

[ebollens]

I have committed an initial overhaul of mwf.server:
0e6c25c

This tries to use the third-party method, and falls back to redirecting through a script that then passes the visitor back to the originating page. This has a cost of 1 HTTP request if same-origin as framework or third-party write works, 2 HTTP requests if no third-party support is guessed correctly and pass through is used, and 3 HTTP requests if third-party write is thought to work but fails and then it has to do a passthru.

Based on my experimentation, a couple questions:

1. It does not seem that many desktop browsers support third-party cookies. Mobile Safari for iOS does not either. Do we know if Android does? If it doesn't, it might make sense to strip out the third-party cookie portion, as a bad guess is an expensive failure, requiring an additional load.
2. I'm really not sure what to do with caching anymore. The js.php file isn't cacheable on first load anymore, because js/core/vars.php dynamically defines a set of variables. However, not caching it at all long-term sucks, though if the cookies expire on the server, we need to know. So what's the right approach here? One thing I was thinking is potentially a dynamic write of another script tag that loads everything except the core stack in a separate js file that is cacheable. However, again, it isn't actually fully cacheable because it accepts parameters that change its contents. Thoughts?

The next steps are as follows:

• (Others) Provide feedback on the above commit and questions.
• (Me) Develop the override support for this functionality.

Please let me know your thoughts. This is the beginning of a pretty large shift in the paradigm.

As a note, it does follow the flowchart posted earlier in this topic.

[Trott]

I recommend drinking the TDD Kool-Aid (it's delicious!) and writing unit tests for the new JavaScript methods.

[Trott]

I'm on it for the unit tests for the JS stuff you've written thus far so don't worry about that stuff.

[Trott]

Might it be better to send 0s and 1s for the settings in the classification cookie rather than the entire "true" and "false" strings that we're sending? These cookies get sent on every HTTP request, so one could make the argument that every byte counts. (We also might want to abbreviate things like we do in the other cookies so f for full and s for standard?)

[ghost]

Android doesn't support third-party cookies either. I'm in favor of
getting rid of that portion.

As far as caching is concerned, here's another blue sky idea that is
similar to what you are proposing:

<script src="http://m.ucsd.edu/assets/js/mwf.js"></script>

mwf.js does the detection and reloads if first time visiting. If not the
first time visiting (no cookie or no window name), write another script
tag to load js.php with the appropriate classification params. Since the
params don¹t change, js.php will be cached.

If javascript is turned off, then just load the basic css.

I realize this is a dramatic departure from our current approach. But I'm
just throwing out ideas here perhaps for future reference.

On 11/2/11 9:50 AM, "Eric Bollens"
reply@reply.github.com
wrote:

I have committed an initial overhaul of mwf.server:
0e6c25c

This tries to use the third-party method, and falls back to redirecting
through a script that then passes the visitor back to the originating
page. This has a cost of 1 HTTP request if same-origin as framework or
third-party write works, 2 HTTP requests if no third-party support is
guessed correctly and pass through is used, and 3 HTTP requests if
third-party write is thought to work but fails and then it has to do a
passthru.

Based on my experimentation, a couple questions:

1. It does not seem that many desktop browsers support third-party
   cookies. Mobile Safari for iOS does not either. Do we know if Android
   does? If it doesn't, it might make sense to strip out the third-party
   cookie portion, as a bad guess is an expensive failure, requiring an
   additional load.
2. I'm really not sure what to do with caching anymore. The js.php file
   isn't cacheable on first load anymore, because js/core/vars.php
   dynamically defines a set of variables. However, not caching it at all
   long-term sucks, though if the cookies expire on the server, we need to
   know. So what's the right approach here? One thing I was thinking is
   potentially a dynamic write of another script tag that loads everything
   except the core stack in a separate js file that is cacheable. However,
   again, it isn't actually fully cacheable because it accepts parameters
   that change its contents. Thoughts?

The next steps are as follows:

• (Others) Provide feedback on the above commit and questions.
• (Me) Develop the override support for this functionality.

Please let me know your thoughts. This is the beginning of a pretty large
shift in the paradigm.

As a note, it does follow the flowchart posted earlier in this topic.

Reply to this email directly or view it on GitHub:
#6 (comment)

[ebollens]

Rich:

• Sounds good on you doing Unit Tests. Its actually probably more effective that the person architecting the solution isn't the one writing the tests, given that then they'll just check out on what I've planned for, not what someone else might think of.
• I like the idea of condensing. I'll work on a future commit to feature/dts to resolve this.

Ike:

• Good to note on the third-party cookie. I'd be content with getting rid of it. It's a lot of code, and more convolution in logic, and if over three quarters of our visitors can't leverage it, then it's all but useless to us.
• We can't change the interface by which users load the MWF, at least not in a minor or revision release. This could be a change for MWF 2, but it doesn't fit for MWF 1.x because of our version paradigm. Very hard to transition people to the new version. Let's talk more about this and if it's a reasonable strategy during the call on Friday, because we could add it in and start pushing content providers towards it, but we need to continue providing functionality through js.php and css.php at least as long as we're in the MWF 1 major version scheme.

All, as a note, this will be a discussion during our meeting on Friday.

[Trott]

This is a side issue and totally not the place to have a lengthy conversation about it, plus since it's arguing with Eric I expect to get thoroughly spanked, but just for the record: I believe that the programmer writing the code should ideally write the unit tests. See anything ever written on TDD or, for an abbreviated take, http://stackoverflow.com/questions/5154371/who-should-write-tests. Functional tests? Yeah, someone else. Unit tests: Totally the programmer writing the code, word is bond.

But back to the much more pressing and important issue of what to do with this cross-domain telemetry stuff: Yikes. I said, yikes. I'll try to think about this some more tonight, but my head hurts.

[ebollens]

On testing, Rich wins ;)

As for development, I think we all win (at least preliminarily):

• 7c351a0 - Minor refactoring of existing functionality.
• 3039560 - Remove third-party cookie functionality.
• b8e398a - Add override functionality.

Some pretty hefty stuff in there to accommodate the cross-domain functionality, but honestly, rewriting mwf.server and mwf.override has actually had the effect that I think they're a bit more clear than they used to be. Could probably still be improved in clarity, mostly by way of a state diagram - pretty tricky keeping track of where each thing is given the way that classification, override and server cookies are all linked to each other.

Please check this out and let me know how its working for you all, as well as provide feedback.

We'll discuss it during tomorrow's meeting as well.

[Trott]

I wrote a few more unit tests, but in commit c19e220, I put in 7 unit test stubs that need writing. They are all marked with @todo. Any chance you could bust them out?

[ebollens]

Over capacity with work through the weekend I'm afraid - but will work on them early next week if needed - at least in time for the upcoming release.

[Trott]

I hear if you get within 30 meters of a hookah bar, that you get super-human coding powers.

[ebollens]

Hmmmm... I'm thinking one may be in need after the developer's meeting!

[Trott]

Oh, wait, it's not super-human coding powers. You just lose your ability to fly. Sorry, I got confused.

[Trott]

Any idea when you intend to merge feature/dts into develop-1.2 so that people can start integration testing in earnest ahead of the Wednesday release? Any reason not to go ahead and do that now-ish?

[ebollens]

I had hoped to get more weigh in from people before merge on Crucible.

Unfortunately, this hasn't been the case.

Therefore, yeah, merge to develop-1.2 is now done in cc581ab.

[Trott]

Sorry you didn't get the level of feedback you were hoping for. I'll see about giving this some good testing now.

[Trott]

Can we clarify what we mean by separate domain?

The comment for the cookie_domain setting says "The domain where the framework resides - this is REQUIRED for any framework service provider that has content providers leveraging it from different domains."

The term "domain" is vague in this context. Would m.ucsf.edu be a different domain from www.ucsf.edu? Or is it all good for everyone on ucsf.edu domains and I only need to set the setting if I want content providers from ucsfmedicalcenter.org to use the framework instance installed on m.ucsf.edu?

[ebollens]

Cookie domain is the domain that the cookie should define as "same-origin".

The cookie is written through document.cookie on the framework server, so your origin is the domain of that cookie.

Thoughts on how to document this correctly?

[Trott]

Examples would probably be the most helpful. Two things in particular:

1. When to use. Which of these are valid use cases for this?

A) If I want www.example.com and m.example.com to both be able to read the MWF cookies served by m.example.com, do I need to set this value?

B) How about example.com and m.example.com?

C) Will it work for example.org or frobozz.biz and m.example.com?

2. What to set it as:

A) "example.com"
B) ".example.com"
C) "m.example.com"

[ebollens]

Rich, if the framework runs on m.ucsf.edu, then valid domains are:

m.ucsf.edu
ucsf.edu

Generally, we'd advise the "tighter" one and not the looser one, as it limits the poisoning surface.

I'm thinking then that we want to say something along the lines of:

"cookie_domain" - A valid domain for the cookies that the framework sets. This must be the exact subdomain, or any higher-level domain, that the framework resides under. For example, if the framework resides at m.example.com, it can be set to m.example.com or example.com.

That said, though, this documentation does bring up another point: in reality, the cookie isn't explicitly defining it's domain based on this value. I'm going to push a future update so that it does, such that it will conform to the above statement - right now, I think that it requires the most descriptive subdomain.

[ebollens]

Rich, interestingly, this is the line causing the loop in Chrome:

if(mwf.site.cookie.domain)
                cookieSuffix += ";domain="+mwf.site.cookie.domain;

According to RFC 2109, it should actually be:

if(mwf.site.cookie.domain)
                cookieSuffix += ";domain=."+mwf.site.cookie.domain;

It's safe to just impose the dot because I strip any first dot in the PHP before setting mwf.site.cookie.domain in vars.php.

However, this doesn't resolve the problem in Chrome, while simply removing it does.

I therefore propose the following changes:

• Do not give the user a choice of the depth of their cookie domain. We'll require it to be the SP (sub)domain.
• Remove "cookie_domain" configuration variable and instead ascertain from mwf.site.asset.root.
• Remove domain specificity in the cookie definition - it will default to SP (sub)domain.
• Adjust mwf.site.local.isSameOrigin() to account for this.

I'll have a commit up in a few minutes that accomplishes this.

[Trott]

On getting it to loop in some browsers and not others, is it possible you skipped the clear-out-the-cookies step? That made the difference for me in iOS Simulator. Well, whatever, looks like we might have a fix, thanks, I'll get testing.

[ebollens]

Patched this in develop-1.2 with commit 6c0bd00.

This commit no longer provides the config option for domain, and removes domain specificity.

Instead, it assumes that one must be under the SP's tightest domain in order to not have to redirect.

As an example, assume the SP is at http://framework.m.example.com/1.2/assets. In this case, any path under framework.m.example.com is valid, such as subdomains like apps.framework.m.example.com and paths like framework.m.example.com/1.3. However, less tight subdomains like m.example.com and example.com are not valid, nor are sibling-level subdomains like other.m.example.com. This might sound like a "rigid" rule, but in reality, it simply means that if you're outside of the same origin as the framework, then you definitely redirect through passthru.php. Removes a lot of user-related configuration risk.

Let me know what you think.

[ebollens]

As an aside, Opera is still not behaving nicely. Seems to think we're doing CSRF.

[Trott]

This seems to have fixed the infinite redirect loop although a warning for UCSD: Using an offline appcache will result in infinite redirect loops for browsers that don't already have the cookies set. I guess you can get around it by making sure you don't cache the wrong JS, but I'm not sure if that will completely defeat the purpose of offline caching. More testing required, I guess.

[ebollens]

Rich, I'm open to thoughts to make this work with app cache. However, if that's going to be a target, I'd like to slate it for a future revision release so that we can test this thing hard today and get it packaged in a 1.2.06 release tomorrow on schedule.

[Trott]

(I guess it doesn't completely defeat the purpose, as you still get a performance boost on loading images etc.)

[ebollens]

Yeah, but it's still a loss, so let's slate it for future investigation.

[Trott]

On my Mac web server, vars.php generates a piece of JS that looks like this:

this.classification="{\"mobile\":false,\"basic\":true,\"standard\":true,\"full\":true}"

Cool.

But on CentOS, it doesn't backslash escape the quotes and the JS blows up. If you know why I'm seeing this and can save me the trouble of figuring it out, awesome.

[ebollens]

Must be with the way that PHP versions or deeper OS differences handle $_COOKIE.

See vars.php line 35:

$classification_cookie_var = '"'.$_COOKIE[$prefix.'classification'].'"';

[ebollens]

Since we know the form, maybe the solution is to do: addslashes(stripslashes($_COOKIE[...

[Trott]

It's the php_ini setting magic_quotes_gpc. We need to use ini_get('magic_quotes_gpc') and add the slashes ourselves or not based on what the server setting is. Or at least that's my take based on five minutes of research. Fun!

[ebollens]

I'm glad they've deprecated it in PHP 5.3, but do you want to go ahead and commit the fix? I'm about to run to class, so won't have time to dev/test for a couple hours.

[Trott]

On it.

[Trott]

Options here:

http://www.php.net/manual/en/security.magicquotes.disabling.php

[ebollens]

Let's implement the runtime one, but do it as a function that we can pass the cookie to in order to have it fix the cookie. Don't want to modify the $_ globals themselves, because that's just a lot of wasted effort in most cases.

[Trott]

Heh, just read this now. OK, will refactor. Was going to say that we might want to put it in a globally loaded/executed place so that all scripts can just assume that magic_quotes_gpc is off, but your idea is better, I think.

[Trott]

Actually, I have to go to a meeting, so I won't get to this right now. But I wonder if we should implement this as a sort of utility helper static class.

[ebollens]

I agree that encapsulating it static within a new utility class (like the HTTPS class for example) makes sense.

[Trott]

I refactored everything to use the static cookie class except for unset_override.php. Someone else should take a look at that one because it looks to me like it doesn't work. Specifically:

if(substr($name, 0, 14) == Config::get('global', 'cookie_prefix').'ovrrdr')

Am I wrong to think that this line depends on the cookie_prefix being exactly six characters or whatever? By default, it's four.

Can you take a look at that script and confirm that I'm not crazy? I'm heading out for the night, so if you have some spare cycles to fix it, awesome. Even more spare cycles to add in the static Cookie stuff for it, then even better. (I think we'll have to add another method or overload an existing one in the static Cookie object for this script to work with it, but I'd be happy to be wrong about that.)

Longer term (like, next release) we should do the same for POST, GET, and REQUEST, as they are subject to magic_quotes_gpc too.

[ebollens]

Rich, thanks for doing that refactor. It looks great!

As for unset_override.php, definitely buggy behavior. Looking at the date stamp, this is legacy code from MWF 1.0 (when it was just called UCLA Mobile), and it may have worked standalone in that environment given that the UCLA Mobile cookie was defined as uclamob_ovrrdr. However, in the MWF context, it doesn't work. Because of the way people interact with the redirect, there's generally not much exposure to this script, though, hence explaining why no-one caught it. The fix is actually as simple as simply removing the substring, which is what I have done in 6f21167.

I have run another set of tests and everything seems to be working on my end. I'm going to close this issue, and unless there are any showstoppers, I will package this tomorrow afternoon as v1.2.06. What a road this cross-domain issue has been.

[Trott]

I don't think the fix to unset_override.php quite works because when the cookie is set, it is set as prefix + overrdr + optional domain key. This will miss cookies set with the domain key.

If the domain key stuff isn't needed, I can remove it and that would make this working code....

[ebollens]

This food poisoning thing really got me off my game.

But yes, you are definitely right. The change broke the optional domain component. Fix is trivial, though. Simply reimplementing the substr, but using an strlen to drive it's concatenation. This is done in 1a5b513.

[Trott]

This is all because of the food poisoning? No more eating for you.

[ebollens]

If there are no more issues, then I'm going to release v1.2.06.

[Trott]

I think we're good to release at this point.