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

Add assurances that a node can't modify the payload in a CREATE transaction #327

Closed
ttmc opened this issue May 23, 2016 · 12 comments
Closed
Assignees

Comments

@ttmc
Copy link
Contributor

ttmc commented May 23, 2016

Right now, when a client sends a payload to a node to create a new asset, the node can change the payload and sign that to create the asset/transaction that actually gets stored.

Sophisticated clients can do things to make such shenanigans detectable by other clients (e.g. by including their public key and a signature in the payload itself, and by sending two transactions, with the first one having the hash of the second payload).

We could make it easier for clients to rest assured that a node can't corrupt their payload before the node creates the asset:

  • Require the payload to include the public key of the client.
  • Add a client-provided payload signature to the transaction model.
  • Add two new steps to transaction validation: 1) Check that the public key of the original owner equals the public key in the payload, 2) Verify the client-provided signature of the payload.

The initial node can't change the client's public key (in both places): the client-provided signature would change.

The initial node can't change the payload only: the client-provided signature would change.

The initial node could change the client's public key and payload, but then it's basically a totally different transaction! The client could see that their transaction never ended up in the bigchain, report a problem, and send their transaction to a different node.

To Consider Next: What about TRANSFER transactions? They can also have a payload (but might not).

@ttmc ttmc changed the title Add assurances that a node can't modify the payload in a CREATE transactions Add assurances that a node can't modify the payload in a CREATE transaction May 23, 2016
@ttmc
Copy link
Contributor Author

ttmc commented Aug 29, 2016

Issue #347 is in the same vein as this one. I wouldn't say that one is a duplicate of the other, but whoever tackles one should look at both.

@TimDaub
Copy link
Contributor

TimDaub commented Oct 17, 2016

@ttmc Removing transform_create was handled in #634 and #590.

Since a CREATE transaction is now signed with the client's private key and since the bdb-driver is evaluating the correctness of the transaction's id that was sent to a node, a scenario like this is resolved.

If correct, please close this ticket.

@r-marques r-marques self-assigned this Oct 31, 2016
@r-marques
Copy link
Contributor

r-marques commented Oct 31, 2016

Proposal

This should address both this issue and #347

In the initial design of BigchainDB it was decided that only federation nodes could issue assets on behalf of the users. This seemed like a good idea at the time but we never added any additional logic or found a use case where this was helpful. Moreover we introduced transaction malleability and other problems #347 .

With the introduction of bigchaindb common one of the solutions was to have the clients themselves issue the assets. This way a client creates and signs a CREATE transaction and sends it to the network. This removes any transaction malleability problems and ensures the asset was created by the actual owner of a particular public key.

The problem now is that when creating an asset the client does not have any condition to fulfill however it still needs to provide a signature in the form of a fulfillment.

Since CREATE transactions are a special case here is how I propose we should handle the conditions and fulfillments:

  • The fulfillment will be an implicit fulfillment:
    • In the case there is only one owners_before the implicit condition will be to provide a signature of the transaction that verifies with the owners_before public key.
    • If there are multiple owners_before the implicit condition will be a threshold condition with a weight equal to the number of owners_before. On other words an m-m multisig.

Note that these are the same rules for default conditions that we already use for transfer transactions with single and multiple owners.

To be discussed:

  • Should the condition key be null since we are not actually using it?

@sohkai
Copy link
Contributor

sohkai commented Oct 31, 2016

What does it mean to have an owners_before for CREATE transactions (or even multiple ones)? Confused--shouldn't it not have any, since it's creating something new?

@r-marques
Copy link
Contributor

The owners_before is who is actually creating the asset, and who needs to provide a fulfillment. owners_after is to whom the asset will be assigned to.

Before when only the federation nodes could create assets owners_before was a federation node and owners_after was a client

@sohkai
Copy link
Contributor

sohkai commented Oct 31, 2016

Ahh... I see (@ttmc I think you were telling me this last week too).

Do we want to support cases where you can immediately register an asset for someone else? This runs into #347 with nodes being able to "steal" assets getting created (albeit now they'll be legitimate ones). Would it be much worse if we just mandated that the owners_before and owners_after in a CREATE are the same? If I'm minting assets, the provenance chain still makes sense if I'm the initial owner.

@r-marques
Copy link
Contributor

r-marques commented Oct 31, 2016

Do we want to support cases where you can immediately register an asset for someone else? This runs into #347 with nodes being able to "steal" assets getting created (albeit now they'll be legitimate ones).

This is a different problem from #347. In #347 the problem was that since the client didn't sign the transaction I could just ask a federation node to create an asset for any public key.
With this solution you are sure that whoever created the asset actually owns the owners_before public key (because it has to provide the signature).

Would it be much worse if we just mandated that the owners_before and owners_after in a CREATE are the same? If I'm minting assets, the provenance chain still makes sense if I'm the initial owner.

We could do this but then we would just be annoying since the only thing a client would need to do would be to create 2 transactions instead of one:

  1. Issue asset and assign it to me
  2. Transfer asset to other

Instead he can just do:

  1. me is issuing and asset for other

@ttmc
Copy link
Contributor Author

ttmc commented Oct 31, 2016

The proposal by @r-marques would work technically, as far as I can tell.

It retains the weirdness of having fulfillments in a CREATE transaction, which causes a lot of confusion.

Question: Why are there fulfillments in a CREATE transaction? What conditions are they fulfilling?

Answer: Implicit conditions...

So we'll have to explain those implicit conditions. That is doable, but I sense we'll get the same question over and over again, until we're sick of it.

I suggest something technically identical to @r-marques proposal, but easier to understand:

  1. Remove fulfillments from the CREATE Transaction data model. (Leave them in TRANSFER Transactions, of course.)
  2. Add a new submitter_pubkey to the Transaction data model (CREATE and TRANSFER), inside the transaction body.
  3. Add a submitter_signature to the end of the Transaction data model (CREATE and TRANSFER): the signature calculated by the submitter, using the private key corresponding to submitter_pubkey.

Why am I suggesting we add submitter_pubkey and submitter_signature to all transactions, even TRANSFER transactions? Because that would also solve issue #339 - Transaction malleability of transactions containing no signatures. (A TRANSFER transaction, today, will contain no signatures if it's only fulfilling a combination of hashlock and timelock conditions.)

This is easy to generalize to multiple submitters: submitter_pubkeys and submitter_signatures would be lists.

Fun fact: Long ago, before the days of conditions and fulfillments, the signature actually was outside the transaction (but still inside the Transaction object). (It was a node signature back then...)

Pragmatics

We could start by implementing @r-marques proposal.

Issue #339 would remain unresolved, as would the confusing fulfillments-in-create-transactions. Those two issues could be resolved by implementing what I described above.

@r-marques
Copy link
Contributor

Because that would also solve issue #339 - Transaction malleability of transactions containing no signatures. (A TRANSFER transaction, today, will contain no signatures if it's only fulfilling a combination of hashlock and timelock conditions.)

Honestly I don't think #339 is an issue.
Cryptoconditions is a flexible spec that allow us to program cryptographic conditions (for good or bad) that dictates how a transaction is verified. If a user decides that a condition does not need a signature then they should be aware that it may lead to transaction malleability. The most that we could do is enforce the use of signatures in conjunction with hashlocks, although I don't think we should restrict how users decide to use crypto conditions.

Regarding the rest I don't think that adding another transaction model, another name for a public key and another way to sign and verify signatures will reduce confusion, not to mention the added complexity to the code

@ttmc
Copy link
Contributor Author

ttmc commented Nov 1, 2016

@r-marques I agree that my proposal would require more changes. Maybe we can revisit it, or something like it, in the future, if necessary (e.g. if we find that fulfillments-in-create-transactions is causing a lot confusion and human error).

For now, your original proposal will work.

👍

@sbellem
Copy link
Contributor

sbellem commented Nov 1, 2016

It would be nice, when time allows, to revisit @ttmc's proposal.

@r-marques r-marques mentioned this issue Nov 9, 2016
3 tasks
@ttmc
Copy link
Contributor Author

ttmc commented Nov 14, 2016

Resolved by pull request #794 (which did more than implement divisible assets).

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

No branches or pull requests

5 participants