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

Chat api #10910

Closed
wants to merge 6 commits into from
Closed

Chat api #10910

wants to merge 6 commits into from

Conversation

bgits
Copy link

@bgits bgits commented Jul 3, 2020

Summary

Implements DApp Chat api as per: status-im/specs#117

Review notes

This is my first PR on this code base, wondering how these changes should be organized and if these state mutation are idiomatic?

There are 3 functions in permissions.cljs that dispatch re-frame events, I'm wondering if they should live elsewhere? Are they inline with convention to dispatch re-frame events if the action is coming from the browser API?

Outstanding bugs

Looking for guidance on how to resolve these issues:

  • The first time a permission is invoked to send a message or join a room it causes a crash upon approval.
  • When grabbing messages and sending over the bridge on iOS the json payload can not be parsed if the message contains the character '. I have not found a way to escape it so for now it is removed.

Testing notes

There is a test dapp which makes calls to the api, it can be run locally and use in the webview. It can be found here: https://github.com/status-im/status-api-tester.git

Platforms

  • Android
  • iOS

Areas that maybe impacted

Security? Two of the calls have the permission :yield-control? true I don't know the full impact of this.

Functional
  • public chats
  • dapps / app browsing

Steps to test

After running npm install in the status-api-test directory one can start the dapp using embark run. It should be accessible from http://localhost:8000/. Open it in the dapp browser and use the Status api tab to test calls. Keep the localhost console open as some function currently just log to console.

status: wip

@bgits bgits requested review from jakubgs and a team as code owners July 3, 2020 17:36
@ghost
Copy link

ghost commented Jul 3, 2020

Hey @bgits, and thank you so much for making your first pull request in status-react! ❤️ Please help us make your experience better by filling out this brief questionnaire https://goo.gl/forms/uWqNcVpVz7OIopXg2

@status-im-auto
Copy link
Member

status-im-auto commented Jul 3, 2020

Jenkins Builds

Click to see older builds (2)
Commit #️⃣ Finished (UTC) Duration Platform Result
✖️ ff8da35 #3 2020-07-03 17:55:34 ~10 min android 📦apk 📲
✖️ ff8da35 #3 2020-07-03 17:57:02 ~11 min android-e2e 📦apk 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✖️ 1e89d0f #4 2020-07-21 01:18:45 ~9 min android 📦apk 📲
1e89d0f #4 2020-07-21 01:20:34 ~11 min android-e2e 📄log
30f574e #5 2020-07-21 11:26:10 ~10 min android-e2e 📄log
✖️ 30f574e #5 2020-07-21 11:33:12 ~17 min android 📦apk 📲

@hesterbruikman
Copy link
Contributor

hesterbruikman commented Jul 6, 2020

@bgits Thanks for kicking this off. I understand from Marketing that a Chat API, in the wider realm of extensions, has been requested by dapps. If the functionality can drive existing dapp audiences to use Status to benefit from messaging features, it makes a strong case to work on this now.

Suggestion/questions

  • Call with Marketing (cc @j-zerah ) to get requirements for a concrete use case of a dapp using the chat API
  • Can you think of any cases that do not require the user to share their public chat key?
  • How would notifications fit in?

cc @0kok0 for security eyes

@flexsurfer
Copy link
Member

thanks @bgits , permissions will be asked only ones, so if user gives permission to dapp, then dapp will be doing next calls silently, sending message on behalf of user is really dangerous, also current UX for permissions isn't great, if we'll have more permissions and dapp will ask all permisions on the first run, they all will be asked one after another
current chat means dapp's public chat ?

@bgits
Copy link
Author

bgits commented Jul 6, 2020

  • Can you think of any cases that do not require the user to share their public chat key?

There are some as such gas abstraction or gasless voting where the dapp just needs to submit a signed ethereum transaction to a channel.

@bgits
Copy link
Author

bgits commented Jul 6, 2020

thanks @bgits , permissions will be asked only ones, so if user gives permission to dapp, then dapp will be doing next calls silently, sending message on behalf of user is really dangerous, also current UX for permissions isn't great, if we'll have more permissions and dapp will ask all permisions on the first run, they all will be asked one after another
current chat means dapp's public chat ?

I understand the security concerns. Perhaps for now it makes sense to put this behind either a "Security" or "Advanced" flag that is by default turned off until we think of a better approach?

Another approach that I think requires less security concern is allowing dapps to send messages from a random key. This is still useful for certain applications such a gasless voting & gas abstraction but removes the concern of dapps impersonating a users to do malicious things.

@j-zerah
Copy link

j-zerah commented Jul 6, 2020

@bgits @hesterbruikman - Some very interesting ideas have come up in partner outreach. Id love to discuss further and perhaps even loop those teams in to collaborate on these.

Two high level ideas from partners:
1.) Notifications from a dapp indicating someone that their position in a defi protocol has changed/is at risk
2.) Sending messages to friends inviting them to partake in a prediction market via the chat interface

@bgits
Copy link
Author

bgits commented Jul 6, 2020

1.) Notifications from a dapp indicating someone that their position in a defi protocol has changed/is at risk
For this I think we can use the notification service @richard-ramos created and extend it to use waku.

Sending messages to friends assumes they are already on Status?

@j-zerah
Copy link

j-zerah commented Jul 6, 2020

Sending messages to friends assumes they are already on Status?

Correct. Imagine some friends have a private group chat and one of the members shares a prediction market. The other friends can take a position in the market directly from the chat interface with a simple tap

@hesterbruikman
Copy link
Contributor

hesterbruikman commented Jul 9, 2020

cc @corpetty for planning purposes. We'll need security requirements for a chat API to be used by dapps. I suggest limiting to use case 1 described by @j-zerah described:

1.) Notifications from a dapp indicating someone that their position in a defi protocol has changed/is at risk

(most generalizable, least privacy sensitive)

I'm thinking of a timeframe of around 2 months, so we don't postpone this indefinitely. I understand it could draw dapps and bring utility, and more functioning dapps, to Status. We'll likely need UI work on permissions etc after we have a security model as a reference point. Does that sound feasible @corpetty?

cc @iurimatias @andremedeiros

It seems using onMessage to respond over the bridge is not ideal
for send larger payload and the preferred method is to use postMessage
to communicate with webview
@0kok0
Copy link

0kok0 commented Jul 22, 2020

I assume this will start with messages being sent on the users behalf, later exposing wallet functionality, accessing images etc.

Iteration 0:

Dapp requirements

Example threat: Poorly implemented collectible dapps lure users into compromising their and contact's privacy by pushing/pulling messages in the clear.

  • Clear data usage policy (GDPR?)
  • https, potentially self-signed included in the permission
  • Can we require/enforce waku usage for the dapp to facilitate e2e encryption?
  • ethereum inherited goodies: let's require every API call to be signed by a dapp pubkey

API gateway:

Example threat: Malicious Dapp successfully circumvents API endpoint safety measures and can execute code on the user's device.

API gateway process for all API calls that can be strictly isolated. Implements least authority. Performs the following tasks for all API calls:

  • logging and throttling: User can see what Dapps did on its behalf, can revoke access. Malicious dapps/Endpoints will try to authenticate using a variety of credential combinations/tokens. Must fail after too many authentication attempts in a certain amount of time.
  • Dapp input sensitization
  • Prevent excessive usage and data exposure due to the central point for data and object-specific policies, considering their individual sensitivity, relying on clients to perform the data filtering before displaying it to the user.
  • limits the damage of e.g., injections
  • enforces https (minimum)

Permission/Token:

  • use OS provided permissions: https://developer.android.com/guide/topics/permissions/overview
  • State Dapp Data usage policy and granted permissions
  • API tokens exchanged after handshake (minimum https/https), dapps get assigned a token, that:
    • is revocable
    • TTL, expiration, must not be longer than one year, so we implement dynamic least authority without requiring user reviewing permissions manually (like browser, FB,..)

@bgits
Copy link
Author

bgits commented Jul 22, 2020

I assume this will start with messages being sent on the users behalf, later exposing wallet functionality, accessing images etc.

Iteration 0:

Dapp requirements

Example threat: Poorly implemented collectible dapps lure users into compromising their and contact's privacy by pushing/pulling messages in the clear.

  • Clear data usage policy (GDPR?)
  • https, potentially self-signed included in the permission
  • Can we require/enforce waku usage for the dapp to facilitate e2e encryption?
  • ethereum inherited goodies: let's require every API call to be signed by a dapp pubkey

API gateway:

Example threat: Malicious Dapp successfully circumvents API endpoint safety measures and can execute code on the user's device.

API gateway process for all API calls that can be strictly isolated. Implements least authority. Performs the following tasks for all API calls:

  • logging and throttling: User can see what Dapps did on its behalf, can revoke access. Malicious dapps/Endpoints will try to authenticate using a variety of credential combinations/tokens. Must fail after too many authentication attempts in a certain amount of time.
  • Dapp input sensitization
  • Prevent excessive usage and data exposure due to the central point for data and object-specific policies, considering their individual sensitivity, relying on clients to perform the data filtering before displaying it to the user.
  • limits the damage of e.g., injections
  • enforces https (minimum)

Permission/Token:

  • use OS provided permissions: https://developer.android.com/guide/topics/permissions/overview

  • State Dapp Data usage policy and granted permissions

  • API tokens exchanged after handshake (minimum https/https), dapps get assigned a token, that:

    • is revocable
    • TTL, expiration, must not be longer than one year, so we implement dynamic least authority without requiring user reviewing permissions manually (like browser, FB,..)

The existing API already exposes wallet functionality using the EIP-1102 standard. It also exposes some chat functionality: https://status.im/developer_tools/status_extras/status_dapp_api.html

How to make these API enhancements secure is something we want to do and think specific exploit cases will crystalize during testing. It would be helpful to have specific exploits that use API so they can be tested against and designed to mitigate.

In the case of: "Example threat: Malicious Dapp successfully circumvents API endpoint safety measures and can execute code on the user's device.", How can we create this scenario using the chat-api?

@0kok0
Copy link

0kok0 commented Jul 22, 2020

In the case of: "Example threat: Malicious Dapp successfully circumvents API endpoint safety measures and can execute code on the user's device.", How can we create this scenario using the chat-api?

Please, if you ever possess this type of information, never disclose it publicly to prove a point. Send me a DM if you require a more concrete/testable threat model. I agree it's very generic.

However, I am afraid I have to disagree that we should do exploit driven security development; that's the definition of being reactive.
Also, I think having an API Gateway will payoff due to enhanced maintainability in the future and not only improve security. Building an API will allow an attacker to scale any potential exploit quickly, and in the current maturity and decentralized nature of the ecosystem, we should assume Dapps are coming and going, with little to no reasonable security assumptions.

Let me ask a question: would you still like to use Linux if there was no kernel isolation, no package manager, or an hour to create a package?

@bgits
Copy link
Author

bgits commented Jul 22, 2020

In the case of: "Example threat: Malicious Dapp successfully circumvents API endpoint safety measures and can execute code on the user's device.", How can we create this scenario using the chat-api?

Please, if you ever possess this type of information, never disclose it publicly to prove a point. Send me a DM if you require a more concrete/testable threat model. I agree it's very generic.

However, I am afraid I have to disagree that we should do exploit driven security development; that's the definition of being reactive.
Also, I think having an API Gateway will payoff due to enhanced maintainability in the future and not only improve security. Building an API will allow an attacker to scale any potential exploit quickly, and in the current maturity and decentralized nature of the ecosystem, we should assume Dapps are coming and going, with little to no reasonable security assumptions.

Let me ask a question: would you still like to use Linux if there was no kernel isolation, no package manager, or an hour to create a package?

Not disagreeing with you. I think having an actual example makes the threat more tangible and less prone to repeating.

@0kok0
Copy link

0kok0 commented Jul 22, 2020

You're right, I'll make sure to provide detailed examples during the next cycle

@flexsurfer
Copy link
Member

feel free to reopen

@flexsurfer flexsurfer closed this Dec 4, 2020
@jakubgs jakubgs deleted the chat-api branch September 21, 2022 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants