Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
324 stars 205 forks source link

add and fully adopt "timer brand": identify a source of time without granting wakeup/repeater authority #5798

Open warner opened 2 years ago

warner commented 2 years ago

What is the Problem Being Solved?

Today we discussed an attack in which off-chain clients use the published "terms" of a contract to access the TimerService it is using, and then instruct that service to make a bunch of wakeup calls, or a high-frequency repeater, as a DoS vector.

"Time" in distributed software is a fuzzy thing at best, and we don't want to enshrine one particular notion of time as the "one true time". Also, since clocks (at least high-resolution ones) enable code to read from a timing-based covert channel, we certainly don't want confined code to get ambient access to a clock. I like our current approach in which time sources are represented as ocaps, with an object that grants "what time is it now?" queries as well as "wake me up in 10 minutes" commands. This also matches well with having multiple sources of time: you just get multiple timer service objects.

Contracts publish which notion of time they're using so that clients can correctly compare things like option expiration dates. If the contract says the option expires at time 1234 according to clock A, and I want to compose that with some other contract which makes a derivative right that expires at time 1230, I want to make sure the second contract is also using clock A, otherwise bad things will ensue. So we need clocks to be comparable, independent of the authority we're sharing.

So we need contracts to publish a description of the kind of TimerService they're using, but we don't want this to enable an outsider to actually use that service.

After reading https://github.com/Agoric/agoric-sdk/issues/3612#issuecomment-932358730 and https://github.com/Agoric/agoric-sdk/issues/3612#issuecomment-939534241 , I'm thinking that this is similar to the way a Brand object describes a token type, without (directly?) granting the authority to e.g. make new Purses of that type. (I'm not sure whether Brands are the right notion to use, or Issuers, but the vernacular usage of "Brand" feels appropriate).

If each TimerService had a related (powerless) TimerBrand or TimerIdentity object, then the contract terms could safely cite the brand. Or, better yet, whenever they needed to reference a time value, they could instead emit a { time, timerID } pair, in much the same way our Amount pattern combines a number (for "Nat" -type Amounts) and a Brand. Two Amounts with the same Brand are comparable (X > Y, X >= Y, X === Y), while Amounts with different Brands are not comparable. A client who asks contract A when the option expires, and wants to ensure that contract B's option lasts at least as long, will want to do expiryA <= expiryB, and that fits very nicely into the AmountMath utilities (which would throw an error if the compared values have different Brands).

Description of the Design

So I think the swingset-side task is:

Elsewhere within our chain (I'll make a separate ticket for this), we need to make sure the full-powered TimerService is unavailable outside the contracts that need it. That means:

A deeper change, which I think is interesting (but I don't know how much work would be involved) would be to define an Amount-like { time, timerID } structure, maybe using AmountMath.Nat to manipulate it, and change all timer-sensitive contracts (at least the Vault, which needs to compute interest on a particular schedule) to use them. That would remove the TimerID from the contract's terms, and instead include it in each time value that crosses its API.

Security Considerations

Preventing access to the TimerService is important to prevent DoS attacks that leverage the timer.

Test Plan

Simple unit test which fetches a TimerID twice and asserts they are identical, plus one that demonstrates the lack of setWakeup or other API methods on the ID object.

warner commented 2 years ago

cc @erights @Chris-Hibbert , especially about whether "Brand" is an accurate word for this authority-less unforgeable identifier

erights commented 2 years ago

Just FYI, there's a related previous attempt at https://github.com/Agoric/agoric-sdk/pull/3778/files?diff=split&w=1 . I wouldn't spend too much time on it though. I doesn't try to restrict the authority provided by the timer, but it does try to use the timer + time together to represent a labeled time object in much the same way that our amounts use an issuer brand + a value to represent a labeled value. Idea originally from @katelynsills .

erights commented 2 years ago

A deeper change, which I think is interesting (but I don't know how much work would be involved) would be to define an Amount-like { time, timerID } structure, maybe using AmountMath.Nat to manipulate it, and change all timer-sensitive contracts (at least the Vault, which needs to compute interest on a particular schedule) to use them. That would remove the TimerID from the contract's terms, and instead include it in each time value that crosses its API.

Exactly. The Timer/TimerBrand pair is closely analogous to the Issuer/Brand pair. One can ask a Timer for its TimerBrand. One cannot ask the TimerBrand for its Timer. However, one must be able to ask the TimerBrand "is this your Timer"? That way, a client like Zoe that has both a Timer and TimerBrand that allegedly agree can ensure that both the Timer and TimerBrand agree on the correspondence at least once. If they were both well behaved, the correspondence will be stable. If only one is well behaved, by definition it won't agree on the other. Only if they are jointly ill behaved, they can still damage only themselves and those who relied on them being well behaved.

Without this, a malicious Timer can claim that another Timer's TimerBrand is its own. Zoe does check the mutual agreement of Issuer and Brand before registering them. Zoe would need to do the same thing here.

The similarity of Time/TimerBrand/LabeledTime to Issuer/Brand/Amount finally proves the time-money equivalence conjecture.

erights commented 2 years ago

but I don't know how much work would be involved

That old PR has an interesting hack called sharedTimeAuthority it its timeMath.js file to ease a transition from the old bare bigint representations of time to the new labeled time. The TimeMath also shows that as long as we're transitioning to a labeled representation, we should take that opportunity to also distinguish absolute time from relative duration.

There seems to be enough good here to build on that starting from here might take less time than starting from scratch. OTOH, I would be scared of trying to squeeze such an effort into MN1. Does the motivating problem need to be solved for MN1?

warner commented 2 years ago

Ok, I'll build:

const { service, brand } = makeDurableTimerService(..); // two facets in same cohort
assert.equal(await E(service).getTimerBrand(), brand);
assert(await E(brand).isMyTimerService(service));

The existing setup pattern, timerService = await E(timerVat).createTimerService(timerDevice), will only return the service.. the caller will need to use getTimerBrand() to retrieve the brand and then publish it to the Board or whatever.

@erights any thoughts on the spelling of isMyTimerService()? I think that matches the ERTP brand.isMyIssuer() pattern.

erights commented 2 years ago

makeDurableTimerServiceKit returns the pair.

makeDurableTimerService is perhaps deprecated, but as you say, you can still get the brand from the timer.

Given that the noun (phrase) in question is "timer service", then yes, isMyTimerService is the correct name. If we can instead just call it "timer" then isMyTimer. Can we call it "timer" or did we already use up that name for something related?

erights commented 2 years ago

"timer service" is fine. With this change, we'll refer to this much less than "timer brand" anyway.

erights commented 2 years ago

@warner please let me know if you'd find https://github.com/Agoric/agoric-sdk/pull/5809 helpful. Thanks.

warner commented 2 years ago

FYI https://github.com/Agoric/agoric-sdk/tree/5668-vat-timer is my WIP branch, the timer service facets are set up here:

https://github.com/Agoric/agoric-sdk/blob/5668-vat-timer/packages/SwingSet/src/vats/timer/vat-timer.js#L653-L683

Chris-Hibbert commented 2 years ago

we should take that opportunity to also distinguish absolute time from relative duration.

The typescript already distinguishes duration (RelativeTime) from absolute (Timestamp). I don't know that those are the right names, but the distinction should be perpetuated.

If we can instead just call it "timer" then isMyTimer. Can we call it "timer" or did we already use up that name for something related?

I don't think there's a distinct thing that's a Timer. Zoe uses timer: as a label in exit rules, but it's a label for the thing we're discussing. I also see timeAuhority, but it's used interchangeably with timer and timerService. TimerWaker is used to refer to an object with a wake method.

warner commented 2 years ago

Yeah Time and TimeDelta is what my branch is using right now, both are BigInts but are logically distinct. Those are the names used by the python standard library (my usual source of wisdom).

I think TimerService is a useful concept for the largest collection of features, and is the current name. I've introduced clock as the attenuated facet that only offers get-current-time.

Chris-Hibbert commented 2 years ago

I like clock as the name of a read-only facet.

In my house, the calendar hanging near the clock has additional powers for scheduling, and is read-write.

zarutian commented 2 years ago

If #3657 ever gets implemented then analogous "condition checker brand" must be implemented for the same reasons stated in the opening comment of this issue.

warner commented 2 years ago

The new Timer Service now offers a getTimerBrand(), plus methods to verify the match between TimerService, Clock, and TimerBrand.

It does not yet accept or return branded Timestamps.. that's the next step.

Other next steps are to change Zoe and the Board and Terms and such, to carry around brands instead of more powerful objects. I think our ultimate plan was to have bootstrap register the chain-time TimerService with zoe. Then later, when a client submits an exit criteria with a branded timestamp, zoe can look up the matching TimerService and use it to schedule a wakeup to trigger the exit (or query the right Clock when something requests an exit and zoe needs to confirm that it's not too late). When time appears in other places (Terms?), clients can use the TimerBrand. In this scheme, timeouts or time-based exit critera don't need to include TimerBrand as a separate argument because it's already baked into the branded Timestamp. cc @erights @Chris-Hibbert

Chris-Hibbert commented 2 years ago

See #5799 for some of the follow-up.