fix!: only string named methods

[erights]

Staged on #2797

Closes: #XXXX Refs: #2793 #1809 #2797

Description

This PR prohibits remotables from having symbol named methods, or methods named 'then' or 'constructor'. IOW, it only allows string-named methods other than 'then' or 'constructor'. If a 'constructor' is provided on the input object to making a remotable, it is ignored, so that class prototypes can be used as such input.

Security Considerations

Narrower attack surface. Fewer cases.

Scaling Considerations

none

Documentation Considerations

Need to document what we allow as method names of remotables.

Testing Considerations

Fixed the tests, so that they pass. Having read them while fixing them, I think these fixed original tests are adequate.

Compatibility Considerations

This is a compat break. That why fix! rather than fix:. A client could not adopt this until after they had purged all prohibited remotable method names from their code.

On the flip side, this is a huge step towards compat with ocapn, which by convention uses only one namespace of string method names. Ironically, in ocapn the convention is that the method names on the wire are only symbols. An endo object will always convert the method name to a string.

Our prohibition of 'then' and 'constructor' are currently remaining incompats with ocapn. For use to meet ocapn here, we'll have to do some kind of hilbert hotel, lifting, for example, a protocol 'then' into a JS '__then__' or something.

Upgrade Considerations

It is not clear how to stage this so that code running from before this change and code running after this change can talk to each other.

• [ ] Update NEWS.md for user-facing changes.

[erights]

Putting this back into Draft until I make some changes:

• Split the PR into two staged PRs.
  1. change all remotable creation to avoid symbol-named methods, with the possible exception of Symbol.asyncIterator.
  2. enforce that remotables cannot have symbol-named methods
  • With a possible special carve-out to grandfather in Symbol.asyncIterator with its current wire representation, while enforcing that a remotable cannot have both a method named with the string @@asyncIterator.

The reason for the split is that the first still does not create any compat problems, assuming that none of the prohibited symbols were actually sent in production. This helps phase the coordinated changes to agoric-sdk.

We would only do the special case for Symbol.asyncIterator if we're actually sending it between vats in production in either endo or agoric-sdk.

[kriskowal]

Where I would like to land with this change is a bijection where all valid method names have one and only one corresponding JavaScript reification.

I think that means:

wire	javascript
@@asyncIterator	Symbol.asyncIterator
constructor	Symbol.for('constructor')
then	Symbol.for('then')
other string	other string

My preference is to avoid Symbol.iterator entirely since the signature over the wire differs from the local signature, and assuming it’s not actually present anywhere. I am pretty sure we have no risk of breaking for other symbols, but defer to @mhofman about the course for maintaining the chain.

[erights]

since the method names constructor, prototype, and then create attack surface.

Yes for constructor and then. What's the problem with a remotable method named prototype? (I admit it is weird, but I don't see a problem.)

[erights]

Re constructor, then, and @@asyncIterator, very clever mapping these 1-1 to JS-only symbols. I had thought any solution to these would need a Hilbert hotel. But since there's no other way to mention these symbols from OCapN, I think we can avoid finding more rooms in the hotel. Does this make sense, or does this solution still need a Hilbert hotel somewhere?

Double checking:

We would then prohibit remotables from having methods that, as the JS level, are named with the JS strings @@asyncIterator or then, and we would ignore what seems to be a JS method named with the JS string constructor.

Yes?