Resolved revoke ballot issue & added security fixes #53
Conversation
007vedant
commented
Mar 29, 2022
007vedant
commented
- implemented passcode based encryption and decryption of ballots using PyNaCl
- modified the casting vote workflow to resolve issue of revoke ballot being CPU intensive
- modified the db schema to support the new workflow and encryption
- added voter.salt (largeBinary), voter.key(largeBinary) attributes to voter table. - updated ballot.id and ballot.voter as GUID type - added a custom GUID type
- modified custom GUID class to UUID class - changed Voter.keys to Voter.ballot_ids(Longtext type) - updated Ballot.id to default UUID field - updated Ballot.voter to String field (holds str(uuid))
- modified elections_voting_page() to perform encryption on ballots with passcode - modified elections_edit() to perform decryption on ballots or alert if wrong passcode
|
New code looks OK, am going to test it out. |
elekto/controllers/elections.py
Outdated
| passcode = passcode.encode("utf-8") | ||
| kdf = pwhash.argon2i.kdf | ||
| salt = utils.random(pwhash.argon2i.SALTBYTES) | ||
| key = kdf(secret.SecretBox.KEY_SIZE, passcode, salt) | ||
| box = secret.SecretBox(key) |
There was a problem hiding this comment.
Move this out to a utility or security module and call that over here, the general idea is to have slim controllers and fat services and to create abstractions. And move all the encoding/decodings to a separate module.
elekto/controllers/elections.py
Outdated
| # decrypting encrypted ballot ids | ||
| kdf = pwhash.argon2i.kdf | ||
| passcode = F.request.form["password"].encode("utf-8") | ||
| key = kdf(secret.SecretBox.KEY_SIZE, passcode, voter.salt) | ||
| box = secret.SecretBox(key) |
There was a problem hiding this comment.
Repeated code, creates inconsistency and possible bugs.
elekto/controllers/elections.py
Outdated
| voter.ballot_ids = json.dumps( | ||
| enc_ballot_ids | ||
| ) # json serialized encrypted ballot ids |
There was a problem hiding this comment.
@jberkus can you take a look into this, @007vedant I think this is not what we want to do as per the discussion we had on slack
There was a problem hiding this comment.
Aren't we storing a UUID for the voter in the ballot.voter of all the votes and storing the encryption of that UUID in voter.encrypted_ballot instead of voter.ballot_ids?
There was a problem hiding this comment.
@kalkayan I'll do the required modifications as per our discussion.
There was a problem hiding this comment.
@kalkayan modifications done as per the review via latest updates
- changed Voter.ballot_id to LargeBinary type (encrypted byte string)
- added encryption for ballot.voter in ballot_id attribute of Voter schema - removed redundant code
- handles the encryption workflow in controllers/elections.py - provides APIs for getting encrypted and decrypted strings
|
- moved encryption module to elekto/core - updated exception handlign in decrypt()
|
|
You need to add pynacl to requirements.txt |
- stores UUID as stringified hex values
- added PyNacl
|
|
@kalkayan, wanna give your +1 here before we merge? |
|
We can merge this @jberkus. |
