dfinity / wg-identity-authentication

Repository of the Identity and Wallet Standards Working Group
https://wiki.internetcomputer.org/wiki/Identity_%26_Authentication
Apache License 2.0
31 stars 9 forks source link

ICRC-28: Proposal to add security for IC wallet users #33

Open dostro opened 1 year ago

dostro commented 1 year ago

Problem Statement

There is a chance that some IC wallets send 3rd parties a delegation identity with unscoped permissions. This opens a trust assumption that the 3rd party application will not use this delegation identity to drain the user of all their assets. Although the feature to allow users to sign in with the same principal across applications is required for many web3 use cases, we don't believe it should come with such an enormous risk.

Note: We have verified this is not a problem for II or NFID users, but NFID needs a solution given its intention to support global principals.

Proposed Solution

The NFID team proposes a standard mechanism for 3rd parties to request a delegation identity from IC wallets with the same principal across applications that guarantees user safety against drain attacks:

  1. 3rd party application MUST set an array of target canisters to which this delegation identity will make authenticated calls without user approvals. These canisters SHOULD be under the 3rd party application's control, otherwise the developer opens a trust assumption that other canister controllers won't carry out drain attacks on their shared pool of users.
  2. Wallet provider MUST verify the frontend origin making the request and MUST verify each canister has whitelisted that frontend origin to request it as a target.

Target canister implementation

We assume each wallet provider will have its own mechanism for verifying the frontend origin making the call. We propose a standard method on each target canister that wallet providers will query (as an update call for secure consensus):

// Rust implementation

#[update]
async fn get_trusted_origins() -> Vec<String> {
    vec![
        String::from("dscvr.one") // to be replaced with application's frontend origin(s)
    ]
}

NFID implementation

Developers using NFID will be supplied with an SDK for setting targets. If targets are not set, users will not be able to select their main NFID profile with which to continue to the 3rd party application. image

Future improvements

More granular permissions - on a method-by-method basis - is a more preferable route but given the immediate user risk and benefit of a global principal, we would move to prioritize the immediate-term fix.

frederikrothenberger commented 1 year ago

Hi @dostro

Thanks for the contribution! I think it would be best if you created a PR for a draft spec and give it an ICRC number. Then we could also provide feedback to specific lines of the draft.

Just one comment so far: I would stick to the origin as it is understood by browsers, in particular include the scheme and port.

Also, since the list is (mostly) static, it could easily be certified and requested via query which would be a lot more efficient, especially if the list of targets is very long.

dostro commented 1 year ago

@frederikrothenberger is this the official repo? https://github.com/dfinity/ICRC/tree/main

frederikrothenberger commented 1 year ago

@dostro: Yes. However, you don't need to create the PR there. Rather, create an issue with the lowest, still unclaimed number and link to the PR in this repo.

dostro commented 1 year ago

FYI I submitted ICRC-28 as a proposal to standardize the security of delegation requests. Would be great to discuss at tomorrow's WG.