Jose Toro submission review (Serokell test task)

jose-review.md

Review

Hi, Jose!

Please see the review for your submission.

Most part of review will be conveyed in terms of validating criteria.

Review process is designed to be interactive, so that we will point you to mistakes you made in your submission and propose you to resolve some of these. Criteria will be checked in blocks, you'll get rundown along with descriptions as result of each iteration.

Also, please note that from now on, latest test task description (along with archive) can be found here.

Off-criteria comments

1. Regarding comment in code:

should use all fields in transaction if attacker have the ability to send any data on the net, but is ok in this case

Well, attacker can initiate conversation with any node :). See latest description, Security model description is still messy, but at least elaborated.

2. Could you elaborate on your blockchain solution, are forks deeper than 1 possible? (It's hard for me to determine it from code)?

Validation criteria

Criterion 7: Transaction id is cryptographically insecure

1. If some hash function is used, it should be strong one.
2. If no hash is used, equivallent solution should be sufficiently secure

Note, that signature of transaction by ed25519 is not considered a secure solution unless you provide strong argument about that (that signature is unique, resulting value uncorrelated).

Criterion 8: It's possible to create transaction with any sender

Typically, transaction is secured by signature of Sender. If it is not being done or is being done in wrong way, this will allow Adversary to construct arbitrary transaction and abuse Ledger.

Criterion 9: Double-send: repeated send of eavesdropped transaction

If an adversary eavesdrops transaction T = (ReceiverAddress, SenderAddress, Amount) from the conversation, he can send T to network again and this will cause T to be executed second time without intention by Sender (which will lead obviously to decrease of funds on Sender's account).

Criterion 21: Transaction may get lost

Assume all nodes remain online, network parameters (CONC, DELAY) not set, stabilityTimeot=60000.

Some valid transaction is submitted to a single node in cluster. After stabilityTimeout every node returns 0 on QUERY txId.

This criterion is validated by test: few transactions operating with different accounts A1, A2, A3, .. are simultaneously submitted to few different nodes of 8-node cluster (in potentially different order). As a result at least one of submitted transactions is completely lost after stabilityTimeout.

Hi George,

about your first concern, the security model stated that “Each node in the system behaves honestly at any given moment”, so Adversary should not be able to impersonate a node, so I only need to hide the SK from Adversary during the conversation, so sending the transfer amount encrypted is good enough, that and the fact I wanted to make it as fast as possible let me to that solution. Again, this solution is no good for a real case on open Internet where correct security should be use, I took this program as a toy implementation so I went for the hackathonish solution (barely covering requirements, fast and easy to code)

about my blockchain solution, maybe the names are not the best, rather than using the type:
type BlockChain = [(Ledger,[Block Transaction])]
the app works more like:
type BlockChain = (Ledger,[Block Transaction])
type ForkL = [BlockChain]
then replace all BlockChain for ForkL. This list will contain all forks on the blockchain where the first is the one on use, the function addChain compares two fork and decides which to use base on the time of the fork. I use the time of the fork to decide which fork to use because:
1- Each node in the system behaves honestly at any given moment
2- I did not wanted to implement things like worker nodes.
3- You only ask to make nodes agree after a certain time, and say nothing of how, and using time was easy.
If I had to implement a blockchain I would not use a Haskell list in production, its ok for a small task to show Haskell abilities. In production I would go for a SQL database and save transactions and blocks in it.
For last, forks deeper than one are possible, addFork has a function for going down and reversing ledger operations.

About the Criterions, are those things my program is not doing correctly? I thought (for this case, not the real world) that I satisfied then satisfactorily.

about your first concern, the security model stated that “Each node in the system behaves honestly at any given moment”, so Adversary should not be able to impersonate a node, so I only need to hide the SK from Adversary during the conversation, so sending the transfer amount encrypted is good enough, that and the fact I wanted to make it as fast as possible let me to that solution. Again, this solution is no good for a real case on open Internet where correct security should be use, I took this program as a toy implementation so I went for the hackathonish solution (barely covering requirements, fast and easy to code)

To be honest, we're actively thinking on how to rewrite the section in more straightforward way.
Now it barely looks like a concise model.
We say "Adversary can not impersonate node" b/c otherwise it would be harder to come up with proper consensus.
But at same time we wanted to allow Adversary to catch some part of conversation and repeat it with some changes of bytes.

So you're right in your assumption generally, but just imagine for a moment attacker is by some magic limited to only resend one message at whole lifetime... I don't know. Sorry, hate being imprecise/informal but can't easily get away from it.

About the Criterions, are those things my program is not doing correctly? I thought (for this case, not the real world) that I satisfied then satisfactorily.

Yeah, some tests I ran in my mind while reviewing your code. Automation for testing submissions of test task is not ready yet, so I do it manually and could be mistaken in my conclusions (you may argue).

Ok, I can see my solution failing (regarding security) if you allow attacker to resend, although it can be easily fix using all fields of transaction to generate the hash. But don’t change it, I kind of don’t want to fix it :).
Regarding criterion 7, I just picked the first hash function that gave me the required number of bytes that could be read be human (easier to test code that way), a version that return human unreadable bytes would be better but I don’t really think your test will exhaust the first one.
Could you elaborate a bit on criterion 21, thanks.

few transactions operating with different accounts A1, A2, A3, .. are simultaneously submitted to few different nodes of 8-node cluster (in potentially different order). As a result at least one of submitted transactions is completely lost after stabilityTimeout.

Imagine this experiment.

Essentially if new block arrives at node N which had some older block (forming a fork), older block will be thrown away (useful transactions not preserved anywhere).