Merge pull request #1015 from 0xProject/feature/contracts/removeCallerSigType

Remove SignatureType.Caller
This commit is contained in:
Amir Bandeali
2018-08-24 15:09:39 -07:00
committed by GitHub
9 changed files with 67 additions and 187 deletions

View File

@@ -37,7 +37,7 @@ contract Whitelist is
bytes internal TX_ORIGIN_SIGNATURE;
// solhint-enable var-name-mixedcase
byte constant internal VALIDATOR_SIGNATURE_BYTE = "\x06";
byte constant internal VALIDATOR_SIGNATURE_BYTE = "\x05";
constructor (address _exchange)
public

View File

@@ -48,14 +48,16 @@ contract MixinSignatureValidator is
)
external
{
require(
isValidSignature(
hash,
signerAddress,
signature
),
"INVALID_SIGNATURE"
);
if (signerAddress != msg.sender) {
require(
isValidSignature(
hash,
signerAddress,
signature
),
"INVALID_SIGNATURE"
);
}
preSigned[hash][signerAddress] = true;
}
@@ -172,22 +174,6 @@ contract MixinSignatureValidator is
isValid = signerAddress == recovered;
return isValid;
// Implicitly signed by caller.
// The signer has initiated the call. In the case of non-contract
// accounts it means the transaction itself was signed.
// Example: let's say for a particular operation three signatures
// A, B and C are required. To submit the transaction, A and B can
// give a signature to C, who can then submit the transaction using
// `Caller` for his own signature. Or A and C can sign and B can
// submit using `Caller`. Having `Caller` allows this flexibility.
} else if (signatureType == SignatureType.Caller) {
require(
signature.length == 0,
"LENGTH_0_REQUIRED"
);
isValid = signerAddress == msg.sender;
return isValid;
// Signature verified by wallet contract.
// If used with an order, the maker of the order is the wallet contract.
} else if (signatureType == SignatureType.Wallet) {
@@ -225,34 +211,6 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.PreSigned) {
isValid = preSigned[hash][signerAddress];
return isValid;
// Signature from Trezor hardware wallet.
// It differs from web3.eth_sign in the encoding of message length
// (Bitcoin varint encoding vs ascii-decimal, the latter is not
// self-terminating which leads to ambiguities).
// See also:
// https://en.bitcoin.it/wiki/Protocol_documentation#Variable_length_integer
// https://github.com/trezor/trezor-mcu/blob/master/firmware/ethereum.c#L602
// https://github.com/trezor/trezor-mcu/blob/master/firmware/crypto.c#L36
} else if (signatureType == SignatureType.Trezor) {
require(
signature.length == 65,
"LENGTH_65_REQUIRED"
);
v = uint8(signature[0]);
r = signature.readBytes32(1);
s = signature.readBytes32(33);
recovered = ecrecover(
keccak256(abi.encodePacked(
"\x19Ethereum Signed Message:\n\x20",
hash
)),
v,
r,
s
);
isValid = signerAddress == recovered;
return isValid;
}
// Anything else is illegal (We do not return false because

View File

@@ -36,12 +36,10 @@ contract MSignatureValidator is
Invalid, // 0x01
EIP712, // 0x02
EthSign, // 0x03
Caller, // 0x04
Wallet, // 0x05
Validator, // 0x06
PreSigned, // 0x07
Trezor, // 0x08
NSignatureTypes // 0x09, number of signature types. Always leave at end.
Wallet, // 0x04
Validator, // 0x05
PreSigned, // 0x06
NSignatureTypes // 0x07, number of signature types. Always leave at end.
}
/// @dev Verifies signature using logic defined by Wallet contract.

View File

@@ -281,32 +281,6 @@ describe('MixinSignatureValidator', () => {
expect(isValidSignature).to.be.false();
});
it('should return true when SignatureType=Caller and signer is caller', async () => {
const signature = ethUtil.toBuffer(`0x${SignatureType.Caller}`);
const signatureHex = ethUtil.bufferToHex(signature);
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signerAddress,
signatureHex,
{ from: signerAddress },
);
expect(isValidSignature).to.be.true();
});
it('should return false when SignatureType=Caller and signer is not caller', async () => {
const signature = ethUtil.toBuffer(`0x${SignatureType.Caller}`);
const signatureHex = ethUtil.bufferToHex(signature);
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signerAddress,
signatureHex,
{ from: notSignerAddress },
);
expect(isValidSignature).to.be.false();
});
it('should return true when SignatureType=Wallet and signature is valid', async () => {
// Create EIP712 signature
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
@@ -440,53 +414,6 @@ describe('MixinSignatureValidator', () => {
expect(isValidSignature).to.be.false();
});
it('should return true when SignatureType=Trezor and signature is valid', async () => {
// Create Trezor signature
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const orderHashWithTrezorPrefixHex = signatureUtils.addSignedMessagePrefix(orderHashHex, SignerType.Trezor);
const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex);
const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey);
// Create 0x signature from Trezor signature
const signature = Buffer.concat([
ethUtil.toBuffer(ecSignature.v),
ecSignature.r,
ecSignature.s,
ethUtil.toBuffer(`0x${SignatureType.Trezor}`),
]);
const signatureHex = ethUtil.bufferToHex(signature);
// Validate signature
const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signerAddress,
signatureHex,
);
expect(isValidSignature).to.be.true();
});
it('should return false when SignatureType=Trezor and signature is invalid', async () => {
// Create Trezor signature
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const orderHashWithTrezorPrefixHex = signatureUtils.addSignedMessagePrefix(orderHashHex, SignerType.Trezor);
const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex);
const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey);
// Create 0x signature from Trezor signature
const signature = Buffer.concat([
ethUtil.toBuffer(ecSignature.v),
ecSignature.r,
ecSignature.s,
ethUtil.toBuffer(`0x${SignatureType.Trezor}`),
]);
const signatureHex = ethUtil.bufferToHex(signature);
// Validate signature.
// This will fail because `signerAddress` signed the message, but we're passing in `notSignerAddress`
const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
notSignerAddress,
signatureHex,
);
expect(isValidSignature).to.be.false();
});
it('should return true when SignatureType=Presigned and signer has presigned hash', async () => {
// Presign hash
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
@@ -520,6 +447,42 @@ describe('MixinSignatureValidator', () => {
);
expect(isValidSignature).to.be.false();
});
it('should return true when message was signed by a Trezor One (firmware version 1.6.2)', async () => {
// messageHash translates to 0x2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b
const messageHash = ethUtil.bufferToHex(ethUtil.toBuffer('++++++++++++++++++++++++++++++++'));
const signer = '0xc28b145f10f0bcf0fc000e778615f8fd73490bad';
const v = ethUtil.toBuffer('0x1c');
const r = ethUtil.toBuffer('0x7b888b596ccf87f0bacab0dcb483124973f7420f169b4824d7a12534ac1e9832');
const s = ethUtil.toBuffer('0x0c8e14f7edc01459e13965f1da56e0c23ed11e2cca932571eee1292178f90424');
const trezorSignatureType = ethUtil.toBuffer(`0x${SignatureType.EthSign}`);
const signature = Buffer.concat([v, r, s, trezorSignatureType]);
const signatureHex = ethUtil.bufferToHex(signature);
const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
messageHash,
signer,
signatureHex,
);
expect(isValidSignature).to.be.true();
});
it('should return true when message was signed by a Trezor Model T (firmware version 2.0.7)', async () => {
// messageHash translates to 0x2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b
const messageHash = ethUtil.bufferToHex(ethUtil.toBuffer('++++++++++++++++++++++++++++++++'));
const signer = '0x98ce6d9345e8ffa7d99ee0822272fae9d2c0e895';
const v = ethUtil.toBuffer('0x1c');
const r = ethUtil.toBuffer('0x423b71062c327f0ec4fe199b8da0f34185e59b4c1cb4cc23df86cac4a601fb3f');
const s = ethUtil.toBuffer('0x53810d6591b5348b7ee08ee812c874b0fdfb942c9849d59512c90e295221091f');
const trezorSignatureType = ethUtil.toBuffer(`0x${SignatureType.EthSign}`);
const signature = Buffer.concat([v, r, s, trezorSignatureType]);
const signatureHex = ethUtil.bufferToHex(signature);
const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync(
messageHash,
signer,
signatureHex,
);
expect(isValidSignature).to.be.true();
});
});
describe('setSignatureValidatorApproval', () => {

View File

@@ -1,4 +1,13 @@
[
{
"version": "1.0.1-rc.5",
"changes": [
{
"note": "Remove Caller and Trezor SignatureTypes",
"pr": 1015
}
]
},
{
"version": "1.0.1-rc.4",
"changes": [

View File

@@ -53,11 +53,6 @@ export const signatureUtils = {
return signatureUtils.isValidECSignature(prefixedMessageHex, ecSignature, signerAddress);
}
case SignatureType.Caller:
// HACK: We currently do not "validate" the caller signature type.
// It can only be validated during Exchange contract execution.
throw new Error('Caller signature type cannot be validated off-chain');
case SignatureType.Wallet: {
const isValid = await signatureUtils.isValidWalletSignatureAsync(
provider,
@@ -82,12 +77,6 @@ export const signatureUtils = {
return signatureUtils.isValidPresignedSignatureAsync(provider, data, signerAddress);
}
case SignatureType.Trezor: {
const prefixedMessageHex = signatureUtils.addSignedMessagePrefix(data, SignerType.Trezor);
const ecSignature = signatureUtils.parseECSignature(signature);
return signatureUtils.isValidECSignature(prefixedMessageHex, ecSignature, signerAddress);
}
default:
throw new Error(`Unhandled SignatureType: ${signatureTypeIndexIfExists}`);
}
@@ -293,10 +282,6 @@ export const signatureUtils = {
signatureType = SignatureType.EthSign;
break;
}
case SignerType.Trezor: {
signatureType = SignatureType.Trezor;
break;
}
default:
throw new Error(`Unrecognized SignerType: ${signerType}`);
}
@@ -306,7 +291,7 @@ export const signatureUtils = {
/**
* Combines the signature proof and the Signature Type.
* @param signature The hex encoded signature proof
* @param signatureType The signature type, i.e EthSign, Trezor, Wallet etc.
* @param signatureType The signature type, i.e EthSign, Wallet etc.
* @return Hex encoded string of signature proof with Signature Type
*/
convertToSignatureWithType(signature: string, signatureType: SignatureType): string {
@@ -333,12 +318,6 @@ export const signatureUtils = {
const prefixedMsgHex = ethUtil.bufferToHex(prefixedMsgBuff);
return prefixedMsgHex;
}
case SignerType.Trezor: {
const msgBuff = ethUtil.toBuffer(message);
const prefixedMsgBuff = hashTrezorPersonalMessage(msgBuff);
const prefixedMsgHex = ethUtil.bufferToHex(prefixedMsgBuff);
return prefixedMsgHex;
}
default:
throw new Error(`Unrecognized SignerType: ${signerType}`);
}
@@ -350,7 +329,7 @@ export const signatureUtils = {
*/
parseECSignature(signature: string): ECSignature {
assert.isHexString('signature', signature);
const ecSignatureTypes = [SignatureType.EthSign, SignatureType.EIP712, SignatureType.Trezor];
const ecSignatureTypes = [SignatureType.EthSign, SignatureType.EIP712];
assert.isOneOfExpectedSignatureTypes(signature, ecSignatureTypes);
// tslint:disable-next-line:custom-no-magic-numbers
@@ -361,11 +340,6 @@ export const signatureUtils = {
},
};
function hashTrezorPersonalMessage(message: Buffer): Buffer {
const prefix = ethUtil.toBuffer('\x19Ethereum Signed Message:\n' + String.fromCharCode(message.byteLength));
return ethUtil.sha3(Buffer.concat([prefix, message]));
}
function parseValidatorSignature(signature: string): ValidatorSignature {
assert.isOneOfExpectedSignatureTypes(signature, [SignatureType.Validator]);
// tslint:disable:custom-no-magic-numbers

View File

@@ -76,20 +76,6 @@ describe('Signature utils', () => {
);
expect(isValidSignatureLocal).to.be.true();
});
it('should return true for a valid Trezor signature', async () => {
dataHex = '0xd0d994e31c88f33fd8a572552a70ed339de579e5ba49ee1d17cc978bbe1cdd21';
address = '0x6ecbe1db9ef729cbe972c83fb886247691fb6beb';
const trezorSignature =
'0x1ce4760660e6495b5ae6723087bea073b3a99ce98ea81fdf00c240279c010e63d05b87bc34c4d67d4776e8d5aeb023a67484f4eaf0fd353b40893e5101e845cd9908';
const isValidSignatureLocal = await signatureUtils.isValidSignatureAsync(
provider,
dataHex,
trezorSignature,
address,
);
expect(isValidSignatureLocal).to.be.true();
});
});
describe('#isValidECSignature', () => {
const signature = {
@@ -270,15 +256,6 @@ describe('Signature utils', () => {
r: '0xaca7da997ad177f040240cdccf6905b71ab16b74434388c3a72f34fd25d64393',
s: '0x46b2bac274ff29b48b3ea6e2d04c1336eaceafda3c53ab483fc3ff12fac3ebf2',
};
it('should concatenate v,r,s and append the Trezor signature type', async () => {
const expectedSignatureWithSignatureType =
'0x1baca7da997ad177f040240cdccf6905b71ab16b74434388c3a72f34fd25d6439346b2bac274ff29b48b3ea6e2d04c1336eaceafda3c53ab483fc3ff12fac3ebf208';
const signatureWithSignatureType = signatureUtils.convertECSignatureToSignatureHex(
ecSignature,
SignerType.Trezor,
);
expect(signatureWithSignatureType).to.equal(expectedSignatureWithSignatureType);
});
it('should concatenate v,r,s and append the EthSign signature type when SignerType is Default', async () => {
const expectedSignatureWithSignatureType =
'0x1baca7da997ad177f040240cdccf6905b71ab16b74434388c3a72f34fd25d6439346b2bac274ff29b48b3ea6e2d04c1336eaceafda3c53ab483fc3ff12fac3ebf203';

View File

@@ -5,6 +5,10 @@
{
"note": "Add WalletError and ValidatorError revert reasons",
"pr": 1012
},
{
"note": "Remove Caller and Trezor SignatureTypes",
"pr": 1015
}
]
},

View File

@@ -133,23 +133,20 @@ export enum SignatureType {
Invalid,
EIP712,
EthSign,
Caller,
Wallet,
Validator,
PreSigned,
Trezor,
NSignatureTypes,
}
/**
* The type of the Signer implementation. Some signer implementations use different message prefixes (e.g Trezor) or implement different
* The type of the Signer implementation. Some signer implementations use different message prefixes or implement different
* eth_sign behaviour (e.g Metamask). Default assumes a spec compliant `eth_sign`.
*/
export enum SignerType {
Default = 'DEFAULT',
Ledger = 'LEDGER',
Metamask = 'METAMASK',
Trezor = 'TREZOR',
}
export enum AssetProxyId {