Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Support external_ballot_identifer as a string #71

Merged
merged 31 commits into from
Jan 28, 2020
Merged

Support external_ballot_identifer as a string #71

merged 31 commits into from
Jan 28, 2020

Conversation

AddressXception
Copy link
Contributor

@AddressXception AddressXception commented Jan 23, 2020 •
edited
Loading

Fixes #65

Checklist

🚨Please review the guidelines for contributing to this repository.

  • 🤔 CONSIDER adding a unit test if your PR resolves an issue.
  • ✅ DO check open PR's to avoid duplicates.
  • ✅ DO keep pull requests small so they can be easily reviewed.
  • ✅ DO build locally before pushing.
  • ✅ DO make sure tests pass.
  • ✅ DO make sure any new changes are documented.
  • ✅ DO make sure not to introduce any compiler warnings.
  • ❌AVOID breaking the continuous integration build.
  • ❌AVOID making significant changes to the overall architecture.

Description

This PR adds support for providing an external_ballot_identifer as a string. The identifier is saved as part of the encrypter ballot record on the encrypter device, and is then cached in a hash table when running through ballot registration, cast, and spoil.

We also add support to export/import the ballot from the encrypter device as that was missing from the existing implementation.

Additionally a few memory leaks are fixed in in bignum.c, crypto.c, decryption/coordinator.c, decryption/trustee.c, voting/coordinator.c, voting/encrypter.c, etc.

Improve debug logging. Add more api documentation. Modify MAX_BALLOT_PAYLOAD to 2000.

Explicitly add // TODO: to track remaining work.

Remaining Work

Note this PR is a work in progress and currently breaks the simple example.

  • Remove the simple_example
  • fix CI/CD integration
  • fix LG2M integration
  • update readme to reflect changes
  • instrument test run times on the console
  • verify on windows

To Test

  1. Run the api example
  2. Expected: the api example succeeds

@AddressXception
Support external_ballot_identifer as a string. Export/import ballots …
8071d5a 
…from encrypter

Add support for encrypter to export ballots to the file system.  Import encrypter ballots into coordinator.  Add a hashtable implementation that tracks ballot cast/spoil state against the external identifier (ballot_collection.c).  Support loading ballots into a buffer.  

Fix memory leaks in bignum.c, crypto.c, decryption/coordinator.c, decryption/trustee.c, voting/coordinator.c, voting/encrypter.c, etc.

Rework the API example to demonstrate the new external id and file system changes.

Improve debug logging.  Add more api documentation.  Modify MAX_BALLOT_PAYLOAD to 2000.  Explicitly add // TODO: to track remaining work.

Note this commit is a work in progress and currently breaks the simple example.
@AddressXception AddressXception added the draft Do not merge yet label Jan 23, 2020
@AddressXception
Copy link
Contributor Author

I apologize for the size of this PR. generally we should avoid making PR's so large

AddressXception
AddressXception commented Jan 23, 2020
View reviewed changes
examples/api/main.c Outdated Show resolved Hide resolved
@AddressXception AddressXception added this to the Phase 3 milestone Jan 23, 2020
@AddressXception
Remove Simple Example, add api doc stubs
9547fa6 
Add stubs for the api documentation.  Remove the simple example and use the api sample in the PR pipeline workflow
@AddressXception
Merge remote-tracking branch 'origin/master' into feature/ballot-iden…
35b58bb 
…tifier
@AddressXception
Fix Merge Issue
f5d5c63
@AddressXception
Verify malloc success
04ac827
@lgtm-com
Copy link

lgtm-com bot commented Jan 24, 2020

This pull request introduces 4 alerts when merging 04ac827 into bb04be9 - view on LGTM.com

new alerts:

  • 1 for Expression has no effect
  • 1 for Local variable hides global variable
  • 1 for Dubious NULL check
  • 1 for Use of potentially dangerous function

@lgtm-com
Copy link

lgtm-com bot commented Jan 24, 2020

This pull request introduces 2 alerts when merging e7d4f8b into bb04be9 - view on LGTM.com

new alerts:

  • 1 for Expression has no effect
  • 1 for Use of potentially dangerous function

@AddressXception
Document consumers resonsible for free
349a75f 
when calling an api that generates a filename as an out parameter, the caller is responsible for managing the memory of the out parameter when the call is successful
@AddressXception
Rename static coordinators in API to differentiate local vars
daec352 
add an underscore prefix to static coordinator instance handles to differentiate vars until #62 is complete
@AddressXception
add new enum cases to the end on voting coordinator
4a24656
@AddressXception
Add convenience logging
190dfd2
@AddressXception
Handle error cases on file access
818f8a9 
use convenience logging
@AddressXception
use consistent file formatting when rebuilding tracker string
67ea359
@AddressXception
optimize null check
aef1521
@AddressXception
explicitly set tracker array positions to NULL on failure
13a191b
@AddressXception
Reformat refactor Decryption_Coordinator_receive_share
54b5b80
@AddressXception
guard tallies_initialized when freeing resources
dd567e2 
explicitly initialize num_tallies
@AddressXception
rename _encrypted_ballot_count
707ddbe
@AddressXception
don't return early
ab0ba22
@AddressXception AddressXception mentioned this pull request Jan 27, 2020
Open
@lgtm-com
Copy link

lgtm-com bot commented Jan 27, 2020

This pull request introduces 5 alerts when merging ab0ba22 into 6aa4464 - view on LGTM.com

new alerts:

  • 3 for Comparison where assignment was intended
  • 1 for Expression has no effect
  • 1 for Use of potentially dangerous function

@lgtm-com
Copy link

lgtm-com bot commented Jan 27, 2020

This pull request introduces 5 alerts when merging 9f6fc27 into 6aa4464 - view on LGTM.com

new alerts:

  • 3 for Comparison where assignment was intended
  • 1 for Expression has no effect
  • 1 for Use of potentially dangerous function

@AddressXception
Copy link
Contributor Author

AddressXception commented Jan 27, 2020 •
edited
Loading

@raulgarciamsft PR feedback addressed to the extent it can be for this effort. Added #74 to address the result code issues you mentioned, and #75 to address error states in an API example. Just a couple of questions outstanding. Thanks for taking the time to look at this so thoroughly.

@lgtm-com
Copy link

lgtm-com bot commented Jan 27, 2020

This pull request introduces 2 alerts when merging 7a6a588 into 6aa4464 - view on LGTM.com

new alerts:

  • 1 for Expression has no effect
  • 1 for Use of potentially dangerous function

@AddressXception AddressXception added enhancement New feature or request and removed draft Do not merge yet labels Jan 27, 2020
@lgtm-com
Copy link

lgtm-com bot commented Jan 28, 2020

This pull request introduces 2 alerts when merging bc26137 into 6aa4464 - view on LGTM.com

new alerts:

  • 1 for Expression has no effect
  • 1 for Use of potentially dangerous function

@echumley-msft echumley-msft merged commit d9dd0a1 into master Jan 28, 2020
@echumley-msft echumley-msft deleted the feature/ballot-identifier branch January 28, 2020 14:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@echumley-msft echumley-msft echumley-msft approved these changes

@pamtaro pamtaro Awaiting requested review from pamtaro

@raulgarciamsft raulgarciamsft Awaiting requested review from raulgarciamsft

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
Phase 3
Development

Successfully merging this pull request may close these issues.

API_EncryptBallot accepts string identifier
4 participants
@AddressXception @pamtaro @raulgarciamsft @echumley-msft