radixdlt / radixdlt-scrypto

Scrypto is the asset-oriented smart contract programming language of the Radix network. It allows you to quickly build secure and composable dApps.
https://developers.radixdlt.com/
Other
407 stars 121 forks source link

Re-presenting badges #106

Closed kroggen closed 2 years ago

kroggen commented 2 years ago

(from discussion on Discord)

Badges give access rights to the called method, and it would be able to do anything with it (presenting it) on the behalf of the caller... It can be useful, but also it is a lot of trust.


Maybe it could be made explicit whether a token can be re-presented by a component to other components.

I can think of scenarios where that such behavior would be really useful. A travel booking component might require me to present my KYC/identity badge but would then also present it to the airline, hotel and rental car booking components, which have a legitimate interest in knowing my identity.

On the other hand I would not want a misbehaving "travel booking" component taking out a loan somewhere using my identity.


Proposal:

The 1st component could specify to which other components it will re-present the badge so the user can check that before signing the transaction

It could be something similar to the #[auth(badge_ref)] decoration on a method.

#[auth(badge_ref)]
#[present_to(a1b2...7890 and c3d4...0987)]
pub fn do_something() {
    ...
}

Or using the same auth decorator:

#[auth(badge_ref -> a1b2...7890 and c3d4...0987)]
pub fn do_something() {
    ...
}

(the component addresses are abbreviated above, they should be the complete address)

Then:

Option 1: the transaction could include to which other components the badge is allowed to be presented

Option 2: the Radix Engine logic could control that by itself: once a function with that "decoration" is called, the marked badges for that function can only pre presented to the defined components.

devmannic commented 2 years ago

Thanks for starting this discussion @kroggen. IMHO this is quite relevant to securing component interactions and I think it's very important for the future of Scrypto to have a more nuanced BucketRef type.

Here are my thoughts on use cases and implementation:

Vocabulary: I'd call these transferable or non-transferable or partially transferable BucketRefs, with additional description based on how "far" they can go (ie. how many hops) and "where" they can go (ie. which packages/components they can reach)

I do not like the macro syntax with hardcoded addresses, it is extremely limited since it becomes baked into the ledger and is not generally known ahead of deployment time. Also I don't think the package should be the only place where a declaration of how a BucketRef will be used is made. This could be fixed by explicity making it part of the ABI so the BucketRef issuer has explicit guarantees. A macro could still support that by generated unique types though if that syntax is prefered. (Though I'd rather see better function arguments than macros generally speaking, but you can have both)

There are a few use cases I see:

  1. 0-hop: BadgeRef created within a component, that cannot be transferred outside the component (cannot be used in a cross-component call / or return. Uses:
    • minting/burning when Component controls the authorization with no intention of giving up control
  2. 1-hop: BadgeRef created to be passed cross-component but not "farther" than intended. Uses:
    • for authorization between components, including from the tx-initiator's Account to blueprint/component function/method call.
  3. multihop/generic: BadgeRef created as the most generic proof of contents of a Bucket (what we have today)
    • to show proof of a specific asset amount during the course of a transaction, can be used the same way as 0-hop or 1-hop but less safe.
    • used as proof of asset ownership, not necessarily for authorization. ie. a swap showing proof of liquidity prior so a component for in-transaction decision making. For best composability, unbounded hops are needed.

These "hops" would represent the "distance" between the current context of the underlying Bucket and the BucketRef. Or they represent the "distance" (in, say, call frames) from where the BucketRef is created to where it is allowed to be sbor Decoded. I'm not sure which is better.

I think you may want to further define where the 1-hop and multii-hop can go to:

There are 3 axes: packages X blueprints X component instances

  1. within a single package a single blueprint, single instance [pbi]
  2. within a single package, a single blueprint, multiple instances [bpI]
  3. within a single package, multiple blueprints, single instances [pBi]
  4. within a single package, multiple blueprints, multiple instances [pBI]
  5. within multiple packages, ... (may not need more granularity than this) [P..]

My preference would be to have multiple BucketRef types (either with generics or not) to encode the distance ("hops") and the allowed destination ("pbi"). It must be types, and not just values encoded inside the BucketRef struct because they need to be part of the ABI to give any guarantee to the Bucket owner. (Of course at the kernel level it may be implemented as extra metadata associated with a Rid and enforced by the Radix Engine runtime.)

For the most typical "auth" case used with the macro, the specific type would be elided in most cases and thus transparent anyway, but just provide extra security. For the more complex cases, the Bucket (or Vault) owner would want to specify when creating the Ref how it may be used. Then the receiving package/blueprint/component has to only specificy the correct type based on how they plan to use it in function/method args. Then, source code review gives assurances of the intent, and at runtime the Bucket owner has maximum control.

Interested in hearing @iamyulong and RDX Works' thoughts on this design. I think this has the potentially to really fill a gap in the current security model around authentication/authorization, and to do so while keeping with the asset-oriented design.

iamyulong commented 2 years ago

Thank you both for the proposed enhancements on Scrypto badge model!

I see the desire to have runtime-supported constraints on how BucketRefs are passed around, especially for the identify/KYC use case.

I've brought this up to the team for further discussion. More updates will be provided.

iamyulong commented 2 years ago

Hi, just a quick update on this. The team recognise the need to control where/how a BucketRef moves and explored different options (hop-based constraints, allowlist-based constraints and resourceDef-based constraints). This has a large impact on Scrypto lang, transaction and resim, and has a potential conflict with inner-component design. No implementation plan is made as of now as further thinking is required.

kroggen commented 2 years ago

Thanks for the update!

Why the allow-list is not suitable? (if you can share)

The idea would be to have it defined on the first called function, then the engine would control to which other components that badge could be transferred, checking either at each inter-component call or when the components try to use the badge.

Another option would be to have the list on the transaction. In this case the list could be on the first called function too, but the wallet would read that and include the list on the transaction. This case would make the transaction bigger though.

kroggen commented 2 years ago

The engine can get the allow-list on the outer-most called method. If there is no allow-list, it means that the badge can only be used on that component.

If the badges are always used with the authorize() and present() functions, then these functions could include the logic to control the presentation.

When the above functions are called they can get the current component address and check if they are on the list. If not, the transaction is cancelled.

0xOmarA commented 2 years ago

This has been fixed with v0.4.0 where proofs now become restricted after the first time they're transferred to a child call-frame. When a Proof is restricted, it can not be used to call methods on other components, and can not be put in the AuthZone.

I'm closing this issue since all proofs are now essentially single-hop. Feel free to reopen it if there is more discussion to be had here.