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

RFC: Ethereum attestation #557

Closed
wants to merge 8 commits into from
Closed

Conversation

CodeSandwich
Copy link
Contributor

@CodeSandwich CodeSandwich commented Mar 5, 2021

This is an RFC and a documentation for Ethereum attestation strategy. It's built on top of #525 and supports radicle-dev/radicle-upstream#965. The implementation of the smart contract is here: radicle-dev/radicle-contracts#129.

@CodeSandwich CodeSandwich added protocol Something concerning the core protocol rfc Request For Comments labels Mar 5, 2021
@kim
Copy link
Contributor

kim commented Mar 7, 2021

Is it feasible in practice to get a signature using the transaction key over some arbitrary data?

With this, we are back to a system which is only usable if:

  • upstream is installed, configured, and online
  • the web3 provider is available
  • the link identity is available and consistent (modulo the expiration window)

In other words, it’s an online verification. I understand how limited Ethereum is, but this just isn’t very useful.

@CodeSandwich
Copy link
Contributor Author

CodeSandwich commented Mar 7, 2021

@kim

get a signature using the transaction key over some arbitrary data

I'm sorry, but I'm not sure what do you mean by that, could you explain that? Ok, I get it, that wasn't my brightest moment :D

In other words, it’s an online verification.

Yes, it is and it can't be different to have trust in it. Unless you have a fresh view of both networks, you don't know if the other side still hasn't revoked its claim.

@NunoAlexandre
Copy link
Contributor

In other words, it’s an online verification.

Yes, it is and it can't be different to have trust in it. Unless you have a fresh view of both networks, you don't know if the other side still hasn't revoked its claim.

Ya, this was something we discussed with @cloudhead and agreed on the tradeoff.

but this just isn’t very useful.

How do you mean 'useful', @kim?

Copy link
Contributor

@NunoAlexandre NunoAlexandre left a comment

Choose a reason for hiding this comment

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

Looks fair to the plan to me 🍿

Before approving, I want to understand what Kim means by an online-only verification setup not being useful.

docs/rfc/ethereum_attestation.md Outdated Show resolved Hide resolved
docs/rfc/ethereum_attestation.md Outdated Show resolved Hide resolved
docs/rfc/ethereum_attestation.md Outdated Show resolved Hide resolved
docs/rfc/ethereum_attestation.md Outdated Show resolved Hide resolved
docs/rfc/ethereum_attestation.md Outdated Show resolved Hide resolved
@kim
Copy link
Contributor

kim commented Mar 8, 2021

I'm thinking that we didn't think through our threat model properly. Consider this:

Say we have established the link ethereum <-> radicle-link somehow. Now, an attacker gets access to the ethereum private key. What is she going to do about the attestation?

Nothing of course!

Funds are flowing in to the address because of the reputation of the off-chain identity, so there's no reason to change it to a different one. Ideally, the attacker finds a way to transfer funds away without anyone noticing, so the money keeps coming.

When this happens, and we still have access to the compromised key, then the effect of revoking the registration should be that transfers which get triggered by some other contract are cancelled, and it is no longer possible to set up new funding (unless one uses the address directly, of course). Revoking the off-chain identity is a weak defense, because it can't be checked on-chain. I'm not sure how things are set up, would this be possible?

Ok, what about the other way round: the off-chain identity is compromised? In this case, the attacker would want to change the ethereum address to one she has control over. Because she can't change the root hash (without losing the identity) and the mapping is 1:1, that's impossible. We can recover from this by pointing the ethereum account to a fresh off-chain identity.

One problem with this is that we shall under no circumstances lose the ethereum private key.

That's probably just generally a weakness of how ethereum accounts work, but to drive the point home: if we'd introduce expiration for the registration contract, the attacker who already controls the off-chain identity just needs to sit out the expiration window and register with a different account.


If this all makes sense, then it looks like we can spare ourselves to look up the registration from the chain tip just for verification. We only need to do this when we actually want to set up a transfer / funding contract. Which means that we could just store the transaction receipt (instead of only the hash) of the registration off-chain, and verify signatures offline.

Thoughts?

@CodeSandwich
Copy link
Contributor Author

The smart contracts aren't aware of attestations and even if they were, they wouldn't be able to verify them. That means that the off-chain software bears the full responsibility of checking the attestations and telling the smart contracts to do something in case of an emergency. Does it address your doubts?

@kim
Copy link
Contributor

kim commented Mar 8, 2021

That means that the off-chain software bears the full responsibility of checking the attestations and telling the smart contracts to do something in case of an emergency. Does it address your doubts?

No. If that responsibility is off-chain, then you either didn't understand what I wrote, or this feature is completely pointless and I'm vetoing it.


In order to claim an ID, call `claim` using your Ethereum account.
It will emit an event `Claimed`, which later can be queried to discover your attestation.
The claims have no expiration date and don't need to be renewed.
Copy link
Contributor

Choose a reason for hiding this comment

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

This means that the link can be revoked with only the ethereum account, right? Is that desirable? Basically, since the Link identity doesn't point to a specific attestation, it can be revoked from the ethereum side, without having the Link key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right, if you loose your Link key, you need to be able to revoke the attestation using only your Ethereum key.

Copy link
Contributor

Choose a reason for hiding this comment

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

The payload above has an expiration date. So this "claim" seems to be false 😏

@cloudhead
Copy link
Contributor

The transaction payload could be stored in Link, it would then be possible to verify the signature offline. Of course, you'd still eventually have to check that this transaction is included in a block, but at least you'd know that this ethereum key signed this radicle id?

@CodeSandwich
Copy link
Contributor Author

@cloudhead
Yes, we've discussed the offline verification. The problem with it is that unless you query the Ethereum client, you don't know if the claim has been published or if it was revoked. We came to a conclusion that a proof this weak isn't reliable enough to be useful and isn't worth the added complexity.

@kim
Copy link
Contributor

kim commented Mar 8, 2021 via email

@CodeSandwich
Copy link
Contributor Author

Instead, on-chain transactions which go through our contracts must check the claim status, and fail if there is none.

How could that be done? We could store on-chain a signature made with the Link key, but unless the Ethereum runtime can query Link identity chain, it wouldn't prove anything. The only actor, who can reasonably well tell, what's the state of the world is off-chain software and IMO that's where we should decide if an attestation is fine.

@CodeSandwich
Copy link
Contributor Author

After considering it, we could have a scheme, where we would keep Link signatures on-chain. We could let anybody revoke any of these signatures as long as the transaction holds a payload signed by the revoked Link key. In that case on-chain contracts would be able to tell if the attestation is complete as long as Link posts these revocations to both Ethereum and identity chain 🤔

@kim
Copy link
Contributor

kim commented Mar 8, 2021

Let me phrase it differently one more time: an ethereum address is just a key pair, right? So, if you can produce a signature, you can prove that you own it. A radicle-link root hash, however, is not a key pair -- this is by design, because otherwise you will have to keep the secret key in "secure offline storage", which people are notoriously bad at. Instead, we have a "survivable key compromise" scheme, where you can secure your identity with a k-of-n multisig.

The problem is that there is now no way I could think of which would allow a concise proof of ownership to be stored on-chain -- even if you put a multisig there, the set of keys that produced it may have been revoked in the meantime. This means that the only guarantee we have is that there is only one ethereum address which can claim a URN at any point in time. Because the reverse direction can't be verified on-chain, it is basically pointless: if you don't know the latest off-chain revision, you may still send money to the wrong address.

We said that anchoring with the goal of stopping progress if there is proof that a later revision of a link ID exists is a separate concern, and that it is not obvious how to solve it without a bulk-anchoring service. However, without solving it first, I do not see how the idea of attestations makes any sense.

@cloudhead
Copy link
Contributor

I suggest to drop this feature for now, and have it designed by a real
cryptographer.

How about the subset of this feature which is not a proof of ownership, but just a "profile link"?

Eg these claims:

  • "As the provable owner of this Radicle ID, this is my ethereum address"
  • "As the provable owner of this Ethereum account, this is my Radicle ID"

And basically we DON'T do the two-way attestation. It's just a one way link, which serves the product purpose?

@kim
Copy link
Contributor

kim commented Mar 8, 2021

Sure. This doesn't require any addition to the radicle-link protocol, though. The claim doesn't even need to be stored inside the identity doc, it only needs to be signed using one or more of the delegate keys. This has the added benefit of it being distributable in any way one would like.

@CodeSandwich
Copy link
Contributor Author

Where would you see the claim stored on Link side?

@kim
Copy link
Contributor

kim commented Mar 8, 2021 via email

@CodeSandwich
Copy link
Contributor Author

Ok, what are advantages and disadvantages of storing the claim there instead of the identity JSON?

@kim
Copy link
Contributor

kim commented Mar 8, 2021

You can publish it anywhere. If it wasn’t for the additional overhead of coming up with formal specifications of radicle “methods”, I’d even suggest to use a DID — they are exactly that: a way to claim different identities, where it is up to the consumer to verify the claims against their respective “methods”.

@kim
Copy link
Contributor

kim commented Mar 8, 2021

Ah scrap that, you’d want to retain a causal happens-before relationship to be able to revoke it. It’s still just previous_payload.with_ext(eth_addr).

@CodeSandwich
Copy link
Contributor Author

@kim It looks very flexible and powerful, I think that it's worth implementing at some point! I also like the solution, which lets you undoubtedly decide on-chain, if the attestation is right, it's the ultimate solution of all solutions.

For now maybe we should stay pragmatic and come up with something that is simple, easy to implement and fills just our current use case. For example right now we don't need to store the claim in multiple places, we only need to look it up with Link. I think that if we stick with extending the identity JSON, we can reuse many existing tools, file formats and behaviors:

  • we don't increase complexity with additional files
  • we can reuse most of the infrastructure for lookup in the identity file, also it's a logical place to store an Ethereum claim
  • we don't need to manage expirations separately
  • we don't need to design, who can update the Ethereum claim and when

It's by no means a perfect solution, but for now it seems good enough. We can deprecate it and gradually migrate when we need to design the right solution.

@kim
Copy link
Contributor

kim commented Mar 9, 2021

I am still not convinced that this is a good idea, because it is too easy to create a false sense of security. Anyways, the only security measure is expiration, which should be set rather low so as to force people to convince themselves that everything is still under their control. Because of this, I'd prefer the expiration timestamp to be local to the ethereum claim (ie. not apply to anything else within the identity document).

So, the whole thing can be done without any changes to librad, approximately like so:

pub type EthereumAddr = String; // TODO: make typesafe

// Note: goes through serde for now, but beware of JSON canonicalisation!
#[derive(serde::Serialize, serde::Deserialize)]
pub struct RadicleEthereumClaim {
    pub addr: EthereumAddr,
    pub expires: chrono::DateTime<Utc>,
}

lazy_static! {
    static ref RAD_ETH_NS: Url = Url::parse("https://radicle.xyz/ethereum/claim/v1").unwrap();
}

impl librad::identities::payload::HasNamespace for RadicleEthereumClaim {
    fn namespace() -> &'static Url {
        &RAD_ETH_NS
    }
}

// So we can remove by passing `None` to `payload.set_ext`
// (yeah right, because of first-order kindedness we can actually make this a blanket impl. lolrust)
impl librad::identities::payload::HasNamespace for Option<RadicleEthereumClaim> {
    fn namespace() -> &'static Url {
        &RAD_ETH_NS
    }
}

// use preconfigured default
let me = librad::identities::local::default(storage)?.unwrap();
let payload = me.payload.clone().with_ext(RadicleEthereumClaim {
    addr: "0xdeafbeef".to_owned(),
    expires: chrono::Utc::today().and_hms(0, 0, 0) + chrono::Duration::days(30),
});
let delegations = me.delegations.clone();
librad::identities::person::update(storage, me.urn(), me, payload, delegations)?;

JSON would look something like this:

{
    "version": 0,
    "replaces": "fcf10d74dd5de1e1a902b438c05a29de6a8c6958",
    "payload": {
        "https://radicle.xyz/link/identities/person/v1": {
            "name": "cloudhead"
        },
        "https://radicle.xyz/ethereum/claim/v1": {
            "addr": "0xdeafbeef",
            "expires": "2021-04-08T00:00:00Z"
        }
    },
    "delegations": [
        "hyn4hnppkiu61kpx91o9n5jtj37brujcgj7yp8d1derwz4fbk3tqjw",
        "hyb8kud543qkfdxkge6ecj6zziuam6w6fqhujgebbfuufmpdxt5uok"
    ]
}

@CodeSandwich
Copy link
Contributor Author

CodeSandwich commented Mar 9, 2021

That looks quite simple already, which is great. What are the advantages of moving the Ethereum claim from the person identity to a separate payload? I've got a few issues with this approach:

  • IIUC due to lack of support for Ethereum claims in librad in order to use it in 3rd party (e.g. in Upstream) we need to either import yet another library (which we need to write first) or reimplement the logic there
  • we probably can't reuse the generic expiration logic built for identities
  • separate claims in identity have separate local expiration dates anyway, how is a separate payload better?

@kim
Copy link
Contributor

kim commented Mar 9, 2021

  1. There is no relation between RFC: Identity Proofs #525 and this one, they have very different semantics
  2. There is no support for expirations as of now, so there isn't anything to re-implement
  3. If and when RFC: Identity Proofs #525 gets implemented, it will be a separate library / tool either way
  4. This library is rather unlikely to ever include a web3 client (for "social proofs", a generic HTTP/S client suffices)
  5. I have expressed my concerns about this feature ad nauseam, but I'm told this is a "product need". Since "product" == Upstream (and in fact, the feature is only usable with Upstream as it stands), it probably just belongs there.
  6. Should there be a convincing solution to identity binding in the future, it will most likely work very differently to both RFC: Identity Proofs #525 and this one, but the old version will need to continue to work, possibly indefinitely. Instead of trying to guess from the shape of the payload which version applies, the code can just look for an unambiguous namespace.
  7. The whole point of user-defined top-level namespaces is to make things extensible, and be able to come up with breaking semantics in the future.

Is that enough? ;)

@CodeSandwich
Copy link
Contributor Author

Thank you, that makes sense, I'll update the proposal :)

@CodeSandwich
Copy link
Contributor Author

@kim @NunoAlexandre Updated

NunoAlexandre
NunoAlexandre previously approved these changes Mar 9, 2021
Copy link
Contributor

@NunoAlexandre NunoAlexandre left a comment

Choose a reason for hiding this comment

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

LGTM, just a nit to refer to the address field consistently.

docs/rfc/ethereum_attestation.md Outdated Show resolved Hide resolved
docs/rfc/ethereum_attestation.md Outdated Show resolved Hide resolved
NunoAlexandre
NunoAlexandre previously approved these changes Mar 9, 2021
Copy link
Contributor

@FintanH FintanH left a comment

Choose a reason for hiding this comment

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

A couple of things aren't clear in this RFC:

  1. Is this only regarding to Person documents?
  2. "Link ID" is used a lot, but I'm unsure as to what that's actually referring to. Is it a URN? Is it the Conent ID? Is it something else?

docs/rfc/ethereum_attestation.md Outdated Show resolved Hide resolved

In order to claim an ID, call `claim` using your Ethereum account.
It will emit an event `Claimed`, which later can be queried to discover your attestation.
The claims have no expiration date and don't need to be renewed.
Copy link
Contributor

Choose a reason for hiding this comment

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

The payload above has an expiration date. So this "claim" seems to be false 😏

To only revoke a claim without creating a new one, use ID `0`,
which is guaranteed to not match any existing identity.

Currently supported `version` values:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is version supposed to be in the payload above? Or where did it come from?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or would you use claim/v1 and claim/v2 URLs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The payload is the Link-stored data, the version is used only in Ethereum. It has no corelation with the Link payload URL versioning.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I missed it in the contract function :)

`000000000000000000000000fb3102b74d7254eed7f18a31a3ba1ea946bb1a99`
- `2` - an `id` is a SHA-256 root hash

We need to deploy an official instance of the `Claims` smart contract and
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we attest this? weh weh weh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't, there can be multiple instances deployed and it won't affect the overall security. If you start using a different instance, you probably won't notice other people's claims and they won't notice yours

Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly tongue-in-cheek :) But thanks for the clarification 😄

You need to verify that the given Ethereum address claims back the link ID,
see [Discovery from an Ethereum address](#discovery-from-an-ethereum-address).

## Revocation of an attestation
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be good to call out here that a revocation will only be as good as the propagation of the document in the network. What I mean is that if I revoke my address, I still have to wait for others to fetch those changes -- so could still end up transferring MONIEZ to the old address.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@CodeSandwich
Copy link
Contributor Author

The payload above has an expiration date. So this "claim" seems to be false smirk

The payload goes to Link identity JSON, the claim you're referring to is an Ethereum counterpart. The first has an expiration date, the latter not.

@FintanH
Copy link
Contributor

FintanH commented Mar 9, 2021

The payload goes to Link identity JSON, the claim you're referring to is an Ethereum counterpart. The first has an expiration date, the latter not.

Thanks for clarifying :)

@CodeSandwich
Copy link
Contributor Author

Is this only regarding to Person documents?

We've decided that yes, an Ethereum address can be connected only with a person. Maybe at some point it will make sense to add support for projects, but for now we wouldn't be sure what to do with that. I've added a clarification.

@CodeSandwich
Copy link
Contributor Author

@FintanH

"Link ID" is used a lot, but I'm unsure as to what that's actually referring to. Is it a URN? Is it the Conent ID? Is it something else?

Good point, I've updated the document to be more precise in context of the identities spec and its vocabulary. Now it should be clear, when we're using an identity root and when we're talking about an identity as an abstract concept of telling users apart.

@CodeSandwich CodeSandwich marked this pull request as ready for review March 11, 2021 09:28
@CodeSandwich CodeSandwich requested a review from a team as a code owner March 11, 2021 09:28
@CodeSandwich
Copy link
Contributor Author

This RFC is no longer about Link, I'm closing this PR and moving it to Upstream

@NunoAlexandre NunoAlexandre deleted the igor/rfc-ethereum-attestation branch March 11, 2021 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol Something concerning the core protocol rfc Request For Comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants