dfinity / motoko

Simple high-level language for writing Internet Computer canisters
Apache License 2.0
507 stars 98 forks source link

Refined Principals #3176

Open crusso opened 2 years ago

crusso commented 2 years ago

@JensGroth and @robin-kunzler have noticed that users often fail to consider the different sorts of principals when doing authentication, e.g. unintentionally creating accounts for anonymous principals.

Perhaps we should consider projecting the IC principals into a more refined Motoko (variant?) type, forcing users to consider all the possible cases when handling principals, much as option types force users to explicitly deal with nulls.

One backwards compatible way to do this might be to extend the context type (with the current caller : Principal field) to have a second field of the more refined caller type and deprecate the old caller field, or just refine Principal into a more concrete type and perhaps warn when testing equality at type Principal.

ggreif commented 2 years ago

Well, Principal is already a tagged type, right? So what we need is something like

type PrincipalRole = Anonymous | User | ...
type ICPrincipal (role : PrincipalRole) = <magic>
type UserPrincipal = ICPrincipal User
type Principal = ∃role. ICPrincipal role

and convince developers to only open accounts for UserPrincipals :-)

Then we would also need some switch caller { case (user : UserPrincipal) ...; _ } GADT-like pattern matching to make this work.

nomeata commented 2 years ago

It goes against the (well, my) philosophy of a “mostly opaque” principal (and what's so wrong if a canister, or “the general public” a.k.a. the anonymous user can open an account). So I'll state my opposition here for the record, but otherwise will shut up :-)

ggreif commented 2 years ago

The problem is that some person A, accidentally logged in as anonymous and creates an account with sensitive information (e.g. telephone number or birthday). Then another user B makes the same mistake and suddenly sees the profile of A. Leaking out information is bad.

nomeata commented 2 years ago

But isn't that a frontend problem? Why can you even create an account when you aren’t logged in?

Ah, maybe a login request should have caused a unique random and non-clashing principal to be used :-D

robin-kunzler commented 2 years ago

Thanks for creating the issue and for the discussion!

The problem is that some person A, accidentally logged in as anonymous and creates an account with sensitive information (e.g. telephone number or birthday). Then another user B makes the same mistake and suddenly sees the profile of A. Leaking out information is bad.

I agree with @ggreif.

But isn't that a frontend problem? Why can you even create an account when you aren’t logged in?

@nomeata : I agree that one would expect a frontend not to create an account unless the user has logged in (e.g. using Internet Identity). However, from a security perspective IMO such a property should be enforced also in the canister. The canisters are not necessarily always used with a front-end. And essentially in every canister we've reviewed, the developers do not restrict account creation to self-authenticating principals. So I think it would be good if the dev is forced to choose what kind of principal they want when reading the caller.

Please see also the security best practice for context where this discussion comes from.

nomeata commented 2 years ago

And essentially in every canister we've reviewed, the developers do not restrict account creation to self-authenticating principals

But that’s a good thing, at least partly! One of the (many) visions for the Internet Computer is the “strong canister principle”. Anything that you can do from the outside with the IC you should also be able to do from a canister. For automation, mash-ups, etc.

If we now encourage developers to restrict access to those principals that correspond to external users, they will (intentionally or accidentially) prevent canisters from using their services.

The fact that principals even have structure, and are not completely opaque, was a hacky work-around the fact that we didn’t want to store the data elsewhere (for decentralization etc.), now we face the consequences. Hence my resistence. . Maybe @rossberg can phrase these concerns more convincingly. Or maybe that ship has sailed anyways…

robin-kunzler commented 2 years ago

@nomeata : Thanks, see your point. But there are security issues around this, such as what @ggreif describes:

The problem is that some person A, accidentally logged in as anonymous and creates an account with sensitive information (e.g. telephone number or birthday). Then another user B makes the same mistake and suddenly sees the profile of A. Leaking out information is bad.

This can get even more severe if money was involved, e.g. thinking of NFTs or token transfers.

Is this no issue from your perspective? If you agree this is an issue, do you have suggestions on how one could address it in a different way than proposed here?

nomeata commented 2 years ago

I could accede that the “anonymous” principal is more special, and expose the caller as ? principal – this preserves the idea that a canister should not care about who is invoking it, but may care about if there is someone.

But I indeed do not think it’s a serious issue. The premise of “logged in as the anonymous user” is already flawed – if your front-end has a notion of “logged in”, but doesn’t actually log you in or fails to sign the requests, then… it just seems buggy. But I understand the sentiment to make such bugs easier to detect.

(There was a time before we had anonymous requests, and one possible design then would have been to keep the system simple, and leave it completely to agents to still sign such requests, possibly with a random, or a particular hard-coded key. If we’d taken that route, probably nobody would have thought of bothering the canisters with the concept.)

robin-kunzler commented 2 years ago

I could accede that the “anonymous” principal is more special, and expose the caller as ? principal – this preserves the idea that a canister should not care about who is invoking it, but may care about if there is someone.

@nomeata : I like your suggestion (just addressing the special case of the anonymous principal), since it solves the concern from my perspective. WDYT @crusso ?

But I indeed do not think it’s a serious issue. The premise of “logged in as the anonymous user” is already flawed – if your front-end has a notion of “logged in”, but doesn’t actually log you in or fails to sign the requests, then… it just seems buggy. But I understand the sentiment to make such bugs easier to detect.

I agree, the issues we saw around this were usually 'low' risk or sometimes 'medium' risk. The reason for bringing this up is not the severity of the individual issues, but the fact that we have seen issues around this multiple times. I also agree that such a frontend would be buggy (and we have seen such bugs!). I think the above solution improves the situation as a 'defense in depth' mechanism: