Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transparency with membranes when using addressable private symbols #7

Open
caridy opened this issue Sep 16, 2018 · 28 comments
Open

Transparency with membranes when using addressable private symbols #7

caridy opened this issue Sep 16, 2018 · 28 comments

Comments

@caridy
Copy link

caridy commented Sep 16, 2018

Membranes are weird, and I think you did a great job explaining how this proposal could operate with membranes. I can see an issue with transparency when leaking private symbols.

The fact that the private symbols in this proposal are addressable by reference, makes it possible to leak them thru the membrane, (e.g.: by to return them when invoking a method via a the membrane):

const data = Symbol.private();

class C {
  constructor() {
    this[data] = 42;
  }
  getData() {
    return this[data];
  }
  getDataSymbol() {
    return data;
  }
}

const o = new C();
const p = new Membrane(o); // assuming Membrane implements the right semantics
p.getData(); // yields `42`
p[p.getDataSymbol()]; // yields `undefined`

To be honest, this is a very edge case, why sharing a private symbol in the first place? But the underlaying issue is concerning considering that if you use a regular symbol, everything works just fine, even if the membrane is masking the symbol with a dry-symbol that maps to a wet-symbol.

The example above cannot be implemented without making the private symbol a regular symbol when it passes thru the membrane, so it can be detected when it comes back as a property accessor, which in principle breaks the transparency invariant because the objects are not similar anymore.

The issue here is not really about the semantics of the private symbol when accessed via a proxy, but IMO it is with the fact that the symbols are addressable. We discussed a lot about this problem when working on the basic principles for the generalization of private fields (which we haven't really get a chance to work on). This is one of the reasons why we have chosen to use syntax for the creation and usage of private fields anyways.

@zenparsing zenparsing changed the title transparency with membranes when using addressable private symbols Transparency with membranes when using addressable private symbols Sep 17, 2018
@zenparsing
Copy link
Owner

Thanks, @caridy. I think this is a crucial point and I'm glad you've taken the time to explain it.

To be honest, this is a very edge case, why sharing a private symbol in the first place?

Indeed. When I look at the code you've provided I'm left wondering why we think it should work. Given the statement that proxies should not intercept private symbols, the membrane's asymmetry in this case seems completely "normal" and expected to me. I wonder if this a case of "it hurts when I do this - well then don't do it"?

The issue here is not really about the semantics of the private symbol when accessed via a proxy, but IMO it is with the fact that the symbols are addressable.

So from your point of view, the problem arises because the private "key" is reified and consequently allowed by the language to pass beyond of the scope in which it was created (and thus allowed to cross membranes).

Membranes are also not transparent with respect to "private field" keys:

class C {
  #x;
  static foo() {
    let m = new Membrane(new this());
    m.#x; // throws - not transparent?
  }
}

But (the argument goes) since the key in this model is syntax-only, there is less of a chance of this assymetry arising in practice.

I'm going to take some time to think this over before I respond. Thanks again!

@jridgewell
Copy link

When I look at the code you've provided I'm left wondering why we think it should work.

Actually, I'm the opposite. I can't understand why it doesn't work without any issues, provided of course that Membrane is just creating a Proxy instance of its parameter with some handler.

Are you wrapping the return value from C.p.getDataSymbol? So it's not the same private symbol anymore? Are you wrapping other symbols, or other primitives?

@zenparsing
Copy link
Owner

@jridgewell

We can assume that new Membrane returns a proxy whose target is some "shadow target" (i.e. an object that is not o). It does this so that:

  1. It can expose a different interface from o (all of its properties are transitively "membraned"), and
  2. It can satisfy all of the proxy invariant checks.

Private-symbol-keyed operations bypass the handler and are applied directly to the proxy target. For a membrane, the proxy target is the "shadow target", and not o.

So when we do p[p.getDataSymbol()], the private symbol lookup is done on the "shadow target", which does not have the requested property.

As @caridy says, "membranes are weird". I should probably not assume any user intuition one way or another with them.

@zenparsing
Copy link
Owner

This has clarified for me what I believe is @erights position. Here is my re-telling:

The capability to access private state, by definition, means that "interlopers" are not allowed to eavesdrop on that access. But in order to be both secure and transparent, a membrane must be allowed to eavesdrop on all access. Therefore, the private state access capability must be restricted to what might be regarded in practical terms as "one side of the membrane".

One way to do this is to bind the private state access capability to a lexical scope in such a way as to prevent passing the capability around as a first class value. If the private access capability is to be reified, then it must be reified as a normal object that an "eavesdropping" membrane can transparently substitute for the actual capability. Hence the "PrivateName" type that appears in the decorators proposal.

I'll add my responses to this point-of-view in a follow-up message.

@jridgewell
Copy link

It can satisfy all of the proxy invariant checks.

What are the invariant checks?

Private-symbol-keyed operations bypass the handler and are applied directly to the proxy target. For a membrane, the proxy target is the "shadow target", and not o.

Can't a get/set handler be installed on this shadow target when a private symbol crosses the membrane?

class C {
  constructor() {
    this[data] = 42;
  }
  getDataSymbol() {
    return data;
  }
}
const o = new C();
const p = new Membrane(o);

// sym has crossed the membrane
const sym = p.getDataSymbol();

// Now membranes need to be aware of the private `sym`
// Internally somewhere, membranes start listening for access on `sym`:
Object.defineProperty(shadowTargetOrPrototypeOfShadowTarget, sym, {
  get() {
    // same as the proxy's get handler
  },
  set(v) {
    // same as the proxy's set handler
  }
});

// Back in user land
p[sym]; // => 42

@bathos
Copy link

bathos commented Sep 17, 2018

What are the invariant checks?

Correct me if I’m wrong @zenparsing, but I think what’s being referred to there is, for example, the need to report the "correct" own property names if the target object is frozen. Proxies are about customizing the behavior of the internal object methods of target, but the object being “proxied” is nonetheless target and must satisfy a number of invariants about behavior that depend on the state of target. With membranes, I think the desire is to entirely decouple the internal object from the external object, so the target of the proxy would not be the same object as the inside-the-membrane object.

...I think.

@zenparsing These summaries have been super instructive. The picture of the full scope of the problems people are concerned with is starting to take shape, though some of it is still pretty fuzzy for me.

@zenparsing
Copy link
Owner

@bathos That's correct. In order to make sure that the proxy respects the "object model invariants" (e.g. that a property cannot go from non-configurable back to configurable), the proxy internally checks any "answer" from a handler method against the proxy target, to make sure that it's being honest. For an example, see step 10 of the proxy version of GetOwnProperty.

@caridy
Copy link
Author

caridy commented Sep 18, 2018

@zenparsing about:

I wonder if this a case of "it hurts when I do this - well then don't do it"

Not really. The problem is that, today, with the current semantics, you can create a transparent membrane that will always work. Asking devs to not do something because their code might not work with membrane is not how it works because precisely, communicating with untrusted code that you don't own or know, is one of the primary use-cases. I think our goal should be to find solutions that preserve certain characteristics of the language, and I hope this is one of those that we can preserve.

@zenparsing
Copy link
Owner

@caridy Can we please put together a list of ways in which membranes are not currently transparent?

@erights
Copy link

erights commented Sep 18, 2018

There are two main non-transparencies that I am aware of:

Built-in methods see internal slots across realms

For all built-in abstractions, like Date, that define instance with internal slots, membranes make these appear to have been defined by classes with private fields, or equivalently, by the WeakMap model of private fields. This is ironic, since private fields were, among other things, designed to resemble the semantics of internal slots. Private fields got this more right than internal slots did.

const nearDate = new Date(...);
const farDate = // ... an instance of Date from another realm
const blueDate = // ... an instance of Date across a membrane boundary
const nearMyDate = new MyDate(...);  // ... like Date but defined by a class
const farMyDate = // ... an instance of the MyDate class from another realm
const blueMyDate = // ... an instance of the MyDate class across a membrane boundary

nearDate.getFullYear();  // works.
farDate.getFullYear();  // works.
blueDate.getFullYear();  // works.
nearMyDate.getFullYear();  // works.
farMyDate.getFullYear();  // works.
blueMyDate.getFullYear();  // works.

Date.prototype.getFullYear.call(nearDate);  // works
FarDate.prototype.getFullYear.call(farDate);  // works.
BlueDate.prototype.getFullYear.call(blueDate);  // works
MyDate.prototype.getFullYear.call(nearMyDate);  // works
FarMyDate.prototype.getFullYear.call(farMyDate);  // works.
BlueMyDate.prototype.getFullYear.call(blueMyDate);  // works

Date.prototype.getFullYear.call(farDate);  // works. THIS IS THE NON-TRANSPARENCY
Date.prototype.getFullYear.call(blueDate);  // does not work
MyDate.prototype.getFullYear.call(farMyDate);  // does not work
MyDate.prototype.getFullYear.call(blueMyDate);  // does not work

Can't prevent behavioral inheritance cycles

When setting an alleged [[Prototype]] that would cause a cyclic inheritance chain, if the chain consists only of non-proxies, then the setting that would cause the cycle is immediately rejected. If there are proxies in the chain, then there is no literal cycle. Rather; the cycle is a matter of dynamic behavior which cannot be prohibited up front.

Under some very limited situations, this difference can be leveraged to detect the existence of a proxy.

@zenparsing
Copy link
Owner

Great, thanks!

@erights
Copy link

erights commented Sep 18, 2018

The private symbol proposal here does not repair the above discrepancy. Attempting to use use private symbols as proposed would not only not repair the discrepancy, it would destroy transparency. I assume the attempt would proceed as follows:

When making a proxy for an instance of Date or MyDate, i.e., a proxy whose real target is that instance, we would also create a similar instance to use as the proxy's shadow.

Note that the proxy's target and shadow cannot be the same Date or MyDate instance, because, for example for a yellow proxy for a blue target date instance, the blue date would inherit from the blue Date.prototype or blue MyDate.prototype, whereas the shadow would inherit from the yellow one.

When another blue object with access to the blue date updates its internal slot (or with access to the blue myDate updates its private field) with setFullYear, the membrane of course cannot notice that at the time.

When another yellow object does a getFullYear, because this proposal does not propose that a handler trap be invoked, the yellow object reads a year that differs from the year on the genuine blue target date.

And vice versa for a setFullYear on the yellow side followed by a getFullYear on the blue side.

These problems are normally avoided by having the handler,

  • during a get* trap, start by updating the shadow state to match the target state.
  • during a set* trap, finish by updating the target state to match the shadow state.

The handler can do this for state that is accessible to anything that has a reference to the objects in question. The handler needs no special privilege. If we tried to repair the above problem by adding traps, then the handler still could not update the state in question because it cannot access the state it needs to synchronize.

@zenparsing
Copy link
Owner

It seems that in the case presented here the membrane is conspiring to subvert itself, as it is using a shadow target with pre-installed private symbol-keyed properties containing meaningful data, and also (effectively) allowing those private symbols through the membrane.

In my analysis so far I have assumed that the shadow target would not have any private symbols pre-installed on it.

@jridgewell
Copy link

When another blue object with access to the blue date updates its internal slot (or with access to the blue myDate updates its private field) with setFullYear, the membrane of course cannot notice that at the time.
When another yellow object does a getFullYear, because this proposal does not propose that a handler trap be invoked, the yellow object reads a year that differs from the year on the genuine blue target date.

Following these two statements, the core issue here is that we cannot keep yellow instances up to date with non-trappable operations on blue instances. So I think your argument in favor of the WeakMap model is that because it because it's not possible to keep the yellow instances up to date, any operation that would do a non-trappable operation should throw. Correct?

Isn't this solvable by exposing a new trap that will be called when a non-trappable operation is about to occur? This shouldn't expose the private symbol itself, just that a private symbol is about to do something on our membrane:

const priv = Symbol.private();
class Blue {
  [priv] = 1;
  
  run() {
    return this[priv];
  }
}

const yellow = new Proxy(new Blue, {
  // "private", or "nontrappable", or insert-name-here
  // Prop is not passed. That would be a leak.
  private(target, receiver) {
    // For example, we can throw here to prevent the non-trappable operation.
    // Throwing here should be the same as WeakMap private semenatics
    throw new Error('attempted to access non-trappable field on a membrane');

    // Or, we could inspect if target and receiver are on the same side of a membrane
    // in which case they should be allowed.
    // This supports virtual private state on the same side of the membrane.
    if (membraneSide(target) === membraneSide(receiver)) {
      // Allow it.
      // Only cross-membrane operations can cause issues.
    } else {
      // Deny it.
      // Cross-membrane operations can cause issues.
      throw new Error('attempted to access non-trappable field on a membrane');
    }
  }
});

// Run will access #x, a non-trappable operation
// It should throw in the private trap.
yellow.run();

@bathos
Copy link

bathos commented Sep 18, 2018

MyDate.prototype.getFullYear.call(farMyDate); // does not work

Should this be read as implying that MyDate.prototype.getFullYear is a custom implementation (that accesses private state), not inherited from Date.prototype?

@zenparsing
Copy link
Owner

@jridgewell

I think you're right. A (very) old version of private symbols had a trap named "unknownPrivateSymbol" for just such a purpose.

Of course, this means that we have to give up the elegant 1:1 symmetry between the trap methods and the Reflect API, but that may be an acceptable trade-off for a simpler object model all-around.

@bakkot
Copy link

bakkot commented Sep 18, 2018

Isn't this solvable by exposing a new trap that will be called when a non-trappable operation is about to occur?

Part of the design of the current proposal is that

no JS code outside of a class can detect or affect the existence, name, or value of any private field of instances of said class without directly inspecting the class's source, unless the class chooses to reveal them

With such a trap, that goal would not be met, right? That is, I could write a proxy with such a trap and pass it to a class method which operates on a private field and thereby detect the existence (but not name etc) of the field, by the triggering of the trap?

That's not terrible, but seems kinda bad.

Of course, this means that we have to give up the elegant 1:1 symmetry between the trap methods and the Reflect API

This seems bad. I have written code which looks like

x = new Proxy(
  target,
  new Proxy(
    {},
    {
      get(target, prop, receiver) {
        return (...args) => {
          console.log(prop, args);
          return Reflect[prop](...args);
        };
      },
    }
  )
);

which relies on the symmetry between Reflect and traps to instrument every proxy trap without observably affecting the behavior of the underlying object. I imagine I'm not the only one.

(More typically I would do Reflect.ownKeys(Reflect).map etc, but sometimes I want the handler to be a proxy anyway for other reasons, in which case this can be nicer.)

@jridgewell
Copy link

That is, I could write a proxy with such a trap and pass it to a class method which operates on a private field and thereby detect the existence (but not name etc) of the field, by the triggering of the trap?

Because the WeakMap model would throw an error, I think we've already made it detectable. Either way, I'm much more concerned with a leak of the private happening than detecting that a private exists.

which relies on the symmetry between Reflect and traps to instrument every proxy trap without observably affecting the behavior of the underlying object.

Can't we introduce a Reflect.unknownPrivateSymbol that noops? Or does whatever default we decide.

@bakkot
Copy link

bakkot commented Sep 18, 2018

Because the WeakMap model would throw an error, I think we've already made it detectable.

The error would be thrown inside of the class, where it's in a position to catch the error. The proxy trap would be triggered in code not controlled by the class.

Can't we introduce a Reflect.unknownPrivateSymbol that noops? Or does whatever default we decide.

Sure, that'd work. Though it's a little awkward.

@jridgewell
Copy link

The proxy trap would be triggered in code not controlled by the class.

You're right, this would open up a new detection. I think I'm still ok with this approach, though, because I can't image how it would be useful to detect that a private field is there. Like, what behavior would you change to somehow extract private info?

Sure, that'd work. Though it's a little awkward.

We could rework the API. Maybe it's "unknown" versions of get/getOwnPropertyDescriptor/has instead of a boolean-returning/throwing unknownPrivateSymbol. I just think these issues are solvable at the Proxy level instead of at the private-field-weakmap-branding level.

@zenparsing
Copy link
Owner

Leaving uknownPrivateSymbol aside for a moment, let's assume that a membrane is defined such that private symbols are simply not allowed to pass through.

Let's say that the membrane just throws when an API attempts to pass a private symbol through. It fulfills the requirement that private access remain isolated to "one side" of the membrane (without invoking the dubious magic of syntax). Why is this not sufficient?

@caridy
Copy link
Author

caridy commented Sep 19, 2018

Let's say that the membrane just throws when an API attempts to pass a private symbol through. It fulfills the requirement that private access remain isolated to "one side" of the membrane (without invoking the dubious magic of syntax). Why is this not sufficient?

This is not sufficient because in most cases, the code that you isolate via a membrane is not code that you own, and therefore it means that not all code can be used via a membrane. If we say that 99.9% of the code can be isolated via a membrane (assuming the example from @erights, which is a very odd and weird code that doesn't work), most other code will work just fine. But not allowing private symbols to pass thru the membrane just means that any implementation of friends where a private symbol is used as the main communication channel, will not work, if the private symbol has to go thru the membrane.

@zenparsing
Copy link
Owner

If we say that 99.9% of the code can be isolated via a membrane

Is that true though? For instance, any code that assumes a mutable global environment won't work with secure membranes.

Also, are we sure that the examples above are truly weird? I have seen a bit of code out there that peels off functions from prototypes (e.g. the WeakMap prototype) and then calls them with various receivers. Usually in such cases both the "peeled-off" functions and the receivers are owned by the same code, and hence would be on the same side of a membrane. I fully expect a similar pattern to hold for private symbols.

I'm wondering what kind of private-symbol-based friendship pattern would have the friends on separate sides of a membrane. If they are on separate sides of the membrane, then you've lost the defining feature of the friend relationship: privacy.

At a higher level, I realize that transparency is a valuable feature for membranes, and I'm not pushing back against it lightly. But transparency is not infinitely valuable, and has to be weighed against the costs of supporting it while also introducing privacy into the object model. In my judgement the cost of the current solution (#things) is enormously high.

The system complexity cost of private symbols, on the other hand, is quite low. We've covered some ways that membranes can deal with private symbols:

  • Just let them pass though (assuming that shadow targets do not store meaningful data in private-symbol named properties)
  • Throw when code attempts to pass a private symbol through the membrane
  • Introduce an "unknownPrivateSymbol"-type trap that would throw an error for membranes

What is the takeaway here? Am I to understand none of these are sufficient, and that syntax-level restriction is a hard requirement for private state "keying"?

@jridgewell
Copy link

To save a discussion @erights and I had during the meeting, I revised my above "pass a reified private symbol through a membrane" example to describe it to him better:

class C {
  constructor() {
    this[data] = 42;
  }
  getDataSymbol() {
    return data;
  }
}
const o = new C();
const p = new Membrane(o);


// Now pretend we're on the yellow side of the membrane
// sym has crossed the membrane
const sym = p.getDataSymbol();
p[sym]; // => 42









function Membrane(object) {

  return new Proxy(object, handler = {
    get(target, propery, receiver) {
      return new Proxy(returnValue, handler);
    },

    apply(target, this, args) {
      const sym = target.apply(this, args);

      if (sym.private) {
        // Now membranes need to be aware of the private `sym`
        // Internally somewhere, membranes start listening for access on `sym`:
        Object.defineProperty(shadowTargetOrPrototypeOfShadowTarget, sym, {
          get() {
            // same as the proxy's get handler
          },
          set(v) {
            // same as the proxy's set handler
          }
        });
      }

      return sym;
    }
  });
}

We have two options when a reified private symbol passes through a membrane as a return value (value at a property access, ect).

We can just throw when a private symbol is passed through, and pretend that pretend that they're only lexical to the original side of the membrane. This behavior makes it similar to how non-reified PrivateNames in the current proposal work (since if they're not reified, you can't pass it through the membrane).

Or, we can install the private symbol onto the shadowTarget's prototype (actually, all of the prototypes of the shadowTarget) so that we may trap them on the shadowTarget. This is probably expensive, so I can understand why he doesn't like this choice.

But I'm also curious what @erights plans to do with a reified PrivateName passing through an membrane?

@Igmat
Copy link

Igmat commented Oct 20, 2018

@caridy, @jridgewell all this thread is built on wrong assumption that Symbols should be treated as objects that has to be packed after crossing boundary. But they don't have to. Instead they should be treated as primitives (even though technically they aren't) - in this case sample from first post will just work and return 42.

@jridgewell
Copy link

all this thread is built on wrong assumption that Symbols should be treated as objects that has to be packed after crossing boundary. But they don't have to.

This is incorrect. The thread is discussing how we can maintain a membrane's blue-yellow semantics with proxies that are transparent.

If you take {} (an object) from one side of a membrane, it cannot be strict equal to the object that is returned by accessing an the object through membrane:

const obj = {
  prop: {}
};

const membrane = new Membrane(obj);
assert.notEqual(obj.prop, membrane.prop);

So it's not as simple as just letting the private symbol pass through and grab the property off the target. There must be some way for the proxy to either trap the property and wrap the value, or the proxy must throw when it detects. But if the proxy could trap, we break encapsulation.

Note too, that the target of the proxy is not obj.prop, but a clone of it on the other side of the membrane. This clone necessarily doesn't have the private symbol (how could it know about the private symbol?). So doing membrane[priv] would access the clone[priv], which is undefined. My suggestion above makes use of this fact.

The other option is to just throw when a proxy handler is about to be invoked with a private symbol, aka non-transparency. I'm good with that approach, too.

@Igmat
Copy link

Igmat commented Oct 22, 2018

@caridy and @jridgewell sorry, I didn't spent enough time to understand the problem with membrane, and my initial comment is indeed wrong, thanks for more detailed explanation.
But after some investigation I realized, that it's still not an issue, since it could be solved in Membrane implementation (e.g. as you shown in #7 (comment) even though there are few additional steps that should be taken to trap exposed private symbol).

And even more, it doesn't break encapsulation, because if private symbol is returned from public property or method, then it's part of public API.

@jridgewell
Copy link

And even more, it doesn't break encapsulation, because if private symbol is returned from public property or method, then it's part of public API.

Agreed, that's how I solved the issue in my comment above.

Note, though, that while transparency is nice feature for library authors, it is not a requirement for private symbols. We can define transparency with another proxy class (or with some flag/handler, etc) at a later time and ship private symbols with non-transparency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants