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

Review: comments welcome 📋 #1

Open
wants to merge 5 commits into
base: review
Choose a base branch
from
Open

Review: comments welcome 📋 #1

wants to merge 5 commits into from

Conversation

sandreae
Copy link
Member

@sandreae sandreae commented Apr 12, 2024

This PR is for collecting feedback/comments on the design document (add inline comments to the document).

Copy link

@mixmix mixmix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you write really well.

i'm half way through. I hope it's useful/ not annoying 😅


Peer-to-peer protocols such as p2panda use public-key cryptography and digital signatures to establish the identity of peers and the authenticity of messages replicated on a network. Any peer can verify that messages it receives were created by the claimed peer (public key), and that they have not been tampered with by any third parties. This allows peers to replicate messages freely, safe in the knowledge that if tampering occurs it can be detected.

What if we don't want to share data with anyone though? Even if we know it won't be tampered with, we may want a stricter system where an author has additional control over where their data can travel, and how peers are allowed to interact with it. If we understand "authorship" as equal to "ownership" then we can say that only an owner has "read" and "write" authority over the data they created ("write" authority being relevant to long-lived mutable data).
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the word "owner" loads dangerous expectations in a distrbuted digital space. Namely "i will now delete my record". Good luck, "owner"

Copy link
Member Author

@sandreae sandreae Apr 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I appreciate the issue with this naming for sure. I'll argue the point a little, but also consider other options.

To me it works here as introducing the ability to act as owner is exactly what we are trying to do with access control and capabilities. With this system the default behaviour is for the creator of a resource (now called owner) to not share it with anyone unless they explicitly give permission (hand out a capability). They own the right to decide how they distribute their resources, they can share it widely or only with some very close friends, and they can specify (via delegation) how others can interact with their resource. Of course the more you do this, the less control you have in malicious scenarios.

In terms of how we understand ownership in other areas of our lives, this seems quite aligned. I own a microphone, I share it with you, I'm still the owner although you have the microphone sometimes. (I'm only talking about understanding of the word here, not the actual mechanics of the system).


## Notable challenges

p2panda is a peer-to-peer protocol with first class support for offline-first functionality. We make very few assumptions about the nature of the network messages are carried on, delays and temporary partitions are expected (a property often referred to as delay tolerance) and messages may arrive out-of-order. Accounting for these properties presents some challenges when designing a suitable access control system.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it an access control system or, capability system? I am a noob but I thought they were different

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be using terminology incorrectly, but I understand access control to be the general functionality we want, and the means to doing this being a system of issuing capability tokens.

An access control list would be a different approach.

# Definitions

**peer:** a user possessing knowledge of the private key for an ed25519 signing keypair; publishes operations and syncs document state with other peers
**owner:** the peer who created a document
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we call this ´originalAuthor´ you could go fancy with progenitor

**owner:** the peer who created a document
**issuer:** the peer issuing authority via a capability
**receiver:** the peer receiving authority via a capability
**authorizer:** the peer authorizing a request or operation against a capability
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one confuses me. Is it a "verifier" of an operation? Is it possible for operations to be published which are "not valid".

i guess there are:

  • valid rpc/ requests
  • valid operations

requests often discuss "authorization".. but if an operation has been pulished (offline, elsewhere) authoring has already happened... So all you can do is verify? I am def bringing assumptions from my scuttlebutt implementation that may be wrong

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get caught onthis cos "authorize" is "can author". Authoring of messages in p2p context is not same as acceptance though

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"authorizer" is not related to the author of a resource, but the peer acting as the authority system at any moment. They look at operations they are sent, check that a capability exists which contains the required authority, and then chooses to accept or reject the operation accordingly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

**authorizer:** the peer authorizing a request or operation against a capability
**invoker** the peer attempting to use the authority a capability delegates (by publishing operations or syncing documents)
**document:** a uniquely identifiable resource which capabilities are associated with
**operation:** messages peers publish to the network containing changes to documents, require that suitable authority is demonstrated before being accepted by another peer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can messages be accepted "written to disk cos valid sig chain", but not accepted cos "failed capability verification"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, only operations with the required authority are accepted.

| `subject` | `PublicKey` | Yes | The initial (root) issuer in any capability chain |
| `action` | `String` | Yes | The action a capability gives authority to perform |
| `conditions` | `Conditions` | Yes | Conditions which restrict this capability |
| `not_before` | `Integer` | No | "Not before" UTC Unix Timestamp in seconds (valid from) |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 danger, I think this is a soft requirement and is just inviting attackers to lie?
you can catch them lying if you spot them "time travelling" (timestamps not monotonically increasing in causal DAG)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with expires I guess

Copy link
Member Author

@sandreae sandreae Apr 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not_before and expires are not related to operation timestamps (this is in `conditions) but the valid lifetime of a capability token. No peer (authorizer) should accept a request which depends on a capability which has expired (checking against system time), even if the operationd being authorized would have been accepted under the contained conditions.

The interplay between these two sets of timestamp restrictions is very interesting!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not_before and expires are measured against system clock when requests occur, from_timestamp and to_timestamp relate to a operation timestamps.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if you want operation restrictions to not rely on timestamps at all, the you can use from_seq and to_seq which relates to positions in an authors document contribution log.


### Empty conditions

If the conditions are empty then the action contained in the capability is authorized for any document owned by the `subject` &/ `issuer`.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confusing to me. The subject is a document typically, not an owner (peer/group)?

is &/ meant to symbolize something?

Copy link
Member Author

@sandreae sandreae Apr 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The subject is the owner peer or group, this is different from the UCAN spec where a subject can be the resource itself. Hmmm, that should be improved maybe (the naming) so that it is clearly not the same.

}
```

By including an `"expires"` field which contains a timestamp one day ahead of the `"to_timestamp"` condition we are allowing operations within the capabilities scope to arrive up to one day late, but no longer. After `"expires"` has passed we can reject updates from authors depending on this capability, even if they would have fallen within the authorised scope. Correct configuration of this behaviour requires an understanding of the networks you will be using and the degree of delay you wish to tolerate. Misconfiguration has the potential to artificially introduce network partitions, where one peer accepted operations which another never received, however the capability used has now expired. This "deadlock" is expected to be resolved when new capabilities are issued, or if the operations are due for garbage collection in any case.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use consistent language "one day later".

note that "ahead" is ambiguous here. Any time movement which uses a vague direction presumes a sense (direction) of the reader relative to time. e.g. Not everyone sees "i moved the event back" to mean th same thing !


### `not_before`

The UTC Unix Timestamp in milliseconds specifying at which point this capability becomes valid.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposal: don't use utc, use utc/1000
This is used in DID contexts. It's fine enough granularity (you don't need ms acuracy) and consistently saves bytes?

class Capability {
issuer = Anna
receiver = Billie
subject = Anna
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

authRoot? issuerRoot

i keep thinking subject is eg a document. I think the name could be clearer, but you may be going with other conventions ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good feedback, thanks 👍

README.md Outdated

# Validation

Validation of capabilities must be performed by an authorizer before a any new operation arriving at a peer, or any sync request, is authorized.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sync / synchronous / synchronization / synchronisation ?


## Principal Alignment

In delegation, the `receiver` field of every proof must match the `issuer` field of the capability being delegated to. This alignment must form a chain back to the `subject` for each resource.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a "proof"? Do you mean "capability"?

I understand I think you're describing a capability chain, but I read the first sentence multiple times and it didn't parse.

you're delegating to other capabilities?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the proof is the previous capability in the delegation chain. I can re-write this to be clearer, thanks.

@mixmix
Copy link

mixmix commented Apr 13, 2024

I notice your capabilities don't form a tangle (dag). I think that could help considerably with building and verifying the "capability document" ?

@sandreae
Copy link
Member Author

I notice your capabilities don't form a tangle (dag). I think that could help considerably with building and verifying the "capability document" ?

They can form a DAG for sure, by appending them all to the same document. We want to explore different ways of doing this (one per app space, one per "root" capability, etc...) so haven't specified it further here.

@sandreae
Copy link
Member Author

you write really well.

i'm half way through. I hope it's useful/ not annoying 😅

Thank you for the review 🙏 really useful to hear you responses, think there are several places I can improve wording. Let's continue discussion on the more knotty stuff.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
sandreae and others added 2 commits May 4, 2024 06:41
Co-authored-by: Tom Ward <[email protected]>
Co-authored-by: Tom Ward <[email protected]>
| Field | Type | Required | Description |
| ------------ | ----------------------------- | -------- | ------------------------------------------------------- |
| `issuer` | `PublicKey` | Yes | Public key of the issuing author |
| `receiver` | `PublicKey` / `GroupId` / `*` | Yes | The receiver of this capability |
Copy link
Member Author

@sandreae sandreae May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some further consideration and feedback I'm leaning hard towards removing any notion of groups from this specification. I still think it's the correct mechanism for modelling role based access-control in p2panda, but the capability system itself doesn't need to be aware of this. Membership of a particular group can be used to identify the initial set of capabilities a peer should be issued. Removal from a group would mean capabilities should no longer be renewed, and if supported, already issued capabilities should be revoked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants