Allow child to get the parent origin

[radotzki]

Hi 👋 This is the first time I contribute to this project, I hope you will find it useful 😊

In some cases, where the parent domain is unknown (e.g penpal is used inside an SDK), it can be useful for the child to know the parent origin.

This pull request implements this feature by adding a new promise (parentOriginPromise) to the response of the connectToParent method.

Code example:

const penpal = Penpal.connectToParent({methods: {...}})
penpal.parentOriginPromise.then(parentOrigin => console.log(parentOrigin))

[Aaronius]

Hey! Thanks for opening a pull request. This is an interesting idea. Do you mind explaining what you would do with the parent origin? I understand it could be many things, but I'm interested in hearing about a concrete example.

[radotzki]

I work at Portis and we develop an embedded Ethereum wallet. We have an SDK that apps can use in order to integrate with our platform.

The SDK is basically a proxy for our wallet: when a user starts an action on the app, the SDK sends a postMessage to our wallet, where the private key is saved, the wallet signs the action and response with the signed action.

For security reasons, we want to validate the app's domain (where the SDK is embedded) and check if it's included in the pre-configured allow list. The best way to get the app's domain is to use the event.origin of the postMessage.

We can send a regular postMessage just for this specific flow, however, I think it would be much more elegant to use Penpal for that :)

[Aaronius]

Thanks for explaining @radotzki. It makes sense. It kind of sucks that we have to add a separate promise for this, but I seem to agree that it's probably the best option. Overloading the resolution value for connection.promise makes it a little messy/inconsistent. It would probably need to be resolved with an object containing a parent property and a parentOrigin property, which would be inconsistent with the resolution value of connection.promise in the parent (which would still just be the child object). I'm curious to know how you feel about that.

Anyway, overloading the resolution value of connection.promise would be a breaking change anyway and would need to be done in a major version, so I think we should proceed with what you've done here.

We need tests for the added behavior and an updated readme. If you'd like to do that, please do. Otherwise, I'll try to tackle it sometime soon. Thanks for the PR and thanks for improving the Ethereum community!

[Aaronius]

@radotzki, would you rather be able to specify an array of acceptable origins and have Penpal handle the security restrictions?

[radotzki]

Thanks @Aaronius 🙏

Your are completely right, I wanted to avoid breaking changes, and this is the reason I chose to add a new separate promise.

I’ll add tests and will update the readme :)

We can’t specify an array of acceptable origins, because anyone can create a decentralized application on unique domains and use Portis. We can’t know the origins in advance.

[Aaronius]

We can’t specify an array of acceptable origins, because anyone can create a decentralized application on unique domains and use Portis. We can’t know the origins in advance.

I don't quite understand this. I know it's a pain, but do you mind explaining this in more detail? How and when do you determine what's an acceptable origin?

[radotzki]

When a developer create an application and want to use Portis, he can set an allow list of domains for his application. Then, his application id can be used only on those domains.

You can see this video that demonstrates how it works: https://www.loom.com/share/90217c82e9c84f51aa97665f14b0451f

[stewartschillingsdi]

Just adding my experience using PenPal for the same basic use case (Domain whitelisting for the Storage Spanner extension for Adobe Launch). We include a whitelist as an array of domain end-match patterns as an inline script that is hardcoded into the iFrame's source. We then check the domain of the iframe's referrer against the whitelist and refuse the connection if there is no match.

To manage the whitelists we have an application which, when a new whitelist needs to be published calls an AWS lambda function that creates the new iFrame HTML (with updated whitelist), stores it on S3, and makes it available via Cloudfront on a client-specific domain.

On Fri, Sep 6, 2019 at 9:40 AM Itay Radotzki notifications@github.com wrote:

When a developer create an application and want to use Portis, he can set an allow list of domains for his application. Then, his application id can be used only on those domains.

You can see this video that demonstrates how it works: https://www.loom.com/share/90217c82e9c84f51aa97665f14b0451f

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Aaronius/penpal/pull/32?email_source=notifications&email_token=AFPAYSU56744TG6GDOGB3WTQIJTXLA5CNFSM4IPPAS52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6DBREQ#issuecomment-528881810, or mute the thread https://github.com/notifications/unsubscribe-auth/AFPAYSR4QL6Y4WPHOS2AAG3QIJTXLANCNFSM4IPPAS5Q .

--

Stewart Schilling <stewart.schilling@searchdiscovery.com>
Analytics Architect
271 17th Street NW, Atlanta, GA 303 63  |  Suite #1700
searchdiscovery.com <http://www.searchdiscovery.com/>  |  mobile

608-553-2630

[Aaronius]

@rodotzki, if I understand right, this Portis view is inside an iframe:

And that iframe has access to the domains configured from within the Portis interface:

If that's the case, why couldn't the Portis iframe view feed the configured domains into Penpal when it's connecting to the parent?

@stewartschillingsdi Would your use case be solved by allowing you to pass an array of origins into connectToParent and having Penpal prevent the connection if the parent origin doesn't match?

[stewartschillingsdi]

@Aaronius/penpal penpal@noreply.github.com - We've already solved the situation as described. We would not have a list of domains to send since we're using domain end-match patterns. Thanks though.

On Sat, Sep 7, 2019 at 12:37 PM Aaron Hardy notifications@github.com wrote:

@rodotzki, if I understand right, this Portis view is inside an iframe:

[image: image] https://user-images.githubusercontent.com/210820/64478254-1b35f800-d163-11e9-8f84-261ad93cb474.png

And that iframe has access to the domains configured from within the Portis interface:

[image: image] https://user-images.githubusercontent.com/210820/64478269-3acd2080-d163-11e9-8e92-7a39797b88bf.png

If that's the case, why couldn't the Portis iframe view feed the configured domains into Penpal when it's connecting to the parent?

@stewartschillingsdi https://github.com/stewartschillingsdi Would your use case be solved by allowing you to pass an array of origins into connectToParent and having Penpal prevent the connection if the parent origin doesn't match?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Aaronius/penpal/pull/32?email_source=notifications&email_token=AFPAYSXXNLIVBJHQBMQ3Z5LQIPRFZA5CNFSM4IPPAS52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6E53PQ#issuecomment-529128894, or mute the thread https://github.com/notifications/unsubscribe-auth/AFPAYSVGUA3CJY5LV44RCK3QIPRFZANCNFSM4IPPAS5Q .

--

Stewart Schilling <stewart.schilling@searchdiscovery.com>
Analytics Architect
271 17th Street NW, Atlanta, GA 303 63  |  Suite #1700
searchdiscovery.com <http://www.searchdiscovery.com/>  |  mobile

608-553-2630

[Aaronius]

Sorry @stewartschillingsdi, I should have said, "Would your use case have been solved by allowing you to pass an array of origins into connectToParent and having Penpal prevent the connection if the parent origin doesn't match?" I understand you have end-match patterns, so if Penpal allowed you to pass a regular expression to determine if the parent origin is allowed, would that have worked?

I'm trying to figure out if we're able and if it's preferable to solve the use cases that both you and @roditzki are presenting by handling the origin restriction logic inside Penpal instead of outside Penpal.

[stewartschillingsdi]

Hi Aaronius/penpal. Thanks for the explanation. As long as the array of domains or domain regex patterns is fully contained in the iframe source, then yes, it would have been somewhat helpful at that point in time to be able to pass it into the connectToParent method. I'll DM you some code that will explain who we did it.

On Mon, Sep 9, 2019 at 11:34 AM Aaron Hardy notifications@github.com wrote:

Sorry @stewartschillingsdi https://github.com/stewartschillingsdi, I should have said, "Would your use case have been solved by allowing you to pass an array of origins into connectToParent and having Penpal prevent the connection if the parent origin doesn't match?" I understand you have end-match patterns, so if Penpal allowed you to pass a regular expression to determine if the parent origin is allowed, would that have worked?

I'm trying to figure out if we're able and if it's preferable to solve the use cases that both you and @roditzki are presenting by handling the origin restriction logic inside Penpal instead of outside Penpal.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Aaronius/penpal/pull/32?email_source=notifications&email_token=AFPAYSRASXEOWNUZ7I3BWOTQIZ3JJA5CNFSM4IPPAS52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6IHV3Y#issuecomment-529562351, or mute the thread https://github.com/notifications/unsubscribe-auth/AFPAYSUZDV4PGQSYMCBRK7TQIZ3JJANCNFSM4IPPAS5Q .

--

Stewart Schilling <stewart.schilling@searchdiscovery.com>
Analytics Architect
271 17th Street NW, Atlanta, GA 303 63  |  Suite #1700
searchdiscovery.com <http://www.searchdiscovery.com/>  |  mobile

608-553-2630

[radotzki]

@Aaronius thanks for the suggestion - much appreciated!

[Aaronius]

I'm going to close this out. It seems like you found a reasonable alternative and I haven't heard any requests for this from anyone else.

[AlexandreBonaventure]

@Aaronius actually I've started using your library for setting up a SSO auth system, and I wish I would be able to pass a regexp in parentOrigin parameters for additional security, in order to match all subdomains. I can submit a PR if that makes sense for you. Thanks for letting me know

[Aaronius]

@AlexandreBonaventure I've started down this road before and there's more to it than what you might think initially. I'll try to explain. The child (the iframe) initiates the handshake with the parent. In other words, the child is the first one to send a message. This is intentional and preferred for reasons I won't get into at this point. If you were to pass a regular expression for the parentOrigin inside the child, the child would need to use * for the target origin on that initial handshake message, because parentOrigin is a regular expression rather than a valid origin string.

The only significant content on that first handshake message is an array of method names the child is exposing to the parent, so it's probably not much of a security issue to use * for the target origin on that message, but it's worth mentioning that if a nefarious website were loading your iframe, the website would know the names of the methods the child is attempting to expose to the parent.

Once the parent receives the handshake message, the parent sends a handshake-reply message to the child. It's important to understand though that at this point the parent resolves the connection.promise that was returned to the consumer from connectToChild. In other words, the parent doesn't wait for the child to acknowledge that it received the handshake reply before calling the connection a success. If the handshake-reply gets sent back to the child and that child sees that the parent's origin does not match the provided parentOrigin regex, then the child would ignore the handshake reply (specifically, the child would log a message saying that the parent's origin doesn't match parentOrigin). Meanwhile, the parent's connection.promise was resolved, when it really should have been rejected because the child didn't actually end up accepting the handshake reply. So you're stuck in this weird limbo where the parent thinks the connection was successfully made while the child does not.

I think the long-term solution to this is to change the 2-way handshake process (handshake + handshake-reply) into a 3-way handshake process, kind of like a TCP handshake. In other words, the child sends a SYN message, the parent sends back a SYN/ACK message, then the child sends back an ACK message. The child only considers the connection successful when it receives the SYN/ACK from the parent and the parent only considers the connection successful when it receives the ACK from the child. Regarding the potential security issue I mentioned earlier, the child would only send its method names to the parent on the ACK message it sends to the parent, which is better security because at that point the child would be sending the ACK message to a specific, qualifying origin, rather than * (only the SYN message from the child would need to use * as the target origin).

I say this knowing that your use case is a valid, real one and one I'd like to address. It'll just take some pretty heavy restructuring of the handshake process. It's on my TODO list, but alongside everything else in life. Thanks for bringing it up.

[Aaronius]

@AlexandreBonaventure I spent some time working on a 3-way handshake tonight. I think I got it about nailed down, but tests are giving me a hard time. Oddly, when I run the tests, some fail, but when I run Karma in debug mode, they all pass. If you want to give a shot at figuring out what's going on, that would be a good step in the direction of providing regex support for parentOrigin. To see what I'm talking about, check out the three-way-handshake branch. Run npm ci, then npm run test:watch. You'll probably see tests fail. But then click the Debug button in the browser window:

Then open up the developer console and notice the console messages show all the tests passing.

Anyway, I'll spend some more time digging into it when I get a chance.

[AlexandreBonaventure]

wow very comprehensive answer! I did not expect that , thanks a lot @Aaronius for taking the time to explain the issue. It all makes sense. This is not the highest priority for me since I still have a security with the https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/frame-ancestors headers. But I'll definetly try to take a look whenever I have some free time.

[Aaronius]

I figured it out. Getting closer!

[loganvolkers]

@Aaronius for documentation purposes this may help

Source via mermaid: https://mermaid-js.github.io/mermaid-live-editor/#/edit/eyJjb2RlIjoic2VxdWVuY2VEaWFncmFtXG4gIENoaWxkIC0-PiBQYXJlbnQ6IHN5blxuXHRQYXJlbnQgLT4-IENoaWxkOiBzeW5BY2tcbiAgQ2hpbGQgLT4-IFBhcmVudDogYWNrXG4gIENoaWxkIC0-PiBQYXJlbnQ6IGNhbGxcbiAgUGFyZW50IC0-PiBDaGlsZDogcmVwbHlcbiIsIm1lcm1haWQiOnsidGhlbWUiOiJkZWZhdWx0In0sInVwZGF0ZUVkaXRvciI6ZmFsc2V9

[Aaronius]

Thanks @loganvolkers!

[Aaronius]

Hey folks, thanks for the discussion. I'm preparing v5 for release and could use your help testing it out. It includes regex support for parentOrigin. See https://github.com/Aaronius/penpal/issues/43