-
Notifications
You must be signed in to change notification settings - Fork 107
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
[WIP] Add confidential prefix check utility #1059
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,25 @@ use ekiden_core::mrae::sivaessha2; | |
use super::{PrivateKeyType, PublicKeyType, EMPTY_PRIVATE_KEY, EMPTY_PUBLIC_KEY}; | ||
use sodalite; | ||
|
||
static PREFIX_LEN: usize = 12; | ||
|
||
pub fn is_encrypted(data: &[u8]) -> bool { | ||
if data.len() < PREFIX_LEN { | ||
return false; | ||
} | ||
let prefix = &data[..PREFIX_LEN]; | ||
// u8 representation of "confidential" | ||
let confidential_prefix = [99, 111, 110, 102, 105, 100, 101, 110, 116, 105, 97, 108]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be better to make this a module-level constant. |
||
return prefix == confidential_prefix; | ||
} | ||
|
||
pub fn remove_prefix(data: Vec<u8>) -> Vec<u8> { | ||
if data.len() < PREFIX_LEN { | ||
return data; | ||
} | ||
data[PREFIX_LEN..].to_vec() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this check that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It depends on the scheme we use. It should be fine to assume it’s the first couple of bytes. We just need to make sure the prefix chosen is not valid bytecode so that we don’t mistake a valid transaction for an encrypted one. Is there a specific reason why we might want the prefix to be anywhere in the ‘data’ that I’m missing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think @Yawning is saying that the prefix could be anywhere in the data, but that the function should maybe first check if Could return the data unchanged otherwise or even return an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see good point. I'll add the check. The assumption I should've stated was that |
||
} | ||
|
||
pub fn encrypt( | ||
plaintext: Vec<u8>, | ||
nonce: Vec<u8>, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we feel pretty good that this is otherwise going to be an invalid transaction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. We need a better guarantee, preferably in the form of an EIP or some existing standard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are known "reserve" or "invalid" evm byte code. can't we use those somehow creatively?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a link I can read?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ethereum/EIPs#191
namely:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, does the
data
blob include the CREATE/CALL instruction (e.g.,0xf0
or0xf1
) or is the create/call implicit and only the initcode/sighash+abi is in the data blob?If it is the latter this is more complicated as we need a value which at the same time:
There are some opcodes which are invalid (e.g., based on this it seems like e.g.
0x0c
is an invalid opcode). But of course a future revision could add opcodes in there. We could use some combination which is unlikely, e.g.0x000c0d636f...6c
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create/call is implicit based upon there existing a to address or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then something like some invalid bytecodes at the start and then some other specific value like
Web3c-Confidential
should be fine? For initcode the invalid opcodes should never appear and for a hash a specific constant is very very unlikely (but possible - can be made more unlikely by being longer).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it looks like parity uses the fact that WASM modules prepend the 4 byte prefix
\0asm
to know if it's dealing with WASM code. This prefix is assumed to be part of both the initcode and the bytecode of the contract. See https://github.com/oasislabs/parity/blob/ekiden/ethcore/src/factory.rs#L33.For confidential contracts we can do something similar.
For
CREATE
, say, in the web3c.js client, we can prepend\0pri
(or some other 4 byte prefix to be consistent with wasm's prefix) to thedata
field in the transaction.During the CREATE, in the runtime, we can check for the prefix, and when we finally store the bytecode of the contract (which drops the initcode), we can re-prepend the
\0pri
to the stored byte code. On all subsequentCALL
s, we can do the check for the prefix by looking at the account's stored code, and decrypt the data field if its spotted.This scheme is mainly different from the previous above conversation because, for CALLs, the
data
field itself doesn't have a prefix and is just encrypted. As a result, we can remove the possibility of the sighash colliding with our prefix. Furthermore, the\0
byte represents the STOP opcode in the EVM, and so during CREATE, we don't have to worry about a collision with valid bytecode either, since that won't ever be the first instruction.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this sounds good 👍