From 6a7114fdb4eb7da9e831fc02b66cac32cc9d0a18 Mon Sep 17 00:00:00 2001 From: Matt Condon Date: Tue, 17 Apr 2018 11:36:24 -0300 Subject: [PATCH] Bouncer contract implementation (#883) * feat: implement bouncer contracts * fix: rename Bouncer to SignatureBouncer --- contracts/ECRecovery.sol | 25 +++++- contracts/access/SignatureBouncer.sol | 90 +++++++++++++++++++ contracts/mocks/BouncerMock.sol | 22 +++++ contracts/mocks/ECRecoveryMock.sol | 17 +++- test/access/SignatureBouncer.test.js | 119 ++++++++++++++++++++++++++ test/helpers/hashMessage.js | 8 -- test/helpers/sign.js | 22 +++++ test/library/ECRecovery.test.js | 48 +++++++---- 8 files changed, 322 insertions(+), 29 deletions(-) create mode 100644 contracts/access/SignatureBouncer.sol create mode 100644 contracts/mocks/BouncerMock.sol create mode 100644 test/access/SignatureBouncer.test.js delete mode 100644 test/helpers/hashMessage.js create mode 100644 test/helpers/sign.js diff --git a/contracts/ECRecovery.sol b/contracts/ECRecovery.sol index 0c82a50d9..b2076f0e3 100644 --- a/contracts/ECRecovery.sol +++ b/contracts/ECRecovery.sol @@ -18,12 +18,16 @@ library ECRecovery { * @param hash bytes32 message, the hash is the signed message. What is recovered is the signer address. * @param sig bytes signature, the signature is generated using web3.eth.sign() */ - function recover(bytes32 hash, bytes sig) internal pure returns (address) { + function recover(bytes32 hash, bytes sig) + internal + pure + returns (address) + { bytes32 r; bytes32 s; uint8 v; - //Check the signature length + // Check the signature length if (sig.length != 65) { return (address(0)); } @@ -52,4 +56,21 @@ library ECRecovery { } } + /** + * toEthSignedMessageHash + * @dev prefix a bytes32 value with "\x19Ethereum Signed Message:" + * @dev and hash the result + */ + function toEthSignedMessageHash(bytes32 hash) + internal + pure + returns (bytes32) + { + // 32 is the length in bytes of hash, + // enforced by the type signature above + return keccak256( + "\x19Ethereum Signed Message:\n32", + hash + ); + } } diff --git a/contracts/access/SignatureBouncer.sol b/contracts/access/SignatureBouncer.sol new file mode 100644 index 000000000..ad0267de1 --- /dev/null +++ b/contracts/access/SignatureBouncer.sol @@ -0,0 +1,90 @@ +pragma solidity ^0.4.18; + +import "../ownership/Ownable.sol"; +import "../ownership/rbac/RBAC.sol"; +import "../ECRecovery.sol"; + +/** + * @title SignatureBouncer + * @author PhABC and Shrugs + * @dev Bouncer allows users to submit a signature as a permission to do an action. + * @dev If the signature is from one of the authorized bouncer addresses, the signature + * @dev is valid. The owner of the contract adds/removes bouncers. + * @dev Bouncer addresses can be individual servers signing grants or different + * @dev users within a decentralized club that have permission to invite other members. + * @dev + * @dev This technique is useful for whitelists and airdrops; instead of putting all + * @dev valid addresses on-chain, simply sign a grant of the form + * @dev keccak256(`:contractAddress` + `:granteeAddress`) using a valid bouncer address. + * @dev Then restrict access to your crowdsale/whitelist/airdrop using the + * @dev `onlyValidSignature` modifier (or implement your own using isValidSignature). + * @dev + * @dev See the tests Bouncer.test.js for specific usage examples. + */ +contract SignatureBouncer is Ownable, RBAC { + using ECRecovery for bytes32; + + string public constant ROLE_BOUNCER = "bouncer"; + + /** + * @dev requires that a valid signature of a bouncer was provided + */ + modifier onlyValidSignature(bytes _sig) + { + require(isValidSignature(msg.sender, _sig)); + _; + } + + /** + * @dev allows the owner to add additional bouncer addresses + */ + function addBouncer(address _bouncer) + onlyOwner + public + { + require(_bouncer != address(0)); + addRole(_bouncer, ROLE_BOUNCER); + } + + /** + * @dev allows the owner to remove bouncer addresses + */ + function removeBouncer(address _bouncer) + onlyOwner + public + { + require(_bouncer != address(0)); + removeRole(_bouncer, ROLE_BOUNCER); + } + + /** + * @dev is the signature of `this + sender` from a bouncer? + * @return bool + */ + function isValidSignature(address _address, bytes _sig) + internal + view + returns (bool) + { + return isValidDataHash( + keccak256(address(this), _address), + _sig + ); + } + + /** + * @dev internal function to convert a hash to an eth signed message + * @dev and then recover the signature and check it against the bouncer role + * @return bool + */ + function isValidDataHash(bytes32 hash, bytes _sig) + internal + view + returns (bool) + { + address signer = hash + .toEthSignedMessageHash() + .recover(_sig); + return hasRole(signer, ROLE_BOUNCER); + } +} diff --git a/contracts/mocks/BouncerMock.sol b/contracts/mocks/BouncerMock.sol new file mode 100644 index 000000000..2add95874 --- /dev/null +++ b/contracts/mocks/BouncerMock.sol @@ -0,0 +1,22 @@ +pragma solidity ^0.4.18; + +import "../access/SignatureBouncer.sol"; + + +contract SignatureBouncerMock is SignatureBouncer { + function checkValidSignature(address _address, bytes _sig) + public + view + returns (bool) + { + return isValidSignature(_address, _sig); + } + + function onlyWithValidSignature(bytes _sig) + onlyValidSignature(_sig) + public + view + { + + } +} diff --git a/contracts/mocks/ECRecoveryMock.sol b/contracts/mocks/ECRecoveryMock.sol index 5abfb8c84..091055161 100644 --- a/contracts/mocks/ECRecoveryMock.sol +++ b/contracts/mocks/ECRecoveryMock.sol @@ -7,10 +7,19 @@ import "../ECRecovery.sol"; contract ECRecoveryMock { using ECRecovery for bytes32; - address public addrRecovered; - - function recover(bytes32 hash, bytes sig) public returns (address) { - addrRecovered = hash.recover(sig); + function recover(bytes32 hash, bytes sig) + public + pure + returns (address) + { + return hash.recover(sig); } + function toEthSignedMessageHash(bytes32 hash) + public + pure + returns (bytes32) + { + return hash.toEthSignedMessageHash(); + } } diff --git a/test/access/SignatureBouncer.test.js b/test/access/SignatureBouncer.test.js new file mode 100644 index 000000000..472a41226 --- /dev/null +++ b/test/access/SignatureBouncer.test.js @@ -0,0 +1,119 @@ + +import assertRevert from '../helpers/assertRevert'; +import { signHex } from '../helpers/sign'; + +const Bouncer = artifacts.require('SignatureBouncerMock'); + +require('chai') + .use(require('chai-as-promised')) + .should(); + +export const getSigner = (contract, signer, data = '') => (addr) => { + // via: https://github.com/OpenZeppelin/zeppelin-solidity/pull/812/files + const message = contract.address.substr(2) + addr.substr(2) + data; + // ^ substr to remove `0x` because in solidity the address is a set of byes, not a string `0xabcd` + return signHex(signer, message); +}; + +contract('Bouncer', ([_, owner, authorizedUser, anyone, bouncerAddress, newBouncer]) => { + before(async function () { + this.bouncer = await Bouncer.new({ from: owner }); + this.roleBouncer = await this.bouncer.ROLE_BOUNCER(); + this.genSig = getSigner(this.bouncer, bouncerAddress); + }); + + it('should have a default owner of self', async function () { + const theOwner = await this.bouncer.owner(); + theOwner.should.eq(owner); + }); + + it('should allow owner to add a bouncer', async function () { + await this.bouncer.addBouncer(bouncerAddress, { from: owner }); + const hasRole = await this.bouncer.hasRole(bouncerAddress, this.roleBouncer); + hasRole.should.eq(true); + }); + + it('should not allow anyone to add a bouncer', async function () { + await assertRevert( + this.bouncer.addBouncer(bouncerAddress, { from: anyone }) + ); + }); + + context('modifiers', () => { + it('should allow valid signature for sender', async function () { + await this.bouncer.onlyWithValidSignature( + this.genSig(authorizedUser), + { from: authorizedUser } + ); + }); + it('should not allow invalid signature for sender', async function () { + await assertRevert( + this.bouncer.onlyWithValidSignature( + 'abcd', + { from: authorizedUser } + ) + ); + }); + }); + + context('signatures', () => { + it('should accept valid message for valid user', async function () { + const isValid = await this.bouncer.checkValidSignature( + authorizedUser, + this.genSig(authorizedUser) + ); + isValid.should.eq(true); + }); + it('should not accept invalid message for valid user', async function () { + const isValid = await this.bouncer.checkValidSignature( + authorizedUser, + this.genSig(anyone) + ); + isValid.should.eq(false); + }); + it('should not accept invalid message for invalid user', async function () { + const isValid = await this.bouncer.checkValidSignature( + anyone, + 'abcd' + ); + isValid.should.eq(false); + }); + it('should not accept valid message for invalid user', async function () { + const isValid = await this.bouncer.checkValidSignature( + anyone, + this.genSig(authorizedUser) + ); + isValid.should.eq(false); + }); + }); + + context('management', () => { + it('should not allow anyone to add bouncers', async function () { + await assertRevert( + this.bouncer.addBouncer(newBouncer, { from: anyone }) + ); + }); + + it('should be able to add bouncers', async function () { + await this.bouncer.addBouncer(newBouncer, { from: owner }) + .should.be.fulfilled; + }); + + it('should not allow adding invalid address', async function () { + await assertRevert( + this.bouncer.addBouncer('0x0', { from: owner }) + ); + }); + + it('should not allow anyone to remove bouncer', async function () { + await assertRevert( + this.bouncer.removeBouncer(newBouncer, { from: anyone }) + ); + }); + + it('should be able to remove bouncers', async function () { + await this.bouncer.removeBouncer(newBouncer, { from: owner }) + .should.be.fulfilled; + }); + }); +}); diff --git a/test/helpers/hashMessage.js b/test/helpers/hashMessage.js deleted file mode 100644 index 1323f95eb..000000000 --- a/test/helpers/hashMessage.js +++ /dev/null @@ -1,8 +0,0 @@ -import utils from 'ethereumjs-util'; - -// Hash and add same prefix to the hash that ganache use. -module.exports = function (message) { - const messageHex = Buffer.from(utils.sha3(message).toString('hex'), 'hex'); - const prefix = utils.toBuffer('\u0019Ethereum Signed Message:\n' + messageHex.length.toString()); - return utils.bufferToHex(utils.sha3(Buffer.concat([prefix, messageHex]))); -}; diff --git a/test/helpers/sign.js b/test/helpers/sign.js new file mode 100644 index 000000000..30e894334 --- /dev/null +++ b/test/helpers/sign.js @@ -0,0 +1,22 @@ +import utils from 'ethereumjs-util'; + +/** + * Hash and add same prefix to the hash that ganache use. + * @param {string} message the plaintext/ascii/original message + * @return {string} the hash of the message, prefixed, and then hashed again + */ +export const hashMessage = (message) => { + const messageHex = Buffer.from(utils.sha3(message).toString('hex'), 'hex'); + const prefix = utils.toBuffer('\u0019Ethereum Signed Message:\n' + messageHex.length.toString()); + return utils.bufferToHex(utils.sha3(Buffer.concat([prefix, messageHex]))); +}; + +// signs message using web3 (auto-applies prefix) +export const signMessage = (signer, message = '', options = {}) => { + return web3.eth.sign(signer, web3.sha3(message, options)); +}; + +// signs hex string using web3 (auto-applies prefix) +export const signHex = (signer, message = '') => { + return signMessage(signer, message, { encoding: 'hex' }); +}; diff --git a/test/library/ECRecovery.test.js b/test/library/ECRecovery.test.js index 1d69837ab..7ea55823f 100644 --- a/test/library/ECRecovery.test.js +++ b/test/library/ECRecovery.test.js @@ -1,6 +1,13 @@ -var ECRecoveryMock = artifacts.require('ECRecoveryMock'); -var hashMessage = require('../helpers/hashMessage.js'); +import { + hashMessage, + signMessage, +} from '../helpers/sign'; +const ECRecoveryMock = artifacts.require('ECRecoveryMock'); + +require('chai') + .use(require('chai-as-promised')) + .should(); contract('ECRecovery', function (accounts) { let ecrecovery; @@ -16,8 +23,8 @@ contract('ECRecovery', function (accounts) { let message = web3.sha3(TEST_MESSAGE); // eslint-disable-next-line max-len let signature = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be89200'; - await ecrecovery.recover(message, signature); - assert.equal(signer, await ecrecovery.addrRecovered()); + const addrRecovered = await ecrecovery.recover(message, signature); + addrRecovered.should.eq(signer); }); it('recover v1', async function () { @@ -26,34 +33,45 @@ contract('ECRecovery', function (accounts) { let message = web3.sha3(TEST_MESSAGE); // eslint-disable-next-line max-len let signature = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e001'; - await ecrecovery.recover(message, signature); - assert.equal(signer, await ecrecovery.addrRecovered()); + const addrRecovered = await ecrecovery.recover(message, signature); + addrRecovered.should.eq(signer); }); it('recover using web3.eth.sign()', async function () { // Create the signature using account[0] - const signature = web3.eth.sign(accounts[0], web3.sha3(TEST_MESSAGE)); + const signature = signMessage(accounts[0], TEST_MESSAGE); // Recover the signer address from the generated message and signature. - await ecrecovery.recover(hashMessage(TEST_MESSAGE), signature); - assert.equal(accounts[0], await ecrecovery.addrRecovered()); + const addrRecovered = await ecrecovery.recover( + hashMessage(TEST_MESSAGE), + signature + ); + addrRecovered.should.eq(accounts[0]); }); it('recover using web3.eth.sign() should return wrong signer', async function () { // Create the signature using account[0] - const signature = web3.eth.sign(accounts[0], web3.sha3(TEST_MESSAGE)); + const signature = signMessage(accounts[0], TEST_MESSAGE); // Recover the signer address from the generated message and wrong signature. - await ecrecovery.recover(hashMessage('Test'), signature); - assert.notEqual(accounts[0], await ecrecovery.addrRecovered()); + const addrRecovered = await ecrecovery.recover(hashMessage('Test'), signature); + assert.notEqual(accounts[0], addrRecovered); }); it('recover should fail when a wrong hash is sent', async function () { // Create the signature using account[0] - let signature = web3.eth.sign(accounts[0], web3.sha3(TEST_MESSAGE)); + let signature = signMessage(accounts[0], TEST_MESSAGE); // Recover the signer address from the generated message and wrong signature. - await ecrecovery.recover(hashMessage(TEST_MESSAGE).substring(2), signature); - assert.equal('0x0000000000000000000000000000000000000000', await ecrecovery.addrRecovered()); + const addrRecovered = await ecrecovery.recover(hashMessage(TEST_MESSAGE).substring(2), signature); + addrRecovered.should.eq('0x0000000000000000000000000000000000000000'); + }); + + context('toEthSignedMessage', () => { + it('should prefix hashes correctly', async function () { + const hashedMessage = web3.sha3(TEST_MESSAGE); + const ethMessage = await ecrecovery.toEthSignedMessageHash(hashedMessage); + ethMessage.should.eq(hashMessage(TEST_MESSAGE)); + }); }); });