domenic / promises-unwrapping

The ES6 promises spec, as per September 2013 TC39 meeting
1.24k stars 95 forks source link

[[PromiseConstructor]] vs .constructor and instanceof #65

Closed rossberg closed 10 years ago

rossberg commented 10 years ago

I wonder why we need [[PromiseConstructor]]. Wouldn't it be more consistent with JavaScript mechanics to stick to .constructor? The information is not used for branding, so being overly defensive shouldn't be necessary here.

Moreover, why does Promise.cast compare against that constructors for equality? Wouldn't it make more sense to do an instanceof check? That is, if x already has a "subtype" of this, then don't wrap it.

domenic commented 10 years ago

The idea of Promise.cast is to be able to give you a high-integrity version of an untrusted promise. So the instanceof check is not what you want, because you want to be able to do stuff like:

ses.def(Promise); // deeply freeze Promise, securing it against modification.

var thing = getUntrustedPromiseOrThenable();

Promise.cast(thing).then(function () {
  console.log("2");
});

console.log("1");

function getUntrustedPromiseOrThenable() {
  return new class extends Promise {
    then(f, r) {
      f(5);
    }
  };
}

If Promise.cast did an instanceof check, it would return thing straight away, and you would end up logging "2" then "1" instead of the desired "1" then "2". By securing Promise and then using Promise.cast, you "downcast" to a known-safe instance.


As for why it doesn't use .constructor... hmm. Ah, right, I remember. So that things can't fake you out by faking their constructor. E.g.

function getUntrustedPromiseOrThenable() {
  return {
    then(f, r) {
      f(5);
    }

    constructor: Promise
  };
}
rossberg commented 10 years ago

On 30 October 2013 02:14, Domenic Denicola notifications@github.com wrote:

The idea of Promise.cast is to be able to give you a high-integrity version of an untrusted promise. So the instanceof check is not what you want, because you want to be able to do stuff like:

ses.def(Promise); // deeply freeze Promise, securing it against modification. var thing = getUntrustedPromiseOrThenable(); Promise.cast(thing).then(function () { console.log("2");}); console.log("1"); function getUntrustedPromiseOrThenable() { return new class extends Promise { then(f, r) { f(5); } };}

If Promise.cast did an instanceof check, it would return thing straight away, and you would end up logging "2" then "1" instead of the desired "1" then "2". By securing Promise and then using Promise.cast, you "downcast" to a known-safe instance.

Note that instanceof would change only the case where Promise.cast actually tries to perform an upcast -- which should normally be a no-op. Sure, even a subclass might be broken, but that's OO, and we seem to accept the implications everywhere else.

I admit that I'm at a bit of a loss what the specific scenario is where high integrity would matter. Why is there a desire for promises to natively support higher integrity than every other class in the language? (Of course, nothing keeps a secure library from providing that itself. You can implement a "secure" cast yourself.)


As for why it doesn't use .constructor... hmm. Ah, right, I remember. So that things can't fake you out by faking their constructor. E.g.

function getUntrustedPromiseOrThenable() { return { then(f, r) { f(5); }

constructor: Promise

};}

I understand, but again, why do we try to fix this for promises when it is considered acceptable for every other builtin class?

domenic commented 10 years ago

Note that instanceof would change only the case where Promise.cast actually tries to perform an upcast -- which should normally be a no-op.

I don't understand. I gave an example of attempting to perform a downcast from a bad subclass to a trusted base class, and then showed how if you switched to instanceof, that would not work.

For upcasts, Promise.cast has a different use: e.g. doing CancellablePromise.cast(nonCancellablePromise).cancel() should work, since it upcasts from a base promise which does not have a cancel method to one that has a cancel method. Again this is not a no-op.

I admit that I'm at a bit of a loss what the specific scenario is where high integrity would matter.

Avoiding flow control hazards as shown above---"zalgo" in today's parlance---is a pretty common desire.

Of course, nothing keeps a secure library from providing that itself. You can implement a "secure" cast yourself.

This is not true without some form of reliable branding, which is what is provided by [[PromiseConstructor]].

rossberg commented 10 years ago

On 30 October 2013 20:39, Domenic Denicola notifications@github.com wrote:

Note that instanceof would change only the case where Promise.cast actually tries to perform an upcast -- which should normally be a no-op.

I don't understand. I gave an example of attempting to perform a downcast from a bad subclass to a trusted base class, and then showed how if you switched to instanceof, that would not work.

Your example does an upcast, not a downcast. ;)

But in any case, I fail to see what you've won with the cast in your example. Once you invoke .then on the new promise that will rely on calling .then on the original promise anyway. And that is still broken.

For upcasts, Promise.cast has a different use: e.g. doing

CancellablePromise.cast(nonCancellablePromise).cancel() should work, since it upcasts from a base promise which does not have a cancel method to one that has a cancel method. Again this is not a no-op.

That would be a downcast.

I admit that I'm at a bit of a loss what the specific scenario is where

high integrity would matter.

Avoiding flow control hazards as shown above---"zalgo" in today's parlance---is a pretty common desire.

I don't see it. How is wrapping it into a new promise supposed to magically fix a bad promise? What can you reliably do with it?

Of course, nothing keeps a secure library from providing that itself. You

can implement a "secure" cast yourself.

This is not true without some form of reliable branding, which is what is provided by [[PromiseConstructor]].

No, branding is perfectly possible using a simple WeakSet. Or private properties, but TC39 botched that.

erights commented 10 years ago

I admit that I'm at a bit of a loss what the specific scenario is where high integrity would matter. Why is there a desire for promises to natively support higher integrity than every other class in the language?

Hi Andreas, protection from plan interference attacks has always been a major purpose of this non-blocking style of promises starting from their invention in E. See chapter 13 of my thesis and http://www.infoq.com/presentations/Secure-Mashups-in-ECMAScript-5 , slides at https://es-lab.googlecode.com/files/RemainingHazardsAndMitigatingPatterns.pdf , where the relevant attack and defense starts at slide 44. The Q library as implemented at https://code.google.com/p/es-lab/source/browse/trunk/src/ses/makeQ.js as part of SES and Dr. SES has all these integrity and security properties.

Such protection becomes mush more pressing in ES6 because of the introduction of proxies. Proxies introduce many more interleaving points. The expression

Promise.cast(untrustedA).then(untrustedB, untrustedC)

must not run any code contributed by these untrusted objects during the call, or during the turn in which this call occurs. See the extensive discussion at Promises/A+ about the empty-stack requirement, where these attack and defense issues were discussed.

domenic commented 10 years ago

Apologies for the terminology confusion. But, the key point here is "real" promises guarantee that their handlers will execute in future terms, no matter how bad the thenable they are resolved to is. So in my original example, Promise.cast as currently specified will ensure you log "1" then "2".

rossberg commented 10 years ago

On 30 October 2013 22:48, Mark S. Miller notifications@github.com wrote:

I admit that I'm at a bit of a loss what the specific scenario is where high integrity would matter. Why is there a desire for promises to natively support higher integrity than every other class in the language?

Hi Andreas, protection from plan interference attacks has always been a major purpose of this non-blocking style of promises starting from their invention in E. See chapter 13 of my thesis and http://www.infoq.com/presentations/Secure-Mashups-in-ECMAScript-5 , slides at https://es-lab.googlecode.com/files/RemainingHazardsAndMitigatingPatterns.pdf, where the relevant attack and defense starts at slide 44. The Q library as implemented at https://code.google.com/p/es-lab/source/browse/trunk/src/ses/makeQ.js as part of SES and Dr. SES has all these integrity and security properties.

Such protection becomes mush more pressing in ES6 because of the introduction of proxies. Proxies introduce many more interleaving points. The expression

Promise.cast(untrustedA).then(untrustedB, untrustedC)

must not run any code contributed by these untrusted objects during the call, or during the turn in which this call occurs. See the extensive discussion at Promises/A+ about the empty-stack requirement, where these attack and defense issues were discussed.

OK, I see. Thanks for the pointers, and for patiently ignoring my ignorance of previous debates. :) I understand better now why this is something you want to do in a secured environment. But I still don't understand why this is a concern of ES, and not functionality that e.g. Caja/SES or similar environments would provide. That is not to say that I'm opposed to including it in ES proper, I just don't understand the rationale. It seems to me that at least one of the following should be the case to justify it:

  1. The feature is clearly useful independent of more comprehensive security frameworks.
  2. The feature cannot be implemented reliably or efficiently in such frameworks.

I'm still having difficulties seeing how either of these is the case.

/Andreas

erights commented 10 years ago

Why are encapsulation, modularity, abstraction, defensive programming useful independent of more comprehensive security frameworks? Why have lexical scoping, Object.freeze, deterministic WeakMaps, invariant-preserving direct Proxies etc, run-to-completion turns? Many of the improvements to modern ES starting with ES5 came from trying to move it towards those reliability and integrity lessons that came from the object-capability world. This is the "security as extreme modularity" premise.

Also, we're trying to minimize the number of differences between SES and ES that need to be explained, so that an ES programmer can know what to avoid so that their code is SES compatible.

The particular protection at stake here is for the same reason as run-to-completion turns and keeping generators shallow: to avoid plan interference by interleaving, and to enable code to be defensive against such interference. Proxies make the interleaving issue so much worse that it becomes even more pressing to find a way to defend against it.

rossberg commented 10 years ago

On 31 October 2013 13:28, Mark S. Miller notifications@github.com wrote:

Why are encapsulation, modularity, abstraction, defensive programming useful independent of more comprehensive security frameworks? Why have lexical scoping, Object.freeze, deterministic WeakMaps, invariant-preserving direct Proxies etc, run-to-completion turns? Many of the improvements to modern ES starting with ES5 came from trying to move it towards those reliability and integrity lessons that came from the object-capability world. This is the "security as extreme modularity" premise.

Well, you know that I'm all for that. However, at the class level JavaScript does not provide much safety at all. For classes, you always had to program it yourself, or rely on some additional framework -- and the features you list guarantee that you can. My question essentially is what is inherently different about this particular case of a class abstraction that would change those rules. It's not like this is affecting the core semantics of promises.

But anyway, I don't want to beat a dead horse.

domenic commented 10 years ago

Sounds like we've mostly reached an understanding here. If I may, one parting characterization of the situation that may help @rossberg-chromium: